Skip to content

fix(openai): choose the wire protocol by model, not by auth mode - #783

Merged
ericleepi314 merged 1 commit into
mainfrom
fix/openai-provider-routing
Aug 2, 2026
Merged

fix(openai): choose the wire protocol by model, not by auth mode#783
ericleepi314 merged 1 commit into
mainfrom
fix/openai-provider-routing

Conversation

@ericleepi314

Copy link
Copy Markdown
Collaborator

OpenAIProvider picked its wire protocol from the auth mode: a ChatGPT subscription meant the Responses API, an API key meant Chat Completions. Those are independent axes, and conflating them made the API-key route unusable for the newest models.

Probed live against the real API, all with tools attached:

model tools, no effort tools + effort
gpt-5 200 200
gpt-5.4 (default) 200 400
gpt-5.6-luna 400 400

"Function tools with reasoning_effort are not supported for <model> in /v1/chat/completions. To use function tools, use /v1/responses or set reasoning_effort to 'none'."

For luna that fires even with no effort in the body, because its default reasoning level is not none — so an agentic run, which always sends tools, could not use it at all on an API key.

Chat Completions' support for reasoning models is a per-model matrix that shifts each release. Responses serves them uniformly and is the endpoint the error itself points at, so protocol now follows the model and auth only picks the route. This is OpenCode's shape, which the task asked to match: its provider facade returns model: responses, its Codex plugin rewrites URL and bearer token over that same protocol, and it excludes max from the OpenAI effort type at compile time.

The part that took the work

The Responses path was written assuming "Responses implies ChatGPT subscription", and that assumption outlived the routing code. Carrying API-key traffic over it meant auditing everywhere it was baked in — each of these is a bug that would otherwise have surfaced in a run, not in review:

  • Retry was silently lost. The transport-drop retry lives in the base class's chat_stream_response; returning early from it opts every Responses request out. That retry is why 8 of 89 terminal-bench 2.1 trials stopped dying on peer closed connection. Dispatch moved to _stream_attempt, so both protocols sit inside the loop.
  • Cost would have read $0.00. build_usage_dict hardcoded billing_mode: subscription, which cost_tracker treats as free — right for a flat-rate plan, wrong for a metered key. Verified on a live run that now records $0.0016371 where it previously recorded nothing.
  • A 429 was unretryable. Non-200s raised a bare RuntimeError, which carries no status_code, so is_rate_limit_error said no and Retry-After was discarded. Now ResponsesHTTPError(RuntimeError), carrying status and response.
  • max_tokens was dropped, justified by Codex-backend behaviour ("Match codex cli") that doesn't apply to the public API. Callers pass it as a bound: compaction, the permission classifier, the /goal judge, the advisor. Worse, max_output_tokens_escalate re-issues with a larger value after truncation — with the value dropped, that retry was byte-identical to the request that just failed.
  • Proxies would have been bypassed. providers.openai.base_url is user-configurable, so this provider also reaches LiteLLM/vLLM/Azure. The switch is scoped to api.openai.com, reading $OPENAI_BASE_URL as well as the config key — the SDK falls back to it whenever base_url is None, which is the normal state. The OAuth-eligibility gate had the identical blind spot: proxy in the env + no key + a stored login sent prompts to chatgpt.com with no error. Both now share one predicate.

Also

--provider openai --model gpt-4o --effort high 400'd on its first call — the wire boundary injects reasoning_effort for every OpenAI-compatible provider and non-reasoning models reject it. Now stripped before the Chat Completions fallback, scoped to the first-party provider so OpenRouter and DeepSeek keep their effort, including the openrouter/openai/gpt-5.6-luna config the evals run on.

eval/harbor/clawcodex_agent.py documented two effort routes; there are three, and the same --ak effort=max runs at a different level on each (max on OpenRouter, xhigh on an OpenAI key, high on a ChatGPT plan). This repo has already mis-attributed eval results once to a stale effort claim.

