Skip to content

fix(auth): harden failover recovery - #175

Merged
kaitranntt merged 101 commits into
kaitranntt:mainfrom
warelik:devin/1786486516-auth-failover-recovery
Aug 19, 2026
Merged

fix(auth): harden failover recovery#175
kaitranntt merged 101 commits into
kaitranntt:mainfrom
warelik:devin/1786486516-auth-failover-recovery

Conversation

@warelik

@warelik warelik commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Hardens the auth failover and recovery pipeline in CPAPlus — the mirror of CPA #4881, adapted for Plus-specific routing paths and executor topology.

When a provider returns an empty completion (HTTP 200, finish_reason: stop, zero content/tool-calls/tokens — observed on AI Studio and Antigravity paths), today the response passes through unchanged: no retry, no cooldown, session affinity stays pinned to the failing auth. Agent harnesses die with "empty response, finishReason=stop".

This PR treats empty terminal completions as retriable failures and fixes a chain of related issues exposed during live failover testing.

Key changes (24 commits, 47 files)

Empty-completion detection and retry

  • empty_completion.go (new): conservative predicates for OpenAI SSE streams and non-stream JSON — empty iff terminal AND zero non-whitespace content AND zero tool calls AND zero completion tokens. reasoning_content counts as content; unrecognized formats → no behavior change.
  • conductor_stream.go: when bootstrap read closes with an empty aggregate (before any byte reaches the client), synthesize retriable errEmptyCompletion, MarkResult(failure), continue model/auth loop.
  • conductor_execution.go, conductor_home.go: same judgment on non-stream and home paths.
  • SSE frame parsing (5420c121-equivalent): complete newline-less data: frames are evaluated immediately; truncated JSON and partial [DONE] remain buffered.
  • Split-stream classification: detect empty completions even when SSE frames arrive split across chunks.

Auth selection and session affinity

  • Concurrent affinity CAS: selector.go uses generation-token compare-and-swap to prevent ABA rebind races between concurrent requests for the same session.
  • Alias-group preservation: when a cached auth becomes unavailable, the selector now rebinds the full alias group instead of only the primary name. CompareAndReplaceGroup CAS prevents split-brain.
  • Quarantine propagation: failed affinity targets are quarantined across all aliases, not just the directly-failed one.

Stream and error handling

  • TTFT timer: stops on upstream activity; restarts fresh on refresh retry.
  • Route exhaustion headers: Retry-After and x-request-id survive the route-summary wrapper via Headers() and SafeResponseHeaders().
  • Sealed stream errors: openAIStreamSanitizedError no longer exposes Unwrap() — raw upstream payloads stay sealed. SafeResponseHeaders() returns a cloned header copy at wrap time.
  • Trusted direct responses: TrustedDirectResponse dual-flag bypass preserves passthrough headers.
  • WebSocket terminal errors: rebuilt to match format expectations; recoverable errors covered by regression tests.
  • Surface errors before DONE: check pending stream errors before sending the terminal [DONE] frame.

Upstream credential handling

  • 401/403 classification: upstream auth errors treated as credential faults, quota/balance errors preserved separately.
  • Gemini tool-call ID stabilization: prevents translator-generated ID mismatches.

Provider-specific coverage

  • OpenAI Chat Completions and Responses API
  • Claude/Anthropic translated and native payloads
  • Gemini safety stops, tool calls, code execution, and reasoning
  • Home and plugin executors
  • Antigravity and Plus-specific routing paths

Test plan

  • go test -count=1 -timeout 300s ./... — all packages pass
  • go test -race -count=1 -timeout 300s ./sdk/cliproxy/auth — race-free
  • go build ./... — clean
  • 19 new/modified test files covering: empty-completion detection (11+ format cases), session affinity concurrent binding, route exhaustion header forwarding, TTFT timer lifecycle, WebSocket error sanitization, trusted direct response passthrough, Responses discriminator, stream peek, home concurrency busy headers
  • Focused affinity/TTFT tests repeated 20×
  • Live CPAPlus tool-call smoke test: HTTP 200 in 1.236s after restart

Mirror

CPA: router-for-me/CLIProxyAPI#4881 (equivalent changes adapted from the shared codebase)

Related issues

  • #4988 — frame separator aggregation (follow-up)
  • #4989 — affinity namespace metadata propagation (follow-up)
  • #4990 — event chunk parsing (follow-up)

W ARELIK added 2 commits August 12, 2026 05:44
Quarantine failed affinity targets across requests and bound stalled streams by time to first meaningful chunk. Preserve protocol-specific terminal failures while retrying truly empty completions.
Pre-bind cache-miss selections so concurrent requests for one session stay sticky. Keep stream first-chunk failover explicit and disabled by default.
@warelik

