fix(conductor): skip auth suspension for count_tokens 404s#2895
fix(conductor): skip auth suspension for count_tokens 404s#2895ArcaneOrion wants to merge 1 commit into
Conversation
Non-Anthropic upstreams (e.g. third-party proxies) don't support the /v1/messages/count_tokens endpoint and return 404. The conductor treated this as "model not found" and suspended the auth for 12 hours, blocking ALL subsequent requests for the same model with 503 errors. Skip MarkResult for 404 errors in executeCountMixedOnce so count_tokens failures don't trigger auth suspension. Other status codes (401, 429, 500, etc.) continue to be recorded normally. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
This pull request targeted The base branch has been automatically changed to |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request ensures that 404 errors from count_tokens endpoints on non-Anthropic upstreams do not trigger a 12-hour authentication suspension. The logic now skips MarkResult for these specific errors and includes a new test case to verify the behavior. One piece of feedback notes that bypassing MarkResult also skips result hooks, which could lead to a loss of observability for these events; it is suggested to call the hooks directly or refactor the result marking logic.
| if result.Error.HTTPStatus != http.StatusNotFound { | ||
| m.MarkResult(execCtx, result) | ||
| } else { | ||
| logEntryWithRequestID(execCtx).Debugf( | ||
| "skipping MarkResult for count_tokens 404 on auth=%s model=%s: %s", | ||
| auth.ID, upstreamModel, errExec.Error(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
While skipping MarkResult for 404 errors correctly prevents the 12-hour auth suspension for unsupported count_tokens endpoints, it also bypasses the m.hook.OnResult call. This means these failures will not be visible to any registered hooks (e.g., for metrics, logging, or auditing). If observability for these events is required, consider calling m.hook.OnResult directly or refactoring MarkResult to support a flag that skips suspension while still processing the result for hooks.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92c2f75f63
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if result.Error.HTTPStatus != http.StatusNotFound { | ||
| m.MarkResult(execCtx, result) |
There was a problem hiding this comment.
The new if result.Error.HTTPStatus != http.StatusNotFound guard skips MarkResult for all count-token 404s, not just the non-Anthropic "404 page not found" unsupported-endpoint case described in the commit message. In executeCountMixedOnce, this also suppresses model cooldown updates for legitimate model-not-found 404s (for example, stale registry entries against providers that use 404 for missing models), so the same bad auth/model pair keeps being selected and failing on subsequent ExecuteCount calls instead of being temporarily suspended.
Useful? React with 👍 / 👎.
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary
This prevents /v1/messages/count_tokens 404s (common for non-Anthropic upstreams that don’t implement the Anthropic-specific endpoint) from being treated as “model not found” and triggering a 12-hour auth/model suspension that can cascade into 503s for normal traffic on the same model.
What changed
- In
executeCountMixedOnce, skipMarkResultwhenCountTokensreturns HTTP 404, and emit a debug log instead. - Add a regression test
TestManagerExecuteCount_404DoesNotSuspendAuthto verify a count_tokens 404 does not suspend/cooldown the auth and that a subsequentExecutestill succeeds.
Notes (non-blocking)
- Skipping
MarkResultmeanshook.OnResultwon’t see these 404s; if you rely on hooks for metrics/monitoring, consider recording count_tokens 404s via a separate path that doesn’t affect auth state. - The debug log includes
errExec.Error(); please ensure CountTokens error strings cannot contain sensitive data.
Test plan
go test ./sdk/cliproxy/auth/...go test ./...
This is an automated Codex review result and still requires manual verification by a human reviewer.
2 participants
Summary
/v1/messages/count_tokensbecause they don'tsupport this Anthropic-specific endpoint
causing cascading 503 errors for ALL requests on that model
MarkResultfor 404 errors inexecuteCountMixedOnceso count_tokens failuresdon't trigger auth suspension
Test plan
TestManagerExecuteCount_404DoesNotSuspendAuth— verifies count_tokens 404 does notsuspend auth and subsequent Execute still succeeds
sdk/cliproxy/auth/...test suite passes with no regressions