Verification

  • Full suite 9526 passed, 0 failed.
  • Live end-to-end: --provider openai --model gpt-5.6-luna --effort xhigh completes real agentic tasks (writes modules plus pytest suites and drives them green), reads images, and --effort max clamps rather than 400s.
  • 36 tests in tests/test_openai_provider_routing.py; mutation-tested with 17 mutants, all killed.
  • Four pre-existing tests began failing because the default model now routes to Responses, slipping past their mock of the OpenAI SDK and reaching the network. They assert Chat Completions chunk shapes, so they are pinned to gpt-4o — the protocol they are written against, still live for non-reasoning models.

Known, deliberately not fixed here

GeminiProvider is the one provider that doesn't call strip_responses_item_blocks, so an openai → gemini mid-session /model switch leaks openai_responses_item blocks into its converter. Pre-existing and soft-failing (unknown blocks are dropped, not raised); this change widens who is exposed. Worth its own fix.

🤖 Generated with Claude Code

`OpenAIProvider` picked its protocol from the AUTH MODE: a ChatGPT
subscription meant the Responses API, an API key meant Chat Completions.
Those are independent axes, and conflating them made the API-key route
unusable for the newest models.

Probed live against the real API 2026-08-01, all with tools attached:

    model          tools, no effort   tools + effort
    gpt-5          200                200
    gpt-5.4        200                400
    gpt-5.6-luna   400                400

with the 400 reading "Function tools with reasoning_effort are not
supported for <model> in /v1/chat/completions. To use function tools, use
/v1/responses or set reasoning_effort to 'none'." For luna it fires even
with no effort in the body, because that model's default reasoning level
is not 'none' — so an agentic run, which always sends tools, could not use
it at all on an API key.

Chat Completions' support for reasoning models is therefore a per-model
matrix that shifts with each release. Responses serves all of them
uniformly and is the endpoint the error itself points at, so protocol is
now keyed on the model and auth only decides the route:

  * `_use_responses()` picks the protocol from the model's capability.
    Subscription still forces Responses (Codex speaks only that).
  * `_subscription_stream_request()` picks endpoint+headers by auth mode —
    the Codex backend with OAuth, or `{base_url}/responses` with a bearer
    key. Same wire format either way.
  * The `reasoning` block is gated on the model, and the effort ceiling on
    the auth mode: the ChatGPT backend keeps low/medium/high, the public
    API takes xhigh and degrades `max` to it. Sending `reasoning` to a
    non-reasoning model is a hard 400; the same request without it works.
  * The 401 OAuth-refresh path is now subscription-only. On an API key a
    401 means a rejected key, and refreshing reported "login expired" for
    what is really a bad key.

