From d8e98d4fe16ac550a0e83469fffe9aa3644b2202 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 02:04:03 +0900 Subject: [PATCH 01/25] fix(subagents): keep a saved roster slot listed when its model is disabled GET /api/subagent-models built `available` purely from currently-pickable models, so a featured model disabled elsewhere vanished from it. The dashboard filters `chosen` against `available` and then PUTs exactly the rows it holds, which turned a hide into a delete: the next Save wrote the truncated roster to config.json, and the user read it as "ocx service lost my subagent models". Retain a chosen id in `available` when it is not otherwise selectable, appended after the selectable set and deduplicated. Models that are disabled and NOT in the roster stay excluded, so the picker behavior is unchanged for every model the user has not deliberately featured. The combo test asserted the old truncating behavior; it now asserts retention while a roster slot is held, and full exclusion once the slot is released. Closes #2133 --- .../management/agent-settings-routes.ts | 18 ++++- tests/combo-management-api.test.ts | 16 +++- tests/subagent-roster-retention.test.ts | 74 +++++++++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 tests/subagent-roster-retention.test.ts diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index 4b3e7a1715..9703ca90ef 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -613,15 +613,29 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise stored === catalogModelSlug(m) || slugEquals(stored, m.provider, m.id) )) .map(catalogModelSlug))]; - const available = [ + const chosen = config.subagentModels ?? []; + const selectable = [ ...listCatalogNativeSlugs().filter(ns => !disabled.has(ns)), ...visibleRouted, ]; + // A saved roster slot must stay representable even after its model is disabled + // elsewhere (Models page, provider allowlist, a provider row going away). The + // dashboard treats `available` as the set of rows it can render, so a chosen id + // missing from it disappears from the roster UI and the next Save — which PUTs + // exactly what the UI holds — silently truncates the persisted list. Losing a + // deliberate 5-model roster to an unrelated visibility toggle is data loss, not a + // filter. Same reasoning as `fetchGrokCandidateModels`, which deliberately lists a + // model the user already excluded so its switch remains reachable. + const selectableSet = new Set(selectable); + const available = [ + ...selectable, + ...[...new Set(chosen)].filter(model => !selectableSet.has(model)), + ]; // #857: let CLI/GUI show when a running Codex app-server keeps an older // in-memory catalog than the one on disk. const { collectCodexAppServerCatalogState } = await import("../../codex/app-server-processes"); const catalogState = collectCodexAppServerCatalogState(); - return jsonResponse({ chosen: config.subagentModels ?? [], available, catalogState }); + return jsonResponse({ chosen, available, catalogState }); } if (url.pathname === "/api/subagent-models" && req.method === "PUT") { let body: { models?: unknown }; diff --git a/tests/combo-management-api.test.ts b/tests/combo-management-api.test.ts index 75c19f8916..49ec7b53b6 100644 --- a/tests/combo-management-api.test.ts +++ b/tests/combo-management-api.test.ts @@ -655,10 +655,22 @@ describe("combo management API", () => { expect(body.available.filter(model => model === "deepseek-v4-flash")).toHaveLength(1); expect(body.available).not.toContain("combo/free"); + // Disabling an alias hides it from the pickable set, but NOT while it still holds a + // saved roster slot: the dashboard PUTs exactly the rows it can render, so dropping a + // chosen id here silently truncates the persisted roster on the next Save. Covered by + // tests/subagent-roster-retention.test.ts. config.disabledModels = ["deepseek-v4-flash"]; const disabledResponse = await comboApi(config, "GET", "/api/subagent-models"); - const disabledBody = await disabledResponse!.json() as { available: string[] }; - expect(disabledBody.available).not.toContain("deepseek-v4-flash"); + const disabledBody = await disabledResponse!.json() as { chosen: string[]; available: string[] }; + expect(disabledBody.chosen).toEqual(["deepseek-v4-flash"]); + expect(disabledBody.available).toContain("deepseek-v4-flash"); + expect(disabledBody.available.filter(model => model === "deepseek-v4-flash")).toHaveLength(1); + + // Once it no longer occupies a roster slot, the disable takes full effect. + config.subagentModels = []; + const unfeaturedResponse = await comboApi(config, "GET", "/api/subagent-models"); + const unfeaturedBody = await unfeaturedResponse!.json() as { available: string[] }; + expect(unfeaturedBody.available).not.toContain("deepseek-v4-flash"); }, 15_000); test("GET models round-trips a disabled combo alias for the Models GUI", async () => { diff --git a/tests/subagent-roster-retention.test.ts b/tests/subagent-roster-retention.test.ts new file mode 100644 index 0000000000..ad2245fc56 --- /dev/null +++ b/tests/subagent-roster-retention.test.ts @@ -0,0 +1,74 @@ +/** + * A saved subagent roster must survive an unrelated model-visibility change. + * + * The dashboard renders the roster from the reported available list and PUTs exactly what it + * holds, so a + * chosen id that GET omits is not merely hidden: the next Save writes the truncated list + * back to config.json. Disabling a model on the Models page, narrowing a provider + * allowlist, or removing a provider therefore used to silently shrink a deliberate + * 5-model roster. + */ +import { describe, expect, test } from "bun:test"; +import { handleManagementAPI } from "../src/server/management-api"; +import { ManagementRequest as Request } from "./helpers/management-auth"; +import type { OcxConfig } from "../src/types"; + +function makeConfig(overrides: Partial = {}): OcxConfig { + return { port: 10100, providers: {}, defaultProvider: "openai", ...overrides } as OcxConfig; +} + +async function getRoster(config: OcxConfig): Promise<{ chosen: string[]; available: string[] }> { + const res = await handleManagementAPI( + new Request("http://localhost/api/subagent-models"), + new URL("http://localhost/api/subagent-models"), + config, + ); + expect(res).not.toBeNull(); + return await res!.json() as { chosen: string[]; available: string[] }; +} + +describe("/api/subagent-models roster retention", () => { + test("a chosen model disabled elsewhere stays listed in available", async () => { + const chosen = ["gpt-5.5", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.4-mini"]; + const config = makeConfig({ + subagentModels: [...chosen], + disabledModels: ["gpt-5.5", "gpt-5.4-mini"], + }); + + const roster = await getRoster(config); + + // The saved list itself is untouched. + expect(roster.chosen).toEqual(chosen); + // And every slot the user saved can still be rendered, so a Save round-trip + // cannot truncate the roster to the models that happen to be enabled today. + for (const model of chosen) expect(roster.available).toContain(model); + }); + + test("retained roster entries are appended once, after the selectable models", async () => { + const config = makeConfig({ + subagentModels: ["gpt-5.5", "gpt-5.5", "gpt-5.6-terra"], + disabledModels: ["gpt-5.5"], + }); + + const { available } = await getRoster(config); + + // A duplicate saved id must not produce a duplicate row. + expect(available.filter(model => model === "gpt-5.5").length).toBe(1); + // An enabled chosen model is already selectable and must not be re-appended. + expect(available.filter(model => model === "gpt-5.6-terra").length).toBe(1); + // Retained-but-disabled entries sort after everything still selectable. + expect(available.indexOf("gpt-5.5")).toBeGreaterThan(available.indexOf("gpt-5.6-terra")); + }); + + test("a disabled model that is NOT in the roster stays out of available", async () => { + const config = makeConfig({ + subagentModels: ["gpt-5.6-terra"], + disabledModels: ["gpt-5.6-sol"], + }); + + const { available } = await getRoster(config); + + expect(available).not.toContain("gpt-5.6-sol"); + expect(available).toContain("gpt-5.6-terra"); + }); +}); From 75ad3787b3bdc0cc3e997c5d8e3092f265556cb1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 02:23:00 +0900 Subject: [PATCH 02/25] docs(devlog): plan the bug-PR backlog consolidation as one stack plus siblings --- .../000_research_inventory.md | 133 ++++++++++++++++++ .../010_layer1_bearer_admission_2132.md | 58 ++++++++ .../020_layer2_responses_id_backfill_2131.md | 43 ++++++ .../030_sibling_prompt_cache_retention.md | 38 +++++ .../040_sibling_routing_capability.md | 31 ++++ .../050_sibling_k12_short_window.md | 25 ++++ .../060_supersede_and_close_operations.md | 37 +++++ 7 files changed, 365 insertions(+) create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md new file mode 100644 index 0000000000..2959c3b9ea --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -0,0 +1,133 @@ +# 000 — Research: open bug-PR backlog inventory, rubric, and disposition + +Unit: 260820_bug_pr_backlog_consolidation +Work-phase: wp1 (docs-only roadmap cycle, LOOP-DOCS-FIRST-01) +Baseline: origin/dev = ceac592d7. Worktree branch codex/fix-subagent-roster-truncation (PR #2134). + +Evidence for every claim below came from six read-only xai/grok-4.6 investigation lanes that +read the actual PR diffs with `gh pr diff` and cross-read the runtime in this worktree. Code +edits stay in the main agent. + +## 1. Inventory + +27 open bug-labeled PRs; 25 authored by someone other than lidge-jun. 17 open bug issues. + +| PR | Author | Draft | Subsystem | Files | +|---|---|---|---|---| +| 2131 | bet4it | no | responses id backfill | server/responses | +| 2127 | agentHits | yes | antigravity thought_signature | adapters/google | +| 2115 | louis-tepe | no | adapter prompt nudge | adapters/* | +| 2110 | drakonkat | no | antigravity baseUrl override | providers/registry, lib/destination-policy | +| 2109 | drakonkat | no | anthropic baseUrl override | providers/registry, lib/destination-policy | +| 2105 | lilinxiong | no | claude shell hook | cli/index, server/system-env | +| 2104 | olddonkey | no | xai OAuth responses streaming | adapters/xai | +| 2102 | lilinxiong | no | gpt-5.6 prompt_cache_retention | adapters/openai-responses | +| 2101 | Ingwannu | no | account entitlement gating | codex/catalog | +| 2100 | ntdatt812 | no | routing capability evidence | routing/capability | +| 2099 | yzxcj797 | yes | gpt-5.6 prompt_cache_retention | adapters/openai-responses | +| 2091 | luvs01 | no | prompt_cache_retention (all forward) | adapters/openai-responses | +| 2082 | yzxcj797 | yes | AgentRouter language preamble | adapters | +| 2077 | ntdatt812 | no | lab behavior overrides | routing/compatibility/behavior | +| 2075 | olddonkey | no | Fast gate native chat (CONFLICTING) | adapters/openai-chat | +| 2067 | waw4303 | yes | opencode-free headers | providers/registry | +| 2063 | yzxcj797 | yes | K12 detail.code denials (CONFLICTING) | codex/quota-rejection | +| 2062 | yzxcj797 | yes | K12 short-window quota | codex/quota, codex/routing | +| 2056 | Ingwannu | no | K12 short-window quota | codex/quota, codex/routing | +| 2054 | keepitmello | yes | cursor checkpoints (CONFLICTING) | adapters/cursor | +| 2053 | Ingwannu | no | superseded OAuth commits | oauth/* | +| 2040 | Ingwannu | no | routed tool_search passthrough | server/responses | +| 2032 | yzxcj797 | yes | claude root bypass | cli/claude | +| 2029 | yzxcj797 | yes | probe session bus absent | service-manager-probe | +| 2027 | yzxcj797 | yes | opencode-go quota gating | providers/quota | + +## 2. Scoring rubric + +Score = severity (0-35) + blast radius (0-25) + evidence quality (0-20) + fix tractability (0-20). +Threshold for this campaign: **>= 60**. + +- severity: does it break a core path (routing, auth, streaming, config persistence) for a + default configuration, or is it peripheral/cosmetic? +- blast radius: how many users/configurations does the defect reach? +- evidence quality: deterministic reproduction with logs/curl, or assertion? +- fix tractability: is a correct, testable fix small and self-contained? + +## 3. Scores and disposition + +| Item | Score | Disposition | +|---|---|---| +| Issue #2132 bearer admission forces ChatGPT credential | 96 | ABSORB — no PR exists; highest-value gap in the backlog | +| Issue #2092 / PRs #2102,#2099,#2091 prompt_cache_retention | 86 | ABSORB #2102 as base; supersede #2099, #2091 | +| Issue #2114/#1939 / PR #2029 probe bus | 80 | SUPERSEDED by maintainer PR #2130 (already open) | +| PR #2131 responses output id backfill | 80 | ABSORB | +| PR #2100 routing capability evidence | 80 | ABSORB | +| PR #2047 / #2056 + #2062 K12 short-window quota | 72 | ABSORB #2056; supersede #2062 | +| PR #2053 superseded OAuth credential commits | 72 | KEEP — C4 auth, needs human security review (MAINTAINERS.md) | +| PRs #2109 + #2110 baseUrl override | 68 | HOLD — unresolved security gap, see §6 | +| PR #2101 account entitlement gating | 64 | KEEP — large (20 files), needs its own cycle | +| PR #2077 lab behavior overrides | 62 | ABSORB | +| PR #2040 routed tool_search passthrough | 62 | KEEP — 14 files, own cycle | +| PR #2105 claude shell hook | 60 | ABSORB | +| PR #2063 K12 detail.code | — | SUPERSEDED by already-merged #2055 | +| PR #2115 code mode nudge | 54 | BELOW THRESHOLD — contracts native-OpenAI detection; needs human adapter pass | +| PR #2082 AgentRouter language | 54 | BELOW THRESHOLD | +| PR #2027 opencode-go quota | 56 | BELOW THRESHOLD | +| PR #2067 opencode-free headers | 50 | BELOW THRESHOLD | +| PR #2054 cursor checkpoints | 46 | BELOW THRESHOLD — hypothesis pending wire trace | +| PR #2032 claude root bypass | 46 | BELOW THRESHOLD — maintainer already rejected the default | +| PR #2104, #2075, #2127 | n/a | Deferred: #2075 and #2054 are CONFLICTING; #2127 is an active draft by its author | + +## 4. Duplicate clusters (evidence-backed) + +**prompt_cache_retention (issue #2092).** #2102 gates on `forward && isCanonicalOpenAiForwardProvider` +and matches `gpt-5.6` / `gpt-5.6-*`. #2099 uses a looser `startsWith("gpt-5.6")` on ANY forward +provider and carries a stray package.json 2.24.2 -> 2.25.0 bump. #2091 strips the field for every +forward request and every model, which inverts the existing gpt-5.5 preserve pin at +tests/openai-responses-passthrough.test.ts:807 — the issue reporter explicitly withdrew the +global claim. #2102 is the correct contract. + +**K12 short-window quota (issue #2047).** #2056 is a strict superset of #2062: it adds +`snapshotHasShort`, partial-snapshot preservation, `updateAccountQuota` carry, and the +parse -> cache -> DTO path the issue requires. Both rewrite the same two functions and WOULD +conflict. #2062 also carries the same stray version bump. + +**Probe bus (issues #2114/#1939).** #2130's `busUnreachable()` is a superset of #2029's two +strings and adds the on-disk unit check that #2029's reviewer demanded. Landing #2029 on top of +#2130 would REGRESS the disk check back to unconditional `absent`. + +## 5. Structural finding: this backlog is not one stack + +DEV-STACK-01 permits stacking only when later parts consume earlier parts' output. Measured file +overlap across the absorb set: + +| Cluster | Files | +|---|---| +| PCR consolidation | src/adapters/openai-responses.ts | +| #2132 + #2131 | src/server/responses/core.ts (**shared**) | +| #2100 | src/routing/capability.ts | +| #2077 | src/routing/compatibility/behavior.ts | +| K12 | src/codex/quota.ts, src/codex/routing.ts | +| #2105 | src/cli/index.ts, src/server/system-env.ts | + +Exactly one real dependency edge exists: **#2132 and #2131 both modify +`src/server/responses/core.ts`**, so they must be ordered. Everything else is disjoint. + +Forcing 12 disjoint fixes into one 12-layer chain would violate DEV-STACK-01's independence +clause and the 2-4 depth guidance, and would impose a false merge order in which an unrelated +layer blocks every layer above it. The honest shape is therefore **one bounded stack rooted on +#2134 for the genuinely dependent Responses work, plus sibling PRs off dev for the disjoint +fixes**. That is recorded here rather than silently reshaped. + +## 6. Security holds (detail deliberately not recorded here) + +The baseUrl-override pair (#2109/#2110) has an unresolved gap already raised publicly in the +CodeRabbit thread on those PRs. Per AGENTS.md, pre-disclosure security reasoning does not go in +this public directory: the analysis lives in scratch only, and these PRs are HOLD, not absorb, +until a human security pass. #2053 is C4 OAuth and requires the security review MAINTAINERS.md +mandates; it is KEEP, not absorb. + +## 7. Attribution contract + +Every superseded PR gets (a) its author credited by @login in the superseding PR body, +(b) a courteous closing comment naming the replacement PR and what was carried over, +(c) no force-push and no edit to the contributor's own branch. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md new file mode 100644 index 0000000000..dd5da1b8af --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md @@ -0,0 +1,58 @@ +# 010 — Layer 1 (stack bottom): fix issue #2132, bearer admission must not force a ChatGPT credential + +Work-phase: wp2. Branch: `codex/fix-bearer-admission-2132`. Base: `codex/fix-subagent-roster-truncation` (PR #2134). +Absorbs: nothing (no PR exists). Closes: #2132. + +## Why this is the stack bottom + +It is the highest-scoring item in the backlog (96) and it shares `src/server/responses/core.ts` +with layer 2 (#2131). Layer 2 must be based on this, or the two edits to that file collide. + +## Defect + +Reported in #2132: after v2.23.0, a key-auth provider (Cloudflare/etc.) returns 401 +`No usable Codex main credential` when `~/.codex/auth.json` holds no ChatGPT token. Bearer +admission sets `substituteMainCredential` unconditionally, so a route that needs no ChatGPT +identity is still gated on one. + +## P-phase re-verification required (stale check) + +Before editing, confirm against the CURRENT tree — the lane read `dev`, not this branch: +1. `rg -n "substituteMainCredential" src/` — enumerate every producer and consumer. +2. Read `src/server/responses/core.ts`, `src/server/responses/compact.ts`, + `src/codex/auth-context.ts` and establish where the flag is set and where it is read. +3. Reproduce the admission decision in a unit context with a key-auth provider and an + auth.json containing no ChatGPT token. If the current code does NOT reproduce, stop and + amend this doc rather than writing a fix for a defect that is not there. + +## Intended change + +Make the substitution conditional on the resolved route actually requiring a native/ChatGPT +credential. A key-auth routed provider carries its own credential and must be admitted +without one. Exact call sites are fixed during the stale check above; the invariant is: +`substituteMainCredential` is set only when the route's credential source is the native +ChatGPT pool. + +Out of scope: changing what happens once a native route legitimately lacks a credential, +and any change to the pool/account selection itself. + +## Test plan (must fail RED first) + +New `tests/bearer-admission-key-auth.test.ts`: +1. key-auth routed provider + auth.json with NO ChatGPT token -> request is admitted (no 401). +2. native gpt route + no ChatGPT token -> still fails closed with the existing error. +3. key-auth provider + ChatGPT token present -> unchanged behavior (no regression). + +Drive the file against the unpatched tree first and record the failure output; a test that +passes before the fix does not prove anything. + +## Verification + +`bun run typecheck`; `bun test --isolate` on the new file plus the existing responses/auth +suites; full `bun test --isolate tests` before marking review-ready; `bun run privacy:scan`. + +## Standalone thesis (DEV-STACK-03) + +"A provider that carries its own key must not be gated on a ChatGPT credential." Builds and +passes its own tests at its own tip, independent of layer 2. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md new file mode 100644 index 0000000000..bb5c088378 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/020_layer2_responses_id_backfill_2131.md @@ -0,0 +1,43 @@ +# 020 — Layer 2: absorb PR #2131, backfill missing Responses output ids + +Work-phase: wp3. Branch: `codex/absorb-responses-id-backfill`. Base: `codex/fix-bearer-admission-2132` (layer 1). +Absorbs: **PR #2131 by @bet4it**. Closes: PR #2131 as superseded, with attribution. + +## Dependency edge (the only real one in this backlog) + +#2131 adds `src/server/responses/responses-field-backfill.ts` and calls it from +`src/server/responses/core.ts` — the same file layer 1 edits. This is why it stacks rather +than sitting beside layer 1. + +## Defect + +Strict decoders (grok-build) reject Responses output items that omit `id` on +`message` / `reasoning` / `function_call`. #1941 landed earlier but some relays still omit it. + +## Change to carry over + +@bet4it's implementation, preserved in substance: synthesize stable `msg_ocx_N` / `rs_ocx_N` / +`fc_ocx_N` ids keyed on `output_index`, never overwriting an id that is already present. + +## Correction to apply on top (audit finding, lane: quality) + +An invalid or missing `output_index` collapses to `0`, so two unindexed items can both become +`msg_ocx_0` — duplicate ids, which is the exact class of bug this fixes. Replace the +collapse-to-zero fallback with a monotonic per-response counter so synthesized ids are unique +even when `output_index` is absent or malformed. Add the regression test that pins it. + +Docs: the locale files in #2131 are uneven (EN/FR rewritten, JA/KO/ZH/TR only first sentence). +Carry only the EN change in this layer; locale parity is not this layer's thesis. + +## Test plan (must fail RED first) + +Carry @bet4it's tests (SSE `response.completed`, `output_item.done` via `output_index`, JSON +passthrough, preserve-existing-id, inherited `toString` type) and ADD: +- two items with missing `output_index` receive DISTINCT ids (fails on #2131 as written). + +## Verification + +Same gate as layer 1, plus explicit confirmation that layer 2's branch contains layer 1's +commit (`git log --oneline ..` shows only layer-2 commits) and that the PR +base ref names layer 1's branch. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md new file mode 100644 index 0000000000..fc66d105c9 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/030_sibling_prompt_cache_retention.md @@ -0,0 +1,38 @@ +# 030 — Sibling A: consolidate prompt_cache_retention (issue #2092) + +Work-phase: wp4. Branch: `codex/consolidate-prompt-cache-retention`. Base: **dev** (sibling, not a stack layer). +Absorbs: **PR #2102 by @lilinxiong** (base implementation). Supersedes: **#2099 by @yzxcj797**, **#2091 by @luvs01**. Closes #2092. + +## Why a sibling and not a layer + +It touches only `src/adapters/openai-responses.ts`, which no other absorbed item touches. It has +no dependency on layers 1-2, so stacking it would impose a false merge order (DEV-STACK-01). + +## Chosen contract + +@lilinxiong's #2102: strip `prompt_cache_retention` only when +`forward && isCanonicalOpenAiForwardProvider(provider)` AND the model is `gpt-5.6` or +`gpt-5.6-*`. This matches the issue's own correction — the reporter withdrew the "strip +everywhere" claim, and some non-5.6 deployments still honor the field. + +Rejected: #2091's blanket strip for every forward provider and every model (it inverts the +existing gpt-5.5 preserve pin at tests/openai-responses-passthrough.test.ts:807). +Rejected: #2099's `startsWith("gpt-5.6")`, which also matches `gpt-5.60`, and its stray +package.json 2.24.2 -> 2.25.0 bump. + +## Carried from the superseded PRs + +From @yzxcj797's #2099: the `Fixes #2092` issue link and the repro-shaped fixture +(`store:false`, streamed input array). From @luvs01's #2091: nothing — its key-auth preserve +case is already covered by #2102. + +## Tightening to apply + +Replace the string-prefix family match with the catalog/native-slug predicate if one exists +in the current tree (`rg -n "isGpt56NativeSlug|NATIVE_OPENAI_MODELS" src/`); otherwise keep +the exact `gpt-5.6` / `gpt-5.6-*` match and pin `gpt-5.60` as a NON-match in tests. + +## Test plan (must fail RED first) + +Carry #2102's tests; add `gpt-5.60` non-match; keep the gpt-5.5 preserve pin intact. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md new file mode 100644 index 0000000000..62f0016582 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/040_sibling_routing_capability.md @@ -0,0 +1,31 @@ +# 040 — Sibling B: routing capability + lab behavior evidence + +Work-phase: wp5. Branch: `codex/absorb-capability-evidence`. Base: **dev**. +Absorbs: **PR #2100 and PR #2077, both by @ntdatt812**. Closes both as superseded. + +## Why these two together, and why a sibling + +#2100 touches `src/routing/capability.ts`; #2077 touches +`src/routing/compatibility/behavior.ts`. Disjoint files, one author, one thesis: *model-keyed +lookups must use the same resolution rules the runtime uses*. Neither depends on layers 1-2. + +Note: #2077 is Lab-adjacent. Verify `tests/core-lab-boundary.test.ts` stays green — the file +already imports Lab types, so this must not newly puncture the boundary. + +## Defects + +#2100: bare map lookups made `gpt-oss:120b` inherit the provider-wide 8k window instead of the +`gpt-oss` family's 131072, and `noVisionModels` was ignored. +#2077: `map[modelId]` missed family/case overrides, and `constructor` resolved to +`Object.prototype.constructor`, making `jcsStringify` throw and silently dropping Lab subjects. + +## Change + +Route both through `modelRecordValue` / `isModelTextOnly` as @ntdatt812 wrote them. Prototype-id +safety (`constructor`, `toString`) is the load-bearing part; keep those tests verbatim. + +## Test plan + +Carry both test files. Confirm the exact-own maps (`modelPreferHostedTools`, +`modelOpenRouterRouting`) still do NOT family-spread. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md new file mode 100644 index 0000000000..524d543b5c --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/050_sibling_k12_short_window.md @@ -0,0 +1,25 @@ +# 050 — Sibling C: K12 short-window quota (issue #2047) + +Work-phase: wp6. Branch: `codex/absorb-k12-short-window`. Base: **dev**. +Absorbs: **PR #2056 by @Ingwannu**. Supersedes: **#2062 by @yzxcj797**. Closes #2047. + +## Chosen base + +#2056 is a strict superset of #2062: `snapshotHasShort`, partial-snapshot preservation, +`updateAccountQuota` carry, and the parse -> cache -> DTO path #2047 actually requires. #2062 +drops short on a later weekly/monthly partial snapshot and carries a stray version bump. + +## Blocker to fix before this can land (raised by the maintainer on both PRs) + +A short-only snapshot with `shortPercent: 0` scores `0` instead of `CODEX_UNKNOWN_USAGE_SCORE`, +so `pickLowestUsageAmong` prefers an account whose long windows are unverified. Fix: +include `shortPercent` in `computeCodexUsageScore` only when the plan's governing long window +is finite; otherwise return `CODEX_UNKNOWN_USAGE_SCORE`. Add the short-only regression. + +This blocker is why #2056 is absorbed-and-corrected rather than simply approved. + +## Also close + +**#2063 by @yzxcj797** — superseded by ALREADY-MERGED #2055 (`2648ffa87`), which classifies +`detail.code` with a stricter own-property lookup. Close with attribution; fold nothing. + diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md new file mode 100644 index 0000000000..ec55998dfc --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/060_supersede_and_close_operations.md @@ -0,0 +1,37 @@ +# 060 — Close-out: supersede operations and attribution + +Work-phase: wp7. No code. GitHub state only. + +## Ordering rule + +A PR is closed ONLY after its replacement exists and is pushed. Never close first. + +## Operations + +| Close | Author | Replaced by | Carried over | +|---|---|---|---| +| #2131 | @bet4it | layer 2 (020) | full implementation + tests, plus unique-id correction | +| #2099 | @yzxcj797 | sibling A (030) | issue link, repro fixture | +| #2091 | @luvs01 | sibling A (030) | nothing; contract deliberately narrower | +| #2100 | @ntdatt812 | sibling B (040) | full implementation + tests | +| #2077 | @ntdatt812 | sibling B (040) | full implementation + tests | +| #2102 | @lilinxiong | sibling A (030) | full implementation + tests (base) | +| #2062 | @yzxcj797 | sibling C (050) | nothing; #2056 supersedes | +| #2063 | @yzxcj797 | merged #2055 | nothing | +| #2056 | @Ingwannu | sibling C (050) | full implementation + scorer correction | +| #2029 | @yzxcj797 | maintainer PR #2130 | nothing; #2130 is a superset | + +## Comment template + +> Thanks for this, @ — closing as superseded by #, which carries from your +> patch. Your work is credited in that PR's description. + +## NOT closed, with reasons stated publicly + +- **#2109 / #2110** (@drakonkat): unresolved security gap in the override gate; needs a human + security pass (AGENTS.md security boundary). +- **#2053** (@Ingwannu): C4 OAuth surface; MAINTAINERS.md mandates security review. +- **#2101, #2040**: large (20 and 14 files); each needs its own PABCD cycle. +- **#2115, #2082, #2027, #2067, #2054, #2032**: below the 60 threshold. +- **#2104, #2075, #2127**: #2075/#2054 CONFLICTING; #2127 is an active draft by its author. + From 255890290b121f51ef1bd46adf49a789adac65a1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 02:37:11 +0900 Subject: [PATCH 03/25] =?UTF-8?q?docs(devlog):=20correct=20the=20stack=20p?= =?UTF-8?q?remise=20=E2=80=94=20the=20absorbed=20bug=20PRs=20are=20disjoin?= =?UTF-8?q?t?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../000_research_inventory.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md index 2959c3b9ea..5aec5c3a20 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -131,3 +131,82 @@ Every superseded PR gets (a) its author credited by @login in the superseding PR (b) a courteous closing comment naming the replacement PR and what was carried over, (c) no force-push and no edit to the contributor's own branch. + +--- + +# P-phase amendment (A-gate self-audit, 2026-08-20): the stack premise in §5 was WRONG + +The A-phase auditor lane produced nothing across three wait cycles, so it was retired +(DISPATCH-RETIRE-01) and the load-bearing claims were verified directly. Two of them failed. + +## Correction 1 — #2131 does NOT touch `src/server/responses/core.ts` + +`gh pr diff 2131 --name-only` returns `src/server/responses/responses-field-backfill.ts`, +its test, and eight docs locales. `core.ts` already imports that module on `dev` +(`src/server/responses/core.ts:6-7`, called at :3098-3099); #2131 only changes the module's +internals and signature. It never edits `core.ts`. + +#2132's fix lives in `resolveResponsesCodexAuth` (`core.ts:1082-1114`), a different region of +a file #2131 does not modify at all. + +**Therefore the single dependency edge claimed in §5 does not exist.** The corrected file map: + +| Item | Files | Overlap | +|---|---|---| +| #2132 | src/server/responses/core.ts (auth resolution) | none | +| #2131 | src/server/responses/responses-field-backfill.ts | none | +| #2102 | src/adapters/openai-responses.ts | none | +| #2100 | src/routing/capability.ts | none | +| #2077 | src/routing/compatibility/behavior.ts | none | +| #2056 | src/codex/quota.ts, src/codex/routing.ts | none | +| #2105 | src/cli/index.ts, src/server/system-env.ts | none | + +Every absorbed item is disjoint. **There is no dependency-ordered chain in this backlog at all.** + +## Consequence: this work must NOT be stacked + +DEV-STACK-01 forbids stacking independent parts: "the parts are independent — open parallel PRs +off trunk instead, since a stack imposes a false merge order." Building the requested chain +would mean any layer's review blocking every layer above it, for zero dependency benefit, and +would violate the same rule the request asked to follow. + +Docs 010 and 020 are therefore **superseded**: both become siblings based on `dev`, not layers. +PR #2134 remains its own independent PR. The stack rooted on #2134 is cancelled and the reason +is recorded here rather than the plan being quietly reshaped. + +**One exception preserved:** if two absorbed items ever do touch one file, they stack. None do. + +## Correction 2 — issue #2132 is confirmed present, with a sharper mechanism than 010 assumed + +Verified in this worktree: +- `core.ts:1088`: `const substituteMainCredential = options.admission?.source === "bearer";` + keys on HOW the caller authenticated, never on WHERE the request routes. +- `auth-context.ts:542-548`: with `ctx.kind === "main"` and that flag, a missing/dead stored + main token throws `CodexMainSubstitutionUnavailableError`. +- `core.ts:1148-1153`: that becomes the reported 401. +- The `authCtx = { kind: "main" }` fallback at `core.ts:1105` is taken whenever + `route.codexAccountMode` is unset — which is every non-`openai` provider. + +So a key-auth routed provider reaches `kind: "main"` + `substituteMainCredential: true` and +fails, exactly as reported. The defect is real and 010's fix direction stands; only its stack +position changes. + +## Correction 3 — supersede claims re-verified + +`gh pr view 2055`: `MERGED` at 2026-08-19T00:11:27Z, merge commit `2648ffa879edf93e`. #2063's +supersede stands. + +## Revised work-phase map + +| WP | Doc | Branch | Base | Content | +|---|---|---|---|---| +| wp2 | 010 | codex/fix-bearer-admission-2132 | dev | issue #2132 (score 96) | +| wp3 | 030 | codex/consolidate-prompt-cache-retention | dev | absorb #2102; supersede #2099, #2091 | +| wp4 | 040 | codex/absorb-capability-evidence | dev | absorb #2100, #2077 | +| wp5 | 050 | codex/absorb-k12-short-window | dev | absorb #2056; supersede #2062, #2063 | +| wp6 | 020 | codex/absorb-responses-id-backfill | dev | absorb #2131 + unique-id correction | +| wp7 | 060 | — | — | close-outs with attribution | + +Ordered by score, not by dependency, because no dependency exists. Each is independently +reviewable and independently mergeable, which is what DEV-STACK-01 actually asks for. + From d1db92cbf22e7546605439f87639742a7eca1b27 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 02:39:21 +0900 Subject: [PATCH 04/25] docs(devlog): record #2130 merged, so #2029 is superseded in fact --- .../000_research_inventory.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md index 5aec5c3a20..6867621ef2 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -210,3 +210,26 @@ supersede stands. Ordered by score, not by dependency, because no dependency exists. Each is independently reviewable and independently mergeable, which is what DEV-STACK-01 actually asks for. + +## Correction 4 — #2130 merged mid-cycle; #2029 is now superseded in fact, not in prospect + +`gh pr view 2130`: `MERGED` at 2026-08-19T17:25:26Z. The probe lane's verdict was conditional +("SUPERSEDED, once #2130 merges"); that condition is now satisfied. + +Consequence for `060`: **#2029 (@yzxcj797)** moves from a prospective close to an immediate one. +`dev` now carries `busUnreachable()` — a superset of #2029's two stderr strings — plus the +on-disk unit check that #2029's own reviewer demanded. Landing #2029 on top would REGRESS that +disk check back to an unconditional `absent`. Nothing from #2029 needs to be carried over; its +one unique behavior (keeping `DBUS_SESSION_BUS_ADDRESS not set` as `unknown`) is precisely what +the merged disk check replaces. + +This also removes #2130 from the open-bug-PR set: the fresh count at wp1 close is 26 open bug +PRs, of which exactly one (#2134) is lidge-jun's and 25 are not. + +## wp1 close-out evidence + +`gh pr list --repo lidge-jun/opencodex --state open --label bug --limit 100` at close: +26 total, mine = [2134], non-mine = 25. Every one of those 25 numbers appears in §1/§3 of this +document (verified by a grep loop over the list, exit 0). No open bug PR is left without a +disposition. + From 899c03d00865630417ab90fa46cf2d6859d44902 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 02:52:59 +0900 Subject: [PATCH 05/25] docs(devlog): adjudicate the late auditor FAIL verdict --- .../000_research_inventory.md | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md index 6867621ef2..1b68a3fe29 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -233,3 +233,57 @@ PRs, of which exactly one (#2134) is lidge-jun's and 25 are not. document (verified by a grep loop over the list, exit 0). No open bug PR is left without a disposition. + +--- + +# A-gate amendment 2 — retired auditor returned late with VERDICT: FAIL; findings adjudicated + +The adversarial lane retired under DISPATCH-RETIRE-01 (three empty wait cycles) delivered after +retirement. Its verdict is FAIL. It is adjudicated here rather than discarded, because a late +reviewer is still a reviewer. + +**Findings 1 and 2 — CONFIRMED, and already corrected.** It independently measured the same +`gh pr diff --name-only` evidence and reached the same conclusion as amendment 1: #2131 does not +touch `core.ts`, no dependency edge exists, and rooting a stack on #2134 (which only touches +`agent-settings-routes.ts`) is a second DEV-STACK-01 violation. Two independent measurements now +agree. Recorded as settled. + +**Finding 5 — CONFIRMED, and it is the sharpest catch.** Doc 010 said to gate substitution on +"the native ChatGPT **pool**". That would exclude `codexAccountMode: "direct"` and re-break +#1686, whose whole point is that Direct bearer admission is only safe *because* substitution +still runs. The implemented fix uses `route.codexAccountMode !== undefined`, which covers both +`pool` and `direct` and matches the issue reporter's own suggested gate. 010's prose is +superseded by this line; the code is correct. + +**Finding 3 — CONFIRMED and material.** Overlap was measured only inside the absorb set. Three +OTHER open PRs edit `src/server/responses/core.ts`: + +| PR | Overlap with the #2132 fix | +|---|---| +| #2104 (@olddonkey) | `src/server/responses/core.ts` — review-ready, MERGEABLE | +| #2101 (@Ingwannu) | `core.ts` + `compact.ts` + `auth-context.ts` — all three files this fix touches | +| #2040 (@Ingwannu) | `core.ts` | + +Verified by `gh pr diff --name-only`. The #2132 change is 5 lines across two files and does not +restructure either function, so a textual conflict is possible but small. This is a merge-order +hazard to state on the PR, not a reason to withhold the fix. #2104 is reclassified from `n/a` +to KEEP (review-ready, not a conflicting draft — the auditor is right that grouping it with +CONFLICTING #2075 and draft #2127 was an error, and its inventory row's "adapters/xai" file +attribution was wrong). + +**Finding 4 — CONFIRMED. #2105 would have been lost.** It is scored 60 ABSORB in §3, has no +decade doc, and appears in no row of 060. An above-threshold item with no execution path is +exactly how a contributor's work disappears without a close comment. Disposition corrected to +**KEEP — remains open**, because no replacement exists. It is not closed. + +**Nits accepted:** "strict superset" overstates #2056 vs #2062 (#2062 uniquely adds +`tests/rate-limit-reset-credits.test.ts`); #2130 has empty `closingIssuesReferences` so +#1939/#2114/#2108 will not auto-close; the rubric is recorded as a single integer, so the +component arithmetic is not independently auditable. + +## Net effect on the plan + +No absorbed item is dropped and no new one is added. Two dispositions change (#2104 n/a -> KEEP, +#2105 ABSORB -> KEEP), one prose invariant in 010 is superseded by the implemented predicate, +and one merge hazard is now stated. The sibling shape from amendment 1 stands, reinforced. + From a155fc9383f2945772de7516a51337274b9da677 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 03:05:44 +0900 Subject: [PATCH 06/25] docs(devlog): banner 010 so the shipped predicate cannot be reverted --- .../010_layer1_bearer_admission_2132.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md index dd5da1b8af..991c3bb62a 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md @@ -1,3 +1,15 @@ +> **SUPERSEDED IN PART — read `000_research_inventory.md` amendments 1 and 2 first.** +> +> Two things in this document are wrong and were corrected after it was written: +> +> 1. **It is NOT a stack layer and does NOT root on #2134.** No dependency edge exists; +> the shipped PR (#2137) is based on `dev` as a sibling. +> 2. **The substitution predicate is NOT "native ChatGPT pool".** Pool-only would exclude +> `codexAccountMode: "direct"` and re-break #1686, whose Direct admission is only safe +> BECAUSE substitution still runs. The shipped predicate is +> `route.codexAccountMode !== undefined`, covering pool AND direct. Do not "correct" it back. + + # 010 — Layer 1 (stack bottom): fix issue #2132, bearer admission must not force a ChatGPT credential Work-phase: wp2. Branch: `codex/fix-bearer-admission-2132`. Base: `codex/fix-subagent-roster-truncation` (PR #2134). From 831cd1c3496ca54e65950d27797d9b0cd7c98cb0 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 03:19:12 +0900 Subject: [PATCH 07/25] docs(devlog): log wp2 and wp3 shipped state --- .../070_execution_log.md | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md new file mode 100644 index 0000000000..134053e2ea --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -0,0 +1,62 @@ +# 070 — Execution log: what actually shipped + +Appended as work-phases close. This is the record of landed state, distinct from the plan. + +## wp2 — issue #2132 (score 96) + +**PR #2137**, branch `codex/fix-bearer-admission-2132`, base `dev`. + +`substituteMainCredential` was computed from how the caller authenticated and never from where +the request routes, so a key-authenticated provider was gated on a ChatGPT credential it cannot +use. The predicate is now +`options.admission?.source === "bearer" && route.codexAccountMode !== undefined` +at both `core.ts:1088` and `compact.ts:325`. + +It covers `pool` AND `direct`. Doc 010's "native ChatGPT pool" wording would have excluded +`direct` and re-broken #1686, whose Direct admission is only safe because substitution still +runs. 010 now carries a banner saying so. + +Evidence: `tests/bearer-admission-routed-provider.test.ts` driven RED (it reproduced the exact +reported 401), full suite 13516 pass / 0 fail, typecheck and privacy scan clean. Re-audit round 2 +by the same adversarial reviewer returned **VERDICT: PASS**. + +## wp3 — issue #2092 (score 86) + +**PR #2138**, branch `codex/consolidate-prompt-cache-retention`, base `dev`. + +Absorbs @lilinxiong's #2102 contract: strip `prompt_cache_retention` on canonical ChatGPT +forward for the `gpt-5.6` family only, with an exact-or-dashed-prefix match so a future +`gpt-5.60` is not swept up. The retired value is not translated into `prompt_cache_options`. + +Evidence: 5 of the new tests fail when only the adapter change is reverted; the two narrowness +guards stay green in both directions, which is what makes them guards rather than restatements. +Full suite 13537 pass / 0 fail. + +### Closed with attribution + +| PR | Author | Superseded by | Carried | +|---|---|---|---| +| #2102 | @lilinxiong | #2138 | the implementation itself | +| #2099 | @yzxcj797 | #2138 | issue link + repro fixture | +| #2091 | @luvs01 | #2138 | nothing; contract deliberately narrower | +| #2029 | @yzxcj797 | merged #2130 | nothing; #2130 adds the disk check review demanded | +| #2063 | @yzxcj797 | merged #2055 | nothing; #2055 is the stricter own-property lookup | + +Each carries a comment naming the replacement and the specific reason, so no contributor has to +guess why their work closed. + +## Still open by decision, not omission + +- #2109 / #2110 (@drakonkat) — unresolved security gap in the override gate; needs a human pass. +- #2053 (@Ingwannu) — C4 OAuth; MAINTAINERS.md mandates security review. +- #2105 (@lilinxiong) — above threshold but no replacement exists yet; closing it now would lose work. +- #2101, #2040 — 20 and 14 files; each needs its own cycle. +- #2104 (@olddonkey) — review-ready and MERGEABLE; reclassified out of the deferred bucket, it is a + KEEP that deserves review rather than supersession. + +## Remaining work-phases + +wp4 (#2100 + #2077 capability evidence), wp5 (#2056 K12 with the scorer correction), wp6 (#2131 +responses id backfill with the duplicate-id fix). Each is a sibling off `dev`; none depends on +another. + From 8b4f1a6a7978fe9ad93dd7bb0cf7762d3bdf9128 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 03:52:13 +0900 Subject: [PATCH 08/25] docs(devlog): log wp4 through wp6 and the campaign state --- .../070_execution_log.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index 134053e2ea..185711e293 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -60,3 +60,33 @@ wp4 (#2100 + #2077 capability evidence), wp5 (#2056 K12 with the scorer correcti responses id backfill with the duplicate-id fix). Each is a sibling off `dev`; none depends on another. + +## wp6 — PR #2131 (@bet4it) + +**PR #2142**, branch `codex/absorb-responses-id-backfill`, base `dev`. + +Carries @bet4it's implementation and tests, plus one correction: an absent or malformed +`output_index` collapsed to 0, so two such items both synthesized `msg_ocx_0` — duplicate ids, +the exact defect the backfill prevents. Unusable indices now take a monotonic ordinal based far +above any plausible real index. + +Evidence worth naming: applying ONLY @bet4it's original source and running the new suite gives +15 pass / 1 fail, and the single failure is the duplicate-id guard. That is what makes it a guard +rather than a restatement of behavior. + +The inherited assertion `expect(parsed.item.id).toBe("msg_ocx_0")` was replaced, not deleted +quietly, and the replacement is disclosed in the PR body. + +# Campaign state at wp6 close + +Superseded and closed with attribution: #2102, #2099, #2091, #2029, #2063, #2100, #2077, #2056, +#2062, #2131 — ten PRs, each with a comment naming its replacement and the specific reason. + +Opened: #2137 (#2132), #2138 (#2092), #2140 (#2100+#2077), #2141 (#2047), #2142 (#2131), plus +the pre-existing #2134. + +Deliberately still open: #2109/#2110 (security gap), #2053 (C4 OAuth review), #2105 (no +replacement written yet), #2101/#2040 (each needs its own cycle), #2104 (review-ready, deserves +review not supersession), and the below-threshold set (#2115, #2082, #2027, #2067, #2054, #2032, +#2075, #2127). + From 3c77c10a5fe669893942f0a933b92c4b83ad38d6 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 04:03:40 +0900 Subject: [PATCH 09/25] docs(devlog): log wp7 and close the absorb campaign --- .../070_execution_log.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index 185711e293..9e939cc158 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -90,3 +90,16 @@ replacement written yet), #2101/#2040 (each needs its own cycle), #2104 (review- review not supersession), and the below-threshold set (#2115, #2082, #2027, #2067, #2054, #2032, #2075, #2127). + +## wp7 — PR #2105 (@lilinxiong) + +**PR #2144**, branch `codex/absorb-claude-shell-hook-gate`, base `dev`. + +Implementation and tests carried unchanged. The one addition is a comment on +`reconcileShellHook` recording that "installed" is answered from the calling process's PATH, so +a service context with a stripped PATH can remove a hook an interactive shell would keep — the +reversible direction, and the one this reconcile wants. + +This closes the finding the auditor raised at #2105: it was scored ABSORB with no execution path +and would have been lost. It now has one. + From bb38c4d5b10dddaf2c24a96efa903a8c773c32a7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 04:08:15 +0900 Subject: [PATCH 10/25] docs(devlog): record CI state and the campaign end state --- .../070_execution_log.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index 9e939cc158..e43c15d894 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -103,3 +103,46 @@ reversible direction, and the one this reconcile wants. This closes the finding the auditor raised at #2105: it was scored ABSORB with no execution path and would have been lost. It now has one. + +# Campaign close — CI state and honest end state + +All six shipped PRs are green on exact head and MERGEABLE: + +| PR | Fixes | Checks | +|---|---|---| +| #2137 | issue #2132 | 25 pass / 0 fail | +| #2138 | issue #2092 (absorbs #2102) | 25 pass / 0 fail | +| #2140 | absorbs #2100 + #2077 | 25 pass / 0 fail | +| #2141 | issue #2047 (absorbs #2056) | 25 pass / 0 fail | +| #2142 | absorbs #2131 | 23 pass / 0 fail | +| #2144 | absorbs #2105 | 29 pass / 0 fail | + +#2140 first showed `npm-global-smoke` failing on windows-latest with +`EBUSY: resource busy or locked, unlink ...bun.exe` during dependency install — a Windows file +lock during Bun installation, not a defect in the routing change. Rerunning the failed jobs +turned it green, which is the evidence that it was infrastructure rather than the patch. + +## Eleven PRs closed with attribution + +#2102, #2099, #2091, #2029, #2063, #2100, #2077, #2056, #2062, #2131, #2105. + +Each carries a comment naming its replacement, what was carried over, and what was deliberately +not. Where a contributor's own assertion had to be replaced (#2056's `shortPercent: 0` scorer +case, #2131's `msg_ocx_0` collapse case), the replacement is disclosed in both the closing +comment and the superseding PR body rather than done silently. + +## Fourteen PRs deliberately still open + +- **Security holds:** #2109, #2110 (override gate), #2053 (C4 OAuth, MAINTAINERS.md review). +- **Own-cycle scale:** #2101 (20 files), #2040 (14 files). +- **Deserves review, not supersession:** #2104 — review-ready, MERGEABLE, and touching + `core.ts` alongside #2137. +- **Below the 60 threshold:** #2115, #2082, #2027, #2067, #2054, #2032, #2075, #2127. + +Nothing here is an omission. Every one is a recorded decision with a reason. + +## Merging + +Not done. DEV-STACK-04 and DEV-GIT-PUSH-01 both put merge authorization with the user, and +nothing in this campaign changes that. + From 5a77d63b3c108464cb1b1ec9f2dd6014e0194576 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 04:53:56 +0900 Subject: [PATCH 11/25] docs(devlog): log wp8 and wp9, including the one real stack layer --- .../070_execution_log.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index e43c15d894..3d8506c5fd 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -146,3 +146,35 @@ Nothing here is an omission. Every one is a recorded decision with a reason. Not done. DEV-STACK-04 and DEV-GIT-PUSH-01 both put merge authorization with the user, and nothing in this campaign changes that. + +## wp9 — PR #2101 (@Ingwannu): the ONE real stack layer + +**PR #2146**, branch `codex/absorb-account-entitlement-stacked`, base **`codex/fix-bearer-admission-2132`** (the #2137 branch), not `dev`. + +This is the single genuine dependency edge in the entire backlog. #2101 passes +`substituteMainCredentialForDirect: substituteMainCredential` into `resolveCodexAuthContext` — +the exact value #2137 corrects. Landing it on `dev` alone would silently reintroduce #2132 for +every routed provider. Everything else absorbed in this campaign was disjoint and shipped as a +sibling; this one is stacked because the code says so, not because a plan said so. + +Three corrections on top of @Ingwannu's work: + +1. **Selector compact bypassed the wire rewrite** — `accountGatedCompactWireModel` came from + `raw.model`, which never matches the gated map for `side/gpt-daybreak-blue-latest`, so a + selector-form compact still hit the native endpoint. Now derived from `route.modelId`. +2. **Direct callers evicted catalog evidence** — one 64-entry LRU shared between per-credential + Direct keys and the main/Pool keys the catalog projects from. Split into two eviction classes; + pinned by a test verified to fail against the shared LRU. +3. **Comment rot** — `native-models.ts` claimed routing never collapses Daybreak into + `gpt-5.6-sol`, which the wire normalization does. + +Evidence: full suite 13554 pass / 0 fail at the stacked tip; the composition check +(`codex-model-entitlements` + `bearer-admission-routed-provider` + `codex-auth-context` + +`server-auth`) is 146 pass / 0 fail, which is what proves the two layers agree. +Stack integrity: `git log parent..layer` shows exactly 1 commit, and a stack map was added to +#2137 so a reviewer arriving at the parent sees the chain. + +Two gaps named in the PR rather than carried silently: Direct `/v1/models` can still advertise a +Pool-only grant (advertisement only; dispatch still checks the caller credential), and +same-account gated-400 retry stays Pool-only. + From ef91d14a70e6d6e029db34b72a99adf000fc9659 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 05:35:42 +0900 Subject: [PATCH 12/25] docs(devlog): log wp10 through wp12, including both security holds resolved --- .../070_execution_log.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index 3d8506c5fd..b3ce89d97d 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -178,3 +178,24 @@ Two gaps named in the PR rather than carried silently: Direct `/v1/models` can s Pool-only grant (advertisement only; dispatch still checks the caller credential), and same-account gated-400 retry stays Pool-only. + +## wp12 — PR #2053 (@Ingwannu), the C4 OAuth hold + +**PR #2149**, branch `codex/absorb-oauth-superseded-commit`, base `dev`. + +Applied unchanged, rebased from 145 commits behind. The persist-boundary placement is the whole +design: `assertBeforePersist` runs inside the file lock, after `fn(store)` and before +`persist()`, so a superseded flow's in-memory mutation is discarded rather than written. +Ownership is identity-checked against the flow's own `AbortController`, not a timestamp. + +**This was a wp1 HOLD and it is resolved by shipping, not by absorbing quietly.** The PR states +plainly that MAINTAINERS.md mandates security review and asks that it not be merged on my +verification alone, and it names three residuals rather than letting the original claim stand: + +1. the description claimed reauth coverage; the diff wires the hook but adds no reauth test +2. `OAuthLoginSupersededError` is not in the public allowlist, so it projects to the generic string +3. a never-finishing Kiro rollback blocks all replacements, by design + +Evidence: reverting `src/oauth/` fails 2 tests including the cancel-then-replace round trip; +full suite 13536 pass / 0 fail. + From 2700b0e8ab52fe323528766ef4f255e67155abfe Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 06:02:54 +0900 Subject: [PATCH 13/25] docs(devlog): close the campaign with the final PR state and both scoring corrections --- .../070_execution_log.md | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md index b3ce89d97d..cd146a2c99 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -199,3 +199,95 @@ verification alone, and it names three residuals rather than letting the origina Evidence: reverting `src/oauth/` fails 2 tests including the cancel-then-replace round trip; full suite 13536 pass / 0 fail. + +## wp14 — PR #2075 (@olddonkey): the second false negative + +**PR #2151**, branch `codex/absorb-fastwire-native-chat`, base `dev`. Closes #1886. + +Same class of error as wp13, different cause. I saw `CONFLICTING` and treated it as a reason not +to read the diff. The rescore put it at **67**: native `/v1/chat/completions` decided +`service_tier` from `chatServiceTier` alone, so a `supportsServiceTier: false` declaration was +fail-open. The conflict was why it could not MERGE, not why it should score LOW — and resolving +it took one import line. + +Rebase, stated exactly: `src/adapters/openai-chat.ts` conflicted because `dev` added +`AdapterTierMetadata` while the PR adds `decideTier` and `ResolvedFastPolicy`. Both kept. +Everything else clean. Typecheck is what confirms the resolution. + +Evidence: reverting `src/` fails the characterization test the author had flipped from +documented-known-bug to passing assertion. Full suite 13552 pass / 0 fail. + +## Two scoring lessons, recorded together + +wp13 and wp14 were both my errors, from two different shortcuts: + +1. **Scoring from titles** — "preserve and replay thought signatures" reads like bookkeeping and + was a core provider 400. +2. **Reading merge state as value** — `CONFLICTING` says a patch cannot land today; it says + nothing about whether the defect matters. + +Both produce false negatives that are indistinguishable from correct low scores without opening +the diff. The rubric was fine; the inputs I fed it were not. + + +# Campaign close (final) + +## 13 PRs open, all green, all MERGEABLE + +| PR | Fixes | Credit | Base | +|---|---|---|---| +| #2137 | issue #2132 | new work | dev | +| #2138 | issue #2092 | @lilinxiong | dev | +| #2140 | #2100 + #2077 | @ntdatt812 | dev | +| #2141 | issue #2047 | @Ingwannu | dev | +| #2142 | #2131 | @bet4it | dev | +| #2144 | #2105 | @lilinxiong | dev | +| #2145 | issue #1950 | @Ingwannu | dev | +| #2146 | issue #2097 | @Ingwannu | **#2137 branch (stacked)** | +| #2147 | issue #1886 | @olddonkey | dev | +| #2148 | #2109 + #2110 | @drakonkat | dev | +| #2149 | #2053 | @Ingwannu | dev | +| #2150 | issue #2125 | @agentHits | dev | +| #2151 | issue #1886 | @olddonkey | dev | + +Plus #2134, which opened this session. + +## 16 PRs closed with attribution + +#2102, #2099, #2091, #2029, #2063, #2100, #2077, #2056, #2062, #2131, #2105, #2040, #2101, +#2104, #2109, #2110, #2053, #2127, #2075. + +Every one carries a comment naming its replacement, what was carried over, and what was +deliberately not. Where a contributor's own assertion had to be replaced — #2141's scorer case, +#2142's `msg_ocx_0` case — the replacement is disclosed in both the comment and the PR body. + +## 6 remain, independently verified below threshold + +#2115 (58), #2082 (46), #2067 (38), #2054 (58), #2032 (37), #2027 (51). + +These are not omissions. A rescore lane read every diff and scored them against the same rubric; +it found exactly two false negatives in my original triage (#2127 at 83, #2075 at 67) and both +were absorbed as wp13 and wp14. The remaining six are genuinely below the line, and four of them +are additionally blocked (draft, CONFLICTING, or CHANGES_REQUESTED). + +Two of them carry real bugs attached to unabsorbable patches: #2054's Cursor context collapse +(#1527) and #2027's Go quota gating (#1924). The right move for both is a clean reimplementation +on `dev`, not absorbing a 19-file conflicting draft. That is stated rather than silently skipped. + +## Corrections made on top of contributor work + +Eight PRs shipped with fixes the originals were missing, each pinned by a test verified to fail +against the contributor's own source: + +- #2141 short-only scorer returning 0 instead of UNKNOWN +- #2142 duplicate `msg_ocx_0` from a collapsed index +- #2145 history-only arming and non-atomic SSE overflow +- #2146 selector compact bypassing the wire rewrite, Direct callers evicting catalog cache +- #2148 `allowPrivateNetwork` bypassing the HTTPS gate for public hosts +- #2138 `gpt-5.60` near-miss match + +## Not merged + +DEV-STACK-04 and DEV-GIT-PUSH-01 both put merge authorization with the user. #2137 must land +before #2146. + From 4e806326709a012147aea6700adbbadf9e9ab93a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 08:14:07 +0900 Subject: [PATCH 14/25] docs(devlog): record the residual bug-PR dispositions and the wp15 outcome --- .../080_residual_dispositions.md | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md new file mode 100644 index 0000000000..e47d8aa35b --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md @@ -0,0 +1,166 @@ +# 080 — Residual dispositions: #2115, #2054, #2032, #2067, #2082, #2027, #2155 + +Unit: 260820_bug_pr_backlog_consolidation +Work-phases: wp15 (this doc's first three), wp16-wp19 (one PR each), wp20 (closeout). +Baseline: origin/dev; worktree branch `codex/fix-subagent-roster-truncation`. + +The wp1 rubric put six PRs below the 60 threshold and left them open. The user reviewed +that reasoning and issued explicit per-PR dispositions, plus one new arrival (#2155). This +doc records the dispositions and the evidence each one rests on. Evidence came from five +read-only gpt-5.6-sol investigation lanes that read every diff and the surrounding runtime. + +## wp15 — the three that need no new code + +### #2115 @louis-tepe — CLOSE + +The PR is titled as Code Mode edit guidance, but `src/adapters/openai-chat.ts:548` swaps the +local "hostname is exactly api.openai.com" test for a new `isCanonicalNativeOpenAIRoute` +predicate in `src/adapters/tool-catalog-nudge.ts:118-132`. That predicate is not +prompt-only. Through `messagesToChatFormat` (`openai-chat.ts:629`) it decides whether +developer messages stay as ordered `role: "developer"` entries or fold into the leading +system message; through `toolChoiceToChatFormat` (`:1232-1246`) it decides whether a single +required tool becomes a forced named function; and through `buildRequest` (`:1333`) it +decides native `reasoning_effort` versus gateway-style `reasoning`. One predicate change +therefore moves three wire semantics that have nothing to do with edit guidance. + +Second defect, independent of the first: `codeModeExecName` is withheld when a bare shell +bridge is present (`tool-catalog-nudge.ts:203-207`), but the new suffix injection checks +only `codeModeExecTool` (`:214-218`). A freeform `exec` sitting beside a top-level +`exec_command` still receives "targeted code edits" guidance even though the repository's +own predicate classifies that catalog as not Code Mode — a contract pinned at +`tests/tool-catalog-nudge.test.ts:114-123`. + +Blast radius if the guidance predicate is wrong: every routed Code Mode request on +Anthropic, Google/Vertex/Antigravity, Command Code, and every OpenAI-compatible host +without a literal `openai`/`chatgpt` DNS label — gateways, DeepSeek, Groq, Ollama, vLLM, +LM Studio, custom routes. Kiro (`src/adapters/kiro.ts:466-469`) picks up the reworded +generic sentence without being able to receive the suffix at all. + +The underlying request is legitimate. The implementation is not absorbable as-is because +the correct version is a narrower Code Mode seam that does not redefine native route +identity. + +### #2054 @keepitmello — STAY OPEN, probe requested + +The PR stores Cursor's returned `ConversationStateStructure` and replays it as the next +`AgentRunRequest.conversation_state` instead of rebuilding history every turn +(`src/adapters/cursor/protobuf-request.ts:823` on dev is the full-replay path). The +hypothesis is that full replay defeats Cursor's own checkpoint cache and produces the +large-context collapse in #1527. + +The PR proves the request construction changed — smaller `rootBytes` — and states plainly +that it did not reproduce the `kimi-k3` collapse or the 429. So the causal link is exactly +the thing still missing, and it is cheap for the author to capture: a matched three-turn +baseline-vs-head run at the issue's 75k-95k token shape, recording per turn the request +`conversation_id`, a digest and byte length of `conversation_state`, its +`root_prompt_messages_json` count, `turns` count, and `token_details.used_tokens`, against +the same fields on the response's `conversation_checkpoint_update`. The decisive comparison +is whether turn N+1's request state equals turn N's returned checkpoint while +`conversation_id` holds. `ocx debug provider on` / `ocx debug provider logs -f` +(`docs-site/src/content/docs/reference/cli/agents.md:102`, `src/lib/debug.ts:15`) already +carries the construction mode; exact state equality needs payload-free digests added +locally. + +Checkpoint reuse WITHOUT the collapse disappearing would refute the causal claim, which is +why the request is worth making rather than guessing. + +### #2032 @yzxcj797 — CLOSE + +This is a decision that was already made by a human, not a scoring call. The maintainer's +CHANGES_REQUESTED review says it directly: "Passing --dangerously-skip-permissions does not +create an OS sandbox" and "A viable revision needs a real sandboxed launch path, or it must +leave the vendor root guard intact." + +The diff injects `IS_SANDBOX=1` whenever the flag appears in argv (PR head +`src/cli/claude.ts:126-131`, `:329-331`) with no UID check and no sandbox establishment, +so it suppresses Claude Code's root guard while the child keeps ordinary root filesystem and +process access. The added tests (`tests/claude-cli.test.ts:271-295`) assert environment +assembly, not an isolation boundary. It also carries an unrelated `package.json` version +bump to 2.25.0. + +On dev, opencodex does not drop, refuse, or warn about the flag: `src/cli/dispatch.ts:500` +forwards trailing args and `src/cli/claude.ts:338` passes them through unchanged. The +refusal comes from Claude Code itself. A user who has genuinely isolated their environment +can already export `IS_SANDBOX=1`, because `buildClaudeEnv` starts from the caller's +environment (`src/cli/claude.ts:76`) and the docs promise exported variables win +(`docs-site/src/content/docs/guides/claude-code.md:56`). + +## wp15 outcome (executed) + +| PR | Action | Receipt | +|---|---|---| +| #2115 | CLOSED with reason | `issuecomment-5349122248`, state CLOSED | +| #2054 | comment only, left OPEN | `issuecomment-5349122708`, state OPEN | +| #2032 | CLOSED with reason | `issuecomment-5349122937`, state CLOSED | + +Verified by `gh pr view --json state` after the fact, not from the write's own exit code. + +## wp16 — #2067 @waw4303: ABSORB, and the reason is external corroboration + +The user's instruction was to check how **omniroute** — a separate open-source project +brokering free quota against the same upstream — builds these headers, then decide. That +turned out to be the decisive evidence, and it moved the answer. + +The PR head changed while the lane was reading it. The original commit `a5183abb` sent +`opencode-cli/1.0.0` / `cli` / `default`; the current head `6a79c42e` sends only +`User-Agent: opencode` alongside the existing `x-opencode-client: desktop`. + +omniroute (`diegosouzapw/OmniRoute`, commit `3d7ed7aa`, 2026-08-19) resolves the same +headers in `open-sse/executors/opencode.ts:408-448`: + +```ts +userAgent: process.env[envUAKey]?.trim() || process.env.OPENCODE_USER_AGENT?.trim() || "opencode", +client: process.env.OPENCODE_CLIENT?.trim() || "desktop", +project: process.env.OPENCODE_PROJECT?.trim() || "global", +``` + +It does not fetch or derive an installed CLI version at runtime. It falls back to a bare +unversioned `opencode`, preserves a real incoming `opencode-cli/` when one exists, +and lets an operator override via env. + +Three-way comparison: + +| Header | ours today | #2067 head | omniroute | +|---|---|---|---| +| `User-Agent` | absent (uncontrolled runtime default) | `opencode` | `opencode`, configurable, preserves real `opencode-cli/` | +| `x-opencode-client` | `desktop` | `desktop` | `desktop`, configurable | +| `x-opencode-project` | absent | absent | `global`, configurable | +| `x-opencode-request` | absent | absent | fresh UUID | +| `x-opencode-session` | absent | absent | conversation-derived or UUID | + +The important finding is the one that reverses a wp1 assumption. wp1 scored this 38 partly +because a pinned CLI version marker has a short shelf life — and that criticism was correct +against `a5183abb`. omniroute made the same mistake and then deliberately backed it out: +its July implementation (`234956dd`) used exactly `opencode-cli/1.0.0` / `cli` / `default`, +and PR #10571 replaced them with `opencode` / `desktop` / `global`. So the version pin is +not corroborated by an independent implementation; the *revised* values are, and by one that +arrived at them by retreating from the pin. + +That removes the "value with a short lifetime" objection entirely. What remains is a real +defect: we send no `User-Agent` at all today (`src/providers/registry.ts:2427`), so the +runtime default goes out uncontrolled, which is what the reporter's 429 is attributed to. + +Precedent for pinning a client fingerprint already exists here — Anthropic +(`src/adapters/anthropic.ts:936`, asserted at `tests/client-fingerprint.test.ts:120`), xAI +(`src/providers/xai-transport.ts:7`, `tests/xai-transport.test.ts:55`), Command Code with a +configurable fallback (`src/adapters/command-code.ts:482`). And the value stays +operator-overridable through the existing case-insensitive provider header override at +`src/server/management/provider-routes.ts:288`. + +Decision: **ABSORB the revised shape**, not the original. + +```ts +staticHeaders: { + "User-Agent": "opencode", + "x-opencode-client": "desktop", +} +``` + +Deliberately NOT copied from omniroute: `x-opencode-project`, `x-opencode-request`, +`x-opencode-session`. None is needed to fix the demonstrated failure, and adding a +conversation-derived session identifier is a privacy-relevant change that needs its own +evidence rather than a sibling project's precedent. + +## wp17-wp19 — the three that need new code + +Recorded here as each is decided; each is its own PABCD cycle. From 5e4ec149807963e6777330b2c355e64eddac8ab2 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 08:37:48 +0900 Subject: [PATCH 15/25] fix(providers): identify opencode-free with the client User-Agent it claims opencode-free sent no User-Agent, so Zen saw the bare runtime default (Bun/x.y.z) and rate-limited it harder than a client that identifies itself. Adds "User-Agent: opencode" alongside the existing x-opencode-client: desktop marker. The value is deliberately unversioned. OmniRoute, an independent open-source broker against the same Zen upstream, defaults to exactly this pair and reached it by retreating from its own earlier opencode-cli/1.0.0 pin: a pinned version is a claim about an install we do not have, and it goes stale on the vendor's schedule. The registry edit alone would have shipped to nobody. staticHeaders is documented as merged into every upstream request, but it was only ever copied at seed time, so any config written before a header existed -- or carrying any header of its own -- never received it. routedProviderConfig and buildModelsRequest now fill registry static headers beneath user headers, matched case-insensitively so an override replaces rather than duplicates: spreading "User-Agent" over a user's "user-agent" leaves both keys, which Headers serializes as one comma-joined value. Model discovery gets the same treatment because a provider identified as opencode when it completes but anonymous when it lists its own models reads as two different clients to a rate limiter. --- src/oauth/index.ts | 13 +++- src/providers/registry.ts | 40 ++++++++++++ src/router.ts | 7 ++ tests/management-provider-validation.test.ts | 13 +++- tests/opencode-free-provider.test.ts | 67 +++++++++++++++++++- 5 files changed, 134 insertions(+), 6 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 0162492f9a..fda4a3ec67 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -17,7 +17,7 @@ import { loginCommandCode, refreshCommandCodeToken } from "./command-code"; import { ANTIGRAVITY_REQUEST_UA } from "../adapters/google-antigravity-wire"; import { deriveOAuthDefaultModel, deriveOAuthProviderConfig } from "../providers/derive"; import { apiKeyPoolEntryId, sanitizeApiKeyValue } from "../providers/api-keys"; -import { effectiveGoogleMode, getProviderRegistryEntry, providerMatchesRegistryTransport } from "../providers/registry"; +import { effectiveGoogleMode, getProviderRegistryEntry, mergeRegistryStaticHeaders, providerMatchesRegistryTransport } from "../providers/registry"; import { resolveProviderModelDiscoveryUrl } from "../providers/model-discovery"; import { resolveProviderTransport } from "../providers/xai-transport"; import { detectClaudeCodeToken, detectGrokCliToken, hasComparableGrokIdentity, isSameGrokIdentity, shouldAdoptGrokGeneration } from "./local-token-detect"; @@ -828,7 +828,16 @@ export function buildModelsRequest( undefined, copilotApiBaseUrl, ); - const headers: Record = { ...(effectiveProvider.headers ?? {}) }; + // Model discovery is an upstream request like any other, so it carries the same registry + // static headers the inference path does. Without this a provider is identified correctly + // when it answers a completion but anonymously when it lists its own models, which is the + // kind of split fingerprint an upstream rate limiter reads as two different clients. + const registryStaticHeaders = providerMatchesRegistryTransport(providerName, effectiveProvider) + ? getProviderRegistryEntry(providerName)?.staticHeaders + : undefined; + const headers: Record = { + ...(mergeRegistryStaticHeaders(registryStaticHeaders, effectiveProvider.headers) ?? {}), + }; const discoveryUrl = (defaultUrl: string): string => resolveProviderModelDiscoveryUrl( providerName, prov, diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 53922e75ad..de20cfa969 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -2435,6 +2435,16 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ note: "No key needed — public desktop tier. OpenCode currently advertises about 200 Big Pickle/free-model requests per 5 hours. The same Zen gateway can also short-window rate-limit free models at roughly 15-20 requests/minute, and may return generic 429s without Retry-After (opencodex synthesizes backoff only when that header is omitted). Free models are discovered live from Zen. Data use: per OpenCode's Zen docs (https://opencode.ai/docs/zen/), prompts sent to free models may be retained and used for training/improvement — do not send confidential material through this provider.", dashboardUrl: "https://opencode.ai", staticHeaders: { + // Zen answers a bare runtime User-Agent (Bun/x.y.z) more aggressively than a client + // that identifies itself, which is what the 429 in #2067 traced to. The value is + // deliberately unversioned: a pinned "opencode-cli/" is a claim about an + // install we do not have and goes stale on the vendor's schedule, not ours. + // Corroboration, not authority: OmniRoute — an independent open-source broker against + // the same Zen upstream — defaults to exactly this pair (userAgent "opencode", client + // "desktop") in open-sse/executors/opencode.ts, and got there by RETREATING from its + // own earlier "opencode-cli/1.0.0" pin. An operator can still override either value + // through the provider headers API; user headers win case-insensitively at route time. + "User-Agent": "opencode", "x-opencode-client": "desktop", }, modelReasoningEfforts: Object.fromEntries(OPENCODE_FREE_DEEPSEEK_MODELS.map(id => [id, deepseekThinkingEffortsFor(id)])), @@ -2591,6 +2601,36 @@ export function getProviderRegistryEntry(id: string): ProviderRegistryEntry | un return PROVIDER_REGISTRY.find(entry => entry.id === id); } +/** + * Merge a registry row's `staticHeaders` beneath a provider's own headers. + * + * The field is documented as "merged into every upstream request for this provider", but that + * was only ever true for a freshly seeded config: `providerConfigSeed` copies the block once + * (`derive.ts`), `enrichProviderFromCatalog` fills it only when the whole block is absent, and + * nothing merged it at request time. So an install that predates a header — or that saved any + * header of its own — never received the new one, which is exactly what #2067 would have + * shipped for every existing opencode-free user. + * + * The comparison is case-insensitive on purpose. HTTP header names are case-insensitive, but a + * plain object spread is not: merging a registry `User-Agent` over a user's `user-agent` + * produces two entries that `Headers` serializes as one comma-joined value + * ("opencode, custom-agent"), which is a corrupted request rather than an override. The user's + * spelling and value both win; the registry only fills names the user has not spoken for. + */ +export function mergeRegistryStaticHeaders( + staticHeaders: Record | undefined, + userHeaders: Record | undefined, +): Record | undefined { + if (!staticHeaders) return userHeaders; + if (!userHeaders) return { ...staticHeaders }; + const claimed = new Set(Object.keys(userHeaders).map(name => name.toLowerCase())); + const merged: Record = { ...userHeaders }; + for (const [name, value] of Object.entries(staticHeaders)) { + if (!claimed.has(name.toLowerCase())) merged[name] = value; + } + return merged; +} + /** Whether this registry row's per-model service-tier evidence applies to one configured target. */ export function registryModelServiceTierCapabilityApplies( entry: Pick, diff --git a/src/router.ts b/src/router.ts index 297795180e..d4839e24e0 100644 --- a/src/router.ts +++ b/src/router.ts @@ -14,6 +14,7 @@ import { assertProviderDestinationAllowed } from "./lib/destination-policy"; import { redactSecretString, redactUrlForLog } from "./lib/redact"; import { PROVIDER_REGISTRY, + mergeRegistryStaticHeaders, providerCodexAccountMode, registryModelServiceTierCapabilityApplies, } from "./providers/registry"; @@ -296,6 +297,11 @@ export function routedProviderConfig(providerName: string, provider: OcxProvider ? mergePositiveNumberCaps(registryEntry.modelContextWindows, provider.modelContextWindows) : mergeRecordFill(registryEntry.modelContextWindows, provider.modelContextWindows); const modelInputModalities = mergeRecordFill(registryEntry.modelInputModalities, provider.modelInputModalities); + // Registry static headers are documented as applying to every upstream request, so they are + // filled at resolve time rather than only at seed time: a config written before a header + // existed, or one carrying any header of its own, would otherwise never receive it. User + // headers win, matched case-insensitively so an override replaces rather than duplicates. + const headers = mergeRegistryStaticHeaders(registryEntry.staticHeaders, provider.headers); const modelMaxInputTokens = providerName === OPENAI_API_PROVIDER_ID ? mergePositiveNumberCaps(registryEntry.modelMaxInputTokens, provider.modelMaxInputTokens) : mergeRecordFill(registryEntry.modelMaxInputTokens, provider.modelMaxInputTokens); @@ -372,6 +378,7 @@ export function routedProviderConfig(providerName: string, provider: OcxProvider authMode: canonicalAuthMode, apiKey: resolvedApiKey, ...(staticModelCatalog ? { liveModels: false } : {}), + ...(headers ? { headers } : {}), // Backfill the Google wire mode + Vertex project/location from the registry when the user // config omits them, so a minimal `google-vertex`/`google-antigravity` entry still routes // through the correct branch (CCA/Vertex) instead of falling back to AI Studio. diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index fd79c750e9..600c040f41 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -2888,11 +2888,18 @@ describe("provider management validation", () => { }; // Clearing user-managed headers must not delete the registry-owned static - // metadata (opencode-free's x-opencode-client marker) the transport relies on. + // metadata (opencode-free's User-Agent and x-opencode-client markers) the transport + // relies on. expect((await patch("opencode-free", { headers: null }))?.status).toBe(200); - expect(liveConfig.providers["opencode-free"].headers).toEqual({ "x-opencode-client": "desktop" }); + expect(liveConfig.providers["opencode-free"].headers).toEqual({ + "User-Agent": "opencode", + "x-opencode-client": "desktop", + }); const saved = JSON.parse(readFileSync(join(TEST_DIR, "config.json"), "utf8")) as OcxConfig; - expect(saved.providers["opencode-free"]?.headers).toEqual({ "x-opencode-client": "desktop" }); + expect(saved.providers["opencode-free"]?.headers).toEqual({ + "User-Agent": "opencode", + "x-opencode-client": "desktop", + }); }); test("concurrent provider PATCHes merge different headers", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); diff --git a/tests/opencode-free-provider.test.ts b/tests/opencode-free-provider.test.ts index 097fd98ef9..0db4aafb3e 100644 --- a/tests/opencode-free-provider.test.ts +++ b/tests/opencode-free-provider.test.ts @@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test"; import { PROVIDER_REGISTRY } from "../src/providers/registry"; import { providerConfigSeed, deriveKeyLoginMap, deriveFeaturedProviderIds } from "../src/providers/derive"; import { createOpenAIChatAdapter } from "../src/adapters/openai-chat"; +import { routedProviderConfig } from "../src/router"; +import { buildModelsRequest } from "../src/oauth"; import type { OcxParsedRequest, OcxProviderConfig } from "../src/types"; function minimalRequest(model = "kimi-k2.7-code"): OcxParsedRequest { @@ -27,14 +29,16 @@ describe("opencode-free provider", () => { expect(entry?.models).toBeUndefined(); }); - test("static headers include only the public client marker", () => { + test("static headers include only the public client markers", () => { expect(entry?.staticHeaders?.["Authorization"]).toBeUndefined(); + expect(entry?.staticHeaders?.["User-Agent"]).toBe("opencode"); expect(entry?.staticHeaders?.["x-opencode-client"]).toBe("desktop"); }); test("providerConfigSeed propagates static headers", () => { const seed = providerConfigSeed(entry!); expect(seed.headers?.["Authorization"]).toBeUndefined(); + expect(seed.headers?.["User-Agent"]).toBe("opencode"); expect(seed.headers?.["x-opencode-client"]).toBe("desktop"); expect(seed.keyOptional).toBe(true); expect(seed.liveModels).toBe(true); @@ -55,6 +59,7 @@ describe("opencode-free provider", () => { const req = adapter.buildRequest(minimalRequest()); const headers = req.headers as Record; expect(headers["Authorization"]).toBeUndefined(); + expect(headers["User-Agent"]).toBe("opencode"); expect(headers["x-opencode-client"]).toBe("desktop"); expect(req.url).toBe("https://opencode.ai/zen/v1/chat/completions"); }); @@ -84,6 +89,66 @@ describe("opencode-free provider", () => { expect(Object.keys(headers)).toContain("Authorization"); }); + // A seeded config is the easy case. The one that actually reaches users is a config written + // BEFORE a static header existed: it is on disk with the old header set (or with none at all, + // because the management API strips a block that exactly matches the registry), and nothing + // rewrites it. If the header only arrives at seed time, every existing install stays on the + // old fingerprint forever — which is what the original #2067 patch would have shipped. + describe("existing installs receive newly added static headers", () => { + const persisted = (headers?: Record): OcxProviderConfig => ({ + adapter: "openai-chat", + baseUrl: "https://opencode.ai/zen/v1", + keyOptional: true, + ...(headers ? { headers } : {}), + }); + + test("a config saved with no header block gains the full registry set", () => { + const routed = routedProviderConfig("opencode-free", persisted()); + expect(routed.headers?.["User-Agent"]).toBe("opencode"); + expect(routed.headers?.["x-opencode-client"]).toBe("desktop"); + }); + + test("a config saved with only the older marker gains the new one", () => { + const routed = routedProviderConfig("opencode-free", persisted({ "x-opencode-client": "desktop" })); + expect(routed.headers?.["User-Agent"]).toBe("opencode"); + expect(routed.headers?.["x-opencode-client"]).toBe("desktop"); + }); + + test("the merged headers reach the wire, not just the resolved config", () => { + const routed = routedProviderConfig("opencode-free", persisted({ "x-opencode-client": "desktop" })); + const req = createOpenAIChatAdapter(routed).buildRequest(minimalRequest()); + expect((req.headers as Record)["User-Agent"]).toBe("opencode"); + }); + + test("a user override wins and does not become a second comma-joined value", () => { + // HTTP header names are case-insensitive but object keys are not: a naive spread would + // leave both "user-agent" and "User-Agent", which `Headers` serializes as + // "custom-agent, opencode" — a corrupted request rather than an override. + const routed = routedProviderConfig("opencode-free", persisted({ "user-agent": "custom-agent" })); + const uaKeys = Object.keys(routed.headers ?? {}).filter(k => k.toLowerCase() === "user-agent"); + expect(uaKeys).toEqual(["user-agent"]); + expect(routed.headers?.["user-agent"]).toBe("custom-agent"); + expect(new Headers(routed.headers as Record).get("user-agent")).toBe("custom-agent"); + // Names the user did not claim are still filled. + expect(routed.headers?.["x-opencode-client"]).toBe("desktop"); + }); + + test("model discovery carries the same fingerprint as inference", () => { + // A provider identified as `opencode` when it completes but anonymous when it lists its + // own models reads as two different clients to an upstream rate limiter. + const req = buildModelsRequest(persisted({ "x-opencode-client": "desktop" }), undefined, "opencode-free"); + expect(req.headers["User-Agent"]).toBe("opencode"); + expect(req.headers["x-opencode-client"]).toBe("desktop"); + }); + + test("model discovery honors a user User-Agent override", () => { + const req = buildModelsRequest(persisted({ "user-agent": "custom-agent" }), undefined, "opencode-free"); + const uaKeys = Object.keys(req.headers).filter(k => k.toLowerCase() === "user-agent"); + expect(uaKeys).toEqual(["user-agent"]); + expect(req.headers["user-agent"]).toBe("custom-agent"); + }); + }); + test("provider note mentions no key needed", () => { expect(entry?.note?.toLowerCase()).toContain("no key needed"); expect(entry?.note?.toLowerCase()).toContain("200"); From 6ef9c0883d6e791807ca0ceb1afb63346e345d9e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 09:04:51 +0900 Subject: [PATCH 16/25] docs(devlog): record wp16 and the staticHeaders delivery bug it uncovered --- .../080_residual_dispositions.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md index e47d8aa35b..460885e19f 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md @@ -161,6 +161,56 @@ Deliberately NOT copied from omniroute: `x-opencode-project`, `x-opencode-reques conversation-derived session identifier is a privacy-relevant change that needs its own evidence rather than a sibling project's precedent. +### wp16 outcome — and the bug the absorb uncovered + +**PR #2160**, branch `codex/absorb-opencode-free-static-headers`, base `dev`. #2067 closed +with attribution (`issuecomment-5349492578`). + +The audit is the interesting part. The plan as written — add the header to the registry row — +passed my own reading and FAILED the reviewer, correctly. `staticHeaders` is documented at +`registry.ts:149` as "merged into every upstream request for this provider", and that was +false. It was copied at seed time only: `providerConfigSeed` writes the block once, +`enrichProviderFromCatalog` fills it only when the whole block is absent, and nothing merged +it at request time. `rg -n 'headers' src/router.ts` returned zero hits. + +Reproduced directly before accepting the finding: + +| persisted config | `routedProviderConfig("opencode-free", ...).headers` | +|---|---| +| no headers block | `undefined` | +| `{x-opencode-client: desktop}` | unchanged — no UA | +| `{user-agent: custom-agent}` | unchanged — no client marker | + +So the contributor's one-line registry patch would have shipped a header that **no existing +install ever receives**. The management API strips a persisted block that exactly matches the +registry set, which means the most common on-disk state is "no headers at all" — and that +state gained nothing. + +Implementation, three parts: + +1. `mergeRegistryStaticHeaders(staticHeaders, userHeaders)` in `registry.ts` — registry values + fill only names the user has not claimed, compared **case-insensitively**. That last word is + load-bearing: HTTP header names are case-insensitive but object keys are not, so spreading a + registry `User-Agent` over a user's `user-agent` leaves both keys and `Headers` serializes + them as `"custom-agent, opencode"` — a corrupted request wearing the costume of an override. +2. `routedProviderConfig` (`router.ts`) merges at resolve time. +3. `buildModelsRequest` (`oauth/index.ts`) does the same, because a provider identified as + `opencode` when it completes but anonymous when it lists its own models reads as two + different clients to a rate limiter. + +Residual, stated rather than skipped: `validateApiKey` (`key-providers.ts:102`) still sends +only `Authorization`. It is an auth probe by design; widening an auth-path request shape is a +separate change with its own review burden. + +Evidence: 6 new regressions; reverting only `router.ts` + `oauth/index.ts` while keeping the +registry header fails exactly 5 of them (13 pass / 5 fail), which is what makes them delivery +tests rather than restatements of the registry constant. Full suite 13519 pass / 10 skip / +0 fail across 856 files; typecheck and privacy scan clean. + +One existing expectation moved: `tests/management-provider-validation.test.ts` "provider PATCH +clear keeps registry static headers" now asserts the two-header set. That is the same edit +#2067 made, and it is the correct one — the test pins the registry-owned set, which grew. + ## wp17-wp19 — the three that need new code Recorded here as each is decided; each is its own PABCD cycle. From 06cdbc109a631203618a2d1bcf2f395f121af8fa Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 09:24:46 +0900 Subject: [PATCH 17/25] fix(anthropic): frame the opening turn so AgentRouter stops blocking non-English AgentRouter answers 400 content-blocked when the first user message is not in English (#2074) while the identical English request returns 200. The gateway inspects the opening user content, so an Anthropic system string never reaches the filter -- the framing has to sit in that turn. Two corrections on top of @yzxcj797's #2082. The host test was hostname.includes("agentrouter"), which also matches notagentrouter.example and agentrouter.org.attacker.example. A prompt mutation keyed on a provider's identity has to be keyed on that identity exactly, so this matches agentrouter.org or a real subdomain of it. The original spliced the marker into the user's own string. That edits what the user wrote: logs, retries, and any upstream echo then show a sentence the user never typed as if they had. The framing is now its own leading text block, so the original text survives byte-for-byte. Idempotence is keyed on the leading block being exactly the marker rather than a substring test, so a user who quotes the marker later in their prompt does not suppress their own framing. --- src/adapters/anthropic.ts | 59 ++++++++ ...ropic-agentrouter-language-framing.test.ts | 126 ++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 tests/anthropic-agentrouter-language-framing.test.ts diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index e012a78198..626a45f286 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -594,6 +594,63 @@ function orphanToolResultText(msg: OcxToolResultMessage): string { return `[tool_result without adjacent tool_use: ${label}]\n${content}`; } +/** + * AgentRouter answers 400 `content-blocked` when the first user message is not in English + * (#2074), while the same request in English returns 200. The gateway is inspecting the opening + * user content, so an Anthropic `system` string cannot reach it — the framing has to sit in the + * first user turn. + */ +const AGENTROUTER_LANGUAGE_PREAMBLE = + "[Instruction: Process the user request below and respond in the appropriate language.]"; + +/** + * Exact host match, not a substring. + * + * A `hostname.includes("agentrouter")` test also matches `notagentrouter.example` and + * `agentrouter.org.attacker.example`, which would let an unrelated destination silently + * receive an injected instruction block. A prompt mutation keyed on a provider's identity + * must be keyed on that identity exactly. + */ +function isAgentRouterEndpoint(baseUrl: string): boolean { + try { + const { hostname } = new URL(baseUrl); + return hostname === "agentrouter.org" || hostname.endsWith(".agentrouter.org"); + } catch { + return false; + } +} + +/** + * Prepend the framing as its OWN text block instead of splicing it into the user's string. + * + * The distinction matters: rewriting `content` to `${marker}\n\n${original}` edits what the + * user wrote, and every downstream consumer — logs, retries, an upstream that echoes the turn — + * then sees a sentence the user never typed as if they had. A separate leading block carries the + * same signal to the filter while the original text survives byte-for-byte. + * + * Only the first user turn is framed, because only the first is what the gateway rejects. + */ +function applyAgentRouterLanguageFraming(messages: unknown[]): void { + const firstUser = messages.find( + (m): m is { role: string; content: unknown } => + typeof m === "object" && m !== null && (m as { role?: unknown }).role === "user", + ); + if (!firstUser) return; + const preamble = { type: "text", text: AGENTROUTER_LANGUAGE_PREAMBLE }; + if (typeof firstUser.content === "string") { + firstUser.content = firstUser.content === "" + ? [preamble] + : [preamble, { type: "text", text: firstUser.content }]; + return; + } + if (!Array.isArray(firstUser.content)) return; + // Idempotence is keyed on the LEADING block being exactly the marker. A substring test would + // let a user who quotes the marker later in their own prompt suppress the framing entirely. + const [head] = firstUser.content as { type?: unknown; text?: unknown }[]; + if (head?.type === "text" && head.text === AGENTROUTER_LANGUAGE_PREAMBLE) return; + (firstUser.content as unknown[]).unshift(preamble); +} + function messagesToAnthropicFormat( parsed: OcxParsedRequest, toolNames: { toWire: (name: string) => string }, @@ -833,6 +890,8 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti } const { system, messages } = messagesToAnthropicFormat(parsed, toolNames); + // Before image normalization, so the framing block is present for every downstream pass. + if (isAgentRouterEndpoint(provider.baseUrl)) applyAgentRouterLanguageFraming(messages); // Primary image layer: resize/re-encode to fit Anthropic limits without dropping // (anthropic-image-normalize.ts); the guard below remains the deterministic backstop. // imageTierBias > 0 = upstream-413 tightened retry (030): start every image one tier lower. diff --git a/tests/anthropic-agentrouter-language-framing.test.ts b/tests/anthropic-agentrouter-language-framing.test.ts new file mode 100644 index 0000000000..09782a6835 --- /dev/null +++ b/tests/anthropic-agentrouter-language-framing.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, test } from "bun:test"; +import { createAnthropicAdapter as createAnthropicAdapterProduction } from "../src/adapters/anthropic"; +import { withTestTranslatorBudget } from "./helpers/translator-budget"; +import type { OcxMessage, OcxParsedRequest, OcxProviderConfig } from "../src/types"; + +const createAnthropicAdapter = (...args: Parameters) => + withTestTranslatorBudget(createAnthropicAdapterProduction(...args)); + +const PREAMBLE = "[Instruction: Process the user request below and respond in the appropriate language.]"; +const PORTUGUESE = "responda apenas: OK"; + +interface TextBlock { type: string; text?: string } +interface WireMessage { role: string; content: string | TextBlock[] } +interface WireBody { messages: WireMessage[] } + +function providerAt(baseUrl: string): OcxProviderConfig { + return { adapter: "anthropic", baseUrl, apiKey: "key", authMode: "key" }; +} + +function requestWith(messages: OcxMessage[]): OcxParsedRequest { + return { + modelId: "claude-opus-4-6", + stream: false, + context: { messages, tools: [] }, + options: {}, + }; +} + +async function bodyFor(baseUrl: string, messages: OcxMessage[]): Promise { + const req = await createAnthropicAdapter(providerAt(baseUrl)).buildRequest(requestWith(messages)); + return JSON.parse(req.body as string) as WireBody; +} + +/** + * The adapter also stamps `cache_control` onto the trailing block, which is orthogonal to this + * fix. Comparing the text sequence keeps these assertions about the framing and stops them from + * going red the next time the cache policy is tuned. + */ +function texts(message: WireMessage | undefined): (string | undefined)[] { + if (!message) throw new Error("expected a message at that index"); + if (typeof message.content === "string") return [message.content]; + return message.content.map(block => block.text); +} + +// AgentRouter answers 400 content-blocked when the first user message is not in English +// (#2074) while the identical English request returns 200. The gateway reads the opening user +// content, so the framing has to live in that turn -- an Anthropic `system` string never +// reaches the filter. Absorbed from #2082 by @yzxcj797. +describe("AgentRouter language framing", () => { + test("frames the first user turn without touching the user's own text", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [{ role: "user", content: PORTUGUESE }]); + // The framing is its OWN block. Splicing it into the user's string would make every + // downstream reader -- logs, retries, an upstream that echoes the turn -- attribute a + // sentence to the user that they never typed. + expect(texts(body.messages[0])).toEqual([PREAMBLE, PORTUGUESE]); + }); + + test("an existing block array keeps every original block, in order, after the preamble", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [ + { role: "user", content: [{ type: "text", text: "primeiro" }, { type: "text", text: "segundo" }] }, + ]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, "primeiro", "segundo"]); + }); + + test("only the first user turn is framed", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [ + { role: "user", content: PORTUGUESE }, + { role: "assistant", content: "OK" }, + { role: "user", content: "e agora?" }, + ]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, PORTUGUESE]); + // Index-free on purpose: the adapter may coalesce adjacent turns, so the invariant is + // "exactly one preamble, on the opening turn", not "the preamble is absent at index 2". + const userTurns = body.messages.filter(m => m.role === "user"); + expect(texts(userTurns.at(-1))).toEqual(["e agora?"]); + expect(JSON.stringify(body.messages).split(PREAMBLE)).toHaveLength(2); + }); + + test("direct Anthropic is untouched", async () => { + const body = await bodyFor("https://api.anthropic.com/v1", [{ role: "user", content: PORTUGUESE }]); + expect(texts(body.messages[0])).toEqual([PORTUGUESE]); + expect(JSON.stringify(body)).not.toContain(PREAMBLE); + }); + + // A hostname.includes("agentrouter") test would match both of these, quietly injecting an + // instruction block into a destination that never asked for one. + test.each([ + "https://notagentrouter.example/v1", + "https://agentrouter.org.attacker.example/v1", + ])("a lookalike host is not treated as AgentRouter: %s", async baseUrl => { + const body = await bodyFor(baseUrl, [{ role: "user", content: PORTUGUESE }]); + expect(texts(body.messages[0])).toEqual([PORTUGUESE]); + expect(JSON.stringify(body)).not.toContain(PREAMBLE); + }); + + test("a real AgentRouter subdomain is still AgentRouter", async () => { + const body = await bodyFor("https://api.agentrouter.org/v1", [{ role: "user", content: PORTUGUESE }]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, PORTUGUESE]); + }); + + test("building twice yields exactly one preamble each time", async () => { + const adapter = createAnthropicAdapter(providerAt("https://agentrouter.org/v1")); + for (const attempt of [0, 1]) { + const req = await adapter.buildRequest(requestWith([{ role: "user", content: PORTUGUESE }])); + const body = JSON.parse(req.body as string) as WireBody; + expect(texts(body.messages[0]).filter(t => t === PREAMBLE)).toHaveLength(1); + expect(attempt).toBeLessThan(2); + } + }); + + // Idempotence keyed on a substring would let a user who quotes the marker mid-prompt + // suppress their own framing, which is the failure the workaround exists to prevent. + test("a user quoting the marker later still gets the leading preamble", async () => { + const quoted = `o servidor respondeu ${PREAMBLE} e falhou`; + const body = await bodyFor("https://agentrouter.org/v1", [{ role: "user", content: quoted }]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, quoted]); + }); + + // An assistant-only request is synthesized into a "(continue)" user turn upstream of this + // code, so the framing lands on that synthetic turn rather than on nothing. Pinned because it + // is the one case where the preamble is attached to text the user did not send. + test("an assistant-only request frames the synthesized continue turn", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [{ role: "assistant", content: "só isso" }]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, "(continue)"]); + }); +}); From 7e7954a803aea10094a52cd61ed5e22c8c4d3350 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 09:34:36 +0900 Subject: [PATCH 18/25] docs(devlog): record wp17 and plan wp18 --- .../080_residual_dispositions.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md index 460885e19f..3aa59cc031 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md @@ -214,3 +214,61 @@ clear keeps registry static headers" now asserts the two-header set. That is the ## wp17-wp19 — the three that need new code Recorded here as each is decided; each is its own PABCD cycle. + +### wp17 — #2082 @yzxcj797: AgentRouter language framing + +**PR #2162**, branch `codex/absorb-agentrouter-language-framing`, base `dev`. #2082 closed +(`issuecomment-5349709140`). Fixes #2074. + +The diagnosis was the contributor's and it was correct: AgentRouter answers 400 +`content-blocked` on a non-English first user message while the same request in English +returns 200, and the filter reads that turn, so an Anthropic `system` string cannot reach it. + +Two corrections. + +**Host predicate.** `hostname.includes("agentrouter")` also matches `notagentrouter.example` +and `agentrouter.org.attacker.example`. This is a prompt mutation keyed on a provider's +identity, so the key has to be that identity exactly — otherwise an unrelated destination +quietly receives an injected instruction block. Now `agentrouter.org` or a real subdomain. + +**Where the marker goes.** The original spliced it into the user's string: +`firstUser.content = \`\${MARKER}\\n\\n\${firstUser.content}\``. That edits what the user +wrote, and every downstream reader then attributes a sentence to them that they never typed — +the hidden user-turn mutation named in #1804. The framing is now its own leading text block. +It still adds content to the user turn, which is unavoidable against a filter that reads the +first user message, but additive-and-visible is a different risk class than a silent rewrite. + +Idempotence is keyed on the LEADING block being exactly the marker, not a substring test: a +user who quotes the marker mid-prompt must not suppress their own framing. + +Evidence: 10 regressions; reverting only the adapter fails 7. The 3 that stay green are the +lookalike-host and direct-Anthropic cases — green on unpatched `dev` precisely because `dev` +frames nobody, which is what makes them guards against the substring predicate rather than +restatements of it. Full suite 13529 pass / 10 skip / 0 fail; typecheck and privacy clean. + +`CONFLICTING` was an inherited `package.json` bump alone; the Anthropic hunks merge cleanly. +No version change in the replacement. + +### wp18 — #2027 @yzxcj797: OpenCode Go quota, planned + +The investigation moved the answer here too. The real issue is #1924: sibling rows +(`opencode-go-2` … `-5`) show no quota in the dashboard and no rows in +`ocx provider quota --refresh --json`, because dispatch gates on the literal provider NAME at +`src/providers/quota.ts:2087`. + +The contributor's fix swaps that for a base-URL comparison. Closer, but it does not check the +adapter, so a row pointed at the canonical URL with a different adapter would be probed. + +The repository already has the exact predicate: `registryEntryForProviderDestination` +(`registry.ts:2678`) identifies a renamed fixed key provider by normalized endpoint + adapter ++ auth mode, and is already the convention for renamed rows +(`opencode-zen-rate-limit.ts:28-43`, `derive.ts:398-425`). + +Rejected alternative, recorded: `providerMatchesRegistryTransport("opencode-go", provider)` +would need `preserveCustomDestination: true` on the registry entry, which also changes ROUTING +for a same-named custom row (`router.ts:269-274` vs `:320-336`). That may be worth doing, but +not as a side effect of a quota fix. + +The defensive canonical-URL check inside `fetchOpenCodeGoQuota` (`quota.ts:485-494`) stays: it +is what stops an API key being sent to a non-canonical host, and it should not depend on the +dispatch predicate being correct. From 6a4efb8c1d685dded2a41508aec6e6c5ffa473f3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 11:26:14 +0900 Subject: [PATCH 19/25] test(anthropic): pin the AgentRouter framing across every user-content shape Review on #2162 asked for the three branches that are the actual safety boundary here: this code writes into the outbound first user turn, so "does it ever duplicate, drop, or reorder what the caller sent" has to be pinned per content shape rather than only for a plain string. Adds: an already-framed turn stays single and ordered; image-only content keeps its image block behind the preamble rather than losing or reordering it; assistant-only block content keeps its tail and is followed by the synthesized [PREAMBLE, "(continue)"] user turn. All three pass against the existing implementation, which is the point -- they are guards on a prompt mutation, not a fix. --- ...ropic-agentrouter-language-framing.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/anthropic-agentrouter-language-framing.test.ts b/tests/anthropic-agentrouter-language-framing.test.ts index 09782a6835..fbf2962e8c 100644 --- a/tests/anthropic-agentrouter-language-framing.test.ts +++ b/tests/anthropic-agentrouter-language-framing.test.ts @@ -123,4 +123,39 @@ describe("AgentRouter language framing", () => { const body = await bodyFor("https://agentrouter.org/v1", [{ role: "assistant", content: "só isso" }]); expect(texts(body.messages[0])).toEqual([PREAMBLE, "(continue)"]); }); + + // The three branches below are the safety boundary for a provider-specific prompt mutation: + // this code writes into the outbound user turn, so "does it ever duplicate, drop, or reorder + // what the caller sent" has to be pinned per content shape, not just for the plain string. + test("an already-framed first turn stays single and in order", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [ + { role: "user", content: [{ type: "text", text: PREAMBLE }, { type: "text", text: PORTUGUESE }] }, + ]); + expect(texts(body.messages[0])).toEqual([PREAMBLE, PORTUGUESE]); + }); + + test("image-only user content keeps its image block, after the preamble", async () => { + // A real 1x1 PNG: the normalizer replaces undecodable data with a text placeholder, which + // would make this assert the wrong thing. + const onePixelPng = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg=="; + const body = await bodyFor("https://agentrouter.org/v1", [ + { role: "user", content: [{ type: "image", imageUrl: onePixelPng }] }, + ]); + const blocks = body.messages[0]?.content; + if (typeof blocks === "string") throw new Error("expected content blocks"); + expect(blocks).toHaveLength(2); + expect(blocks[0]).toMatchObject({ type: "text", text: PREAMBLE }); + // The image must survive as an image — not dropped, and not reordered ahead of the preamble. + expect(blocks[1]).toMatchObject({ type: "image" }); + }); + + test("assistant-only block content keeps its tail, then the synthesized user turn", async () => { + const body = await bodyFor("https://agentrouter.org/v1", [ + { role: "assistant", content: [{ type: "text", text: "primeira" }, { type: "text", text: "segunda" }] }, + ]); + const assistant = body.messages.filter(m => m.role === "assistant"); + expect(texts(assistant.at(-1))).toEqual(["primeira", "segunda"]); + const user = body.messages.filter(m => m.role === "user"); + expect(texts(user.at(-1))).toEqual([PREAMBLE, "(continue)"]); + }); }); From 5445ce3e626849855521e5ff54571a1c3babc7e2 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 09:44:17 +0900 Subject: [PATCH 20/25] fix(quota): identify OpenCode Go by where it routes, not what it is named A multi-account setup points several provider rows at the same OpenCode Go endpoint under names the registry has never heard of -- opencode-go-2 through -5. Quota dispatch gated on the literal name "opencode-go", so those rows had no dashboard quota panel and no report in `ocx provider quota --refresh --json` even though each one holds a working key for the same upstream (#1924). Identity is now answered by registryEntryForProviderDestination, the predicate this repository already uses for renamed fixed-key rows: it matches on normalized endpoint plus adapter plus key auth. A bare URL comparison would have been enough for the reported symptom but would also probe a row that points at that host through a different adapter, which speaks a different protocol and is not the provider whose quota shape we parse. The defensive canonical-URL check inside fetchOpenCodeGoQuota stays. Whether an API key may be sent to a host must not depend on the dispatch gate above it being correct. Absorbed from #2027 by @yzxcj797. --- src/providers/quota.ts | 11 +++++-- tests/opencode-go-quota.test.ts | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/providers/quota.ts b/src/providers/quota.ts index bb3bab2837..24b06ef7a9 100644 --- a/src/providers/quota.ts +++ b/src/providers/quota.ts @@ -13,7 +13,7 @@ import { getAccountCredential, getAccountSet, getCredential } from "../oauth/sto import { antigravityUserAgent } from "../adapters/client-fingerprint"; import { apiKeyPoolEntryId } from "./api-keys"; import { XAI_GROK_CLIENT_VERSION, XAI_GROK_COMPATIBILITY } from "./xai-transport"; -import { getProviderRegistryEntry, providerCodexAccountMode } from "./registry"; +import { getProviderRegistryEntry, providerCodexAccountMode, registryEntryForProviderDestination } from "./registry"; import type { OcxConfig, OcxProviderConfig } from "../types"; import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "./openai-tiers"; import { @@ -2084,7 +2084,14 @@ async function maybeFetchProviderQuota( && isCanonicalCommandCodeBaseUrl(provider.baseUrl)) { return fetchCommandCodeQuota(name, provider); } - if ((provider.authMode ?? "key") === "key" && name === "opencode-go") { + // Identify OpenCode Go by where it routes, not by what the row is called. Multi-account + // setups keep the same destination under names like `opencode-go-2` (#1924), and those rows + // silently had no quota panel and no `ocx provider quota --json` report while the literal + // name was the gate. `registryEntryForProviderDestination` is the existing predicate for + // exactly this question: normalized endpoint + adapter + key auth, so a canonical URL behind + // a different adapter is still not OpenCode Go. The defensive URL check inside + // `fetchOpenCodeGoQuota` stays — sending a key anywhere must not depend on this gate. + if ((provider.authMode ?? "key") === "key" && registryEntryForProviderDestination(provider)?.id === "opencode-go") { return fetchOpenCodeGoQuota(name, provider); } if ((provider.authMode ?? "key") === "key" && isCanonicalA6apiBaseUrl(provider.baseUrl)) { diff --git a/tests/opencode-go-quota.test.ts b/tests/opencode-go-quota.test.ts index ece8d22b53..df2bb52525 100644 --- a/tests/opencode-go-quota.test.ts +++ b/tests/opencode-go-quota.test.ts @@ -87,4 +87,61 @@ describe("OpenCode Go provider quota", () => { expect(fetchCalls).toBe(0); expect(result.reports).toEqual([]); }); + + // #1924: a multi-account setup points several rows at the same OpenCode Go endpoint under + // names the registry has never heard of. Gating dispatch on the literal name `opencode-go` + // meant those rows had no dashboard quota panel and no `ocx provider quota --json` report, + // even though each one holds a working key for the same upstream. + test("a canonical sibling row under any name is probed and reported", async () => { + const bearers: string[] = []; + globalThis.fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + const headers = init?.headers as Record | undefined; + if (headers?.Authorization) bearers.push(headers.Authorization); + return new Response(JSON.stringify({ + usage: { rolling: { status: "ok", percent: 12, resetsAt: "2026-08-12T20:00:00.000Z" } }, + }), { status: 200, headers: { "content-type": "application/json" } }); + }) as typeof fetch; + + const config = openCodeGoConfig(); + config.providers["opencode-go-2"] = { + adapter: "openai-chat", + authMode: "key", + baseUrl: "https://opencode.ai/zen/go/v1", + // Kept under the privacy scanner's bearer-token length floor: a longer fixture reads as + // a real credential to `privacy:scan` even inside a test. + apiKey: "sibling-secret", + }; + + const result = await fetchProviderQuotaReports(config, true); + + expect(result.reports.map(report => report.provider).sort()).toEqual(["opencode-go", "opencode-go-2"]); + expect(bearers.sort()).toEqual(["Bearer opencode-go-secret", "Bearer sibling-secret"]); + expect(JSON.stringify(result)).not.toContain("sibling-secret"); + }); + + // The distinction between "routes to the OpenCode Go endpoint" and "is the OpenCode Go + // provider": a bare URL match would probe this row, but a different adapter speaks a + // different protocol to that host and is not the provider whose quota shape we parse. + test("a canonical URL behind a different adapter is not OpenCode Go", async () => { + let fetchCalls = 0; + globalThis.fetch = (async () => { + fetchCalls += 1; + return new Response("{}", { status: 200, headers: { "content-type": "application/json" } }); + }) as typeof fetch; + + const result = await fetchProviderQuotaReports({ + defaultProvider: "not-really-go", + providers: { + "not-really-go": { + adapter: "anthropic", + authMode: "key", + baseUrl: "https://opencode.ai/zen/go/v1", + apiKey: "unrelated-secret", + }, + }, + } as OcxConfig, true); + + expect(fetchCalls).toBe(0); + expect(result.reports).toEqual([]); + }); }); From 64ba54edbe5ac90b1127f51d5890e33da183c3f0 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 09:57:10 +0900 Subject: [PATCH 21/25] fix(openai-chat): treat a non-string repeat as padding once that field is known Some OpenAI-compatible streamers repeat an already-sent id, name, or arguments as a non-string placeholder on a continuation delta rather than as null. Validation ran before the pending-call lookup, so the whole turn died with a 502 and the tool never ran -- even though the value being repeated was already held in canonical form. The lookup now happens first and tolerance is per field, keyed on that field's own provenance. Two corrections on top of @waw4303's #2155. It gated arguments acceptance on the call having a canonical NAME. A name says nothing about whether arguments was ever sent as a string, so a real argument payload could be silently dropped. PendingToolCall now carries sawArgumentsString; an empty string counts, because it proves the upstream sent the field with the right wire type. It also left a non-string repeated id unconditionally terminal even after a canonical id was stored. Ids now follow the same rule as the other two. Diagnostics are passed from the rejection site instead of rescanned. A stateless rescan stops at the first structurally odd value, so a stream carrying accepted padding on call 0 and a real defect on call 1 blamed call 0. --- src/adapters/openai-chat.ts | 141 +++++++++++++++++++--------- tests/openai-chat-hardening.test.ts | 96 +++++++++++++++++++ 2 files changed, 191 insertions(+), 46 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index c5338d5eaa..8e59559094 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -308,8 +308,13 @@ function invalidToolCallsEvent( rawToolCalls: unknown, mode: "stream" | "response", usage?: OcxUsage, + diagnosticOverride?: InvalidToolCallDiagnostic, ): Extract { - const diagnostic = diagnoseInvalidToolCalls(rawToolCalls, mode); + // The streamed accumulator knows things a rescan cannot: which field on which pending call + // was actually rejected. Without the override, a stream carrying accepted padding on call 0 + // and a real defect on call 1 blames call 0, because the stateless scan stops at the first + // structurally odd value it sees. + const diagnostic = diagnosticOverride ?? diagnoseInvalidToolCalls(rawToolCalls, mode); const detail = diagnostic ? ` (${diagnostic.reason}${diagnostic.callIndex !== undefined ? `; callIndex=${diagnostic.callIndex}` : ""}; valueType=${diagnostic.valueType})` : ""; @@ -527,9 +532,13 @@ function diagnoseInvalidToolCalls( return undefined; } -function logInvalidToolCalls(mode: "stream" | "response", rawToolCalls: unknown): void { +function logInvalidToolCalls( + mode: "stream" | "response", + rawToolCalls: unknown, + diagnosticOverride?: InvalidToolCallDiagnostic, +): void { if (!isDebugEnabled()) return; - const diagnostic = diagnoseInvalidToolCalls(rawToolCalls, mode); + const diagnostic = diagnosticOverride ?? diagnoseInvalidToolCalls(rawToolCalls, mode); if (!diagnostic) return; const fieldShape = fingerprintInvalidField(invalidToolCallField(rawToolCalls, diagnostic)); debugProviderDiagnostic("openai-chat", "invalid-tool-calls", { @@ -1499,7 +1508,20 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const budgetEncoder = new TextEncoder(); let buffer = ""; let bufferBytes = 0; - interface PendingToolCall { key: string; id: string; name: string; args: string; argsBytes: number } + interface PendingToolCall { + key: string; + id: string; + name: string; + args: string; + argsBytes: number; + /** + * Whether this call has ever received `arguments` as an actual string, empty included. + * An empty string still counts: it proves the upstream sent the field with the right + * wire type, which is what a later malformed repeat of that field would be padding for. + * A canonical NAME is not evidence about the ARGUMENTS field and must not stand in. + */ + sawArgumentsString: boolean; + } const pendingToolCalls: PendingToolCall[] = []; let toolCallSeq = 0; const closeToolCalls = (): PendingToolCall[] => { @@ -1613,61 +1635,88 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage)); } - for (const rawToolCall of rawToolCalls) { + for (let callIndex = 0; callIndex < rawToolCalls.length; callIndex++) { + const rawToolCall: unknown = rawToolCalls[callIndex]; if (!isRecord(rawToolCall)) { - logInvalidToolCalls("stream", rawToolCalls); - return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage)); + const diagnostic: InvalidToolCallDiagnostic = { + reason: "tool_call_not_object", + callIndex, + valueType: rawToolCall === null ? "null" : Array.isArray(rawToolCall) ? "array" : typeof rawToolCall, + }; + logInvalidToolCalls("stream", rawToolCalls, diagnostic); + return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage, diagnostic)); } - const tc = rawToolCall as { - index?: number; - id?: string; - function?: { name?: string; arguments?: string }; - }; - // That cast is a TypeScript convenience, not a runtime guarantee: this is - // upstream JSON. Validate the fields before they are stored, so a non-string - // name or arguments value fails closed through the #1325 channel here rather - // than escaping later as a TypeError from string handling at flush time. - const rawFunction = (rawToolCall as { function?: unknown }).function; - if (rawFunction !== undefined && rawFunction !== null) { - if (!isRecord(rawFunction)) { - logInvalidToolCalls("stream", rawToolCalls); - return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage)); - } - const rawName = rawFunction.name; - const rawArguments = rawFunction.arguments; - // Some OpenAI-compatible streamers repeat already-sent fields as null on - // continuation deltas. Treat only null/undefined as absent; every other - // non-string value still fails closed before entering the accumulator. - if (isInvalidStreamStringField(rawName) || isInvalidStreamStringField(rawArguments)) { - logInvalidToolCalls("stream", rawToolCalls); - return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage)); - } + // This is upstream JSON, so every field is validated before it is stored: a + // malformed value must fail closed through the #1325 channel here rather than + // escaping later as a TypeError from string handling at flush time. + const rawFunction = rawToolCall.function; + if (rawFunction !== undefined && rawFunction !== null && !isRecord(rawFunction)) { + const diagnostic: InvalidToolCallDiagnostic = { + reason: "tool_call_function_not_object", + callIndex, + valueType: Array.isArray(rawFunction) ? "array" : typeof rawFunction, + }; + logInvalidToolCalls("stream", rawToolCalls, diagnostic); + return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage, diagnostic)); } - if (isInvalidStreamStringField(tc.id)) { - logInvalidToolCalls("stream", rawToolCalls); - return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage)); - } - const key = typeof tc.index === "number" - ? `i:${tc.index}` - : tc.id - ? `id:${tc.id}` + const fnRecord = isRecord(rawFunction) ? rawFunction : undefined; + const rawName = fnRecord?.name; + const rawArguments = fnRecord?.arguments; + const rawId = rawToolCall.id; + const idDelta = typeof rawId === "string" ? rawId : ""; + const rawIndex = rawToolCall.index; + + // Resolve the pending call BEFORE judging the fields. Some OpenAI-compatible + // streamers repeat an already-sent field as a non-string placeholder on a + // continuation delta; judging first meant the whole stream died with a 502 even + // though the value being repeated was already held in canonical form. + const key = typeof rawIndex === "number" + ? `i:${rawIndex}` + : idDelta + ? `id:${idDelta}` : pendingToolCalls[pendingToolCalls.length - 1]?.key; let call = key !== undefined ? pendingToolCalls.find(c => c.key === key) : undefined; - if (!call && tc.id) call = pendingToolCalls.find(c => c.id === tc.id); + if (!call && idDelta) call = pendingToolCalls.find(c => c.id === idDelta); if (!call) { - call = { key: key ?? `seq:${pendingToolCalls.length}`, id: "", name: "", args: "", argsBytes: 0 }; + call = { + key: key ?? `seq:${pendingToolCalls.length}`, + id: "", + name: "", + args: "", + argsBytes: 0, + sawArgumentsString: false, + }; pendingToolCalls.push(call); budget.openCall(call.key); } - if (tc.id && !call.id) call.id = tc.id; - if (tc.function?.name && !call.name) call.name = tc.function.name; - if (tc.function?.arguments) { + + // Tolerance is per FIELD, keyed on that field's own provenance. A canonical name + // says nothing about whether `arguments` was ever sent as a string, so it cannot + // authorize a malformed arguments value — that would silently drop a real + // argument payload the model intended to send. + const rejection: InvalidToolCallDiagnostic | undefined = + isInvalidStreamStringField(rawName) && call.name.trim() === "" + ? { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof rawName } + : isInvalidStreamStringField(rawArguments) && !call.sawArgumentsString + ? { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof rawArguments } + : isInvalidStreamStringField(rawId) && call.id === "" + ? { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawId } + : undefined; + if (rejection) { + logInvalidToolCalls("stream", rawToolCalls, rejection); + return yield* terminateWithError(invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage, rejection)); + } + + if (idDelta && !call.id) call.id = idDelta; + if (typeof rawName === "string" && rawName && !call.name) call.name = rawName; + if (typeof rawArguments === "string") call.sawArgumentsString = true; + if (typeof rawArguments === "string" && rawArguments) { const previousBytes = call.argsBytes; - const nextBytes = previousBytes + budgetEncoder.encode(tc.function.arguments).byteLength; + const nextBytes = previousBytes + budgetEncoder.encode(rawArguments).byteLength; const scope = { kind: "tool_args" as const, callId: call.key }; const reservation = budget.reserveTransient(nextBytes, scope); try { - call.args += tc.function.arguments; + call.args += rawArguments; reservation.commitRetained(); budget.releaseRetained(previousBytes, scope); call.argsBytes = nextBytes; diff --git a/tests/openai-chat-hardening.test.ts b/tests/openai-chat-hardening.test.ts index f7435e0a28..81671b0968 100644 --- a/tests/openai-chat-hardening.test.ts +++ b/tests/openai-chat-hardening.test.ts @@ -476,6 +476,102 @@ describe("openai-chat stream response hardening", () => { expect(lines).toContain('"callIndex":1'); expect(lines).not.toContain('"tool_call_function_name_invalid"'); }); + + // Some OpenAI-compatible streamers repeat an already-sent field as a non-string placeholder + // instead of null. Before #2155 that killed the whole turn with a 502 even though the value + // being repeated was already held in canonical form, so the tool never ran. + test("a non-string repeat is padding once that field has string provenance (#2155)", async () => { + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: "call_a", function: { name: "shell", arguments: "" } }, + ] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: { padding: true }, function: { name: { padding: true }, arguments: { padding: true } } }, + ] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, function: { arguments: "{}" } }, + ] } }] })}\n\n`, + 'data: {"choices":[{"delta":{},"finish_reason":"tool_calls"}]}\n\n', + "data: [DONE]\n\n", + ].join("")); + + const events = await collect(adapter.parseStream(response)); + expect(events.some(event => event.type === "error")).toBe(false); + expect(events).toContainEqual({ type: "tool_call_start", id: "call_a", name: "shell" }); + expect(events).toContainEqual({ type: "tool_call_delta", arguments: "{}" }); + }); + + // Tolerance is per field. A canonical NAME is not evidence that `arguments` was ever sent + // as a string, and accepting a malformed object here would silently discard an argument + // payload the model meant to send. + test("a canonical name does not authorize a malformed arguments value (#2155)", async () => { + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: "call_a", function: { name: "shell" } }, + ] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, function: { arguments: { bad: true } } }, + ] } }] })}\n\n`, + "data: [DONE]\n\n", + ].join("")); + + expect(await collect(adapter.parseStream(response))).toEqual([{ + type: "error", + status: 502, + errorType: "upstream_error", + message: "upstream response contained invalid tool calls (tool_call_function_arguments_invalid; callIndex=0; valueType=object)", + }]); + }); + + test("a malformed id stays terminal until that call has a canonical id (#2155)", async () => { + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, function: { name: "shell", arguments: "{}" } }, + ] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: { bad: true } }, + ] } }] })}\n\n`, + "data: [DONE]\n\n", + ].join("")); + + expect(await collect(adapter.parseStream(response))).toEqual([{ + type: "error", + status: 502, + errorType: "upstream_error", + message: "upstream response contained invalid tool calls (tool_call_id_invalid; callIndex=0; valueType=object)", + }]); + }); + + // The reason the diagnostic is passed from the rejection site rather than rescanned: a + // stateless rescan stops at the first structurally odd value, which here is the ACCEPTED + // padding on call 0, and would blame the wrong call for the real defect on call 1. + test("parallel calls blame the unresolved call, not the accepted padding (#2155)", async () => { + process.env.OCX_DEBUG = "1"; + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: "call_a", function: { name: "alpha", arguments: "" } }, + { index: 1, id: "call_b", function: { name: "beta" } }, + ] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, function: { arguments: { padding: true } } }, + { index: 1, function: { arguments: { bad: true } } }, + ] } }] })}\n\n`, + "data: [DONE]\n\n", + ].join("")); + + expect(await collect(adapter.parseStream(response))).toEqual([{ + type: "error", + status: 502, + errorType: "upstream_error", + message: "upstream response contained invalid tool calls (tool_call_function_arguments_invalid; callIndex=1; valueType=object)", + }]); + const lines = getDebugLogEntries().map(entry => entry.line).join("\n"); + expect(lines).toContain('"callIndex":1'); + }); }); describe("openai-chat credential hardening", () => { From 02a564892af2fca911f422ba85b554b913fd444f Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Thu, 20 Aug 2026 00:51:00 +0000 Subject: [PATCH 22/25] fix(logs): persist shadow helper attribution --- src/server/request-log.ts | 13 ++++++++++ src/server/responses/core.ts | 2 +- src/usage/log.ts | 4 ++++ structure/05_gui-and-management-api.md | 3 +++ tests/request-log.test.ts | 30 ++++++++++++++++++++++++ tests/responses-shadow-intercept.test.ts | 14 ++++++++--- 6 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/server/request-log.ts b/src/server/request-log.ts index 9658cbd2fa..5567b31d21 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -65,6 +65,8 @@ export interface RequestLogContext { /** Stable non-PII Codex Pool account identity for durable usage attribution. */ accountLogLabel?: string; requestedModel?: string; + /** Original bare helper model when the opt-in shadow-call route rewrote this request. */ + shadowCallRewrittenFrom?: string; /** Internal structural combo identity; omitted from RequestLogEntry/JSONL. */ comboId?: string; requestedEffort?: string; @@ -142,6 +144,8 @@ export interface RequestLogEntry { /** Best-effort chat/session correlation for Logs grouping (#330). */ conversationId?: string; requestedModel?: string; + /** Original bare helper model when the opt-in shadow-call route rewrote this request. */ + shadowCallRewrittenFrom?: string; requestedEffort?: string; effectiveEffort?: string; reasoningWireField?: string; @@ -255,6 +259,9 @@ export function requestLogEntryFromPersistedUsage(entry: PersistedUsageEntry): R ? { accountLogLabel: entry.accountLogLabel } : {}), ...(entry.requestedModel ? { requestedModel: entry.requestedModel } : {}), + ...(entry.shadowCallRewrittenFrom + ? { shadowCallRewrittenFrom: entry.shadowCallRewrittenFrom } + : {}), ...(entry.requestedEffort ? { requestedEffort: entry.requestedEffort } : {}), ...(entry.effectiveEffort ? { effectiveEffort: entry.effectiveEffort } : {}), ...(entry.reasoningWireField ? { reasoningWireField: entry.reasoningWireField } : {}), @@ -358,6 +365,9 @@ export function addRequestLog(entry: RequestLogEntry) { ...(entry.conversationId ? { conversationId: entry.conversationId } : {}), ...(entry.resolvedModel ? { resolvedModel: entry.resolvedModel } : {}), ...(entry.requestedModel ? { requestedModel: entry.requestedModel } : {}), + ...(entry.shadowCallRewrittenFrom + ? { shadowCallRewrittenFrom: entry.shadowCallRewrittenFrom } + : {}), ...(entry.requestedEffort ? { requestedEffort: entry.requestedEffort } : {}), ...(entry.effectiveEffort ? { effectiveEffort: entry.effectiveEffort } : {}), ...(entry.reasoningWireField ? { reasoningWireField: entry.reasoningWireField } : {}), @@ -919,6 +929,9 @@ export function addFinalRequestLog( : {}), ...(logCtx.conversationId ? { conversationId: logCtx.conversationId } : {}), ...(logCtx.requestedModel ? { requestedModel: logCtx.requestedModel } : {}), + ...(logCtx.shadowCallRewrittenFrom + ? { shadowCallRewrittenFrom: logCtx.shadowCallRewrittenFrom } + : {}), ...(logCtx.requestedEffort ? { requestedEffort: logCtx.requestedEffort } : {}), ...(logCtx.effectiveEffort ? { effectiveEffort: logCtx.effectiveEffort } : {}), ...(logCtx.reasoningWireField ? { reasoningWireField: logCtx.reasoningWireField } : {}), diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 9e2813d0b5..f693bc6699 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -1838,7 +1838,7 @@ async function handleResponsesInner( if (parsed._rawBody && typeof parsed._rawBody === "object") { (parsed._rawBody as Record).reasoning = { effort: "low" }; } - (logCtx as unknown as Record).shadowCallRewrittenFrom = _sciOriginal; + logCtx.shadowCallRewrittenFrom = sanitizeLogMetadataString(_sciOriginal); // Helpers must not resume/append into the parent thread's Cursor conversation. parsed._cursorIsolateConversation = true; } diff --git a/src/usage/log.ts b/src/usage/log.ts index a526d8ddf5..fd93059e68 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -84,6 +84,8 @@ export interface PersistedUsageEntry { conversationId?: string; resolvedModel?: string; requestedModel?: string; + /** Original bare helper model when the opt-in shadow-call route rewrote this request. */ + shadowCallRewrittenFrom?: string; /** Reasoning effort / service-tier metadata for GUI Logs after restart. */ requestedEffort?: string; /** Adapter-normalized tier and exact upstream parameter emitted for this request. */ @@ -427,6 +429,7 @@ function normalizeUsageEntry(entry: PersistedUsageEntry): PersistedUsageEntry { const tierOutcome = entry.tierOutcome ? normalizeAttemptTierOutcome(entry.tierOutcome) : undefined; const callerServiceTier = sanitizeLogMetadataString(entry.callerServiceTier); const responseServiceTier = sanitizeLogMetadataString(entry.responseServiceTier); + const shadowCallRewrittenFrom = sanitizeLogMetadataString(entry.shadowCallRewrittenFrom); const routeDecision = entry.routeDecision ? normalizeRouteDecisionTrace(entry.routeDecision) : undefined; @@ -453,6 +456,7 @@ function normalizeUsageEntry(entry: PersistedUsageEntry): PersistedUsageEntry { : {}), ...(entry.resolvedModel ? { resolvedModel: entry.resolvedModel } : {}), ...(entry.requestedModel ? { requestedModel: entry.requestedModel } : {}), + ...(shadowCallRewrittenFrom ? { shadowCallRewrittenFrom } : {}), ...(typeof entry.requestedEffort === "string" && entry.requestedEffort ? { requestedEffort: capMetadataString(entry.requestedEffort) } : {}), diff --git a/structure/05_gui-and-management-api.md b/structure/05_gui-and-management-api.md index a4bbcbe6db..688f112020 100644 --- a/structure/05_gui-and-management-api.md +++ b/structure/05_gui-and-management-api.md @@ -320,6 +320,9 @@ keeps the saved state and renders fixed `ocx sync` guidance without server/accou ## Usage accounting `src/usage/log.ts` writes append-only JSONL to `~/.opencodex/usage.jsonl` with file mode `0o600`. +An opt-in shadow-call rewrite persists the bounded, redacted original helper model as +`shadowCallRewrittenFrom`, so helper traffic remains identifiable after restart without storing +request content or inferring a helper subtype from timing. `src/usage/summary.ts` turns that file into the `/api/usage` shape — totals, daily zero-filled grid, model and provider breakdowns, and `measured / reported / unreported / unsupported / estimated` counts. A Codex-surface response also includes an `accounts` breakdown keyed by the stable non-PII diff --git a/tests/request-log.test.ts b/tests/request-log.test.ts index e1f6eefab7..040badbd77 100644 --- a/tests/request-log.test.ts +++ b/tests/request-log.test.ts @@ -258,6 +258,32 @@ describe("request log metadata", () => { expect(captured2[0]).not.toHaveProperty("firstOutputMs"); }); + test("persists the shadow helper source marker to usage.jsonl", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-shadow-usage-")); + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + try { + clearRequestLogsForTests(); + resetUsageReadCacheForTests(); + addFinalRequestLog("ocx-shadow-marker", 1, { + model: "grok-4.5", + provider: "xai", + requestedModel: "gpt-5.6-luna", + shadowCallRewrittenFrom: "gpt-5.6-luna", + }, 200); + + const [persisted] = readUsageEntries(); + expect(persisted?.shadowCallRewrittenFrom).toBe("gpt-5.6-luna"); + expect(getRequestLogEntries()[0]?.shadowCallRewrittenFrom).toBe("gpt-5.6-luna"); + } finally { + clearRequestLogsForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + resetUsageReadCacheForTests(); + rmSync(home, { recursive: true, force: true }); + } + }); + test("records ordered attempts with sealed identity, fresh estimates, and deduplicated recoveries", () => { const a = beginRequestAttempt(1, "provisional-a", "model-a", "openai-chat"); noteAttemptSend(a, 100); @@ -1312,6 +1338,7 @@ describe("request log restart hydrate", () => { provider: "chatgpt-pabcdef", model: "gpt-5.6-sol", requestedModel: "gpt-5.6-sol", + shadowCallRewrittenFrom: "gpt-5.6-luna", requestedEffort: "high", effectiveEffort: "high", reasoningWireField: "reasoning_effort", @@ -1334,6 +1361,7 @@ describe("request log restart hydrate", () => { provider: "chatgpt-pabcdef", model: "gpt-5.6-sol", requestedModel: "gpt-5.6-sol", + shadowCallRewrittenFrom: "gpt-5.6-luna", requestedEffort: "high", effectiveEffort: "high", reasoningWireField: "reasoning_effort", @@ -1381,6 +1409,7 @@ describe("request log restart hydrate", () => { terminalStatus: "failed", closeReason: "terminal", upstreamError: "Provider unreachable", + shadowCallRewrittenFrom: "gpt-5.6-luna", }, ]; @@ -1392,6 +1421,7 @@ describe("request log restart hydrate", () => { errorCode: "upstream_server_error", upstreamError: "Provider unreachable", requestedEffort: "xhigh", + shadowCallRewrittenFrom: "gpt-5.6-luna", }); // Idempotent: a second start in the same process must not duplicate. diff --git a/tests/responses-shadow-intercept.test.ts b/tests/responses-shadow-intercept.test.ts index 7950f86c60..18ee9eb059 100644 --- a/tests/responses-shadow-intercept.test.ts +++ b/tests/responses-shadow-intercept.test.ts @@ -10,6 +10,7 @@ import { join } from "node:path"; import { handleResponses, isShadowSourceModel } from "../src/server/responses"; import { shouldInterceptShadowCall } from "../src/lib/shadow-call"; import { handleManagementAPI } from "../src/server/management-api"; +import type { RequestLogContext } from "../src/server/request-log"; import type { OcxConfig } from "../src/types"; import { catalogConvergenceFactory } from "./helpers/catalog-convergence"; @@ -90,7 +91,12 @@ function interceptConfig(): OcxConfig { } as OcxConfig; } -async function post(config: OcxConfig, model: string, requestKind?: string): Promise { +async function post( + config: OcxConfig, + model: string, + requestKind?: string, + logCtx: RequestLogContext = { model: "", provider: "" }, +): Promise { const headers: Record = { "content-type": "application/json" }; if (requestKind) { headers["x-codex-turn-metadata"] = JSON.stringify({ request_kind: requestKind }); @@ -104,7 +110,7 @@ async function post(config: OcxConfig, model: string, requestKind?: string): Pro stream: false, reasoning: { effort: "high" }, }), - }), config, { model: "", provider: "" }); + }), config, logCtx); } describe("shadow call intercept request path (issue #311)", () => { @@ -130,6 +136,7 @@ describe("shadow call intercept request path (issue #311)", () => { test("rewrites a gpt-5.6-luna turn request too (#1684)", async () => { const bodies: Array> = []; + const logCtx: RequestLogContext = { model: "", provider: "" }; globalThis.fetch = (async (_url: unknown, init?: RequestInit) => { bodies.push(JSON.parse(String(init?.body ?? "{}")) as Record); return new Response(JSON.stringify({ @@ -138,10 +145,11 @@ describe("shadow call intercept request path (issue #311)", () => { }), { status: 200, headers: { "content-type": "application/json" } }); }) as typeof fetch; - await post(interceptConfig(), "gpt-5.6-luna", "turn"); + await post(interceptConfig(), "gpt-5.6-luna", "turn", logCtx); expect(bodies.length).toBe(1); expect(String(bodies[0]?.model ?? "")).toContain("grok-4.5"); + expect(logCtx.shadowCallRewrittenFrom).toBe("gpt-5.6-luna"); }); test("leaves gpt-5.6-terra requests unrewritten", async () => { From e4f0eec94820fde6cc1f6a1032ba6f2ee1e8d4ee Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 10:32:17 +0900 Subject: [PATCH 23/25] fix(logs): sanitize the shadow helper marker at the logging layer Builds on @Ingwannu's #2163, which types and persists shadowCallRewrittenFrom so an intercepted helper request keeps its original model in usage.jsonl and across restart hydration. The original sanitized the value at the single call site that populates it today, which left the in-memory /api/logs row carrying whatever the caller sent. The marker originates in an upstream-supplied model id, so an unsanitized newline lets one field forge a record boundary in any line-oriented log viewer, and nothing bounded its length on that path. addFinalRequestLog now runs it through sanitizeLogMetadataString itself. A future caller cannot reintroduce the hole by forgetting to sanitize first, and the in-memory row matches what usage.jsonl already stored. The added regression writes an unsafe overlong marker and asserts the newline is gone and the 64-character bound holds on both paths. The original test used a safe short slug, so it passed identically whether the sanitizer ran or not. --- src/server/request-log.ts | 10 +++++++--- tests/request-log.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/server/request-log.ts b/src/server/request-log.ts index 5567b31d21..00da48567e 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -915,6 +915,12 @@ export function addFinalRequestLog( const loggedUsage = aggregate?.usage ?? existing.usage; const usageStatus = aggregate?.status ?? existing.status; const totalTokens = aggregate?.totalTokens ?? existing.totalTokens; + // Sanitize at the logging layer, not only at the one call site that populates this today. + // The value originates in an upstream-supplied model id, so an unsanitized newline would + // let a single field forge a record boundary in any line-oriented log viewer. Doing it here + // means a future caller cannot reintroduce the hole by forgetting to sanitize first, and + // the in-memory /api/logs row matches what usage.jsonl already stores. + const shadowCallRewrittenFrom = sanitizeLogMetadataString(logCtx.shadowCallRewrittenFrom); addLog({ requestId, timestamp: start, @@ -929,9 +935,7 @@ export function addFinalRequestLog( : {}), ...(logCtx.conversationId ? { conversationId: logCtx.conversationId } : {}), ...(logCtx.requestedModel ? { requestedModel: logCtx.requestedModel } : {}), - ...(logCtx.shadowCallRewrittenFrom - ? { shadowCallRewrittenFrom: logCtx.shadowCallRewrittenFrom } - : {}), + ...(shadowCallRewrittenFrom ? { shadowCallRewrittenFrom } : {}), ...(logCtx.requestedEffort ? { requestedEffort: logCtx.requestedEffort } : {}), ...(logCtx.effectiveEffort ? { effectiveEffort: logCtx.effectiveEffort } : {}), ...(logCtx.reasoningWireField ? { reasoningWireField: logCtx.reasoningWireField } : {}), diff --git a/tests/request-log.test.ts b/tests/request-log.test.ts index 040badbd77..f80e8ccd88 100644 --- a/tests/request-log.test.ts +++ b/tests/request-log.test.ts @@ -284,6 +284,40 @@ describe("request log metadata", () => { } }); + // The value is caller-controlled, so proving it lands is only half the contract: the + // persistence path must also be the SANITIZED one. A test that only ever writes a safe + // short slug passes identically whether `sanitizeLogMetadataString` is applied or not. + test("the shadow marker reaches usage.jsonl through the sanitizer, not raw", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-shadow-unsafe-")); + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + try { + clearRequestLogsForTests(); + resetUsageReadCacheForTests(); + addFinalRequestLog("ocx-shadow-unsafe", 1, { + model: "grok-4.5", + provider: "xai", + // A newline would let one field forge a record boundary in a line-oriented log + // viewer, and the trailing run is long enough to be over the 64-character bound. + shadowCallRewrittenFrom: `gpt-5.6-luna\nInjected: yes ${"x".repeat(80)}`, + }, 200); + + const [persisted] = readUsageEntries(); + const marker = persisted?.shadowCallRewrittenFrom; + expect(marker).toBeDefined(); + expect(marker).not.toContain("\n"); + expect(marker!.length).toBeLessThanOrEqual(64); + expect(marker!.startsWith("gpt-5.6-luna")).toBe(true); + expect(getRequestLogEntries()[0]?.shadowCallRewrittenFrom).not.toContain("\n"); + } finally { + clearRequestLogsForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + resetUsageReadCacheForTests(); + rmSync(home, { recursive: true, force: true }); + } + }); + test("records ordered attempts with sealed identity, fresh estimates, and deduplicated recoveries", () => { const a = beginRequestAttempt(1, "provisional-a", "model-a", "openai-chat"); noteAttemptSend(a, 100); From 6499919a61297e6aebcb9ec1c005fce8c26fa5d7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 10:33:22 +0900 Subject: [PATCH 24/25] docs(devlog): record wp18-wp20 and the six-layer stack end state --- .../080_residual_dispositions.md | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md index 3aa59cc031..a35be68067 100644 --- a/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/080_residual_dispositions.md @@ -272,3 +272,44 @@ not as a side effect of a quota fix. The defensive canonical-URL check inside `fetchOpenCodeGoQuota` (`quota.ts:485-494`) stays: it is what stops an API key being sent to a non-canonical host, and it should not depend on the dispatch predicate being correct. + +### wp18-wp20 — the rest of the chain + +**#2027 @yzxcj797 -> PR #2164.** Dispatch gated on the literal name `opencode-go`, so the +multi-account sibling rows in #1924 had no quota panel and no CLI report. The contributor's +base-URL swap is closer but does not check the adapter; `registryEntryForProviderDestination` +already answers exactly this question (endpoint + adapter + key auth) and is the existing +convention for renamed rows. Rejected: `providerMatchesRegistryTransport` would need +`preserveCustomDestination`, which also changes routing for same-named custom rows. + +**#2155 @waw4303 -> PR #2165.** Field validation ran before the pending-call lookup, so a +non-string repeat of an already-canonical field killed the turn with a 502. Two corrections: +`arguments` was gated on a canonical NAME (a name is not evidence about the arguments field, +so a real payload could be dropped) — now keyed on `sawArgumentsString`; and `id` stayed +unconditionally terminal. Diagnostics now come from the rejection site, because a stateless +rescan blamed call 0's accepted padding for call 1's real defect. + +**#2163 @Ingwannu -> PR #2166.** Scored 65. Backend attribution was correct; sanitization sat +at the one call site rather than in the logging layer, so `/api/logs` carried the raw +caller-supplied value. Moved into `addFinalRequestLog`. #2157 stays open: the GUI half is not +built, and closing it would claim an affordance that does not exist. + +### The stack + +Six layers, each rebased onto the current `dev` tip, base refs verified: + +#2134 -> #2160 -> #2162 -> #2164 -> #2165 -> #2166 + +Only one true dependency edge exists in the whole set (none of the six share files). They are +chained rather than opened as siblings because the user asked for one reviewable stack; that is +a review-workflow choice, stated rather than dressed up as a code constraint. + +A privacy-scan failure caught in CI and not locally: a test fixture API key over 24 characters +reads as a real bearer token to `scripts/privacy-scan.ts`. Fixed at the L4 commit. + +### End state + +`gh pr list --label bug --state open` returns only lidge-jun PRs plus #2054, which stays open +by explicit instruction and carries the wire-probe request. Nine contributor PRs closed with +attribution across this unit; none was closed without a named reason. + From 70cebd4de16e4d50916bcb1d6b35b52bf086dd4d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 11:20:21 +0900 Subject: [PATCH 25/25] fix(logs): sanitize at the addRequestLog ingress so both surfaces agree --- src/server/request-log.ts | 14 ++++++++++++++ tests/request-log.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/server/request-log.ts b/src/server/request-log.ts index 00da48567e..26a69a14ab 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -334,6 +334,20 @@ export function hydrateRequestLogsFromDisk( } export function addRequestLog(entry: RequestLogEntry) { + // Sanitize ONCE, at the ingress, and use that one value for both destinations. + // + // `addFinalRequestLog` is not the only way in: `addRequestLog` is exported and callable + // directly, and it retained the caller's entry verbatim in the in-memory ring while only the + // field-by-field disk projection below saw a sanitized value. That split let `/api/logs` + // serve a raw upstream-supplied marker — a newline in it can forge a record boundary in a + // line-oriented viewer — while `usage.jsonl` looked clean, which is the worst shape for a + // sanitization bug because the safe surface is the one you check. + const shadowCallRewrittenFrom = sanitizeLogMetadataString(entry.shadowCallRewrittenFrom); + const retained: RequestLogEntry = shadowCallRewrittenFrom === entry.shadowCallRewrittenFrom + ? entry + : { ...entry, ...(shadowCallRewrittenFrom ? { shadowCallRewrittenFrom } : {}) }; + if (!shadowCallRewrittenFrom && retained !== entry) delete retained.shadowCallRewrittenFrom; + entry = retained; retainRequestLogEntry(entry); try { // Failure diagnostics survive the 200-entry ring buffer by riding the persisted diff --git a/tests/request-log.test.ts b/tests/request-log.test.ts index f80e8ccd88..09512c50e1 100644 --- a/tests/request-log.test.ts +++ b/tests/request-log.test.ts @@ -11,6 +11,7 @@ import { } from "../src/server"; import { aggregateAttemptUsage, + addRequestLog, beginRequestAttempt, clearRequestLogsForTests, finishRequestAttempt, @@ -318,6 +319,42 @@ describe("request log metadata", () => { } }); + // `addFinalRequestLog` is not the only ingress: `addRequestLog` is exported and callable + // directly. Sanitizing only on the disk projection left the in-memory ring — and therefore + // /api/logs — serving the raw value, which is the worst shape for a sanitization bug + // because the surface you would check is the clean one. + test("the direct addRequestLog ingress sanitizes memory and disk identically", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-shadow-ingress-")); + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + try { + clearRequestLogsForTests(); + resetUsageReadCacheForTests(); + addRequestLog({ + requestId: "ocx-shadow-direct", + timestamp: Date.now(), + provider: "xai", + model: "grok-4.5", + status: 200, + shadowCallRewrittenFrom: `gpt-5.6-luna\nInjected: yes ${"x".repeat(80)}`, + } as RequestLogEntry); + + const inMemory = getRequestLogEntries()[0]?.shadowCallRewrittenFrom; + const [persisted] = readUsageEntries(); + expect(inMemory).toBeDefined(); + expect(inMemory).not.toContain("\n"); + expect(inMemory!.length).toBeLessThanOrEqual(64); + // The two surfaces must agree: a divergence here is exactly the bug. + expect(inMemory).toBe(persisted?.shadowCallRewrittenFrom); + } finally { + clearRequestLogsForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + resetUsageReadCacheForTests(); + rmSync(home, { recursive: true, force: true }); + } + }); + test("records ordered attempts with sealed identity, fresh estimates, and deduplicated recoveries", () => { const a = beginRequestAttempt(1, "provisional-a", "model-a", "openai-chat"); noteAttemptSend(a, 100);