warelik commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

W ARELIK added 7 commits August 12, 2026 09:50
Generate deterministic IDs for missing Gemini tool calls and match
function responses by name and FIFO order.

Preserve explicit IDs and stable fallback IDs for unmatched responses.
Keep failed credentials excluded across outer attempts so dead keys are
not retried. Carry affinity metadata through copied request options and
append bounded, redacted route summaries to terminal errors.
Aggregate split and newline-less SSE and JSON frames through EOF before
judging an upstream response empty.
Stop the first-chunk timer on the first successful upstream chunk,
including metadata-only prefixes, while preserving bootstrap judgment.
Do not retry after semantic stream commit.
@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Current-head evidence — follow-up commits & validation (head 32723082)

Verified against the current PR head 32723082cbfacacb6804daf2a02bb446b9e359fa (local HEAD, upstream tracking ref, and GitHub PR head.sha all match).

Follow-up commits (newest first)

Commit Scope
32723082 fix(stream): stop TTFT on upstream activity sdk/cliproxy/auth/conductor_stream.go, stream_ttft_test.go
3744665d fix(stream): classify split empty completions sdk/cliproxy/auth/empty_completion.go, empty_completion_export.go, empty_completion_test.go
28668d01 fix(auth): preserve failover state across retries sdk/cliproxy/auth/conductor_execution.go, conductor_selection.go, selector.go, route_tracker.go + 8 test files (12 files, +1215/−52)
65a7364e fix(translator): stabilize Gemini tool call IDs internal/translator/openai/gemini/openai_gemini_request.go (+53), openai_gemini_request_test.go (+303)
  • 65a7364 — Gemini→OpenAI translation now derives deterministic tool-call IDs (sha256("call|msgIdx|partIdx|funcName|payload")[:24], prefixed call_) for unmatched functionCall parts and pairs functionResponse tool messages to those IDs, with stable standalone fallback IDs for unmatched responses. Preserves explicit IDs when present. Regression: TestConvertGeminiRequestToOpenAI_Deterministic100Invocations runs the translation 100 times and asserts byte-identical output.
  • 28668d0 — failover state now survives the outer retry loop: excluded/tried auth IDs are hoisted out of opts.Metadata per attempt (pickOpts copy sites in executeMixedOnce/CountMixedOnce/StreamMixedOnce), so a dead credential is never re-invoked across outer retries; mixed-pool affinity namespace ("mixed" provider + requested model) stamped during selection survives the metadata copy into success Result.Options; route exhaustion preserves typed errors. New regression coverage: outer_retry_exclusions_test.go (255 lines), route_exhaustion_test.go (474 lines), selected_auth_metadata_test.go (153 lines).
  • 3744665 — empty-completion judgment now aggregates split / newline-less SSE and JSON frames through EOF (streamBootstrapState.finish() + StreamBootstrapDetector) before declaring an upstream response empty, instead of judging per-fragment buffers.
  • 3272308 — the first-chunk (TTFT) timer stops on the first successful upstream chunk, including metadata-only prefixes (e.g. : keepalive), while bootstrap judgment is preserved; post-commit failures are not retried.

Correctness semantics (regression-encoded)

  • Dead credentials not re-invoked across outer retriesTestOuterRetryExclusions_NonStream_DisableCooling_InvokedOnce / ..._Stream_..._InvokedOnce use a real retry window (SetRetryConfig(3, 100ms, 3) + 429/Retry-After 5ms) and assert exactly one executor invocation across the whole outer retry span (pre-fix: 4).
  • Mixed affinity cache namespace survives copied metadataTestManagerExecute_MixedAffinityNamespaceRetainedThroughExclusions, TestManagerSelection_NilMetadataPreservesAffinityNamespace, TestPublishSelectedAuthMetadataIncludesStableIndex prove the stamped "mixed" provider+model namespace survives value-copy and a subsequent same-session request binds to the same auth.
  • Typed errors preserved — route exhaustion terminates with typed final errors after bounded candidate traversal (route_exhaustion_test.go, errors.go).
  • Metadata-only first chunk stops TTFT; error-only does notTestManagerExecuteStream_MetadataFirstPrefixStopsTTFTWithoutFailover asserts : keepalive is buffered first, TTFT is stopped, no failover to auth-b, auth-a stays available; readStreamBootstrap invokes the stop callback only on payload chunks (an error chunk returns before the callback).
  • Post-commit errors delivered once without retryTestManagerExecuteStream_PostCommitErrorNotRetried asserts one terminal StreamChunk.Err, exactly one content chunk (no replay), and exactly one executor call.