The Responses path was written assuming "Responses implies ChatGPT
subscription", and that assumption outlived the routing code. Carrying
API-key traffic over it meant auditing every downstream place it was baked
in, each fixed here rather than left to be discovered in a run:

  * Retry. The transport-drop retry loop lives in the base class's
    `chat_stream_response`, so returning early from it — as this class did
    — opts every Responses request out. That retry is why 8 of 89
    terminal-bench 2.1 trials stopped dying on `peer closed connection`.
    The dispatch now overrides `_stream_attempt` instead, so both
    protocols sit inside the loop.
  * Cost. `build_usage_dict` hardcoded `billing_mode: subscription`, which
    `cost_tracker.record_api_usage` treats as free. Correct for a flat-rate
    plan, wrong for a metered key — every API-key request would have
    recorded $0.00. The flag now follows the auth mode. Verified live: a
    real run records $0.0016371 where it previously recorded nothing.
  * Gateways. `providers.openai.base_url` is user-configurable, so this
    provider is also the route to LiteLLM/vLLM/Azure-style proxies, which
    speak Chat Completions universally but implement `/responses` only
    sometimes. The switch is scoped to `api.openai.com`, whose behaviour
    was actually measured; every other host keeps the protocol it has now.
    That check reads `$OPENAI_BASE_URL` as well as the config key, because
    the SDK falls back to it whenever `base_url` is None — which is the
    normal state, since `set_api_key` writes the key only when one is
    passed. Reading just the attribute would have sent Chat Completions to
    a configured egress proxy while the default model went straight to
    OpenAI, taking the API key and the conversation around it silently.
    The OAuth-eligibility gate in `__init__` derived that same rule a second
    time, from the constructor parameter, so it had the identical blind
    spot: a proxy in the env, no API key and a stored ChatGPT login sent
    prompts to chatgpt.com with no error, since an OAuth session cannot be
    proxied at all. Both now share `_is_first_party_base_url`, so the policy
    exists once.
  * Retryability. A non-200 raised a bare `RuntimeError`, which carries no
    `status_code` — so `is_rate_limit_error` and `is_overloaded_error` both
    said no, and a 429 was abandoned instead of backed off with the
    server's own `Retry-After`. Survivable on a plan that rate-limits by
    tier; not on an API key under eval concurrency. Now
    `ResponsesHTTPError(RuntimeError)`, carrying status and response.
  * Output bounds. `max_tokens` was dropped, justified by Codex-backend
    behaviour ("Match codex cli") that does not apply to the public API.
    Callers pass it as a bound, not a preference: compaction summaries, the
    permission classifier, the /goal judge and the advisor all set one. And
    query.py's `max_output_tokens_escalate` lane re-issues with a larger
    value after a truncated reply — with the value dropped, that retry was
    byte-identical to the request that had just truncated, burning a
    full-context turn to reproduce the same failure. Now forwarded as
    `max_output_tokens` on the API-key route only.

Separately, `--provider openai --model gpt-4o --effort high` 400'd on its
first call: the wire boundary injects `extra_body.reasoning_effort` for
every OpenAI-compatible provider, and non-reasoning models reject it
outright. It is now stripped before the Chat Completions fallback —
scoped to the first-party provider, since OpenRouter and DeepSeek
namespace their ids differently and accept the field on models this
capability check would not recognise. Applying it globally would silently
drop effort for them, including the openrouter/openai/gpt-5.6-luna
configuration the evals run on.

`supports_reasoning` excludes `-chat` variants, matching the carve-out
`supports_verbosity` already makes in the same file. This one is
unverified — gpt-5-chat-latest 404s on the account used for probing — but
excluding it costs nothing if wrong (it falls to the older, more exercised
path) whereas including it wrongly is a 400 on every request.

Four pre-existing tests in test_providers.py and test_tool_arg_recovery.py
began failing because the default model now routes to Responses, slipping
past their mock of the OpenAI SDK and reaching the network. They assert
Chat Completions chunk shapes, so they are pinned to gpt-4o — the protocol
they are written against, still live for non-reasoning models.

Shape follows OpenCode, which the task asked to match: its provider facade
returns `model: responses`, its Codex plugin rewrites URL and bearer token
over that same protocol, and it excludes `max` from the OpenAI effort type
at compile time.

eval/harbor/clawcodex_agent.py documented two effort routes; there are now
three, and the same `--ak effort=max` runs at a different level on each
(`max` on OpenRouter, `xhigh` on an OpenAI key, `high` on a ChatGPT plan).
Benchmarks read that file to interpret what a run actually sent, and this
repo has already mis-attributed eval results once to a stale effort claim.

Not fixed here, as pre-existing and separable: GeminiProvider is the one
provider that does not call `strip_responses_item_blocks`, so an
`openai → gemini` mid-session `/model` switch leaks
`openai_responses_item` blocks into its converter. This change widens who
is exposed to it, but it is a cross-provider defect with its own fix.

Verified live end to end: `--provider openai --model gpt-5.6-luna --effort
xhigh` completes real agentic tasks (writes modules plus pytest suites and
drives them green), reads images, and `--effort max` clamps rather than
400s. New tests/test_openai_provider_routing.py covers routing, effort
normalisation, the gateway and env-var carve-outs, OAuth eligibility,
retry, 401 handling, error classification, output bounds and billing;
mutation-tested with 17 mutants, all killed.

