From a0e4e8dee74a589ba8fd0a427c556e9e5e724c29 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 02:45:07 +0900 Subject: [PATCH 1/4] fix(combos): let a local input-admission refusal actually hop the fallback chain #1524. The hop rule for `input_admission_refused` existed but never fired on a real refusal. `comboFailureDecision` tested the generic stop list first, and `classifyError` maps a genuine 413 admission body to `context_length_exceeded` -- so the request hit `return "stop"` two lines before the rule that was written to hop it. The chain ended at the first candidate whose context window was too small, which is exactly the defect the issue reports. The existing test did not catch this because it passed `{"code":"..."}` as a top-level field. That shape classifies to `upstream_error`, misses the stop list, and reaches the hop rule -- so the rule looked alive while the shape the proxy actually emits (`{"error":{"type":..,"code":..}}`) was still stopping. Move the admission check above the stop list. An UPSTREAM `context_length_exceeded` carries no admission marker and still falls through to stop, because retrying that elsewhere is guesswork. This covers both fallback paths at once: policy fallback reaches the same function through `shouldHopPolicyCandidate`. Verification: two cases in tests/routing-policy-fallback.test.ts drive the real error body end to end through the fallback loop -- a local refusal advances to the next candidate, an upstream context verdict does not. Driven red by restoring the original ordering, which reproduces the terminated chain. Note the weaker ablation that also had to be ruled out: disabling only the structured-code arm still passes, because the `message.includes` fallback catches it; ORDER is the load-bearing change. 46 tests green across policy fallback, combos and e2e-style; `bun x tsc --noEmit` clean. --- src/combos/failover.ts | 16 +++++---- tests/routing-policy-fallback.test.ts | 49 +++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/combos/failover.ts b/src/combos/failover.ts index 3944eb2c6f..df9121a8db 100644 --- a/src/combos/failover.ts +++ b/src/combos/failover.ts @@ -121,19 +121,21 @@ export function comboFailureDecision( if (isCyberPolicyCode(options?.code)) return "stop"; const error = classifyError(status, "upstream_error", message); if (isCyberPolicyCode(error.code)) return "stop"; - if (["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code ?? "")) { - return "stop"; - } // A local input-admission refusal (#1524) says "this candidate cannot fit the request", - // not "the request is impossible". Hopping is exactly right: the next candidate may have a - // larger context window. Read it from the structured code OR the body text, because the - // generic classifier maps 413 to its own code and would otherwise swallow this signal. - // Upstream `context_length_exceeded` above still stops. + // not "the request is impossible": the next candidate may have a larger context window. + // + // This MUST be tested before the generic stop list below. `classifyError` maps a real 413 + // admission body to `context_length_exceeded`, so checking the stop list first swallowed + // the signal and ended the chain -- the exact behavior #1524 reports. An UPSTREAM + // `context_length_exceeded` carries no admission marker and still falls through to stop. if (options?.code === "input_admission_refused" || error.code === "input_admission_refused" || message.includes("input_admission_refused")) { return "hop"; } + if (["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code ?? "")) { + return "stop"; + } if ([401, 403, 404, 408, 429].includes(status) || status >= 500) return "hop"; if ([ "permission_denied", diff --git a/tests/routing-policy-fallback.test.ts b/tests/routing-policy-fallback.test.ts index a81d1ddd79..6a73550265 100644 --- a/tests/routing-policy-fallback.test.ts +++ b/tests/routing-policy-fallback.test.ts @@ -54,6 +54,55 @@ describe("policy candidate fallback", () => { ]); }); + test("a local input-admission refusal hops instead of ending the chain (#1524)", async () => { + // #1524: a candidate whose context window cannot fit the request used to TERMINATE the + // fallback chain. It is a local preflight verdict about ONE candidate, not about the + // request, so the next candidate -- which may have a larger window -- must still be tried. + const trace = policyTrace(); + const logCtx = { requestedModel: "policy/daily", routeDecision: trace, attempts: [] } as unknown as RequestLogContext; + const seenModels: string[] = []; + const runCore = async (req: Request, _config: OcxConfig, ctx: RequestLogContext) => { + const body = await req.clone().json() as { model?: string }; + seenModels.push(String(body.model)); + ctx.routeDecision = trace; + seedAttempt(ctx, "provider", String(body.model)); + if (seenModels.length === 1) { + return Response.json( + { error: { message: "Estimated input is far past the context window", type: "input_admission_refused", code: "input_admission_refused" } }, + { status: 413 }, + ); + } + return Response.json({ id: "resp", object: "response", status: "completed", output: [] }); + }; + + const response = await handleResponsesWithPolicyFallback(request(), {} as OcxConfig, logCtx, {}, { runCore }); + + expect(response.status).toBe(200); + expect(seenModels).toEqual(["policy/daily", "provider-b/model-b"]); + }); + + test("an upstream context_length_exceeded still stops the chain (#1524)", async () => { + // The mirror-image contract. An upstream verdict is about the REQUEST, so retrying it + // elsewhere is guesswork -- and hopping would burn every candidate on a doomed request. + const trace = policyTrace(); + const logCtx = { requestedModel: "policy/daily", routeDecision: trace, attempts: [] } as unknown as RequestLogContext; + const seenModels: string[] = []; + const runCore = async (req: Request, _config: OcxConfig, ctx: RequestLogContext) => { + const body = await req.clone().json() as { model?: string }; + seenModels.push(String(body.model)); + ctx.routeDecision = trace; + seedAttempt(ctx, "provider", String(body.model)); + return Response.json( + { error: { message: "context length exceeded", type: "invalid_request_error", code: "context_length_exceeded" } }, + { status: 400 }, + ); + }; + + const response = await handleResponsesWithPolicyFallback(request(), {} as OcxConfig, logCtx, {}, { runCore }); + + expect(response.status).toBe(400); + expect(seenModels).toEqual(["policy/daily"]); + }); test("retries the next policy candidate and keeps distinct physical attempts", async () => { const trace = policyTrace(); const logCtx = { requestedModel: "policy/daily", routeDecision: trace, attempts: [] } as unknown as RequestLogContext; From bc6019bae270ade1d47303143fd197f85d507f2e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 03:10:03 +0900 Subject: [PATCH 2/4] fix(errors): preserve the local admission-refusal code so the hop rule can see it Adversarial pre-merge audit found the previous commit did not actually fix #1524: reordering comboFailureDecision was necessary but not sufficient. core.ts emits the refusal through formatErrorResponse(413, "input_admission_refused", ...), which runs classifyError -- and the message necessarily contains "context window", because that is what it is refusing on. The generic remap at errors.ts rewrote our own code to context_length_exceeded, so the envelope the proxy actually ships carried no admission marker at all. Verified against the real emitter: code came back context_length_exceeded and the decision was still "stop". Preserve the code: classifyError now returns input_admission_refused when that is the type it was given, before the "context window" remap can claim it. The two verdicts need opposite fallback handling -- ours means "this candidate does not fit", an upstream one means "the request is impossible" -- so collapsing them is what ended the chain at the first candidate that was merely too small. Also narrow the hop rule to the structured code. With the code preserved the raw-substring arm is no longer load-bearing, and it was a real hazard: error text is provider-controlled, so any upstream echoing the token could override a terminal verdict. Confirmed reachable before this change. The test fixture was the reason the gap survived review: it hand-built an envelope the proxy does not emit. It now calls formatErrorResponse itself, and a new case pins that an upstream merely mentioning the marker does not hop. Ablations, both driven red independently: disabling the classifyError branch fails, and restoring the original ordering fails. Neither change alone is sufficient, which is why the earlier single-factor ablation was misleading. 102 tests green across policy fallback, combos and bridge; tsc --noEmit clean. --- src/combos/failover.ts | 16 ++++++------ src/lib/errors.ts | 9 +++++++ tests/combos.test.ts | 7 +++++- tests/routing-policy-fallback.test.ts | 36 ++++++++++++++++++++++++--- 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/combos/failover.ts b/src/combos/failover.ts index df9121a8db..a8cf18175f 100644 --- a/src/combos/failover.ts +++ b/src/combos/failover.ts @@ -124,13 +124,15 @@ export function comboFailureDecision( // A local input-admission refusal (#1524) says "this candidate cannot fit the request", // not "the request is impossible": the next candidate may have a larger context window. // - // This MUST be tested before the generic stop list below. `classifyError` maps a real 413 - // admission body to `context_length_exceeded`, so checking the stop list first swallowed - // the signal and ended the chain -- the exact behavior #1524 reports. An UPSTREAM - // `context_length_exceeded` carries no admission marker and still falls through to stop. - if (options?.code === "input_admission_refused" - || error.code === "input_admission_refused" - || message.includes("input_admission_refused")) { + // This MUST be tested before the generic stop list below. Our own refusal message says + // "context window" -- that is what it refuses on -- and the classifier remaps that phrase, + // so checking the stop list first swallowed the signal and ended the chain. An UPSTREAM + // `context_length_exceeded` carries no admission code and still falls through to stop. + // + // Matched on the STRUCTURED code only. The refusal is ours, so it always carries the code + // now that classifyError preserves it; a raw substring test would instead let any upstream + // override a terminal verdict by echoing the token in text we do not control. + if (options?.code === "input_admission_refused" || error.code === "input_admission_refused") { return "hop"; } if (["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code ?? "")) { diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 316002b582..c5523712dc 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -143,6 +143,15 @@ export function classifyError(status: number, type: string, message: string): Oc if (type === CYBER_POLICY_ERROR_CODE || isCyberPolicyMessage(text)) { return { message, type: "invalid_request_error", code: CYBER_POLICY_ERROR_CODE }; } + // A LOCAL preflight refusal keeps its own code (#1524). The message necessarily says + // "context window" -- that is what it is refusing on -- so the generic remap below would + // rewrite it to `context_length_exceeded` and make it indistinguishable from an UPSTREAM + // verdict. The two need opposite fallback handling: ours means "this candidate does not + // fit", theirs means "the request is impossible", so collapsing them ended the chain at + // the first candidate that was merely too small. + if (type === "input_admission_refused") { + return { message, type: "invalid_request_error", code: "input_admission_refused" }; + } if ( text.includes("context_length_exceeded") || text.includes("context window") || diff --git a/tests/combos.test.ts b/tests/combos.test.ts index 899118e961..70bf3a657b 100644 --- a/tests/combos.test.ts +++ b/tests/combos.test.ts @@ -360,7 +360,12 @@ describe("combo failure policy and advancement", () => { // #1524: a LOCAL input-admission refusal means "this candidate cannot fit the request", // not "the request is impossible". The next candidate may have a larger context window, // so the chain must continue instead of ending at the first incompatible target. - expect(comboFailureDecision(413, '{"code":"input_admission_refused"}')).toBe("hop"); + // + // The decision keys on the STRUCTURED code, which the proxy now preserves through + // classifyError. Matching raw text instead would let any upstream override a terminal + // verdict by echoing the token, so that shape must NOT hop. + expect(comboFailureDecision(413, 'refused', { code: 'input_admission_refused' })).toBe('hop'); + expect(comboFailureDecision(400, 'upstream mentions input_admission_refused in prose')).toBe('stop'); // An UPSTREAM context verdict still stops: retrying that elsewhere is guesswork, and a // generic 413 with no structured code keeps its existing conservative handling. expect(comboFailureDecision(400, "context_length_exceeded")).toBe("stop"); diff --git a/tests/routing-policy-fallback.test.ts b/tests/routing-policy-fallback.test.ts index 6a73550265..ac2fc34f28 100644 --- a/tests/routing-policy-fallback.test.ts +++ b/tests/routing-policy-fallback.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; +import { formatErrorResponse } from "../src/bridge"; import { RequestPacingQueueOverloadError } from "../src/providers/request-pacing"; import type { OcxConfig } from "../src/types"; import { beginRequestAttempt, type RequestLogContext } from "../src/server/request-log"; @@ -67,9 +68,15 @@ describe("policy candidate fallback", () => { ctx.routeDecision = trace; seedAttempt(ctx, "provider", String(body.model)); if (seenModels.length === 1) { - return Response.json( - { error: { message: "Estimated input is far past the context window", type: "input_admission_refused", code: "input_admission_refused" } }, - { status: 413 }, + // Built by the PRODUCTION emitter, not by hand. A hand-written envelope hid the real + // defect: formatErrorResponse runs classifyError, whose "context window" remap rewrote + // our own code to context_length_exceeded, so the shape the proxy actually ships never + // carried the marker the hop rule looks for. + return formatErrorResponse( + 413, + "input_admission_refused", + "Estimated input (~500000 tokens) is far past the context window of test-model (100000 tokens)." + + " Start a new session or choose a model with a larger context window.", ); } return Response.json({ id: "resp", object: "response", status: "completed", output: [] }); @@ -81,6 +88,29 @@ describe("policy candidate fallback", () => { expect(seenModels).toEqual(["policy/daily", "provider-b/model-b"]); }); + test("an upstream body that merely echoes the marker does not hop (#1524)", async () => { + // The refusal is ours and always carries the structured code, so the decision keys on that + // alone. Error text is provider-controlled and crosses a trust boundary: matching on it + // would let any upstream override a terminal verdict by mentioning the token. + const trace = policyTrace(); + const logCtx = { requestedModel: "policy/daily", routeDecision: trace, attempts: [] } as unknown as RequestLogContext; + const seenModels: string[] = []; + const runCore = async (req: Request, _config: OcxConfig, ctx: RequestLogContext) => { + const body = await req.clone().json() as { model?: string }; + seenModels.push(String(body.model)); + ctx.routeDecision = trace; + seedAttempt(ctx, "provider", String(body.model)); + return Response.json( + { error: { message: "upstream says: input_admission_refused is not a thing here", type: "invalid_request_error", code: "invalid_request_error" } }, + { status: 400 }, + ); + }; + + const response = await handleResponsesWithPolicyFallback(request(), {} as OcxConfig, logCtx, {}, { runCore }); + + expect(response.status).toBe(400); + expect(seenModels).toEqual(["policy/daily"]); + }); test("an upstream context_length_exceeded still stops the chain (#1524)", async () => { // The mirror-image contract. An upstream verdict is about the REQUEST, so retrying it // elsewhere is guesswork -- and hopping would burn every candidate on a doomed request. From 7551986835060956a31e666e83d88475e916054d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 03:16:48 +0900 Subject: [PATCH 3/4] docs(devlog): record why the first #1524 fix did not work, and state the hop rule's real provenance Two audit rounds corrected this unit, and the correction is more useful than the conclusion. The earlier "Implementation outcome" section claimed the context-window half of #1524 was closed by the input_admission_refused hop code. It was not: core.ts emits that refusal through formatErrorResponse, which runs classifyError, and the message necessarily says "context window" because that is what it refuses on. The generic remap rewrote our own code to context_length_exceeded, so the shipped envelope carried no admission marker. Reordering comboFailureDecision was necessary but changed nothing alone. That survived review because the test fixture hand-built an envelope the proxy does not emit -- a green test proving a shape that never reaches production. Also records the reviewer's provenance caveat: calling this a "local" code overstates it. Policy fallback reads error.code from any response and combo reads the upstream nested code, so an upstream that deliberately emits the code induces a hop. Bounded rather than dangerous -- upstreams already control 429 and 5xx, and traversal is finite via the tried-set and combo exclusions -- so the accurate description is structured-code-only, not provably local. The code comment in failover.ts now says exactly that. Line references refreshed to bc6019bae. --- .../090_1524_capability_preflight.md | 40 +++++++++++++++++++ src/combos/failover.ts | 12 ++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md b/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md index 9502ccc5a5..f058f54c7a 100644 --- a/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md +++ b/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md @@ -106,3 +106,43 @@ This is an optimization of an already-correct chain, not the reported defect. #1 acceptance behavior ("reject candidates that cannot accept the request before retrying") holds today for both context and modality. + +### Audit round r1 correction (at `bc6019bae`) + +The section above was written at `812e7c40b`, BEFORE the real defect was found, and an adversarial +pre-merge audit showed it was describing a fix that did not work. Recorded here rather than +rewritten, because the mistake is the useful part. + +**What the earlier section got wrong.** It claimed the context-window half of #1524 was closed by +the `input_admission_refused` hop code. It was not: `core.ts` emits that refusal through +`formatErrorResponse`, which runs `classifyError` — and the message necessarily contains +"context window", because that is what it is refusing on. The generic remap rewrote our own code +to `context_length_exceeded`, so the envelope the proxy actually shipped carried no admission +marker at all. Reordering `comboFailureDecision` was necessary but changed nothing on its own. + +The reason this survived earlier review is worth naming: the test fixture hand-built an envelope +the proxy does not emit. A green test proved a shape that never reaches production. + +**What now holds, verified by running the production emitter rather than reading it:** + +- `src/lib/errors.ts:152` — `classifyError` returns `input_admission_refused` when that is the + type it was given, placed before the context-window remap. +- `src/combos/failover.ts:135` — the hop rule, matched on the structured code only and tested + before the generic stop list at `:138`. +- `src/server/responses/core.ts:2003` — the emitter. +- `tests/routing-policy-fallback.test.ts` — the fixture calls `formatErrorResponse` itself, plus a + negative case pinning that an upstream merely mentioning the token does not hop. + +Both factors are independently load-bearing: disabling the `classifyError` branch fails the +regression, and restoring the original ordering fails it too. Neither alone is sufficient, which +is exactly why the first single-factor ablation was misleading — it passed because the removed +`message.includes` arm caught the case. + +**Provenance caveat, from the reviewer.** Calling this a "local" code slightly overstates it. +Policy fallback reads `error.code` from any response (`src/server/responses/policy-fallback.ts:61`) +and combo reads the upstream nested code, so an upstream that deliberately emits +`input_admission_refused` induces a hop. That is bounded rather than dangerous: upstreams already +control other hop signals (429, 5xx), and traversal is finite — policy tries each candidate once +via `tried`, combo excludes each attempted target. The accurate description is +**structured-code-only**, not provably local, and the code comment says so. + diff --git a/src/combos/failover.ts b/src/combos/failover.ts index a8cf18175f..dc3578d0a3 100644 --- a/src/combos/failover.ts +++ b/src/combos/failover.ts @@ -129,9 +129,15 @@ export function comboFailureDecision( // so checking the stop list first swallowed the signal and ended the chain. An UPSTREAM // `context_length_exceeded` carries no admission code and still falls through to stop. // - // Matched on the STRUCTURED code only. The refusal is ours, so it always carries the code - // now that classifyError preserves it; a raw substring test would instead let any upstream - // override a terminal verdict by echoing the token in text we do not control. + // Matched on the STRUCTURED code only, which classifyError now preserves for our own + // refusal. A raw substring test would additionally let any upstream override a terminal + // verdict by echoing the token in prose we do not control. + // + // Precise about what this is NOT: an upstream can still SET this code deliberately, since + // both extractors read the upstream error object. That is bounded rather than dangerous -- + // an upstream already controls other hop signals (429, 5xx), and traversal is finite: policy + // tries each candidate once via `tried`, and combo excludes each attempted target. So this is + // structured-code-only, not provably local. if (options?.code === "input_admission_refused" || error.code === "input_admission_refused") { return "hop"; } From 3e1e028219e8b7a37630e563d25bc6537ba8abf7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 03:19:04 +0900 Subject: [PATCH 4/4] docs(devlog): correct the hop-rule line references after the comment expansion --- .../260816_wave34_closeout/090_1524_capability_preflight.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md b/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md index f058f54c7a..edd165a885 100644 --- a/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md +++ b/devlog/_plan/260816_wave34_closeout/090_1524_capability_preflight.md @@ -127,8 +127,8 @@ the proxy does not emit. A green test proved a shape that never reaches producti - `src/lib/errors.ts:152` — `classifyError` returns `input_admission_refused` when that is the type it was given, placed before the context-window remap. -- `src/combos/failover.ts:135` — the hop rule, matched on the structured code only and tested - before the generic stop list at `:138`. +- `src/combos/failover.ts:141` — the hop rule, matched on the structured code only and tested + before the generic stop list at `:144`. - `src/server/responses/core.ts:2003` — the emitter. - `tests/routing-policy-fallback.test.ts` — the fixture calls `formatErrorResponse` itself, plus a negative case pinning that an upstream merely mentioning the token does not hop.