fix(ai): repair Anthropic thinking replay rejected via statusless proxy SSE errors - #3915
Conversation
…xy SSE errors CLIProxyAPI forwards Anthropic's thinking-mutation/signature 400 body as an in-stream SSE error event on an HTTP 200 response, so the thrown error carries no HTTP status. Both repair classifiers required status === 400 and let the session loop on an unrecoverable replay rejection; they now accept the statusless shape while keeping the strict invalid_request_error message checks. Lore-id: 39c0a5se Constraint: statusless match must still require the full invalid_request_error thinking wording Rejected: repairing on any unattributable 400 | doubles failure latency for unrelated request errors Rejected: broadening only via message-status scan | proxy SSE bodies carry no status digits at all Confidence: high Scope-risk: narrow Reversibility: trivial Tested: wire-level HTTP 200 SSE error simulation pre/post fix; statusless classification and repair-retry unit tests Not-tested: live server-side 400 reproduction through CPA (upstream currently normalizes tampered thinking)
…ction Live CLIProxyAPI captures do not forward the upstream 400 body at all -- the client only sees a generic api_error SSE event on an HTTP 200 response. That payload names no cause and matches no transient phrase, so the classifiers and the retry budget both declined it and the turn died on the first attempt while every subsequent request in the session replayed the same rejected history. The masked shape carries no evidence, so the request supplies it: a masked rejection before the first token, on a request that still replays signed thinking blocks, now takes the existing latest-then-full-history repair ladder. Lore-id: 5c1d0a7e Constraint: masked bodies are indistinguishable from a masked 5xx -- gate on the replayed request shape, never on the payload alone Rejected: retry any statusless api_error | hides unrelated upstream failures behind a second identical request Rejected: strip thinking replay pre-emptively behind proxies | loses valid reasoning context on every proxied turn Confidence: high Scope-risk: narrow Reversibility: easy Tested: wire-level SSE proxy simulation for both masked and forwarded bodies; masked run fails on the parent commit and recovers here Not-tested: live CPA traffic against api.anthropic.com
|
Follow-up: the first commit was not sufficient against live CPA traffic. Session evidence ( {"type":"error","error":{"type":"api_error","message":"An error occurred while processing the request."}}Every failing turn follows an assistant message carrying signed
Verification: |
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Verdict: REQUEST_CHANGES
The repair itself is sound and verified, but the PR branch currently shows a stale/DIRTY merge state against origin/dev with no real textual conflict. The author should update/rebase the branch; once GitHub's merge state is clean this is merge-ready.
Red-team review
Scope
- Only
packages/ai/src/providers/anthropic.ts, two test files,packages/ai/CHANGELOG.md, and two credential-free artifact probes are touched. - No new product features; only the existing Anthropic thinking-replay repair ladder is extended to two new proxy shapes.
False-positive / misclassification
- The new
isAnthropicInvalidRequestStatushelper still keeps the strictinvalid_request_error+ thinking-wording checks in the two exported classifiers, so unrelated statusless 400s do not claim the repair. - The masked proxy path (
isAnthropicMaskedProxyRejection) explicitly returnsfalseif the payload still namesinvalid_request_error, so forwarded bodies stay on the strict matchers. - It is gated by
firstTokenTime === undefinedand byhasNativeThinkingBlocks(params.messages), so a maskedapi_erroron a request with no replayed signed thinking surfaces immediately (verified by the new test). - Residual concern: a statusless
api_errorthat is actually a masked 5xx cannot be distinguished from a masked thinking 400. The code accepts this trade-off and only performs one latest→all repair before surfacing the error, so it does not loop. Worth monitoring in production telemetry.
Infinite repair loops / budget exhaustion
- The repair flags (
repairLatestAssistantThinking,repairAllAssistantThinking) are cumulative and guarded by!repairAllAssistantThinking, so the thinking-repair branch can fire at most twice: latest-only, then all-history. providerRetryAttemptis reset to 0 after each repair rebuild, which is consistent with the existing fallback behavior; the repaired request then uses the normal provider retry budget, so there is no new loop source.
History mutation / ladder correctness
- The escalation rule
escalateToAll = thinkingSignatureInvalid || repairLatestAssistantThinkingis preserved and correctly covers the masked case: if the latest-only repair still fails, the next rebuild drops every replayed assistant thinking block. - The new integration test exercises exactly this ladder for the masked shape (3 requests: full → latest-only → all-history).
Regression vs #3465 / #3699 contracts
- The direct HTTP 400 path is unchanged: the same message regexes and
invalid_request_errorcheck are used. - Only the status pre-filter is broadened from
=== 400to400 || undefined; the wording filters are unchanged, so the #3465 repair contract still holds. - The statusless SSE handling mirrors the #3699 Codex statusless SSE pattern.
Test gaps / artifact-only claims
- Unit tests cover statusless
invalid_request_errorclassification for both mutation and signature variants, and the masked genericapi_erroris correctly rejected by the strict matchers. - Integration tests cover the statusless SSE repair, the masked repair, the full-history escalation, and the no-thinking-block guard.
- The live CPA path is proven only by the credential-free
artifacts/issue-3900-sse-proxy-sim.tswire simulation, which exercises the realstreamAnthropic+ Anthropic SDK transport against a local proxy. This is acceptable because it reproduces the exact CPA delivery mechanics without exposing credentials. - Minor gap:
isAnthropicMaskedProxyRejectionis not exported, so there is no direct unit test for the regex matching itself; the integration tests cover it, but exporting a thin classifier test would make the contract more durable. Not a blocker.
Verification (run in local worktree)
After building the missing local native addon (bun --cwd=packages/natives run build) because the worktree had no prebuilt .node files:
bun test packages/ai/test/anthropic-thinking-repair-retry.test.ts packages/ai/test/anthropic-thinking-immutability.test.ts
→ 27 pass, 0 fail
bun --cwd=packages/ai run check
→ biome clean, tsc clean
bun artifacts/issue-3900-sse-proxy-sim.ts
→ {"requests":2,"firstRequestHadSignedThinking":true,"repairedRequestDroppedThinking":true,"stopReason":"stop","text":["recovered"]}
bun artifacts/issue-3900-sse-proxy-sim.ts masked
→ {"requests":2,"firstRequestHadSignedThinking":true,"repairedRequestDroppedThinking":true,"stopReason":"stop","text":["recovered"]}
Merge state
- GitHub reports
mergeStateStatus: DIRTY,mergeable: CONFLICTING. - Local
git merge-tree $(git merge-base HEAD origin/dev) origin/dev HEADauto-mergespackages/ai/CHANGELOG.mdandpackages/ai/src/providers/anthropic.tswith no textual conflict markers. - The merge base is
4bc67a8dwhileorigin/devhas advanced to8893c34; the conflict is stale base, not content. The author should update the branch against currentdev(rebase orUpdate branch). No manual conflict resolution is required.
Once the merge state is clean, this is safe to merge. Do not merge main; base stays dev.
— [repo owner's gaebal-gajae (clawdbot) 🦞]
Review on Yeachan-Heo#3915: the masked classifier was module-private, so its payload contract was only implied by the integration tests. It carries no thinking evidence of its own -- the caller pairs it with `hasNativeThinkingBlocks` -- which makes "which payloads does it claim" the entire contract worth pinning. Exported it and covered the statusless body, the passthrough 400 form, a forwarded `invalid_request_error` body (left to the strict matchers), non-400 statuses, other statusless `api_error` payloads, and non-Error inputs. Lore-id: c4a7e912 Confidence: high Scope-risk: narrow Reversibility: easy Tested: bun test packages/ai/test/anthropic-thinking-immutability.test.ts packages/ai/test/anthropic-thinking-repair-retry.test.ts -- 32 pass
|
Both review points addressed. Merge state. The branch is current with Minor gap — untested classifier. Fair, and sharper than "not a blocker":
On the residual concern (masked 5xx indistinguishable from masked thinking 400): agreed, and it is the reason the gate is |
Fixes #3900.
Root cause
isAnthropicThinkingBlockMutationError/isAnthropicThinkingSignatureInvalidErrorgate onextractHttpStatusFromError(error) === 400. Behind CLIProxyAPI the upstream 400 body arrives as an in-stream SSEerrorevent on an HTTP 200 response;iterateAnthropicEventsthrowsnew Error(sse.data)with no status attached, so both classifiers returnedfalse, the one-shot thinking-replay repair never ran, and every user retry replayed the same rejected history — the unrecoverable loop captured in the CPA traces (same mechanism as the Codex statusless SSE rejection fixed in #3699).Fix
The classifiers now accept both the direct HTTP 400 and the statusless SSE shape via a shared
isAnthropicInvalidRequestStatushelper. The strictinvalid_request_error+ thinking-wording message checks are unchanged, so unrelated transport failures (including CPA's masked genericapi_errorbody) never claim the repair retry. A 500-status error is still rejected.Verification
bun test packages/ai/test/anthropic-thinking-repair-retry.test.ts packages/ai/test/anthropic-thinking-immutability.test.ts— 24 pass, including new statusless SSE classification tests and a statusless repair-retry flow test.bun --cwd=packages/ai run check— clean.artifacts/issue-3900-sse-proxy-sim.ts— credential-free wire-level simulation: local server answers HTTP 200 +event: errorwith the exact captured 400 body through the real Anthropic SDK transport. Pre-fix: 1 request, turn dies with the captured message. Post-fix: repair drops the replayed thinking, second request recovers (stopReason: "stop").artifacts/issue-3900-live-cpa-probe.ts— live probe through a CPA endpoint taken from~/.gjc/agent/models.ymlas fallback credentials; tool-use turns with tampered signed thinking currently pass upstream (CPA normalizes history), so the deterministic proof of the bug path is the wire simulation above.Notes
api_error/ "An error occurred while processing the request."), there is no attributable signal to trigger a repair; that variant still surfaces as a bounded per-turn error and is out of scope here.