Fresh validation (run for this comment)

  • TTFT regressions: go test -count=100 ./sdk/cliproxy/auth/ -run 'TestManagerExecuteStream_(TTFTTimeoutFailsOverToNextAuth|PostFirstChunkDelayNotCutOffByTTFT)$'ok (17.8s)
  • Full suite: go test -count=1 ./...exit 0 (all packages ok)
  • Race: go test -race -count=1 ./internal/translator/openai/gemini/ok; go test -race -count=1 ./sdk/cliproxy/auth/ (TTFT/outer-retry/affinity selection) → ok
  • go build ./...exit 0
  • git diff --checkexit 0
  • Current-head GitHub CI: build ✅ (pass, run 31653416694) and close-when-agents-md-changed ✅ (pass, run 31653415250) on head 32723082.

Independent local reviews

  • 9 independent local review passes completed across projects (CPA, CPAPlus, Studio — caveman-review/ponytail-review/jbcontext-review × each project, each pass additionally using jbcontext); all real findings accepted and fixed, with regression coverage added.
  • CPAPlus final clean re-review found no actionable findings on the current diff.

Maintainer review requested

The Codex connector on this PR currently returns only a setup notice ("create a Codex account and connect to github") and produced no review. A maintainer review of the current head is invited.

No claim of SINGULARITY is made here; this comment reports current-head evidence only.

W ARELIK added 4 commits August 13, 2026 11:40
When route exhaustion fired, the returned routeExhaustionError wrapped only the cause's message, so any upstream non-*Error cause that carried passthrough metadata (Retry-After, X-Request-Id) lost those headers — handlers that collect headers from the final routed error got nothing.

routeExhaustionError now exposes Headers() that resolves the wrapped cause via errors.As and returns a defensive clone via cloneHTTPHeader, so the caller's header map is never mutated. errors.As starts at the cause and stops at the first matching carrier, preserving outermost-wins disambiguation for nested carriers; nil receiver returns nil.
ttftScope now owns a per-attempt timeout() that returns a typed TTFT timeout error when the deadline wins, so the shared refresh path re-arms TTFT for the stale-token retry instead of inheriting an expired budget. A timed-out refresh no longer leaves the failover consumer waiting on a dead timer (the Execute/CountTokens/HttpRequest stubs keep the executor contract), and the retried attempt starts with a clean timer so a post-refresh stream is not truncated.
routeExhaustionClonedError no longer clones the cause Error; it wraps the cause and appends the sanitized route-exhaustion summary to Error() without mutating the cause stored Message. It gains SafeResponseHeaders(), forwarding the wrapped cause trusted response headers (e.g. the Home busy error Retry-After) through route exhaustion so handlers collecting passthrough headers do not lose them. Fresh copy never mutates caller headers.
…e quota/balance

IsRequestFault now treats structured auth markers (authentication_error type, invalid/incorrect/expired_api_key codes, Gemini UNAUTHENTICATED status) as credential faults even when paired with a generic invalid_request_error code, so failover rotates the credential instead of misclassifying as a request fault. Quota/payment surfaces carrying a generic invalid_request_error code or type stay in quota/balance scope. The shared mixed-auth loop pins this contract via conductor_overrides_test.go.
@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Current-head evidence — three follow-up commits & validation (head 286649ae)

Verified against the current PR head 286649ae2055ad228bd360630ca17ef55f257541 (local HEAD, upstream tracking ref, and GitHub PR head.sha all match). This replaces the earlier evidence comment that pointed at the now-superseded head 32723082.

Commits on top of the previously-evidenced head 32723082 (newest first)

Commit Scope Behavior
286649ae fix(auth): classify upstream 401/403 as credential faults and preserve quota/balance internal/clienterror/client_error.go, internal/clienterror/client_error_test.go, sdk/cliproxy/auth/conductor_overrides_test.go IsRequestFault treats structured auth markers (authentication_error type, invalid/incorrect/expired_api_key codes, Gemini UNAUTHENTICATED status) as credential faults even when paired with a generic invalid_request_error code, so failover rotates the credential instead of misclassifying as a request fault. Quota/payment surfaces carrying a generic invalid_request_error code or type stay in quota/balance scope.
e84dc793 fix(auth): forward safe response headers through route exhaustion sdk/cliproxy/auth/route_tracker.go, sdk/cliproxy/auth/route_exhaustion_test.go routeExhaustionClonedError stops cloning the cause Error; it now wraps the cause and appends the sanitized route-exhaustion summary to Error(), and gains SafeResponseHeaders() forwarding the wrapped cause's trusted response headers (e.g. Home busy Retry-After) through route exhaustion. Fresh copy never mutates caller headers.
5548eaa0 fix(stream): restart a fresh TTFT timer and path on refresh retry sdk/cliproxy/auth/conductor_stream.go, sdk/cliproxy/auth/stream_ttft_test.go ttftScope now owns a per-attempt timeout() returning a typed TTFT timeout error when the deadline wins, so the shared refresh path re-arms TTFT for the stale-token retry instead of inheriting an expired budget and leaving the failover consumer waiting on a dead timer.