The module docstring described the API-key path as "unchanged behaviour"
against Chat Completions, which is now the inverse of what the code does.
It has been rewritten around the protocol/route split, since that is the
invariant the next editor needs and the one this change was needed to
establish.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ericleepi314
ericleepi314 merged commit e8dd5ff into main Aug 2, 2026
2 checks passed
ericleepi314 added a commit that referenced this pull request Aug 2, 2026
…s that we lacked

OpenCode enumerates its OpenAI-compatible providers in
`packages/llm/src/providers/openai-compatible-profile.ts`: baseten,
cerebras, deepinfra, deepseek, fireworks, groq, openrouter, togetherai,
xai. Four of them — groq, cerebras, baseten, xai — had no row here.

They are plain `/chat/completions` vendors, so this is metadata only: no
production code, four `ProviderSpec` rows. The registry already
synthesizes a working provider class from a row, which is why the port is
this small.

Model ids are deliberately NOT taken from OpenCode. Its ids for these
vendors have aged — its xai entry predates the grok-4.x line entirely, and
copying it would have shipped `grok-4`/`grok-code-fast-1`, neither of
which still exists. Each id here was read from the vendor's own docs
(2026-08-02) and pinned in `test_provider_registry.VENDOR_DEFAULTS`, which
is the existing guard against exactly this kind of drift.

Verified, since a table of URLs is the easy thing to get quietly wrong:
every base URL answers `/models` with 401/403 rather than 404, and the
real CLI run against each with a bogus key returns a distinct
vendor-shaped auth error — so the URL and auth wiring reach the actual
vendor, not merely a registry entry.

All four declare `dynamic_catalog="openai-compatible"`, so the curated
`available_models` list can be refreshed from the vendor at runtime rather
than stranding users on ids that will age the same way OpenCode's did.

xai takes the Chat Completions route, which is a deliberate departure:
OpenCode defaults its xai facade to the Responses protocol
(`providers/xai.ts`, `model: responses`), the same shape #783 gave
first-party OpenAI. That switch was justified by a measured defect —
`/chat/completions` rejects tools outright for some reasoning models — and
no equivalent measurement exists here, because no xAI key was available to
probe. Assuming the defect transfers would be assuming the conclusion.
Chat Completions is what every OpenAI-compatible vendor supports;
`test_xai_uses_chat_completions_not_responses` marks the decision so it is
revisited deliberately rather than by drift.

Examined and deliberately NOT ported:

  * OpenRouter's `usage: {include: true}` (openrouter.ts:57-62). Probed
    live against the real API: the response is byte-identical with and
    without it — `cost` and `cost_details` come back either way. Sending it
    would be cargo cult.
  * DeepSeek. OpenCode treats it as a plain profile with no special
    handling; clawcodex already does more (prompt-prefix-cache usage
    re-mapping, and `reasoning_content` in both directions).
  * The profile-table architecture itself, which `ProviderSpec` already
    matches and exceeds — 23 rows to OpenCode's 9.

While pinning the new aliases, the collision test turned out to only
iterate `_SPECS`, so it could not see a new alias shadowing one of the
seven hand-written providers (anthropic, deepseek, gemini, minimax,
openai, openrouter, zai) — a mutant adding `deepseek` as an xai alias
survived it. Widened to the full namespace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericleepi314 added a commit that referenced this pull request Aug 2, 2026
…s that we lacked

OpenCode enumerates its OpenAI-compatible providers in
`packages/llm/src/providers/openai-compatible-profile.ts`: baseten,
cerebras, deepinfra, deepseek, fireworks, groq, openrouter, togetherai,
xai. Four of them — groq, cerebras, baseten, xai — had no row here.

They are plain `/chat/completions` vendors, so this is metadata only: no
production code, four `ProviderSpec` rows. The registry already
synthesizes a working provider class from a row, which is why the port is
this small.

