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..1b68a3fe29 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md @@ -0,0 +1,289 @@ +# 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. + + +--- + +# 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. + + +## 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. + + +--- + +# 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. + 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..991c3bb62a --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/010_layer1_bearer_admission_2132.md @@ -0,0 +1,70 @@ +> **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). +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. + 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..cd146a2c99 --- /dev/null +++ b/devlog/_plan/260820_bug_pr_backlog_consolidation/070_execution_log.md @@ -0,0 +1,293 @@ +# 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. + + +## 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). + + +## 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. + + +# 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. + + +## 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. + + +## 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. + + +## 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. + 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. 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"); + }); +});