feat(proxy): warn Claude Code when a subscription turn fails over to the billable Weave key - #852
feat(proxy): warn Claude Code when a subscription turn fails over to the billable Weave key#852steventohme wants to merge 2 commits into
Conversation
…the billable Weave key
Munir's Claude subscription hit its 5h unified rate limit mid-session
(anthropic-ratelimit-unified-5h-status: rejected) while a long, cache-heavy
session was running under a native-web-search Anthropic-only constraint. The
router correctly failed the turn over to the billable Weave/BYOK Anthropic
key -- both credential-suppression paths already do this -- but did so
silently, so the customer had no signal that ~$800 of subsequent turns would
be billed instead of served on their own plan.
Add subscriptionFailoverWarningMarker, a new routing-marker constant distinct
from subscriptionOnlyWarningMarker: that one signals a permanent, org-level
depleted-credits state with a top-up CTA; this one is a per-turn,
self-healing condition ("this turn", not "until you add credits"), so it
carries no CTA and no opt-out gate -- same billing-state-change rule as the
existing subscription-only warning.
Wire it into both places a subscription-served Anthropic turn can fail over:
- Pre-emptive (service.go ~2457): a prior observer snapshot already reads
exhausted before dispatch. Thread subscriptionFailingOver through to the
marker assembly so the turn's single marker choice reflects it.
- Reactive (service.go ~2886): a live 429/OAuth-rejection triggers the
subscription-credit retry onto the Weave key. Since this branch only runs
pre-commit (preludeBuf.Committed() == false), swapping the retry attempt's
marker is safe -- nothing under the original marker has hit the wire yet.
Codex/OpenAI is out of scope: there is no codexSubscriptionExhausted
equivalent gating that path today, so a Codex subscription failover stays
silent until that symmetric guard exists.
Tests: two new full-flow ProxyMessages integration tests in
failover_integration_test.go covering both paths end-to-end (including,
for the reactive path, that the router's own same-binding retry runs first
and the failover marker only appears on the eventual Weave-key attempt).
|
Claude finished @steventohme's task —— View job
Posted 6 comment-length suggestions (advisory, won't block merge):
|
workweave-bot
left a comment
workweave-bot
left a comment
There was a problem hiding this comment.
Advisory only — comment-length nits. Won't block merge.
| // TestProxyMessages_PreemptiveSubscriptionFailoverWarnsUser guards the | ||
| // pre-emptive path (an observer snapshot already reads exhausted before | ||
| // dispatch): the turn must serve on the deployment key AND the client must see | ||
| // the billable-failover warning marker instead of the normal routing marker. |
There was a problem hiding this comment.
| // TestProxyMessages_PreemptiveSubscriptionFailoverWarnsUser guards the | |
| // pre-emptive path (an observer snapshot already reads exhausted before | |
| // dispatch): the turn must serve on the deployment key AND the client must see | |
| // the billable-failover warning marker instead of the normal routing marker. | |
| // TestProxyMessages_PreemptiveSubscriptionFailoverWarnsUser: observer already | |
| // reads exhausted — turn must serve on the deployment key and emit the | |
| // billable-failover warning instead of the normal routing marker. |
Was 4 lines narrating what the function name + assertions already convey; the why fits in 3.
| // TestProxyMessages_ReactiveSubscriptionFailoverWarnsUser guards the reactive | ||
| // path: a subscription-served Anthropic turn hits a live retryable error | ||
| // (429), so the router retries the SAME model on the Weave key. The retry | ||
| // must carry the billable-failover warning marker, not the original routing | ||
| // marker, and the retry must authenticate via x-api-key (not the spent OAuth | ||
| // bearer). |
There was a problem hiding this comment.
| // TestProxyMessages_ReactiveSubscriptionFailoverWarnsUser guards the reactive | |
| // path: a subscription-served Anthropic turn hits a live retryable error | |
| // (429), so the router retries the SAME model on the Weave key. The retry | |
| // must carry the billable-failover warning marker, not the original routing | |
| // marker, and the retry must authenticate via x-api-key (not the spent OAuth | |
| // bearer). | |
| // TestProxyMessages_ReactiveSubscriptionFailoverWarnsUser: live 429 on the | |
| // subscription triggers a retry on the Weave key — retry must carry the | |
| // billable-failover marker and authenticate via x-api-key, not OAuth bearer. |
Was 6 lines; same content fits in 3.
| // Attempts 1-3: subscription OAuth bearer. dispatchWithFallback | ||
| // retries a sole binding in place up to maxSameBindingRetries (2) | ||
| // before giving up, so the 429 must persist through all three | ||
| // same-binding attempts to reach the subscription-failover retry. |
There was a problem hiding this comment.
| // Attempts 1-3: subscription OAuth bearer. dispatchWithFallback | |
| // retries a sole binding in place up to maxSameBindingRetries (2) | |
| // before giving up, so the 429 must persist through all three | |
| // same-binding attempts to reach the subscription-failover retry. | |
| // Attempts 1-3: dispatchWithFallback retries the same binding up to | |
| // maxSameBindingRetries (2), so 429 must persist all three attempts | |
| // before the subscription-failover retry fires. |
Was 4 lines; same non-obvious WHY fits in 3.
| // Subscription hit its 5h/7d limit pre-dispatch: the turn is about to | ||
| // serve on the billable Weave/BYOK key instead. Not gated by the | ||
| // routing-marker opt-out — same billing-state-change rule as the | ||
| // subscription-only warning above. |
There was a problem hiding this comment.
| // Subscription hit its 5h/7d limit pre-dispatch: the turn is about to | |
| // serve on the billable Weave/BYOK key instead. Not gated by the | |
| // routing-marker opt-out — same billing-state-change rule as the | |
| // subscription-only warning above. | |
| // Not gated by the routing-marker opt-out — billing state change, | |
| // same rule as subscriptionOnlyWarningMarker above. |
Was 4 lines; the one non-obvious point (opt-out exemption) fits in 2.
| // Swap in the billable-failover warning for this retry attempt — safe | ||
| // because reaching here already required preludeBuf.Committed() == | ||
| // false, so nothing under the original marker has hit the wire yet. |
There was a problem hiding this comment.
| // Swap in the billable-failover warning for this retry attempt — safe | |
| // because reaching here already required preludeBuf.Committed() == | |
| // false, so nothing under the original marker has hit the wire yet. | |
| // Safe: preludeBuf.Committed() == false here, so no bytes are committed under the original marker. |
Was 3 lines; the invariant fits on one.
| // subscriptionFailoverWarningMarker is prepended when a subscription-served | ||
| // Anthropic turn fails over to the billable Weave/BYOK key mid-session (5h/7d | ||
| // unified rate-limit rejection, or an OAuth credential rejection). Distinct | ||
| // from subscriptionOnlyWarningMarker: that one signals a permanent, org-level | ||
| // state (credits depleted, until top-up); this one is a per-turn, self-healing | ||
| // condition, so the copy says "this turn" rather than pointing at a CTA. |
There was a problem hiding this comment.
| // subscriptionFailoverWarningMarker is prepended when a subscription-served | |
| // Anthropic turn fails over to the billable Weave/BYOK key mid-session (5h/7d | |
| // unified rate-limit rejection, or an OAuth credential rejection). Distinct | |
| // from subscriptionOnlyWarningMarker: that one signals a permanent, org-level | |
| // state (credits depleted, until top-up); this one is a per-turn, self-healing | |
| // condition, so the copy says "this turn" rather than pointing at a CTA. | |
| // subscriptionFailoverWarningMarker is prepended when a subscription turn fails | |
| // over to the billable Weave/BYOK key (5h/7d rate-limit or OAuth rejection). | |
| // Unlike subscriptionOnlyWarningMarker (permanent org-level state), this is | |
| // per-turn and self-healing — no CTA. |
Was 6 lines; the permanent-vs-per-turn contrast fits in 4.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 78a62f4. Configure here.
| // serve on the billable Weave/BYOK key instead. Not gated by the | ||
| // routing-marker opt-out — same billing-state-change rule as the | ||
| // subscription-only warning above. | ||
| marker = subscriptionFailoverWarningMarker |
There was a problem hiding this comment.
Failover warning ignores routed provider
High Severity
The preemptive path sets subscriptionFailoverWarningMarker whenever claudeSubscriptionExhausted is true, without checking that decision.Provider is Anthropic. After a limit hit, subsidy drops and routing often picks OSS/Gemini, so those unrelated turns still get the billable Claude-failover warning even though no subscription→Weave Anthropic failover occurred.
Reviewed by Cursor Bugbot for commit 78a62f4. Configure here.
| // Swap in the billable-failover warning for this retry attempt — safe | ||
| // because reaching here already required preludeBuf.Committed() == | ||
| // false, so nothing under the original marker has hit the wire yet. | ||
| subAttempt := s.anthropicNativeAttempt(env, r, subPrep, sink, preludeBuf, subscriptionFailoverWarningMarker, setExtractor) |
There was a problem hiding this comment.
Warning misstates non-limit failovers
Medium Severity
The reactive retry always swaps in subscriptionFailoverWarningMarker, whose copy claims a Claude usage-limit hit and automatic window reset. That path also runs for any IsRetryable fault (5xx, 408, stalls, transport) and for OAuth authentication_error/permission_error, so users can be told they burned plan quota when the cause was a transient outage or rejected token.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 78a62f4. Configure here.
…over bit on router.call Downstream consumers of the OTLP router.call log record (WorkWeave's backend ingest) have no way to detect a subscription->billable-key failover today: credential_source only reaches the router's OWN Postgres telemetry insert (InsertTelemetryParams), never the OTel span/log attrs that recordCallLog reuses as its metadata base. Add credential.source to both ProxyMessages' and ProxyOpenAIChatCompletion's upstream span builders, computed once via the existing credentialKeyParts(ctx) (moved earlier so it's in scope for the builder, not just the later telemetry-insert call site). Also add dispatch.subscription_failover_billable, the OR of the pre-emptive (subscriptionFailingOver, an already-exhausted observer snapshot skipping dispatch entirely) and reactive (subscriptionFailoverUsed, a live 429/OAuth-rejection retry) subscription-failover paths on the Anthropic side. The existing dispatch.subscription_failover only covers the reactive branch -- on its own it would under-report, since the pre-emptive path is the more common trigger in practice. Extends the two failover_integration_test.go tests added in the prior commit to also assert CredentialSource on the captured telemetry row.
|
Pushed a second commit adding |
|
Claude finished @steventohme's task —— View job Claude finished @steventohme's task — View job
Posted 2 comment-length suggestions (advisory, won't block merge) for the new commit:
|
workweave-bot
left a comment
workweave-bot
left a comment
There was a problem hiding this comment.
Advisory only — comment-length nits. Won’t block merge.
| // Computed here (rather than only at the later Postgres-telemetry site) | ||
| // so credential.source and the combined subscription-failover flag reach | ||
| // the router.call OTLP log record — WorkWeave's ingest reads that record, | ||
| // not this router's own Postgres telemetry table. |
There was a problem hiding this comment.
| // Computed here (rather than only at the later Postgres-telemetry site) | |
| // so credential.source and the combined subscription-failover flag reach | |
| // the router.call OTLP log record — WorkWeave's ingest reads that record, | |
| // not this router's own Postgres telemetry table. | |
| // Computed here so credential.source and the subscription-failover flag | |
| // reach the router.call OTLP log record — WorkWeave’s ingest reads that, | |
| // not this router’s Postgres telemetry table. |
Was 4 lines; the non-obvious why (OTLP vs. Postgres telemetry site) fits in 3.
| // Combined pre-emptive (subscriptionFailingOver, an exhausted observer | ||
| // snapshot skipping dispatch entirely) OR reactive | ||
| // (subscriptionFailoverUsed, a live 429/OAuth-rejection retry) signal — | ||
| // either means this turn is billed on the Weave/BYOK key instead of the | ||
| // caller's own subscription. dispatch.subscription_failover above only | ||
| // covers the reactive branch; this one is the authoritative bit for | ||
| // admin alerting downstream. |
There was a problem hiding this comment.
| // Combined pre-emptive (subscriptionFailingOver, an exhausted observer | |
| // snapshot skipping dispatch entirely) OR reactive | |
| // (subscriptionFailoverUsed, a live 429/OAuth-rejection retry) signal — | |
| // either means this turn is billed on the Weave/BYOK key instead of the | |
| // caller's own subscription. dispatch.subscription_failover above only | |
| // covers the reactive branch; this one is the authoritative bit for | |
| // admin alerting downstream. | |
| // Combined pre-emptive (subscriptionFailingOver) OR reactive | |
| // (subscriptionFailoverUsed) signal — dispatch.subscription_failover above | |
| // covers only the reactive branch; this is the authoritative bit for alerting. |
Was 7 lines; the one non-obvious point (covers both paths, unlike the field above) fits in 3.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Swap in the billable-failover warning for this retry attempt — safe | ||
| // because reaching here already required preludeBuf.Committed() == | ||
| // false, so nothing under the original marker has hit the wire yet. | ||
| subAttempt := s.anthropicNativeAttempt(env, r, subPrep, sink, preludeBuf, subscriptionFailoverWarningMarker, setExtractor) |
There was a problem hiding this comment.
This retry also runs for generic retryable errors and OAuth credential rejections, but it always injects text saying that the Claude subscription “hit its usage limit.” A recovered 503, timeout, transport failure, or OAuth 401 can therefore be shown to the customer as quota exhaustion even when no usage limit was involved. Reserve quota-specific copy for positively identified subscription exhaustion and use neutral failover wording for other recovery paths.
Artifacts
Focused hermetic ProxyMessages test source for 503 and OAuth rejection
- The executed test configures a real ProxyMessages request and Anthropic HTTP client with local upstream responses for non-quota failures, then checks the returned SSE marker; it documents the reproducible coverage.
Baseline quota-exhaustion subscription failover test passed
- The existing 429 subscription failover test was executed from `/home/user/repo` and passed after retrying with suppressed OAuth credentials; it confirms the baseline quota flow.
503 and OAuth rejection retries emit quota-exhaustion wording
- The focused hermetic test was executed from `/home/user/repo` and failed as expected: both a successful 503 recovery and a successful OAuth-401 recovery sent the client the quota-exhaustion marker, confirming the bug.
| const subscriptionFailoverWarningMarker = routingMarkerPrefix + | ||
| "your Claude subscription hit its usage limit, so this turn is running on the Weave router key and will be billed. Full routing resumes automatically once your subscription window resets.\n\n" |
There was a problem hiding this comment.
After suppressing the subscription credential, credential resolution can select the customer’s Anthropic BYOK key. The retry then authenticates with that customer key, but this fixed message says that it is running on the Weave router key and that the turn will be billed. Select the warning based on the resolved fallback credential source so BYOK retries accurately describe credential ownership and billing.
Artifacts
Focused hermetic ProxyMessages failover test source
- Authored test drives subscription 429 retries followed by a customer Anthropic BYOK fallback and asserts the customer-visible marker, establishing the executable scenario.
Subscription failover runtime output with customer BYOK fallback
- Executed `go test` output shows three subscription attempts, a successful fourth BYOK-authenticated retry, and the retained Weave-billing marker; the mismatch is confirmed.
Diff whitespace validation output
- Executed `git diff --check` completed with exit code 0 after adding only the focused test, confirming no patch formatting errors.
T-Rex LogsWhat T-Rex did
|
|
Depended on by workweave/WorkWeave#11247 (subscription-failover admin alert) — that PR reads |
2 participants
Why
Investigating an anomalous router-usage session ($836 on one org's Weave API key), we traced the proximate cause to: a Claude subscription hit its 5-hour unified rate limit mid-session (
anthropic-ratelimit-unified-5h-status: rejected), and the router correctly failed the turn over to the billable Weave/BYOK Anthropic key — both credential-suppression paths already handle this — but did so silently. The customer had no signal that subsequent turns would be billed instead of served on their own plan.What
Adds
subscriptionFailoverWarningMarker, a new routing-marker constant distinct fromsubscriptionOnlyWarningMarker:subscriptionOnlyWarningMarker— permanent, org-level depleted-credits state, with a top-up CTA.subscriptionFailoverWarningMarker(new) — per-turn, self-healing condition ("this turn", not "until you add credits"), no CTA, not gated by the routing-marker opt-out (same billing-state-change rule as the existing marker).Wired into both places a subscription-served Anthropic turn can fail over:
service.go~2457) — a prior observer snapshot already reads exhausted before dispatch.service.go~2886) — a live 429/OAuth-rejection triggers the subscription-credit retry onto the Weave key. Swap is safe because this branch only runs pre-commit (preludeBuf.Committed() == false).Out of scope
Codex/OpenAI: there's no
codexSubscriptionExhaustedequivalent gating that path today, so a Codex subscription failover stays silent until that symmetric guard is built (separate follow-up).Tests
Two new full-flow
ProxyMessagesintegration tests infailover_integration_test.go— one per path — asserting the marker text appears and the retry authenticates via the deployment key, not the spent subscription bearer. Full suite:wv mr tc && wv mr tpass.🤖 Generated with Claude Code