diff --git a/packages/adapters/claude-local/src/server/parse.test.ts b/packages/adapters/claude-local/src/server/parse.test.ts index 3607223e6689..31b40498c1de 100644 --- a/packages/adapters/claude-local/src/server/parse.test.ts +++ b/packages/adapters/claude-local/src/server/parse.test.ts @@ -156,6 +156,69 @@ describe("isClaudeTransientUpstreamError — transcript independence (PEN-3223)" }); }); +describe("isClaudeTransientUpstreamError — login veto transcript independence (PEN-3259)", () => { + const capacity429 = { + type: "result", + subtype: "success", + is_error: true, + api_error_status: 429, + result: + "API Error: Request rejected (429) · All Claude subscription capacity for this tenant is " + + "rate-limited; capacity may reset at 2026-09-13T10:19:59.613Z; retry in 1106s", + }; + + // This copy's CLAUDE_AUTH_REQUIRED_RE does NOT match the bare word + // `unauthorized` — unlike the k8s twin's — so the transcript here quotes a + // phrase this copy actually matches (`failed to authenticate`). Picking a token + // the regex ignores would make the test pass without exercising the change. + const authMentioningStdout = [ + '{"type":"system","subtype":"init","session_id":"s1","model":"claude-opus-4-6"}', + '{"type":"user","message":{"role":"user","content":[{"tool_use_id":"t1","type":"tool_result",' + + '"content":"deploy.log: worker failed to authenticate against the registry, retrying"}]}}', + ].join("\n"); + + it("keeps a genuine 429 transient when the transcript merely mentions auth", () => { + expect( + isClaudeTransientUpstreamError({ parsed: capacity429, stdout: authMentioningStdout }), + ).toBe(true); + }); + + it("classifies that 429 identically with and without the auth-mentioning transcript", () => { + expect(isClaudeTransientUpstreamError({ parsed: capacity429, stdout: authMentioningStdout })).toBe( + isClaudeTransientUpstreamError({ parsed: capacity429, stdout: "" }), + ); + }); + + it("still vetoes when the terminal result event itself reports a login requirement", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: { + type: "result", + subtype: "success", + is_error: true, + api_error_status: 429, + result: "API Error: rate limited — not logged in; please run `claude login`", + }, + stdout: "", + }), + ).toBe(false); + }); + + // This path is LIVE in this copy (unlike the k8s twin): execute.ts calls the + // classifier directly from its `!parsed` fallback, where the transcript is the + // only surface that can carry the CLI's login prompt. + it("still vetoes a transcript login prompt when no result event ever arrived", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: null, + stdout: "API Error: 429 rate_limit_error\nNot logged in. Please run `claude login`.", + stderr: "", + errorMessage: "Claude exited with code 1", + }), + ).toBe(false); + }); +}); + describe("isClaudeTransientUpstreamError", () => { it("classifies the 'out of extra usage' subscription window failure as provider quota", () => { expect( diff --git a/packages/adapters/claude-local/src/server/parse.ts b/packages/adapters/claude-local/src/server/parse.ts index a1963cf6bed5..9403de07aa66 100644 --- a/packages/adapters/claude-local/src/server/parse.ts +++ b/packages/adapters/claude-local/src/server/parse.ts @@ -659,12 +659,32 @@ export function isClaudeTransientUpstreamError(input: { if (parsed && (isClaudeMaxTurnsResult(parsed) || isClaudeUnknownSessionError(parsed) || isClaudePoisonedPreviousMessageIdError(parsed) || isClaudeImageProcessingError(parsed))) { return false; } - // The login and quota vetoes deliberately still read the whole transcript. Both - // can only ever SUPPRESS the transient label, so narrowing them would widen what - // this function grants — the opposite of this rule's defect. See PEN-3223. + // The login veto reads the same bounded surfaces as the haystack below, under + // the same `parsed` condition and for the same reason (PEN-3259). Twin of the + // change in `vendor/paperclip-adapter-claude-k8s/src/server/parse.ts`, which + // carries the full measurement. `detectClaudeLoginRequired` itself is unchanged + // and still reads `stdout` for its other callers, which ask whether the run + // needs re-authentication before any result event exists; what changes is what + // THIS rule passes it. + // + // This copy is the MORE exposed of the two, not the less. Its + // CLAUDE_AUTH_REQUIRED_RE does not match the bare word `unauthorized` (the k8s + // twin's does), but it does match `failed to authenticate` — which is ordinary + // output from git, gh, ssh and container registries rather than anything the + // Claude CLI says about its own session. Evaluating both copies' regexes over + // the same 446 reconstructed transcripts: this one fires on 308, the k8s twin + // on 91, and `failed to authenticate` alone accounts for 285 of the 308. + // (Those transcripts are k8s pod logs, so this is an exposure comparison of the + // two patterns against representative agent output, not a claim about runs this + // adapter served.) + // + // The quota veto below deliberately keeps the wide haystack: narrowing a second + // veto in the same change would grant a second retry family off one measurement, + // and `isClaudeProviderQuotaError` routes to a different outcome than this rule. + // Same latent shape, tracked separately rather than swept in. const loginMeta = detectClaudeLoginRequired({ parsed, - stdout: input.stdout ?? "", + stdout: parsed ? "" : (input.stdout ?? ""), stderr: input.stderr ?? "", }); if (loginMeta.requiresLogin) return false; diff --git a/vendor/paperclip-adapter-claude-k8s/PROVENANCE.md b/vendor/paperclip-adapter-claude-k8s/PROVENANCE.md index db8f863856d8..0e1874591d23 100644 --- a/vendor/paperclip-adapter-claude-k8s/PROVENANCE.md +++ b/vendor/paperclip-adapter-claude-k8s/PROVENANCE.md @@ -96,7 +96,7 @@ A manifest of `sha256(path)` over all 41 in-tree files, sorted by path under `LC_ALL=C`, itself hashes to: ``` -11d9dc9bd0f4539f02cb83959fd3857bae53eec8a5b59f17d5e0505fda5c148e +61714ceda78285c3335bac59a08dc54b33845cd09c0bab61c52fac6b21fdd9a9 ``` Regenerate with: diff --git a/vendor/paperclip-adapter-claude-k8s/src/server/parse.test.ts b/vendor/paperclip-adapter-claude-k8s/src/server/parse.test.ts index 9d790260e7f1..19345ebfcb43 100644 --- a/vendor/paperclip-adapter-claude-k8s/src/server/parse.test.ts +++ b/vendor/paperclip-adapter-claude-k8s/src/server/parse.test.ts @@ -442,6 +442,124 @@ describe("isClaudeTransientUpstreamError — transcript independence (PEN-3223)" }); }); +describe("isClaudeTransientUpstreamError — login veto transcript independence (PEN-3259)", () => { + // The byte-identical shape observed on the vetoed capacity refusals in the + // 2026-09-12 corpus: an unambiguous provider 429 on the terminal result event. + const capacity429 = { + type: "result", + subtype: "success", + is_error: true, + api_error_status: 429, + result: + "API Error: Request rejected (429) · All Claude subscription capacity for this tenant is " + + "rate-limited; capacity may reset at 2026-09-13T10:19:59.613Z; retry in 1106s", + }; + + // A transcript that merely *mentions* auth. `CLAUDE_AUTH_REQUIRED_RE` in this + // copy matches the bare word `unauthorized`, so an agent reading a 401 handler, + // tailing a log, or working on an auth ticket trips it — as this issue's own + // text would. None of this is the CLI asking to be logged in. + const authMentioningStdout = [ + '{"type":"system","subtype":"init","session_id":"s1","model":"claude-opus-4-6"}', + '{"type":"user","message":{"role":"user","content":[{"tool_use_id":"t1","type":"tool_result",' + + '"content":"src/auth.ts:42 if (!token) return res.status(401).send(\\"unauthorized\\");"}]}}', + '{"type":"assistant","message":{"id":"m1","content":[{"type":"text",' + + '"text":"That branch returns unauthorized when the header is absent."}]}}', + ].join("\n"); + + it("keeps a genuine 429 transient when the transcript merely mentions auth", () => { + expect( + isClaudeTransientUpstreamError({ parsed: capacity429, stdout: authMentioningStdout }), + ).toBe(true); + }); + + it("classifies that 429 identically with and without the auth-mentioning transcript", () => { + const withTranscript = isClaudeTransientUpstreamError({ + parsed: capacity429, + stdout: authMentioningStdout, + }); + const withoutTranscript = isClaudeTransientUpstreamError({ + parsed: capacity429, + stdout: "", + }); + expect(withTranscript).toBe(withoutTranscript); + expect(withTranscript).toBe(true); + }); + + it("routes that 429 to the transient retry family end to end", () => { + expect( + classifyClaudeUpstreamFailure({ + failed: true, + zeroTokenProgress: false, + parsed: capacity429, + stdout: authMentioningStdout, + errorMessage: describeClaudeFailure(capacity429), + }), + ).toEqual({ + family: "transient_upstream", + errorCode: "claude_transient_upstream", + capacityCode: null, + }); + }); + + // The narrowing must not blind the veto to a REAL auth failure. When the run is + // genuinely unauthenticated the result event says so on its own bounded surface, + // which is still read. + it("still vetoes when the terminal result event itself reports a login requirement", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: { + type: "result", + subtype: "success", + is_error: true, + api_error_status: 429, + result: "API Error: rate limited — not logged in; please run `claude login`", + }, + stdout: "", + }), + ).toBe(false); + }); + + it("still vetoes on an auth failure reported through parsed.errors[]", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: { + type: "result", + subtype: "success", + is_error: true, + result: "API Error: 503 service unavailable", + errors: [{ message: "authentication required" }], + }, + stdout: "", + }), + ).toBe(false); + }); + + it("still vetoes on an auth failure reported on stderr", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: capacity429, + stdout: authMentioningStdout, + stderr: "Invalid API key · Please run `claude login`", + }), + ).toBe(false); + }); + + // The transcript read is retained where it is the only surface that can carry a + // login prompt: the CLI dying before it emits a result event. Unreachable from + // `classifyClaudeUpstreamFailure` in this copy, live in the `claude-local` twin. + it("still vetoes a transcript login prompt when no result event ever arrived", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: null, + stdout: "API Error: 429 rate_limit_error\nNot logged in. Please run `claude login`.", + stderr: "", + errorMessage: "Claude exited with code 1", + }), + ).toBe(false); + }); +}); + describe("matchClaudeUpstreamCapacityCode", () => { it("returns the penstock exhaustion code from Claude's embedded provider JSON", () => { expect( diff --git a/vendor/paperclip-adapter-claude-k8s/src/server/parse.ts b/vendor/paperclip-adapter-claude-k8s/src/server/parse.ts index bc7a9ce21300..702a86bd5a23 100644 --- a/vendor/paperclip-adapter-claude-k8s/src/server/parse.ts +++ b/vendor/paperclip-adapter-claude-k8s/src/server/parse.ts @@ -394,21 +394,56 @@ export function isClaudeTransientUpstreamError(input: { )) { return false; } - // The login veto deliberately still reads the whole transcript, and that is a - // scope line rather than a clean bill of health. It can only ever SUPPRESS the - // transient label, so narrowing it would widen what this function grants — the - // opposite of this rule's defect — and a login prompt is emitted before any - // result event, where this classifier is unreachable anyway. + // The login veto reads the same bounded surfaces as the haystack below, under + // the same `parsed` condition and for the same reason (PEN-3259, the follow-up + // this comment used to defer). `stdout` is the whole pod log, so a veto that + // reads it suppresses the transient label from *transcript content* rather than + // from the fault — the mirror image of PEN-3223's defect, aimed at a true label + // instead of a false one. // - // It does carry the same defect mirrored into suppression, and that is live, not - // theoretical: in the PEN-3223 corpus 2 of the 9 genuine `api_error_status: 429` - // capacity refusals are vetoed here despite their own bounded surfaces matching, - // because an auth-shaped token appears somewhere in their transcript. Tracked - // separately rather than fixed alongside, because relaxing a veto grants retry - // families and needs its own evidence. + // Measured on 446 failed runs carrying a terminal result event (retained pod + // logs from 2026-09-12 on; 403 ×281, 429 ×95, 503 ×70). On that population the + // transcript-reading veto fires 91 times and the bounded-surface veto fires + // **0** times — i.e. not one run in the corpus reports an auth failure on its + // own result event, so every firing was transcript-only. 89 of the 91 are the + // bare word `unauthorized`, which any agent reading a 401 handler or tailing a + // log will write; this issue's own text would do it. + // + // What narrowing admits, enumerated rather than counted: 27 runs move from "no + // family at all" to `transient_upstream` — 19 Penstock capacity refusals ("All + // Claude subscription capacity for this tenant is rate-limited", each carrying + // an explicit reset timestamp) and 8 Anthropic 503s ("Service temporarily + // unavailable. This is a server-side issue, usually temporary"). Both families + // are transient by definition. **No 403 is admitted** — after PEN-3223 a 403's + // bounded surfaces do not match the transient regex, so the veto was doing no + // work on the other 64 firings. Nothing moves the other way: 0 runs lose a + // transient label. The widening is exactly those two families and nothing else. + // + // THAT SAFETY PROPERTY IS INHERITED FROM PEN-3223, NOT INTRINSIC — so do not + // cherry-pick this hunk ahead of it. Measured as a counterfactual on the same + // 446 runs: on the pre-PEN-3223 wide haystack this narrowing alone takes the + // transient label from 237 runs to 301, and the 403s in it from 99 to **136**. + // Without the haystack narrowing in front of it, relaxing the veto makes + // PEN-3223's defect worse rather than fixing this one; with it, 403s go to 0. + // + // Note what is NOT changed: `detectClaudeLoginRequired` is shared and is correct + // as written. Its other callers ask "does this run need re-authentication?" + // before any result event exists, where the transcript is the only surface that + // carries the prompt. What changes is what THIS rule passes it. + // + // Narrowing a veto grants retry families, so it is gated on the same `parsed` + // condition rather than applied unconditionally: with no result event there are + // no bounded surfaces, the CLI's login prompt is emitted before any result + // event, and that is exactly the shape the transcript read exists to catch. + // + // `stderr` is deliberately retained, matching `buildClaudeTerminalResultHaystack`. + // It is measured inert rather than assumed so: on all 446 runs, including or + // excluding it changes the verdict 0 times. It is kept because it is bounded + // CLI diagnostic output, so it is the right surface for a login prompt on a run + // that did produce a result event. const loginMeta = detectClaudeLoginRequired({ parsed, - stdout: input.stdout ?? "", + stdout: parsed ? "" : (input.stdout ?? ""), stderr: input.stderr ?? "", }); if (loginMeta.requiresLogin) return false;