Model ids could not come from OpenCode, because its profile table has none
to give: `OpenAICompatibleProfile` is `{provider, baseURL}` and nothing
else, for all nine entries, and `providers/xai.ts` carries no ids either.
Each id here was read from the vendor's own docs (2026-08-02) and pinned
in `test_provider_registry.VENDOR_DEFAULTS`, the existing drift guard.

Two rows needed model configs, not just registry entries:

  * `gpt-oss-120b` and `gpt-oss-20b` had no `MODEL_CONFIGS` row, so
    `get_model_config`'s prefix fallback — `key.rsplit("-", 1)[0]` —
    reduced them to `gpt` and matched the gpt-5.x family. Cerebras's
    default model is a bare `gpt-oss-120b`, so it silently inherited
    gpt-5.5's 272k context window, 128k output cap and $3/$15 pricing.
    The window is the damaging one: it sizes auto-compaction, so a session
    would run past the real 131k limit and die on a context-length 400
    rather than compacting.
  * The namespaced `openai/gpt-oss-120b` that groq and baseten serve
    dodged that prefix but fell to the generic 200k default — also larger
    than the truth, so the same failure arriving later. Explicit rows for
    the namespaced forms are the remedy `get_model_config`'s own docstring
    documents.

    Adding rows to that table risks poisoning unrelated ids through the
    same prefix fallback, so every id's resolution was snapshotted before
    and after: only the two intended ones moved.

`catalog_mode` is new on `ProviderSpec`. `dynamic_catalog` alone meant
"discovered REPLACES static", which is right for the three rows that had
it — sglang, vllm and ollama are local servers whose static ids are
placeholder stubs — and backwards for a hosted vendor, whose curated list
is deliberate and whose raw `/models` also lists speech, moderation and
embedding models. Under the inherited default, Groq's curated ids vanished
from the picker and ASR/TTS models took their place. The hosted rows now
say `hybrid`, matching what `openrouter_provider` already passes by hand.

Verified, since a table of URLs is the easy thing to get quietly wrong:
every base URL answers `/models` with 401/403 rather than 404, and the
real CLI run against each with a bogus key returns a distinct
vendor-shaped auth error — so the URL and auth wiring reach the actual
vendor, not merely a registry entry.

All four declare `dynamic_catalog="openai-compatible"`, so the curated
`available_models` list can be refreshed from the vendor at runtime rather
than stranding users on ids that will age the same way OpenCode's did.

xai takes the Chat Completions route, which is a deliberate departure:
OpenCode defaults its xai facade to the Responses protocol
(`providers/xai.ts`, `model: responses`), the same shape #783 gave
first-party OpenAI. The reason is structural rather than a coin flip —
`openai_responses` is imported only by `openai_provider`, and
`_use_responses` sits behind `_is_first_party_base_url()`, which #783
scoped to api.openai.com, so Responses is not something a registry row can
select at all. It would need a hand-written class and a carve-out in that
gate. Chat Completions is also known-good for this model rather than
assumed: OpenCode's own docs serve grok-4.5 over both protocols, routing it
to `/chat/completions` in `web/src/content/docs/go.mdx` and to `/responses`
in `zen.mdx`. `test_xai_requests_go_to_chat_completions` asserts the URL
actually requested, so the decision is revisited deliberately, not by
drift.

Examined and deliberately NOT ported:

  * OpenRouter's `usage: {include: true}` (openrouter.ts:57-62). Probed
    live against the real API: the response is byte-identical with and
    without it — `cost` and `cost_details` come back either way. Sending it
    would be cargo cult.
  * DeepSeek. OpenCode treats it as a plain profile with no special
    handling; clawcodex already does more (prompt-prefix-cache usage
    re-mapping, and `reasoning_content` in both directions).
  * The profile-table architecture itself, which `ProviderSpec` already
    matches and exceeds — 23 rows to OpenCode's 9.

