fix(signature): harden cross-provider signature boundaries - #209
fix(signature): harden cross-provider signature boundaries#209warelik wants to merge 181 commits into
Conversation
Add model_not_supported to resumableCooldownReasons so a model suspended with model_not_supported resumes when that same model succeeds. The failure sets a 12-hour temporary suspension whose registry counterpart would otherwise never clear even after the cooldown expires and the model serves requests successfully.
An OpenAI-compatible stream or Responses-API stream can emit a tool_calls or function_call delta carrying only an id / call_id without a function name or arguments. Previously, isMeaningfulToolCall and hasMeaningfulResponsesCallItem accepted these ID-only scaffolds as meaningful content (setting acc.hasToolCalls = true). In readStreamBootstrap (sdk/cliproxy/auth/conductor_stream.go), when an upstream chunk carries an error but bootstrap.hasMeaningfulOutput() is true, the error is suppressed, appended to the buffer, and readStreamBootstrap returns nil error. executeStreamWithModelPool then sees bootstrapErr == nil and assumes the stream started successfully, permanently disabling failover for that request and delivering an unusable partial tool call to the client. Fix the defect at sdk/cliproxy/auth/empty_completion.go:274-276 by removing the call.ID check in isMeaningfulToolCall, and at sdk/cliproxy/auth/empty_completion.go:776-783 by dropping the item.ID and item.CallID disjuncts in hasMeaningfulResponsesCallItem. This ensures that tool call deltas require a name, arguments, input, or result before being marked meaningful. This aligns with the Claude branch in the same file (sdk/cliproxy/auth/empty_completion.go:822), which already requires both ID and Name (strings.TrimSpace(b.ID) != "" && strings.TrimSpace(b.Name) != ""). Mirrors upstream fix on router-for-me/CLIProxyAPI PR #4881.
Responses-API tool calls (e.g. web_search_call, computer_call) carry payload in action rather than arguments/input. Include nonEmptyJSONPayload check for item.Action so valid completions are not discarded as empty.
…aude conversion Extract thoughtSignature / thought_signature and include signature in Claude thinking content blocks in ConvertGeminiResponseToClaudeNonStream. Classify parts with thoughtSignature as thinking to prevent reasoning text leakage and ensure parity with streaming converter. Fixes HTTP 400 (Invalid signature in thinking block) on multi-turn conversations with extended thinking. Refs router-for-me/CLIProxyAPI#5106
Add message.reasoning fallback to ConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream to match the fallback behavior in the streaming path. Refs router-for-me/CLIProxyAPI#5105
…chat completions OpenAI reasoning models send max_completion_tokens instead of max_tokens. Map max_completion_tokens to request.generationConfig.maxOutputTokens when max_tokens is absent, preserving priority. Refs: router-for-me/CLIProxyAPI#5108
Align non-streaming Claude-to-OpenAI Chat Completions translator with streaming path by writing message.reasoning_content instead of message.reasoning. Refs router-for-me/CLIProxyAPI#5104
…anslators Generate sequential deterministic tool and call IDs for Gemini requests without explicit IDs to prevent prompt cache misses across multi-turn conversations. Refs router-for-me/CLIProxyAPI#5107
Do not classify non-thought parts carrying thoughtSignature as thinking to prevent swallowing visible text. Bind thinkingSignature strictly to parts where thought: true is set so functionCall signatures do not overwrite thinking signatures. Guard flushThinking to avoid emitting phantom empty thinking blocks on functionCall carriers. Handle signature-only finish chunks in streaming path without opening empty text blocks. Add regression tests for carrier text parts, functionCall carriers, signature precedence, and streaming parity. Refs router-for-me/CLIProxyAPI#5106
Detect upstream HTTP 200 SSE/JSON error payloads (429, 503, 401, 403) during bootstrap before forwarding, allowing auth rotation instead of swallowing the error or forwarding broken streams. Refs router-for-me/CLIProxyAPI#4881
Align Gemini->Claude streaming signature handling with antigravity reference. Open thinking block for standalone signature when none is active or when previous block is signed.
…and tool calls Route thought signatures through carrier thinking blocks in streaming Gemini-to-Claude conversion when attached to visible text or tool calls.
Providers answering 429 for an exhausted daily quota can attach a RetryInfo hint far shorter than the real recovery window. Gemini and Antigravity were observed returning 479417207ns while the key stayed dead for the rest of the day. Both quota paths took that hint verbatim, so an exhausted credential returned to the pool half a second later and BackoffLevel never advanced past its current step: every retry recomputed the same level and immediately overwrote the deadline with the sub-second hint. Compute the escalating ladder first and let a provider hint only push the deadline further out, never pull it in. A genuine long hint still wins; a sub-second one can no longer undercut the ladder. Covered by TestMarkResultSubSecondQuotaHintStillEscalates and TestApplyAuthFailureStateSubSecondQuotaHintStillEscalates in sdk/cliproxy/auth/cooldown_backoff_test.go.
Record BackoffLevel increments and last-failure timestamp even when disable-cooling is true, while keeping credentials immediately usable without blackout or suspension. Add per-attempt failure logging to MarkResult to surface failure details.
Do not apply the escalating quota ladder floor when a 429 response explicitly specifies a zero or non-positive RetryAfter duration (e.g. transient websocket connection limit errors). Escalating quota cooldown remains gated to positive retry hints and default quota exhaustion.
The per-attempt failure logging test replaced the whole logrus hook map with an empty one during cleanup, deleting every hook the test process had already installed. That made later tests order-dependent and could silently disable process-wide hooks such as log forwarding. Deep-clone the pre-test hook map before AddHook and restore it in cleanup, matching setupTestLoggerHook in conductor_warn_logging_test.go.
decideAntigravity429 classifies a RATE_LIMIT_EXCEEDED 429 whose retry hint is shorter than three seconds as an instant retry on the same credential rather than an exhausted quota. The unconditional ladder floor still replaced that hint with a quota cooldown step, parking a still-usable credential for up to the full ladder window. Carry the executor classification through statusErr and Result so the ladder floor only applies to 429s that were not decisively classified as a short-lived rate limit. Exhausted quota and unclassified bodies keep the floor and keep escalating exactly as before.
The transient rate-limit flag only reached the conductor from the non-stream execution path. Streaming failures built their results from retryAfterFromError alone, and the Antigravity token-count path built statusErr by hand, so both still floored a provider-classified short-lived 429 at the quota ladder and parked a usable credential. Set TransientRateLimit next to every RetryAfter assignment in the streaming pool, and build the token-count errors through newAntigravityStatusErr so they inherit the same classification.
…close readStreamBootstrap consulted streamError() at channel close without finishing the bootstrap state first. flushData() runs only on a blank separator line or from finish(), so an SSE error event whose data line is newline-terminated but never followed by that blank line stays buffered in dataLines: the provider error is never evaluated, the bootstrap reports closed=true, and the caller receives an empty stream instead of a routable failure it can fail over on. hasMeaningfulOutput() already returns false once streamErr is set with no content, so finalizing first cannot swallow a real completion. Regression test: TestReadStreamBootstrapFinalizesDetectorAtEOF.
The Antigravity executor raises a synthetic 429 while an auth sits in a short cooldown. That cooldown is a local, self-imposed pause of at most a few minutes, but the error carried only a positive retryAfter hint and no classification, so isTransientRateLimitError() returned false and MarkResult()/applyAuthFailureState() read it as an exhausted upstream quota. BackoffLevel then escalated toward the 30 minute ceiling and parked an account that was never throttled upstream. Set transientRateLimit on all three cooldown short-circuits (Execute, executeClaudeNonStream, ExecuteStream) so the conductor rotates to the next auth instead of escalating backoff. Covered by TestAntigravityShortCooldownErrorIsTransient, which asserts the classification on all three entry points.
classifyClaudeUpstreamError built ordinary (non-unified) Claude 429s as claudeRateLimitError wrapping a statusErr with no transientRateLimit flag, so isTransientRateLimitError() returned false and MarkResult() treated an ordinary model-level throttle as exhausted quota, escalating BackoffLevel toward the 30 minute ceiling and parking a credential that was only briefly throttled. Mark the ordinary path transient. Unified 5h/7d rejections keep the quota ladder untouched. Covered by TestClassifyClaudeUpstreamError_OrdinaryRateLimitIsTransient and TestClassifyClaudeUpstreamError_UnifiedRejectionNotTransient.
|
Resolved #5150 queue+ review thread. P2
Pushes:
Verified:
Unresolved #5150 threads: 3 cache-file P2s (13). |
…back index freshness Mirror of CLIProxyAPI stock #5150 cache P2 fixes: - Loaded, not-found snapshots for absent replay values so Replace uses CAS against absence. - Delete is a no-op when the snapshot is not found. - Alias eviction tombstones values via KVCompareAndSwap, with KVDel fallback for backends that do not support CAS. - rollBackClaudeThinkingReplayAliasHome checks index record freshness and CAS-tombstones the committed value instead of unconditional KVDel. P2 review threads: claude_thinking_replay_cache.go:179, :795, :828.
|
Mirrored the three cache-file P2 fixes from CLIProxyAPI #5150:
Plus push: |
Mirror of CLIProxyAPI stock P2 fix: `claudeThinkingReplayScopeFromRequest` now applies `capClaudeThinkingReplayAliasMessages` before `ResolveClaudeThinkingReplaySessionKey`, and registration time uses the same helper. Session IDs already scope by credential in this fork. P2 review: claude_thinking_replay.go:57.
|
Mirrored the replay alias cap fix:
Plus push: |
…ure boundaries (exact head 1100eca)
Mirror of CLIProxyAPI stock #5150 fix: the cached-suffix ambiguity check now runs for both partial and full suffix-of-request matches, so duplicate cached turns sharing the same visible content fail closed instead of restoring the wrong signature. Added `TestClaudeThinkingReplayFindStartIndex_RefusesAmbiguousFullSuffix`. P2 review: helps/claude_thinking_replay.go:275.
|
Mirrored the
Plus push: |
…ure boundaries (exact head 1f9bee5)
Mirror of CLIProxyAPI stock #5150 fix: `rightmostSubsequenceMatch` now checks viability per candidate using `canMatchEarlier` and rejects duplicate cached turns that the preceding cached turns cannot consume. A single retained (thinking-bearing) candidate still disambiguates. Added `TestClaudeThinkingReplayFindStartIndex_RefusesPerTurnDuplicateCandidates`. P2 review: helps/claude_thinking_replay.go:333.
|
Mirrored the
Plus push: |
…write `cacheClaudeThinkingReplayContent` now checks the `replaced` result from `ReplaceClaudeThinkingReplayIfUnchanged` and registers the response's assistant-message alias only when the cache write succeeded. If the cache write fails (CAS lost, stale snapshot, KV error, or missing/failed replay record), the alias is not published. Added `TestCacheClaudeThinkingReplayContent_DoesNotRegisterAliasOnFailedCacheWrite`. P2 review: claude_thinking_replay.go:159.
|
Mirrored the
Added the regression test. Plus push: |
…tool provenance for alias hashing Mirror of CLIProxyAPI stock #5150 fix: - `rightmostSubsequenceMatch` fails closed when more than one viable thinking-bearing candidate exists. - `ClaudeThinkingReplayAssistantMessageHash` strips tool-use provenance before hashing. Added the two regression tests. P2 review: helps/claude_thinking_replay.go:357, :439.
|
Mirrored the
Plus push: |
…nd rebind alias group on unavailable conflicting auth Mirror of CLIProxyAPI #5150 fixes: - `restoreKimiThinkingReplayContent` fails closed with more than one retained thinking-bearing candidate. - `SessionAffinitySelector.Pick` rebinds the full alias group to the winning auth when the conflicting auth is unavailable. Added regression tests. P2 review: kimi_thinking_replay.go:189, selector.go:756.
|
Mirrored
Added regression tests. Plus push: |
…ure boundaries (exact head 1cebb90)
…overwrite Mirror of CLIProxyAPI #5150 fix: - `SessionAffinitySelector.Pick` uses `rebindConflictingAliases` (a single compare-and-replace via `SessionCache.CompareAndReplaceAliases`) instead of unconditional `SetAliases` when rebinding a conflicting unavailable auth. - If a concurrent caller already rebound the group, the loser falls back to the current cache binding rather than overwriting it. Added concurrency tests for `rebindConflictingAliases`. P2 review: selector.go:757.
|
Mirrored
Added concurrency tests for Plus push: |
…ure boundaries (exact head 16fb83b)
…e replay anchor Mirror of CLIProxyAPI #5150 fixes: - `SessionCache.ReplaceAliasesIfUnchanged` merges same-auth alias groups before rebinding to the winning auth; `rebindConflictingAliases` now uses it. - `NonThinkingContentParts` rejects content with no visible anchor. Added regression tests. P2 review: session_cache.go:360, replay_content.go:96.
|
Mirrored
Added regression tests. Plus push: |
…ure boundaries (exact head dd8fda1)
…nal rebind Mirror of CLIProxyAPI #5150 fix: - Added `compactSessionAliasesWithKeep` and updated `ReplaceAliasesIfUnchanged` to preserve the current request's session IDs when compacting the rebound alias group. Added `TestReplaceAliasesIfUnchanged_KeepsRequestedAliases`. P2 review: session_cache.go:390.
|
Mirrored
Added regression test. Plus push: |
…ure boundaries (exact head bda3d43)
Summary
Harden signature boundaries and failover for
CLIProxyAPIPlus(Plus counterpart of router-for-me/CLIProxyAPI#5150):sdk/cliproxy/auth/empty_completion.go: a GeminithoughtSignaturealone no longer counts as content; visible text, a tool call, or positive token usage is required.internal/translator/gemini/claude/gemini_claude_request.go: Claudethinking.signatureis routed throughGeminiReplaySignatureOrBypassbefore becoming a GeminithoughtSignature.internal/translator/claude/gemini/claude_gemini_response.go: Claudesignature_deltaevents are emitted as GeminithoughtSignaturecarriers in stream and non-stream paths.internal/translator/gemini-cli/claude/gemini-cli_claude_response.go: gemini-cli responses now preservethoughtSignaturein both stream (signature_delta) and non-stream (thinking.signature) output.internal/signature/claude_messages_sanitize.go: compat-mode thinking blocks are preserved but their signatures still go through compatibility logic. The fallback that keeps decodable E/R-shaped signatures only keeps unprefixed, non-foreign values, preventing Gemini E-prefixed signatures from slipping through the Claude fallback.internal/runtime/executor/claude_thinking_replay.go: replay matching is anchored to the first echoed assistant message, so compacted/truncated history does not mis-align older cached thinking onto later turns.internal/runtime/executor/claude_thinking_replay.go: unsigned/non-replayable responses no longer clear prior signed replay turns.internal/runtime/executor/helps/claude_thinking_replay_session.go&internal/cache: the sessionless fallback scope is now message-hash-aliased, so compacted history that changesmessages.0still resolves to the original conversation scope and can replay cached turns.internal/cache&internal/runtime/executor: replay aliases are a multi-session list with conversation first-user context, so two sessionless conversations that share a visible message do not collapse. Home KV aliases are bounded by a per-credential LRU index.Test plan
go build ./...go test ./...Stock counterpart
Rebased with merge-order hygiene on top of #213, #214, and the latest #5150 so all three PRs can be merged in any order.