Add admin-only /testall diagnostics with AST audit and runtime sim#139
Merged
gamblecodezcom merged 2 commits intoMar 4, 2026
Merged
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 Guide/testall is upgraded into a deterministic, in-process diagnostics runner that snapshots/restores runtime state, drives AST + runtime simulations, writes structured reports and rotating logs, and tightens admin-only + observability guarantees; new AST and runtime scripts plus docs/QA metadata keep the functionality map and operator docs in sync. Sequence diagram for the upgraded /testall end-to-end diagnostics runsequenceDiagram
actor Admin
participant Bot as Bot_process_index_js
participant RotLog as TestAllRotatingLogger
participant AST as ast_audit_js
participant Runtime as testall_runtime_js
participant Reports as Reports_dir
participant Logs as TestAll_logs_dir
Admin->>Bot: /testall command
Bot->>Bot: isAdmin(ctx)
Bot->>RotLog: createTestAllRotatingLogger()
Bot->>Bot: createTestAllStateSnapshot()
Bot->>Bot: override ctx.reply/send/edit/answer (sink)
Bot->>Bot: stub Telegram API methods
rect rgb(235,235,255)
Bot->>Bot: withStep(AST Audit Step)
Bot->>AST: runTestAllExternalAudit() via execFile(ast_audit_js)
AST-->>Reports: write ast_audit_json
AST-->>Reports: write structural_audit_md
AST-->>Bot: exit status
end
rect rgb(235,255,235)
Bot->>Runtime: execFile(testall_runtime_js) with TESTALL_EXPORTS=1
Runtime->>Bot: require(index_js).bot
Runtime->>Runtime: install async error handlers
Runtime->>Runtime: stub bot.telegram methods
Runtime->>Reports: read ast_audit_json
loop For each command in ast.commands
Runtime->>Bot: bot.handleUpdate(message /command)
Bot-->>Runtime: command handler executes
Runtime->>Runtime: record apiCalls
end
loop For each callback sample
Runtime->>Bot: bot.handleUpdate(callback_query data)
Bot-->>Runtime: action handler executes or fallback
Runtime->>Runtime: detect fallbackHandled
end
Runtime->>Reports: write testall_report_json
Runtime->>Reports: write testall_report_md
Runtime-->>Bot: stdout JSON {status, json, md}
end
Bot->>Bot: parse runtime stdout JSON
rect rgb(255,245,235)
Bot->>Bot: withStep(Command Tests)
Bot->>Bot: safeInvokeCommand for each BOT_KNOWN_COMMANDS
Bot->>Bot: withStep(Action/Callback Tests)
Bot->>Reports: read ast_audit_json
Bot->>Bot: safeInvokeAction for each ast.actions
end
Bot->>Bot: withStep(FSM + pagination checks)
Bot->>Bot: withStep(Admin flow checks)
rect rgb(255,235,235)
Bot->>Bot: withStep(Rollback + Persist Reports)
Bot->>Bot: restoreTestAllStateSnapshot(snapshot)
Bot->>RotLog: listLogArtifacts()
Bot->>Reports: write consolidated testall_report_json
Bot->>Reports: write testall_report_md
Bot->>Reports: write testall_report_txt
end
Bot-->>RotLog: close()
Bot->>Logs: testall_log and rotated files updated
Bot->>Admin: progress replies and final quick summary
Bot->>Admin: DM batched Markdown report (chunkTextForTelegram)
Bot->>Bot: restore original ctx and Telegram methods
Bot-->>Admin: /testall run complete
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds AST static-analysis and a deterministic runtime test harness for /testall, plus state snapshot/rollback, rotating logging, multi-format report generation (JSON/MD/TXT), documentation updates, and new report artifacts; includes package.json dependency additions and CI-safe lazy loading for the logger. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin (/testall)
participant Bot as Bot Core
participant Snapshot as State Snapshot
participant AST as AST Audit
participant Runtime as Runtime Simulator
participant Logger as Rotating Logger
participant Reporter as Report Generator
participant Restore as State Restore
Admin->>Bot: Trigger /testall
Bot->>Snapshot: createTestAllStateSnapshot()
Snapshot-->>Bot: snapshot
Bot->>AST: runTestAllExternalAudit()
AST->>AST: parse JS/TS (Babel) and emit ast_audit.json
AST-->>Bot: audit results
Bot->>Runtime: run simulations (commands/callbacks)
Runtime->>Logger: stream verbose logs to rotating files
Runtime->>Runtime: execute synthetic updates, record API calls
Runtime-->>Bot: runtime results
Bot->>Reporter: write reports (JSON/MD/TXT) and embed log refs
Reporter-->>Bot: report paths
Bot->>Restore: restoreTestAllStateSnapshot(snapshot)
Restore-->>Bot: state restored
Bot-->>Admin: DM/report final summary
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
scripts/testall_runtime.js,bot.telegram.callApistill delegates to the original implementation for most methods (onlygetMeis special-cased), which means some commands/callbacks that use rawcallApior unstubbed methods (e.g.getChat,getChatMember) can still hit the real Telegram API; consider stubbingcallApiitself or explicitly blocking/recording all methods to keep the simulation deterministic and offline. - The
/testallhandler overridesbot.telegram.*methods andctx.reply/friends without a top-leveltry/finally, so if an unexpected error occurs before the restoration block, the process could be left with stubbed methods; wrapping the override/restore region intry/finallywould make this safer. - The new snapshot helpers (
createTestAllStateSnapshot/restoreTestAllStateSnapshot) rely onstructuredClone, which is only available in newer Node versions; if this code is ever run on an older runtime, it will throw at startup—consider adding a small polyfill or a guarded fallback implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `scripts/testall_runtime.js`, `bot.telegram.callApi` still delegates to the original implementation for most methods (only `getMe` is special-cased), which means some commands/callbacks that use raw `callApi` or unstubbed methods (e.g. `getChat`, `getChatMember`) can still hit the real Telegram API; consider stubbing `callApi` itself or explicitly blocking/recording all methods to keep the simulation deterministic and offline.
- The `/testall` handler overrides `bot.telegram.*` methods and `ctx.reply`/friends without a top-level `try/finally`, so if an unexpected error occurs before the restoration block, the process could be left with stubbed methods; wrapping the override/restore region in `try/finally` would make this safer.
- The new snapshot helpers (`createTestAllStateSnapshot` / `restoreTestAllStateSnapshot`) rely on `structuredClone`, which is only available in newer Node versions; if this code is ever run on an older runtime, it will throw at startup—consider adding a small polyfill or a guarded fallback implementation.
## Individual Comments
### Comment 1
<location path="index.js" line_range="13882-13891" />
<code_context>
+ bot.telegram.sendMessage = async (chatId, text, extra = {}) => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Global monkeypatching of bot.telegram during /testall risks affecting concurrent updates.
Because these replacements apply to the shared bot instance, any other updates processed while /testall is running will also hit the stubbed methods, potentially dropping or misrouting real user messages, edits, and pins. This behavior should be isolated from live traffic—for example by running the full simulation only in a dedicated test process, using a separate Telegraf instance for /testall, or gating the monkeypatches behind a non‑production flag.
</issue_to_address>
### Comment 2
<location path="index.js" line_range="13671-13680" />
<code_context>
+ tipsStore: structuredClone(tipsStore),
+ promoHistoryEntries: structuredClone(Array.from(promoHistoryStore.entries())),
+ smartButtonEntries: structuredClone(Array.from(smartButtonTracker.entries())),
+ sshvSessionEntries: structuredClone(Array.from(sshvSessions.entries()).map(([adminId, session]) => [adminId, {
+ adminId: Number(adminId),
+ active: true,
+ cwd: session.cwd || SSHV_DEFAULT_CWD,
+ buffer: normalizeSshvBufferLines(session.buffer),
+ lastCommand: String(session.lastCommand || '').slice(0, 200),
+ locked: Boolean(session.locked),
+ createdAt: Number(session.createdAt || Date.now()),
+ lastActivity: Number(session.lastActivity || Date.now()),
+ editorMode: session.editorMode ? {
+ filePath: String(session.editorMode.filePath || ''),
+ fileArg: String(session.editorMode.fileArg || ''),
</code_context>
<issue_to_address>
**issue (bug_risk):** The SSHV session snapshot does not round‑trip original values and can mutate state after restore.
`createTestAllStateSnapshot` builds new SSHV session objects with normalized values (e.g. `active: true`, `createdAt: Number(session.createdAt || Date.now())`, truncated `buffer`, `editorMode.draft`), and `restoreTestAllStateSnapshot` writes these back into `sshvSessions`. Running `/testall` can therefore change real session state (activating inactive sessions, overwriting original timestamps, truncating buffers and drafts). The snapshot should instead store and restore the original session objects (or a deep clone of them), and any redacted/normalized view for reporting should be derived separately from that snapshot.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
@sourcery-ai resolve |
1 similar comment
Owner
Author
@sourcery-ai resolve |
gamblecodezcom
changed the title
/testall runner, AST audit scripts, deterministic reports and rotating logs/testall runner, AST audit scripts, deterministic reports and rotating logs. @sourcery-ai resolve
Owner
Author
|
Fix everything @sourcery-ai resolve |
sourcery-ai
Bot
changed the title
/testall runner, AST audit scripts, deterministic reports and rotating logs. @sourcery-ai resolve![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
qa/context/bot_capabilities.json (1)
3-5: Redundant entry: both "A" and "a" are listed.Per the BOT_KNOWN_COMMANDS comment in index.js (line 300),
/Ais normalized to lowercase by the group guard, so'a'covers both. Listing both in this capabilities manifest is redundant and may cause confusion in downstream tooling that consumes this file.Consider removing the uppercase
"A"entry.♻️ Suggested fix
"commands": [ - "A", "a", "admin",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@qa/context/bot_capabilities.json` around lines 3 - 5, Remove the redundant uppercase command entry by deleting "A" from the "commands" array in qa/context/bot_capabilities.json so only "a" remains; this aligns with the BOT_KNOWN_COMMANDS normalization (see the group guard behavior in index.js) and prevents duplicate command representations.scripts/ast_audit.js (2)
9-18: WrapreaddirSyncin try/catch for resilience.Per project guidelines,
readdirSynccalls should be wrapped in try/catch to handle permission errors or inaccessible directories gracefully.Based on learnings: "For Runewager test files: wrap
readdirSynccalls in try/catch."🛡️ Suggested defensive handling
function listFiles(dir) { const out = []; - for (const ent of fs.readdirSync(dir, { withFileTypes: true })) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch (err) { + console.warn(`Warning: Could not read directory ${dir}: ${err.message}`); + return out; + } + for (const ent of entries) { if (['node_modules', '.git'].includes(ent.name)) continue; const p = path.join(dir, ent.name); if (ent.isDirectory()) out.push(...listFiles(p)); else if (/\.(js|ts)$/.test(ent.name)) out.push(p); } return out; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ast_audit.js` around lines 9 - 18, The listFiles function should wrap the fs.readdirSync(dir, { withFileTypes: true }) call in a try/catch to avoid crashing on permission/inaccessible directories; catch errors around that call inside listFiles and on error either return an empty array (skip that directory) or log a warning and continue so the recursion proceeds safely; ensure the catch only affects the current directory iteration and retains existing behavior of pushing .js/.ts file paths when readdirSync succeeds.
133-133: Expand catch-all regex detection.Line 133 only checks for the exact pattern
'/.*/', but per project guidelines, catch-all patterns include variants like(.*),(.+),(?:.*),^(?:.*)$, and(.+)?.Based on learnings: "expand
isCatchAllRegexPattern()to detect(.*),(.+),(?:.*),^(?:.*)$,(.+)?patterns."♻️ Suggested enhancement
-const fallback = data.actions.find(a => a.pattern.kind === 'regex' && a.pattern.value === '/.*/'); +function isCatchAllRegex(pattern) { + if (pattern.kind !== 'regex') return false; + const v = pattern.value; + // Match common catch-all patterns: /.*/, /(.*)/, /(.+)/, /(?:.*)/, etc. + return /^\/\^?\(?[\?:]*[\.\*\+]+\)?\$?\/[a-z]*$/i.test(v); +} +const fallback = data.actions.find(a => isCatchAllRegex(a.pattern));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ast_audit.js` at line 133, The current fallback detection only matches the exact string '/.*/'; replace the inline equality check used in const fallback = data.actions.find(...) with a call to a new helper isCatchAllRegexPattern(patternValue) and implement that helper to recognize catch-all variants such as /.*/, (.*), (.+), (?:.*), ^(?:.*)$, (.+)? and their anchored or slash-delimited forms; implement the helper by normalizing the input (strip surrounding slashes, optional ^/$ anchors, optional non-capturing group syntax (?:), and optional trailing ?), then test the normalized body against the core patterns like '.*' or '.+' (and their groupings) so fallback correctly detects all allowed catch-all regex variants referenced in the review.index.js (2)
14139-14146: Redundant rollback call.
restoreTestAllStateSnapshot(snapshot)is called twice: once inside the "Rollback + Persist Reports" step (line 14070) and again here (line 14140). While harmless due to idempotency, the second call appears to be defensive redundancy. Consider documenting the intent or removing the duplicate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 14139 - 14146, The duplicate defensive call to restoreTestAllStateSnapshot(snapshot) should be removed (or explicitly documented) because it's already invoked in the "Rollback + Persist Reports" step; update the catch block where restoreTestAllStateSnapshot is invoked again and instead only set reportData.rollback.restored = true when appropriate or add a comment explaining idempotency if you intentionally keep it; refer to the restoreTestAllStateSnapshot function and the reportData.rollback/restored/error fields to locate and adjust the logic so the rollback is not executed twice.
14027-14039: Consider defensive error handling for AST file read.The synchronous
JSON.parse(fs.readFileSync(...))at line 14029 will throw if the file is missing or malformed. WhilewithStepcatches this, a specific error message would improve debuggability.♻️ Optional: add explicit try/catch
await withStep('Action/Callback Tests (registered actions + regex samples)', async (step) => { const astPath = path.join(__dirname, 'reports', 'ast_audit.json'); - const ast = JSON.parse(fs.readFileSync(astPath, 'utf8')); + let ast; + try { + ast = JSON.parse(fs.readFileSync(astPath, 'utf8')); + } catch (err) { + throw new Error(`Failed to load AST audit: ${err.message}`); + } for (const action of ast.actions || []) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 14027 - 14039, The JSON.parse(fs.readFileSync(astPath, 'utf8')) call can throw if the AST file is missing or malformed; wrap the read+parse for ast (the astPath/ast variable usage inside the withStep callback) in an explicit try/catch, and on error set step.warning (or step.error if available), push a clear error entry into reportData.callbackResults (or a dedicated reportData.errors array), set reportData.status = 'issues' and skip the action loop (return or continue) so safeInvokeAction is not run against an undefined ast; this keeps withStep but provides a specific, actionable error message for debugging.docs/features/10-deploy-ops.md (1)
139-146: Minor documentation improvement: consider adding retention details.The
rotating-file-streampackage supportsmaxFilesoption for retention. The documentation mentions "old files are pruned automatically (maxFilesretention)" but doesn't specify the actual retention count. According to the code snippet fromindex.js:13736-13784, themaxFilesis set to14. Consider adding this detail for completeness.📝 Optional improvement
### 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). +- Rotated files are gzip compressed and old files are pruned automatically (`maxFiles: 14` retention — ~2 weeks of history). - Final `/testall` reports include references to latest + retained rotating log artifacts for audit traceability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/10-deploy-ops.md` around lines 139 - 146, The docs mention automatic pruning via `maxFiles` but omit the actual retention count; update the `/testall` rotating log policy description to explicitly state that `maxFiles` is set to 14 (per the implementation in `index.js` around the rotating-file-stream setup), e.g., "rotated files are gzipped and pruned with a retention of 14 files (`maxFiles: 14`)", and ensure this change references the `/testall` rotating stream configuration so readers can correlate the docs with the `maxFiles` setting.
🤖 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 14181-14198: The admin DM loop currently calls
bot.telegram.sendMessage while sendMessage is still stubbed (ADMIN_IDS loop
using chunkTextForTelegram and awaiting bot.telegram.sendMessage), so messages
are recorded but not sent; move the restoration of the originalTelegramMethods
(restore bot.telegram.sendMessage, editMessageText, answerCallbackQuery,
deleteMessage, pinChatMessage, unpinChatMessage, sendPhoto, sendAnimation,
sendDocument from originalTelegramMethods) to before the ADMIN_IDS loop so the
real methods are used when iterating and sending chunks to admins.
- Around line 13721-13733: The chunkTextForTelegram function fails to split a
single input line that itself exceeds limit, causing chunks to exceed Telegram's
size; update chunkTextForTelegram to detect when a single line.length > limit
and slice that line into segments <= limit (filling the current chunk first if
space allows, then pushing full-sized segments directly to chunks) and continue
with any remainder, referencing the existing variables chunk, chunks and limit
so behavior integrates with the current loop and final push logic.
- Around line 14155-14158: The quickSummary message built in the
originalCtxFns.reply branch uses an incorrect format; replace usage of
quickSummary (and stop using the ad-hoc string) with the standardized
legacyTestAllSummaryFormat pattern so the reply matches "TestAll complete — X
passed, Y warnings, Z failures." Specifically, derive passed/warnings/failures
from reportData.summary (use reportData.summary.commandsPassed or compute passed
from totals, reportData.summary.warnings or callbacksFailed mapping, and
reportData.summary.commandsFailed/callbacksFailed as failures according to the
legacyTestAllSummaryFormat semantics) and pass that formatted string to
originalCtxFns.reply instead of the current quickSummary.
In `@qa/context/bot_capabilities.json`:
- Around line 434-435: The "mapExcerpt" value in
qa/context/bot_capabilities.json is truncated mid-sentence ("error block with
guidanc") which can break downstream consumers; update the mapExcerpt field (the
JSON string value for "mapExcerpt") to either: 1) complete the excerpt sensibly
or append a clear truncation marker like "..." so it doesn't end mid-word, 2)
remove the mapExcerpt field if not needed, or 3) replace it with the full
intended text — ensure the edited "mapExcerpt" string remains valid JSON
(properly escaped and closed) and that any code reading this key (e.g., tools
that parse bot_capabilities.json) will receive a well-formed string.
In `@scripts/ast_audit.js`:
- Around line 127-132: The code builds RegExp objects from user-controlled
strings in the actionRegexes mapping (see actionRegexes, a.pattern.value and new
RegExp) without error handling; wrap the RegExp construction in a try/catch so
malformed patterns don't crash the script, and on error either skip that action
or attach a null/invalid marker and log the offending pattern (use
process.stderr or logger) so the script continues safely; ensure the returned
object shape stays consistent (e.g., return { ...a, re: null, invalidPattern:
true } or omit the entry) so downstream code can handle missing/invalid regexes.
- Around line 154-155: Before calling fs.writeFileSync to write
'reports/ast_audit.json' ensure the reports directory exists by creating it if
necessary (e.g., call fs.mkdirSync('reports', { recursive: true }) or use
fs.promises.mkdir) immediately before the fs.writeFileSync that writes report;
apply the same fix to the other fs.writeFileSync that writes into the 'reports'
directory later in the file so both write operations won't fail when the
directory is missing.
- Around line 43-51: Remove the dead helper fnHasRequireAdmin: locate the
function definition named fnHasRequireAdmin (which uses traverse and
getCalleeName to check for requireAdmin) and delete that entire function
declaration since it is never used; also remove any now-unused imports/variables
that were only referenced by this function (e.g., traverse or related helpers)
if they become unused after deletion.
In `@scripts/testall_runtime.js`:
- Around line 19-20: The code reads ast_audit.json into ast without handling a
missing file; update the startup in scripts/testall_runtime.js to defensively
check for the file at astPath (e.g., using fs.existsSync or try/catch around
JSON.parse) before calling fs.readFileSync and JSON.parse, and if missing emit a
clear error/log via process.stderr or processLogger and either create a sensible
default ast object or exit gracefully; ensure references to astPath and the ast
variable (and the surrounding require('../index.js') usage) are updated so
downstream code won't run against undefined ast.
---
Nitpick comments:
In `@docs/features/10-deploy-ops.md`:
- Around line 139-146: The docs mention automatic pruning via `maxFiles` but
omit the actual retention count; update the `/testall` rotating log policy
description to explicitly state that `maxFiles` is set to 14 (per the
implementation in `index.js` around the rotating-file-stream setup), e.g.,
"rotated files are gzipped and pruned with a retention of 14 files (`maxFiles:
14`)", and ensure this change references the `/testall` rotating stream
configuration so readers can correlate the docs with the `maxFiles` setting.
In `@index.js`:
- Around line 14139-14146: The duplicate defensive call to
restoreTestAllStateSnapshot(snapshot) should be removed (or explicitly
documented) because it's already invoked in the "Rollback + Persist Reports"
step; update the catch block where restoreTestAllStateSnapshot is invoked again
and instead only set reportData.rollback.restored = true when appropriate or add
a comment explaining idempotency if you intentionally keep it; refer to the
restoreTestAllStateSnapshot function and the reportData.rollback/restored/error
fields to locate and adjust the logic so the rollback is not executed twice.
- Around line 14027-14039: The JSON.parse(fs.readFileSync(astPath, 'utf8')) call
can throw if the AST file is missing or malformed; wrap the read+parse for ast
(the astPath/ast variable usage inside the withStep callback) in an explicit
try/catch, and on error set step.warning (or step.error if available), push a
clear error entry into reportData.callbackResults (or a dedicated
reportData.errors array), set reportData.status = 'issues' and skip the action
loop (return or continue) so safeInvokeAction is not run against an undefined
ast; this keeps withStep but provides a specific, actionable error message for
debugging.
In `@qa/context/bot_capabilities.json`:
- Around line 3-5: Remove the redundant uppercase command entry by deleting "A"
from the "commands" array in qa/context/bot_capabilities.json so only "a"
remains; this aligns with the BOT_KNOWN_COMMANDS normalization (see the group
guard behavior in index.js) and prevents duplicate command representations.
In `@scripts/ast_audit.js`:
- Around line 9-18: The listFiles function should wrap the fs.readdirSync(dir, {
withFileTypes: true }) call in a try/catch to avoid crashing on
permission/inaccessible directories; catch errors around that call inside
listFiles and on error either return an empty array (skip that directory) or log
a warning and continue so the recursion proceeds safely; ensure the catch only
affects the current directory iteration and retains existing behavior of pushing
.js/.ts file paths when readdirSync succeeds.
- Line 133: The current fallback detection only matches the exact string '/.*/';
replace the inline equality check used in const fallback =
data.actions.find(...) with a call to a new helper
isCatchAllRegexPattern(patternValue) and implement that helper to recognize
catch-all variants such as /.*/, (.*), (.+), (?:.*), ^(?:.*)$, (.+)? and their
anchored or slash-delimited forms; implement the helper by normalizing the input
(strip surrounding slashes, optional ^/$ anchors, optional non-capturing group
syntax (?:), and optional trailing ?), then test the normalized body against the
core patterns like '.*' or '.+' (and their groupings) so fallback correctly
detects all allowed catch-all regex variants referenced in the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4836b6b0-a82d-4983-aaa1-a07decdf5cff
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
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.mdscripts/ast_audit.jsscripts/testall_runtime.js
Comment on lines
+13721
to
+13733
| function chunkTextForTelegram(text, limit = 3800) { | ||
| const chunks = []; | ||
| let chunk = ''; | ||
| for (const line of String(text || '').split('\n')) { | ||
| if ((chunk + line).length > limit) { | ||
| chunks.push(chunk); | ||
| chunk = ''; | ||
| } | ||
| chunk += `${line}\n`; | ||
| } | ||
| if (chunk) chunks.push(chunk); | ||
| return chunks; | ||
| } |
There was a problem hiding this comment.
Edge case: single lines exceeding the limit are not split.
If a single line exceeds limit, the function adds it whole after pushing the previous chunk, potentially exceeding Telegram's message size. In practice this is unlikely for testall reports, but worth noting.
🛡️ Optional defensive fix
function chunkTextForTelegram(text, limit = 3800) {
const chunks = [];
let chunk = '';
for (const line of String(text || '').split('\n')) {
+ // If a single line exceeds limit, split it
+ if (line.length > limit) {
+ if (chunk) { chunks.push(chunk); chunk = ''; }
+ for (let i = 0; i < line.length; i += limit) {
+ chunks.push(line.slice(i, i + limit));
+ }
+ continue;
+ }
if ((chunk + line).length > limit) {
chunks.push(chunk);
chunk = '';
}
chunk += `${line}\n`;
}
if (chunk) chunks.push(chunk);
return chunks;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 13721 - 13733, The chunkTextForTelegram function fails
to split a single input line that itself exceeds limit, causing chunks to exceed
Telegram's size; update chunkTextForTelegram to detect when a single line.length
> limit and slice that line into segments <= limit (filling the current chunk
first if space allows, then pushing full-sized segments directly to chunks) and
continue with any remainder, referencing the existing variables chunk, chunks
and limit so behavior integrates with the current loop and final push logic.
Comment on lines
+14155
to
+14158
| if (originalCtxFns.reply) { | ||
| const quickSummary = `🧪 TestAll complete — status: ${reportData.status}. Commands failed: ${reportData.summary ? reportData.summary.commandsFailed : 0}, callbacks failed: ${reportData.summary ? reportData.summary.callbacksFailed : 0}.`; | ||
| try { await originalCtxFns.reply(quickSummary); } catch (_) {} | ||
| } |
There was a problem hiding this comment.
Summary format does not match required guideline pattern.
Per coding guidelines, /testall must output: TestAll complete — X passed, Y warnings, Z failures. The current format deviates:
🧪 TestAll complete — status: ${status}. Commands failed: X, callbacks failed: Y.
The legacyTestAllSummaryFormat variable at line 13824 defines the correct pattern but is never used.
🔧 Proposed fix to match required format
if (originalCtxFns.reply) {
- const quickSummary = `🧪 TestAll complete — status: ${reportData.status}. Commands failed: ${reportData.summary ? reportData.summary.commandsFailed : 0}, callbacks failed: ${reportData.summary ? reportData.summary.callbacksFailed : 0}.`;
+ const totalPassed = (reportData.commandResults.filter((r) => r.ok).length) + (reportData.callbackResults.filter((r) => r.ok && !r.skipped).length);
+ const totalWarnings = reportData.steps.filter((s) => s.warning).length;
+ const totalFailed = (reportData.summary ? reportData.summary.commandsFailed : 0) + (reportData.summary ? reportData.summary.callbacksFailed : 0);
+ const quickSummary = `🧪 TestAll complete — ${totalPassed} passed, ${totalWarnings} warnings, ${totalFailed} failures.`;
try { await originalCtxFns.reply(quickSummary); } catch (_) {}
}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 14155 - 14158, The quickSummary message built in the
originalCtxFns.reply branch uses an incorrect format; replace usage of
quickSummary (and stop using the ad-hoc string) with the standardized
legacyTestAllSummaryFormat pattern so the reply matches "TestAll complete — X
passed, Y warnings, Z failures." Specifically, derive passed/warnings/failures
from reportData.summary (use reportData.summary.commandsPassed or compute passed
from totals, reportData.summary.warnings or callbacksFailed mapping, and
reportData.summary.commandsFailed/callbacksFailed as failures according to the
legacyTestAllSummaryFormat semantics) and pass that formatted string to
originalCtxFns.reply instead of the current quickSummary.
Comment on lines
+14181
to
+14198
| for (const adminId of ADMIN_IDS) { | ||
| const chunks = chunkTextForTelegram(dmReport, 3500); | ||
| for (const chunk of chunks) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await bot.telegram.sendMessage(adminId, chunk, { parse_mode: 'Markdown' }).catch(() => {}); | ||
| } | ||
| } | ||
| if (chunk) chunks.push(chunk); | ||
| for (const c of chunks) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await ctx.reply(c, { parse_mode: 'Markdown' }); | ||
|
|
||
| bot.telegram.sendMessage = originalTelegramMethods.sendMessage; | ||
| bot.telegram.editMessageText = originalTelegramMethods.editMessageText; | ||
| bot.telegram.answerCallbackQuery = originalTelegramMethods.answerCallbackQuery; | ||
| bot.telegram.deleteMessage = originalTelegramMethods.deleteMessage; | ||
| bot.telegram.pinChatMessage = originalTelegramMethods.pinChatMessage; | ||
| bot.telegram.unpinChatMessage = originalTelegramMethods.unpinChatMessage; | ||
| bot.telegram.sendPhoto = originalTelegramMethods.sendPhoto; | ||
| bot.telegram.sendAnimation = originalTelegramMethods.sendAnimation; | ||
| bot.telegram.sendDocument = originalTelegramMethods.sendDocument; | ||
|
|
There was a problem hiding this comment.
Bug: Admin DM reports sent via stubbed bot.telegram.sendMessage.
The DM loop (lines 14181-14187) executes while bot.telegram.sendMessage is still stubbed. The stub restoration happens afterward (line 14189), so admin DMs are recorded but never actually sent.
🐛 Fix: restore stubs before sending DMs
+ // Restore Telegram methods BEFORE sending DM reports
+ bot.telegram.sendMessage = originalTelegramMethods.sendMessage;
+ bot.telegram.editMessageText = originalTelegramMethods.editMessageText;
+ bot.telegram.answerCallbackQuery = originalTelegramMethods.answerCallbackQuery;
+ bot.telegram.deleteMessage = originalTelegramMethods.deleteMessage;
+ bot.telegram.pinChatMessage = originalTelegramMethods.pinChatMessage;
+ bot.telegram.unpinChatMessage = originalTelegramMethods.unpinChatMessage;
+ bot.telegram.sendPhoto = originalTelegramMethods.sendPhoto;
+ bot.telegram.sendAnimation = originalTelegramMethods.sendAnimation;
+ bot.telegram.sendDocument = originalTelegramMethods.sendDocument;
+
for (const adminId of ADMIN_IDS) {
const chunks = chunkTextForTelegram(dmReport, 3500);
for (const chunk of chunks) {
// eslint-disable-next-line no-await-in-loop
await bot.telegram.sendMessage(adminId, chunk, { parse_mode: 'Markdown' }).catch(() => {});
}
}
-
- bot.telegram.sendMessage = originalTelegramMethods.sendMessage;
- bot.telegram.editMessageText = originalTelegramMethods.editMessageText;
- bot.telegram.answerCallbackQuery = originalTelegramMethods.answerCallbackQuery;
- bot.telegram.deleteMessage = originalTelegramMethods.deleteMessage;
- bot.telegram.pinChatMessage = originalTelegramMethods.pinChatMessage;
- bot.telegram.unpinChatMessage = originalTelegramMethods.unpinChatMessage;
- bot.telegram.sendPhoto = originalTelegramMethods.sendPhoto;
- bot.telegram.sendAnimation = originalTelegramMethods.sendAnimation;
- bot.telegram.sendDocument = originalTelegramMethods.sendDocument;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const adminId of ADMIN_IDS) { | |
| const chunks = chunkTextForTelegram(dmReport, 3500); | |
| for (const chunk of chunks) { | |
| // eslint-disable-next-line no-await-in-loop | |
| await bot.telegram.sendMessage(adminId, chunk, { parse_mode: 'Markdown' }).catch(() => {}); | |
| } | |
| } | |
| if (chunk) chunks.push(chunk); | |
| for (const c of chunks) { | |
| // eslint-disable-next-line no-await-in-loop | |
| await ctx.reply(c, { parse_mode: 'Markdown' }); | |
| bot.telegram.sendMessage = originalTelegramMethods.sendMessage; | |
| bot.telegram.editMessageText = originalTelegramMethods.editMessageText; | |
| bot.telegram.answerCallbackQuery = originalTelegramMethods.answerCallbackQuery; | |
| bot.telegram.deleteMessage = originalTelegramMethods.deleteMessage; | |
| bot.telegram.pinChatMessage = originalTelegramMethods.pinChatMessage; | |
| bot.telegram.unpinChatMessage = originalTelegramMethods.unpinChatMessage; | |
| bot.telegram.sendPhoto = originalTelegramMethods.sendPhoto; | |
| bot.telegram.sendAnimation = originalTelegramMethods.sendAnimation; | |
| bot.telegram.sendDocument = originalTelegramMethods.sendDocument; | |
| // Restore Telegram methods BEFORE sending DM reports | |
| bot.telegram.sendMessage = originalTelegramMethods.sendMessage; | |
| bot.telegram.editMessageText = originalTelegramMethods.editMessageText; | |
| bot.telegram.answerCallbackQuery = originalTelegramMethods.answerCallbackQuery; | |
| bot.telegram.deleteMessage = originalTelegramMethods.deleteMessage; | |
| bot.telegram.pinChatMessage = originalTelegramMethods.pinChatMessage; | |
| bot.telegram.unpinChatMessage = originalTelegramMethods.unpinChatMessage; | |
| bot.telegram.sendPhoto = originalTelegramMethods.sendPhoto; | |
| bot.telegram.sendAnimation = originalTelegramMethods.sendAnimation; | |
| bot.telegram.sendDocument = originalTelegramMethods.sendDocument; | |
| for (const adminId of ADMIN_IDS) { | |
| const chunks = chunkTextForTelegram(dmReport, 3500); | |
| for (const chunk of chunks) { | |
| // eslint-disable-next-line no-await-in-loop | |
| await bot.telegram.sendMessage(adminId, chunk, { parse_mode: 'Markdown' }).catch(() => {}); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 14181 - 14198, The admin DM loop currently calls
bot.telegram.sendMessage while sendMessage is still stubbed (ADMIN_IDS loop
using chunkTextForTelegram and awaiting bot.telegram.sendMessage), so messages
are recorded but not sent; move the restoration of the originalTelegramMethods
(restore bot.telegram.sendMessage, editMessageText, answerCallbackQuery,
deleteMessage, pinChatMessage, unpinChatMessage, sendPhoto, sendAnimation,
sendDocument from originalTelegramMethods) to before the ADMIN_IDS loop so the
real methods are used when iterating and sending chunks to admins.
Comment on lines
+434
to
+435
| "mapExcerpt": "# RUNEWAGER_FUNCTIONALITY_MAP.md\n\n_Last audited: 2026-03-04 (/runewager-audit pass — 0 critical, 0 warnings after fix)_\n_Source of truth files: `index.js`, `test/*.test.js`, scripts under `scripts/`, deployment/runtime docs in repo root._\n\n---\n\n## 1. High‑Level Overview\n\nRunewager is a Telegraf-based Telegram bot that provides:\n- User onboarding (age gate, account/Discord guidance, username linking).\n- Promo flows (DB-backed promo manager + eligibility + claim lifecycle).\n- Giveaway flows (creation, join, eligibility checks, auto finalization, admin controls).\n- Helpful Tooltips (scheduled/random posts to configured target chat; formerly \"Content Drops\").\n- Admin operations (broadcasts, diagnostics, SSHV console, bug triage, backups).\n- Runtime health and deploy tooling (`/health`, scripts, systemd template).\n\nNavigation is driven by inline menus plus command aliases. Persistent user/admin menu headers are used in DMs for fast access.\n\n## 2. System Architecture Summary\n\n### Core runtime\n- Single runtime app in `index.js`.\n- Telegraf command handlers + callback handlers.\n- Per-user mutable state stored in memory and persisted to JSON runtime snapshots.\n\n### State/storage layers\n- In-memory stores: users, giveaway state, analytics, promo manager store, helpful tooltips store (`tipsStore`), broadcast config, SSHV sessions.\n- File persistence under `data/` (runtime snapshots + promo DB + optional backups).\n- Periodic persistence timer + startup restore.\n\n### Routing model\n- `bot.command(...)` for slash commands.\n- `bot.action(...)` for inline button callbacks (literal + regex handlers).\n- `bot.on('text')` for pending “await input” state machine.\n\n### Utility subsystems\n- Eligibility checks, validation helpers, markdown escaping, analytics/event logging.\n- Admin audit logging (`adminLog` and NDJSON appends).\n- SSHV command execution safety checks and state restoration.\n\n## 3. Global Bot Behaviors\n\n- Unknown command handling routes to a structured error block with guidanc" | ||
| } No newline at end of file |
There was a problem hiding this comment.
Truncated mapExcerpt may cause parsing issues downstream.
The mapExcerpt field appears to be truncated mid-sentence (ends with "error block with guidanc"). If this field is consumed by other tooling or displayed to users, the truncation could cause confusion or JSON parsing edge cases if not handled properly.
Consider either:
- Completing the excerpt with proper truncation indicator (e.g.,
...) - Removing the field if full content isn't needed
- Storing the full content if it's required
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@qa/context/bot_capabilities.json` around lines 434 - 435, The "mapExcerpt"
value in qa/context/bot_capabilities.json is truncated mid-sentence ("error
block with guidanc") which can break downstream consumers; update the mapExcerpt
field (the JSON string value for "mapExcerpt") to either: 1) complete the
excerpt sensibly or append a clear truncation marker like "..." so it doesn't
end mid-word, 2) remove the mapExcerpt field if not needed, or 3) replace it
with the full intended text — ensure the edited "mapExcerpt" string remains
valid JSON (properly escaped and closed) and that any code reading this key
(e.g., tools that parse bot_capabilities.json) will receive a well-formed
string.
Comment on lines
+43
to
+51
| function fnHasRequireAdmin(fnNode) { | ||
| let found = false; | ||
| traverse(fnNode, { | ||
| CallExpression(p) { | ||
| if (getCalleeName(p.node.callee) === 'requireAdmin') found = true; | ||
| } | ||
| }, fnNode.scope, fnNode.state, fnNode.parentPath); | ||
| return found; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how fnHasRequireAdmin is called
rg -n -A3 'fnHasRequireAdmin' scripts/ast_audit.jsRepository: gamblecodezcom/Runewager
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Get the full context of fnHasRequireAdmin function and surrounding code
head -80 scripts/ast_audit.js | tail -50Repository: gamblecodezcom/Runewager
Length of output: 2690
🏁 Script executed:
#!/bin/bash
# Check the traverse import and understand what traverse() expects
rg -n -B5 'traverse\(' scripts/ast_audit.js | head -40Repository: gamblecodezcom/Runewager
Length of output: 1496
🏁 Script executed:
#!/bin/bash
# Search for all calls to fnHasRequireAdmin across the entire codebase
rg -n 'fnHasRequireAdmin\s*\(' --type jsRepository: gamblecodezcom/Runewager
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check the imports and traverse function signature
head -20 scripts/ast_audit.jsRepository: gamblecodezcom/Runewager
Length of output: 725
🏁 Script executed:
#!/bin/bash
# Check if fnHasRequireAdmin is exported or used elsewhere
rg -n 'fnHasRequireAdmin' --type jsRepository: gamblecodezcom/Runewager
Length of output: 128
Remove the unused fnHasRequireAdmin function.
The function is defined at lines 43-51 but is never called anywhere in the codebase. Dead code should be removed to maintain clarity and reduce maintenance burden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ast_audit.js` around lines 43 - 51, Remove the dead helper
fnHasRequireAdmin: locate the function definition named fnHasRequireAdmin (which
uses traverse and getCalleeName to check for requireAdmin) and delete that
entire function declaration since it is never used; also remove any now-unused
imports/variables that were only referenced by this function (e.g., traverse or
related helpers) if they become unused after deletion.
Comment on lines
+127
to
+132
| const actionRegexes = data.actions.filter(a => a.pattern.kind === 'regex').map(a => { | ||
| const last = a.pattern.value.lastIndexOf('/'); | ||
| const body = a.pattern.value.slice(1, last); | ||
| const flags = a.pattern.value.slice(last + 1); | ||
| return { ...a, re: new RegExp(body, flags) }; | ||
| }); |
There was a problem hiding this comment.
Wrap RegExp construction in try/catch.
Line 131 constructs a RegExp from user-controlled pattern values without error handling. Malformed regex patterns in the AST will cause the script to crash.
🛡️ Suggested fix
const actionRegexes = data.actions.filter(a => a.pattern.kind === 'regex').map(a => {
const last = a.pattern.value.lastIndexOf('/');
const body = a.pattern.value.slice(1, last);
const flags = a.pattern.value.slice(last + 1);
- return { ...a, re: new RegExp(body, flags) };
-});
+ try {
+ return { ...a, re: new RegExp(body, flags) };
+ } catch (err) {
+ console.warn(`Warning: Invalid regex ${a.pattern.value} at ${a.file}:${a.line}`);
+ return { ...a, re: null };
+ }
+}).filter(a => a.re !== null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const actionRegexes = data.actions.filter(a => a.pattern.kind === 'regex').map(a => { | |
| const last = a.pattern.value.lastIndexOf('/'); | |
| const body = a.pattern.value.slice(1, last); | |
| const flags = a.pattern.value.slice(last + 1); | |
| return { ...a, re: new RegExp(body, flags) }; | |
| }); | |
| const actionRegexes = data.actions.filter(a => a.pattern.kind === 'regex').map(a => { | |
| const last = a.pattern.value.lastIndexOf('/'); | |
| const body = a.pattern.value.slice(1, last); | |
| const flags = a.pattern.value.slice(last + 1); | |
| try { | |
| return { ...a, re: new RegExp(body, flags) }; | |
| } catch (err) { | |
| console.warn(`Warning: Invalid regex ${a.pattern.value} at ${a.file}:${a.line}`); | |
| return { ...a, re: null }; | |
| } | |
| }).filter(a => a.re !== null); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ast_audit.js` around lines 127 - 132, The code builds RegExp objects
from user-controlled strings in the actionRegexes mapping (see actionRegexes,
a.pattern.value and new RegExp) without error handling; wrap the RegExp
construction in a try/catch so malformed patterns don't crash the script, and on
error either skip that action or attach a null/invalid marker and log the
offending pattern (use process.stderr or logger) so the script continues safely;
ensure the returned object shape stays consistent (e.g., return { ...a, re:
null, invalidPattern: true } or omit the entry) so downstream code can handle
missing/invalid regexes.
Comment on lines
+154
to
+155
| 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)); |
There was a problem hiding this comment.
Ensure reports directory exists before writing.
Line 155 writes to 'reports/ast_audit.json' without ensuring the directory exists. This will fail if the reports/ directory doesn't exist.
🛡️ Suggested fix
+const reportDir = path.join(root, 'reports');
+fs.mkdirSync(reportDir, { recursive: true });
+
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));
+fs.writeFileSync(path.join(reportDir, 'ast_audit.json'), JSON.stringify(report, null, 2));Also update line 217 similarly:
-fs.writeFileSync('reports/structural_audit.md', md);
+fs.writeFileSync(path.join(reportDir, 'structural_audit.md'), md);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ast_audit.js` around lines 154 - 155, Before calling fs.writeFileSync
to write 'reports/ast_audit.json' ensure the reports directory exists by
creating it if necessary (e.g., call fs.mkdirSync('reports', { recursive: true
}) or use fs.promises.mkdir) immediately before the fs.writeFileSync that writes
report; apply the same fix to the other fs.writeFileSync that writes into the
'reports' directory later in the file so both write operations won't fail when
the directory is missing.
Comment on lines
+19
to
+20
| const { bot } = require('../index.js'); | ||
| const ast = JSON.parse(fs.readFileSync(astPath, 'utf8')); |
There was a problem hiding this comment.
Add defensive handling for missing ast_audit.json.
Line 20 synchronously reads ast_audit.json without error handling. If this file doesn't exist (e.g., when running testall_runtime.js standalone without first running ast_audit.js), the script crashes with an unhelpful error.
🛡️ Suggested defensive check
const { bot } = require('../index.js');
-const ast = JSON.parse(fs.readFileSync(astPath, 'utf8'));
+let ast;
+try {
+ ast = JSON.parse(fs.readFileSync(astPath, 'utf8'));
+} catch (err) {
+ console.error(JSON.stringify({
+ status: 'error',
+ error: `Failed to read AST audit: ${err.message}. Run scripts/ast_audit.js first.`
+ }));
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { bot } = require('../index.js'); | |
| const ast = JSON.parse(fs.readFileSync(astPath, 'utf8')); | |
| const { bot } = require('../index.js'); | |
| let ast; | |
| try { | |
| ast = JSON.parse(fs.readFileSync(astPath, 'utf8')); | |
| } catch (err) { | |
| console.error(JSON.stringify({ | |
| status: 'error', | |
| error: `Failed to read AST audit: ${err.message}. Run scripts/ast_audit.js first.` | |
| })); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/testall_runtime.js` around lines 19 - 20, The code reads
ast_audit.json into ast without handling a missing file; update the startup in
scripts/testall_runtime.js to defensively check for the file at astPath (e.g.,
using fs.existsSync or try/catch around JSON.parse) before calling
fs.readFileSync and JSON.parse, and if missing emit a clear error/log via
process.stderr or processLogger and either create a sensible default ast object
or exit gracefully; ensure references to astPath and the ast variable (and the
surrounding require('../index.js') usage) are updated so downstream code won't
run against undefined ast.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+106
to
+109
| 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 .then() without .catch() detector is logically flawed: for typical promise chains like foo().then(...).catch(...), the CallExpression for .then has a parent MemberExpression whose own parent is the .catch(...) CallExpression, so checking only the direct parent CallExpression misses these and incorrectly classifies them as unhandled, inflating promises.thenNoCatch with false positives. [logic error]
Severity Level: Major ⚠️
- ⚠️ Async audit reports inflated with benign .then chains.
- ⚠️ /testall diagnostic summary overstates unhandled promise chains.
- ⚠️ Operators may chase non-existent error handling issues.
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')) { | |
| const parentPath = p.parentPath; | |
| const parent = parentPath?.node; | |
| let chainedCatch = false; | |
| if (parent?.type === 'CallExpression' && getCalleeName(parent.callee)?.endsWith('.catch')) { | |
| chainedCatch = true; | |
| } else if (parent?.type === 'MemberExpression') { | |
| const grandParent = parentPath.parentPath?.node; | |
| if (grandParent?.type === 'CallExpression' && getCalleeName(grandParent.callee)?.endsWith('.catch')) { | |
| chainedCatch = true; | |
| } | |
| } | |
| if (!chainedCatch) data.promises.thenNoCatch.push({ file: rfile, line, callee }); | |
| } |
Steps of Reproduction ✅
1. Run the AST auditor via `node scripts/ast_audit.js` (entry at
`scripts/ast_audit.js:1`), which traverses all JS/TS files and, in the `CallExpression`
visitor at `scripts/ast_audit.js:83-111`, pushes entries into `data.promises.thenNoCatch`
when `callee?.endsWith('.then')`.
2. In `rateLimiter.js:93-107`, inspect the per-chat queue implementation where `const
newTail = prevTail.then(async () => { ... }).catch(() => {});` chains a `.catch()` call
after `.then()`, and similarly in `_globalSlot` at `rateLimiter.js:49-72` where
`_globalTail = _globalTail.then(async () => { ... }).catch(() => {});` uses the same
pattern.
3. For each `.then` in these chains, the Babel AST produces a `CallExpression` node whose
`callee` is a `MemberExpression` ending in `.then`; the parent path for that `.then` call
is a `MemberExpression` whose grandparent node is the `.catch(...)` `CallExpression`, so
at `scripts/ast_audit.js:106-109` the `parent` variable is a `MemberExpression`, not a
`CallExpression`, causing `chainedCatch` to be `false` and the `.then` calls to be
incorrectly recorded in `data.promises.thenNoCatch`.
4. When the script finishes, it writes `report.promises.thenNoCatch` to
`reports/ast_audit.json` and surfaces it in `reports/structural_audit.md` under "Async &
Error Handling" at `scripts/ast_audit.js:188-190`, and this count is further surfaced in
`/testall` summaries via `summary.astSnapshot.asyncThenWithoutCatch` in
`scripts/testall_runtime.js:270` and `index.js:14109`, inflating the reported number of
".then() without direct chained .catch()" instances even though these specific chains do
have a `.catch()` attached.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/ast_audit.js
**Line:** 106:109
**Comment:**
*Logic Error: The `.then()` without `.catch()` detector is logically flawed: for typical promise chains like `foo().then(...).catch(...)`, the `CallExpression` for `.then` has a parent `MemberExpression` whose own parent is the `.catch(...)` `CallExpression`, so checking only the direct parent `CallExpression` misses these and incorrectly classifies them as unhandled, inflating `promises.thenNoCatch` with false positives.
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. |
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.
@sourcery-ai resolve
User description
Motivation
Description
scripts/ast_audit.jsthat writesreports/ast_audit.jsonandreports/structural_audit.mdand wired it into the diagnostic workflow.scripts/testall_runtime.jsand in-process/testallenhancements inindex.jswhich create ephemeral runtime snapshots (createTestAllStateSnapshot/restoreTestAllStateSnapshot), simulateBOT_KNOWN_COMMANDSand AST-discovered actions viabot.handleUpdate, persist artifacts (reports/testall_report.json,reports/testall_report.md,reports/testall_report.txt), and DM batched summaries toADMIN_IDS./testallvia therotating-file-streamdependency andcreateTestAllRotatingLogger(logs written todata/logs/testall/testall.log*with daily/10M rotation, gzip and retention)./testallto admin-only (isAdmin(ctx)), non-blocking step failures, progress steps (withStep), deterministic API stubbing during simulation, andTESTALL_EXPORTS=1export path for test harnesses.package.json/package-lock.jsonupdates (addedrotating-file-streamand AST dev deps), exportedbotunder test mode, and updated docs (docs/INDEX.md,docs/features/10-deploy-ops.md,RUNEWAGER_FUNCTIONALITY_MAP.md,AGENTS/SESSION_LOG.md) and QA contexts to reflect new audit/reporting artifacts and invariants.Testing
node scripts/ast_audit.js, which completed and wrotereports/ast_audit.jsonandreports/structural_audit.md.node scripts/testall_runtime.js, which completed and producedreports/testall_report.json/.md/.txtand rotating-log artifacts, and reported a final summary status ofissuesdue to captured async errors (these are observable in the report).npm test, which passed (60/60tests passing).Codex Task
Summary by Sourcery
Introduce an admin-only, comprehensive in-process
/testalldiagnostics runner that drives AST auditing and deterministic runtime simulation, persists structured reports and rotating logs, and documents the new audit workflow and invariants.New Features:
/testallcommand to orchestrate AST audit + runtime simulation, snapshot and restore runtime state, summarize results, and DM batched reports to admins./testallexecutions with daily/size-based rotation, compression, and retention policy.Enhancements:
BOT_KNOWN_COMMANDSto cover additional admin and user commands while documenting intentional normalization behavior for aliases./testallto be admin-only, non-blocking on individual step failures, and to log unmatched callback data via the global fallback handler for better observability./testallbehavior, audit invariants, and logging policy.Build:
rotating-file-streamas a runtime dependency and Babel parser/traverse/types as dev dependencies for AST-based auditing.Documentation:
/testallaudit pipeline, generated report artifacts, rotating log policy, and command/callback audit invariants in the main docs and functionality map.Tests:
reports/to serve as reference outputs for the new diagnostics.CodeAnt-AI Description
Add comprehensive admin-only /testall runner, AST structural auditor, deterministic reports and rotating logs
What Changed
Impact
✅ Deterministic audit artifacts for admins✅ Rotating diagnostic logs retained for investigations✅ Batched admin DM reports for faster debugging💡 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