Fresh validation (run for this comment, local working tree clean)

  • go test -count=1 ./internal/clienterror/ok (0.356s)
  • go test -count=1 ./sdk/cliproxy/auth/ -run 'TestStream|TestRouteExhaustion|TestConductorOverrides|TestTTFT|TestOuterRetry'ok (1.569s)
  • go test -race -count=1 ./internal/clienterror/ ./sdk/cliproxy/auth/ -run 'TestStream|TestRouteExhaustion|TestConductorOverrides|TestIsRequestFault|TestClientError'ok (clienterror 1.387s, auth 3.421s)
  • go build ./...exit 0
  • go vet ./internal/clienterror/ ./sdk/cliproxy/auth/exit 0
  • go test -count=1 ./... (full suite) → all packages ok, no failures/panics
  • git diff --check main...HEADexit 0
  • git status --short: no tracked-file modifications (untracked build/cache dirs only)

Environment note

This sandbox cannot write Go build dirs to /tmp (mkdir /tmp/go-build...: operation not permitted), so the run pinned TMPDIR, GOTMPDIR, and GOCACHE to repo-local dirs (.go-tmp, .go-cache) for the commands above. All exits above are with those pinned; /tmp was only redirected, never written.

PR checks / mergeability (GitHub, at evidence time)

  • build ✅ SUCCESS (run 31712308339, job 94488132909)
  • close-when-agents-md-changed ✅ SUCCESS (run 31712304856, job 94488120797)
  • mergeable: MERGEABLE, mergeStateStatus: CLEAN

No claim of SINGULARITY is made here; this comment reports current-head evidence only.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Soliciting a Codex review of the current head 286649ae2055ad228bd360630ca17ef55f257541 (the three commits documented in the current-head evidence comment above). The prior @codex review was issued against the now-superseded head 32723082 and the connector returned only a setup notice; this is a fresh request for the current head.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Current-head evidence — delete-only ponytail refactor (head 53be13ad)

Two delete-only findings implemented and fully verified.

1. conductor_stream.go: reuse release()

  • Deleted package-private ttftScope.stopTimerAndRelease() — body was byte-identical to release() (same lock, committed flag, timer stop, cancel+nil).
  • Replaced all 13 call sites with scope.release(). Single release() implementation remains (commit(), fire(), wrap cleanup untouched). conductor_execution.go unchanged.

2. selector.go: reuse extractExcludedAuthIDs

  • Deleted package-private excludedAuthIDsFromOptions.
  • Switched all 4 Pick sites (RoundRobin, WeightedRoundRobin, FillFirst, SessionAffinity) to existing helper extractExcludedAuthIDs(opts.Metadata).
  • Deleted the unreachable map[string]bool conversion branch. Current producers are map[string]struct{} or []string; nil/empty semantics preserved (all consumers use len/map-read). One extractExcludedAuthIDs implementation remains (in conductor_execution.go, unchanged).

Verification

  • git diff --check clean; gofmt clean.
  • go build ./... rc=0; go vet ./sdk/cliproxy/auth/ rc=0.
  • TTFT focused tests (normal + -race): timeout failover, post-first-chunk not cut off, refresh-retry-fresh-TTFT-timer — all pass.
  • Full auth package normal rc=0 and -race rc=0. Selector / session-affinity / outer-retry tests pass.
  • Notes: go test ./... surfaces only internal/util failing a static no-copy invariant because the untracked in-tree module caches (.go-cache/gopath, .tmp_build/gopath) are scanned by filepath.WalkDir; proven environmental by running internal/util green in a clean worktree (no in-tree caches) at HEAD, with and without this diff. No behavior change — no new test added.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Current-head evidence — surfacing stream errors before [DONE] (head 56985a0e)

Verified against current PR head 56985a0e22b76970ca470db2d1011c16e42bf37f (local HEAD, fork remote, and GitHub PR head.sha all match). This replaces the earlier evidence comments that pointed at the now-superseded heads 32723082 / 286649ae / 53be13ad.

Commit

