diff --git a/docs/skills/ce-code-review.md b/docs/skills/ce-code-review.md index abec72ed1..1b5c85513 100644 --- a/docs/skills/ce-code-review.md +++ b/docs/skills/ce-code-review.md @@ -112,7 +112,7 @@ When you pass a PR number or URL, trivial automated PRs (lockfile bumps, chore v ### Cross-model adversarial pass -When adversarial is selected and the working tree is the reviewed head (current branch, or a PR whose local tree already matches the PR head), the adversarial lens runs through **one different model provider than the host** in a separate read-only process. A started peer **replaces** the in-process `adversarial` persona. They never both receive the same brief. The in-process persona runs only if the peer cannot start. Remote PR or branch diffs stay on the in-process persona, because that reviewer can inspect the fetched refs. +When adversarial is selected and the working tree is the reviewed head (current branch, or a PR whose local tree already matches the PR head), the adversarial lens runs through **one different model provider than the host** in a separate read-only process. A started peer **replaces** the in-process `adversarial` persona. They never both receive the same brief. The in-process persona runs if the peer cannot start, or if the started peer returns only session-quota or auth-context failure — in that case the next announced different-family peer is tried when one is eligible, otherwise the local persona covers the lens. After a stubborn transient rate limit, one same-route retry then local; no recipient switch. Remote PR or branch diffs stay on the in-process persona, because that reviewer can inspect the fetched refs. Agreement between the peer and another in-process reviewer is a strong promotion signal in synthesis. diff --git a/skills/ce-code-review/SKILL.md b/skills/ce-code-review/SKILL.md index 230830944..b907b00ca 100644 --- a/skills/ce-code-review/SKILL.md +++ b/skills/ce-code-review/SKILL.md @@ -48,7 +48,7 @@ Follow these boundaries in order; references supply the detail but never change 1. Resolve the reviewed diff and intent. 2. Read `references/persona-catalog.md`, then select the risk-driven reviewer roster and discover applicable standards paths. Do not select or dispatch personas without that catalog load. -3. When adversarial is selected for a local reviewed tree, start and persist the sanctioned cross-model job **before any local persona dispatch**. Invoking this skill authorizes its configured/allowlisted peer route after the required recipient-and-code-egress disclosure; do not ask for a second confirmation or skip merely because the user did not separately repeat that authorization. An explicit user prohibition on external review still wins. A started peer replaces the local adversarial persona; only an actual scope, allowlist, availability, authentication, or start failure leaves the local fallback. +3. When adversarial is selected for a local reviewed tree, start and persist the sanctioned cross-model job **before any local persona dispatch**. Invoking this skill authorizes its configured/allowlisted peer route after the required recipient-and-code-egress disclosure; do not ask for a second confirmation or skip merely because the user did not separately repeat that authorization. An explicit user prohibition on external review still wins. A started peer replaces the local adversarial persona; only an actual scope, allowlist, availability, authentication, or start failure leaves the local fallback at Stage 3d. Fold-in may still run the did-not-run fallback in `references/cross-model-review.md` when skip evidence shows session/usage quota or execution-context auth, or restore local after a failed same-route rate-limit retry. 4. Before any local dispatch, read `references/dispatch-reviewers.md`; if it is not loaded, stop and load it. Then dispatch the materialized local roster as a foreground concurrent batch sized to the host's active-agent cap — spawn multiple reviewers in one message with background execution off where the harness runs same-message calls concurrently, and collect every reviewer before synthesis (one blocking wait on Claude-style harnesses; repeated non-polling collection waits on async `spawn_agent` harnesses); degrade to serial where it does not. Detaching local review into a polled background job is forbidden; the cross-model peer is the only detached work and overlaps with this batch. Shell no-ops and wakeup polling are forbidden. 5. After the reviewer returns are ready, read `references/finish-review.md`; if it is not loaded, stop and load it. Fold in the peer once, run the documented findings mechanics, run every validator the reference selects, and only then return the report. Never synthesize directly from raw reviewer artifacts. The exact Actionable Findings, Coverage, and Verdict completion fields are required. When a peer ran, Coverage must record its route plus the literal keyed fields `model_requested`, `model_actual`, `effort_requested`, `effort_actual`, `receipt_supported`, and `independence_verified` from the artifact; never shorten that tuple to a model family or vague "high reasoning" claim. In the multi-agent path, emit only this skill's report; do not also invoke a harness-native findings/reporting tool. The native review tool belongs only to the explicit Quick Review Short-Circuit. Bare and `mode:agent` reviews never apply fixes; only explicit `apply:local` can enter the apply stage. @@ -175,7 +175,7 @@ Reviewer personas are selected in layers. The persona catalog in `references/per - `api-contract-reviewer` — routes, serializers, type signatures, versioning - `data-migration-reviewer` — migration files / schema dumps / backfills (see spawn gate in Stage 3) - `reliability-reviewer` — error handling, retries, timeouts, background jobs -- `adversarial-reviewer` lens — >=50 changed code lines, or auth / payments / persistence writes / event publication / retry or concurrency semantics / external APIs, or a **silent-pass verification mechanism** regardless of size. Satisfy this lens with the independent cross-model adversarial pass when a sanctioned peer job starts successfully. Dispatch the in-process `adversarial-reviewer` only as the fallback when the peer cannot start; do not run both same-brief reviews. +- `adversarial-reviewer` lens — >=50 changed code lines, or auth / payments / persistence writes / event publication / retry or concurrency semantics / external APIs, or a **silent-pass verification mechanism** regardless of size. Satisfy this lens with the independent cross-model adversarial pass when a sanctioned peer job starts successfully. Dispatch the in-process `adversarial-reviewer` when the peer cannot start, when fold-in runs the did-not-run fallback, or when fold-in restores local after a failed same-route rate-limit retry; do not run both same-brief reviews. - `previous-comments-reviewer` — PR with existing review comments (PR-only, comment-gated) **Stack-specific conditional (per diff):** `julik-frontend-races-reviewer` (Stimulus/Turbo, DOM events, async UI) and `swift-ios-reviewer` (Swift/SwiftUI/UIKit, entitlements, Core Data, `.pbxproj`). @@ -455,7 +455,7 @@ Distinguish an empty successful search from a failed or unavailable search: ### Stage 3d: Bind the adversarial route and final roster -Complete this stage **before reading persona prompt assets or entering Stage 4**. It owns the exclusive choice between a cross-model adversarial peer and the in-process `adversarial-reviewer`; later stages consume that choice and must not decide it again. +Complete this stage **before reading persona prompt assets or entering Stage 4**. It owns the exclusive choice between a cross-model adversarial peer and the in-process `adversarial-reviewer`; later stages consume that choice and must not decide it again, except the fold-in did-not-run fallback or the fold-in in-process restore after a failed same-route rate-limit retry. Generate the review run ID now so both routes share one artifact directory: @@ -473,7 +473,7 @@ echo "$RUN_DIR"; When adversarial was selected and scope is `local-aligned` or standalone, read `references/cross-model-review.md` from this skill's directory in full, attest the host, resolve and sanction one fixed route, and make its required egress announcement. Before start, write the reference's compact orchestrator-owned adversarial review brief to the run directory: intent plus the material risk divisions inferred from the current file inventory and diff, without embedding the diff or mechanically copying every path. Then start the detached peer job using the reference's exact invocation and persist its job ID, target, requested model/reasoning, and start epoch in working state. -- If the runner returns a job ID, the peer owns the adversarial lens for this run. Remove `adversarial-reviewer` from the local roster immediately. Do not read its local persona asset or dispatch it later, even if the peer eventually fails. +- If the runner returns a job ID, the peer owns the adversarial lens for this run. Remove `adversarial-reviewer` from the local roster immediately. Do not read its local persona asset or dispatch it later — except the fold-in did-not-run fallback in `references/cross-model-review.md` when skip evidence is session/usage quota or execution-context auth, or the fold-in in-process restore after a failed same-route rate-limit retry. - If no job starts because of a dispatch-infrastructure failure (a non-zero exit before any job id, an unresolved `$SKILL_DIR`/script path), first attempt the bounded same-route hand recovery from `references/cross-model-review.md` before accepting the fallback: re-run the identical resolved route, holding target/model and read scope fixed, while each failure is a new plausibly recoverable one and the shared peer deadline holds. If recovery returns a job id, treat it as the branch above (the peer owns the lens; remove `adversarial-reviewer`). Only when recovery is exhausted — a failure repeats or the deadline is spent — or the peer was never eligible to start (gate not met, host un-attestable, no different provider, CLI missing/unauthed), keep `adversarial-reviewer` in the local roster as the fallback and record the peer skip reason for Coverage. - In `pr-remote` / `branch-remote`, do not start the peer; keep the selected in-process adversarial reviewer because it can inspect the reviewed refs. diff --git a/skills/ce-code-review/references/cross-model-review.md b/skills/ce-code-review/references/cross-model-review.md index 5773b5caa..274721762 100644 --- a/skills/ce-code-review/references/cross-model-review.md +++ b/skills/ce-code-review/references/cross-model-review.md @@ -154,10 +154,18 @@ Repeat that call until the job is terminal or the derived deadline is spent; do - In final Coverage, name `cross_model_route`, `model_requested`, `effort_requested`, `receipt_supported`, `model_actual`, `effort_actual`, and `independence_verified` from the artifact. Keep the literal `unverified`; never compress a request into a serving claim such as "via Codex high" when actual model or effort is unverified. - **Never started / not run** — the job was never started (gates not met, host un-attestable, no different provider reachable, CLI missing/unauthed): the pass simply didn't run. Note "cross-model pass: not run" in Coverage for human-facing markdown; stay silent in `mode:agent`. Ignore any `*.raw.json` leftovers — they are not fold-in artifacts. - **Dispatch-infrastructure failure** — the runner or worker itself crashed: a non-zero exit before any job starts, a preflight/detach failure, or an unresolved `$SKILL_DIR`/script path. This is distinct from the gate-not-met skips above (there, no dispatch was attempted), so do not fold it into the silent not-run bucket on the first error. The two failure shapes recover at different points. A **no-job-id** preflight failure (exit before any job id, unresolved `$SKILL_DIR`) is recovered entirely at **Stage 3d's no-job branch**, before the local roster is materialized — the only point where re-running the start can still recover cross-model corroboration and, failing that, cleanly fall to the in-process reviewer (which then covers the lens; only corroboration is lost). Do **not** re-attempt that case here at fold-in: Stage 4 may already have dispatched the in-process `adversarial-reviewer`, so a fold-in peer re-run would put both on the same brief and violate the exclusive routing boundary. This step handles only the **job-id-returned-then-failed** crash — its failed job is reaped here and the in-process reviewer is already gone. For it, re-run the **same resolved fixed route** by hand — holding the target and model, the `git diff ` read scope, and the adversarial persona brief fixed — while each failure is a new, plausibly recoverable one and the shared peer deadline holds. This is a same-route retry, deliberately distinct from the quota rule below, which requires a newly disclosed route. Stop once a failure repeats or the deadline is spent; the hand recovery is then the adversarial lens's only cover, so the Coverage line must report the adversarial lens as **degraded**, not merely cross-model corroboration lost. A hand recovery may not substitute a different target or provider, widen the read scope, or relax the read-only trust boundary — those make the recovered peer untrustworthy, not merely unavailable. -- **Ran but produced no usable output** — the job reached `done` (or any terminal state) yet no `adversarial-.json` exists (the peer ran and egressed but returned nothing schema-shaped — unparseable output, empty findings the script dropped). Distinct from not-run: note "cross-model pass: peer ran, no usable output" in human-facing markdown Coverage. Never fail the review. -- **Started but not `done`** — the final status read reports `failed`, `timeout`, or `died-without-result` (a job reaped at the shared deadline records `timeout`, with the reap noted in its reason) → still non-blocking, but never silent: name the peer and its terminal state in Coverage (e.g. "cross-model adversarial peer: timeout"). Silent absence stays correct only for passes that never started or were skipped. +- **Ran but produced no usable output** — the job reached `done` (or any terminal state) yet no `adversarial-.json` exists (the peer ran and egressed but returned nothing schema-shaped — unparseable output, empty findings the script dropped). Distinct from not-run: note "cross-model pass: peer ran, no usable output" in human-facing markdown Coverage. Never fail the review. If skip evidence is session/usage quota or execution-context auth, this is not merely empty output — run the did-not-run fallback below before treating the lens as covered. +- **Started but not `done`** — the final status read reports `failed`, `timeout`, or `died-without-result` (a job reaped at the shared deadline records `timeout`, with the reap noted in its reason) → still non-blocking, but never silent: name the peer and its terminal state in Coverage (e.g. "cross-model adversarial peer: timeout"). Silent absence stays correct only for passes that never started or were skipped. If skip evidence is session/usage quota or execution-context auth, run the did-not-run fallback below rather than leaving the lens uncovered. - Empty `findings` → note "cross-model pass: no additional issues" in Coverage. -- **Classify the skip reason before deleting.** Read `out.log` before cleanup, including bounded lines prefixed `peer skip evidence:`, and name observed quota, authentication, or capability failure. An authentication-shaped peer failure (`not logged in`, `please log in`, 401, or CLI text prompting login) describes only the peer's execution context: a sandboxed host — e.g. a restricted Codex task denying spawned commands network or keychain access — produces the identical signal to a genuine account logout, so classify it as a cross-model execution-context authentication failure and never report it as the user's account being logged out or prompt the user to run a login command on that basis. The cross-model pass is additive and the local review still completed; obtaining it requires a context where the peer CLI can reach the network (for example, outside the restricted sandbox). After the same quota or usage-limit evidence appears more than once in this session, do not retry that route automatically. A retry uses a newly resolved, disclosed, and sanctioned fixed route; never silently continue to another recipient. +- **Classify the skip reason before deleting.** Read `out.log` before cleanup, including bounded lines prefixed `peer skip evidence:`. Judge the full diagnostic; do not grep for a closed phrase list. Login-shaped peer text describes only the peer's execution context: a sandboxed host produces the same signal as a genuine logout, so never report it as the user's account being logged out or prompt a login command. + - Session or usage quota (the route cannot review now): do not retry that route; run the did-not-run fallback. + - Execution-context authentication: same — the peer did not review; run the did-not-run fallback. + - Transient rate limit (throttle, retry shortly, without quota/session exhaustion): one same-route retry of the already disclosed route. If that retry still produces no review, dispatch in-process `adversarial-reviewer`. Do not run the did-not-run fallback and do not switch recipients. + - Anything else: name the observed failure; do not run the did-not-run fallback. +- **Did-not-run fallback** (first quota or execution-context-auth observation, including the first one): the started job did not cover the adversarial lens. Do not retry that same route. Then: + 1. If this is the first replacement attempt, the failed recipient was **not** an explicit user-stated preference (Step 1 item 1), and another attested-different installed+allowlisted target remains: announce that new recipient and start a new job with a new `CROSS_MODEL_FIXED_ROUTE`. Wait for it with the remaining shared deadline and fold its artifact the same way as the first job. That job owns the lens. Never switch recipients inside the worker. If this replacement also ends in quota or execution-context auth, do not start a third peer; take step 2. + 2. Otherwise (explicit recipient, or no other eligible peer): dispatch in-process `adversarial-reviewer` now. Coverage records the peer as not-run for quota/auth and that the lens used the local fallback. + A config or default selection is not an explicit user-stated preference. One replacement only; never silently continue to another recipient. - After fold-in (or after deadline reaping), delete the consumed job directory (`/jobs//`) — its log and result are review content and must not outlive their use. - A finding sharing a fingerprint with in-process `adversarial` promotes only when the artifact records `independence_verified: true`. Cursor-default artifacts default false; an unattested host skips automatic dispatch. diff --git a/skills/ce-code-review/references/dispatch-reviewers.md b/skills/ce-code-review/references/dispatch-reviewers.md index 1d6172b09..de028bd56 100644 --- a/skills/ce-code-review/references/dispatch-reviewers.md +++ b/skills/ce-code-review/references/dispatch-reviewers.md @@ -104,8 +104,8 @@ The artifact file **must** carry the full detail-tier fields (`why_it_matters`, #### Cross-model adversarial pass -Stage 3d already made the exclusive route choice and, when applicable, started the detached peer. Do not resolve, start, or substitute a route here. Dispatch only the materialized local roster. +Stage 3d already made the exclusive route choice and, when applicable, started the detached peer. Do not resolve, start, or substitute a route here except the fold-in did-not-run fallback in `references/cross-model-review.md` (quota/auth) or the fold-in in-process restore after a failed same-route rate-limit retry. Dispatch only the materialized local roster. -After the inline fast pass has completed and the local reviewer batch has started, prepare synthesis inputs while reviewers run. Do not poll the peer during that wave. After local reviewers finish, if Stage 3d persisted a peer job ID, perform the reference's single bounded status/wait/reap sequence and fold in whatever terminal artifact is available. Attribute from the artifact and clean up through the runner. A failure or timeout stays non-blocking and is named in Coverage; it never triggers a late in-process adversarial retry. Peer findings enter ordinary synthesis, but agreement promotion requires top-level `independence_verified: true`; false or absent independence is useful evidence, not different-model corroboration. Coverage must say whether the adversarial lens ran cross-model or used the in-process fallback. +After the inline fast pass has completed and the local reviewer batch has started, prepare synthesis inputs while reviewers run. Do not poll the peer during that wave. After local reviewers finish, if Stage 3d persisted a peer job ID, perform the reference's single bounded status/wait/reap sequence and fold in whatever terminal artifact is available. Attribute from the artifact and clean up through the runner. A failure or timeout stays non-blocking and is named in Coverage; it never triggers a late in-process adversarial retry, except the did-not-run fallback in `references/cross-model-review.md` when skip evidence is session/usage quota or execution-context auth, or the fold-in in-process restore after a failed same-route rate-limit retry. Peer findings enter ordinary synthesis, but agreement promotion requires top-level `independence_verified: true`; false or absent independence is useful evidence, not different-model corroboration. Coverage must say whether the adversarial lens ran cross-model or used the in-process fallback. The peer return enters Stage 5 as reviewer `adversarial-`, like any persona artifact. A pass that never started is recorded as not run (or as the in-process fallback when selected); a started peer that fails, times out, dies, or is reaped is named with its terminal state rather than vanishing silently. diff --git a/skills/ce-code-review/references/persona-catalog.md b/skills/ce-code-review/references/persona-catalog.md index 9696f30d6..ec2ed0cca 100644 --- a/skills/ce-code-review/references/persona-catalog.md +++ b/skills/ce-code-review/references/persona-catalog.md @@ -35,7 +35,7 @@ Spawned when the orchestrator identifies relevant patterns in the diff. The orch | `api-contract` | `api-contract-reviewer` | An externally consumed boundary changes: route/request/response definitions, serializers, published event schemas, API versioning, or a public package signature with evidenced downstream callers. A new or changed exported symbol inside one module is insufficient by itself. | | `data-migration` | `data-migration-reviewer` | Migration files, schema dumps (`db/schema.rb`, `structure.sql`), backfill scripts, data transformations — **not** model/query-only changes without migration artifacts | | `reliability` | `reliability-reviewer` | Error handling, retry logic, circuit breakers, timeouts, background jobs, async handlers, health checks | -| `adversarial` | `adversarial-reviewer` fallback | >=50 changed code lines; auth/payments; persistence writes or event publication; retry/partial-failure or concurrency/ordering semantics; external APIs; or a silent-pass verification mechanism. The lens runs through the independent cross-model peer when a sanctioned peer starts. Spawn this in-process persona only when the peer cannot start. A started peer and the fallback are mutually exclusive. | +| `adversarial` | `adversarial-reviewer` fallback | >=50 changed code lines; auth/payments; persistence writes or event publication; retry/partial-failure or concurrency/ordering semantics; external APIs; or a silent-pass verification mechanism. The lens runs through the independent cross-model peer when a sanctioned peer starts. Spawn this in-process persona when the peer cannot start, when fold-in runs the did-not-run fallback, or when fold-in restores local after a failed same-route rate-limit retry. A started peer that produced a review artifact and the fallback are mutually exclusive. | | `previous-comments` | `previous-comments-reviewer` | **PR-only AND comment-gated.** Reviewing a PR that has existing review comments or review threads from prior review rounds. Skip entirely when no PR metadata was gathered in Stage 1, OR when Stage 1's `hasPriorComments` flag is false (no `reviews` and no `comments` on the PR). | ## Stack-Specific Conditional (2 personas) diff --git a/skills/ce-code-review/scripts/cross-model-adversarial-review.sh b/skills/ce-code-review/scripts/cross-model-adversarial-review.sh index b292b730b..ec067c6bf 100755 --- a/skills/ce-code-review/scripts/cross-model-adversarial-review.sh +++ b/skills/ce-code-review/scripts/cross-model-adversarial-review.sh @@ -910,14 +910,11 @@ run_provider() { log "wrote $n finding(s) to $OUT (reviewer adversarial-$provider)" else log "provider $provider produced no usable schema-shaped output; skipping fold-in" - # Surface bounded peer output so the orchestrator can - # reason about WHY it was skipped (quota/usage-limit exhaustion vs an ordinary - # empty review) and, in a repeated-pass session, deprioritize an exhausted - # route. Harness-agnostic: the agent classifies from the text; this only makes - # the evidence visible in out.log. Surface BOTH streams -- the error can be on - # stdout (grok's 402) or stderr (claude/cursor auth/quota). Bash builtins only - # (the route sandbox has no tail/tr). Prefer structured error fields because - # a raw tail can discard the actionable message in a large CLI envelope. + # Surface bounded peer output so the orchestrator can reason about WHY it + # was skipped (quota/usage-limit exhaustion vs an ordinary empty review). + # Prefer structured error fields because a raw tail can discard the + # actionable message in a large CLI envelope. Surface BOTH streams -- the + # error can be on stdout (grok's 402) or stderr (claude/cursor auth/quota). if [ -s "$PEERLOG" ]; then _pt="$(bounded_failure_evidence "$PEERLOG")" log " peer skip evidence: $_pt" diff --git a/tests/review-skill-contract.test.ts b/tests/review-skill-contract.test.ts index 492f836d3..36f55a107 100644 --- a/tests/review-skill-contract.test.ts +++ b/tests/review-skill-contract.test.ts @@ -1217,10 +1217,52 @@ describe("cross-model peer skip legibility", () => { // to classify a quota/usage-limit exhaustion (harness-agnostic reasoning). expect(referenceSrc).toContain("peer skip evidence:") expect(referenceSrc).toMatch(/quota|usage-limit/i) - expect(referenceSrc).toMatch(/more than once in this session/i) + if (worker.includes("ce-code-review")) { + expect(workerSrc).not.toContain("peer skip class:") + expect(referenceSrc).not.toContain("peer skip class:") + expect(referenceSrc).toMatch(/did-not-run fallback/i) + expect(referenceSrc).toMatch(/Judge the full diagnostic/i) + expect(referenceSrc).toMatch(/never silently continue to another recipient/i) + expect(referenceSrc).toMatch(/explicit user-stated preference/i) + expect(referenceSrc).toContain("in-process `adversarial-reviewer`") + } else { + expect(referenceSrc).toMatch(/more than once in this session/i) + } }) } + test("code review restores the adversarial lens after a quota or auth no-review", async () => { + const skill = await readRepoFile("skills/ce-code-review/SKILL.md") + const dispatch = await readRepoFile( + "skills/ce-code-review/references/dispatch-reviewers.md", + ) + const reference = await readRepoFile( + "skills/ce-code-review/references/cross-model-review.md", + ) + + expect(skill).toMatch(/did-not-run fallback/) + expect(dispatch).toMatch(/did-not-run fallback/) + expect(reference).toMatch( + /another attested-different installed\+allowlisted target remains/i, + ) + expect(reference).toMatch(/announce that new recipient and start a new job/i) + expect(reference).toMatch(/Wait for it with the remaining shared deadline/i) + expect(reference).toMatch(/do not start a third peer/i) + expect(reference).toMatch( + /Otherwise \(explicit recipient, or no other eligible peer\)/i, + ) + }) + + test("code review exclusivity pointers allow in-process restore after a failed same-route rate-limit retry", async () => { + const skill = await readRepoFile("skills/ce-code-review/SKILL.md") + const dispatch = await readRepoFile( + "skills/ce-code-review/references/dispatch-reviewers.md", + ) + + expect(skill).toMatch(/failed same-route rate-limit retry/) + expect(dispatch).toMatch(/failed same-route rate-limit retry/) + }) + // A restricted host sandbox (e.g. a Codex task with network disabled) denies // the spawned peer CLI network/keychain, producing the exact same // `Not logged in` signal as a genuine account logout. The classifier surfaces diff --git a/tests/skills/ce-code-review-cross-model-routes.test.ts b/tests/skills/ce-code-review-cross-model-routes.test.ts index 6dde25966..30a775f73 100644 --- a/tests/skills/ce-code-review-cross-model-routes.test.ts +++ b/tests/skills/ce-code-review-cross-model-routes.test.ts @@ -600,6 +600,28 @@ describe("cross-model-adversarial-review skip paths — non-blocking, no file", const r = run(["codex", "claude", "HEAD", runDir], runDir, env) expect(r.stderr).toContain("Not logged in") expect(r.stderr).toContain("terminal_reason=api_error") + expect(r.stderr).toContain("peer skip evidence:") + expect(r.stderr).not.toContain("peer skip class:") + }) + + test("surfaces a Claude session-limit 429 as skip evidence, not a completed review", () => { + const payload = JSON.stringify({ + result: "You have hit your session limit", + api_error_status: 429, + terminal_reason: "api_error", + }) + const { env } = sandbox( + ["claude"], + `#!/bin/sh\ncat >/dev/null\nprintf '%s' '${payload}'\nexit 1\n`, + ) + const runDir = makeRunDir() + const r = run(["codex", "claude", "HEAD", runDir], runDir, env) + expect(r.code).toBe(0) + expect(r.files).not.toContain("adversarial-claude.json") + expect(r.stderr).toContain("peer skip evidence:") + expect(r.stderr).toContain("You have hit your session limit") + expect(r.stderr).toContain("api_error_status=429") + expect(r.stderr).not.toContain("peer skip class:") }) test("ancillary structured fields do not hide an unrecognized human-readable diagnostic", () => {