Employer-ready pass: docs, live-AI demo, Cloudflare bundle — plus the AI-fallback bug it uncovered - #113
Merged
Merged
Conversation
aiDiagnose ran at max_tokens=1024 with a 10s timeout. No current model can return a complete structured diagnosis in that budget: nine live sonnet-5 runs against the real pg-replication prompt came back at 2259-3742 output tokens and 28-40s. Every response arrived truncated with stop_reason=max_tokens, JSON.parse threw "Unterminated string", aiDiagnose returned null, and the caller degraded to its rule-based heuristics. The failure was invisible: it looked like "AI is unavailable" rather than "the budget is too small". It affected the pg-replication agent, the kafka agent, and `bundle ingest` — regardless of whether an API key was set. - ai-diagnosis.ts: 60s / 6144 tokens (above the measured 3742 ceiling, since 4096 leaves ~9% headroom and one wordier finding list would truncate again) - ai-client.ts fallbacks: 60s / 4096 - ai-explainer.ts: 4096 / 30s, measured 1407-1484 tokens at 15.2-16.4s - root-cause-synthesis.ts: 4096 / 30s, measured 749-1522 tokens at 8.8-17.7s Both are foreground paths with an operator waiting, so they take 30s rather than 60s: a fast fallback beats a long hang. Both were falling back before. - root-cause-synthesis.ts also parsed with a bare .trim() and no stripCodeFence, so 2 of 3 live calls threw on a ```json fence — a failure mode independent of the token budget. - parseStandardDiagnosisResponse now repairs raw control characters inside JSON string literals and retries once, after a live response lost its diagnosis to "Bad control character in string literal at position 1741". - Corrected 8 stale "10s / 1024" claims, including a spec reference to claude-sonnet-4-20250514, retired 2026-06-15. All 7 callClaude call sites audited; each passes both values explicitly, so no interactive path regressed. 18 hermetic regression tests added, 8 of them verified to fail against the old constants. No live API calls in tests. Still at 1024: ai-diagnosis-universal.ts:126 and ask.ts:218. Neither parses its response, so truncation there is a visibly cut-off answer rather than a silent fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two copy-pasteable dead ends in scan output. scan.ts pointed at `crisismode init --plugin my-check`, but init only parses --agent, so the suggested command errors out. incident-summary.ts suggested `crisismode diagnose <finding-id>`. diagnose reads positionals[0] as a *target* name, and only PLUG-<n> is routed to a plugin's diagnose verb — so the hint worked for plugin findings and failed with `Target "PG-001" not found in config` for every agent finding. diagnoseCommandFor now derives the addressable target from the trailing parenthesized group scan builds into `service`, keeps the raw id for PLUG-<n>, and falls back to the bare command rather than emitting something broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README was 477 lines and doubled as a manual, while QUICKSTART (205) and GETTING_STARTED (263) largely repeated it — three competing onboarding paths. - README down to 258 lines; depth moved into docs/cli-reference.md and docs/coverage.md - one home per audience: QUICKSTART = using the CLI, GETTING_STARTED = developing on it, CONTRIBUTING = contributing (its duplicated setup section now links out) Corrected claims that contradicted the codebase: - the agent table listed Redis, AWS, deploy-rollback, db-migration, queue-backlog and config-drift as "Live (execute-capable)" when their manifests say simulator_only — the README was overstating what the tool itself reports. Now uses the real two-label vocabulary and the verified 9-of-26 live-validated count. - both agent guides told readers to set plugin.maturity to "beta"/"stable"; neither is a valid PluginMaturity - playbook-authoring.md's flagship example failed validation until a missing capability id was added - GETTING_STARTED claimed 19 agents against 26 actual Every command and flag verified against a fresh build; no broken internal links or anchors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
record-demo.sh ran under `env -u ANTHROPIC_API_KEY` so the capture was offline and deterministic. That also meant the recording showed the rule-based fallback — a fixed 92% confidence with three findings and no root cause — rather than the product's actual AI path. Recording now runs with the live key against the default model (sonnet-5, no CRISISMODE_AI_MODEL override) so it shows what a user gets out of the box: replication_lag_cascade at 78% with a model-authored root-cause paragraph and six recommendations. 56.2s, 314 events. The trade-off is stated in the script header: the recording makes a live API call and is no longer byte-reproducible. pace-cast.mjs now caps idle gaps at 4s so the real 28-56s model wait does not ship as a frozen frame — the cast was already re-timed for readability rather than real-time, and the header documents the cap. The GIF teaser is generated by the same script but not committed here; it exceeds the repo's 1MB pre-commit ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- vendors asciinema-player v3.17.0 and plays site/assets/demo.cast inline, with the GIF kept only as a fallback (it is not fetched when the player works). Closes a hole where JS-on-but-player-missing rendered an empty box with no GIF at all. - rewrites the demo alt text. It claimed 92% confidence, which is the rule-based fallback's fixed figure; the live-AI recording shows replication_lag_cascade at 78% with a primary-side WAL-generation root cause. It now also says the GIF is only the opening excerpt. - discloses that playback is pace-adjusted and the real AI call took ~30-60s, rather than letting the capped idle gaps imply the tool is faster than it is - player options re-checked against the new cast (314 events, 56.24s, max idle gap 4.0s; poster npt:0:03 still lands on the title card) scripts/package-site.sh emits output/crisismode-site.zip for Cloudflare Pages direct upload: index.html at the zip root plus blog/ and assets/, no .DS_Store, no resource forks, byte-identical across runs. Removes site/index.html.zip: committed 2026-07-20, 21KB, a stale index.html predating the player and no assets/ at all — uploading it to Pages would deploy a page whose player JS, CSS, cast and GIF all 404. Verified by real browser render (headless Chromium against a local server): all assets 200, player paints with the 78% diagnosis and plan table visible, zero console errors, fallback swaps in correctly — repeated against the extracted zip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The teaser rendered at 2.8MB, which the pre-commit hook rejects. Preserving the old Phase 7 cut was not an option: rendering through the plan table costs 1.3MB even at font-size 10 and 4fps, which is past legible. Re-anchors the cut on the Phase 6 heading and drops to 2fps: 905KiB, and it still contains the part worth recording live — the model's own root-cause paragraph at 78% confidence. The anchor stays content-based rather than a fixed timestamp, so a wordier AI response cannot silently truncate it. The size guard now fails the build at 1MB instead of warning at 3MB, so this regresses loudly rather than at commit time. Alt text updated: the excerpt now ends as plan creation begins, not at plan validation. Cloudflare bundle drops from 2.8MB to 990KiB total. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe change restructures project documentation, adds CLI and coverage references, updates AI response budgets and parsing, corrects diagnosis guidance, and adds scripts and assets for recording, packaging, and playing the recovery demo. ChangesDocumentation and runtime updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #113 +/- ##
==========================================
+ Coverage 68.79% 70.16% +1.37%
==========================================
Files 278 278
Lines 14856 14932 +76
Branches 4468 4491 +23
==========================================
+ Hits 10220 10477 +257
+ Misses 3007 2837 -170
+ Partials 1629 1618 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
site/assets/asciinema-player.css (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude this vendored file from Stylelint and record its provenance.
This file is the upstream asciinema-player distribution CSS. Stylelint reports three violations in it:
word-wrapis deprecated at line 120, andMenloandConsolasuse non-lowercase keywords at line 2. Do not edit them. Any edit is lost on the next vendor update and diverges from upstream. Add the path to.stylelintignoreinstead.The file also carries no version or license marker, unlike the other files in this PR, which use SPDX headers. Add a short comment that names the upstream project, the pinned version, and the license, so the vendored copy can be refreshed and attributed.
♻️ Proposed provenance header
+/* + * Vendored from asciinema-player v<VERSION> — https://github.com/asciinema/asciinema-player + * Licensed under the Apache License 2.0. Do not edit; refresh from upstream. + */ .ap-default-term-ff {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/assets/asciinema-player.css` around lines 1 - 2, Add site/assets/asciinema-player.css to .stylelintignore without modifying its vendored CSS content, and add a concise provenance comment identifying asciinema-player, the pinned upstream version, and its license.Source: Linters/SAST tools
scripts/package-site.sh (1)
92-109: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCapture the zip listing once and reuse it.
The script runs
unzip -Z1 "$ZIP"four times. Each run pipes intogrep -qorwc. Withset -o pipefail,grep -qcan close the pipe after the first match and leaveunzipkilled bySIGPIPE, which makes the pipeline status non-zero. At line 93 that turns a correct archive into a false "index.html is not at the zip root" failure. At line 97 it can hide a real leak, because theifcondition then evaluates false. The output is small today, so the pipe buffer usually absorbs it, but a single capture removes the interaction and the repeated work.♻️ Proposed refactor
+ENTRIES="$(unzip -Z1 "$ZIP")" + # Assert the layout Pages needs rather than asking the reader to eyeball it. -unzip -Z1 "$ZIP" | grep -qx 'index.html' || { +printf '%s\n' "$ENTRIES" | grep -qx 'index.html' || { echo "error: index.html is not at the zip root; Pages would 404 on /." >&2 exit 1 } -if unzip -Z1 "$ZIP" | grep -qE '(^|/)(\.DS_Store|\._|__MACOSX/)|\.zip$'; then +if printf '%s\n' "$ENTRIES" | grep -qE '(^|/)(\.DS_Store|\._|__MACOSX/)|\.zip$'; then echo "error: excluded files leaked into the archive:" >&2 - unzip -Z1 "$ZIP" | grep -E '(^|/)(\.DS_Store|\._|__MACOSX/)|\.zip$' >&2 + printf '%s\n' "$ENTRIES" | grep -E '(^|/)(\.DS_Store|\._|__MACOSX/)|\.zip$' >&2 exit 1 fi @@ -echo " $((ZIP_BYTES / 1024)) KiB ($ZIP_BYTES bytes), $(unzip -Z1 "$ZIP" | wc -l | tr -d ' ') entries" +echo " $((ZIP_BYTES / 1024)) KiB ($ZIP_BYTES bytes), $(printf '%s\n' "$ENTRIES" | wc -l | tr -d ' ') entries"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/package-site.sh` around lines 92 - 109, Capture the output of unzip -Z1 "$ZIP" once in a listing variable before the validation checks, then reuse that captured listing for the index.html assertion, excluded-file detection and diagnostic output, while preserving the existing patterns and archive summary behavior.site/index.html (1)
933-940: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProvide a text alternative when the player runs.
The GIF
alttext at line 938 is the only description of the recording, and it lives inside<noscript>. When JavaScript is enabled, the browser never exposes that text, and the player mount at line 933 has no accessible name. Assistive technology then presents an unlabeled region. Add a label on the mount, or add a visually hidden summary next to it, so the recording is described in both paths.♻️ Proposed change
- <div class="terminal-body terminal-player" id="terminal-player"></div> + <div class="terminal-body terminal-player" id="terminal-player" + role="group" aria-label="Terminal recording of the crisismode demo: a PostgreSQL replication-lag cascade, diagnosed live and recovered through a 10-step plan with a human approval gate."></div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/index.html` around lines 933 - 940, Add an accessible name or visually hidden text description to the terminal player mount identified by `id="terminal-player"`, ensuring the recording is described when JavaScript runs while preserving the existing `<noscript>` fallback and its GIF alt text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guide-verification/2026-08-09-docs-pass-report.md`:
- Around line 114-117: Update the “Flags” statement in the report to avoid
claiming that only --json, --target, and --agent appear across all
documentation. Either list every documented flag, including those in
docs/cli-reference.md, or explicitly limit the sentence to newly added flags.
In `@docs/guides/your-first-agent.md`:
- Around line 772-777: Update the maturity-label guidance around plugin.maturity
to distinguish diagnosis validation from execute-verified recovery: allow a real
deployment with validated diagnosis to use "live_validated" without requiring a
mutating recovery, reserve "dry_run_only" for diagnosis validated only through
dry runs, and retain "simulator_only" until live diagnosis is confirmed.
In `@docs/playbook-authoring.md`:
- Around line 254-260: The “Evict expired keys” playbook entry must not describe
arbitrary deletion as TTL-based eviction. Update the command and surrounding
metadata to use a bounded, expiry-aware operation that only targets expired
keys, or revise the title, capability, risk, preservation, and success criteria
to accurately describe deleting arbitrary keys.
In `@GETTING_STARTED.md`:
- Around line 176-184: Resolve MD040 by adding the text language identifier to
every specified fenced block: GETTING_STARTED.md:176-184, QUICKSTART.md:40-58,
README.md:58-66 and 91-115, and docs/cli-reference.md:107-109, 114-116, and
130-133. Preserve all existing project-layout, scan-output, recovery-transcript,
and row contents.
- Around line 18-19: Update the pnpm setup instruction in GETTING_STARTED.md to
install the version declared by the packageManager field in package.json, such
as by enabling Corepack or explicitly pinning pnpm@10.30.3, so local setup
matches CI.
In `@scripts/pace-cast.mjs`:
- Around line 144-161: Ensure the verification failure path in the pace-cast
output flow removes the already-written outPath before exiting, so a rejected
cast cannot overwrite the committed asset; keep successful output behavior
unchanged.
In `@scripts/record-demo.sh`:
- Line 30: Validate the DECISION argument in scripts/record-demo.sh immediately
after its default assignment, allowing only approve, skip, or reject as
documented. Reject any other value with an error and exit before recording or
invoking the driver here-document, while preserving the existing behavior for
valid values.
- Around line 68-69: Update the temporary-path setup in the demo recording
script to create a temporary directory, then define DRIVER as an .exp file
inside that directory. Keep the existing cleanup trap, but remove the directory
recursively so both the mktemp-created location and the generated driver file
are cleaned up atomically.
- Around line 170-176: Update the agg installation hint in the script’s
tool-check logic to require version 1.9.0 or newer, matching the --select usage
in the GIF generation command. Keep the existing missing-tool validation
behavior unchanged.
In `@site/assets/demo.cast`:
- Around line 33-37: Update getCatalogEntry, matchCatalog, and isCatalogCovered
in the catalog flow so entries whose expiresAt is earlier than the demo/current
trigger time are treated as out of date and do not report catalog coverage;
include the expiry validation in isCatalogCovered output, then re-record
site/assets/demo.cast with the corrected result.
In `@site/index.html`:
- Around line 1423-1445: Update the initialization flow around
AsciinemaPlayer.create to fetch and parse assets/demo.cast before constructing
the player, calling showTerminalFallback() when fetching or parsing fails. Pass
the successfully loaded cast data into AsciinemaPlayer.create, while retaining
its existing try/catch to handle synchronous player-construction failures.
In `@src/__tests__/ai-response-budgets.test.ts`:
- Around line 167-173: Add a valid statePreservation.before capture to the
elevated-risk system_action fixture identified by stepId step-004, preserving
the existing action fields and satisfying the recovery-plan safety contract.
---
Nitpick comments:
In `@scripts/package-site.sh`:
- Around line 92-109: Capture the output of unzip -Z1 "$ZIP" once in a listing
variable before the validation checks, then reuse that captured listing for the
index.html assertion, excluded-file detection and diagnostic output, while
preserving the existing patterns and archive summary behavior.
In `@site/assets/asciinema-player.css`:
- Around line 1-2: Add site/assets/asciinema-player.css to .stylelintignore
without modifying its vendored CSS content, and add a concise provenance comment
identifying asciinema-player, the pinned upstream version, and its license.
In `@site/index.html`:
- Around line 933-940: Add an accessible name or visually hidden text
description to the terminal player mount identified by `id="terminal-player"`,
ensuring the recording is described when JavaScript runs while preserving the
existing `<noscript>` fallback and its GIF alt text.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac8e5c1d-850e-45d9-94e0-ea2d68548436
⛔ Files ignored due to path filters (3)
site/assets/asciinema-player.min.jsis excluded by!**/*.min.jssite/assets/demo.gifis excluded by!**/*.gifsite/index.html.zipis excluded by!**/*.zip
📒 Files selected for processing (32)
CONTRIBUTING.mdGETTING_STARTED.mdQUICKSTART.mdREADME.mddocs/architecture.mddocs/cli-reference.mddocs/coverage.mddocs/guide-verification/2026-08-09-docs-pass-report.mddocs/guides/creating-a-recovery-agent.mddocs/guides/your-first-agent.mddocs/playbook-authoring.mdscripts/pace-cast.mjsscripts/package-site.shscripts/record-demo.shsite/assets/asciinema-player.csssite/assets/demo.castsite/index.htmlspecs/architecture/operator-health-and-ai-services.mdsrc/__tests__/ai-client.test.tssrc/__tests__/ai-diagnosis-token-budget.test.tssrc/__tests__/ai-response-budgets.test.tssrc/__tests__/ai-summary.test.tssrc/__tests__/incident-summary.test.tssrc/agent/kafka/ai-diagnosis.tssrc/agent/pg-replication/ai-diagnosis.tssrc/cli/commands/scan.tssrc/cli/incident-summary.tssrc/framework/ai-client.tssrc/framework/ai-diagnosis.tssrc/framework/ai-explainer.tssrc/framework/evidence-bundle-respond.tssrc/framework/root-cause-synthesis.ts
Comment on lines
+33
to
+37
| [0.1,"o","\u001b[2m Approved by: jane.chen@example.com\u001b[22m\r\r\n"] | ||
| [0.1,"o","\u001b[2m Expires: 2026-05-15T10:00:00Z\u001b[22m\r\r\n"] | ||
| [0.1,"o","\u001b[2m Covers: routine, elevated risk levels\u001b[22m\r\r\n"] | ||
| [0.05,"o","\r\r\n"] | ||
| [0.16,"o","\u001b[32m ✓ Catalog entry found for this scenario\u001b[39m\r\r\n"] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the demo catalog fixture and the catalog expiry check.
rg -n -C5 -e 'pg-replication-standard-recovery' -e '2026-05-15' -e 'jane.chen' --glob '!site/assets/*'
rg -n -C6 -e 'expiresAt' -e 'expires' -e 'isExpired' --glob '*.ts' -g '!**/*.test.ts' | rg -n -i -C6 'catalog'Repository: trs-80/crisismode
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -u
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== locate cast and keywords =="
rg -n -C4 -e 'Approved by:|Expires:|Covers:|Catalog entry found|Catalog entry.*matched|jane.chen@example.com|2026-05-15|2026-08-09|pg-replication-standard-recovery' . || true
echo "== cast sections =="
if [ -f site/assets/demo.cast ]; then
nl -ba site/assets/demo.cast | sed -n '1,80p'
echo "---"
nl -ba site/assets/demo.cast | sed -n '140,190p'
fiRepository: trs-80/crisismode
Length of output: 22746
🏁 Script executed:
#!/bin/bash
set -u
echo "== src/framework/catalog.ts =="
cat -n src/framework/catalog.ts | sed -n '1,190p'
echo "== src/demo/runner.ts relevant =="
cat -n src/demo/runner.ts | sed -n '1,90p'
echo "== src/framework/display.ts relevant =="
cat -n site/assets/demo.cast | sed -n '160,176p'
echo "== cast phase 2/3 lines =="
cat -n site/assets/demo.cast | sed -n '14,38p'
echo "== find all shouldRequireApproval calls =="
rg -n -C4 'shouldRequireApproval|catalogCovers' src --glob '*.ts'Repository: trs-80/crisismode
Length of output: 13838
Treat expired catalog entries as out of date.
The demo publishes jane.chen@example.com, which is acceptable. The demo-trigger date is 2026-08-09T15:37:18.237Z, but getCatalogEntry() still has expiresAt: 2026-05-15T10:00:00Z; matchCatalog() then ignores the expiry and reports a match. Fix the expiresAt handling in src/framework/catalog.ts and include this check in isCatalogCovered() output, then re-record the cast.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/assets/demo.cast` around lines 33 - 37, Update getCatalogEntry,
matchCatalog, and isCatalogCovered in the catalog flow so entries whose
expiresAt is earlier than the demo/current trigger time are treated as out of
date and do not report catalog coverage; include the expiry validation in
isCatalogCovered output, then re-record site/assets/demo.cast with the corrected
result.
Comment on lines
+1423
to
+1445
| let player; | ||
| try { | ||
| player = AsciinemaPlayer.create('assets/demo.cast', terminalMount, { | ||
| theme: 'crisismode', | ||
| fit: 'width', | ||
| loop: true, | ||
| // No idleTimeLimit: scripts/pace-cast.mjs assigns every gap and | ||
| // clamps the longest to 4s, so there is nothing left to trim. | ||
| // Verified against the current cast: 314 events, 56.2s, max gap 4.0s. | ||
| // Trimming here would also cut the deliberate pause on the approval | ||
| // gate, which is the point of that frame. | ||
| poster: 'npt:0:03', | ||
| controls: 'auto', | ||
| }); | ||
| } catch { | ||
| // A broken vendored player should degrade to the GIF, not an empty box. | ||
| showTerminalFallback(); | ||
| return; | ||
| } | ||
|
|
||
| // Autoplay is muted-equivalent (no audio), so browsers allow it. If a | ||
| // policy blocks it the controls remain, so this stays non-fatal. | ||
| Promise.resolve(player.play()).catch(() => {}); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
asciinema-player AsciinemaPlayer.create inline cast source object option
💡 Result:
In the asciinema player library, the AsciinemaPlayer.create function is used to initialize and mount the player into the DOM [1][2][3]. While the basic usage involves passing a URL string as the src argument [1][2][4], you can also pass an object to the src argument to provide the recording data directly (inline) or to customize how the data is fetched [5][6]. When using an object for the src argument, the following structure is supported: { data: data } This is used to provide the recording content directly [5][6]. The data value can be a string (v1, v2, or v3 format), an array (v2 or v3 format), an object (v1 format), or a function that returns the recording data (optionally asynchronously) [5][6]. Example: AsciinemaPlayer.create( { data: '{"version": 2, "width": 80, "height": 24}\n[1.0, "o", "hello"]' }, document.getElementById('demo')); { url: url, parser: parser } This allows you to specify a custom parser or configuration when loading from a URL [5][6]. { url: url, fetch: {... } } This allows you to customize the HTTP fetch request (e.g., adding headers or credentials) when loading from a URL [6]. The third argument to AsciinemaPlayer.create, opts, remains a separate configuration object used to control player behavior such as speed, theme, or loop settings, and is distinct from the src source object [1][7][2].
Citations:
- 1: https://docs.asciinema.org/manual/player/api/
- 2: https://docs.asciinema.org/manual/player/quick-start/
- 3: https://docs.asciinema.org/manual/player/
- 4: https://github.com/asciinema/asciinema-player/blob/master/README.md
- 5: https://docs.asciinema.org/manual/player/loading/
- 6: https://dataswamp.org/~incal/emacs-init/node_modules/asciinema-player/README.md
- 7: https://docs.asciinema.org/manual/player/options/
🏁 Script executed:
<analysis>
- Initial web search supports that AsciinemaPlayer.create accepts an object source with `{ data: cast }`.
- Need inspect repository site/index.html and related scripts to verify exact context and fallback behavior.
</analysis>
[task verify_review_comment_1/1]
[observations]
- Original concern: AsciinemaPlayer.create('assets/demo.cast', ...) uses async fetch; catch around create does not cover fetch failure.
- Need verify vendored player version/usage and fallback wiring.
[pending]
- Inspect site/index.html around terminal fallback and AsciinemaPlayer script.
- Inspect scripts/demo/package? scripts for vendored player.
- Run deterministic source parse/check for Try/Catch scope around AsciinemaPlayer.create and any global error handling or player.on events.
[actions]
- Use rg/fd/sed to locate relevant files and code.
[done]
- Initial web results incorporated.
[scratchpad_end] -->
#!/bin/bash
set -euo pipefail
printf 'Files matching asciinema/player related:\n'
git ls-files | rg -i 'asciinema|player|package|demo|site/index\.html' || true
printf '\nRelevant index.html lines 1380-1455:\n'
sed -n '1380,1455p' site/index.html | cat -n -v
printf '\nOccurrences of showTerminalFallback and terminalMount:\n'
rg -n "showTerminalFallback|terminalMount|AsciinemaPlayer|create\(" site/index.html scripts site 2>/dev/null || trueRepository: trs-80/crisismode
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching asciinema/player related:\n'
git ls-files | rg -i 'asciinema|player|package|demo|site/index\.html' || true
printf '\nRelevant index.html lines 1380-1455:\n'
sed -n '1380,1455p' site/index.html | cat -n -v
printf '\nOccurrences of showTerminalFallback and terminalMount:\n'
rg -n "showTerminalFallback|terminalMount|AsciinemaPlayer|create\(" site/index.html scripts site 2>/dev/null || trueRepository: trs-80/crisismode
Length of output: 50374
Show the GIF when the cast cannot load.
AsciinemaPlayer.create('assets/demo.cast', ...) returns synchronously and loads the cast asynchronously, so the try/catch around construction does not handle missing or failed casts. Fetch the cast before creating the player and call showTerminalFallback() for fetch/parse failures, keeping the catch for synchronous player failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/index.html` around lines 1423 - 1445, Update the initialization flow
around AsciinemaPlayer.create to fetch and parse assets/demo.cast before
constructing the player, calling showTerminalFallback() when fetching or parsing
fails. Pass the successfully loaded cast data into AsciinemaPlayer.create, while
retaining its existing try/catch to handle synchronous player-construction
failures.
The two call sites left at max_tokens=1024 were measured across 39 live
sonnet-5 calls and set explicitly:
- ai-diagnosis-universal.ts: 4096 / 45s. Measured 1076-1891 tokens and
10.1-23.4s across the three prompt shapes it builds, so 1024 truncated
every shape including the simplest question, and the old 15s timeout was
below the widest shape's 15.4-23.4s.
- ask.ts REPL: 6144 / 45s. Measured 543-3638 tokens and 6.7-37.1s; only the
first turn ever fit in 1024, and the old 30s was below a long runbook
session's 29.0-37.1s.
Truncation was also undetectable. callClaude returned only concatenated text
and discarded stop_reason, so a cut-off answer was indistinguishable from a
finished one. A text heuristic is not good enough: in 24 forced-truncation
trials, 2 of 20 truncated answers ended on a period and read as complete.
Adds a strictly additive callClaudeDetailed() returning a typed
{text, stopReason}; callClaude delegates to it, so no existing signature or
behaviour changes. `truncated` is a required field on both result types, and
both ask modes now say plainly that an answer was cut off — verified at the
real CLI, not only in tests.
Also fixes the hermeticity break this new export caused.
ai-timeout-reporting.test.ts mocked ai-client with `{...actual, callClaude}`,
so callClaudeDetailed fell through to a real Anthropic client and the suite
made live authenticated calls (genuine 401s with fresh request_ids). Mocking
@anthropic-ai/sdk itself means no future export can reach the network,
whatever the module mock spreads. Confirmed by re-breaking the spread mock:
2 loud failures and 0 network attempts, against 6 live calls before.
New tests mutation-tested: reverting the fix fails 9 of 17.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`init --agent <name>` scaffolds a CHECK PLUGIN, not an agent. In a codebase where "agent" is a distinct core concept (RecoveryAgent, src/agent/*, AgentManifest), the flag actively misled — the help text even read `init --agent <name> Scaffold a check plugin`, contradicting itself. `--plugin` is now canonical. `--agent` still works as a deprecated alias with a one-line stderr notice, and `--plugin` wins if both are passed, so no existing user breaks. Help text, the scan hint and all three completion scripts now agree with the real surface. Two latent bugs in the same path, found while fixing it: - a valueless `--plugin`/`--agent` crashed with a raw internal error; it is now a usage error exiting 1 - a name like `../../evil` scaffolded OUTSIDE checks/ — path traversal via an unvalidated directory name. Names are now restricted to letters, digits, ".", "_" and "-", exiting 1 on anything else. Also drops `init --agent` from the `--local` documentation row, since init never honored `--local`. Verified at the real CLI after a build: traversal exits 1, valueless exits 1, the deprecated alias exits 0 and warns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov flagged 9 uncovered lines in the token/timeout fix, concentrated in the new error-recovery code — exactly the paths that must not rot silently. Coverage across the five affected files goes from 85.93% to 99.74% lines and 78.49% to 98.56% branches, prioritising this PR's new behaviour over raw percentage: escapeControlCharsInStrings escape-tracking through \" and \\, the \uXXXX fallback, and the retry-once contract proved by spying on JSON.parse (1 call when already valid, 1 when the repair is a no-op and rethrows, 2 when the repair runs and must still throw). Two assumptions in the brief were wrong and are corrected here: diagnoseCommandFor and root-cause-synthesis's stripCodeFence were already covered, and ai-client's 110/136 are ternary arms rather than the abort path, which was also already covered. The 9 missing incident-summary lines were all printIncidentSummary. Four lines stay uncovered deliberately — two are genuinely unreachable, two are pre-existing cosmetic string arms not worth contorting a fixture for. Separately, the elevated-risk fixture in ai-response-budgets.test.ts violated this repo's own safety contract. It could not even be validated (TypeError in validator.ts at the missing requiredCapabilities), and once runnable it failed TWO checks: state preservation AND human notification for elevated+ plans. A fixture that breaks the safety rules teaches the wrong shape to anyone who copies it. Swept all 11 test files in scope: 2 plan fixtures, 3 violations total, all fixed, plus a `command: 'shell'` that is not a member of the Command union. No deliberately-invalid negative tests were touched. Adds a mechanical test that runs every plan fixture through the real validator, so future validator rules apply automatically instead of waiting for a reviewer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four CodeRabbit findings on the demo tooling, each reproduced before fixing. pace-cast.mjs wrote outPath and only then verified that output bytes matched input. record-demo.sh passes site/assets/demo.cast as outPath, so a failed check left the rejected cast in place — having already overwritten the committed asset — while printing "refusing to ship". Reproduced by forcing a byte mismatch and watching the committed file's hash change. It now writes a sibling temp and renames only after the check passes, with cleanup wired to exit, SIGINT, SIGTERM and SIGHUP; verified across success, forced failure, SIGTERM, SIGINT and an uncaught throw with zero leftovers. record-demo.sh leaked a temp file per run: `mktemp -t ...).exp` appends `.exp` to the path mktemp already created, so `cat` wrote to a different file and the trap only removed the second one. Eight leaked zero-byte files were sitting in $TMPDIR from earlier runs. Now uses mktemp -d with a recursive trap. DECISION was documented as [approve|skip|reject] but accepted any argument and was interpolated into the expect here-doc as Tcl, so a typo burned a live API call. It is now validated, exiting 2. Going further than the finding: $REPO_ROOT was interpolated into Tcl too, and a checkout path containing brackets provably breaks the driver with "invalid command name" — the here-doc is now quoted and the path and decision pass through the environment. Adds an agg >= 1.9.0 check, since --select is load-bearing for the GIF cut and only existence was verified before. No behavioural change to the recording: same content-anchored Phase 6 cut, same 2fps and 1MB budget, and pace-cast still verifies output bytes equal input. site/assets/demo.cast is byte-identical, confirmed by hash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five review findings, all verified against the code before changing anything; two were narrower than the underlying defect. The maturity guidance required execute-verified recovery before a contributor could use `live_validated`, but docs/coverage.md separates diagnosis maturity from execute-verified recovery: a deployment that validated diagnosis can legitimately be `live_validated`, and `dry_run_only` covers dry-run-validated diagnosis. As written it pushed contributors to under-report. The finding named your-first-agent.md, but the same rule was wrong in CONTRIBUTING.md — the canonical location the guide links to — so both are fixed. The playbook example was worse than mislabelled. A step carried the `cache.expiry.trigger` capability at routine risk while the command scanned `*` and unlinked the first 1000 keys regardless of TTL, so a contributor copying it would delete live keys and cause a cache-miss storm. The replacement deletes nothing itself: it relies on Redis passive expiration through a bounded 1000-key TTL touch, leaving live and persistent keys in place, and it validates at 13 safety checks through the real CLI. The same example had a broken three-backtick fence that rendered half of it as body text. Also: corrects the flag-audit claim in the docs-pass report without rewriting what was true when it was written, and labels 17 unlabelled code fences (MD040) — 10 more than the review listed. MD040 is now zero across every file outside docs/superpowers, and all links and anchors resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
Follow-up wave: review findings + coverage (5 commits)Addresses all 12 CodeRabbit findings and the Codecov gap. Each finding was verified against the code before acting; none were refuted, and three turned out narrower than the underlying defect. Two real bugs in the demo tooling, both reproduced first
Beyond the finding: Path traversal in Test suite was making live API calls. A new A test fixture violated this repo's own safety contract — and couldn't even be validated (TypeError on missing Docs — the maturity rule wrongly required execute-verified recovery for Coverage 85.93% → 99.74% lines, 78.49% → 98.56% branches on the affected files, prioritising the new error-recovery paths. Four lines left uncovered deliberately and explained. Known, not landed here
|
Owner
Author
|
@coderabbitai review The previous review was rate limited, so the last 5 commits (075a616..e904513) have not been reviewed. Please review them:
|
|
|
Both findings from a local CodeRabbit review of 075a616..e904513, which the GitHub integration could not review because it was rate limited. scan.ts reported "No check plugins found", but the count it checks is healthPlugins — filtered at line 429 to plugins declaring the `health` verb. A repo with only diagnose-only or plan-only plugins would be told nothing was found while plugins were in fact discovered. The docs-pass report also still recorded the scan hint as `init --agent my-check`, contradicting its own superseding note four lines earlier that says both now read `--plugin`. Verified at the real CLI, not only in tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/cli-reference.md (2)
89-90: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCorrect the
downexit-code table.Missing service configuration exits with code 0, not 2. Update the second row so
downexits 0 when no services are configured and exit 2 only for parsing errors such as unrecognized flags,--configmissing values, and invalid service arguments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cli-reference.md` around lines 89 - 90, The `down` exit-code row in the CLI reference incorrectly assigns missing service configuration to exit code 2. Update that row so no configured services exits 0, while exit code 2 is reserved for parsing errors such as unrecognized flags, missing `--config` values, or invalid service arguments.
61-62: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd the missing per-command flags to this table.
docs/cli-reference.mdsays this is the complete flag set, butcrisismode bundlealso has--output <file>, andcrisismode downuses--config <path>with exit code 2 for unrecognized flags. Addbundle --outputanddown --configto the appropriate command rows, or state that the table omits bundle-specific usage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cli-reference.md` around lines 61 - 62, Update the CLI flag table in docs/cli-reference.md to accurately document the command-specific options: add bundle --output <file> and down --config <path> to their respective command rows, or revise the completeness statement to explicitly exclude bundle-specific usage.
🧹 Nitpick comments (3)
src/__tests__/ai-diagnosis-response-recovery.test.ts (1)
96-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
withUnitSeparatorto match the character used.The fixture uses
\u0001(start of heading), not the unit separator (\u001F). The name and the surrounding prose disagree with the value, which can mislead a later reader who changes the fixture.♻️ Proposed rename
- const withUnitSeparator = + const withStartOfHeading = '{"status":"identified","scenario":"pipe\u0001delimited","confidence":0.5,"findings":[]}'; - expect(() => JSON.parse(withUnitSeparator)).toThrow(/control character/i); + expect(() => JSON.parse(withStartOfHeading)).toThrow(/control character/i); - const result = parseStandardDiagnosisResponse(withUnitSeparator); + const result = parseStandardDiagnosisResponse(withStartOfHeading);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/ai-diagnosis-response-recovery.test.ts` around lines 96 - 104, Rename the test fixture variable withUnitSeparator to reflect the \u0001 start-of-heading character it contains, and update its reference in the test; do not change the fixture value or assertions.src/__tests__/ai-universal-token-budget.test.ts (1)
205-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the console spy in
afterEach.
consoleSpy.mockRestore()runs only on the success path. If an assertion above it fails, the spy stays installed for the rest of the file and suppressesconsole.erroroutput from later tests. Addvi.restoreAllMocks()toafterEach, asask-token-budget.test.tsalready does.♻️ Proposed change
afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); if (originalApiKey !== undefined) {Then drop the inline
consoleSpy.mockRestore()calls at the end of the two timeout tests.Also applies to: 241-253, 297-310
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/ai-universal-token-budget.test.ts` around lines 205 - 212, Update the afterEach cleanup in the test suite to call vi.restoreAllMocks() so console spies are restored even when assertions fail, while preserving the ANTHROPIC_API_KEY restoration. Remove the inline consoleSpy.mockRestore() calls from both timeout tests, including the ranges identified in the comment.src/framework/ai-client.ts (1)
83-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep the duplicate stop-reason union small, or align it with
@anthropic-ai/sdk'sStopReason.
@anthropic-ai/sdk@0.115.0 declaresMessage.stop_reasonasStopReason | null, whose non-null values match the currentClaudeStopReasonnon-null union. There is no TypeScript error here, so this is only an optional cleanup if you want this module to derive Claude stop reasons from the SDK type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/framework/ai-client.ts` around lines 83 - 98, Optionally replace the duplicated ClaudeStopReason union with the corresponding StopReason type from `@anthropic-ai/sdk`, preserving the existing nullability and CallClaudeResult.stopReason contract. If retaining the local alias, keep its non-null members aligned with the SDK’s StopReason values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/playbook-authoring.md`:
- Around line 263-269: Replace the pipeline in docs/playbook-authoring.md lines
263-269 with a bounded SCAN loop that explicitly checks failures from each Redis
SCAN and TTL command, propagating any failure so the recovery step cannot report
success incorrectly; avoid adding pipefail with unhandled head-induced SIGPIPE.
Apply the identical failure-propagation change to the checked-in example in
playbooks/examples/redis-memory-pressure.md lines 62-68.
In `@src/cli/commands/ask.ts`:
- Around line 114-121: Update the truncated-response handling in runAsk so
machine-output mode emits a truncation marker alongside result.response despite
printWarning and printInfo suppression. Preserve the existing human-readable
warning behavior while ensuring --json consumers can detect that the answer was
truncated.
In `@src/cli/commands/init.ts`:
- Around line 32-35: Update pluginNameFrom to treat only undefined as absent;
boolean false must reach the validation error path and be rejected. Add
regression coverage through runInit for both plugin: false and agent: false,
confirming neither silently creates the configuration.
---
Outside diff comments:
In `@docs/cli-reference.md`:
- Around line 89-90: The `down` exit-code row in the CLI reference incorrectly
assigns missing service configuration to exit code 2. Update that row so no
configured services exits 0, while exit code 2 is reserved for parsing errors
such as unrecognized flags, missing `--config` values, or invalid service
arguments.
- Around line 61-62: Update the CLI flag table in docs/cli-reference.md to
accurately document the command-specific options: add bundle --output <file> and
down --config <path> to their respective command rows, or revise the
completeness statement to explicitly exclude bundle-specific usage.
---
Nitpick comments:
In `@src/__tests__/ai-diagnosis-response-recovery.test.ts`:
- Around line 96-104: Rename the test fixture variable withUnitSeparator to
reflect the \u0001 start-of-heading character it contains, and update its
reference in the test; do not change the fixture value or assertions.
In `@src/__tests__/ai-universal-token-budget.test.ts`:
- Around line 205-212: Update the afterEach cleanup in the test suite to call
vi.restoreAllMocks() so console spies are restored even when assertions fail,
while preserving the ANTHROPIC_API_KEY restoration. Remove the inline
consoleSpy.mockRestore() calls from both timeout tests, including the ranges
identified in the comment.
In `@src/framework/ai-client.ts`:
- Around line 83-98: Optionally replace the duplicated ClaudeStopReason union
with the corresponding StopReason type from `@anthropic-ai/sdk`, preserving the
existing nullability and CallClaudeResult.stopReason contract. If retaining the
local alias, keep its non-null members aligned with the SDK’s StopReason values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b0c21490-269b-4451-b2a1-32ea001c4f7d
📒 Files selected for processing (35)
CONTRIBUTING.mdGETTING_STARTED.mdQUICKSTART.mdREADME.mddocs/agents/backup-verification.mddocs/architecture.mddocs/cli-reference.mddocs/guide-verification/2026-08-09-docs-pass-report.mddocs/guides/creating-a-check-plugin.mddocs/guides/creating-a-recovery-agent.mddocs/guides/your-first-agent.mddocs/playbook-authoring.mdplaybooks/examples/redis-memory-pressure.mdscripts/pace-cast.mjsscripts/record-demo.shsrc/__tests__/ai-client.test.tssrc/__tests__/ai-diagnosis-response-recovery.test.tssrc/__tests__/ai-explainer-fallback.test.tssrc/__tests__/ai-explainer-plan-fixtures.tssrc/__tests__/ai-response-budgets.test.tssrc/__tests__/ai-timeout-reporting.test.tssrc/__tests__/ai-universal-token-budget.test.tssrc/__tests__/ask-token-budget.test.tssrc/__tests__/completions.test.tssrc/__tests__/incident-summary-print.test.tssrc/__tests__/incident-summary.test.tssrc/__tests__/init-plugin-flag.test.tssrc/__tests__/root-cause-synthesis-ai-path.test.tssrc/cli/commands/ask.tssrc/cli/commands/completions.tssrc/cli/commands/init.tssrc/cli/commands/scan.tssrc/cli/index.tssrc/framework/ai-client.tssrc/framework/ai-diagnosis-universal.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/guides/creating-a-recovery-agent.md
- docs/guides/your-first-agent.md
- CONTRIBUTING.md
- docs/guide-verification/2026-08-09-docs-pass-report.md
- src/tests/ai-response-budgets.test.ts
- scripts/pace-cast.mjs
- src/tests/incident-summary.test.ts
- GETTING_STARTED.md
- docs/architecture.md
- src/cli/commands/scan.ts
- scripts/record-demo.sh
- QUICKSTART.md
- README.md
`crisismode ask --json` reached machine mode but printed the raw answer with
console.log, while the truncation notice went through printWarning/printInfo —
both of which return early when the output mode is machine (output.ts:460 and
:470). A machine consumer therefore received a partial answer with no marker.
In practice it was worse than reported: the command emitted no JSON at all.
That defeats the point of the truncation work earlier in this PR. A cut-off
answer must never be presentable as a complete one, and an automated caller is
the one least able to notice.
Both single-shot and REPL paths now emit one
{"type":"ask",question,answer,source,truncated} JSONL record per turn through
the existing jsonOut() convention, matching what down, triage and readiness
already produce. Human and pipe output are byte-identical and the warning
wording is untouched.
11 hermetic tests with @anthropic-ai/sdk mocked, including a guard proven to
fail if the marker is suppressed again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cache-eviction example replaced earlier in this PR fixed the safety problem — it no longer deletes keys — but its exit status came from the final `while` loop rather than the Redis commands. Reproduced with a fake redis-cli on PATH: the pipeline exits 0 both when SCAN fails outright with a connection refused, and when an intermediate TTL fails before a later success. A recovery step that silently succeeds after its cleanup failed is worse than one that fails loudly, because the engine proceeds believing memory pressure was handled. `set -o pipefail` is not the fix and was verified not to be: with it, a HEALTHY Redis exits 141 from head's SIGPIPE, trading a silent success for a spurious failure. Both files now carry an identical cursor-driven, bounded SCAN loop with no pipeline at all. Each SCAN reply is fed through a here-document and every redis-cli exit status is tested directly, so a SCAN or TTL failure exits 1. The safety property is preserved: read-only SCAN and TTL only, no DEL, UNLINK or EXPIRE, capped at 1000 keys and 50 scan rounds. Verified through the real CLI: both the checked-in example and the docs Complete Example report 13 safety checks passed, dry-run compiles 8 steps with step 4 still routine, and shellcheck -s sh is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pluginNameFrom returned early on `value === false`, so runInit(undefined,
{ plugin: false }) was read as "flag not given" and wrote crisismode.yaml
instead of reporting invalid input.
Checked why the case existed before removing it: --plugin and --agent are
registered as { type: 'string' } with no default, so under parseArgs an
omitted flag is always undefined, a valueless flag is true, --plugin= is "",
and --no-plugin lands on its own key. Verified against real node:util
parseArgs on node v26.5.1. The false branch guarded an unreachable state, so
dropping it cannot break the ordinary omitted-flag path, and no change is
needed at the parse level.
Adds 5 regression tests, including parser-level ones pinning the behaviour the
fix depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Checking redis-cli's exit status was not enough. Without -e, redis-cli exits 0 even when Redis returns an error reply, so the step reported success under NOAUTH, NOPERM, WRONGTYPE or an unknown command. Reproduced against a real redis-server 8.8.0: with requirepass set, the checked-in snippet printed "touched 0 keys, deleted none" and exited 0 while every command returned NOAUTH; an ACL-restricted --user reproduced it with NOPERM. The elevated step is the one that matters most, and it did not need an auth failure to fail silently at all: `CONFIG SET maxmemory 8gigabytes` exits 0 without -e on a healthy authenticated Redis, so a bad unit alone made an elevated step report the memory ceiling raised when nothing had changed. -e alone was also insufficient for the INFO/DBSIZE step, because runtime.ts compiles an sh block into a single structured_command and only the last line's status is reported. Proved with an ACL user granted +dbsize but not +info: exit 0 with -e alone, exit 1 once set -e is added. Two prose snippets deliberately keep no -e: one is the destructive anti-pattern the page argues against, and the other is the broken pipeline one-liner that -e would not fix anyway. Also corrects a paragraph that claimed "a clean run is the only way to exit 0", which was not true before this change. Healthy path re-verified per command (INFO bulk string, DBSIZE integer 0, CONFIG SET +OK, empty keyspace, TTL -1 and -2, non-string types, and a 1500-key keyspace still stopping at 1000). The shared step-4 block stays byte-identical across both files. playbook validate reports 13 safety checks passed on both, dry-run 13 checks over 8 steps with step 4 routine and step 7 elevated, and shellcheck -s sh is clean on all 5 extracted blocks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.cbmignore is not a gitignore — everything it lists stays tracked, shipped and required at runtime. It only keeps site/assets/ out of the codebase-memory knowledge graph: the minified asciinema-player bundle contributes hundreds of single-letter symbols ($, $A, $e, $t, ...) that outrank real source in full-text search, and no structural query about this codebase should resolve into third-party minified output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Three requested items, plus a product bug the demo work uncovered.
The bug (unplanned, and the most important change here)
aiDiagnoseran atmax_tokens=1024with a 10s timeout. No current model can return a complete structured diagnosis in that budget — nine live sonnet-5 runs against the real pg-replication prompt came back at 2259–3742 output tokens and 28–40s. Every response arrived truncated withstop_reason=max_tokens,JSON.parsethrewUnterminated string,aiDiagnosereturnednull, and the caller degraded to rule-based heuristics.The failure was invisible: it looked like "AI is unavailable" rather than "the budget is too small". It affected the pg-replication agent, the kafka agent and
bundle ingest, regardless of whether an API key was set.ai-explainerandroot-cause-synthesiswere failing the same way —root-cause-synthesisalso parsed with a bare.trim()and nostripCodeFence, so 2 of 3 live calls threw on a ```json fence.Budgets are now set per call site with measured headroom rather than one global number: 60s/6144 for the non-interactive toolkit, 30s/4096 for the two foreground paths where a fast fallback beats a long hang. All 7
callClaudecall sites audited. 18 hermetic regression tests added, 8 verified to fail against the old constants, no live API calls in tests.1. Documentation
README 477 → 258 lines, with depth moved into
docs/cli-reference.mdanddocs/coverage.md. The three overlapping onboarding docs now have one home per audience.Claims that contradicted the codebase were corrected — most notably the agent table listed six agents as "Live (execute-capable)" when their manifests say
simulator_only, so the README was overstating what the tool itself reports. Also fixed: both agent guides told readers to use invalidPluginMaturityvalues,playbook-authoring.md's flagship example failed validation, and the agent count was 19 vs 26 actual.2. Demo re-recorded against live AI
Recording previously ran under
env -u ANTHROPIC_API_KEY, so it showed the rule-based fallback's fixed 92%/three-finding output. It now runs the default model with noCRISISMODE_AI_MODELoverride, showing what a user actually gets:replication_lag_cascadeat 78% with a model-authored root cause.Honesty trade-offs are documented in the script rather than hidden: the cast is no longer byte-reproducible, and
pace-cast.mjscaps idle gaps at 4s so the real 28–56s model wait doesn't ship as a frozen frame. The page discloses that playback is pace-adjusted.3. Site and Cloudflare bundle
Plays the real cast inline via vendored asciinema-player v3.17.0, GIF as fallback only.
scripts/package-site.shemitsoutput/crisismode-site.zip—index.htmlat the zip root, no.DS_Store, byte-identical across runs. Removessite/index.html.zip, a stale artifact whose upload would have deployed a page with four 404ing assets.The GIF teaser was 2.8MB against a 1MB pre-commit ceiling; re-anchored to Phase 6 at 2fps for 905KiB, keeping the model's root-cause paragraph. Total bundle 990KiB.
Verification
typecheckclean,lintclean, 3192 passed / 24 skipped across 227 filesrule-based,Set ANTHROPIC_API_KEY,timed out,could not be parsedKnown and deliberately left
ai-diagnosis-universal.ts:126andask.ts:218remain at 1024 tokens. Neither parses its response, so truncation there is a visibly cut-off answer rather than a silent fallback.🤖 Generated with Claude Code
Summary by CodeRabbit
--plugin, while retaining deprecated compatibility.