While pinning the new aliases, the collision test turned out to only
iterate `_SPECS`, so it could not see a new alias shadowing one of the
seven hand-written providers (anthropic, deepseek, gemini, minimax,
openai, openrouter, zai) — a mutant adding `deepseek` as an xai alias
survived it. It is now seeded from provider ids AND `PROVIDER_ALIASES`,
since names like `glm` and `z.ai` are aliases rather than ids and were
shadowable through the same hole.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericleepi314 added a commit that referenced this pull request Aug 2, 2026
…s that we lacked (#784)

* feat(providers): add the four OpenAI-compatible vendors OpenCode ships that we lacked

OpenCode enumerates its OpenAI-compatible providers in
`packages/llm/src/providers/openai-compatible-profile.ts`: baseten,
cerebras, deepinfra, deepseek, fireworks, groq, openrouter, togetherai,
xai. Four of them — groq, cerebras, baseten, xai — had no row here.

They are plain `/chat/completions` vendors, so this is metadata only: no
production code, four `ProviderSpec` rows. The registry already
synthesizes a working provider class from a row, which is why the port is
this small.

Model ids could not come from OpenCode, because its profile table has none
to give: `OpenAICompatibleProfile` is `{provider, baseURL}` and nothing
else, for all nine entries, and `providers/xai.ts` carries no ids either.
Each id here was read from the vendor's own docs (2026-08-02) and pinned
in `test_provider_registry.VENDOR_DEFAULTS`, the existing drift guard.

Two rows needed model configs, not just registry entries:

  * `gpt-oss-120b` and `gpt-oss-20b` had no `MODEL_CONFIGS` row, so
    `get_model_config`'s prefix fallback — `key.rsplit("-", 1)[0]` —
    reduced them to `gpt` and matched the gpt-5.x family. Cerebras's
    default model is a bare `gpt-oss-120b`, so it silently inherited
    gpt-5.5's 272k context window, 128k output cap and $3/$15 pricing.
    The window is the damaging one: it sizes auto-compaction, so a session
    would run past the real 131k limit and die on a context-length 400
    rather than compacting.
  * The namespaced `openai/gpt-oss-120b` that groq and baseten serve
    dodged that prefix but fell to the generic 200k default — also larger
    than the truth, so the same failure arriving later. Explicit rows for
    the namespaced forms are the remedy `get_model_config`'s own docstring
    documents.

    Adding rows to that table risks poisoning unrelated ids through the
    same prefix fallback, so every id's resolution was snapshotted before
    and after: only the two intended ones moved.

`catalog_mode` is new on `ProviderSpec`. `dynamic_catalog` alone meant
"discovered REPLACES static", which is right for the three rows that had
it — sglang, vllm and ollama are local servers whose static ids are
placeholder stubs — and backwards for a hosted vendor, whose curated list
is deliberate and whose raw `/models` also lists speech, moderation and
embedding models. Under the inherited default, Groq's curated ids vanished
from the picker and ASR/TTS models took their place. The hosted rows now
say `hybrid`, matching what `openrouter_provider` already passes by hand.

Verified, since a table of URLs is the easy thing to get quietly wrong:
every base URL answers `/models` with 401/403 rather than 404, and the
real CLI run against each with a bogus key returns a distinct
vendor-shaped auth error — so the URL and auth wiring reach the actual
vendor, not merely a registry entry.

All four declare `dynamic_catalog="openai-compatible"`, so the curated
`available_models` list can be refreshed from the vendor at runtime rather
than stranding users on ids that will age the same way OpenCode's did.

xai takes the Chat Completions route, which is a deliberate departure:
OpenCode defaults its xai facade to the Responses protocol
(`providers/xai.ts`, `model: responses`), the same shape #783 gave
first-party OpenAI. The reason is structural rather than a coin flip —
`openai_responses` is imported only by `openai_provider`, and
`_use_responses` sits behind `_is_first_party_base_url()`, which #783
scoped to api.openai.com, so Responses is not something a registry row can
select at all. It would need a hand-written class and a carve-out in that
gate. Chat Completions is also known-good for this model rather than
assumed: OpenCode's own docs serve grok-4.5 over both protocols, routing it
to `/chat/completions` in `web/src/content/docs/go.mdx` and to `/responses`
in `zen.mdx`. `test_xai_requests_go_to_chat_completions` asserts the URL
actually requested, so the decision is revisited deliberately, not by
drift.

Examined and deliberately NOT ported:

  * OpenRouter's `usage: {include: true}` (openrouter.ts:57-62). Probed
    live against the real API: the response is byte-identical with and
    without it — `cost` and `cost_details` come back either way. Sending it
    would be cargo cult.
  * DeepSeek. OpenCode treats it as a plain profile with no special
    handling; clawcodex already does more (prompt-prefix-cache usage
    re-mapping, and `reasoning_content` in both directions).
  * The profile-table architecture itself, which `ProviderSpec` already
    matches and exceeds — 23 rows to OpenCode's 9.

While pinning the new aliases, the collision test turned out to only
iterate `_SPECS`, so it could not see a new alias shadowing one of the
seven hand-written providers (anthropic, deepseek, gemini, minimax,
openai, openrouter, zai) — a mutant adding `deepseek` as an xai alias
survived it. It is now seeded from provider ids AND `PROVIDER_ALIASES`,
since names like `glm` and `z.ai` are aliases rather than ids and were
shadowable through the same hole.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(git): stop test_run_git_timeout racing a real 1ms git log

The test ran `git log` with `timeout=0.001` and asserted
`rc != 0 or stdout == ""`. That is a race against the runner: when git
finishes inside the millisecond the call succeeds with real output and the
assertion fails. It failed, passed, then failed again across three CI runs
of an unrelated change.

Passing was no better than failing — a timeout that never fires exercises
none of the branch under test, so the green runs asserted nothing.

Now forces `subprocess.TimeoutExpired` and asserts what `_run_git`
actually returns for it: `("", "Command timed out", -1)`. Mutating the
return code or the message fails it; previously neither did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericleepi314 added a commit that referenced this pull request Aug 2, 2026
…OpenRouter reasoning (#785)

Four cross-provider follow-ups deferred from #783 and #784. All were
silent: nothing errored, the numbers and the transcript were just wrong.

## Cached prompt tokens were billed at the full input rate

`prompt_tokens` on the OpenAI-compatible wire INCLUDES tokens served from
the prompt cache, with the cached count reported separately under
`prompt_tokens_details.cached_tokens`. The base usage builder read only
the total, so every cached token was priced as fresh input and the cache
hit-rate was invisible.

Both builders now split onto the convention `compute_cost` already
understands — `input_tokens` = miss, `cache_read_input_tokens` = hit,
`cache_creation_input_tokens` = 0 (this wire has no cache-write charge).
Every pricing tier already carried a `cache_read` rate, and two of them
(`_TIER_MUSE_SPARK`, `_TIER_GPT_56_LUNA`) said in comments that the rate
was inert "for when that lands". This is that; those comments now say it
is live.

Measured against DeepSeek: a 2613-token prompt came back as 53 miss /
2560 hit. Pricing that turn without the split over-reports it 7.5x for
`openai/gpt-5.6-luna` and 29x for `deepseek-v4-pro`.

Three interactions had to be fixed with it:

  * `DeepSeekProvider._build_usage_dict` calls `super()` and then does its
    own `prompt_tokens - hit`. With the base splitting first, that
    subtraction ran twice and drove billable input to ZERO — under-
    reporting cost on the provider whose prefix cache fires nearly every
    turn. It now recovers the original total from the parts.
  * `openai_responses.build_usage_dict` recorded the hit alongside an
    untouched `input_tokens`, so the two double-counted: consumers sum
    input + cache_creation + cache_read, and that turn reported 5173 for a
    2613-token prompt. It billed the cached portion at BOTH rates and
    inflated the prompt size `get_pricing` uses to select a tier, which
    for gpt-5.6-luna can cross the 272K boundary. Since #783 routes by
    model, one `OpenAIProvider` can take either wire, so the two agreeing
    is now a tested invariant rather than a coincidence.
  * `agent_server._usage_token_total` summed input+output only, ignoring
    the cache fields already present in the snapshot it reads — an
    under-count of 98% of the prompt on a warm cache.

Both builders reject a non-numeric `cached_tokens` rather than coercing
it. `int()` accepts anything with `__int__`, so a `MagicMock` usage stub
yielded 1 and invented a one-token cache hit; `bool` is excluded because
it is an `int` subclass and `True` would read as a cached token. They also
catch `OverflowError`, which descends from `ArithmeticError` and so slipped
past a `ValueError`-only guard — stdlib `json.loads` accepts a bare
`Infinity`, so a vendor emitting one ended the turn.

## OpenRouter reasoning was discarded entirely

Verified live: a streamed `openai/gpt-5.6-luna` turn over OpenRouter
carries `delta.reasoning` and `delta.reasoning_details`, and no
`reasoning_content` key at all. Both extraction sites read only
`reasoning_content`, so every reasoning token from the provider this
repo's benchmarks run on was dropped. Reading either name recovers it —
336 characters across 71 thinking chunks on the same prompt.

`reasoning_details` is deliberately not read: it repeats the same text in
a shape nothing downstream consumes, and arrives as `reasoning: None` when
the trace is encrypted.

## Gemini strips ChatGPT replay blocks explicitly

`GeminiProvider` was the one converter of four not calling
`strip_responses_item_blocks`. This is defence in depth, NOT a bug fix —
the converter's `if parts:` guard already drops a message whose blocks all
fell through its if/elif chain, so the observable result is identical. The
point is that the correctness stops depending on two implicit
fallthroughs: adding an `else` branch or a placeholder part, the natural
way to support some future block type, would otherwise start forwarding
ChatGPT replay items to Gemini silently.

## README provider list

Listed 25 against an actual 30, omitting meta, groq, cerebras, baseten and
xai. A test now asserts the list equals `PROVIDER_INFO` so it cannot drift
again.

## Deferred, and why

`agent_loop_compat.py`'s cumulative `result.usage` aggregator drops the
cache keys, and `compute_session_cost`'s advisor branch passes none. Both
are PRE-EXISTING and documented as such in the code — they were already
wrong for Anthropic and DeepSeek, whose wires are natively split. This
change widens their blast radius to OpenAI-compatible providers rather
than creating the defect, and fixing the aggregator is a contract change
to stream-json. `logging.py`'s `NonNullableUsage.total_tokens` and
`status_line_command`'s context bar have the same gap with no production
caller today.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericleepi314 added a commit that referenced this pull request Aug 2, 2026
Headline: fusion models (#771) — pair a text-only reasoning model with a
vision-capable one so it can read screenshots, diagrams and code images.
`deepseek-v4-pro` rejects an image content block outright, so a pasted
screenshot used to end the turn; a fusion model describes the image with
the second model first and hands the base model text.

Verified end to end on Terminal-Bench 2.1's `code-from-image` — transcribe
handwritten pseudocode from a PNG and reproduce its output — with
`deepseek-v4-flash` + `openai:gpt-5.6-luna` (#787). The base model alone
returns a 400 on the same image, so the pass is attributable to the fusion
path rather than the base coping.

Also in 1.4.0: GPT-5.6 Sol/Terra/Luna (#773); groq, cerebras, baseten and
xai take the provider registry to 30 (#784); `/mode` becomes
`/permissions` with a three-level picker (#768); `AskUserQuestion` renders
a real picker instead of returning JSON to the model (#774); the OpenAI
provider picks its wire protocol from the model rather than the auth mode
(#783); cached prompt tokens bill at the cache rate (#785, #786); headless
runs stop reporting a cut-short run as success (#777#782).

Version bumped in all five spots (pyproject, install.sh INSTALLER_VERSION,
gatewayClient CLAWCODEX_VERSION, src/__init__.py fallback, uv.lock).
CHANGELOG `[Unreleased]` covered only through #773 and was backfilled with
#774#787; PR citations added to the pre-existing entries so coverage is
checkable. #766 is docs-only and deliberately uncited.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant