[don-petry]

Summary

This fixes the Phase 2 blocker (#1957) so the Opus 5.5 canary can resume. It covers AC-1 to AC-4, AC-6 (syntax part) and AC-7.

• Correct Sonnet 5 model ID (D1): claude-sonnet-5-0 becomes claude-sonnet-5.
  • Changed in engine.sh (the action model and the triage, deep and action chains) and in the aw.sh model allowlist.
  • model-pricing.tsv gets claude-sonnet-5 rows at the existing Sonnet 5 rates. The claude-sonnet-5-* glob doesn't match the bare ID, so without these rows Sonnet 5 calls would go unpriced.
• Current Claude Code CLI in CI (D2): 8 workflows cached the CLI under a literal latest key, so a 2.1.138 install from before Opus 5.5 kept being reused.
  • A new step before the cache step resolves latest to the real npm version, so a new CLI release misses the cache.
  • On a cache miss the install step installs exactly that resolved version, pinned through a step-level env:.
  • If npm can't be reached, the step falls back to a weekly key.
  • A version pinned in vars.CLAUDE_CODE_VERSION passes through unchanged.
  • pr-review and dev-lead now always log the CLI version in use.
• Honest error classification (D3):
  • Removed claude.*usage|usage.*claude. Every failed call's JSON output contains the model name and a usage object, so this pattern matched every Claude error.
  • _claude_chain_invoke now classifies each failed hop by its api_error_status:
    • 429, 529, 402 or 413: throttled;
    • 404: model unavailable;
    • 400: request rejected;
    • 5xx: server error.
  • Each of those tries the next model and logs a warning that says why. Any other error stops the chain immediately.
  • Each failed hop's error goes to $RUNNER_TEMP/cascade/claude-chain-hops.log, which is uploaded with the pr-review-debug-* artifact. This is where Opus 5.5's real error will show up. Only the error fields are kept (is_error, api_error_status, subtype, result), capped at 1000 bytes and passed through redact_secrets.
  • The chain returns 2 (the cross-provider fallback signal) only when its last hop was throttled. review-one-pr.sh decides on fallback by scanning that last hop's output.
• Deep-tier safety net (AC-4): the deep chain is now claude-opus-5-5,claude-opus-4-8,claude-sonnet-5. If Opus 5.5 fails, the review drops to the model it replaced instead of failing. Token records already store the model actually used, so the canary report ([Phase 2] Canary go/no-go report: compare deep-tier cost, cache-read cost and latency by model from token-usage artifacts #1951) still counts only real Opus 5.5 calls.
• Copilot fallback (D4, syntax only): triage now passes --deny-tool shell --deny-tool write instead of the rejected --deny-tool '*'. The classic PAT (ghp_) problem needs a maintainer to swap in a fine-grained token, so it isn't fixed here.

Problem

Closes #1957; blocks #1899 and #1900 under epic #1895. Since v1.17.0 reached pr-review/v1-next, every PR review that escalated to the deep tier failed:

• claude-opus-5-5 exited with rc=1 and 0 tokens, and the log said "throttled";
• claude-sonnet-5-0 returned a 404, also reported as "throttled";
• Copilot then failed with Invalid rule format: *.

Risk

Medium. This changes fallback behaviour in the shared engine, used by pr-review, dev-lead and personas.

• A chain now stops early on errors that aren't worth retrying, such as 401/403 or a max-turns error. Before, those were hidden as "throttling" and the next model was tried.
• A chain that ends on a 404, 400 or 5xx now returns 1 instead of 2.
• Text-mode calls (no TOKEN_LOG_FILE) behave as before.
• The workflow changes only affect the cache key and the pinned install version.

Test plan

• shellcheck --severity=warning -x scripts/engine.sh scripts/aw.sh: clean.
• actionlint (with shellcheck) on the 8 changed workflows: no new findings compared with main.
• I dry-ran the resolve step with a stubbed npm for three cases (latest resolves, npm fails, pinned version). The cache key and install version match in each, except the degraded case where npm is unreachable.
• New tests/dev-lead/unit/test_engine_error_class.bats (14 tests) covers:
  • 404, 400, 5xx and 401 errors, and a genuine 429;
  • all hops 404, and a 429 hop followed by a 404 last hop;
  • an error with no status code;
  • the hop log turned on and off;
  • redaction, including that envelope metadata isn't copied;
  • the pattern no longer matching an error envelope, while real Claude caps still match.
  • The first 12 tests: 8 fail against main's engine.sh, so they reproduce the bug.
• Updated test_engine_chain.bats, test_engine_new_models.bats and test_model_dispatch.bats for the corrected ID and deep chain. Added claude-sonnet-5 cases to model_pricing.bats.
• test_duplicate_decl_gate.bats and persona_runner.bats pass after engine.sh started sourcing lib/redact.sh.
• I ran all 54 bats files that reference the engine, pricing, stub or review scripts on this branch and on main, then compared the failures. No new failures. The remaining 49 fail identically on main and are environmental in this container (e.g. test_fix_issue.bats).

Rollback

Revert this PR. If a release has already been cut, move pr-review/v1-next back with scripts/cut-release.sh pr-review <previous> --promote --channel next --push. The only lasting side effect is new Actions cache entries.

Monitoring

• Channel cut: this reaches pr-review/v1-next (and the dev-lead next channel) only after Release Manager autocut cuts a release. That happens on the Canary Rollout cron within about 4h of merge.
• What to check after the cut:
  • the pr-review job log shows claude-code cache version: <x.y.z> and claude-code installed: (or cache hit:) with a version newer than 2.1.138;
  • deep reviews complete, with no [claude] model claude-sonnet-5-0 lines;
  • any Opus 5.5 failure is labelled (unavailable (HTTP 404…), rejected the request (HTTP 400)) and its detail is in cascade/claude-chain-hops.log in the pr-review-debug-* artifact.
• Still to do after merge: AC-5 liveness, running the [Phase 2] Add a workflow_dispatch runner for model-ab.sh plus a liveness/effort probe, so the Opus 5.5 non-regression A/B actually executes #1950 probe for every model in the chains, and recording the result on [Phase 2] Canary the deep-tier swap through the v1-next ring and monitor before stable promotion #1899.

Interaction contract

• N/A — this PR does not add or change an agentic role.

Checklist

• shellcheck scripts/*.sh passes for any changed shell scripts.
• Docs updated where behavior or standards changed (inline comments at each change; no standards doc covers the classifier).

🤖 Generated with Claude Code

https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

<source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">

Summary by CodeRabbit

• Bug Fixes
  • Claude Code workflows now align cached versions with the version installed, including when using latest.
  • Claude model fallback now handles additional API errors and continues to eligible backup models. Corrected model identifiers and updated fallback options.
  • Copilot triage now avoids requesting shell and file-editing tools.
  • Updated recognition of the current Claude model name to prevent false warnings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5a18e926-5025-43f0-aae3-d2fc1239d36d

📥 Commits

Reviewing files that changed from the base of the PR and between d3f1bae and 1ad231f.

📒 Files selected for processing (10)

• .github/workflows/daily-pr-review-health.yml
• .github/workflows/dependency-advisory.yml
• .github/workflows/dev-lead-reusable.yml
• .github/workflows/docs-health-check.yml
• .github/workflows/gh-aw-cross-org.yml
• .github/workflows/pr-review.yml
• .github/workflows/readme-refresh.yml
• .github/workflows/skill-eval-report.yml
• scripts/engine.sh
• tests/dev-lead/unit/test_engine_error_class.bats

📝 Walkthrough

Walkthrough

Workflows now resolve Claude CLI versions for cache keys and installation. The engine updates Claude model identifiers and fallback behavior, classifies and logs failed hops, and changes Copilot triage tool-denial arguments. Tests cover the updated model chains and failure handling.

Changes

Claude CLI cache versions

Layer / File(s)	Summary
Resolve versions for caching and installation .github/workflows/daily-pr-review-health.yml, .github/workflows/dependency-advisory.yml, .github/workflows/dev-lead-reusable.yml, .github/workflows/docs-health-check.yml, .github/workflows/gh-aw-cross-org.yml, .github/workflows/pr-review.yml, .github/workflows/readme-refresh.yml, .github/workflows/skill-eval-report.yml	Each workflow passes pinned versions through and resolves latest through npm when possible. Cache keys use the resolved version; when lookup fails, the cache key uses a UTC week-based value and installation remains set to latest. The dev-lead workflow logs the installed or cached CLI version. The PR review workflow returns installation failures and logs the version after a fresh install.

Claude engine routing and error handling

Layer / File(s)	Summary
Update model identifiers and fallback order scripts/aw.sh, scripts/engine.sh, tests/dev-lead/unit/test_engine_chain.bats, tests/dev-lead/unit/test_engine_new_models.bats, tests/dev-lead/unit/test_model_dispatch.bats, tests/model_pricing.bats	The known-engine list and Claude chains use claude-sonnet-5. The deep chain adds claude-opus-4-8 between Opus 5.5 and Sonnet 5. Tests update model-chain and dispatch expectations and assert Sonnet 5 pricing outputs for two dates.
Classify and handle failed engine hops scripts/engine.sh, tests/dev-lead/fixtures/engines/stub-claude, tests/dev-lead/unit/test_engine_error_class.bats, .gitleaksignore	Claude failures are classified using API status when available, or result text and stderr otherwise. Selected failure classes advance to the next model; terminal failures retain their exit code. Failed-hop logs are best-effort and redact error details when the redaction helper is available. Copilot triage uses separate shell and write denials. Tests cover classification, fallback, and logging; .gitleaksignore adds ignores for two historical test-fixture findings.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (10 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The changes implement the reviewable parts of #1957: they replace the invalid model ID, add the deep fallback, resolve and install the Claude CLI version, classify API errors, preserve redacted hop de…	Provide independent reviewable evidence that scripts/lib/model-pricing.tsv contains the required appended claude-sonnet-5 pricing row with the specified effective-dated rates. The file is excluded by the review rules, so its contents ca…

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: the valid Sonnet 5 model ID, fresh CLI caching, and Claude chain error handling. It is specific and related to the pull request.
Description check	✅ Passed	The description includes all required sections and provides specific problem context, risk, test results, rollback steps, monitoring guidance, and checklist status.
Out of Scope Changes check	✅ Passed	The changed workflows support the #1957 CLI cache and installation requirement, including workflows that use the Claude CLI cache. The engine, fixtures, tests, and .gitleaksignore changes support th…

Full details: Linked Issues check

Explanation

The changes implement the reviewable parts of #1957: they replace the invalid model ID, add the deep fallback, resolve and install the Claude CLI version, classify API errors, preserve redacted hop details, update tests, and replace the invalid Copilot rules. The pricing requirement cannot be confirmed because scripts/lib/model-pricing.tsv is excluded from review. The summary reports pricing tests, but it does not establish that the required append-only row for claude-sonnet-5 exists. The liveness probe and issue recording are non-coding follow-up work and do not affect this coding verdict.

Resolution

Provide independent reviewable evidence that scripts/lib/model-pricing.tsv contains the required appended claude-sonnet-5 pricing row with the specified effective-dated rates. The file is excluded by the review rules, so its contents cannot be inspected here.

Full details: Docstring Coverage

Explanation

Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (10 skipped: 10 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1958
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-09-27T02:14:57Z

[don-petry]

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check SonarCloud is still pending. I'll re-check automatically.
Next attempt after: 2026-09-27T02:14:57Z

[don-petry]

No description provided.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T02:45:07Z.

[gemini-code-assist]

Code Review

This pull request corrects the Claude Sonnet 5 model identifier from claude-sonnet-5-0 to claude-sonnet-5 across configurations, pricing tables, and tests. It also fixes a critical issue where all JSON error envelopes from Claude were misclassified as rate limits due to an overly broad regex pattern; this is resolved by introducing robust, status-code-based error classification (_claude_hop_failure_class) and detailed hop logging. Additionally, the Copilot triage fallback is fixed by replacing an invalid wildcard tool denial with explicit tool names. Feedback on these changes suggests adding defensive file checks before reading logs with head to prevent potential hangs, and tightening a test assertion in the new BATS suite to check for a specific non-zero exit code rather than any non-zero status.

[codeant-ai]

CodeAnt Nitpicks

1 code suggestion

1. When jq is unavailable, configured API-error responses fail with “command not found” and emit no JSON, unlike normal JSON responses.

Possible bug · tests/dev-lead/fixtures/engines/stub-claude:70-77

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T02:48:13Z.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T02:49:57Z.

[don-petry]

Addressed the CodeAnt nitpick in tests/dev-lead/fixtures/engines/stub-claude: the API-error JSON branch (the is_error:true envelope) called jq unconditionally, while the normal JSON branch already had a command -v jq → printf fallback. I added the same fallback to the error branch so that when jq is unavailable it emits an equivalent JSON envelope via printf instead of failing with "command not found" and no output. Tests still green (12/12 in test_engine_error_class.bats).

[don-petry]

Acknowledged — informational notice, no action needed. This reports that the Codex code-review usage limit was reached; it is a billing/quota notice about the reviewer service, not a finding against this PR's diff. No follow-up required.

[don-petry]

Acknowledged — informational notice, no action needed. Qodo reviews are paused because the workspace trial ended; this is a billing notice about the reviewer service, not a finding against this PR. No follow-up required.

[don-petry]

Acknowledged — informational. This is CodeRabbit's auto-generated PR summary/overview, a neutral description of the diff rather than an actionable finding. No change requested, no action needed.

[don-petry]

Acknowledged — informational. SonarCloud's Quality Gate passed on this PR; this is a status notice with no action required.

[don-petry]

Clarification for reviewers: the dev-lead replies posted at 01:50–01:51Z were computed against the stale head 6274c2c and were never pushed. The branch head is d3f1bae. The actual fixes are:

Thread	What actually landed
CodeAnt: install race	33d813a: the resolve step outputs install, and each install step pins CLAUDE_CODE_VERSION to it through a step-level env:. The names install_spec and RESOLVED_CLAUDE_CODE_VERSION in the dev-lead reply don't exist.
CodeAnt: hop-log exposure	beddd2c: fixed, not skipped. The log keeps only {is_error, api_error_status, subtype, result}, capped at 1000 bytes and passed through redact_secrets. Without the helper, only the header line is written.
Gemini: head guards	beddd2c: [ -f ] guards, with 1000-byte caps (not 4000).
Gemini: exact exit code	beddd2c: [ "$status" -eq 1 ].
CodeAnt nitpick: stub without jq	d3f1bae: printf fallback in the error-envelope branch.
gitleaks failure on beddd2c	d3f1bae: the dummy token is assembled at runtime, and the two historical findings are fingerprinted in .gitleaksignore. A local gitleaks git 8.24.3 scan of d8c2361..HEAD reports no leaks.

test_engine_error_class.bats now has 14 tests, all passing.

---

Generated by Claude Code

[don-petry]

dev-lead / dispatch failure (run 36286514214): not a defect in this PR's code. That run started on the stale head 6274c2c. It reworked the review findings independently, then hit a conflict in scripts/engine.sh against the fixes already pushed (33d813a, beddd2c, d3f1bae). The #1607 guard refused to force-push over them, as designed (Refusing to force-push over unincorporated human work). The check is attached to 6274c2c, not the current head d3f1bae, so it doesn't gate the merge. No re-run is needed: every finding it targeted is already fixed on the current head (see the table in my previous comment).

---

Generated by Claude Code

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T02:53:02Z.

[don-petry]

Evidence for the pricing AC that CodeRabbit marked inconclusive: *.tsv files are excluded from its review. The complete scripts/lib/model-pricing.tsv diff (d8c2361..d3f1bae) is below. It adds two new rows and leaves every existing row unchanged, per the file's append-only rule:

 claude-sonnet-5-*	2026-09-01	3.00	0.30	3.75	15.00
+claude-sonnet-5	2026-06-30	2.00	0.20	2.50	10.00
+claude-sonnet-5	2026-09-01	3.00	0.30	3.75	15.00
 claude-haiku-4-5*	2025-10-01	1.00	0.10	1.25	5.00

The rates and effective dates match the existing claude-sonnet-5-* rows. tests/model_pricing.bats checks the result with price_for claude-sonnet-5 2026-07-15 → 2.00 0.20 2.50 10.00 and price_for claude-sonnet-5 2026-09-27 → 3.00 0.30 3.75 15.00.

The docstring-coverage warning is advisory. The new functions (_claude_hop_failure_class, _claude_log_hop) have header comments in the repo's shell style, so I've made no change for it.

---

Generated by Claude Code

Auto-dismissed (#617): coderabbitai[bot] CHANGES_REQUESTED on a superseded commit. The bot re-reviews the new head automatically — a valid concern will return as a fresh review.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T03:00:30Z.

[donpetry-bot]

Superseded by automated re-review at 1ad231f50cc3cc5618acf7863443c93aa5fbe448 — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: d3f1baeba926eac8a6a8c755771daf48fbe23133
Review mode: triage-approved (single reviewer)

Summary

Solid, well-tested fix for the #1957 deep-tier blocker: corrects the Sonnet 5 model ID, freshens the CLI cache key across 8 workflows, replaces the over-broad rate-limit regex with status-code-based hop classification, and adds an opus-4-8 safety hop. The engine logic, tests (14 new bats tests, 8 of which reproduce the bug on main), pricing rows, and commit-scoped .gitleaksignore entries all verify clean. Escalating only because two CodeRabbit review threads on the current head are unresolved, and one of them is a verified real inconsistency in readme-refresh.yml.

Linked issue analysis

Closes #1957 (Phase 2 blocker under epic #1895). Verified against the issue: D1 (invalid claude-sonnet-5-0 → claude-sonnet-5) fixed in engine.sh, aw.sh, and model-pricing.tsv; D2 (stale cached CLI 2.1.138) fixed via a resolve-latest step + pinned install in all 8 workflows; D3 (claude.*usage regex hiding every Claude error) replaced by api_error_status classification with an honest exit contract and a redacted hop log; D4 fixed syntactically (--deny-tool shell/write). AC-4 safety hop added. Remaining ACs (AC-5 liveness, probe runs) are explicitly deferred post-merge, which matches the issue plan. Substantively addressed.

Findings

1. [MEDIUM — verified, blocks approval] readme-refresh.yml keeps its job-level CLAUDE_CODE_VERSION: ${{ vars.CLAUDE_CODE_VERSION || '2.1.138' }} pin — the only one of the 8 changed workflows not defaulting to latest. The new resolver step reads vars.CLAUDE_CODE_VERSION || 'latest' (ignoring the job env), and the install step's step-level env overrides the job pin, so the workflow is silently unpinned from 2.1.138 while the dead pin line and its 'Pin ... for reproducible caching' comment remain. Unpinning is likely the PR's intent (2.1.138 is exactly the stale version [Phase 2 blocker] Deep-tier reviews fail on v1-next: invalid claude-sonnet-5-0 ID (404), stale cached Claude Code CLI 2.1.138, and a rate-limit regex that hides every Claude error #1957 complains about), but the leftover default is now misleading — either change the job env to 'latest' to match the other workflows, or thread env.CLAUDE_CODE_VERSION into REQUESTED_VERSION. This is the unresolved CodeRabbit thread on that file (rated Major); it needs a small code fix or an explicit disposition.
2. [LOW] Unresolved CodeRabbit thread on scripts/engine.sh (hop-log): when the failed hop's stdout is not parseable JSON, _claude_log_hop writes the first 1000 bytes of raw stdout (through redact_secrets) to the debug artifact. The cap + redaction + internal-artifact scope make this an acceptable, documented trade-off, but the thread should be answered/resolved (or the fallback swapped for a placeholder per the suggestion).
3. [INFO] .gitleaksignore additions are commit-scoped to historical commit beddd2c (a literal dummy ghp_ token in a test, since rewritten to assemble at runtime) — consistent with the file's existing pattern; gitleaks CI is green on head. The run_secret_scanning MCP tool was unavailable in this run; no MCP scan performed.
4. [INFO] Engine chain semantics verified: non-retryable errors (e.g. 401) propagate immediately; exit 2 is now keyed on the LAST hop being throttled, matching how review-one-pr.sh scans the final hop's output; warnings deliberately avoid rate-limit-pattern tokens. Tests cover all branches.

CI status

All required checks green on d3f1bae (shellcheck, actionlint, bats, unit-tests, CodeQL, SonarCloud, gitleaks, agent-shield, all workflow-governance guards). dev-lead dispatch/ci-relay/resume show CANCELLED (automation jobs, not PR validation); cubic reported NEUTRAL; the CodeRabbit status is PENDING but its review is posted. No CI failures.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

1. Resolve all unresolved review thread comments from other reviewers
2. Ensure all CI checks pass after your changes
3. Rebase on the target branch if behind
4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

[don-petry]

Both findings in the cycle-1 review (which reviewed d3f1bae) are already fixed on the current head, b81747f:

1. [MEDIUM] readme-refresh.yml pin: in all 8 workflows, REQUESTED_VERSION now reads ${{ env.CLAUDE_CODE_VERSION || vars.CLAUDE_CODE_VERSION || 'latest' }}. readme-refresh's job default of 2.1.138 therefore passes through to both the cache key and the install, and the pin is no longer dead. CodeRabbit verified this and resolved the thread.
2. [LOW] hop-log fallback: non-JSON stdout is now replaced with (non-JSON stdout omitted) instead of being logged raw. A new bats case covers it (15/15 pass). CodeRabbit verified this and resolved the thread.

There are no unresolved review threads on the PR. No further code change is pending from me; the next review cycle should run against b81747f.

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-27T03:04:30Z.

[don-petry]

Acknowledged — informational. SonarCloud's Quality Gate passed on this PR; this is a status notice with no action required.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 1ad231f50cc3cc5618acf7863443c93aa5fbe448
Review mode: triage-approved (single reviewer)

Summary

Confirmation review after triage clearance and a prior fix-requested cycle (d3f1bae). Both prior findings are resolved on this head: (1) the version resolver now reads env.CLAUDE_CODE_VERSION before vars in all 8 workflows, so readme-refresh.yml's job-level 2.1.138 pin is honored for both the cache key and the install — the pin comment is accurate again; (2) _claude_log_hop no longer copies raw non-JSON stdout into the debug artifact, writing a '(non-JSON stdout omitted)' placeholder instead (CWE-532), with a new bats case pinning it and the teardown early-return fixed per the Graphite thread. The underlying #1957 fix remains sound: valid claude-sonnet-5 id, per-model hop-failure classification keyed on api_error_status (404/400/5xx labelled honestly instead of masquerading as throttling), exit-2 keyed on the LAST hop being throttled, opus-4-8 safety hop in the deep chain, fresh CLI cache keys, and bare claude-sonnet-5 pricing rows with tests.

Linked issue analysis

Closes #1957 (Phase 2 blocker under epic #1895). D1 (invalid claude-sonnet-5-0 → claude-sonnet-5) fixed in engine.sh, aw.sh, model-pricing.tsv; D2 (stale cached CLI) fixed via the resolve step + concrete-version cache key in all 8 workflows, honoring job pins; D3 (over-broad claude.*usage regex) replaced by _claude_hop_failure_class with an honest exit contract and a redacted hop log; D4 fixed (--deny-tool shell/write). AC-4 safety hop (opus-4-8) added to the deep chain. AC-5 liveness/probe runs explicitly deferred post-merge per the issue plan. Substantively addressed.

Findings

1. [RESOLVED] Prior MEDIUM (readme-refresh.yml silently unpinned): REQUESTED_VERSION now reads env.CLAUDE_CODE_VERSION || vars.CLAUDE_CODE_VERSION || 'latest' and the install env falls back through the resolved output — verified at head that the job pin threads through to both cache key and install. Applied uniformly across all 8 workflows.
2. [RESOLVED] Prior LOW (hop log copied raw stdout head): non-JSON stdout is now omitted with a placeholder; JSON envelopes contribute only their error fields (is_error, api_error_status, subtype, result), capped at 1000 bytes and passed through redact_secrets. New bats tests cover text mode, redaction, missing-helper, and log-off paths.
3. [INFO] All 7 review threads (CodeRabbit, CodeAnt, Gemini, Graphite) are resolved on this head; CodeRabbit's earlier review is dismissed. No human reviewer raised questions — remaining bot comments are billing/quota notices, acknowledged with dispositions.
4. [INFO] .gitleaksignore additions are commit-scoped to historical commit beddd2c (literal dummy ghp_ token in a test, since rewritten to runtime assembly) — consistent with the file's existing pattern; gitleaks is green on head. The run_secret_scanning MCP tool was unavailable in this run; no MCP scan performed, gitleaks CI covers the diff.
5. [INFO] Chain semantics re-verified: 401/unclassified errors propagate immediately; 404/400/5xx log a named ::warning:: and advance the hop; exit 2 requires the LAST hop to be genuinely throttled, matching review-one-pr.sh's final-hop scan. Warning phrasing deliberately avoids rate-limit tokens. No workflow security smells: no new secret handling, action pins unchanged, resolver input comes from trusted vars/env.

CI status

All checks green on 1ad231f: bats, unit-tests, unit, shellcheck, actionlint, CodeQL (actions + python), SonarCloud Quality Gate passed, gitleaks, agent-shield, and all workflow-governance guards. Zero in-progress checks. CANCELLED entries (dev-lead dispatch/ci-relay/resume, superseded review/Dismiss runs) are automation jobs, not PR validation; cubic reported NEUTRAL; dependency-audit ecosystem jobs SKIPPED (none detected).

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 1ad231f50cc3cc5618acf7863443c93aa5fbe448
Review mode: triage-approved (single reviewer)

Summary

Fixes the Phase 2 deep-tier blocker (#1957): corrects the invalid claude-sonnet-5-0 model id to claude-sonnet-5 across engine chains, the aw.sh allowlist, and pricing (with bare-id rows the glob missed); makes the CLI cache key resolve latest to the concrete npm version in 8 workflows so stale CLIs stop being reused; replaces the over-broad claude.*usage rate-limit regex with per-hop classification by api_error_status (429/529/402/413 → throttled, 404 → unavailable, 400 → rejected, 5xx → server error, else propagate) plus a redacted hop log; adds claude-opus-4-8 as the deep-chain safety hop; and fixes the copilot triage --deny-tool syntax. 32 new/updated bats tests pin the behavior.

Linked issue analysis

Linked issue #1957 enumerates four defects (D1–D4) with run evidence; this PR addresses D1, D2, D3, D4 (syntax part), and AC-4 exactly as specified — the remaining PAT swap explicitly needs a maintainer and is out of scope. Substantively addressed.

Findings

• Cycle-1 findings verified fixed on the current head: all 8 workflows honour a job-level env.CLAUDE_CODE_VERSION pin before vars/latest (including readme-refresh.yml), and the hop log omits non-JSON stdout. Final commit 1ad231f is a test-only teardown fix.
• All 7 review threads are resolved; bot nitpicks (CodeAnt jq fallback in stub-claude) were addressed in code.
• .gitleaksignore additions are narrowly scoped to one historical commit's intentionally fake test token (now assembled at runtime); gitleaks CI passes on the head.
• Workflow additions are safe: version expressions enter shell via step-level env: (no inline interpolation), no new secret handling, actions stay SHA-pinned.
• run_secret_scanning MCP tool was unavailable in this run; relied on the passing gitleaks check.
• Minor (non-blocking): the "trying next in chain" warning also prints when the failed hop is the last in the chain, and hop-log truncation at 1000 bytes happens before redaction, so a token split at the boundary would leave a fragment. Neither affects correctness.

CI status

All checks green on 1ad231f: shellcheck, bats, actionlint, unit-tests, CodeQL, SonarCloud Quality Gate, gitleaks, agent-shield, and all stub/permission guards pass. CANCELLED entries are superseded runs from earlier pushes on this PR; skipped audits are ecosystem-not-detected. Cubic reported NEUTRAL.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 5/7 required advisory bots reported before the gate's quiescence-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 1ad231f50cc3cc5618acf7863443c93aa5fbe448
Review mode: triage-approved (single reviewer)

Summary

Confirmation review (triage-approved mode) of the #1957 deep-tier unblock. Four coordinated fixes: (1) the invalid claude-sonnet-5-0 model ID corrected to claude-sonnet-5 in engine.sh chains, aw.sh allowlist, and model-pricing.tsv (bare-ID rows added because the claude-sonnet-5-* glob does not match the bare ID); (2) 8 workflows resolve the literal 'latest' CLI cache key to the concrete npm version so new releases miss the cache, with a weekly-key fallback when npm is unreachable and pass-through for a pinned vars.CLAUDE_CODE_VERSION — identical implementation in all 8, actions/cache pin unchanged; (3) honest chain-error classification: the over-broad 'claude.usage' rate-limit regex removed, failed hops classified by api_error_status (429/529/402/413→throttle-and-continue, 404/400/5xx→labelled-and-continue, anything else propagates immediately), exit 2 reserved for a genuinely throttled last hop, and a redacted, size-capped hop log added to the debug artifact; (4) triage Copilot fallback uses valid --deny-tool shell/write syntax instead of the rejected ''. Deep chain gains claude-opus-4-8 as a known-good safety hop. 14 new bats tests pin the classifier (including redaction and hop-log hygiene); existing chain/dispatch/pricing tests updated consistently.

Linked issue analysis

Closes #1957 (Phase 2 blocker under epic #1895). D1 (invalid sonnet-5-0 ID), D2 (stale cached CLI 2.1.138), D3 (rate-limit regex masking every Claude error), and the syntax half of D4 (--deny-tool '*') are all substantively addressed and match the issue's own recommendations. Explicitly deferred with rationale: the D4 classic-PAT swap (needs a maintainer token change) and AC-5 liveness probes — tracked on #1899. Deferral is documented in the PR body's monitoring section.

Findings

No blocking findings.

• .gitleaksignore (scrutinized): the two new entries are SHA-scoped to historical commit beddd2c and suppress an obvious dummy fixture (ghp_ + alphabet + digits) used in a redaction test; verified the literal exists only at that commit — the current head assembles it at runtime via string concatenation. Matches the file's established suppression pattern; gitleaks CI passes at head.
• Hop log hygiene is sound: only envelope error fields, 1000-byte cap, redact_secrets applied, non-JSON stdout omitted, header-only when the redact lib is missing — each behavior covered by a dedicated test.
• Workflow edits add no new actions and keep the existing actions/cache SHA pin; secrets handling is unchanged (gh-aw-cross-org's GH_TOKEN/GOOGLE_API_KEY env forwarding predates this PR).
• Prior cycle-1 findings (readme-refresh.yml pin inconsistency and the stub-claude jq fallback) are confirmed fixed at head: all 8 workflows use the identical env||vars||latest resolution, and stub-claude's error branch has a printf fallback when jq is absent.
• The run_secret_scanning MCP tool was not available in this run; relied on the passing gitleaks check plus manual inspection of the diff — no credential material found in added content.
• Semantic note (accepted, documented in the PR's Risk section): chains now stop early on non-retryable errors (e.g. 401) that were previously masked as throttling, and a chain ending on 404/400/5xx returns 1 instead of 2 — intentional per the fix's design, covered by tests.

CI status

All required checks green at 1ad231f: SonarCloud Quality Gate passed, CodeQL, Agent Security Scan, gitleaks secret scan, actionlint, shellcheck, bats/unit tests, and all repo validation gates SUCCESS. Skipped/cancelled entries are superseded runs and inapplicable ecosystem audits. Advisory-bot gate proceeded on 5/7 quiescence-timeout (two bots billing-blocked — Codex and Qodo — not adverse findings).

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.