56985a0e fix(openai): surface errors before stream DONEsdk/api/handlers/openai/openai_handlers.go, sdk/api/handlers/openai/openai_handlers_stream_peek_test.go (+599/−4).

Fixes the cross-project P2: every initial streaming peek loop must consume a buffered pending error on errChan before committing SSE headers and writing 200 data: [DONE].

  • Three close branches fixed (handleStreamingResponse, handleStreamingResponseViaResponses, handleCompletionsStreamingResponse): on dataChan close, the peek now consumes an immediately-available pending error via pendingOpenAIStreamError(errChan) (mirrors the existing pendingClaudeStreamError in the Claude handler); when pending, it writes the sanitized error and cancels instead of emitting [DONE].
  • Three direct errChan writes sanitized: the existing case errMsg := <-errChan paths now route through sanitizeOpenAIErrorMessage, parity with the shared ForwardStream terminal normalization. Because CPAPlus lacked both helpers, pendingOpenAIStreamError and the strict sanitizeOpenAIErrorMessage (status normalization, Body cleared, DirectResponse=false, credential/value redaction to [REDACTED]) were added in-package; the redaction engine mirrors CPA's proven redactResponsesStreamErrorText approach.

Regressions (deterministic actual-handler tests, not direct helper calls)

TestStreamingPeekConsumesBufferedPendingError — chat, ViaResponses, and legacy completions each drive the real handler/executor/auth plumbing through a registered fake streaming executor whose chunk channel closes after buffering a chunk error carrying a secret. Asserts: error status (never 200), no data: [DONE], secret not leaked, and [REDACTED] present.

TestStreamingPeekCleanCloseStillEmitsDone — control for all three paths: a valid content chunk then a clean close still emits 200 data: [DONE].

ViaResponses is exercised by registering the model with SupportedEndpoints: [openAIResponsesEndpoint] only, routing chat through the third streaming peek.

Fresh validation (run for this comment)

  • go test -count=1 -run 'TestStreamingPeek' ./sdk/api/handlers/openai/ok (all 6 subtests pass)
  • go test -race -count=1 -run 'TestStreamingPeek' ./sdk/api/handlers/openai/ok
  • go test -race -count=3 -run 'TestStreamingPeek' ./sdk/api/handlers/openai/ok
  • go test -count=1 ./sdk/api/handlers/openai/ok
  • go test ./sdk/api/handlers/ok; go test -race ./sdk/api/handlers/ok
  • go build ./...exit 0
  • go vet ./sdk/api/handlers/openai/exit 0; gofmt -l → clean
  • go test -race ./sdk/api/handlers/openai/ full → only pre-existing races in TestPrepareCodexMultiAgentV2ToolsAtResponsesBoundary* / TestResponsesPreparesCodexMultiAgentV2ToolsForHTTPAndSSE, reproduced identically at base 53be13ad without this diff (unrelated multi-agent tool-prep, not introduced by this change).

Environment / hygiene

  • This sandbox cannot write Go build dirs to /tmp, so runs pinned TMPDIR/GOTMPDIR/GOCACHE to repo-local untracked dirs (.go-tmp-cpa, .go-cache-cpa2); /tmp was redirected, never written.
  • Only the two files above are in the commit; no config/auth/secrets/cache contents/request identifiers touched. Tracked working tree clean (untracked build/cache/binaries only).

No claim of SINGULARITY is made here; this comment reports current-head evidence only.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Soliciting a Codex review of the current head 56985a0e22b76970ca470db2d1011c16e42bf37f (the fix documented in the current-head evidence comment above). The prior @codex review requests were issued against now-superseded heads (32723082, 286649ae, 53be13ad) and the connector returned only a setup notice; this is a fresh request for the current head.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@warelik

warelik commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Current-head evidence — full OpenAI sink parity (head 6bc6cd62)

Verified against the current PR head 6bc6cd62ed0de47726f181fb7c53e05e4c7ffdce.

Scope

Single commit fix(openai): sanitize all terminal errors, touching only the 6 owned OpenAI files. No protected files, configs, auth, caches, .bak, or producer/forwarder code touched. No runtime/service action.

