fix(profile,codex): compare model ids through their owner, relay the provider's status - #222
Merged
Merged
Conversation
`assertExactProfileRequest` stripped the harness prefix from the request's model but not from the profile-qualified model it compared against, so an exactly matching pair was refused and the message printed two identical strings: `request model "kimi-code/kimi-for-coding" conflicts with agent_profile.model "kimi-code/kimi-for-coding"` (#212). A kimi-code director could not run at all: agent-runtime requires an explicit `model.provider`, and the only provider the Kimi CLI has a config.toml key for is the harness itself, which is the shape this check refused. Compose the profile's declared id the way the caller composes it — agent-runtime `profileBridgeWireModel` keeps ONE harness prefix, so a provider equal to the harness collapses into it — then strip that id with the same `modelWithinHarness` the request goes through. The provider-only branch gets the same rule. The refusal now prints the two harness-relative values it actually compared. This supersedes #161, which refused that profile shape outright to keep a harness name out of a CLI's `model_provider`. That invariant now holds where it applies: the composed wire id carries one harness prefix, a provider equal to it is spent there, and no `<harness>/<harness>/<model>` route survives the comparison — so the codex test asserts the CLI arguments rather than the profile shape, and README states the current rule. The shipped profiles/opencode-generalist.json (provider `opencode`, harness `opencode`) was refused by the same defect and is covered here. Tests: the first direct tests for the function — claude-code, codex, opencode and kimi-code, including the exact ids measured in #212. Five of the eleven fail against the previous comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every codex failure left the bridge as `type: "upstream"`, whatever the
provider had said. Downstream that is one undifferentiated class: a capacity
refusal that clears by itself, a rate-limit refusal worth one backoff, and a
malformed request that will fail identically forever all read the same, so a
retry policy can only guess — and agent-runtime already decodes this exact
channel as `upstreamCode` (`bridgeUpstreamError`) and branches on it.
Read the refusal from both channels codex reports it on: the discriminant it
sets itself (`codex_error_info`) and the provider's JSON body it quotes inside
the message text. The code rides the error envelope's `type`, any reset instant
rides a new `reset_at` beside it, and the CLI's own words stay intact in
`message`. An unrecognized payload still reports `upstream`, because that is the
bridge saying it cannot classify the failure, not a classification.
Both channels are read the way the recordings show them, not the way the enum
reads on paper. `CodexErrorInfo` serializes a unit variant as a bare string but
a variant carrying data as a single-key object, so the commonest recorded
rate-limit refusal — `{"response_too_many_failed_attempts":{"http_status_code":
429}}`, 320 of 1092 — has no `type` field to read. And `other` is codex
declining to classify, so it must not outrank a code the provider did state: the
20 recorded malformed requests arrive as `other` with `invalid_request_error` in
the quoted body, which is exactly the class this change exists to separate.
The end-of-stream path ends through the delta envelope rather than a throw:
`BackendError.code` is the bridge's own closed taxonomy and cannot carry a
provider discriminant. The route answers both shapes 502, so only the code is
new.
Fixtures are recorded, not invented: 514 rollout files in ~/.codex/sessions
scanned on 2026-09-10, 1092 refusals, 8 distinct shapes, each fixture carrying
its source file and count. The one exception is the `resets_at` payload — no
recorded refusal carries a machine-readable reset instant, so that fixture is
built from codex's own `UsageErrorBody` shape and is labelled CONSTRUCTED.
Tests: capacity, rate-limit and malformed-request refusals surface three
distinct codes. Measured red: all 6 relay tests fail against the previous relay,
which threw a `BackendError` instead of yielding a terminal delta; 5 of the 14
fail against a parser that reads only `codex_error_info.type` and lets `other`
win.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hird copy Review of 218ccf9 found the comparison hand-rolled a rule that @tangle-network/agent-profile-materialize already owns and that cli-bridge already depends on. Its own module doc names the failure: "Every writer and every reader must run both sides through here ... Two independent copies of this rule are exactly how that bug arises." Issue #212 was that bug, and the first fix answered it with a third copy. `profileWireModel` is deleted. Model agreement now calls `modelIdsMatch`, with `unqualifyModelId`/`qualifyModelId` for the harness prefix — the one qualification that package does not own, because the request carries it and the profile does not. Cross-run against agent-runtime's own `profileBridgeWireModel` over 14 profile shapes, the check accepts 13 of the ids it composes; both exceptions are profiles with no model, which `assertExecutableAgentProfile` refuses before execution, and the docstring now states them instead of claiming unqualified parity. One of the two is newly pinned by a test. The #161 invariant is enforced where it actually holds. The README claimed no `<harness>/<harness>/<model>` route existed; it did. Measured with a fake spawner: `codex/codex/gpt-5-codex` reached the CLI as `-c model_provider="codex"`, a provider no config.toml defines, and the profile branch could never have stopped it — that request carries no profile at all. CodexBackend now drops a provider segment equal to its own name when it builds the argv, which covers every route into the backend, and the README says that rather than describing a composition rule. Red checks, measured in this worktree: reverting the argv drop fails the new codex-backend case; running the new profile cases against the base implementation fails 5 of 11, including both #212 ids and the shipped profiles/opencode-generalist.json shape. The `claude`/`kimi` alias prefixes are kept, against the review's advice: a backend's registered name and the HarnessId it passes to this file are independent, so a ClaudeBackend registered under its default `claude` name routes `claude/opus` into a `claude-code` check. Measured — dropping the aliases fails tests/docker-executor.test.ts with `request model "claude/opus" does not select harness "claude-code"`. The alias tests stay, and a comment names the route that reaches them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…idge's own codes Review of 59075a8 found the relay did not reach the outcome it was written for, read one channel at the wrong nesting, and let provider text choose the bridge's own meaning. Three fixes, plus a fixture that claimed a provenance it did not have. RETRY. The relayed code reaches agent-runtime as `upstreamCode`, but its `classifyDriverFailure` branches on only three bridge codes and otherwise on `status`, which this path never set. Measured against the installed runtime (classifyDriverFailure over BackendTransportError): the malformed-request refusal classified `transient` before the change AND with the code relayed but no status — so a 400 was still re-driven to the attempt ceiling, the exact failure 59075a8 cited as its motivation. With the provider's status relayed it classifies `terminal`, while the 429 rate-limit shape stays `transient`. The status rides on `ChatDelta.error.status`, a field whose own doc already describes this problem; only codex's parser was not filling it in. NESTING. A data-carrying `CodexErrorInfo` variant nests its fields under the variant key — `{"response_too_many_failed_attempts":{"http_status_code":429}}` — and the discriminant reader handled that while the field reader beside it did not. Both now go through one `codexErrorVariantPayload`. The provider's status is read from the wrapper that states it, beside `error` rather than inside it, which is where the recordings put it. TAXONOMY. The relay channel is shared with the bridge's own codes: the route answers 504 for `timeout`, and agent-runtime never retries `parse_error`. Provider and CLI text reaches that channel, so a quoted body naming either word decided the bridge's status or ended a caller's retries. Relaying a reserved code is now refused and falls back to `upstream`; the CLI's words still reach the caller in `message`. `providerErrorBody` also requires a candidate to look like an error body, because codex quotes tool output into the same prose and the first balanced object there is not the provider's. `reset_at` is deleted rather than fixed. It had no reader — none in agent-runtime's source, none here — and no producer: of 983 recorded refusals, zero carry a machine-readable reset instant. The one fixture that exercised it was constructed for that purpose. FIXTURES. One payload was labelled a verbatim recording and was not: the stream-disconnect refusal carries `codex_error_info: "other"`, which its fixture dropped, so its test pinned a shape occurring nowhere in the recordings. Re-scanned this session with an explicit counting rule now stated in the file: 517 rollouts, 983 refusals, server_overloaded 410 / response_too_many_failed_attempts 309 / other 164 / usage_limit_exceeded 92 / unauthorized 8, and every recorded refusal sets the field. All counts are restated at that denominator; the earlier 1092 is not reproducible under any rule stated here. The `other` bucket decomposes exactly: 139 gateway + 20 invalid_request_error + 5 stream-disconnect. The commit message of 59075a8 claimed "all 6 relay tests fail against the previous relay". That count was wrong — an independent review reproduced 8 — and the test file has since changed, so the red checks measured here are the current ones: reverting the reserved-code guard fails 1, the variant nesting fails 2, the body-shape test fails 1, and letting codex's `other` outrank the provider code fails 3, including the case that previously only counted distinct values and so passed under that collapse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
Two defects measured on the live bridge, plus the review findings against the first
two commits on this branch.
#212 — a profiled turn was refused by an equality bug.
assertExactProfileRequestcompared a request model already stripped of its harness prefix against a profile id
that was not, so an exactly matching pair was refused and the message printed two
identical strings. A kimi-code pursuit settled
driver-failedwith zero tokens on it.Every codex failure left the bridge as one opaque class. A capacity refusal that
clears by itself, a rate-limit refusal worth one backoff, and a request that will fail
identically forever all reached the caller as
type: "upstream", so a retry policycould only guess.
What each change lifts, and whose code it extends
modelIdsMatch/qualifyModelId/unqualifyModelIdfrom@tangle-network/agent-profile-materialize(already a pinned direct dependency, 0.19.2). Its module doc names this exact bug: "Every writer and every reader must run both sides through here … Two independent copies of this rule are exactly how that bug arises." The first fix on this branch answered #212 with a third copy;profileWireModelis now deleted.ChatDelta.error.status, an existing bridge field whose own doc already describes this failure ("a frame with no status falls into 'unknown, assume a bad moment', which retried a permanently malformed request to its ceiling"). Only codex's parser was not filling it in. agent-runtime'sclassifyDriverFailurereads it.#161invariantCodexBackend's argv construction, the only place that also covers a request carrying no profile.No new dependency:
package.jsonandpnpm-lock.yamlare unchanged.Measured evidence from this session
The comparison now agrees with the composer. Cross-run of agent-runtime's own
profileBridgeWireModel(from the installed build) over 14 profile shapes, feedingeach composed id back into the check: 13 accepted. Both exceptions are profiles with
no model — a harness-native
model.defaultand a provider-only profile — whichassertExecutableAgentProfilerefuses before execution. The docstring states themrather than claiming unqualified parity; one is newly pinned by a test.
The
<harness>/<harness>/<model>route existed. The README claimed it could not.Driven with a fake spawner,
codex/codex/gpt-5-codexreached the CLI as-c model_provider="codex"— a provider noconfig.tomldefines. That request carriesno profile, so the profile comparison could never have stopped it.
The relayed code alone did not change any retry decision. Measured against the
installed runtime with
classifyDriverFailureover aBackendTransportErrorcarryingthe recorded malformed-request message:
upstream, no status)transienttransientterminaltransientSo a 400 was still re-driven to the attempt ceiling — the failure the relay commit
cited as its motivation — until the status was relayed, and capacity refusals stay
retryable.
Provider text could choose the bridge's own meaning. A codex message quoting
{"error":{"type":"timeout"}}producedtype: 'timeout', which the route turns into504 instead of 502;
parse_errorin the same position is a code agent-runtime neverretries. Both are refused now and fall back to
upstream, with the CLI's words intactin
message.Fixture provenance was wrong and is corrected. One payload was labelled a verbatim
recording and was not — the stream-disconnect refusal carries
codex_error_info: "other", which the fixture dropped, so its test pinned a shape occurring nowhere inthe recordings. Re-scanned
~/.codex/sessionsthis session under an explicit countingrule now stated in the file (one refusal = one object carrying both
messageandcodex_error_info): 517 rollouts, 983 refusals — server_overloaded 410,response_too_many_failed_attempts 309, other 164, usage_limit_exceeded 92,
unauthorized 8. Every recorded refusal sets the field. The
otherbucket decomposesexactly: 139 gateway + 20
invalid_request_error+ 5 stream-disconnect. All fixturecounts are restated at that denominator; the earlier 1092 is not reproducible under
any rule stated here.
reset_atis deleted, not fixed. It had no reader (none in agent-runtime's source,none here) and no producer: 0 of 983 recorded refusals carry a machine-readable reset
instant. The only fixture exercising it was constructed for that purpose.
Red checks (each new test proven to detect the defect it covers, by reverting only
the source logic and restoring):
profiles/opencode-generalist.jsonshapeotheroutrank the provider code → 3 fail, including the case that previously only counted distinct values and so passed under exactly that collapseTest command and result
Test Files 4 failed | 59 passed | 2 skipped (65),Tests 11 failed | 1095 passed | 24 skipped (1130), 94.07s.origin/main(1d19faf) ran the same full suite:Tests 11 failed | 1063 passed | 24 skipped (1098)with a byte-identical list of failing test names. They arehost-environment failures in subsystems this PR does not touch —
tests/jail.test.ts(5) and
tests/docker-executor.test.ts(3) compare macOS paths before realpath(
/var/…vs/private/var/…),tests/pi-inference-isolation.test.ts(2) readsLinux
/proc/self/environ, andtests/failure-attribution.test.ts(1) fails fromthe same realpath mismatch. The worktree has been removed.
# tests 1 / # pass 1 / # fail 0. Run separately becausevitest's non-zero exit short-circuits the
&&, so the packaged command never reachesit.
in
retained-sessionsandopencode-profile-isolation; both pass in isolation anddid not recur on two subsequent runs. This machine is under load from two live
research pursuits. Reported rather than omitted.
Not in scope
BackendFailureReasonis a shared type, but codex is itsonly producer;
kimi.ts,sandbox.tsand the rest still collapse every providerrefusal into
upstream. The type's doc says so instead of implying the fix alreadyreached them.
src/routes/chat-completions.ts. The status selection there is safe once therelay cannot emit a reserved code, so the fix is applied at the producer. The route
is untouched.
BackendReportedFailureErrorcarries onlymessage and code, so a relayed status does not reach an error body built by
errorResponse. It does reach the streamed frame and the partial-output body.test helpers to resolve symlinked macOS temp paths — a different subsystem and a
different owner.
harnessModelPrefixesalias entries. Review advised deletingclaude/kimiasunreachable. Measured otherwise: a backend's registered name and the
HarnessIditpasses are independent, and dropping the aliases fails
tests/docker-executor.test.tswithrequest model "claude/opus" does not select harness "claude-code". Kept, with a comment naming the route.🤖 Generated with Claude Code