fix(engine): unblock Opus 5.5 deep tier β€” valid Sonnet 5 id, fresh CLI cache, honest chain errors (#1957)

Merged
DO
24d ago
+724-96

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 (#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 #1950 probe for every model in the chains, and recording the result on #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

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.