Product changes

  • whole package: every reachable client-facing error sink now routes through the existing sanitizeOpenAIErrorMessage (redact secrets, normalize status, clear Body, DirectResponse=false). No new sanitizer; shared helpers reused.
  • Responses handlers: nonstream sinks (Compact, nonstream, via-chat), direct-errChan sinks, writeResponsesTerminalError sanitize before building client chunk; both native and via-chat initial peek dataChan-close paths now call pendingOpenAIStreamError before committing headers.
  • Images handlers: all nonstream/direct sinks, writeImagesStreamErrorEvent sanitize, ForwardStream WriteTerminalError sanitize, and all three peek-close families (streamRoutedImages, streamOpenAICompatImages, streamImagesFromResponses) check pending error before clean-close.
  • Videos handlers: all 10 client-facing sinks sanitized.
  • Websocket forward: buildResponsesWebsocketErrorPayload sanitizes the error text before constructing the client error JSON, preserving RFC/error-code framing; fault-classification gate stays on the raw message.

Sink inventory (post-edit)

grep across sdk/api/handlers/openai:

  • WriteErrorResponse(c, errMsg) raw: 0 remaining; all WriteErrorResponse now wrap sanitizeOpenAIErrorMessage.
  • pendingOpenAIStreamError(errChan) close-path guards: 8 (chat, chat-via-responses, completions, responses-native, responses-via-chat, routed/compat/responses images).
  • Remaining .Error.Error() reads are post-sanitize (terminal emitters) or internal classification (shouldReleaseResponsesWebsocketPinnedAuth); none feed an unsanitized client payload.

Tests added (in openai_handlers_stream_peek_test.go)

  • Sanitizer security suite: status normalization (success/out-of-range->500, 2xx/600->500, valid preserved), Body=nil + DirectResponse=false, nested-JSON + route-summary redaction, key-spelling/scheme/quote/escape/assignment forms (api_key, client_secret, api_token, refresh_token, *_secret, dash/underscore, OpenAI/Anthropic/GitHub/Stripe keys, authorization, Bearer/Basic schemes), long-value/no-panic bounds, benign deny-list (not_api_key, token_count, tokenizer, secretariat, mytoken, prose). ~45 cases.
  • Real-handler regressions: Responses native + via-chat peek, three images peek families, images terminal SSE event, websocket terminal-error payload — all assert sanitized non-200 error, secret absent, redaction marker present.

Gates

  • go build ./...: pass
  • go vet ./sdk/api/handlers/openai/: pass
  • go test ./sdk/api/handlers/openai/ (full, fresh): pass
  • go test ./sdk/api/handlers/: pass
  • New tests -race -count=3: pass
  • Full-package -race: the 3 TestPrepareCodexMultiAgentV2Tools* races are pre-existing on base HEAD (reproduced with changes stashed); unrelated to this change and pre-published in test(openai): set Gin mode once per package #179 evidence. New tests themselves race-green.

Commit

6bc6cd62ed0de47726f181fb7c53e05e4c7ffdcefix(openai): sanitize all terminal errors (6 files, +511/-45). Pushed to fork; PR head matches. Checks: build and close-when-agents-md-changed pending.

W ARELIK added 15 commits August 16, 2026 02:29
Port of CPA: bootstrap.hasMeaningfulOutput() gates error-chunk
forwarding; scaffold-only buffer + upstream error fails over.
…g emptiness

A no-argument MCP call such as {"type":"mcp_tool_use","id":"tool_1","name":"lookup","input":{}}
with stop_reason:"tool_use" was misclassified as an empty completion because only tool_use and
server_tool_use received semantic validation, cooling the credential and discarding a valid tool
call. Treat mcp_tool_use with non-empty id and name (or a non-empty input payload) as meaningful,
mirroring how claude_input_tokens.go already handles the block type. Red-proof tests cover
non-stream and SSE variants plus missing-id/missing-name controls.
…xclusions

resetRecoveredExclusions only honored DisableCoolingOverride, so when cooling was disabled
through manager runtime config (globally or per OpenAI-compatible provider), MarkResult left
the failed credential available while the reset dropped its request-scoped exclusion - letting
an outer retry with a supplied delay re-select and hammer the same credential. Use the same
manager-aware predicate (cooldownDisabledForAuth) in the reset path. Red-proof: global and
provider-compat config-disabled cases failed pre-fix; control with cooling enabled still resets.
…sponses args, mixed JSON

- Claude signature_delta frames now deserialize and validate delta.signature, so a
  signature-only thinking block followed by message_stop is meaningful instead of discarded.
- Responses function_call_arguments events only set hasToolCalls when they carry non-empty
  delta/arguments or a call item was already validated, keeping scaffold-only frames retryable.
- Multi-value JSON payloads mark the aggregate unknown/pass-through when any decoded value
  matches no supported shape, so unknown provider-specific output is never discarded as empty.
Red-proof tests per case; race suite x10 green.
…thought signatures

- conductor_stream: when a custom executor returns a StreamResult alongside an immediate
  unauthorized error, the refresh retry now drains the original chunk channel before
  replacing it, so a still-sending producer cannot block indefinitely (mirrors the other
  result-discard paths).
