Add deterministic AST audit and comprehensive /testall runtime simulation with rotating logs and reports#140
Merged
gamblecodezcom merged 2 commits intoMar 6, 2026
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideImplements a deterministic AST-based audit pipeline and a comprehensive, admin-only /testall runtime simulation harness, including rotating logs, snapshot/rollback of in-memory stores, CI-safe wrappers, and synchronized documentation and dependency updates. Sequence diagram for the updated /testall end-to-end diagnostics flowsequenceDiagram
actor Admin
participant Telegram as TelegramServer
participant Bot as RunewagerBot_index_js
participant Logger as TestAllRotatingLogger
participant Snapshot as TestAllStateSnapshot
participant AST as scripts_ast_audit_js
participant Runtime as scripts_testall_runtime_js
participant FS as ReportsFilesystem
Admin->>Telegram: /testall command
Telegram->>Bot: update(message:/testall)
Bot->>Bot: isAdmin(ctx)
alt not admin
Bot-->>Telegram: ignore (no diagnostics)
else admin
Bot->>Logger: createTestAllRotatingLogger()
Logger-->>Bot: rotatingLogger
Bot->>Telegram: reply "Running /testall — full internal diagnostics started."
Bot->>Logger: log(info, testall_started)
Bot->>Snapshot: createTestAllStateSnapshot()
Snapshot-->>Bot: snapshot
Bot->>Bot: stub ctx.reply/sendMessage/etc to sink
Bot->>Bot: stub bot.telegram methods to in-process recorder
loop for each step
Bot->>Telegram: progress reply "Running test i/N — title"
Bot->>Bot: execute step function
alt AST audit step
Bot->>AST: runTestAllExternalAudit() via child_process.execFile
AST-->>Bot: writes ast_audit.json, structural_audit.md
else Command simulation step
Bot->>Bot: safeInvokeCommand for each BOT_KNOWN_COMMANDS
else Callback simulation step
Bot->>Runtime: read ast_audit.json, derive callback samples
Bot->>Bot: safeInvokeAction via bot.handleUpdate(callback_query)
else FSM/pagination/admin-flow steps
Bot->>Bot: simulate pagination/admin callbacks and commands
end
Bot->>Logger: log(step_started/step_finished)
end
Bot->>Snapshot: restoreTestAllStateSnapshot(snapshot)
Bot->>FS: write reports/testall_report.json|.md|.txt
Bot->>Logger: listLogArtifacts()
Logger-->>Bot: latest & retained testall.log* paths
Bot->>Bot: restore real bot.telegram methods
Bot->>Telegram: reply quick summary "TestAll complete — X passed, Y warnings, Z failures."
alt TESTALL_ADMIN_DM enabled and triggered from in-bot /testall
Bot->>Bot: escapeMarkdownV2(dmReport)
loop for each adminId in ADMIN_IDS
Bot->>Telegram: sendMessage(adminId, dmReportChunks, MarkdownV2)
end
end
Bot->>Logger: close()
end
Sequence diagram for scripts/testall_runtime.js deterministic simulationsequenceDiagram
participant CI as CI_or_Shell
participant Runtime as scripts_testall_runtime_js
participant Bot as RunewagerBot_index_js
participant FS as ReportsFilesystem
CI->>Runtime: node scripts/testall_runtime.js
Runtime->>Runtime: set env (CI, DISABLE_RUNTIME, TESTALL_EXPORTS, TESTALL_ADMIN_DM, ADMIN_IDS, BOT_TOKEN, DEVICE)
Runtime->>FS: read reports/ast_audit.json
alt missing or invalid AST report
Runtime-->>CI: stderr JSON { status:error, message }
Runtime->>Runtime: process.exit(1)
else AST OK
Runtime->>Bot: require('../index.js') (bot export)
Runtime->>Runtime: override process.exit to throw
Runtime->>Bot: stub bot.telegram.callApi
Runtime->>Bot: stub bot.telegram methods (sendMessage, editMessageText, answerCallbackQuery, etc.)
Runtime->>Runtime: derive commandSet from ast.commands
Runtime->>Runtime: derive callbackSet from callback emitters, actions, regex samples
loop for each command in commandSet
Runtime->>Bot: bot.handleUpdate(message:"/command")
Bot-->>Runtime: simulated handling
Runtime->>Runtime: record apiCalls, responsePreview, errors
end
loop for each callbackData in callbackSet
Runtime->>Bot: bot.handleUpdate(callback_query:data)
Bot-->>Runtime: simulated handling
Runtime->>Runtime: record apiCalls, fallbackHandled, responsePreview, errors
end
Runtime->>Runtime: collect asyncErrors from uncaughtException/unhandledRejection
Runtime->>FS: write reports/testall_report.json
Runtime->>FS: write reports/testall_report.md
Runtime->>FS: write reports/testall_report.txt
Runtime->>Runtime: restore original process.exit
Runtime-->>CI: stdout JSON { status, json, md, txt }
end
Class-style diagram for /testall diagnostics, snapshot/rollback, and logging componentsclassDiagram
class RunewagerBot_index_js {
+Set BOT_KNOWN_COMMANDS
+bot TelegrafBot
+command testall(ctx)
+runTestAllExternalAudit()
+createTestAllStateSnapshot()
+restoreTestAllStateSnapshot(snapshot)
+chunkTextForTelegram(text, limit)
+createTestAllRotatingLogger()
+isAdmin(ctx)
}
class TestAllRotatingLogger {
+String dir
+String latest
+Boolean usingFallback
+String sessionTag
+log(level, message, extra)
+listLogArtifacts()
+close()
}
class TestAllStateSnapshot {
+Array userEntries
+Object promoStore
+Number giveawayCounter
+Array giveawayRunning
+Array giveawayHistory
+Object tipsStore
+Array promoHistoryEntries
+Array smartButtonEntries
+Array sshvSessionEntries
}
class AstAuditScript {
+listFiles(dir)
+parseFile(file)
+getCalleeName(node)
+evalStaticString(node)
+collectCommandsActions()
+writeJsonReport()
+writeMarkdownReport()
}
class TestAllRuntimeScript {
+dispatchCommand(cmd)
+dispatchCallback(data)
+sampleFromRegex(regexLiteral)
+recordApi(method, payload)
+makeTelegramStub(name)
+generateSummary()
+writeJsonReport()
+writeMarkdownReport()
+writeTxtReport()
}
class RunTestAllShellWrapper {
+rotate_wrapper_logs()
+configureEnvForCI()
+invokeAstAudit()
+invokeRuntimeSim()
}
class data_logs_testall {
+String path = "data/logs/testall"
}
class reports_dir {
+String path = "reports"
}
RunewagerBot_index_js --> TestAllRotatingLogger : creates
RunewagerBot_index_js --> TestAllStateSnapshot : creates
RunewagerBot_index_js --> AstAuditScript : execFile
RunewagerBot_index_js --> TestAllRuntimeScript : execFile
TestAllRuntimeScript --> RunewagerBot_index_js : require bot (TESTALL_EXPORTS=1)
RunTestAllShellWrapper --> AstAuditScript : runs
RunTestAllShellWrapper --> TestAllRuntimeScript : runs
TestAllRotatingLogger --> data_logs_testall : writes
AstAuditScript --> reports_dir : writes
TestAllRuntimeScript --> reports_dir : writes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a self-contained TestAll diagnostic system: AST static audit, deterministic runtime command/callback simulation with mocked Telegram calls, state snapshot/rollback, rotating log retention, multi-format report artifacts, admin-only execution, and a CI-safe wrapper for deterministic runs. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant Bot as Bot (/testall)
participant State as State Snapshot Manager
participant AST as AST Audit
participant Runner as Runtime Harness
participant Logger as Rotating Logger
participant Reports as Report Writer
participant Telegram as Telegram API (mock)
Admin->>Bot: /testall
Bot->>Bot: isAdmin(ctx)
alt admin verified
Bot->>State: createTestAllStateSnapshot()
State-->>Bot: snapshot
Bot->>AST: runTestAllExternalAudit()
AST->>Reports: write structural audit
AST-->>Bot: audit results
Bot->>Runner: execute commands & callbacks
Runner->>Logger: open rotating logger
Runner->>Telegram: mock/send API calls (record)
Telegram-->>Runner: mock responses/errors
Runner->>Reports: write JSON/MD/TXT reports
Runner-->>Bot: results
Bot->>State: restoreTestAllStateSnapshot()
State-->>Bot: restored
Bot->>Bot: chunkTextForTelegram()
Bot->>Telegram: send admin DM (chunked)
Telegram-->>Bot: delivery ack
Bot-->>Admin: summary
else access denied
Bot-->>Admin: deny
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:XXL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In the
/testallhandler,reportData.summarynever definescommandsPassed,callbacksPassed, orwarnings, but those fields are later used to render the legacy summary string, so the quick summary will show incorrect/NaN values; consider computing pass/warning counts explicitly when buildingsummaryand referencing those instead. - The
/testallflow globally stubs and later restoresbot.telegram.*methods on the shared bot instance, which can affect concurrent updates while a long test run is in progress; it may be safer to run the harness in a separate process (only) or to isolate stubbing to a dedicated bot instance used solely for simulation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `/testall` handler, `reportData.summary` never defines `commandsPassed`, `callbacksPassed`, or `warnings`, but those fields are later used to render the legacy summary string, so the quick summary will show incorrect/NaN values; consider computing pass/warning counts explicitly when building `summary` and referencing those instead.
- The `/testall` flow globally stubs and later restores `bot.telegram.*` methods on the shared bot instance, which can affect concurrent updates while a long test run is in progress; it may be safer to run the harness in a separate process (only) or to isolate stubbing to a dedicated bot instance used solely for simulation.
## Individual Comments
### Comment 1
<location path="index.js" line_range="14208-14209" />
<code_context>
- `Duration: ${(durationMs / 1000).toFixed(1)}s | ${totalPassed}/${totalCount} checks passed`,
- `Summary: TestAll complete — ${totalPassed} passed, ${totalWarnings} warnings, ${totalFailed} failures.`,
+ if (originalCtxFns.reply) {
+ const totalPassed = reportData.summary
+ ? reportData.summary.commandsPassed + reportData.summary.callbacksPassed
+ : (reportData.commandResults.filter((r) => r.ok).length + reportData.callbackResults.filter((r) => r.ok && !r.skipped).length);
+ const totalWarnings = reportData.summary
</code_context>
<issue_to_address>
**issue (bug_risk):** Summary computation assumes fields that are never set, which can lead to NaN/undefined in the final /testall message.
Because `reportData.summary` is copied from `totals` and doesn’t define `commandsPassed`, `callbacksPassed`, or `warnings`, the ternary always takes the `summary` branch once it exists, producing `undefined + undefined` and `undefined`. These then surface as `NaN passed` / `undefined warnings` in the legacy summary. You’ll want to either populate `commandsPassed` / `callbacksPassed` / `warnings` when constructing `summary`, or avoid using `summary` here and always recompute these values from `commandResults` / `callbackResults` / `steps`.
</issue_to_address>
### Comment 2
<location path="docs/features/10-deploy-ops.md" line_range="144" />
<code_context>
+
+### Rotating log policy for `/testall`
+
+- `/testall` writes per-step progress and errors to rotating file stream: `data/logs/testall/testall.log`.
+- Rotation triggers: daily interval **or** size threshold (`10M`).
+- Rotated files are gzip compressed and old files are pruned automatically (`maxFiles` retention).
</code_context>
<issue_to_address>
**suggestion (typo):** Add an article before “rotating file stream” to make the sentence grammatically complete.
Consider phrasing this as “writes per-step progress and errors to a rotating file stream” for clearer grammar.
```suggestion
- `/testall` writes per-step progress and errors to a rotating file stream: `data/logs/testall/testall.log`.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+14208
to
+14209
| const totalPassed = reportData.summary | ||
| ? reportData.summary.commandsPassed + reportData.summary.callbacksPassed |
There was a problem hiding this comment.
issue (bug_risk): Summary computation assumes fields that are never set, which can lead to NaN/undefined in the final /testall message.
Because reportData.summary is copied from totals and doesn’t define commandsPassed, callbacksPassed, or warnings, the ternary always takes the summary branch once it exists, producing undefined + undefined and undefined. These then surface as NaN passed / undefined warnings in the legacy summary. You’ll want to either populate commandsPassed / callbacksPassed / warnings when constructing summary, or avoid using summary here and always recompute these values from commandResults / callbackResults / steps.
|
|
||
| ### Rotating log policy for `/testall` | ||
|
|
||
| - `/testall` writes per-step progress and errors to rotating file stream: `data/logs/testall/testall.log`. |
There was a problem hiding this comment.
suggestion (typo): Add an article before “rotating file stream” to make the sentence grammatically complete.
Consider phrasing this as “writes per-step progress and errors to a rotating file stream” for clearer grammar.
Suggested change
| - `/testall` writes per-step progress and errors to rotating file stream: `data/logs/testall/testall.log`. | |
| - `/testall` writes per-step progress and errors to a rotating file stream: `data/logs/testall/testall.log`. |
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+97
to
+101
| if (callee?.endsWith('.then')) { | ||
| const parent = p.parentPath?.node; | ||
| const chainedCatch = parent?.type === 'CallExpression' && getCalleeName(parent.callee)?.endsWith('.catch'); | ||
| if (!chainedCatch) data.promises.thenNoCatch.push({ file: rfile, line, callee }); | ||
| } |
There was a problem hiding this comment.
Suggestion: The detection of .then() calls without .catch() only inspects the immediate parent node, so normal promise chains like promise.then(...).catch(...) (where .catch is called on the result of .then) are incorrectly treated as missing a catch, filling the thenNoCatch report with false positives; the fix is to walk up through any MemberExpression wrappers to find an actual .catch() call in the chain before flagging it. [logic error]
Severity Level: Major ⚠️
- ⚠️ AST audit over-reports missing .catch on safe chains.
- ⚠️ /testall runtime summary mis-states asyncThenWithoutCatch count.
- ⚠️ Admins may chase non-issues in deploy diagnostics.
Suggested change
| if (callee?.endsWith('.then')) { | |
| const parent = p.parentPath?.node; | |
| const chainedCatch = parent?.type === 'CallExpression' && getCalleeName(parent.callee)?.endsWith('.catch'); | |
| if (!chainedCatch) data.promises.thenNoCatch.push({ file: rfile, line, callee }); | |
| } | |
| if (callee?.endsWith('.then')) { | |
| // Walk up past MemberExpression wrappers to see if this .then() is followed by a .catch() | |
| let cursor = p.parentPath; | |
| while (cursor && cursor.node.type === 'MemberExpression') { | |
| cursor = cursor.parentPath; | |
| } | |
| const maybeCall = cursor?.node; | |
| const chainedCatch = maybeCall?.type === 'CallExpression' && getCalleeName(maybeCall.callee)?.endsWith('.catch'); | |
| if (!chainedCatch) data.promises.thenNoCatch.push({ file: rfile, line, callee }); | |
| } |
Steps of Reproduction ✅
1. From the repo root (`/workspace/Runewager`), run the AST audit script `node
scripts/ast_audit.js` (entry point at `scripts/ast_audit.js:44-46`), which parses all
`*.js`/`*.ts` files and traverses them using the `CallExpression` visitor at
`scripts/ast_audit.js:74-102`.
2. During traversal, the visitor hits the Promise chain in `_globalSlot` inside
`rateLimiter.js` (shown in `rateLimiter.js:40-80` via tool read): `_globalTail =
_globalTail.then(async () => { ... }).catch(() => { /* Never break the global chain */
});`. For the inner `.then(...)` call, `getCalleeName` returns a string ending in `.then`,
so the condition at `scripts/ast_audit.js:97` (`if (callee?.endsWith('.then'))`) is
satisfied.
3. For that `.then` call, `p.parentPath.node` is a `MemberExpression` representing the
chained `.catch` call (`<CallExpression for .then>.catch`). The current implementation at
`scripts/ast_audit.js:98-99` only inspects this immediate parent; because it is a
`MemberExpression` (not a `CallExpression`), `parent?.type === 'CallExpression'` is false
and `chainedCatch` is computed as `false` even though the `.catch()` call is present one
level up in the chain.
4. As a result, the `.then` in `_globalSlot` (and similar chains like
`prevTail.then(...).catch(...)` in `enqueue` within `rateLimiter.js:40-80`) is incorrectly
pushed into `data.promises.thenNoCatch` at `scripts/ast_audit.js:100`. This propagates
into `reports/ast_audit.json` (`promises.thenNoCatch` array referenced in
`scripts/testall_runtime.js:275`) and surfaces in the `/testall` runtime summary as
`.then() without direct .catch()` counts, generating false positives for promise chains
that do have a `.catch()` attached.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/ast_audit.js
**Line:** 97:101
**Comment:**
*Logic Error: The detection of `.then()` calls without `.catch()` only inspects the immediate parent node, so normal promise chains like `promise.then(...).catch(...)` (where `.catch` is called on the result of `.then`) are incorrectly treated as missing a catch, filling the `thenNoCatch` report with false positives; the fix is to walk up through any `MemberExpression` wrappers to find an actual `.catch()` call in the chain before flagging it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| } | ||
|
|
||
| (async () => { | ||
| const commandSet = Array.from(new Set(ast.commands.map((c) => c.cmd && c.cmd.value).filter(Boolean))).sort(); |
There was a problem hiding this comment.
Suggestion: The runtime harness currently includes the /testall command in commandSet, so when it simulates all commands it will invoke the /testall handler defined in index.js, which in turn calls runTestAllExternalAudit() and spawns a new scripts/testall_runtime.js process again; when run via scripts/run_testall.sh (where ADMIN_IDS defaults to 1), this creates unbounded recursive /testall invocations and child processes instead of a single self-test run. Excluding testall from the simulated command set avoids this recursive self-invocation while still exercising all other commands. [logic error]
Severity Level: Critical 🚨
- ❌ `/testall` admin diagnostic spawns nested `testall_runtime` child processes.
- ❌ `scripts/run_testall.sh` CI wrapper risks long hangs/timeouts.
- ⚠️ `/testall` internal report generation becomes slow and error-prone.
- ⚠️ Deploy-ops diagnostics in `docs/features/10-deploy-ops.md` unreliable.
Suggested change
| const commandSet = Array.from(new Set(ast.commands.map((c) => c.cmd && c.cmd.value).filter(Boolean))).sort(); | |
| const commandSet = Array.from(new Set( | |
| ast.commands | |
| .map((c) => c.cmd && c.cmd.value) | |
| .filter((value) => value && value !== 'testall') // avoid recursively invoking /testall, which re-runs this harness | |
| )).sort(); |
Steps of Reproduction ✅
1. Generate the AST audit so the runtime harness has command metadata by running `node
scripts/ast_audit.js` from the project root. This writes `reports/ast_audit.json`, whose
`commands` section includes an entry for the `testall` command (`file: "index.js", line:
13831, value: "testall"`) as seen at `reports/ast_audit.json:880-919`.
2. Run the runtime simulator via the documented CI wrapper `scripts/run_testall.sh` (see
`scripts/run_testall.sh:35-55`, which sets `CI=true`, `DISABLE_RUNTIME=1`,
`TESTALL_EXPORTS=1` and then executes `node scripts/ast_audit.js` followed by `node
scripts/testall_runtime.js`), or directly invoke `node scripts/testall_runtime.js`.
3. Inside `scripts/testall_runtime.js`, the harness reads `reports/ast_audit.json` into
`ast` (lines 21-31) and builds `commandSet` from all discovered commands using the line in
question (`scripts/testall_runtime.js:228-229`), so `commandSet` contains `"testall"`
along with all other commands.
4. The harness iterates `commandSet` and, for each command, calls `dispatchCommand(cmd)`
(`scripts/testall_runtime.js:244-248`); for `cmd === 'testall'`, `dispatchCommand`
constructs a synthetic update with message text `"/testall"` and calls `await
bot.handleUpdate(...)` (`scripts/testall_runtime.js:142-156`), which routes to the real
`/testall` handler registered in `index.js:252-703`.
5. The `/testall` handler in `index.js` runs its first step, `"AST Audit Step"`, calling
`const externalAudit = await runTestAllExternalAudit();` (`index.js:478-482`).
`runTestAllExternalAudit` is defined at `index.js:29-68` and uses
`execFileAsync(process.execPath, [runtimeScript], { ..., env: envBase })` to spawn a new
`node scripts/testall_runtime.js` child process (`runtimeScript` is set to
`path.join(__dirname, 'scripts', 'testall_runtime.js')` at `index.js:38-52`).
6. That child process executes the same `scripts/testall_runtime.js` entrypoint, again
loading `../index.js` (which registers the `/testall` handler) and again building
`commandSet` from `ast.commands` including `"testall"`
(`scripts/testall_runtime.js:21-31`, `228-229`). When the child harness reaches
`"testall"` in its own `commandSet`, it repeats step 4 and step 5, causing another
`runTestAllExternalAudit()` call that spawns yet another `scripts/testall_runtime.js`
process.
7. Because there is no guard to stop `/testall` from invoking the external harness when
that harness is already running, this process recursion continues: each
`scripts/testall_runtime.js` instance eventually dispatches `/testall`, whose handler
(`index.js:252-703`) spawns another `scripts/testall_runtime.js` via
`runTestAllExternalAudit()`. The recursion only terminates when the `execFileAsync`
timeout (180 seconds as specified at `index.js:52`) is hit, causing the outer `/testall`
step to fail and the overall harness run to be slow and error-prone instead of completing
a single deterministic pass.
8. Excluding `"testall"` from `commandSet` (as in the improved code) keeps the runtime
harness from simulating the `/testall` command itself; `/testall` remains usable in the
real bot and continues to call `runTestAllExternalAudit()`, but the external harness will
no longer re-enter itself recursively through `/testall`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/testall_runtime.js
**Line:** 229:229
**Comment:**
*Logic Error: The runtime harness currently includes the `/testall` command in `commandSet`, so when it simulates all commands it will invoke the `/testall` handler defined in `index.js`, which in turn calls `runTestAllExternalAudit()` and spawns a new `scripts/testall_runtime.js` process again; when run via `scripts/run_testall.sh` (where `ADMIN_IDS` defaults to `1`), this creates unbounded recursive `/testall` invocations and child processes instead of a single self-test run. Excluding `testall` from the simulated command set avoids this recursive self-invocation while still exercising all other commands.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
qa/context/bot_capabilities.json (2)
434-434: Optional: make excerpt truncation explicit.
mapExcerptcurrently ends mid-token; adding an explicit suffix (e.g.,… [truncated]) would reduce ambiguity when reading QA artifacts.Based on learnings: Any added/removed flow in the Runewager bot must be reflected in
RUNEWAGER_FUNCTIONALITY_MAP.mdin the same change set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qa/context/bot_capabilities.json` at line 434, Update the "mapExcerpt" value in qa/context/bot_capabilities.json to explicitly indicate the excerpt is truncated by appending a suffix such as "… [truncated]" (so consumers know the text ends intentionally mid-document). While editing, verify any newly added or removed bot flows in your change are also reflected in RUNEWAGER_FUNCTIONALITY_MAP.md so the excerpt and source-of-truth file remain consistent.
2-2: Consider makinggeneratedAtoptional for committed artifacts.Using a wall-clock timestamp here creates constant non-functional diffs. If this file is checked into git, consider gating
generatedAtbehind an env flag (or omitting it) to keep artifact diffs deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qa/context/bot_capabilities.json` at line 2, Make the generatedAt field optional to avoid noisy diffs: change the artifact generation code that writes the JSON to only include the "generatedAt" key when an environment flag is set (e.g., INCLUDE_GENERATED_AT=true) or when running a non-committed/debug build; update the serializer that emits the "generatedAt" property so it checks process.env.INCLUDE_GENERATED_AT (or equivalent config) before adding the "generatedAt" key to the output JSON, and ensure committed artifacts omit this field by default.scripts/testall_runtime.js (1)
228-344: Restoreprocess.exitin afinallyblock.If the async runner throws before completion,
process.exitmay stay monkey-patched for the rest of the process lifetime.♻️ Proposed hardening
-(async () => { +(async () => { + try { const commandSet = Array.from(new Set(ast.commands.map((c) => c.cmd && c.cmd.value).filter(Boolean))).sort(); @@ - process.exit = originalProcessExit; console.log(JSON.stringify({ status: summary.status, json: reportJsonPath, md: reportMdPath, txt: reportTxtPath }, null, 2)); + } finally { + process.exit = originalProcessExit; + } })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/testall_runtime.js` around lines 228 - 344, The IIFE currently restores process.exit only at the end so an exception leaves the monkey-patch in place; wrap the main async body of the IIFE in try { ... } finally { process.exit = originalProcessExit; } (or add a finally block around the await dispatch loops and report generation) so that process.exit is always restored even if dispatchCommand/dispatchCallback or file writes throw; reference the existing identifiers process.exit, originalProcessExit, the top-level async () => { ... } IIFE, and the dispatchCommand/dispatchCallback/report write sections when making the change.reports/testall_report.md (1)
125-127: Callback table "Fallback" column may be misleading.The table shows ✅ in the "Fallback" column for all callbacks, but the JSON report shows
fallbackHandled: falsefor all entries. The checkmark appears to indicate "OK" status rather than "fallback was triggered."This is a generator output issue — if intentional, consider renaming the column to "Handled" or "Status" for clarity in
scripts/testall_runtime.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reports/testall_report.md` around lines 125 - 127, The report generator is rendering a misleading "Fallback" column with a checkmark regardless of fallbackHandled value; update the generator in scripts/testall_runtime.js so the table cell uses the actual fallbackHandled boolean from the JSON (show ✅ only when fallbackHandled === true) or rename the header to a neutral term like "Handled" or "Status" and adjust the cell text accordingly; locate the table-generation logic that emits the header string "Fallback" and the cell rendering for admin_auth_bypass (and other callbacks) and change the header or conditional rendering to reflect fallbackHandled accurately.scripts/run_testall.sh (1)
53-54: Node script execution relies onset -efor error handling.With
set -euo pipefailat the top, failures inast_audit.jsortestall_runtime.jswill cause immediate script termination. This is correct behavior for a CI workflow.However, consider adding explicit error context for debugging:
💡 Optional enhancement for better error diagnostics
- node scripts/ast_audit.js - node scripts/testall_runtime.js + echo "Running AST audit..." + node scripts/ast_audit.js || { echo "AST audit failed"; exit 1; } + echo "Running runtime simulation..." + node scripts/testall_runtime.js || { echo "Runtime simulation failed"; exit 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run_testall.sh` around lines 53 - 54, Because run_testall.sh uses set -euo pipefail, failures in the node scripts will abort without context; update run_testall.sh to explicitly capture and report which script failed by checking the exit status after invoking node (for node scripts/ast_audit.js and node scripts/testall_runtime.js) or by wrapping each invocation with a short conditional that prints a descriptive error message including the script name and exit code before exiting non‑zero, so CI logs show clear diagnostic context for failures.AGENTS/SESSION_LOG.md (1)
81-106: Session entry headers use##instead of###.Earlier entries (lines 8, 51, 67) use
### YYYY-MM-DD — Titleformat, but newer entries (lines 81, 89, 95, 100, 106) use## YYYY-MM-DD — Title. Consider maintaining consistent heading levels for the append-only log.📝 Suggested fix for header consistency
-## 2026-03-04 — /testall comprehensive self-contained runner hardening +### 2026-03-04 — /testall comprehensive self-contained runner hardeningApply similar changes to lines 89, 95, 100, and 106.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS/SESSION_LOG.md` around lines 81 - 106, The session log uses inconsistent header levels: change the four entries that currently start with "## 2026-03-04 — /testall comprehensive self-contained runner hardening", "## 2026-03-04 — /testall rotating log retention and report-linking", "## 2026-03-04 — CI smoke fix for rotating logger dependency", and "## 2026-03-04 — /testall post-review fixes (CI + report delivery + summary format)" to use the same level as earlier entries (replace the leading "##" with "###") so all timestamps follow the existing "### YYYY-MM-DD — Title" convention.index.js (1)
13794-13800: Consider throttling fallback log pruning to reduce I/O overhead.When using the fallback writer,
pruneFallbackLogs(14)is called on every single log write. This triggersreaddirSync, sorting, and potentialunlinkSyncoperations for each log line, which can be inefficient during high-frequency logging.Consider pruning only periodically (e.g., every 100 writes) or at session end.
💡 Proposed throttling approach
+ let fallbackWriteCount = 0; + const PRUNE_INTERVAL = 100; + const log = (level, message, extra = {}) => { const payload = { ts: new Date().toISOString(), level, sessionTag, message, ...extra }; const line = `${JSON.stringify(payload)}\n`; if (stream) { stream.write(line); return; } try { fs.appendFileSync(fallbackLogPath, line, 'utf8'); - if (usingFallback) pruneFallbackLogs(14); + fallbackWriteCount += 1; + if (usingFallback && fallbackWriteCount % PRUNE_INTERVAL === 0) { + pruneFallbackLogs(14); + } } catch (_) { // swallow write errors to avoid impacting /testall flow } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 13794 - 13800, The fallback writer currently calls pruneFallbackLogs(14) on every fs.appendFileSync write, causing heavy I/O; introduce a throttle (e.g., a module-level counter like fallbackPruneCounter or a lastPrune timestamp) that increments on each successful append and only calls pruneFallbackLogs(14) when the counter reaches a threshold (e.g., % 100 === 0) or when enough time has passed (e.g., > 1 minute), and reset/update the counter/timestamp afterward; keep the existing usingFallback check and ensure the counter/timestamp is initialized in the same scope as the writer so pruning still runs at session end if desired.docs/INDEX.md (1)
186-186: Label audit counts as generated snapshot metadata.Consider appending source + generation context (e.g., “from
scripts/ast_audit.json YYYY-MM-DD”) to reduce future confusion when counts drift.Based on learnings:
docs/INDEX.mdis treated as operational source-of-truth documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/INDEX.md` at line 186, Update the generated snapshot metadata line "Current AST audit baseline: 520 callback emitters, 268 action handlers, ..." in docs/INDEX.md to append source and generation context (e.g., "from scripts/ast_audit.js on YYYY-MM-DD"); locate the string in docs/INDEX.md and modify the generator or manual update so the line includes the script name and timestamp (ISO date) to clarify provenance whenever the AST audit baseline is updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/INDEX.md`:
- Line 6: Update the canonical line-count value in docs/INDEX.md so all
occurrences match; specifically, reconcile the two differing strings "**Bot
version:** 3.0.0 | `index.js`: ~15,100 lines | Commands: 96 registered (`95`
normalized group-command entries) | Action handlers: 268" and the other entry
that reads "`index.js`: ~14,960 lines" by choosing one canonical count (prefer
the updated "~15,100 lines"), and search the file for any other `index.js`
line-count mentions and replace them to be consistent.
In `@index.js`:
- Around line 14207-14221: The summary code is reading
reportData.summary.commandsPassed, callbacksPassed, and warnings which are not
set and cause NaN; update the logic that builds totalPassed, totalWarnings, and
totalFailed to derive values from reportData.commandResults,
reportData.callbackResults, and reportData.steps when the summary fields are
missing (or ensure reportData.summary populates commandsPassed, callbacksPassed,
and warnings beforehand). Specifically, modify the quick-summary calculation
around originalCtxFns.reply/legacyTestAllSummaryFormat to compute totals with
reportData.commandResults.filter(r => r.ok).length,
reportData.callbackResults.filter(r => r.ok && !r.skipped).length, and
reportData.steps.filter(s => s.warning).length (and analogous failing counts)
whenever reportData.summary.commandsPassed/callbacksPassed/warnings are
undefined so the final message always yields "X passed, Y warnings, Z failures."
In `@package.json`:
- Around line 34-37: Update package.json so `@babel/parser` and `@babel/traverse`
are moved from devDependencies into dependencies because /testall runs
scripts/ast_audit.js at production runtime and requires those modules; leave
`@babel/types` in devDependencies (it isn’t imported). Edit the "devDependencies"
and "dependencies" objects to remove the two packages from devDependencies and
add them to dependencies with the same version ranges, ensuring production
installs (npm ci --omit=dev) will include them.
In `@scripts/ast_audit.js`:
- Around line 146-147: The script currently calls
fs.writeFileSync('reports/ast_audit.json', ...) (and the similar write at Line
208) without ensuring the reports/ directory exists; create the reports
directory before any writes by invoking fs.mkdirSync('reports', { recursive:
true }) (or equivalent async mkdir) early in the script (before the
fs.writeFileSync calls) so both fs.writeFileSync usages (the
'reports/ast_audit.json' writes) will never fail on clean runs.
- Line 95: The CallExpression visitor currently checks for callee === 'Stage'
and also has an unreachable callee === 'new Stage' branch; add a NewExpression
visitor that mirrors the CallExpression logic so registrations created via "new
Stage(...)" are captured: detect nodes of type NewExpression where
node.callee.name === 'Stage' and push the same registration into
data.scene.stageRegs (using the same file/line variables), and remove or ignore
the 'new Stage' string branch in the CallExpression check to avoid dead code.
In `@scripts/testall_runtime.js`:
- Line 217: The fallback detection only checks for calls with method
'answerCallbackQuery' but misses the alias 'answerCbQuery', so update the
fallback predicate that computes the variable fallback to also check for calls
where c.method === 'answerCbQuery' (or normalize method aliases before the
check), ensuring previews.some(...) || calls.some(c => (c.method ===
'answerCallbackQuery' || c.method === 'answerCbQuery') && String(c.payload.text
|| '').includes('Action not available anymore')) so both intercepted aliases are
detected and fallbackHandled is reported correctly.
---
Nitpick comments:
In `@AGENTS/SESSION_LOG.md`:
- Around line 81-106: The session log uses inconsistent header levels: change
the four entries that currently start with "## 2026-03-04 — /testall
comprehensive self-contained runner hardening", "## 2026-03-04 — /testall
rotating log retention and report-linking", "## 2026-03-04 — CI smoke fix for
rotating logger dependency", and "## 2026-03-04 — /testall post-review fixes (CI
+ report delivery + summary format)" to use the same level as earlier entries
(replace the leading "##" with "###") so all timestamps follow the existing "###
YYYY-MM-DD — Title" convention.
In `@docs/INDEX.md`:
- Line 186: Update the generated snapshot metadata line "Current AST audit
baseline: 520 callback emitters, 268 action handlers, ..." in docs/INDEX.md to
append source and generation context (e.g., "from scripts/ast_audit.js on
YYYY-MM-DD"); locate the string in docs/INDEX.md and modify the generator or
manual update so the line includes the script name and timestamp (ISO date) to
clarify provenance whenever the AST audit baseline is updated.
In `@index.js`:
- Around line 13794-13800: The fallback writer currently calls
pruneFallbackLogs(14) on every fs.appendFileSync write, causing heavy I/O;
introduce a throttle (e.g., a module-level counter like fallbackPruneCounter or
a lastPrune timestamp) that increments on each successful append and only calls
pruneFallbackLogs(14) when the counter reaches a threshold (e.g., % 100 === 0)
or when enough time has passed (e.g., > 1 minute), and reset/update the
counter/timestamp afterward; keep the existing usingFallback check and ensure
the counter/timestamp is initialized in the same scope as the writer so pruning
still runs at session end if desired.
In `@qa/context/bot_capabilities.json`:
- Line 434: Update the "mapExcerpt" value in qa/context/bot_capabilities.json to
explicitly indicate the excerpt is truncated by appending a suffix such as "…
[truncated]" (so consumers know the text ends intentionally mid-document). While
editing, verify any newly added or removed bot flows in your change are also
reflected in RUNEWAGER_FUNCTIONALITY_MAP.md so the excerpt and source-of-truth
file remain consistent.
- Line 2: Make the generatedAt field optional to avoid noisy diffs: change the
artifact generation code that writes the JSON to only include the "generatedAt"
key when an environment flag is set (e.g., INCLUDE_GENERATED_AT=true) or when
running a non-committed/debug build; update the serializer that emits the
"generatedAt" property so it checks process.env.INCLUDE_GENERATED_AT (or
equivalent config) before adding the "generatedAt" key to the output JSON, and
ensure committed artifacts omit this field by default.
In `@reports/testall_report.md`:
- Around line 125-127: The report generator is rendering a misleading "Fallback"
column with a checkmark regardless of fallbackHandled value; update the
generator in scripts/testall_runtime.js so the table cell uses the actual
fallbackHandled boolean from the JSON (show ✅ only when fallbackHandled ===
true) or rename the header to a neutral term like "Handled" or "Status" and
adjust the cell text accordingly; locate the table-generation logic that emits
the header string "Fallback" and the cell rendering for admin_auth_bypass (and
other callbacks) and change the header or conditional rendering to reflect
fallbackHandled accurately.
In `@scripts/run_testall.sh`:
- Around line 53-54: Because run_testall.sh uses set -euo pipefail, failures in
the node scripts will abort without context; update run_testall.sh to explicitly
capture and report which script failed by checking the exit status after
invoking node (for node scripts/ast_audit.js and node
scripts/testall_runtime.js) or by wrapping each invocation with a short
conditional that prints a descriptive error message including the script name
and exit code before exiting non‑zero, so CI logs show clear diagnostic context
for failures.
In `@scripts/testall_runtime.js`:
- Around line 228-344: The IIFE currently restores process.exit only at the end
so an exception leaves the monkey-patch in place; wrap the main async body of
the IIFE in try { ... } finally { process.exit = originalProcessExit; } (or add
a finally block around the await dispatch loops and report generation) so that
process.exit is always restored even if dispatchCommand/dispatchCallback or file
writes throw; reference the existing identifiers process.exit,
originalProcessExit, the top-level async () => { ... } IIFE, and the
dispatchCommand/dispatchCallback/report write sections when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d482878-f320-4b55-8ffd-d8a323e4371f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
AGENTS/SESSION_LOG.mdRUNEWAGER_FUNCTIONALITY_MAP.mddocs/INDEX.mddocs/features/10-deploy-ops.mdindex.jspackage.jsonqa/context/bot_capabilities.jsonqa/context/repo_info.jsonqa/state/provider_status.jsonreports/ast_audit.jsonreports/structural_audit.mdreports/testall_report.jsonreports/testall_report.mdreports/testall_report.txtscripts/ast_audit.jsscripts/run_testall.shscripts/testall_runtime.js
| **Last updated:** 2026-03-01 | ||
| **Bot version:** 3.0.0 | `index.js`: ~15,050 lines | Commands: 95 | Action handlers: 270+ | ||
| **Last updated:** 2026-03-04 | ||
| **Bot version:** 3.0.0 | `index.js`: ~15,100 lines | Commands: 96 registered (`95` normalized group-command entries) | Action handlers: 268 |
There was a problem hiding this comment.
Resolve internal line-count mismatch for index.js.
Line 6 says ~15,100 lines, but Line 174 still says ~14,960 lines. Please keep one canonical value to avoid drift in the index.
Based on learnings: maintain docs/INDEX.md as a universal wiring index/source of truth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/INDEX.md` at line 6, Update the canonical line-count value in
docs/INDEX.md so all occurrences match; specifically, reconcile the two
differing strings "**Bot version:** 3.0.0 | `index.js`: ~15,100 lines |
Commands: 96 registered (`95` normalized group-command entries) | Action
handlers: 268" and the other entry that reads "`index.js`: ~14,960 lines" by
choosing one canonical count (prefer the updated "~15,100 lines"), and search
the file for any other `index.js` line-count mentions and replace them to be
consistent.
Comment on lines
+14207
to
+14221
| if (originalCtxFns.reply) { | ||
| const totalPassed = reportData.summary | ||
| ? reportData.summary.commandsPassed + reportData.summary.callbacksPassed | ||
| : (reportData.commandResults.filter((r) => r.ok).length + reportData.callbackResults.filter((r) => r.ok && !r.skipped).length); | ||
| const totalWarnings = reportData.summary | ||
| ? reportData.summary.warnings | ||
| : reportData.steps.filter((s) => s.warning).length; | ||
| const totalFailed = reportData.summary | ||
| ? reportData.summary.commandsFailed + reportData.summary.callbacksFailed | ||
| : (reportData.commandResults.filter((r) => !r.ok).length + reportData.callbackResults.filter((r) => !r.ok && !r.skipped).length); | ||
| const quickSummary = `🧪 ${legacyTestAllSummaryFormat | ||
| .replace('${totalPassed}', String(totalPassed)) | ||
| .replace('${totalWarnings}', String(totalWarnings)) | ||
| .replace('${totalFailed}', String(totalFailed))}`; | ||
| try { await originalCtxFns.reply(quickSummary); } catch (_) {} |
There was a problem hiding this comment.
Bug: Summary references undefined fields, resulting in NaN values.
The totals object (line 14124-14131) only defines commandsFailed, callbacksFailed, adminFlowsFailed, etc., but the summary calculation attempts to access commandsPassed, callbacksPassed, and warnings, which are never set:
const totalPassed = reportData.summary
? reportData.summary.commandsPassed + reportData.summary.callbacksPassed // undefined + undefined = NaNSince reportData.summary is always truthy after line 14131, this will produce NaN in the final summary message, violating the required format: "TestAll complete — X passed, Y warnings, Z failures."
🐛 Proposed fix to compute passed/warnings correctly
const totals = {
commandsFailed: reportData.commandResults.filter((r) => !r.ok).length,
callbacksFailed: reportData.callbackResults.filter((r) => !r.ok).length,
adminFlowsFailed: reportData.adminFlowResults.filter((r) => !r.ok).length,
callbacksFallbackHandled: reportData.callbackResults.filter((r) => r.fallbackHit).length,
durationMs: Date.now() - startTs,
+ commandsPassed: reportData.commandResults.filter((r) => r.ok).length,
+ callbacksPassed: reportData.callbackResults.filter((r) => r.ok && !r.skipped).length,
+ warnings: reportData.steps.filter((s) => s.warning).length,
};
reportData.summary = totals;Based on learnings: "/testall updates in the Runewager bot must preserve structured category checks and final summary: TestAll complete — X passed, Y warnings, Z failures."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 14207 - 14221, The summary code is reading
reportData.summary.commandsPassed, callbacksPassed, and warnings which are not
set and cause NaN; update the logic that builds totalPassed, totalWarnings, and
totalFailed to derive values from reportData.commandResults,
reportData.callbackResults, and reportData.steps when the summary fields are
missing (or ensure reportData.summary populates commandsPassed, callbacksPassed,
and warnings beforehand). Specifically, modify the quick-summary calculation
around originalCtxFns.reply/legacyTestAllSummaryFormat to compute totals with
reportData.commandResults.filter(r => r.ok).length,
reportData.callbackResults.filter(r => r.ok && !r.skipped).length, and
reportData.steps.filter(s => s.warning).length (and analogous failing counts)
whenever reportData.summary.commandsPassed/callbacksPassed/warnings are
undefined so the final message always yields "X passed, Y warnings, Z failures."
Comment on lines
+34
to
+37
| "devDependencies": { | ||
| "@babel/parser": "^7.29.0", | ||
| "@babel/traverse": "^7.29.0", | ||
| "@babel/types": "^7.29.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the repository structure
git ls-files | head -20Repository: gamblecodezcom/Runewager
Length of output: 475
🏁 Script executed:
# Check package.json structure and dependencies
cat -n package.jsonRepository: gamblecodezcom/Runewager
Length of output: 1552
🏁 Script executed:
# Search for /testall implementation and ast_audit references
rg -n "testall|ast_audit" --max-count=20Repository: gamblecodezcom/Runewager
Length of output: 16326
🏁 Script executed:
# Check prod-run.sh and deployment scripts
git ls-files | grep -E "(prod|deploy|ci)" | head -20Repository: gamblecodezcom/Runewager
Length of output: 291
🏁 Script executed:
# Check if Babel is actually imported in runtime files
rg -n "@babel/(parser|traverse|types)" --type=js | head -20Repository: gamblecodezcom/Runewager
Length of output: 207
🏁 Script executed:
# Check prod-run.sh for npm ci --omit=dev usage
cat -n prod-run.shRepository: gamblecodezcom/Runewager
Length of output: 23716
🏁 Script executed:
# Check ast_audit.js to see if all three Babel packages are actually used
cat -n scripts/ast_audit.js | head -50Repository: gamblecodezcom/Runewager
Length of output: 2733
🏁 Script executed:
# Check if `@babel/types` is used in ast_audit.js
rg -n "@babel/types|types\." scripts/ast_audit.js | head -10Repository: gamblecodezcom/Runewager
Length of output: 50
🏁 Script executed:
# Check if /testall is invoked during production runtime or CI
rg -n "testall|ast_audit" prod-run.sh deploy.sh .github/workflows/deploy.yml .github/workflows/ci.ymlRepository: gamblecodezcom/Runewager
Length of output: 50
🏁 Script executed:
# Check when ast_audit.js runs: is it only at deploy time or at runtime?
rg -n "ast_audit\.js" index.js | head -5Repository: gamblecodezcom/Runewager
Length of output: 296
🏁 Script executed:
# Check if `@babel/types` is imported anywhere in ast_audit.js
cat -n scripts/ast_audit.js | grep -i "types" | head -20Repository: gamblecodezcom/Runewager
Length of output: 260
🏁 Script executed:
# Verify /testall command definition in index.js to confirm it's production-facing
rg -B2 -A5 "bot\.command\('testall'" index.jsRepository: gamblecodezcom/Runewager
Length of output: 361
🏁 Script executed:
# Check if /testall appears in any production docs/definitions
rg -n "testall" docs/features/10-deploy-ops.md | head -10Repository: gamblecodezcom/Runewager
Length of output: 813
🏁 Script executed:
# More thorough check: search entire ast_audit.js for `@babel` imports and usage
cat scripts/ast_audit.js | head -100Repository: gamblecodezcom/Runewager
Length of output: 5715
🏁 Script executed:
# Check if `@babel/types` is required as a peer dependency or indirect import
grep -r "@babel/types" scripts/ index.js 2>/dev/null || echo "No `@babel/types` found"Repository: gamblecodezcom/Runewager
Length of output: 89
Move @babel/parser and @babel/traverse to dependencies—they are required at runtime by /testall.
/testall executes scripts/ast_audit.js at production runtime, which imports @babel/parser and @babel/traverse. When prod-run.sh uses npm ci --omit=dev, these devDependencies are excluded, causing a module-not-found error. Note: @babel/types is not imported and should not be moved.
💡 Correct fix (move only the two packages actually used by ast_audit.js)
"dependencies": {
"dotenv": "^16.6.1",
"express": "4.22.1",
"rotating-file-stream": "^3.2.9",
- "telegraf": "^4.16.3"
+ "telegraf": "^4.16.3",
+ "@babel/parser": "^7.29.0",
+ "@babel/traverse": "^7.29.0"
},
"devDependencies": {
- "@babel/parser": "^7.29.0",
- "@babel/traverse": "^7.29.0",
"@babel/types": "^7.29.0"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 34 - 37, Update package.json so `@babel/parser` and
`@babel/traverse` are moved from devDependencies into dependencies because
/testall runs scripts/ast_audit.js at production runtime and requires those
modules; leave `@babel/types` in devDependencies (it isn’t imported). Edit the
"devDependencies" and "dependencies" objects to remove the two packages from
devDependencies and add them to dependencies with the same version ranges,
ensuring production installs (npm ci --omit=dev) will include them.
| if (callee === 'bot.use') data.botUses.push({ file: rfile, line, middleware: getCalleeName(n.arguments[0]) || n.arguments[0]?.type }); | ||
| if (callee === 'ctx.scene.enter' || callee === 'scene.enter') data.scene.enter.push({ file: rfile, line, scene: evalStaticString(n.arguments[0]) }); | ||
| if (callee === 'ctx.scene.leave' || callee === 'scene.leave') data.scene.leave.push({ file: rfile, line }); | ||
| if (callee === 'Stage' || callee === 'new Stage') data.scene.stageRegs.push({ file: rfile, line }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Examine the Stage detection and visitor structure
cat -n scripts/ast_audit.js | head -120Repository: gamblecodezcom/Runewager
Length of output: 7683
🏁 Script executed:
#!/bin/bash
# Check for NewExpression and CallExpression visitors
rg -n "NewExpression|CallExpression" scripts/ast_audit.jsRepository: gamblecodezcom/Runewager
Length of output: 733
🏁 Script executed:
#!/bin/bash
# Check what visitors are actually defined
rg -n "^\s*(CallExpression|NewExpression|FunctionDeclaration|VariableDeclarator|AssignmentExpression)" scripts/ast_audit.jsRepository: gamblecodezcom/Runewager
Length of output: 157
🏁 Script executed:
#!/bin/bash
# Search for stageRegs references and Stage detection logic
rg -n "stageRegs|Stage.*new|new.*Stage" scripts/ast_audit.jsRepository: gamblecodezcom/Runewager
Length of output: 466
new Stage(...) registrations are never detected because they use NewExpression, not CallExpression.
The code at line 95 is inside a CallExpression visitor, but new Stage(...) creates a NewExpression node that is never visited. The condition callee === 'new Stage' is unreachable dead code. Add a NewExpression visitor to detect Stage registrations using the new keyword:
Proposed fix
traverse(ast, {
+ NewExpression(p) {
+ const n = p.node;
+ const line = n.loc?.start?.line || 0;
+ if (getCalleeName(n.callee) === 'Stage') {
+ data.scene.stageRegs.push({ file: rfile, line });
+ }
+ },
FunctionDeclaration(p) {
if (p.node.id?.name) {
let found = false;
p.traverse({ CallExpression(cp) { if (getCalleeName(cp.node.callee) === 'requireAdmin') found = true; } });
functionAdminMap.set(`${rfile}:${p.node.id.name}`, found);
}
},
CallExpression(p) {
const n = p.node; const callee = getCalleeName(n.callee); const line = n.loc?.start?.line || 0;
...
- if (callee === 'Stage' || callee === 'new Stage') data.scene.stageRegs.push({ file: rfile, line });
+ if (callee === 'Stage') data.scene.stageRegs.push({ file: rfile, line });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ast_audit.js` at line 95, The CallExpression visitor currently checks
for callee === 'Stage' and also has an unreachable callee === 'new Stage'
branch; add a NewExpression visitor that mirrors the CallExpression logic so
registrations created via "new Stage(...)" are captured: detect nodes of type
NewExpression where node.callee.name === 'Stage' and push the same registration
into data.scene.stageRegs (using the same file/line variables), and remove or
ignore the 'new Stage' string branch in the CallExpression check to avoid dead
code.
Comment on lines
+146
to
+147
| fs.writeFileSync('reports/ast_audit.json', JSON.stringify(report, null, 2)); | ||
|
|
There was a problem hiding this comment.
Create reports/ before writing artifacts to avoid ENOENT on clean runs.
Writes at Line 146 and Line 208 assume reports/ already exists; deterministic audits should ensure the directory exists.
🛠️ Proposed fix
const report = { scannedFiles:data.files.length, jsTsFiles:data.files.filter(f=>/\.(js|ts)$/.test(f)).length, ...data, missingWhitelist, orphanWhitelist, duplicateCommands, deadButtons, fallbackOnly, fallback, possibleAdminUngated, overlap };
-fs.writeFileSync('reports/ast_audit.json', JSON.stringify(report, null, 2));
+const reportsDir = path.join(root, 'reports');
+fs.mkdirSync(reportsDir, { recursive: true });
+fs.writeFileSync(path.join(reportsDir, 'ast_audit.json'), JSON.stringify(report, null, 2));
@@
-fs.writeFileSync('reports/structural_audit.md', md);
+fs.writeFileSync(path.join(reportsDir, 'structural_audit.md'), md);
console.log('Wrote reports/ast_audit.json and reports/structural_audit.md');Also applies to: 208-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ast_audit.js` around lines 146 - 147, The script currently calls
fs.writeFileSync('reports/ast_audit.json', ...) (and the similar write at Line
208) without ensuring the reports/ directory exists; create the reports
directory before any writes by invoking fs.mkdirSync('reports', { recursive:
true }) (or equivalent async mkdir) early in the script (before the
fs.writeFileSync calls) so both fs.writeFileSync usages (the
'reports/ast_audit.json' writes) will never fail on clean runs.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
scripts/testall_runtime.js (1)
254-254:⚠️ Potential issue | 🟠 MajorFallback detection misses
answerCbQueryalias (reporting false negatives).Line 254 only checks
answerCallbackQuery. In this codebase, callback acknowledgements also useanswerCbQuery, sofallbackHandledcan be undercounted.💡 Proposed fix
- const fallback = previews.some((t) => t.includes('That button is no longer active')) || calls.some((c) => c.method === 'answerCallbackQuery' && String(c.payload.text || '').includes('Action not available anymore')); + const fallback = + previews.some((t) => t.includes('That button is no longer active')) || + calls.some((c) => + (c.method === 'answerCallbackQuery' || c.method === 'answerCbQuery') && + String(c.payload.text || '').includes('Action not available anymore') + );#!/bin/bash set -euo pipefail # Verify both callback-answer aliases are used and inspect fallback predicate. rg -nP '\banswerCbQuery\b|\banswerCallbackQuery\b' index.js telegramSafe.js scripts/testall_runtime.js sed -n '248,260p' scripts/testall_runtime.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/testall_runtime.js` at line 254, The fallback detection currently only checks for calls with method === 'answerCallbackQuery', missing the 'answerCbQuery' alias; update the predicate that sets the fallback variable (which inspects previews and calls) to treat both 'answerCallbackQuery' and 'answerCbQuery' as equivalent — e.g., change the calls.some check to match c.method against both names (or normalize c.method to lowercase and test for both aliases) so fallback counts include either callback-answer alias.docs/INDEX.md (1)
6-6:⚠️ Potential issue | 🟡 MinorUse one canonical
index.jsline count across the doc.Line 6 says
~15,100, but Architecture Notes still shows~14,960, so the index currently conflicts with itself.🛠️ Suggested alignment
-- **Single-file monolith:** All logic in `index.js` (~14,960 lines). +- **Single-file monolith:** All logic in `index.js` (~15,100 lines).Based on learnings: maintain
docs/INDEX.mdas a universal wiring index/source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/INDEX.md` at line 6, The document shows two conflicting index.js line-counts: the "**Bot version:**" summary uses "~15,100" while the "Architecture Notes" section shows "~14,960"; pick one canonical line-count and update all occurrences to match (search for the exact strings "~15,100" and "~14,960" and replace with the chosen canonical value), and ensure the "**Bot version:**" summary and the "Architecture Notes" block are consistent in docs/INDEX.md so the file serves as the single source of truth.index.js (1)
14138-14146:⚠️ Potential issue | 🟠 MajorQuick summary still risks
NaNcounters due to missing summary fields.
reportData.summaryis always truthy, butcommandsPassed,callbacksPassed, andwarningsare not populated before the quick-summary calculation.🐛 Proposed fix
const totals = { commandsFailed: reportData.commandResults.filter((r) => !r.ok).length, callbacksFailed: reportData.callbackResults.filter((r) => !r.ok).length, adminFlowsFailed: reportData.adminFlowResults.filter((r) => !r.ok).length, callbacksFallbackHandled: reportData.callbackResults.filter((r) => r.fallbackHit).length, + commandsPassed: reportData.commandResults.filter((r) => r.ok && !r.skipped).length, + callbacksPassed: reportData.callbackResults.filter((r) => r.ok && !r.skipped).length, + warnings: reportData.steps.filter((s) => s.warning).length, durationMs: Date.now() - startTs, }; @@ - const totalPassed = reportData.summary - ? reportData.summary.commandsPassed + reportData.summary.callbacksPassed + const totalPassed = reportData.summary + ? ((reportData.summary.commandsPassed ?? 0) + (reportData.summary.callbacksPassed ?? 0)) : (reportData.commandResults.filter((r) => r.ok).length + reportData.callbackResults.filter((r) => r.ok && !r.skipped).length); const totalWarnings = reportData.summary - ? reportData.summary.warnings + ? (reportData.summary.warnings ?? 0) : reportData.steps.filter((s) => s.warning).length;Based on learnings: "
/testallupdates in the Runewager bot must preserve structured category checks and final summary:TestAll complete — X passed, Y warnings, Z failures."Also applies to: 14221-14230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 14138 - 14146, The quick-summary calculation builds totals from reportData but never ensures summary fields like commandsPassed, callbacksPassed, and warnings exist, which can produce NaN; before computing totals (the block that creates totals and assigns reportData.summary) initialize or compute commandsPassed = reportData.commandResults.length - commandsFailed (or 0 if missing), callbacksPassed similarly from callbackResults, and warnings = reportData.warnings || 0, then include these properties in the totals object and set reportData.summary = totals; also keep the existing status-setting logic that flips reportData.status = 'issues' when any failed counters are nonzero (apply the same fix to the other quick-summary block that computes a final summary).
🧹 Nitpick comments (2)
AGENTS/SESSION_LOG.md (2)
83-85: Consider varying sentence structure.Three consecutive bullet points begin with "Added," which slightly reduces readability. Consider rephrasing for variety.
✍️ Optional rewording
- Added ephemeral runtime snapshot + rollback helpers (`createTestAllStateSnapshot`, `restoreTestAllStateSnapshot`) covering user/promo/giveaway/tips/promo-history/smart-button/sshv runtime stores. -- Added synthetic command/callback simulation through `bot.handleUpdate` for all `BOT_KNOWN_COMMANDS` entries and AST-discovered actions (literal + representative regex callback samples), with fallback/unmatched callback capture. -- Added deterministic report persistence and batched admin DM summary output for `/testall` (`reports/testall_report.json`, `reports/testall_report.md`). +- Introduced synthetic command/callback simulation through `bot.handleUpdate` for all `BOT_KNOWN_COMMANDS` entries and AST-discovered actions (literal + representative regex callback samples), with fallback/unmatched callback capture. +- Implemented deterministic report persistence and batched admin DM summary output for `/testall` (`reports/testall_report.json`, `reports/testall_report.md`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS/SESSION_LOG.md` around lines 83 - 85, The three bullets in SESSION_LOG.md all start with "Added", so reword them for variety: keep the same technical details but modify the sentence openings and structure around the symbols createTestAllStateSnapshot, restoreTestAllStateSnapshot, bot.handleUpdate, BOT_KNOWN_COMMANDS, and the reports paths (reports/testall_report.json, reports/testall_report.md); for example make one bullet start with a noun phrase (e.g., "Ephemeral runtime snapshot + rollback helpers: createTestAllStateSnapshot/restoreTestAllStateSnapshot..."), another with an action phrase for the synthetic command simulation via bot.handleUpdate and BOT_KNOWN_COMMANDS, and a third that emphasizes output/persistence for deterministic report persistence and batched admin DM summary (reports/testall_report.*), ensuring each bullet conveys the same content but uses varied openings and sentence structures.
81-113: Align entry format with established session log convention.The six new entries (lines 81-113) deviate from the format established in the file header (lines 3-4) and demonstrated in earlier entries:
- Heading level: Uses
##instead of###- Missing structured fields: Lacks the explicit Scope:, Changes:, Tests:, Map updated:, Index updated:, Open items: sections
This inconsistency makes the log harder to scan. Consider reformatting these entries to match the pattern used in lines 8-80.
📋 Example of aligned format for the first new entry
-## 2026-03-04 — /testall comprehensive self-contained runner hardening -- Reworked `/testall` in `index.js` to enforce admin-only entry via `isAdmin(ctx)` and run a full internal step registry with progress updates, non-blocking step failure handling, and deterministic report generation. -- Added ephemeral runtime snapshot + rollback helpers (`createTestAllStateSnapshot`, `restoreTestAllStateSnapshot`) covering user/promo/giveaway/tips/promo-history/smart-button/sshv runtime stores. -- Added synthetic command/callback simulation through `bot.handleUpdate` for all `BOT_KNOWN_COMMANDS` entries and AST-discovered actions (literal + representative regex callback samples), with fallback/unmatched callback capture. -- Added deterministic report persistence and batched admin DM summary output for `/testall` (`reports/testall_report.json`, `reports/testall_report.md`). -- Synced documentation: `docs/INDEX.md` and `RUNEWAGER_FUNCTIONALITY_MAP.md` now reflect the expanded `/testall` behavior and rollback/report invariants. -- Regenerated AST and runtime reports. +### 2026-03-04 — /testall comprehensive self-contained runner hardening + +**Scope:** `index.js`, `scripts/testall_runtime.js`, `reports/`, `docs/INDEX.md`, `RUNEWAGER_FUNCTIONALITY_MAP.md` + +**Changes:** +- Reworked `/testall` in `index.js` to enforce admin-only entry via `isAdmin(ctx)` and run a full internal step registry with progress updates, non-blocking step failure handling, and deterministic report generation. +- Added ephemeral runtime snapshot + rollback helpers (`createTestAllStateSnapshot`, `restoreTestAllStateSnapshot`) covering user/promo/giveaway/tips/promo-history/smart-button/sshv runtime stores. +- Added synthetic command/callback simulation through `bot.handleUpdate` for all `BOT_KNOWN_COMMANDS` entries and AST-discovered actions (literal + representative regex callback samples), with fallback/unmatched callback capture. +- Added deterministic report persistence and batched admin DM summary output for `/testall` (`reports/testall_report.json`, `reports/testall_report.md`). +- Synced documentation: `docs/INDEX.md` and `RUNEWAGER_FUNCTIONALITY_MAP.md` now reflect the expanded `/testall` behavior and rollback/report invariants. +- Regenerated AST and runtime reports. + +**Tests:** `npm test` (60/60 pass); generated reports validated +**Map updated:** yes +**Index updated:** yes +**Open items:** none🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS/SESSION_LOG.md` around lines 81 - 113, The six new changelog entries (titles like "/testall comprehensive self-contained runner hardening", "/testall rotating log retention and report-linking", etc.) use "##" headings and lack the standard structured fields; update each entry to use the established "###" heading level and add the explicit sections "Scope:", "Changes:", "Tests:", "Map updated:", "Index updated:", and "Open items:" with concise content mirroring the current prose (e.g., put the bulletized details under "Changes:", any test/CI notes under "Tests:", and references to docs/AST/runtime updates under "Map updated:"/"Index updated:"); keep the existing sentences but reorganize them into those fields for each of the six entries so they match the formatting pattern used earlier in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.js`:
- Around line 13934-13966: The patch currently overwrites global bot.telegram
methods (sendMessage, editMessageText, answerCallbackQuery, deleteMessage,
pinChatMessage, unpinChatMessage, sendPhoto, sendAnimation, sendDocument)
causing real traffic to be routed to test stubs during /testall; fix by scoping
the monkey-patch so originals (originalTelegramMethods) are only replaced for
the duration of a single /testall run and restored immediately after, or by
adding a runtime check inside each patched wrapper (e.g., in the functions
passed to rateLimitedTestallCall or the start/stop testall handler) to forward
to originalTelegramMethods when the current update is not a testall context;
ensure you restore all originalTelegramMethods on test completion and on error
paths so live updates are never left using the test stubs.
- Around line 13773-13785: pruneFallbackLogs currently only deletes extra
rotated files but does not prevent the active "testall.log" from growing; modify
pruneFallbackLogs (and where fallback logging writes) to implement size-based
rollover: when "testall.log" in testAllLogDir exceeds a threshold (e.g., maxSize
bytes) rename the current "testall.log" to a sequenced rotated name like
"testall.log.1", "testall.log.2", etc. (shifting/renaming older rotated files
upward and dropping the oldest if > maxFiles), then create a fresh empty
"testall.log" for new appends; ensure the function operates on the symbols
pruneFallbackLogs, testAllLogDir, and the "testall.log" base name, uses
fs.statSync to check size, fs.renameSync to rotate, and then deletes extras as
it already does so that single-file growth is bounded by rotation and pruning.
- Around line 14122-14126: The admin command loop is causing recursive
self-invocation because safeInvokeCommand('testall') re-enters the same handler;
fix it by preventing re-entry: either filter out 'testall' from adminCommands
before the loop or add a guard in the loop that skips invoking when command ===
'testall' and the current execution context is already running 'testall';
alternatively implement a small reentrancy guard inside safeInvokeCommand (e.g.,
a Set runningCommands that records command names on entry and returns/throws if
the command is already present) and ensure reportData.adminFlowResults still
receives a sensible skipped result for visibility.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md`:
- Line 414: Update the `/testall` entry in RUNEWAGER_FUNCTIONALITY_MAP.md to
list the TXT artifact alongside JSON and MD by changing the sentence that
currently mentions only `reports/testall_report.json` and
`reports/testall_report.md` to include `reports/testall_report.txt`; reference
the `/testall` command and the `reports/testall_report.txt` artifact so the doc
matches the runtime behavior.
---
Duplicate comments:
In `@docs/INDEX.md`:
- Line 6: The document shows two conflicting index.js line-counts: the "**Bot
version:**" summary uses "~15,100" while the "Architecture Notes" section shows
"~14,960"; pick one canonical line-count and update all occurrences to match
(search for the exact strings "~15,100" and "~14,960" and replace with the
chosen canonical value), and ensure the "**Bot version:**" summary and the
"Architecture Notes" block are consistent in docs/INDEX.md so the file serves as
the single source of truth.
In `@index.js`:
- Around line 14138-14146: The quick-summary calculation builds totals from
reportData but never ensures summary fields like commandsPassed,
callbacksPassed, and warnings exist, which can produce NaN; before computing
totals (the block that creates totals and assigns reportData.summary) initialize
or compute commandsPassed = reportData.commandResults.length - commandsFailed
(or 0 if missing), callbacksPassed similarly from callbackResults, and warnings
= reportData.warnings || 0, then include these properties in the totals object
and set reportData.summary = totals; also keep the existing status-setting logic
that flips reportData.status = 'issues' when any failed counters are nonzero
(apply the same fix to the other quick-summary block that computes a final
summary).
In `@scripts/testall_runtime.js`:
- Line 254: The fallback detection currently only checks for calls with method
=== 'answerCallbackQuery', missing the 'answerCbQuery' alias; update the
predicate that sets the fallback variable (which inspects previews and calls) to
treat both 'answerCallbackQuery' and 'answerCbQuery' as equivalent — e.g.,
change the calls.some check to match c.method against both names (or normalize
c.method to lowercase and test for both aliases) so fallback counts include
either callback-answer alias.
---
Nitpick comments:
In `@AGENTS/SESSION_LOG.md`:
- Around line 83-85: The three bullets in SESSION_LOG.md all start with "Added",
so reword them for variety: keep the same technical details but modify the
sentence openings and structure around the symbols createTestAllStateSnapshot,
restoreTestAllStateSnapshot, bot.handleUpdate, BOT_KNOWN_COMMANDS, and the
reports paths (reports/testall_report.json, reports/testall_report.md); for
example make one bullet start with a noun phrase (e.g., "Ephemeral runtime
snapshot + rollback helpers:
createTestAllStateSnapshot/restoreTestAllStateSnapshot..."), another with an
action phrase for the synthetic command simulation via bot.handleUpdate and
BOT_KNOWN_COMMANDS, and a third that emphasizes output/persistence for
deterministic report persistence and batched admin DM summary
(reports/testall_report.*), ensuring each bullet conveys the same content but
uses varied openings and sentence structures.
- Around line 81-113: The six new changelog entries (titles like "/testall
comprehensive self-contained runner hardening", "/testall rotating log retention
and report-linking", etc.) use "##" headings and lack the standard structured
fields; update each entry to use the established "###" heading level and add the
explicit sections "Scope:", "Changes:", "Tests:", "Map updated:", "Index
updated:", and "Open items:" with concise content mirroring the current prose
(e.g., put the bulletized details under "Changes:", any test/CI notes under
"Tests:", and references to docs/AST/runtime updates under "Map updated:"/"Index
updated:"); keep the existing sentences but reorganize them into those fields
for each of the six entries so they match the formatting pattern used earlier in
the document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0307dc61-bbfc-4a26-b21a-ae619f07a509
📒 Files selected for processing (12)
AGENTS/SESSION_LOG.mdRUNEWAGER_FUNCTIONALITY_MAP.mddocs/INDEX.mddocs/features/10-deploy-ops.mdindex.jsrateLimiter.jsreports/ast_audit.jsonreports/structural_audit.mdreports/testall_report.jsonreports/testall_report.mdreports/testall_report.txtscripts/testall_runtime.js
🚧 Files skipped from review as they are similar to previous changes (1)
- reports/testall_report.json
Comment on lines
+13773
to
+13785
| function pruneFallbackLogs(maxFiles = 14) { | ||
| try { | ||
| const files = fs.readdirSync(testAllLogDir) | ||
| .filter((f) => f.startsWith('testall.log')) | ||
| .sort((a, b) => a.localeCompare(b)); | ||
| const extra = Math.max(0, files.length - maxFiles); | ||
| for (let i = 0; i < extra; i += 1) { | ||
| fs.unlinkSync(path.join(testAllLogDir, files[i])); | ||
| } | ||
| } catch (_) { | ||
| // non-fatal | ||
| } | ||
| } |
There was a problem hiding this comment.
Fallback logger does not actually rotate/truncate the active log file.
In fallback mode, writes always append to testall.log; pruneFallbackLogs() only deletes extra files, so single-file growth is unbounded.
🛠️ Proposed fix (size-based rollover in fallback mode)
const fallbackLogPath = path.join(testAllLogDir, 'testall.log');
+ function rotateFallbackIfNeeded(maxBytes = 10 * 1024 * 1024) {
+ try {
+ const st = fs.existsSync(fallbackLogPath) ? fs.statSync(fallbackLogPath) : null;
+ if (!st || st.size < maxBytes) return;
+ const rotatedName = `testall.log.${new Date().toISOString().replace(/[:.]/g, '-')}`;
+ fs.renameSync(fallbackLogPath, path.join(testAllLogDir, rotatedName));
+ pruneFallbackLogs(14);
+ } catch (_) {
+ // non-fatal
+ }
+ }
+
const log = (level, message, extra = {}) => {
@@
try {
+ if (usingFallback) rotateFallbackIfNeeded();
fs.appendFileSync(fallbackLogPath, line, 'utf8');
- if (usingFallback) pruneFallbackLogs(14);
+ if (usingFallback) pruneFallbackLogs(14);
} catch (_) {Also applies to: 13795-13798
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 13773 - 13785, pruneFallbackLogs currently only
deletes extra rotated files but does not prevent the active "testall.log" from
growing; modify pruneFallbackLogs (and where fallback logging writes) to
implement size-based rollover: when "testall.log" in testAllLogDir exceeds a
threshold (e.g., maxSize bytes) rename the current "testall.log" to a sequenced
rotated name like "testall.log.1", "testall.log.2", etc. (shifting/renaming
older rotated files upward and dropping the oldest if > maxFiles), then create a
fresh empty "testall.log" for new appends; ensure the function operates on the
symbols pruneFallbackLogs, testAllLogDir, and the "testall.log" base name, uses
fs.statSync to check size, fs.renameSync to rotate, and then deletes extras as
it already does so that single-file growth is bounded by rotation and pruning.
Comment on lines
+13934
to
+13966
| const originalTelegramMethods = { | ||
| sendMessage: bot.telegram.sendMessage, | ||
| editMessageText: bot.telegram.editMessageText, | ||
| answerCallbackQuery: bot.telegram.answerCallbackQuery, | ||
| deleteMessage: bot.telegram.deleteMessage, | ||
| pinChatMessage: bot.telegram.pinChatMessage, | ||
| unpinChatMessage: bot.telegram.unpinChatMessage, | ||
| sendPhoto: bot.telegram.sendPhoto, | ||
| sendAnimation: bot.telegram.sendAnimation, | ||
| sendDocument: bot.telegram.sendDocument, | ||
| }; | ||
|
|
||
| bot.botInfo = bot.botInfo || { id: 999, is_bot: true, first_name: 'RunewagerTest', username: 'RunewagerTestBot' }; | ||
| bot.telegram.sendMessage = async (chatId, text, extra = {}) => rateLimitedTestallCall({ method: 'sendMessage', chatId }, async () => { | ||
| recordApiCall('sendMessage', { chat_id: chatId, text, ...extra }); | ||
| testMsgId += 1; | ||
| return { message_id: testMsgId, date: Math.floor(Date.now() / 1000), chat: { id: chatId || 1, type: 'private' }, text: text || '' }; | ||
| }); | ||
| bot.telegram.editMessageText = async (chatId, messageId, inlineMessageId, text, extra = {}) => rateLimitedTestallCall({ method: 'editMessageText', globalOnly: true }, async () => { | ||
| recordApiCall('editMessageText', { chat_id: chatId, message_id: messageId, inline_message_id: inlineMessageId, text, ...extra }); | ||
| return true; | ||
| }); | ||
| bot.telegram.answerCallbackQuery = async (callbackQueryId, text, extra = {}) => rateLimitedTestallCall({ method: 'answerCallbackQuery', globalOnly: true }, async () => { | ||
| recordApiCall('answerCallbackQuery', { callback_query_id: callbackQueryId, text, ...extra }); | ||
| return true; | ||
| }); | ||
| bot.telegram.deleteMessage = async (...args) => rateLimitedTestallCall({ method: 'deleteMessage', globalOnly: true }, async () => { recordApiCall('deleteMessage', { args }); return true; }); | ||
| bot.telegram.pinChatMessage = async (...args) => rateLimitedTestallCall({ method: 'pinChatMessage', globalOnly: true }, async () => { recordApiCall('pinChatMessage', { args }); return true; }); | ||
| bot.telegram.unpinChatMessage = async (...args) => rateLimitedTestallCall({ method: 'unpinChatMessage', globalOnly: true }, async () => { recordApiCall('unpinChatMessage', { args }); return true; }); | ||
| bot.telegram.sendPhoto = async (chatId, media, extra = {}) => rateLimitedTestallCall({ method: 'sendPhoto', chatId }, async () => { recordApiCall('sendPhoto', { chat_id: chatId, media, ...extra }); testMsgId += 1; return { message_id: testMsgId }; }); | ||
| bot.telegram.sendAnimation = async (chatId, media, extra = {}) => rateLimitedTestallCall({ method: 'sendAnimation', chatId }, async () => { recordApiCall('sendAnimation', { chat_id: chatId, media, ...extra }); testMsgId += 1; return { message_id: testMsgId }; }); | ||
| bot.telegram.sendDocument = async (chatId, media, extra = {}) => rateLimitedTestallCall({ method: 'sendDocument', chatId }, async () => { recordApiCall('sendDocument', { chat_id: chatId, media, ...extra }); testMsgId += 1; return { message_id: testMsgId }; }); | ||
|
|
There was a problem hiding this comment.
Global Telegram API monkey-patching can impact live user traffic during /testall.
Overriding bot.telegram.* at process scope affects concurrent real updates while /testall runs, so normal sends can be diverted into test stubs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 13934 - 13966, The patch currently overwrites global
bot.telegram methods (sendMessage, editMessageText, answerCallbackQuery,
deleteMessage, pinChatMessage, unpinChatMessage, sendPhoto, sendAnimation,
sendDocument) causing real traffic to be routed to test stubs during /testall;
fix by scoping the monkey-patch so originals (originalTelegramMethods) are only
replaced for the duration of a single /testall run and restored immediately
after, or by adding a runtime check inside each patched wrapper (e.g., in the
functions passed to rateLimitedTestallCall or the start/stop testall handler) to
forward to originalTelegramMethods when the current update is not a testall
context; ensure you restore all originalTelegramMethods on test completion and
on error paths so live updates are never left using the test stubs.
Comment on lines
+14122
to
+14126
| const adminCommands = ['admin', 'testall', 'testgiveaway', 'sshv', 'deploy_status', 'verify_bot_setup']; | ||
| for (const command of adminCommands) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const result = await safeInvokeCommand(command); | ||
| reportData.adminFlowResults.push(result); |
There was a problem hiding this comment.
Prevent recursive /testall self-invocation during command simulation.
safeInvokeCommand('testall') re-enters the same handler while it is already running, which can cascade into nested /testall runs and runaway execution.
🐛 Proposed fix
await withStep('Command Tests (BOT_KNOWN_COMMANDS)', async (step) => {
const commands = Array.from(BOT_KNOWN_COMMANDS.values()).sort();
for (const command of commands) {
+ if (command === 'testall') {
+ reportData.commandResults.push({
+ command,
+ ok: true,
+ skipped: true,
+ detail: 'Skipped to avoid recursive /testall invocation',
+ apiCalls: 0,
+ });
+ continue;
+ }
// eslint-disable-next-line no-await-in-loop
const result = await safeInvokeCommand(command);
reportData.commandResults.push(result);
@@
- await withStep('Admin Flow Tests', async (step) => {
- const adminCommands = ['admin', 'testall', 'testgiveaway', 'sshv', 'deploy_status', 'verify_bot_setup'];
+ await withStep('Admin Flow Tests', async (step) => {
+ const adminCommands = ['admin', 'testgiveaway', 'sshv', 'deploy_status', 'verify_bot_setup'];Also applies to: 14070-14085
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 14122 - 14126, The admin command loop is causing
recursive self-invocation because safeInvokeCommand('testall') re-enters the
same handler; fix it by preventing re-entry: either filter out 'testall' from
adminCommands before the loop or add a guard in the loop that skips invoking
when command === 'testall' and the current execution context is already running
'testall'; alternatively implement a small reentrancy guard inside
safeInvokeCommand (e.g., a Set runningCommands that records command names on
entry and returns/throws if the command is already present) and ensure
reportData.adminFlowResults still receives a sensible skipped result for
visibility.
| - 2026-03-04: Merged main branch additions — `telegramSafe.js` (global rate-limit patch via `telegramSafe.init(bot)`), `rateLimiter.js` (per-chat + global Telegram API rate limiter), `backend.js` (companion HTTP service on port 3001, `runewager-endpoint.service` systemd unit), updated `prod-run.sh`, `scripts/runewager_redeploy.sh`, `scripts/rw_cpu_guard.sh`. | ||
| - 2026-03-04: /runewager-audit — 17-phase full audit. Findings: 0 critical, 1 warning resolved (dead `adminKeyboard()` function removed — legacy promo keyboard with no callers). Auto-fix applied. 60/60 tests pass post-fix. | ||
| - 2026-03-04: Post-audit alignment verification pass — re-ran `node scripts/ast_audit.js` and regenerated `reports/ast_audit.json` + `reports/structural_audit.md`; confirmed command normalization residual is intentional (`/A` handled through lowercase `a` in group guard), fallback callback handler still preserves user-facing recovery while logging bounded unmatched callback context, and no Scene/Stage FSM wiring is currently present by design. | ||
| - 2026-03-04: Extended `/testall` to execute AST audit + deterministic internal update simulation (all discovered commands/callbacks), writing `reports/testall_report.json` and `reports/testall_report.md` while preserving existing admin-only behavior and summary format. |
There was a problem hiding this comment.
Include TXT artifact in the /testall output note for accuracy.
This line currently lists only JSON + MD, but the runtime also writes reports/testall_report.txt.
📝 Suggested doc correction
-- 2026-03-04: Extended `/testall` to execute AST audit + deterministic internal update simulation (all discovered commands/callbacks), writing `reports/testall_report.json` and `reports/testall_report.md` while preserving existing admin-only behavior and summary format.
+- 2026-03-04: Extended `/testall` to execute AST audit + deterministic internal update simulation (all discovered commands/callbacks), writing `reports/testall_report.json`, `reports/testall_report.md`, and `reports/testall_report.txt` while preserving existing admin-only behavior and summary format.Based on learnings: implement behavior in code first, then sync RUNEWAGER_FUNCTIONALITY_MAP.md to match real behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md` at line 414, Update the `/testall` entry in
RUNEWAGER_FUNCTIONALITY_MAP.md to list the TXT artifact alongside JSON and MD by
changing the sentence that currently mentions only `reports/testall_report.json`
and `reports/testall_report.md` to include `reports/testall_report.txt`;
reference the `/testall` command and the `reports/testall_report.txt` artifact
so the doc matches the runtime behavior.
gamblecodezcom
deleted the
codex/perform-full-structural-audit-for-runewager
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Motivation
RUNEWAGER_FUNCTIONALITY_MAP.md/docs/INDEX.mdinto sync with runtime state and command registrations./testallrunner that can simulate every known command and representative callback payloads without hitting external APIs to improve observability and safety./testalllogging so CI/VPS boots are resilient when rotation dependencies are missing and include rollback/snapshot guarantees for runtime stores.Description
scripts/ast_audit.jsthat emitsreports/ast_audit.jsonandreports/structural_audit.md./testallharness inindex.jswith: admin-only guard (isAdmin(ctx)), ephemeral runtime snapshot/rollback helpers (createTestAllStateSnapshot/restoreTestAllStateSnapshot), syntheticbot.handleUpdatecommand/callback simulation, Telegram API stubbing during simulation, deterministic report persistence (reports/testall_report.json|.md|.txt), and batched admin DM delivery (guarded by env flags)./testallvia lazy-loadedrotating-file-streamwith a fallback append/prune writer to avoid boot crashes; encapsulated increateTestAllRotatingLogger.scripts/testall_runtime.js(runtime simulator) andscripts/run_testall.sh(wrapper to run AST + runtime testall, rotate wrapper logs and guarantee TXT fallback).docs/INDEX.md,docs/features/10-deploy-ops.md, andRUNEWAGER_FUNCTIONALITY_MAP.mdentries describing/testallbehavior, rotation policy, invariants, and command normalization note (/A→acoverage).BOT_KNOWN_COMMANDSinindex.jsto include newly-tracked commands and aliases and added a guardedmodule.exports = { bot }whenTESTALL_EXPORTS=1.package.json/package-lock.jsonchanges to includerotating-file-streamand development AST helpers (@babel/*) as dev dependencies.Testing
npm test, which passed:60/60tests successful.node scripts/ast_audit.js, which completed and wrotereports/ast_audit.jsonandreports/structural_audit.md(audit artifacts generated).node scripts/testall_runtime.js, which completed normally and wrotereports/testall_report.json,reports/testall_report.md, andreports/testall_report.txt(runtime simulation captured a small number of expected async send errors under the CI stub but the harness exited successfully).Codex Task
Summary by Sourcery
Add deterministic AST-based command/callback audit tooling and a comprehensive, admin-only
/testallruntime simulation with rotating logs and persisted reports to improve observability, safety, and CI/ops workflows.New Features:
/testallreports and TXT fallbacks./testallinto a self-contained admin-only diagnostics runner that executes AST and runtime audits, snapshots and restores bot state, and sends summarized admin DMs with report links./testall, with daily/size-based rotation, gzip compression, and retained history referenced from reports.Enhancements:
/testallworkflow, artifacts, and rotating-log policy in the deployment and index docs, and align the functionality map with the audited structure.Build:
rotating-file-streamas a runtime dependency and Babel parser/traverse/types as dev dependencies to support the audit tooling.Documentation:
/testallruntime simulation, artifacts, and logging/rollback invariants.CodeAnt-AI Description
Add deterministic AST audit and a self-contained
/testallruntime simulator with safe logging and rollbackWhat Changed
Impact
✅ Clearer structural audit reports for command/callback coverage✅ Fewer boot failures when rotating-log dependency is missing during CI/VPS startup✅ Shorter admin debugging time with per-run reports and safe, non-persistent runtime tests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Documentation
Chores
Bug Fixes