When an Anthropic upstream returned HTTP 200 but then emitted an SSE
`event: error` frame (overloaded_error / rate_limit_error / api_error /
etc.), Forward's stream branch matched on `err.Error() == "have error in
stream"` and returned `UpstreamFailoverError{StatusCode: 403}` with no
ResponseBody. That dropped three pieces of evidence:
- handleFailoverExhausted → ExtractUpstreamErrorMessage(nil) = "" →
ops_error_logs.upstream_error_message was empty.
- errorPassthroughService.MatchRule(_, 403, nil) could only match rules
without keywords, so keyword-based passthrough rules silently never
fired.
- upstream_errors carried no stream_error record, leaving ops looking at
a generic 403 with no clue whether the upstream was throttled,
overloaded, or rejecting the request. ping-during-slot-wait amplified
this by skipping failover (writerSizeBeforeForward guard), so 403s
ballooned in the ops view well past the upstream's actual rate.
Fix:
- Introduce *sseStreamErrorEventError that carries the SSE data line.
Error() still returns "have error in stream" so existing log searches
keep working.
- Forward extracts via errors.As, appends an OpsUpstreamErrorEvent
(kind="stream_error", with the sanitized message and a truncated raw
body honoring LogUpstreamErrorBody*), and returns
UpstreamFailoverError{StatusCode: 403, ResponseBody: rawJSON}.
StatusCode 403 is preserved verbatim: mapUpstreamError, failover
decisions (shouldFailoverUpstreamError(403)=true), client-visible message,
RetryableOnSameAccount, and rateLimitService side-effects (this path
already didn't invoke them) all match prior behavior. OAuth and API Key
accounts share this path; the API-Key passthrough branch is independent
and already forwards SSE error frames untouched, so it's unaffected.
Adds four unit tests: typed-error contract + RawData, empty data line,
event:error after partial stream output (streamStarted=true), and
non-JSON data line.
The function signature was changed to require a mappedModel parameter
for protocol-aware thinking-block filtering, but this test call site
was not updated. Without the fix, the unit test build fails on CI.
The gateway's thinking-block handling was designed for Anthropic's strict
semantics (drop blocks with missing/invalid signature), but third-party
Anthropic-compatible upstreams have INVERTED semantics:
* DeepSeek `/anthropic`, Kimi `/coding`, GLM, Moonshot, qwen-*-thinking
require ALL historical thinking blocks to round-trip verbatim.
* Stripping any of them produces:
400 "The content[].thinking in the thinking mode must be passed back
to the API"
Without this fix, every multi-turn request from a thinking-capable client
(Claude Code, pi, etc.) to such upstreams loses its thinking blocks and
fails. This becomes especially painful when an account's model_mapping
maps `claude-sonnet-4-6 → deepseek-v4-pro` — `reqModel` looks Anthropic
but the upstream contract is the opposite.
Approach
--------
Branch all thinking-block transforms by the *mapped* model id (after
account model_mapping is applied), classifying into three families:
* `anthropic-strict` claude-/opus-/sonnet-/haiku- → existing behaviour
* `passback-required` deepseek-/kimi-/moonshot-/glm-/ → preserve verbatim
qwen-*-thinking
* `unknown` other models → conservative
(preserve, no retry)
Affected entry points (all guarded):
* Pre-filter on outbound: `FilterThinkingBlocks`
Previously dropped blocks with missing/invalid signature; now skips
entirely for non-strict families. Pre-filter is needed because the
post-error retry path can run out of budget on long conversations
(maxRetryElapsed = 10 s).
* 400 retry rectifier: `FilterThinkingBlocksForRetry`
Disables top-level thinking and converts thinking → text. Now skips
for passback-required (those 400s aren't signature errors and any
transformation breaks the round-trip contract).
* 400 retry rectifier (tools): `FilterSignatureSensitiveBlocksForRetry`
Same family-aware short-circuit.
* 400 detector: `shouldRectifySignatureError`
Returns false for passback-required, so the retry path doesn't even
fire.
Tests
-----
* `thinking_protocol_test.go` — classifier across all known vendor
prefixes plus edge cases (empty, case, qwen non-thinking).
* `thinking_protocol_filter_integration_test.go` — locks in that the
three filter entry points return the body byte-for-byte unchanged
when the model id is passback-required or unknown, and still strip
invalid blocks for anthropic-strict.
This PR supersedes #1350 (which only added the pre-filter without the
upstream-family awareness, and would have made third-party upstreams
worse). Once merged, please close#1350.
Reference issues:
- NousResearch/hermes-agent#16748 — DeepSeek /anthropic strip behaviour
- NousResearch/hermes-agent#15700 — DeepSeek thinking:disabled requirement
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pi maps defaultThinkingLevel 'xhigh' → 'max' for deepseek-v4-pro via
thinkingLevelMap, but normalizeOpenAIReasoningEffort did not recognize
'max', causing all reasoning_effort to be dropped (0% fill rate since
traffic switched from claude to deepseek on May 1).
Add 'max' → 'xhigh' mapping to match Claude's NormalizeClaudeOutputEffort
behavior.
Address copilot-pull-request-reviewer feedback on #2157:
- billing_service_test.go: extend TestGetFallbackPricing_FamilyMatching
with optional expectedOutput / expectedCacheRead fields and assert
full Input/Output/CacheRead pricing for all 4 DeepSeek cases
(v4-pro, v4-flash, deepseek-chat→flash, deepseek-reasoner→flash),
preventing silent regression to 0 output/cache cost.
- billing_service.go: rewrite the DeepSeek block comment to explicitly
describe its scope (V4 Pro/Flash + chat/reasoner aliases, no
unknown-deepseek fallback) and tighten the OpenAI comment to make
it unambiguous that it only describes the OpenAI/Codex branch
immediately below it.
Three independent CI blockers landed on main from concurrent PR merges:
- openai_quota_service.go (introduced by b8169492): const block spacing
not gofmt-compliant + trailing blank line. golangci-lint v2.9 flagged it
on every push after the merge.
- openai_images_failover_test.go (introduced by PR #3155, da30c599):
NewOpenAIGatewayHandler call missing the opsService argument added by
PR #3230 (b62b573f). Test was authored before #3230 and merged without
rebase, causing "not enough arguments" compile error.
- account_quota_reset_test.go: TestIsFixedDailyPeriodExpired_NotExpired
and TestIsFixedWeeklyPeriodExpired_NotExpired used time.Now()-1min as
periodStart, which crosses the 09:00 UTC reset boundary when CI runs in
the 09:00:00-09:00:59 window. Anchoring periodStart to today's 12:00
UTC removes the race.
Adds an admin-side action that mirrors the Codex Desktop "rate-limit reset"
flow against chatgpt.com upstream for OpenAI OAuth accounts.
Backend
- OpenAIQuotaService.QueryUsage / ResetCredit hit /wham/usage and
/wham/rate-limit-reset-credits/consume with the Codex Desktop header set,
reusing OpenAITokenProvider for refreshed tokens and PrivacyClientFactory
for the impersonated Chrome TLS fingerprint.
- Honors the account's configured proxy by reading the eager-loaded
account.Proxy directly (falls back to proxyRepo only when missing).
- GET /api/v1/admin/openai/accounts/:id/quota
POST /api/v1/admin/openai/accounts/:id/reset-quota
- Wire DI for the new service + handler dependency.
Frontend
- OpenAIQuotaResetCell renders a single action row in AccountUsageCell's
OpenAI section: the existing local "查询" (active sampling) is injected
via #pre-actions, alongside a "次数 N" button that doubles as the
upstream query trigger and the available-credit indicator, and a "重置"
button that consumes one credit.
- No duplicate 5h/7d window display; the local UsageProgressBar owns those
bars to avoid confusion.
#3255 introduced payload-aware dedup_key (sha256 over event_type, account_id,
group_id, payload_json). enqueueSchedulerOutbox checks "if payload != nil"
before json.Marshal — but Go interface containing a typed-nil map (returned
by buildSchedulerGroupPayload(empty)) is NOT == nil at the interface level.
So an ungrouped account's account_changed event went through this path:
payload := buildSchedulerGroupPayload(account.GroupIDs) // typed-nil map
enqueueSchedulerOutbox(..., payload) // interface != nil
→ json.Marshal(typedNilMap) = "null"
→ dedup_key hash = sha256(... + "null")
While other call sites pass literal nil:
enqueueSchedulerOutbox(..., nil) // interface == nil
→ payloadJSON stays empty
→ dedup_key hash = sha256(... + "")
The two dedup_keys differ for what should be the same logical event,
silently degrading dedup effectiveness in bursts on ungrouped accounts.
Fix: change buildSchedulerGroupPayload return type from map[string]any to
any so empty input returns true untyped-nil. All call sites pass the
result straight to enqueueSchedulerOutbox(payload any) — no inspection,
no breakage.
Adds regression test TestEnqueueSchedulerOutbox_UngroupedAccountDedupesWithLiteralNilPayload
asserting (1) typed-nil regression doesn't sneak back, (2) dedup_key for
empty-groups payload matches the literal-nil-payload key.
Post-merge audit of #3272 found two regressions:
1. ListOAuthRefreshCandidates used "AND NOT (a AND b)" which, under PG
3-valued logic, evaluates to NULL when both temp_unschedulable_until
and temp_unschedulable_reason are NULL — i.e., the common healthy
account state. Such rows were silently excluded from the background
token refresh worker, so their OAuth access tokens would never get
refreshed and eventually start returning 401.
Verified empirically against PostgreSQL: only 3 of 5 test rows
matched before the fix; after switching to "(a AND b) IS NOT TRUE"
the expected 4 rows match.
2. The new ListOAuthRefreshCandidates method on AccountRepository was
not implemented on stubAccountRepo in api_contract_test.go (build
tag "unit"), breaking "make test-unit" which CI runs in
.github/workflows/backend-ci.yml.
Tests:
- Added IS NOT TRUE and "AND NOT (" assertions to the SQL-shape unit
test so the predicate can't regress to the broken form again.
- "go test -tags=unit ./internal/..." now passes cleanly.
fix: cleanup consumed scheduler outbox rows / 添加 scheduler_outbox 表的清理代码，避免表过大
Additional hardening: WHERE clause adds a 10s grace via
created_at < NOW() - INTERVAL '10 seconds' to defend against the PG
sequence-id vs commit race (id assigned in tx, commit delayed past
watermark advance). Without it, slow committers could lose their
outbox row before the snapshot poller reads it.
PG sequences advance outside transactions, so a slow committer can hold
an id that gets surpassed by the watermark before its commit becomes
visible. Without the grace period, cleanup would delete such rows before
the snapshot poller ever sees them. 10s is a comfortable upper bound
on realistic enqueueSchedulerOutbox commit latency.
Wesley Liddick