- empty_completion: Gemini responses whose only payload is a non-empty thoughtSignature
  (both thoughtSignature and thought_signature spellings) on an otherwise empty part with
  zero/omitted candidatesTokenCount now count as meaningful output, preserving provider
  state for request round-tripping instead of cooling the credential.
Red-proof tests for both; race suite x10 green.
…soning items in bootstrap

- shouldForward now includes positive completion usage (sawUsage && completionTokens > 0),
  so a terminal frame with empty choices and positive usage is delivered immediately instead
  of hanging the client when the upstream holds the stream open.
- Responses reasoning output items require actual payload (non-empty encrypted_content or
  summary) before counting as content; the empty in_progress reasoning scaffold emitted by
  our own translators no longer commits the bootstrap, preserving failover on early errors.
Red-proof tests for both; full suites and race x10 green in CPA and CPAPlus.
…stream errors

The standalone-credential pattern required the token to contain a digit, uppercase letter,
or punctuation, so a lowercase-only value such as "Bearer abcdef" survived redaction in
downstream errors and API error logs. Match the full RFC 6750 b64token charset
([-A-Za-z0-9._~+/=]{3,}) without requiring any particular character class. Red-proof:
lowercase-only and mixed tokens redacted; plain prose ("bearer of bad news") untouched.
Race x10 green.
…short credential masking

- empty_completion: Claude partial_json fragments are parsed semantically - whitespace-only
  objects/arrays/null ("{ }", "[]", "null ") without usable arguments no longer set
  hasToolCalls, so a terminal stop_reason:"tool_use" after them still triggers failover.
- route_tracker: routeExhaustionClonedError.Error() returns the original structured error
  body verbatim when it is valid JSON, keeping clienterror.IsRequestFault classification
  intact so the Responses WebSocket path sends the actionable client error instead of
  silently closing; summaries still append to plain-text causes.
- openai handlers: the standalone auth-scheme pattern drops the {3,} minimum (RFC 6750
  allows a single character) and adds a prose deny-list ("bearer of bad news"), masking
  short credentials without touching natural language.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
@warelik
warelik force-pushed the devin/1786486516-auth-failover-recovery branch from 3b821a4 to 2f7ab80 Compare August 16, 2026 01:56
W ARELIK added 13 commits August 16, 2026 05:25
- openai handlers: masking no longer exempts tokens by content (deny-list removed); instead
  a lowercase scheme word followed by space and lowercase prose words is treated as natural
  language, while standalone credentials of any length ("Bearer of", "Bearer ab") are masked.
- conductor_stream / empty_completion: an observed terminal [DONE] marker on a stream that
  delivered only scaffolding now reports the empty completion immediately, so the conductor
  rotates credentials instead of waiting forever on an open channel.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
… scaffolds

- openai handlers: prose/context exemption removed - any scheme-anchored token-shaped match
  (bearer|basic + RFC 6750 charset, any length) is masked unconditionally, so credentials
  followed by prose words ("bearer abc expired") can no longer leak. Collateral prose
  masking is the accepted trade-off per reviewer.
- empty_completion: Responses function_call/custom_tool_call output items only set
  hasToolCalls when they carry usable identity, name, or input, so the empty scaffolds
  emitted by our own translators stay in bootstrap and preserve failover.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
…done call items

- conductor_stream + config: stream-first-chunk-timeout-seconds promised first-chunk
  enforcement but the timer stopped once headers arrived, letting a headered-but-bodiless
  stream hang forever; repository policy (AGENTS.md L58) forbids extending timeouts past
  an established connection. The option is rescoped as stream-connect-timeout-seconds
  (stream_connect_timeout_ms), with the legacy keys kept as deprecated aliases.
- empty_completion: response.output_item.done now routes through
  hasMeaningfulResponsesCallItem, so empty function_call/custom_tool_call done items stay
  in bootstrap and preserve failover, matching the added-branch behavior.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
…id-key errors

- empty_completion: OpenAI tool/function call argument strings are parsed semantically, so
  "{}" / "[]" / "null" without usable call identity no longer bypass empty-completion
  failover (hasMeaningfulJSONArguments shared with the Responses call-item validator).
- conductor_cooldown: isCredentialScopedError now classifies Gemini 400 "API key not valid"
  as credential-scoped, so multi-model credentials exit the model pool after the first
  rejection instead of re-issuing the request with the same dead key. CPAPlus also gains the
  CredentialScope result wiring in its conductor loops (parity with CPA).
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
…ircuit, semantic arg events

