fix(claude-adapter): read the login veto from the result event, not the transcript (PEN-3259)

[allyblockcast]

Closes PEN-3259. Stacked on #1862 (PEN-3223) — base is that PR's branch, so this diff is just the follow-up. The stacking is load-bearing, not cosmetic; see Why stacked below.

The defect

isClaudeTransientUpstreamError vetoes on detectClaudeLoginRequired, which joins parsed.result + parsed.errors[] + stdout + stderr and tests CLAUDE_AUTH_REQUIRED_RE against every line. stdout is the entire pod log, so a run whose transcript merely mentions auth vetoes its own transient classification — even when the terminal result event is an unambiguous capacity refusal.

This is the mirror image of PEN-3223: same unbounded read, but in a veto, so it suppresses a true label instead of inventing a false one.

The change

Two one-line edits. detectClaudeLoginRequired is unchanged — it is shared and stdout is the right surface for its other callers, which ask whether a run needs re-authentication before any result event exists. What changed is what this rule passes it, gated on parsed exactly as #1862 gates the haystack:

stdout: parsed ? "" : (input.stdout ?? ""),

Everything else in the diff is tests and the measurement written down at the call site.

Measurement

Corpus: 446 failed runs carrying a terminal result event, rebuilt from retained pod logs written since 2026-09-12 (stream == "stdout" chunks reassembled, then run through the adapter's real parseClaudeStreamJson). Composition: api_error_status 403 ×281, 429 ×95, 503 ×70.

	firings
login veto reading the transcript	91
login veto reading the bounded surfaces	0

Not one run in the corpus reports an auth failure on its own result event — so every one of the 91 firings was transcript-only. 89 of the 91 are the bare word unauthorized.

What the widening admits — enumerated, not counted

27 runs move from no family at all to transient_upstream:

• 19 × Penstock capacity refusal — API Error: Request rejected (429) · All Claude subscription capacity for this tenant is rate-limited, each carrying an explicit reset timestamp.
• 8 × Anthropic 503 — Service temporarily unavailable. This is a server-side issue, usually temporary — try again in a moment.

Both families are transient by definition. No 403 is admitted. The other 64 wide-veto firings are 403s that stay non-transient regardless — after #1862 their bounded surfaces do not match the transient regex, so the veto was doing no work on them.

0 runs lose a transient label. The change is a pure widening, and it is exactly those two families.

Why stacked on #1862 — measured, not assumed

Running this narrowing against the pre-#1862 wide haystack on the same 446 runs:

variant	transient total	of which 403
master today	237	99
master + this change alone	301	136
#1862 + this change (shipped)	165	0

On master alone this change makes PEN-3223's defect worse — 37 more permanent authorization failures buying paid retries. The no-403 safety property is inherited from #1862, not intrinsic. This is recorded at the call site so the hunk cannot be quietly cherry-picked ahead of it.

The claude-local twin

Fixed, not scoped out. PEN-3259 predicted it is more exposed; I checked rather than repeating it, and it is — by more than the issue suggested. Evaluating both copies' CLAUDE_AUTH_REQUIRED_RE over the same 446 transcripts:

• claude-local fires on 308; the k8s copy on 91 (local-only 219, k8s-only 2).
• failed to authenticate alone accounts for 285 of the 308 — ordinary git / gh / ssh / container-registry output, not anything the Claude CLI says about its own session.

(Those transcripts are k8s pod logs, so this compares the two patterns against representative agent output; it is not a claim about runs that adapter served. Stated as such in the code.)

The parsed-gating matters more here: this copy's execute.ts calls the classifier directly from its !parsed fallback, which is the live path where a transcript login prompt is the only evidence. That path keeps the transcript read, and a test pins it.

Deliberately out of scope

• isClaudeProviderQuotaError (claude-local) — same latent shape, but narrowing a second veto in the same change would grant a second retry family off one measurement, and it routes to a different outcome. Noted at the call site.
• matchClaudeUpstreamCapacityCode — unchanged; fix(claude-adapter): classify transient upstream from the result event, not the transcript (PEN-3223) #1862 already recorded why it is unreachable.

Verification

• Vendored adapter (reproducing the vendor_claude_k8s CI job exactly — npm ci --include=dev, tsc --noEmit, vitest run): tsc clean, 904/904 tests across 16 files.
• claude-local parse suite: 65/65, with the 4 new PEN-3259 tests confirmed executing under a name filter rather than assumed from a green total.
• PROVENANCE.md integrity hash regenerated and re-verified to match after the final edit.
• Every number above was re-derived against the exact file this PR ships.

⛔ All measurement was read-only. No credential, token or session material was read or quoted.

[allyblockcast]

🔗 Paperclip issue: PEN-3259
🔗 Paperclip issue: PEN-3223

[allyblockcast]

🔗 Paperclip issue: PEN-3259
🔗 Paperclip issue: PEN-3223

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[github-actions]

@ally head fb2f835 has been awaiting review for 10.5h with no review on either surface (pulls/1868/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head fb2f835.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: fb2f835

Critical Issues (1)

• [code] packages/adapters/claude-local/src/server/parse.ts:687 — The claude-local half of this fix is a no-op at every production call site, so PEN-3259 stays live in the copy this PR's own comment calls the more exposed of the two (308/446 vs 91/446). Every caller of isClaudeTransientUpstreamError in this package already computes detectClaudeLoginRequired over the full proc.stdout and gates on it before reaching the classifier: execute.ts:1176 gates execute.ts:1224 (the !parsed path) and execute.ts:1388 (the parsed path), and test.ts:488 gates test.ts:548. On exactly the input the new tests describe — a genuine api_error_status: 429 whose transcript merely says failed to authenticate — the outer veto sets requiresLogin first, transientUpstream is forced false, and resolvedErrorCode (execute.ts:1407) becomes claude_auth_required. The narrowed veto inside parse.ts is never reached with a differing verdict. The k8s twin has no such outer gate (detectClaudeLoginRequired does not appear in vendor/paperclip-adapter-claude-k8s/src/server/execute.ts at all), which is why that half genuinely lands.
  • Either narrow execute.ts:1176's stdout the same way (parsed ? "" : proc.stdout) so the fix reaches the live path, or — if that is deliberately out of scope — rewrite the comment at parse.ts:662-685 to say plainly that this copy's runtime classification is unchanged, and why. As written it argues at length that this copy is the more exposed one, immediately above a change that cannot affect it.

Important Issues (1)

• [tests] packages/adapters/claude-local/src/server/parse.test.ts:159 — The k8s twin added an end-to-end assertion through classifyClaudeUpstreamFailure ("routes that 429 to the transient retry family end to end"); claude-local added only classifier-level tests. That asymmetry is what lets the Critical above pass CI green — all four new tests here assert on isClaudeTransientUpstreamError in isolation, which is the one layer where the change does take effect.
  • Add a test that composes the decision the way execute.ts does — compute detectClaudeLoginRequired over the full transcript, then assert the resulting transientUpstream / errorCode. It fails today, which is the point.

Suggestions (2)

• [comments] packages/adapters/claude-local/src/server/parse.ts:674 — "Evaluating both copies' regexes over the same 446 reconstructed transcripts: this one fires on 308" describes CLAUDE_AUTH_REQUIRED_RE alone, but this copy's detectClaudeLoginRequired (parse.ts:185-190) also skips type: "assistant" lines and ORs a second CLAUDE_GENERIC_AUTH_RE && CLAUDE_GENERIC_AUTH_CONTEXT_RE arm. The function's firing rate is therefore not the regex's. One clause naming which was measured would keep an unusually load-bearing comment exact.
• [comments] vendor/paperclip-adapter-claude-k8s/src/server/parse.ts:397 — "reads the same bounded surfaces as the haystack below" is not quite exact: buildClaudeTerminalResultHaystack also folds in errorMessage and api_error_status, neither of which the login veto sees. Benign in practice — at every call site errorMessage derives from describeClaudeFailure(parsed), so it is redundant with parsed.result — but the claim reads stronger than it is.

Strengths

• The k8s narrowing is correct and well-evidenced. detectClaudeLoginRequired really does read parsed.result + errors[] + stderr (vendor/paperclip-adapter-claude-k8s/src/server/parse.ts:173-185), so the bounded surfaces stay covered; and that adapter surfaces no claude_auth_required code anywhere, so the veto only ever suppressed the transient label — narrowing it forfeits nothing actionable.
• The PEN-3223 ordering counterfactual (237→301 transient, 99→136 403s on the pre-PEN-3223 haystack) is exactly the right thing to measure before relaxing a veto, and recording it as "inherited, not intrinsic — do not cherry-pick this hunk ahead of it" is a note that survives a rebase.
• Declining to narrow the quota veto and matchClaudeUpstreamCapacityCode in the same change, with the measurement recorded rather than acted on, is the correct scope line.
• PROVENANCE.md integrity hash regenerated in the same PR, and the CI-enforced vendor_claude_k8s job is green at this head.

Recommended Action

1. Resolve the Critical before merge — extend the narrowing to execute.ts:1176, or correct the claude-local comment to state that the live path is unchanged.
2. Add the execute-level claude-local test this cycle.
3. Take the comment-precision suggestions opportunistically.

Note, not a review finding: the review check-run from commitperclip PR Review (run 34928222038) is failure at this head; every other context is green.