- codex_websockets_stream: isTerminalEvent now includes response.incomplete and
  response.failed, so a terminal payload ends the request (result channel closed, reqMu
  released) instead of hanging the execution session while reading the persistent socket.
- empty_completion: Claude message_stop marks the whole-stream terminal state (like [DONE]),
  so an empty Claude stream over an open channel is classified empty and rotates instead of
  waiting indefinitely.
- empty_completion: Responses argument events ("{}", "[]", "null") route through
  hasMeaningfulJSONArguments unless a usable call was already established.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
…nd Gemini STOP

- pluginhost executor_route: the direct-plugin bootstrap loop now checks
  detector.IsTerminalEmpty() and emits the empty-completion error immediately when a stream
  delivers only a terminal marker ([DONE] / message_stop) over an open channel, instead of
  waiting indefinitely (AGENTS.md L58).
- empty_completion: Gemini frames whose candidates all finish with STOP now mark the
  whole-stream terminal state, so an empty Gemini completion over an open channel is
  classified empty and fails over rather than hanging.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
A Claude-compatible upstream that emits data: {"type":"message_stop"} without a separate
event: line (a shape exercised by the repo claude executor fixtures) never set the
whole-stream terminal flag, so conductor and direct-plugin bootstrap loops waited
indefinitely on an open channel instead of failing over. chunk.Type == "message_stop" now
marks terminal just like the SSE event field. Red-proof: TestClaudeDataOnlyMessageStopTerminalEmpty
plus the pluginhost open-channel case; race x10 green.
…in plugin stream on terminal-empty

- conductor_cooldown: once a success proves a credential works again, every model suspended
  with the credential-wide invalid_api_key reason is resumed, not just the probed model -
  previously the other models stayed hidden from registry-backed model lists indefinitely.
- pluginhost executor_route: the terminal-empty early return now drains the source channel,
  so unbuffered adapter producers (mapExecutorStreamChunks, RPC wrappers) cannot block and
  RPC stream cleanup always runs.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
…stream

Both terminal-empty exits now drain the upstream chunk channel: the empty-completion
failover/return path and the closed=true substitution path, so unbuffered producers cannot
block after readStreamBootstrap stops early. Pre-publish review (5-skill gate) PASS.
Red-proof: TestConductor_ExecuteStreamDrainsSource* (single-model return and model-pool
failover); race x10 green in CPA and CPAPlus.
Tests for the drain fix: TestConductor_ExecuteStreamDrainsSource* (single-model return and
model-pool failover), verifying upstream producers never block when bootstrap exits early.
… for credential-wide suspensions

- empty_completion: non-empty choices[].delta.images / choices[].message.images (shapes
  emitted by the antigravity and codex translators) count as meaningful output, so
  image-only completions are delivered instead of discarded and cooled.
- conductor_cooldown + registry: sibling-model auto-resume after a successful probe now
  applies only to credential-wide suspensions (reason invalid_api_key, via new
  ModelRegistry.GetClientModelSuspensionReason); model-specific suspensions such as
  model_not_supported, quota, or 404 survive a sibling success, so registry-backed model
  lists never advertise an unroutable model.
Red-proof tests per case (image-only, sibling-suspension-survives, credential-wide resume);
full suites and race x10 green in CPA and CPAPlus. CI note: TestResponsesWebsocketReplays-
ImmediatelyAfterPinnedAuthFailure failure was a CI TCP-teardown flake (local 20/20 + race
x50 green in both repos).
…s as meaningful

- openai handlers: compound-key masking now covers camelCase suffixes (refreshToken,
  clientSecret, apiKey, accessToken, KeyId...) via a case-sensitive suffix branch, so
  upstream errors carrying such assignments no longer leak credentials.
- empty_completion: Claude citations_delta blocks with a non-empty citation object count
  as meaningful output (matching the translator that preserves them as annotations), so
  citations-only completions are delivered instead of discarded and cooled.
Red-proof tests per case; full suites and race x10 green in CPA and CPAPlus.
- empty_completion: openAIResponseOutputItem deserializes result; image_generation_call
  items whose only payload is a non-empty result count as meaningful output, so bootstrap
  no longer withholds a completed image while upstream lingers.
- openai_responses_websocket_forward: buildResponsesWebsocketErrorPayload copies the
  sanitized top-level error fields (type, code, message, param) into the rebuilt error
  object, so the supported flat error form keeps its actionable structure instead of
  dumping the whole document into error.message.
Red-proof tests per case (reversion-verified); race x10 green in CPA and CPAPlus.
@kaitranntt
kaitranntt merged commit d045ec1 into kaitranntt:main Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants