diff --git a/devlog/_plan/260902_bug_label_drawdown/000_plan.md b/devlog/_plan/260902_bug_label_drawdown/000_plan.md new file mode 100644 index 0000000000..7f8c20d0be --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/000_plan.md @@ -0,0 +1,59 @@ +# 000 — bug_label_drawdown: Plan + +## Objective + +Reduce open items carrying the `bug` label — PRs and issues both — from **24** to **3 or +fewer** (5 acceptable if the last few are genuinely blocked). Feature PRs and enhancement +issues are out of scope even when they look adjacent. + +Inventory taken 2026-09-02. + +**14 bug PRs:** #3177 #3176 #3174 #3168 #3164 #3151 #3148 #3144 #3138 #3135 #3121 #3112 +#3109 #3003 +**10 bug issues:** #3170 #3155 #3152 #3150 #3141 #3136 #2999 #2813 #1527 #1419 + +## Loop-spec + +- Archetype: verifier-defined. Each item has a binary terminal state. +- Write scope: whatever a named bug requires, plus `tests/`, plus this devlog unit. +- Out of scope: releases, promotion to `main`/`preview`, npm publish, deployment, + security-boundary rewrites beyond a named issue, other worktrees. +- **Verification policy (user-directed, binding):** never run the repository-wide local + suite; push with `--no-verify` so no hook runs it either. Focused `bun test` files plus + red-green proof. CI trails the work and is judged per batch. +- Merge mechanism: `gh pr merge --squash --admin --delete-branch`. +- **Rebase service is authorized.** A PR whose only defect is staleness gets rebased by us; + when the contributor branch is unpushable, its unique commits are cherry-picked onto a + `codex/` carry branch with author credit preserved and the original closed + `landed-via-maintainer` naming the merge SHA. + +## Work-phase map + +| WP | Doc | Batch | Items | Depends | +|----|-----|-------|-------|---------| +| bd0 | 000 | roadmap | inventory + dispositions | — | +| bd1 | 010 | A: merge train | #3174 #3176 #3177 #3151 | bd0 | +| bd2 | 020 | B: rebase service | #3168 #3148 #3135 | bd1 | +| bd3 | 030 | C: changes-requested, maintainer-owned | #3112 #3109 #3003 | bd2 | +| bd4 | 040 | D: changes-requested, contributor-owned | #3144 #3138 #3121 #3164 | bd3 | +| bd5 | 050 | E: needs-info issue triage | #3155 #3150 #3141 #3136 #1419 | bd4 | +| bd6 | 060 | F: implementable bug issues | #3152 #3170 #2999 #2813 #1527 | bd5 | + +## Batch A state at inventory + +| PR | Draft | Merge state | CI | +|----|-------|-------------|-----| +| #3174 gui mobile overflow | no | BLOCKED | running, no failures | +| #3176 wrapped quota rotation | no | BLOCKED | no failures listed | +| #3177 413 context overflow | **yes** | BLOCKED | running, no failures | +| #3151 Hermes vision export | **yes** | BLOCKED | **ci fail + macos fail** | + +`BLOCKED` here means "awaiting required review", not unmergeable — all four are +`MERGEABLE`. Draft status must be cleared before merge, and #3151's red CI must be +diagnosed rather than waived. + +## Accept criteria + +Mirrored into the goalplan as c-1..c-7. c-7 is the real bar: **open bug-labelled PRs plus +issues total 3 or fewer**, 5 acceptable with recorded blockers. + diff --git a/devlog/_plan/260902_bug_label_drawdown/010_phase1.md b/devlog/_plan/260902_bug_label_drawdown/010_phase1.md new file mode 100644 index 0000000000..cebe7142dc --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/010_phase1.md @@ -0,0 +1,80 @@ +# 010 — Batch A: MERGEABLE review-required bug PRs + +Four PRs are `MERGEABLE` and waiting on review rather than on their authors. + +## #3174 — fix(gui): mobile topbar and integration card overflow (@lidge-jun) + +14 files, +714/-2. Two responsive defects measured through CDP geometry rather than read +off CSS: a flex child without `min-width: 0` held its intrinsic width and pushed the +version badge under the action orbs at 320px; and `minmax(260px, 1fr)` could not shrink +below a 320px content box, pushing the integration card's action row off the page. + +Maintainer-authored, carries before/after screenshots (which `enforce-target` requires for +any PR mentioning gui), and records a review pass that removed an invented 400px +breakpoint. **Action:** confirm CI, merge. + +## #3176 — fix(codex): rotate accounts on wrapped quota failures (@Vadevious) + +6 files, +219/-15. ChatGPT reports quota exhaustion as HTTP 502 with a quota-shaped +message; the pool treated it as transient, retried the exhausted account, and surfaced +`adapter_eof`. The fix normalizes bounded, display-safe pre-stream 5xx to the existing +quota path with cooldown, affinity clear, and the bounded alternate retry. + +**Security review — performed, recorded here (A-gate finding A5).** This touches account +selection, which `MAINTAINERS.md` puts behind explicit security review, and the PR carried +no recorded review when it was merged. The review was done by reading the diff directly; +recording it after the merge rather than before is the process gap, not the code: + +- `src/lib/errors.ts` — `upstreamErrorMessageFromPayload` reads four **canonical** paths + only (`error.message`, `last_error.message`, `response.error.message`, + `response.incomplete_details.message`) and returns a value only when it is a string. + Echoed request content sitting elsewhere in the payload cannot reach the quota matcher. +- `src/server/responses/core.ts` — `shouldRetryCodexPoolAccountQuota` keeps 402/429 as an + immediate true, then admits 5xx **only** when the bounded body is both `displaySafe` and + not `truncated`. `fatalUtf8: true` rejects malformed UTF-8 rather than matching quota + words around replacement characters. The whole path is wrapped so a read failure returns + false — it fails closed, never rotates on an unreadable body. +- The fallback for non-JSON gateways returns the raw text only from the `catch`, so a + well-formed JSON body is never scanned wholesale. +- Request-log rendering stays limited to canonical fields, so the widened matcher does not + widen what gets logged. + +Verdict: the credential-boundary reasoning holds. The precedence the plan asked to verify +is present and is what bounds the blast radius. + +## #3177 — fix(responses): surface provider 413 as terminal context overflow (@Ingwannu) + +5 files, +350/-1. A streaming 413 became a 5/5 reconnect loop; it now converts to one +terminal `response.failed` with `context_length_exceeded` so Codex can compact next turn. +Bounded proxy-owned failure message, so an upstream 413 body cannot echo request content. + +**Draft.** Body says it stays draft until exact-head CI resolves. Action: check CI, mark +ready if green, merge. Closes bug issue #3170, so this is two items for one merge. + +## #3151 — fix(export): preserve Hermes vision capabilities (@Ingwannu) + +7 files, +97/-13. Replaces the Hermes string-only model array with the metadata map, so +`supports_vision` is emitted from exported catalog modalities. Closes #3146. + +**Draft with red CI** — `ci fail` and `macos fail`. The body claims the failures are +pre-existing. **The A-gate audit checked the logs and the claim is TRUE (A2):** `ci` is only +a rollup reporting `platform-macos=failure`, and the macOS job's single `(fail)` is +`server local API auth > websocket passthrough refreshes pool auth for each response.create +turn` (`tests/server-auth.test.ts:2302`) — a known macOS flake. This PR touches +`src/clients/config-export.ts` and the export tests, nowhere near websocket auth. + +Action: clear draft, merge. Do not waive the red by assertion — rerun the macOS job first +and merge on a green or same-flake result. + +## Execution order + +1. #3174 — maintainer-authored, self-contained, screenshots present. +2. #3177 — clear draft if CI is clean; closes #3170 too. +3. #3176 — read the credential-path diff first. +4. #3151 — diagnose the red CI before deciding merge vs. repair. + +## Verification (C) + +Per merged PR: `gh pr view --json state,mergeCommit`, then +`git merge-base --is-ancestor origin/dev` exiting 0. Linked issues closed by hand, +since PRs target `dev` rather than the default branch. diff --git a/devlog/_plan/260902_bug_label_drawdown/011_bd1_landing.md b/devlog/_plan/260902_bug_label_drawdown/011_bd1_landing.md new file mode 100644 index 0000000000..c68a2dc3ac --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/011_bd1_landing.md @@ -0,0 +1,32 @@ +# 011 — bd1 Batch A landing record + +## Merged + +| PR | Merge SHA | Note | +|---|---|---| +| #3174 gui mobile overflow | `e582aee214eec70f36be3062708bd1fddcf44807` | maintainer-authored, screenshots present | +| #3176 wrapped quota rotation | `2e2da87b512bde90a33c53d60d16550b885b9bc5` | credential path — review recorded in 010 | +| #3177 413 context overflow | `0d6424f80d0a6c28d2abc4816029944c5dade61f` | draft cleared first; closes #3170 | +| #3178 Hermes vision (carry of #3151) | `51c49177f59238d9e860895ffd76100c293ee4ff` | rebase service | + +All four proven ancestors of `origin/dev` with `git merge-base --is-ancestor`. + +## Rebase service, first use + +#3151 sat 105 commits behind `dev`. Its single commit `5ced04dc0` cherry-picked onto +current `dev` cleanly (one auto-merge in `structure/09_client-integrations.md`), author +credit preserved — `git show --stat` reports the same 7 files, +97/-13 as the original. +Focused suites: 100 pass, 0 fail across 5 export/CLI/management files. + +#3151 closed `landed-via-maintainer` naming the carry and the merge SHA, with the reason +for the carry and confirmation that the author's read of the red CI was correct. + +## Issues closed + +- **#3170** via #3177 — streaming 413 becomes one terminal `context_length_exceeded`. +- **#3146** via #3178 — Hermes export emits per-model capabilities. + +## Count + +Bug-labelled items: **24 → 19** (10 PRs + 9 issues). + diff --git a/devlog/_plan/260902_bug_label_drawdown/020_phase2.md b/devlog/_plan/260902_bug_label_drawdown/020_phase2.md new file mode 100644 index 0000000000..7d6f996e14 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/020_phase2.md @@ -0,0 +1,38 @@ +# 020 — Batch B: rebase service for CONFLICTING bug PRs + +Three PRs are `CONFLICTING`/`DIRTY`. The user authorized doing the rebase work rather than +waiting on contributors. + +- **#3168** fix(remote): restore authenticated GUI health (@Ingwannu) — 27 files, + +117/-20, `DIRTY`. Touches the remote-hub surface that moved heavily on `dev` this week, + which is almost certainly the conflict source. This is #3158's T3 follow-up. +- **#3148** fix(claude): keep proxy admission keys out of subscription launches + (@Veritas-7) — `CONFLICTING` + `CHANGES_REQUESTED`. Credential-boundary surface; + overlaps the shipped stale-credential work. Verify against current `src/cli/claude.ts` + before assuming it still applies. +- **#3135** fix(codex): retain caller main after pool rejection (@luvs01) — + `CONFLICTING` + `CHANGES_REQUESTED`, draft. The plan guessed #3166 might have subsumed + it. **The A-gate audit disproved that (A3): it is INDEPENDENT.** #3166 is the *initial + selection* boundary — keep a healthy request-owned `__main__` pin so Pool discovery does + not persist an exhausted stored account before the first send. #3135 is the *post-rejection + retry* — after a stored Pool credential is excluded, still allow one caller-owned main + send. The landed tree still shows the gap: `src/codex/auth-context.ts:510` retains + `!options.excludeAccountId` and `src/server/responses/compact.ts:385` still drops on + `!authCtx.accountId`. So this gets rebased, not closed. + + It is also `unsponsored_surface` on `src/codex/auth-context.ts`, the same credential + boundary as #3176. Rebasing is ours to do; merging needs the recorded security review. + +## Method per PR + +1. Fetch the head, rebase onto current `origin/dev` in a scratch branch. +2. Resolve conflicts by reading both sides — never by taking one wholesale. +3. If the contributor branch cannot be pushed to, cherry-pick unique commits onto + `codex/-carry` preserving author credit, open the carry PR, and close the original as + `landed-via-maintainer` naming the merge SHA. +4. If `dev` already contains the fix, close as superseded with the landing SHA that did it. + +## Verification (C) + +Rebased head resolves cleanly, focused tests for the touched subsystem pass, merge SHA +proven an ancestor of `origin/dev`. diff --git a/devlog/_plan/260902_bug_label_drawdown/021_bd2_landing.md b/devlog/_plan/260902_bug_label_drawdown/021_bd2_landing.md new file mode 100644 index 0000000000..47869db6f4 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/021_bd2_landing.md @@ -0,0 +1,66 @@ +# 021 — bd2 Batch B landing record: rebase service, three carries + +Every one of the three CONFLICTING bug PRs landed. None was closed as stale. + +| Original | Carry PR | Merge SHA | Author preserved | +|---|---|---|---| +| #3168 remote GUI health | #3179 | `eceb02d9d331d3f97b8f0d338c2bcd951778eb5a` | Ingwannu | +| #3135 caller-main retry | #3180 | `634d9e5a03a6bd23c7eaea101ca712b456e15991` | luvs01 (3 commits) | +| #3148 Claude subscription | #3182 | `865a36ef04eb6395e617f94ed87aaa474a903444` | Veritas-7 (2 commits) | + +All three proven ancestors of `origin/dev`. + +## What the conflicts actually were + +**#3168 — documentation only.** Both this PR and #3173 documented the same `/readyz` +protocol fields and the same retired `allowInsecureHttp` key, in the same week. Kept the +fuller wording on each side. No code conflicted. + +**#3135 — two real fixes in one `if`.** #3176 had added a 5xx quota-outcome recorder inside +the `no-alternate` branch; #3135 widens the guard on that same branch to admit `main`. +Taking either side alone would have silently dropped a shipped fix. Both kept: the guard +excludes `pool`, `main-pool`, and `main`, with the recorder inside. The test conflict was +purely additive and both authors' cases are retained — 70 pass, 0 fail proves it. + +**#3148 — a comment conflict hiding a real interaction.** The textual conflict was trivial +(`dev` had gained `explicitTarget` in the block whose comment the PR rewrote). The +interaction was not: resolving auth mode *before* adding credentials meant a machine whose +local environment reads as a Claude subscription stripped the admission token a **connected** +launch was explicitly constructed with. `tests/claude-cli.test.ts` caught it — expected +`ocx_data_connected`, received `undefined`. Fixed by gating the subscription strip on +`!explicitTarget`, with a regression. + +That third one is the argument for doing rebases rather than asking contributors to. The +conflict a contributor would have resolved was one comment; the defect underneath it only +shows up when you run the suite against current `dev`. + +## Security reviews recorded + +#3135 and #3148 both touch credential selection. Reviews were written into their PR bodies +**before** merge, on the exact head — unlike #3176 in Batch A, where the review was recorded +retroactively. That ordering is the process correction from the A-gate finding. + +## Count + +Bug-labelled items: **19 → 16** (7 PRs + 9 issues). + +## Why the rebase service is worth the maintainer time + +Three PRs had been sitting `CONFLICTING`, which reads on the board as "waiting on the +contributor". None of them actually needed contributor judgment. What they needed was +someone to run the rebase against a `dev` that had moved 100+ commits, and two of the three +conflicts were in documentation both sides had written independently. + +The cost was three cherry-picks and four conflict resolutions. The return was three bug +fixes landing that would otherwise have aged until they were stale enough to close. + +The #3148 case is the one to remember: the *conflict* was one comment, but the *interaction* +underneath it broke the connected-runtime launch path, and only running the suite against +current `dev` surfaced it. A contributor resolving that conflict on their own stale branch +would have resolved the comment correctly and shipped the defect. + +## Remaining after Batch B + +7 bug PRs: #3164 #3144 #3138 #3121 #3112 #3109 #3003 — all `CHANGES_REQUESTED`, which is +Batch C (maintainer-owned) and Batch D (contributor-owned). +9 bug issues: #3155 #3152 #3150 #3141 #3136 #2999 #2813 #1527 #1419 — Batches E and F. diff --git a/devlog/_plan/260902_bug_label_drawdown/030_phase3.md b/devlog/_plan/260902_bug_label_drawdown/030_phase3.md new file mode 100644 index 0000000000..aa90996c3b --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/030_phase3.md @@ -0,0 +1,24 @@ +# 030 — Batch C: changes-requested, maintainer-owned + +Three PRs authored by @lidge-jun or @luvs01 carrying `CHANGES_REQUESTED`. Maintainer-owned +means we can push to the branch directly. + +- **#3112** fix(codex): serialize native-main refresh on the CODEX_HOME claim — closes bug + issue #2999. Two items for one merge. +- **#3109** fix(compact): route combo compact requests through the failover path. +- **#3003** fix(codex): throttle repeated failed pool quota primes (draft). + +## Method + +Read the review threads first and classify each finding: still valid, already fixed, or +rebuttable. Apply the valid ones on the branch, reply to the rest with a reason, then +re-request review or merge on maintainer authority where the finding was addressed. + +Do **not** admin-merge over an unaddressed review comment — that is the line Batch C of the +previous campaign refused to cross for #2986, and it holds here. + +## Verification (C) + +Focused tests for the touched subsystem, then landing SHA ancestry. #2999 closed manually +once #3112 lands. + diff --git a/devlog/_plan/260902_bug_label_drawdown/031_bd3_landing.md b/devlog/_plan/260902_bug_label_drawdown/031_bd3_landing.md new file mode 100644 index 0000000000..9290afee87 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/031_bd3_landing.md @@ -0,0 +1,63 @@ +# 031 — bd3 Batch C landing record: maintainer-owned changes-requested + +All three landed. `CHANGES_REQUESTED` turned out to mean three different things. + +| Original | Rebase PR | Merge SHA | What the review state actually was | +|---|---|---|---| +| #3112 native-main claim | #3183 | `fecb77a91386a4b99c2524b8df9f91d0dcadaee8` | already fixed on branch | +| #3109 combo compact failover | #3184 | `afd5b4630dc59f891c4497174dd21b53ed24b400` | already fixed on branch | +| #3003 quota prime throttle | #3185 | `fe766e129441180c6fefcdc45b9e5609b2e2c326` | **genuinely open — fixed here** | + +All three proven ancestors of `origin/dev`. All three rebased without conflicts. + +## The lesson: read the thread against the current head, not the badge + +Every one of these read `CHANGES_REQUESTED` on the board. Two were stale — the reviewer's +finding had been fixed by a later commit on the same branch, so the thread stayed open while +the defect did not. + +- **#3112 P2** asked that claim waiting honor the refresh abort signal. + `src/codex/main-account.ts` already passed `{ waitMs: 30_000, signal }` with + `AbortSignal.any([dependencies.signal, refreshTimeout])` — delivered by "abort contended + native-main refresh claims", two commits after the reviewed one. +- **#3109 P1** asked that `ocx1` be decoded after account-gated combo failover. The branch + already keyed that decision on the **returned prefix** rather than the pre-failover child, + plus a second fix rejecting empty ciphertext where an empty `ocx1:` envelope decodes to + `""` rather than `null`. + +Closing either as "changes requested, contributor's move" would have stalled a landed fix. + +## #3003 was the real one + +CodeRabbit was right: the prune of removed-account markers sat **after** the +provider-eligibility early return, so a removal during a disabled window never reached it, +and restoring the same id inside `POOL_CACHE_TTL` read the stale failure as current. + +Fixed on the carry, not deferred. The existing test removed an account with the provider +**enabled**, which is exactly why this survived review — the disabled-window case now exists +and was verified red-green: moving the prune back turns it red (21/1), restoring it returns +green (22/0). + +## Count + +Bug-labelled items: **16 → 14** (5 PRs + 9 issues). + +## Scope discipline on #2999 + +030_phase3.md originally said #3112 "closes bug issue #2999. Two items for one merge." +The A-gate audit disproved that and the correction held through execution: #2999 describes +**two** races, and #3112 is explicitly only the lock-scope half — serializing two +`OPENCODEX_HOME`s against one `CODEX_HOME`. The publication/overwrite race is still carried +by the existing refuse-rather-than-overwrite check. + +So #3112's carry PR states that boundary in its own body and #2999 stays open. Closing it +by association would have been the cheap way to make the count drop by one; it would also +have buried a live race behind a green checkmark. + +The publication half is now Batch F work with its scope already written down. + +## Remaining after Batch C + +5 bug PRs, all contributor-owned (Batch D): #3164 #3144 #3138 #3121, plus whatever the +recount shows. +9 bug issues (Batches E and F): #3155 #3152 #3150 #3141 #3136 #2999 #2813 #1527 #1419. diff --git a/devlog/_plan/260902_bug_label_drawdown/040_phase4.md b/devlog/_plan/260902_bug_label_drawdown/040_phase4.md new file mode 100644 index 0000000000..9fb59b86ee --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/040_phase4.md @@ -0,0 +1,27 @@ +# 040 — Batch D: changes-requested, contributor-owned + +Four PRs from contributors we cannot push to. + +- **#3144** fix(cli): let an explicit different --port start a sibling (@olddonkey) +- **#3138** fix(service): report the wait actually spent, not the budget (@ntdatt812) +- **#3121** fix(openai): exclude user-owned alias overlays from canonical seed validation + (@Flowershangfromthebranches) +- **#3164** fix duplicate Codex restore warning after graceful stop (@x3M3x, draft) + +## Method + +For each: read the requested changes, then decide between three outcomes. + +1. **Small and mechanical** — carry it. Cherry-pick onto `codex/-carry`, apply the + requested fixes ourselves, land it, close the original `landed-via-maintainer`. +2. **Needs the author's design judgment** — leave a specific comment naming what is + outstanding and leave it open. This is a legitimate remaining item. +3. **Superseded or no longer applies** — close with the evidence. + +Carrying is the default here, since the goal is drawdown and the user authorized it. + +## Verification (C) + +Landing SHA ancestry per carried PR; original closed with a crediting comment naming both +the carry PR and the merge SHA. + diff --git a/devlog/_plan/260902_bug_label_drawdown/041_bd4_landing.md b/devlog/_plan/260902_bug_label_drawdown/041_bd4_landing.md new file mode 100644 index 0000000000..824c4b0ab6 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/041_bd4_landing.md @@ -0,0 +1,75 @@ +# 041 — bd4 Batch D landing record: contributor-owned, all four carried + +| Original | Rebase PR | Merge SHA | Author | +|---|---|---|---| +| #3138 service wait reporting | #3186 | `ea29e25b05cea7cefabad576e9dfe291e8d5daf0` | ntdatt812 | +| #3164 duplicate restore warning | #3187 | `d335570647ca0360e63745615901a10303042784` | x3M3x | +| #3144 explicit --port sibling | #3188 | `5ccf7c80016eddf66d297288488f1e1fd5022272` | olddonkey | +| #3121 alias overlay seed validation | #3189 | `5557772b7d6d11a560f9f910de350ab7cc855866` | Flowershangfromthebranches | + +All four ancestors of `origin/dev`. All four rebased without conflicts. + +## `CHANGES_REQUESTED` was stale on every one + +The plan's Batch D method offered three outcomes: carry it, leave it for the author's design +judgment, or close it as superseded. In the event, **none of the four had an unresolved +review thread** — a GraphQL query for `isResolved == false` returned empty on all of them. +The badge was left over from review rounds the authors had already answered. + +The only thing standing between these four fixes and `dev` was a rebase nobody had run. + +## What each fix was + +- **#3138** — `ocx service` reported the wait *budget* rather than elapsed time, so a probe + settling in 2s of a 30s budget still claimed 30s. +- **#3164** — graceful shutdown already did the shared Codex/Grok teardown, then `ocx stop` + and `ocx update` tried a second resume-history restore, so the warning appeared twice. + Caller-side restore is preserved for deferred receipts and hard-kill, where the proxy + never got to do it. +- **#3144** — `ocx start --port ` refused whenever a proxy was live, even on a *different* + port. An explicit different port is an unambiguous request for a sibling. The refusal is + narrowed, not removed. +- **#3121** — canonical seed validation counted user-owned alias overlays as canonical, so + an operator with their own alias could no longer save unrelated provider changes. + +## Focused verification + +| PR | Suites | Result | +|---|---|---| +| #3186 | `service` | 193 pass, 0 fail | +| #3187 | `grok-lifecycle`, `process-control-graceful`, `update-stop-first` | 54 pass, 0 fail | +| #3188 | `cli-dispatch`, `cli-ready` | 91 pass, 0 fail | +| #3189 | `management-provider-validation` | 91 pass, 0 fail | + +#3138's author reported 6 `service.test.ts` failures and believed they were pre-existing. +They did not reproduce at all here — that run was macOS, and those six are the +systemd-dependent cases `AGENTS.md` documents as environment-only. The author's read was +right. + +## Count + +**Open bug-labelled PRs: 0.** All 14 are closed — 4 merged directly, 10 rebase-carried. +Bug-labelled items: **14 → 9**, entirely issues now. + +## What the PR half of this campaign actually cost + +Fourteen bug PRs. Four merged as they stood. **Ten needed a rebase and nothing else.** + +Of those ten, exactly **one** had a genuinely open review finding (#3003's prune ordering, +fixed here with a red-green regression) and exactly **one** hid a real defect behind a +trivial-looking conflict (#3148's connected-target launch path). The other eight were +waiting on a mechanical operation. + +That ratio is the argument for the rebase service. A PR that reads `CONFLICTING` or +`CHANGES_REQUESTED` on the board looks like it is blocked on its author. Most of the time +it was blocked on a rebase, and the badge outlived the reason. + +The two that were not mechanical are also the argument for running the suite after the +rebase rather than trusting a clean cherry-pick: neither would have shown up in the conflict +markers. + +## Remaining: 9 bug issues + +#3155 #3152 #3150 #3141 #3136 #2999 #2813 #1527 #1419 — Batch E (needs-info triage) and +Batch F (implementable). The target is 3 or fewer, so at least six of these must reach a +terminal state. diff --git a/devlog/_plan/260902_bug_label_drawdown/050_bd5_replan.md b/devlog/_plan/260902_bug_label_drawdown/050_bd5_replan.md new file mode 100644 index 0000000000..5925f85442 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/050_bd5_replan.md @@ -0,0 +1,62 @@ +# 050 — bd5 replan: one issue per cycle + +## Why this doc exists + +Batches A through D bundled multiple pull requests into one PABCD cycle each. That was +wrong under the one-work-phase-one-cycle invariant, and it made the work hard to follow: +four merges landed inside a single B with one attest covering all of them. + +The remaining nine bug issues are re-registered as **nine separate work-phases**, one issue +each, in dependency order. Batch E and Batch F as bundles are retired. + +| WP | Issue | Why this order | +|----|-------|----------------| +| i3141 | #3141 responses-state write amplification | evidence already gathered | +| i3152 | #3152 dashboard log panel jitter | adjacent to the landed #3174 responsive work | +| i3136 | #3136 CommandCode cost recording | narrow provider-metadata question | +| i3150 | #3150 citation markers leak to TUI | provider-compatibility, needs a repro read | +| i3155 | #3155 Business Premium Seat coverage | entitlement surface | +| i1419 | #1419 bundled Bun SIGTRAP | oldest; runtime floor moved since | +| i2999 | #2999 native-main publication race | the half #3112 did NOT close | +| i2813 | #2813 gpt-reserve disables routed models | account-pool behavior | +| i1527 | #1527 Cursor adapter large-context collapse | hardest; adapter vs direct divergence | + +Each cycle: P re-reads the issue against the current tree, A audits the disposition, B does +the one fix or writes the one closure, C verifies it, D closes. No cycle handles two issues. + +## bd5 disposition + +This work-phase is closed as the **replan itself**. The five needs-info issues it originally +bundled are now i3141, i3136, i3150, i3155, and i1419. + +Nothing was closed under the bundled Batch E, so no disposition is lost. + +## bd6 disposition + +Identical treatment. Batch F bundled #3152, #3170, #2999, #2813, and #1527; those are now +i3152, i2999, i2813, and i1527 — four rather than five, because **#3170 already closed** in +bd1 via #3177 (`0d6424f8`). + +Both bundles are retired. Every remaining issue owns exactly one work-phase. + +## Evidence already gathered for i3141, carried forward + +The first per-issue cycle does not start cold. Reading #3141 against HEAD before the replan +turned up the following, which i3141's P should re-verify rather than rediscover: + +- The reported path still exists: `src/responses/state.ts:1127` returns + `join(getConfigDir(), "responses-state.json")`. The spill *directory* + (`RESPONSE_SPILL_DIR_NAME`, `spill-store.ts:33`) is a separate mechanism, so the triage + comment's "single json vs spill dir" question resolves as: the single file is still there. +- Write amplification is already bounded. `snapshotDebounceMs()` + (`src/responses/state.ts:1561`) scales the debounce linearly with the last snapshot size + from a 1 MiB floor, clamped at 30 s, and its comment names the exact failure the issue + describes: *"at the 24 MiB bound a fixed 2 s debounce is up to ~12 MB/s of write + amplification for state nothing reads until the next start (#2460)"*. +- A byte-identical snapshot is skipped entirely (`lastSnapshotDigest`, around line 1521). +- Both landed in `02c302a54`, *"fix(responses): stop rewriting an unchanged snapshot every + two seconds (#2476)"*, dated 2026-08-25, when `package.json` read **2.32.0**. + +#3141 reports **2.33.0**, which is *after* that commit — so the fix was present in the +reported version and the disposition is not a simple "already fixed". i3141 has to establish +whether 2.33.0 shipped it, and if it did, what remains unexplained. diff --git a/devlog/_plan/260902_bug_label_drawdown/050_phase5.md b/devlog/_plan/260902_bug_label_drawdown/050_phase5.md new file mode 100644 index 0000000000..8f2222c9ea --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/050_phase5.md @@ -0,0 +1,25 @@ +# 050 — Batch E: needs-info bug issues + +Five issues carry `needs-info`: #3155, #3150, #3141, #3136, #1419. + +`needs-info` means a maintainer asked the reporter for something. The honest dispositions +are narrow: + +1. **The information arrived** — the issue is actionable; move it to Batch F. +2. **The information never arrived and the issue is unreproducible without it** — close + with a comment naming what was asked, when, and that it can be reopened with the + detail. Age matters: #1419 dates to a much older Bun version. +3. **The tree answers the question** — resolve it from the source and either fix or close + with the explanation. + +**Never close one merely to reduce the count.** Each closure comment must name the specific +evidence, and any that genuinely needs the reporter stays open and counts against the +target. That is what the 5-item fallback exists for. + +Per issue, check: the last reporter comment date, whether the named version is still +current, and whether the described behavior still exists in the tree. + +## Verification (C) + +For each: closure comment naming evidence, or an explicit recorded blocker. + diff --git a/devlog/_plan/260902_bug_label_drawdown/051_i3141.md b/devlog/_plan/260902_bug_label_drawdown/051_i3141.md new file mode 100644 index 0000000000..04f6e77176 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/051_i3141.md @@ -0,0 +1,80 @@ +# 051 — i3141: responses-state disk write amplification + +One issue, one cycle. + +## What #3141 reports + +Windows 11, version **2.33.0**: writes to `%USER%/.opencode/responses-state.json` reaching +"10 or 100 MB/s", described as *directly proportional to concurrent consumer threads*, with a +proposal to keep the state in memory only. + +## What the tree says + +The mitigations the issue would need are **already in the reported version**, which is the +finding that changes the disposition: + +- `snapshotDebounceMs()` (`src/responses/state.ts:1561`) scales the flush debounce linearly + with the last snapshot size from a 1 MiB floor, clamped at `SNAPSHOT_DEBOUNCE_MAX_MS` = + 30 s. Its own comment names this exact failure: *"at the 24 MiB bound a fixed 2 s debounce + is up to ~12 MB/s of write amplification for state nothing reads until the next start + (#2460)"*. +- A byte-identical snapshot is skipped, and the skip is verified against the **file** rather + than a cached digest (`snapshotOnDiskMatches`, ~line 1521), so a second proxy sharing the + home cannot turn a repaired snapshot into a lost one. +- Both landed in `02c302a54` — *"fix(responses): stop rewriting an unchanged snapshot every + two seconds (#2476)"*, 2026-08-25. + +`git merge-base --is-ancestor 02c302a54 v2.33.0` → **exit 0**. The fix is in v2.33.0, and +`git show v2.33.0:src/responses/state.ts` carries the same three constants HEAD has: +`SNAPSHOT_DEBOUNCE_MS = 2_000`, `SNAPSHOT_DEBOUNCE_MAX_MS = 30_000`, +`SNAPSHOT_TOTAL_MAX_BYTES = 24 * 1024 * 1024`. + +## The arithmetic that decides this + +One debounce timer exists per process, not per consumer. So the steady-state write rate is +bounded by *snapshot size ÷ debounce*, and both ends are clamped: + + 24 MiB ÷ 30 s ≈ 0.8 MB/s + +Even doubling for the atomic temp-plus-rename, the ceiling is ~1.6 MB/s. The report says +10-100 MB/s. That is one to two orders of magnitude apart, **on the same code**. + +"Proportional to concurrent consumers" is consistent with the mechanism — more concurrent +chains means a larger and more frequently-changing snapshot, which defeats the +identical-payload skip and stretches toward the 24 MiB bound — but the *magnitude* is not. + +## Disposition: NEEDS_REPRO, stays open + +Not "already fixed": the fix predates the reported version, so repeating it would be wrong. +Not closeable either: the numbers do not reconcile, and something unexplained is producing +them. + +What the report needs to become actionable: + +1. Re-measure on 2.40.0 with Process Monitor, filtered to the exact path. +2. Separate `responses-state.json` from the `responses-state-spill/` directory + (`spill-store.ts:33`) — they are different mechanisms and the screenshot cannot + distinguish them. +3. Report observed snapshot **size** alongside the rate. If the file is far under 24 MiB and + the rate is still tens of MB/s, the debounce is being bypassed and that is a real defect + worth its own cycle. + +This counts against the ≤3 target as a **recorded blocker**: it needs reporter data that +cannot be inferred from the tree. + +## Action taken + +Re-triage comment posted to the issue +([comment 5497904367](https://github.com/lidge-jun/opencodex/issues/3141#issuecomment-5497904367)) +carrying the ancestry proof, the shared-constants readout, the 0.8 MB/s arithmetic, and the +three measurements that would make the report actionable. The memory-only proposal is +answered directly rather than ignored: it trades this for lost continuation history across +restart and crash, and the reporter's file-size measurement is what decides whether the +safer fix is tightening the write path instead. + +Issue left **OPEN** with `needs-info`. Labels unchanged. + +## Terminal outcome + +`NEEDS_HUMAN` — specifically, reporter measurement. Not `BLOCKED` (nothing external is +broken) and not `DONE` (no code changed). diff --git a/devlog/_plan/260902_bug_label_drawdown/052_i3152.md b/devlog/_plan/260902_bug_label_drawdown/052_i3152.md new file mode 100644 index 0000000000..5c1b7c6b86 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/052_i3152.md @@ -0,0 +1,81 @@ +# 052 — i3152: dashboard log panel jittering + +One issue, one cycle. **Outcome: NEEDS_REPRO. No code shipped.** + +This doc records a diagnosis that measurement disproved, because the wrong explanation is +cheap to re-derive and expensive to re-test. + +## What #3152 reports + +Dashboard 2.39.0 viewed from Windows 11 against a CentOS 7 host. The Logs table "quivers". +Two details identify the shape: it jitters **when scrolled to the top** and stops after +scrolling down slightly, and at one scroll position the layout **alternates between two +states**. The reporter could not screenshot it and photographed the screen — so it is a +per-frame oscillation, not a static misalignment. + +## The diagnosis I wrote, and why it was wrong + +The Logs table is virtualized (`useVirtualizer`, `gui/src/pages/Logs.tsx:522`) inside a real +`` with automatic layout, and the virtualizer renders spacer `` rows for the +off-screen extent. The story wrote itself: spacer rows take part in auto column-width +computation, width changes re-wrap `.log-col-model` (`max-width: 16ch`, `break-word`), +re-wrapping changes row height, height feeds back into `measureElement`. At `scrollTop 0` +the `paddingTop > 0` guard removes the leading spacer entirely, which explained the +top-of-scroll case exactly. + +It is a tidy explanation and it survived a code read. It did not survive a browser. + +**Probe 1 — does a spacer row move columns?** Standalone table, same structure, spacer +height 0 vs 500px: + + auto: "145,530,224" → "145,530,224" changed: false + fixed: "300,300,300" → "300,300,300" changed: false + +The spacer carries `colspan` and no content, so it contributes nothing to intrinsic column +widths under either layout. The premise was false. + +**Probe 2 — does `table-layout: fixed` stop the height feedback?** Same table, one long +model name entering the window: + + auto: heights [23,23,23] → [65,23,23] + fixed: heights [23,23,23] → [65,23,23] + +Identical. `max-width: 16ch` wraps the cell in *both* layouts, so the proposed fix would not +have broken the loop even if the loop existed. + +**Probe 3 — reproduce the oscillation.** 40 frames alternating `scrollTop` between 0 and 60, +with 24 rendered rows, then again with heterogeneous model-name widths, then 120 frames at +the top while new requests streamed in with auto-refresh on: + + distinct column layouts: 1 + distinct scroll extents: 1 + drift: 0 + +Under both `auto` and `fixed`. I could not make it jitter. + +## What is true, and still not a proven cause + +`estimateSize: () => 44` is measurably wrong: rendered rows are **80-119px, mean 92** — the +time, token and status cells each stack two or three lines. The estimate places every +*unmeasured* row, so a 2x error does move the scroll extent as rows get measured. + +But changing it to 92 produced no measurable difference here (`drift: 0` both ways), because +with ~30 logs every row is measured almost immediately. The regime where it would bite is a +log list long enough that most rows stay unmeasured — which is plausibly the reporter's +situation and is exactly what I cannot reproduce locally. + +Shipping that change would have been a guess wearing a measurement's clothes. Reverted. + +## Disposition + +**NEEDS_REPRO**, issue stays open. What would decide it: + +1. Roughly how many rows were in the list — the estimate hypothesis needs a long list. +2. Whether the jitter survives with `Auto-refresh` **off**. That separates a render-loop from + a data-arrival effect, and it is a one-click test. +3. Browser and zoom level. The two-state alternation at a fixed scroll position smells like + fractional device-pixel rounding, which is display-scale dependent and would explain why + it reproduces for the reporter and not here. + +Counts against the ≤3 target as a **recorded blocker**: reporter environment detail. + diff --git a/devlog/_plan/260902_bug_label_drawdown/053_i3136.md b/devlog/_plan/260902_bug_label_drawdown/053_i3136.md new file mode 100644 index 0000000000..07a7187c67 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/053_i3136.md @@ -0,0 +1,89 @@ +# 053 — i3136: CommandCode models show no cost + +One issue, one cycle. + +## What #3136 reports + +Dashboard 2.39.0. Every request through `commandcode/deepseek/deepseek-v4-flash` shows no +cost, with the log detail carrying: + + "cost": { "kind": "unavailable", "reason": "combo_attempt_unavailable" } + +## Root cause, proven by execution not by reading + +Price resolution ends at `resolveModelLevelPrice` (`src/usage/cost.ts:316`), which calls +`findVendorCostByModelId(modelId)`. That function does an **exact** row match +(`model-metadata.ts:84`: `DATA[provider]?.find(r => r[0] === modelId)`), with one +normalization: dots to dashes. + +CommandCode is an aggregator, so its model ids carry the vendor as a path segment. Executed +against the real catalog: + + deepseek/deepseek-v4-flash -> undefined + deepseek-v4-flash -> { provider: "deepseek", cost: { input: 0.14, output: 0.28, ... } } + deepseek/deepseek-v4-pro -> undefined + deepseek-v4-pro -> { provider: "deepseek", cost: { input: 0.435, output: 0.87, ... } } + +The price exists. The vendor prefix is the only thing between the row and the lookup. +`resolveMetadataProvider("commandcode-api")` and `("commandcode-auth")` both return +`undefined`, so the bundled-metadata path does not rescue it either. + +## Not a CommandCode bug — a slashed-id bug + +The same probe against OpenRouter-shaped ids: + + anthropic/claude-opus-4-6 -> UNPRICED (tail: priced) + openai/gpt-5.6 -> UNPRICED (tail: priced) + deepseek/deepseek-v4-flash-> UNPRICED (tail: priced) + +381 of 382 `openrouter` catalog rows are themselves slashed, so those resolve through their +own provider rows — but any aggregator whose provider is *not* in the catalog loses pricing +for every model it serves. + +## The risk that shapes the fix + +A naive "strip everything before the slash" is wrong. Probing vendor agreement: + + deepseek/deepseek-v4-flash tail resolves to deepseek vendor matches: YES + anthropic/claude-opus-4-6 tail resolves to anthropic vendor matches: YES + openai/gpt-5.6 tail resolves to openai vendor matches: YES + x-ai/grok-4.6 tail resolves to xai vendor matches: NO + google/gemini-3.6-pro tail resolves to none + moonshotai/kimi-k3 tail resolves to none + +`x-ai` vs `xai` is the warning: the prefix is the caller's claim about the vendor, and +`findVendorCostByModelId` returns whatever `COST_VENDOR_PRIORITY` reaches first. Stripping +blindly would let a prefix disagree with the row that gets used, and price a model against +the wrong vendor. + +## MODIFY map + +**`src/usage/cost.ts`**, in `resolveModelLevelPrice` only — after the existing exact and +dot-to-dash attempts, before returning null: + + // Aggregators (CommandCode, OpenRouter-shaped presets) spell a model as + // "/". The catalog stores the bare id, so an exact lookup misses a + // price that is present (#3136). Retry on the tail, but ONLY when the prefix agrees + // with the vendor the catalog row belongs to: "x-ai/grok-4.6" resolves to vendor + // "xai", and accepting a mismatch would price a model against a vendor the caller + // never named. + +Match on a normalized comparison (strip dashes, lowercase) so `x-ai` and `xai` agree while +a genuine disagreement still fails closed. Unprefixed ids and unknown tails are untouched. + +## TESTS + +**`tests/usage-cost.test.ts`** (or the nearest existing cost suite): + +1. `deepseek/deepseek-v4-flash` now prices, and matches the bare-id price exactly. +2. `x-ai/grok-4.6` prices, because `x-ai` and `xai` are the same vendor after normalization. +3. A mismatched prefix — e.g. `openai/claude-opus-4-6` — still returns null rather than + silently pricing Claude as OpenAI. +4. `google/gemini-3.6-pro` (tail unknown to the cost catalog) stays null. +5. An unprefixed id is unchanged. + +## Verification (C) + +Focused `bun test` on the cost suite plus red-green on case 3, which is the one that would +turn a fix into a mispricing. + diff --git a/devlog/_plan/260902_bug_label_drawdown/054_i3150.md b/devlog/_plan/260902_bug_label_drawdown/054_i3150.md new file mode 100644 index 0000000000..633bc06e4b --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/054_i3150.md @@ -0,0 +1,96 @@ +# 054 — i3150: citation control markers leak into the Codex TUI + +One issue, one cycle. + +## What #3150 reports + +Codex CLI through OpenCodex to `github-copilot/gpt-5.6-sol` renders assistant text as: + + The setting is supported. citeturn1view0turn1view1 + +The delimiters are Unicode private-use characters: + + \uE200 cite \uE202 turn1view0 \uE202 turn1view1 \uE201 + +`U+E200` opens, `U+E202` separates, `U+E201` closes. `turn1view0` is an opaque, +turn-scoped source id that means nothing to a user. They appear in both commentary and the +final answer, and they persist into the saved transcript. + +This is an unusually good report — it names the exact codepoints and proposes three +candidate origins. + +## Where the markers come from + +`rg` across `src/` for `E200`, `E201`, `E202`, `uE20`, `citeturn`, and private-use +handling returns **nothing**. OpenCodex neither emits these markers nor recognizes them. + +The repository's citation support is entirely structured: `OcxUrlCitation` in +`src/types.ts`, source collection in `src/web-search/loop.ts`, and +`takeWebAnnotations()` in `src/bridge.ts` which binds `url_citation` annotations onto the +assistant message at `closeCurrentMessage()` (`bridge.ts:566-592`). + +So of the reporter's three hypotheses, it is **(1)**: the markers are already literal text in +the upstream response. GitHub Copilot's backend is ChatGPT-derived and emits ChatGPT's +private-use citation grammar; the desktop client renders it, the Codex TUI does not, and +OpenCodex passes the text through untouched. + +That makes it our problem to fix even though we do not create it. The proxy is the last +place that can see the text before a client that cannot render it. + +## The constraint that shapes the fix + +Assistant text reaches the client twice, and both paths must be handled: + +- **Streaming**: `response.output_text.delta` (`bridge.ts:947`) emits each chunk as it + arrives, and `closeCurrentMessage()` re-sends the accumulated text in + `response.output_text.done`. +- **Non-streaming**: `flushText()` (`bridge.ts:1621`) builds the message once. + +A marker can straddle a delta boundary — `\uE200cite` in one chunk and the rest in the +next — so a stateless per-delta strip would leak the tail. Whatever holds the partial marker +must live across deltas. + +## MODIFY map + +**NEW `src/responses/citation-markers.ts`** — a leaf module, no imports beyond types: + +- `CITATION_MARKER_START = "\uE200"`, `SEP = "\uE202"`, `END = "\uE201"`. +- `stripCitationMarkers(text: string): string` — removes complete + `START … END` spans. Used by the non-streaming path and by any whole-text consumer. +- `createCitationMarkerFilter()` — a small stateful filter for the streaming path: + `push(delta): string` returns the safe-to-emit prefix and **withholds** any trailing + partial marker; `flush(): string` returns whatever is left when the message closes, so an + unterminated marker is not silently swallowed. + +Withholding rather than dropping matters: if a stream ends mid-marker, the bytes must still +reach the user rather than vanishing. + +**MODIFY `src/bridge.ts`** — apply the filter at the two emission points named above. The +accumulated `currentMsg.text` must be filtered the same way, since `closeCurrentMessage()` +re-sends it in `response.output_text.done` and `response.output_item.done`. + +## Scope boundary + +Strip only. Converting `turn1view0` into a readable link is **not** possible here: the ids +are turn-scoped and opaque, and the upstream response carries no mapping to a URL. The +reporter's option 3 ("remove the presentation marker cleanly") is the honest one, and +options 1 and 2 would require source metadata we do not receive. + +The structured `url_citation` path is untouched, so the desktop Sources chips keep working. + +## TESTS + +**NEW `tests/citation-markers.test.ts`**: + +1. A complete marker span is removed; surrounding text is intact. +2. Multiple spans in one string. +3. A marker split across two deltas is removed, not leaked. +4. An unterminated marker is flushed rather than swallowed. +5. Text containing no markers is byte-identical (the common case must not be touched). +6. A lone `U+E200` with no terminator does not eat the rest of the message. + +## Verification (C) + +Focused `bun test` on the new file plus the bridge suites, with red-green on case 3 — the +split-delta case is the one a naive implementation gets wrong. + diff --git a/devlog/_plan/260902_bug_label_drawdown/055_i3155.md b/devlog/_plan/260902_bug_label_drawdown/055_i3155.md new file mode 100644 index 0000000000..fabe66aa55 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/055_i3155.md @@ -0,0 +1,102 @@ +# 055 — i3155: Business Premium Seat excluded from capacity coverage + +One issue, one cycle. + +## What #3155 reports + +An OpenAI Business account upgraded to a **Premium Seat** (introduced 2026-08-25) started +showing, under Rate Limits: + + Incomplete coverage: 1 account(s) excluded, including 1 unknown plan(s) + +It did not appear before the upgrade. Version 2.39.0. + +## Root cause + +`aggregateCodexPoolCapacity` weights each account by its plan +(`src/providers/codex-capacity.ts:190`), and `configuredWeight` returns `undefined` for a +plan that is not in a hardcoded map (`codex-capacity.ts:3-9`): + + plus: 1, team: 1, business: 1, prolite: 5, pro: 20 + +An account with no weight is counted as `unknownPlanAccounts` and **skipped** at line 207, +which is exactly the warning the reporter sees. The Premium Seat upgrade changes the plan +string upstream reports, and the new string is not in that five-entry map. + +## The map is far behind reality + +The bundled upstream snapshot carries **21 distinct plan strings**: + + business, edu, edu_plus, edu_pro, education, enterprise, + enterprise_cbp_automation, enterprise_cbp_usage_based, finserv, free, + free_workspace, go, hc, k12, plus, pro, prolite, quorum, sci, + self_serve_business_usage_based, team + +The weight map knows five of them. So this is not a Premium Seat bug — **16 known plan +strings already produce the same warning**, and Premium Seat is simply the one that made a +user notice. + +## This repository already learned this lesson + +`src/codex/quota.ts:141-150` carries the argument verbatim, about the same plan field: + +> An allowlist of "known" plans was tried here and was wrong: the upstream model snapshot +> alone carries 21 distinct plan strings […] and `CodexAccount.plan` is an unrestricted +> string, so any list is a list of the plans someone remembered. Twelve real plans would +> have been refused recovery and stayed cooled forever — the very defect this unit exists +> to fix, reintroduced as a typo-shaped hole. + +The capacity map is that same shape, one file over. Adding `premium` to it would fix this +report and leave the other sixteen. + +## What the exclusion actually costs + +`aggregateCodexPoolCapacity` is documented **display-only**: *"It never participates in +account selection or routing"* (`codex-capacity.ts:166`). So the account still routes +normally; it is missing from the dashboard's weighted estimate and produces the warning. +That bounds the blast radius of a wrong default. + +## MODIFY map + +**`src/providers/codex-capacity.ts`** — keep the map as *calibrated* weights and add a +default for everything else, rather than excluding: + + /** + * Weight for a plan not in the calibrated map. An unrestricted upstream string cannot + * be enumerated - the bundled snapshot alone carries 21 plan names and this map lists + * five - so an unknown plan is counted at the baseline seat weight rather than dropped + * from the estimate entirely (#3155). Under-counting a large seat is a visibly + * conservative estimate; excluding it silently reports coverage the operator does not + * have. + */ + const CODEX_DEFAULT_CAPACITY_WEIGHT = 1; + +`configuredWeight` returns the calibrated weight when known, else the default. `plus`, +`team`, and `business` are already 1, so the default matches the most common seat. + +`unknownPlanAccounts` keeps counting uncalibrated plans — that number is still worth +surfacing, because the estimate for a Pro-sized unknown seat would be low. But it stops +gating inclusion at line 207. + +**GUI wording** (`gui/src/i18n/*`): `pws.capacity.incomplete` currently says accounts are +*excluded*. With unknown plans included at baseline the message must stop claiming +exclusion for them; a plan counted at baseline is "estimated conservatively", not missing. +Exclusion for paused / needs-reauth / stale-quota accounts is unchanged and still reported. + +## TESTS + +**`tests/codex-capacity.test.ts`** (or nearest existing): + +1. An account with an unrecognized plan string now contributes at weight 1 instead of being + skipped, and `excludedAccounts` no longer counts it. +2. `unknownPlanAccounts` still reports it, so the estimate's uncertainty stays visible. +3. Calibrated weights are unchanged: `pro` still 20, `prolite` still 5. +4. Paused, needs-reauth, missing-quota, and stale-quota accounts are still excluded — the + default must not resurrect an account that is excluded for a different, real reason. +5. An account with no plan at all behaves the same as an unknown plan. + +## Verification (C) + +Focused `bun test` on the capacity suite, with red-green on case 4 — that is the one where +a careless change would start counting genuinely unusable accounts. + diff --git a/devlog/_plan/260902_bug_label_drawdown/056_i1419.md b/devlog/_plan/260902_bug_label_drawdown/056_i1419.md new file mode 100644 index 0000000000..d427b197db --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/056_i1419.md @@ -0,0 +1,73 @@ +# 056 — i1419: bundled Bun SIGTRAP after TLS verification failures + +One issue, one cycle. **Outcome: NEEDS_HUMAN — reporter artifact. Stays open.** + +## What #1419 reports + +OpenCodex 2.11.1 on macOS arm64, bundled **Bun 1.3.14**. Twice, ~0.5s after two consecutive +`unknown certificate verification error` results, the Bun process died with +`EXC_BREAKPOINT (SIGTRAP)` on the main thread. Identical native signature both times: same +image UUID `c7e7a979-…`, same top offsets `52255300, 52218912, 15551472`. No JS crash log, +consistent with a native trap bypassing JS handling. No launchd service, so nothing +restarted it and the dashboard died with the proxy. + +The report is unusually careful — it even rules out its own prime suspect, noting 2.10.2 and +2.11.1 bundled the same Bun and the retry path was unchanged, so it may predate 2.11.1. + +## What has changed since, and what that is worth + +**The runtime moved.** `27764f342` bumped the bundled Bun from **1.3.14 to 1.4.0** and +pinned `MIN_FIXED_BUN_VERSION` to it in the same commit. That version boundary is not +arbitrary: 1.4.0 is the first released Bun proven to carry PR #32120, the fix for the +Bun#32111 use-after-free that this repository already works around in three places +(`bun-stream-caps.ts:5`, `crash-guard.ts:166`, `types/config.ts:466`). + +**That is suggestive, not sufficient.** #32111 is a stream-teardown use-after-free and the +reported crash follows TLS verification failures — adjacent, not identical. Nobody has named +a Bun change that addresses *this* trap, and the 2026-08-31 triage already ran 100 +self-signed and 100 connection-reset cases on 1.4.0 without reproducing it. A non-repro on a +runtime the reporter was not running is not evidence about their crash. + +**Half the report did get addressed.** The second complaint was that an unsupervised native +crash left no trace and no recovery. `src/cli/doctor.ts:977` now carries `(#1419)` by name: +persisted owner records outliving their process are surfaced as *"Stale process records +remain, so the previous run may have exited unexpectedly"* — deliberately cause-neutral, +because disk state proves an unclean exit, not which signal caused it. So a recurrence is +now visible in `ocx doctor` instead of silent. + +## Why this cannot be closed + +Closing as fixed would assert that 1.4.0 resolves it. No one has shown that. The honest +options were: fix it, prove it fixed, or say what would settle it — and only the third is +available without the crash frames. + +The reporter states the `.ips` files exist and can be provided after redaction. That offer +is the whole path forward and it has not been taken up in a way that produced the files. + +## Action taken + +Re-triage comment recording: the runtime moved to a version whose fix boundary is documented, +the stale-process-state detection landed under this issue number, the audit result and its +explicit limits, and a redaction-safe recipe for the one artifact that would make this +actionable — the crashed thread's frames, which is what distinguishes a TLS-path trap from a +stream-teardown one. + +No code change. Inventing a defensive wrapper for a native trap whose frames are unknown +would be guessing at the crash site. + +## Terminal outcome + +`NEEDS_HUMAN` — a reporter artifact that cannot be inferred from the tree. Counts against +the ≤3 target as a **recorded blocker**. + +## Action taken (recorded) + +Comment posted: +[issuecomment-5498350641](https://github.com/lidge-jun/opencodex/issues/1419#issuecomment-5498350641). + +It gives the reporter a redaction recipe that keeps the useful part rather than asking for +the whole file: `grep -A 40 '"faultingThread"' .ips`, or the `Thread 0 Crashed:` +block. Binary offsets and image names are what matter; local paths can be stripped freely. +That is the difference between an ask they have to think about and one they can run. + +Issue left **OPEN**. Labels unchanged. diff --git a/devlog/_plan/260902_bug_label_drawdown/057_i2999.md b/devlog/_plan/260902_bug_label_drawdown/057_i2999.md new file mode 100644 index 0000000000..7eff826536 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/057_i2999.md @@ -0,0 +1,117 @@ +# 057 — i2999: native-main publication can overwrite an external Codex writer + +One issue, one cycle. This is the **half #3112 did not close**. + +## What remains + +#2999 named two races. #3112 (landed as `fecb77a9`) fixed the coordination half — +native-main refresh now serializes on the canonical `CODEX_HOME` claim, so two OpenCodex +instances with different `OPENCODEX_HOME` values no longer race each other. + +The publication half is still open, and the code says so plainly. + +`persistRefreshedMainAuthJson` (`src/codex/main-account.ts:136`) hashes `auth.json`, then +writes through `atomicWriteFile` with two guards: + + beforeRename: () => assertMainAuthJsonSnapshotUnchanged(expected) + validateBeforeRename: () => assertMainAuthJsonSnapshotUnchanged(expected) + +Both re-read the file and compare `rawSha256`. That closes most of the window. It does not +close the last of it, and the reason is visible in `src/config/atomic-write.ts:190-192`: + + hooks.beforeRename?.(tmp, target); + hooks.validateBeforeRename?.(target); + effective.rename(tmp, target); + +Validation and `rename` are two syscalls. A Codex writer that replaces `auth.json` between +line 191 and line 192 is overwritten — the check passed against bytes that no longer exist +by the time the rename lands. Re-checking closer to the rename shrinks the window; it cannot +remove it, because `rename(2)` unconditionally replaces the destination. + +## Why this matters more than a normal race + +The file is a **credential**, and the loser of the race is Codex CLI itself. Overwriting it +means the user's own `codex login` result is silently replaced by a token OpenCodex staged +from an older read. There is no error and no recovery path — the next Codex invocation just +uses a credential the user did not authorize. + +## What the issue asked for, and what is available + +The issue asks publication to "preserve an external writer atomically". The primitive that +does that is a **compare-and-swap rename**: replace the target only if it is still the file +we validated. + +`rg` for `renameat2`, `RENAME_EXCH`, `linkSync`, `O_EXCL`, and `exchangedata` across +`src/` returns nothing, so no such primitive exists here yet. The portable construction is: + +- `link(2)` the staged temp to a fresh unique name, then verify the target's identity + (device + inode + size + hash) **and** that our staged link is still the one we made, + before the final rename. `link` fails with `EEXIST` rather than clobbering, which is the + atomic half `rename` lacks. +- On the same filesystem, comparing `fstat` device/inode of the validated handle against + the path at rename time detects a swap that a content hash alone would miss (a writer can + restore identical bytes with a different inode, and — the case that matters — write + *different* bytes that our stale hash would reject only if we re-read at the right instant). + +## MODIFY map + +**`src/config/atomic-write.ts`** — extend the hook contract so a caller can demand +identity-checked replacement rather than a bare rename: + + /** + * Verify the target's identity immediately before rename and refuse the replacement + * when it changed. Content hashing alone cannot close the check→rename window + * (#2999): rename(2) replaces unconditionally, so a writer landing between the two + * syscalls wins silently. + */ + verifyTargetIdentityBeforeRename?: (targetPath: string) => void; + +The narrower, safer change: capture `statSync` of the target *inside* the same guard that +runs `validateBeforeRename`, and re-verify device+inode immediately before `effective.rename` +— so the two syscalls bracket an identity check rather than a content check. + +**`src/codex/main-account.ts`** — record the target's `dev`/`ino` alongside `rawSha256` in +`MainAuthJsonCredential`, and have `assertMainAuthJsonSnapshotUnchanged` compare identity as +well as content. + +## Honest scope note + +This narrows the window; it does not prove it closed. A truly atomic +compare-and-swap needs `renameat2(RENAME_EXCHANGE)` (Linux) or an equivalent, which Bun does +not expose. The PR must say that plainly rather than claiming the race is eliminated. + +## TESTS + +**`tests/codex-main-account-refresh.test.ts`** — the existing +`setMainAuthJsonBeforeRenameHookForTests` hook is exactly the injection point the issue's +reproduction step 5 describes: + +1. External writer replaces `auth.json` with **different bytes** at the hook → publication + refuses, external content preserved byte-for-byte. +2. External writer replaces it with **identical bytes but a new inode** → identity check + catches what the hash cannot. +3. No external writer → publication succeeds, tokens updated (the happy path must not + regress). +4. The canonical target still exists after a refused publication — never unlinked. + +## Verification (C) + +Focused `bun test` on the refresh suite, red-green on case 1 and case 2 separately: case 1 +must fail without the guard, case 2 must fail with only a content hash. + +## Outcome + +Landed as #3199, `c17bc94c2faa9b296a95d8529019579df177de02`. Identity (`dev`+`ino`) is now +compared alongside `rawSha256`, failing closed when identity cannot be read on either side. + +`bun test ./tests/codex-main-account-refresh.test.ts` — 7 pass, 0 fail. Removing only the +identity check reds the same-bytes-new-inode case (6/1) and leaves the rest green, which is +the proof it does work the hash did not. + +**#2999 stays OPEN, re-scoped.** The check runs before `rename(2)`, not atomically with it. +Closing the last window needs `renameat2(RENAME_EXCHANGE)` or an equivalent, which Bun does +not expose. Claiming the race eliminated would have been the easy way to drop the count by +one; the comment on the issue says plainly what is closed and what is not. + +Terminal outcome: `DONE` for the publication guard, with the atomic primitive recorded as +remaining work on the issue. diff --git a/devlog/_plan/260902_bug_label_drawdown/058_i2813.md b/devlog/_plan/260902_bug_label_drawdown/058_i2813.md new file mode 100644 index 0000000000..182ee3b666 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/058_i2813.md @@ -0,0 +1,80 @@ +# 058 — i2813: routed models unselectable during Luna Reserve + +One issue, one cycle. **Outcome: client-side limitation, documented. Issue closes.** + +## What #2813 reports + +Codex 2.34.0 on Windows 11. Once the 5-hour ChatGPT quota is exhausted and Codex activates +`gpt-reserve` / Luna Reserve, **all other picker entries become unselectable — including +OpenCodex routed models**, which run on independent providers and credentials and consume +none of the exhausted quota. + +The reporter framed the right question themselves: if the Codex client gates availability +before requests reach OpenCodex, can the proxy work around it, and if not, say so. + +## Where the gate lives + +OpenCodex does not model this at all. `rg` across `src/` and `docs-site/` for +`gpt-reserve` returns nothing: we never emit a reserve marker, and the catalog sync path has +no reserve concept. + +The installed Codex CLI **0.150.1** binary settles it. Strings show the reserve state is a +**server-supplied** field, not a local inference: + + struct RateLimitSnapshot with 9 elements + limit_name primary secondary credits individual_limit + spend_control_reached plan_type rate_limit_reached_type + + struct RateLimitReachedType with 1 element + struct RateLimitStatusDetails with 4 elements + rate_limit spend_control primary_window additional_rate_limits + +and the transport that carries it: + + x-codex-rate-limit-reached-type + x-codex-safety-buffering-faster-model + +Alongside them, two UI surfaces named for exactly this state: `model_availability_nux` and +`hide_rate_limit_model_nudge`. + +Notably, `gpt-reserve` itself does **not** appear in the binary. The reserve model and the +gating decision both arrive from the ChatGPT backend; the client renders what it is told. + +## Why the proxy cannot fix this + +The picker is populated and gated **before** any request reaches OpenCodex, from a +`RateLimitSnapshot` the client receives on its own authenticated ChatGPT connection. Nothing +in the model catalog we sync — the only channel we own — participates in that decision. + +Three non-options, stated so they are not re-litigated: + +- **Catalog representation.** We already write routed entries as ordinary catalog models. The + gate is not reading our entries' shape; it is applying a global availability state. +- **Suppressing the reserve state.** The header arrives on the client's own ChatGPT + connection, not through the proxy's data plane. There is nothing for us to intercept. +- **Faking quota headroom.** Even if reachable, misreporting a user's quota to their own + client is the kind of fix that produces a worse bug — and it would be lying to the user + about their account. + +## Disposition + +The reporter's fallback ask is the correct outcome: **document it as a Codex client +compatibility limitation.** That is honest, actionable for anyone who hits it, and does not +leave a bug open against code that cannot contain the defect. + +Workaround worth naming: routed models stay reachable through any client that does not gate +on the ChatGPT rate-limit snapshot — Claude Code through the proxy, or a direct HTTP client +against `/v1`. The proxy and its providers are unaffected; only the Codex picker is. + +## MODIFY map + +**`docs-site/src/content/docs/guides/codex-integration.md`** — a short subsection under the +existing troubleshooting material: what the user sees, why it happens, that it is +client-side, and the workaround. English source only; translated locales are left rather +than half-translated. + +## Verification (C) + +`rg` proof that the section exists and names the reserve state. Docs build runs in CI's +`gates` job. + diff --git a/devlog/_plan/260902_bug_label_drawdown/059_i1527.md b/devlog/_plan/260902_bug_label_drawdown/059_i1527.md new file mode 100644 index 0000000000..7d44193021 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/059_i1527.md @@ -0,0 +1,53 @@ +# 059 — i1527: Cursor large-context collapse / rate-limit asymmetry + +One issue, one cycle. **Outcome: no proxy-side defect left that the evidence supports. +Issue stays open as a recorded blocker (`needs-info`) pending a matched direct-vs-adapter +trace.** + +## What #1527 reports + +Large-context turns through the Cursor adapter either collapse to a short answer or hit +429 while the same conversation in the Cursor client stays healthy. The reporter's +control run was direct Cursor on the same account. + +## What has already landed against it + +| Mechanism | Fix | Evidence on `dev` | +| --- | --- | --- | +| Full-history replay every turn | checkpoint continuation (#2277) | `src/adapters/cursor/request-builder.ts` reuses returned conversation state | +| Abort after terminal frame logged as `turn-failed` with `expectedClose:false` | #2118 | `live-transport.ts` run loop: `if (this.emittedTerminal && isCursorAbortError(failure)) return;` before `classifyTurnFailure`; both post-terminal and pre-terminal cases in `tests/cursor-cancel-provenance.test.ts` | +| Retry storm on 429 / `RESOURCE_EXHAUSTED` | `transport-retry.ts` excludes them | non-retryable classification | +| Envelope over-replay (cumulative checkpoint+suffix, empty-history skip, contiguous tool results), result deletion, initiating-turn drop | #2865 | assembled-set guard (`CURSOR_EXTERNAL_ROOT_BLOB_LIMIT` = 192 blobs), `cursor_root_envelope_limit` HTTP 400 with measured counts | + +Re-read this cycle: `live-transport.ts` L569-600 (`summarizeFailure` / `classifyTurnFailure`), +L708-742 (drain loop, post-terminal abort return), L1411-1420 (abort listener installs +`failAndClear(new Error("Cursor request was aborted"))`). The post-terminal abort exits the +drain loop before any classification, so no `turn-failed` summary is emitted for a completed +turn. Nothing in the current tree reproduces the misclassification the issue's log showed. + +## What remains and why it cannot be fixed from here + +Two residual observations are not explained by any of the above: + +1. **429 asymmetry** — the adapter path is rate-limited where the direct client is not. +2. **`cache_read_tokens` on the direct client** — Cursor's own path may get prefix-cache + hits on prompts OpenCodex re-sends cold after a restart/compaction/lineage change. + +Both need a matched pair: the same large-context task through OpenCodex (with +`ocx debug provider on` and the `[ocx:cursor:run-request]` `rootBlobs` / `rootBytes` / +`continuationMode` lines) and through the Cursor client on the same account, close in time, +plus Cursor's reported `cache_read_tokens` for the direct run. That trace requires a live +Cursor account under real large-context load. It cannot be produced from the repository. + +Speculative changes (for example pre-emptively re-shaping the replay prefix to chase cache +hits without knowing Cursor's cache key) fail the DEV-NECESSITY-01 gate: no evidence that +they alter the reported outcome, and real risk of regressing the continuation path that +#2277 / #2865 verified. + +## Disposition + +- Post one status comment: what landed since the last maintainer note, the two residuals, + the exact trace that would settle them. +- Apply `needs-info`. Keep the issue open. Recorded as a blocker for criterion c-7. +- Zero source diff. Verification for this cycle is the focused regression that guards the + one #1527 mechanism we did fix: `bun test tests/cursor-cancel-provenance.test.ts`. diff --git a/devlog/_plan/260902_bug_label_drawdown/060_phase6.md b/devlog/_plan/260902_bug_label_drawdown/060_phase6.md new file mode 100644 index 0000000000..3ef7dae028 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/060_phase6.md @@ -0,0 +1,36 @@ +# 060 — Batch F: implementable bug issues + +Five issues describe defects concrete enough to fix. + +- **#3152** dashboard log panel layout jittering (`gui`) — adjacent to #3174's responsive + work. Likely a measured-geometry fix in the same style. +- **#3170** provider input size limit handled gracefully — closes via #3177. **Confirmed by + the A-gate audit (A4):** the body says `Closes #3170` and the diff maps a streaming 413 + to a terminal `context_length_exceeded` instead of the 5/5 reconnect loop. GitHub's + `closingIssuesReferences` is empty only because the PR targets `dev`, so close by hand. +- **#2999** native-main refresh can overwrite external Codex writers (`account-pool`) — + **the plan was wrong to assume #3112 closes it (A4).** The issue describes two races; + #3112 is explicitly only the *lock-scope* half — serializing two `OPENCODEX_HOME`s + against one `CODEX_HOME`. The named publication/overwrite race is still carried by the + existing refuse-not-overwrite check. So #3112 landing does **not** close #2999: the + publication half needs its own fix, or the issue stays open with that scope recorded. +- **#2813** Codex Luna Reserve / gpt-reserve disables routed models after the 5-hour quota + is exhausted (`account-pool`) — needs a real reproduction of the reserve-mode gate. +- **#1527** Cursor adapter large-context turns collapse while direct Cursor stays healthy + (`provider-compatibility`, `streaming`) — the hardest of the five; likely a request-shape + or budget difference between adapter and direct paths. + +## Order + +Verify the two that other PRs close first (#3170, #2999) — those are free if Batch A and C +land. Then #3152, then #2813, then #1527. + +## Method per implemented fix + +Reproduce from the issue, locate the defect with `path:line` evidence, fix the root cause +rather than the symptom, add a focused regression proven red-green, land as its own squash +merge closing the issue. + +## Verification (C) + +Focused suite output with counts, red-green proof, landing SHA ancestry, issue closed. diff --git a/devlog/_plan/260902_bug_label_drawdown/061_p3193.md b/devlog/_plan/260902_bug_label_drawdown/061_p3193.md new file mode 100644 index 0000000000..56147bb171 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/061_p3193.md @@ -0,0 +1,33 @@ +# 061 — p3193: allow `POST /v1/alpha/search` on the loopback listener + +Work-phase `p3193` of the bug-label drawdown. One PR, one PABCD cycle. + +## Source + +- PR #3193 by @alan7629 (draft, head `8aec58c19`, base `dev`), "fix(server): allow alpha search on loopback listener". Fixes #3192 (already closed). +- Checks green (hygiene / enforce-target / CodeRabbit), no reviews. + +## Finding + +The fix is correct and one line: `loopbackRouteAllowed()` in `src/server/index.ts` never admitted `/v1/alpha/search`, so the unauthenticated loopback listener 404'd every native Codex web search on a direct-spawn host. The handler at `src/server/index.ts:1647` does its own `resolveApiAuth`, so admitting it here does not bypass auth: a loopback caller without a ChatGPT credential is refused inside `handleSearch` (`validateForwardAdmissionCredential`). + +The contributor branch cannot land as-is: the editor round-tripped the file through a lossy encoding. Every em-dash became `??`, `⚠️` became mojibake, `→` became `?`, ~30 unrelated comment lines in `src/server/index.ts` plus the test file changed, and the new test carries a duplicated `expect`. + +## Decision + +Reimplement on a clean branch from `origin/dev` (`fcf0da257`), credit the author with `Co-authored-by`, land via admin squash-merge, close #3193 with a pointer to the landed SHA. + +## Diff + +- `src/server/index.ts`: add `if (path === "/v1/alpha/search") return req.method === "POST";` next to `/v1/responses/compact`; extend the allowlist doc-comment with why the relay belongs there and where its auth lives. +- `tests/loopback-listener-integration.test.ts`: move `/v1/alpha/search` out of the denied list (POST) and into the method-mismatch section (GET still 404); add a focused test that POST on loopback is not 404 and the body is the handler's own refusal (message ≠ "opencodex API key required"), while the public listener still answers 401 "opencodex API key required". + +## Checks (focused, no full suite) + +- `bun test tests/loopback-listener-integration.test.ts` → 29 pass / 0 fail. +- `bun run typecheck` clean. `bun run privacy:scan` passed. + +## Landing + +Recorded in `062_p3193_landing.md` once merged. + diff --git a/devlog/_plan/260902_bug_label_drawdown/062_p3193_landing.md b/devlog/_plan/260902_bug_label_drawdown/062_p3193_landing.md new file mode 100644 index 0000000000..9136e0dba5 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/062_p3193_landing.md @@ -0,0 +1,10 @@ +# 062 — p3193 landing + +- Reimplementation PR: #3205 `fix(server): allow POST /v1/alpha/search on the loopback listener`, branch `codex/260902-p3193-loopback-alpha-search`, head `1b21bd652`. +- Admin squash-merge → `53c09a247` on `dev`; ancestry proven with `git merge-base --is-ancestor 53c09a247 origin/dev`. +- #3193 closed with a credit comment pointing at the landed SHA (author co-credited in the commit). #3192 was already closed. +- Audit: reviewer subagent (xai/grok-4.6) failed the first pass on docs-site allowlist drift (en/fr/zh-tw/tr) and the stale "four allowlisted routes" title; both fixed, second pass passed. +- Check receipt: `.codexclaw/evidence/01a05dad-de70-7522-87a0-b82747a6d34c/test-receipt.json` — 29 pass / 0 fail on the loopback file; typecheck and privacy:scan clean. +- Test note: in the test environment the admitted path answers 503 (native-main maintenance gate) rather than the relay's 401; the assertion accepts either and rejects 404, which is what proves the gate opened. +- Trailing CI on `dev` tracked in the regaudit work-phase. + diff --git a/devlog/_plan/260902_bug_label_drawdown/063_i3217.md b/devlog/_plan/260902_bug_label_drawdown/063_i3217.md new file mode 100644 index 0000000000..8eed7993b8 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/063_i3217.md @@ -0,0 +1,64 @@ +# 063 — i3217: Spark flattening of the reserved `functions` namespace → `execexec` + +Work-phase `i3217`. One issue, one cycle. Opened by @alex-jordan547 during the regaudit pass. + +## Symptom + +Codex 0.150.1 + `gpt-5.3-codex-spark` on the native ChatGPT forward route: text-only turns +complete, any `exec` turn loops with `unsupported custom tool call: execexec`. Bypassing the +proxy works. Reproduced locally on 2026-09-02 with ocx 2.40.0 (25 hits in one 60 s run). + +## Root cause (traced, not inferred) + +A tap on a dev proxy built from this tree recorded three things per turn: + +1. Inbound `additional_tools` from Codex: one `namespace` group named `functions` holding + `custom exec`, `function wait`, `function request_user_input`. That is what + `create_tools_json_for_responses_lite` in codex-rs produces for Responses Lite. +2. Outbound body to `chatgpt.com/backend-api/codex`: the group is gone — `additional_tools` + now holds the three tools flat. `stripSparkCompatibility()` + (`src/adapters/openai-responses.ts`) flattens *every* `namespace` group for + `*codex-spark*` models, in both `body.tools` and `additional_tools`. It was written in + July (`7defec111`) when the only namespace groups Codex sent were MCP-style; the reserved + `functions` group arrived with Codex 0.147, after 2.24.2 — which is why the reporter's + "worked on 2.24.2" is true and no proxy release regressed it. +3. Upstream SSE: `custom_tool_call { name: "exec", namespace: "exec" }`. The backend, given a + flat `custom exec` declaration that the model addresses through the `functions` namespace + it was trained on, answers with a namespace equal to the tool name. The proxy relays it + untouched. codex-rs `ToolName::new(namespace, name).with_default_namespace()` treats only + `None | "" | "functions"` as default, so `flat_tool_name` concatenates → `execexec`. + +The parser already knows this shape: `buildTools` flattens `functions` for routed providers on +purpose, and `customToolNamespaces` deliberately skips it. Only the Spark stripper predates it. + +## Fix + +`src/adapters/openai-responses.ts` `stripSparkCompatibility`: + +- Keep a `namespace` group whose name is the reserved `functions` namespace as a group. Still + filter its children (drop `tool_search` etc., strip `defer_loading`) so the Spark + restrictions hold inside it. Drop the group only if nothing survives. +- Keep flattening non-`functions` groups (unchanged behaviour for MCP-style groups). +- `custom` stays in `SPARK_SAFE_TOOL_TYPES`? No — Spark accepts freeform `custom` inside + `functions` (codex-rs sends exactly that and it works direct). The stripper's "drop custom" + rule was written for a backend that rejected top-level custom tools; inside the reserved + group it is what the direct client sends. Allow `custom` inside the `functions` group only. + +Defensive scrub on the client-facing side of the canonical forward route: a `custom_tool_call` +or `function_call` whose `namespace` equals its own `name` is never a legitimate identity +(codex-rs would concatenate it). Delete that `namespace` in the passthrough SSE/JSON rewrite so +a future backend quirk cannot re-open the loop. Applied only when the request did not declare +a namespace group of that name. + +## Tests (focused, red without the fix) + +- `tests/openai-responses-passthrough.test.ts`: Spark passthrough keeps the `functions` group + with its `custom exec` child in `additional_tools`; an MCP-style group is still flattened; + `tool_search` inside `functions` is still dropped. +- Relay test: upstream `custom_tool_call {name:"exec", namespace:"exec"}` reaches the client + without `namespace` on the canonical forward route; a declared MCP namespace is untouched. + +## Landing + +`064_i3217_landing.md`. + diff --git a/devlog/_plan/260902_bug_label_drawdown/064_i3217_landing.md b/devlog/_plan/260902_bug_label_drawdown/064_i3217_landing.md new file mode 100644 index 0000000000..4360ab9061 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/064_i3217_landing.md @@ -0,0 +1,10 @@ +# 064 — i3217 landing + +- PR #3224 `fix(responses): keep the reserved functions group intact for codex-spark (#3217)`, branch `codex/260902-i3217-spark-functions-namespace`, head `21b73c22b`. +- Admin squash-merge → `d23eab43a` on `dev`; `git merge-base --is-ancestor d23eab43a origin/dev` exit 0. +- #3217 closed as completed with the cause, the SHA, and the interim install path. +- Root cause was proven, not inferred: a tap on a dev proxy built from this tree recorded the flattened outbound group and the `namespace:"exec"` answer; the same tap with the fix recorded the group intact and a bare `exec` answer, and the `codex exec` turn ran `pwd` (0 `execexec`, previously 25 per minute). +- Audit: reviewer (xai/grok-4.6) pass; residual "no stream:false case" closed in B before C. +- Checks: focused set 267 pass / 0 fail (receipt), `bun run test:changed` 5794 pass / 0 fail across 301 files, typecheck and privacy:scan clean. Red-without-fix proven for both new tests. +- Trailing CI on `dev` for `d23eab43a` tracked in `regaudit2`. + diff --git a/devlog/_plan/260902_bug_label_drawdown/070_regaudit.md b/devlog/_plan/260902_bug_label_drawdown/070_regaudit.md new file mode 100644 index 0000000000..ee8e8a78a7 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/070_regaudit.md @@ -0,0 +1,70 @@ +# 070 — regaudit: main→dev regression audit, trailing CI, count + +Terminal work-phase of the bug-label drawdown. Runs after every landing (dependsOn i1527, p3193). + +## Scope + +- `origin/main` = v2.39.0 promotion tip (`af6113a03`). `dev` keeps moving while this audit + runs (other maintainers are landing feature PRs), so counts are pinned to a snapshot: at the + first pass `origin/main..origin/dev` was 141 commits / 82 src-touching; by the second pass + (tip `5bc6939d8`) it was 145 / 84 plus 23 tests-only commits. Every commit that entered after + the first snapshot was reviewed in the second pass, so the union covers the whole range. +- `origin/main` is not a fast-forward ancestor of `dev` (15 promotion merge commits are unique + to `main`); the range is still `main..dev` because promotion merges carry no source of their + own. +- Four independent read-only reviewers (xai/grok-4.6): two split the src-touching commits and + hunt behavioral regressions for a default-config user (broken previously-working paths, + credential leaks, Node-only APIs, changed status/error contracts, reintroduced bugs); one + covers the tests-only commits (does any weaken a guarantee?) plus the two newest feature + commits; one runs the MAINTAINERS.md security-boundary pass over workflows, release tooling, + auth-cors, service-secrets, remote, management, and client code, and confirms + `privacy:scan` stays wired in CI. +- Trailing CI on `dev` is judged here, per the user's "CI 후행" policy. Push-triggered runs on + `dev` skip the Windows shards (`platform-windows` is `workflow_dispatch` only) and are + cancelled by the next push, so an exact-head verdict needs a `workflow_dispatch` on the tip: + branch `codex/regaudit-ci-5bc6939d8` = `origin/dev` tip, run 33552542958. +- Every bug-train landing SHA is re-proven an ancestor of `origin/dev`. +- The devlog stack (`codex/260902-bug-pr-closeout-stack`) lands as its own docs PR. +- Final recount against c-7. + +## Landing ancestry (re-proven this cycle) + +#3174 e582aee21, #3176 2e2da87b5, #3177 0d6424f80, #3178 51c49177f, #3179 eceb02d9d, +#3180 634d9e5a0, #3182 865a36ef0, #3183 fecb77a91, #3184 afd5b4630, #3185 fe766e129, +#3186 ea29e25b0, #3187 d33557064, #3188 5ccf7c800, #3189 5557772b7, #3194 c87071400, +#3195 f3bcc67a7, #3196 52d941640, #3197 4be4326d7, #3198 ef6a163c7, #3199 c17bc94c2, +#3200 fcf0da257, #3201 c7f3f6f31, #3202 59449fa83, #3203 55400efd5, #3205 53c09a247 — +all `git merge-base --is-ancestor origin/dev` exit 0. + +## Trailing CI on dev + +Most runs in the train were **cancelled** by the next push (concurrency group), so the signal is +the runs that completed: + +| run | head | result | failing job → test | classification | +|---|---|---|---|---| +| 33543314151 | 52d941640 (#3196) | failure | test 2/4 → `provider-quota` "pool reports tolerate a malformed persisted plan" | **real, already repaired** by #3200 `fcf0da257` (test moved onto the #3198 contract) | +| 33543314151 | 52d941640 | failure | macos → same provider-quota test | same | +| 33546279148 | fcf0da257 (#3200) | failure | macos → `lab-live-pinned-timeouts` "preserves the output byte ceiling as output_byte_limit" received `first_byte_timeout` | **flake**: `firstByteTimeoutMs: 30` in `BASE_LIMITS` races the loopback server on a loaded macOS runner; the test and `src/lib/lab-live-pinned-sender.ts` are unchanged since `d9655f31b`, which is already on `main`. Linux shards 1-4 passed the same file. | +| 33520193493 | 9232df0e6 | failure | test 3/4 → responses-state "shutdown drain cap expiry" | pre-train, timing flake (not in this campaign's diff) | +| 33514747317 / 33477777613 | 408652698 / 58be3c5bb | failure | macos → port-selection / websocket pool auth | pre-train macOS timing flakes, same family the memory notes as known | +| 33549107560 | 0d73d6557 (#2986, not ours) | failure | macos → `codex-prompt-route` "36. comment-after-bracket fallback project document with a bare key" expected in-flight probe refusal, received a completed probe | **flake**: the case races a 200 ms probe against the second GET; the quoted-key sibling passed in the same run; the file is untouched since `main` (`aa16a71e0`); no other run in this train hit it. | +| 33548615686 / 33550885829 | 6a6efa928 / 4a382beed (not ours) | cancelled | — | superseded by the next push | +| 33551966282 | 5bc6939d8 (#3209, not ours) | push-triggered | — | tracked; Windows skipped | +| 33552542958 | 5bc6939d8 | **workflow_dispatch, exact head, Windows shards on** | — | the promotion-grade verdict for this audit; recorded in 071 | + +Last fully green dev run before the train: `22a643a00` (2026-09-01T16:41Z). Verdict on the train +so far: one genuine CI regression (#3196's test contract drift) which was caught and repaired +inside the train by #3200; no other failing job points at a commit from this campaign. + +## Reviewer verdicts + +Full text in `071_regaudit_landing.md`. Summary: all four passes returned `VERDICT: pass` with +no high-confidence default-path regression. Medium suspects are design decisions on opt-in or +non-default paths (unreadable `config.json` now fails closed in `ocx start`; streaming provider +413 becomes a terminal SSE overflow; launchd Claude mode ignores dotenv-only Anthropic env; +compaction without a canonical OpenAI route forwards to the default provider). Two security +residuals are recorded for follow-up, neither a new grant: live `service-api-token` reads skip +the owner-only mode check that `.prev` enforces; hub `managementPublicOrigin` replaces the +observed scheme so pairing cannot see TLS-stripped HTTP on the public listener (the official +client already refuses plaintext). diff --git a/devlog/_plan/260902_bug_label_drawdown/071_regaudit_landing.md b/devlog/_plan/260902_bug_label_drawdown/071_regaudit_landing.md new file mode 100644 index 0000000000..50f253f3b4 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/071_regaudit_landing.md @@ -0,0 +1,120 @@ +# 071 — regaudit landing + +## Reviewer verdicts (verbatim tails) + +### Pass 1 — src-touching commits, first half (Faraday, xai/grok-4.6) + +VERDICT: pass +c7f3f6f31 compaction can send a bare native id to a non-OpenAI default (medium) +0d6424f80 streaming 413 becomes terminal SSE overflow, not HTTP 413 (medium) +865a36ef0 dotenv-only Claude creds can be classified proxy (medium) +efefe3671 standalone still shows dead key-rotation UI (medium) +2e2da87b5 quota-word 5xx can cool/rotate a pool account (low) +51c49177f Hermes export model list shape changed (low) +f3bcc67a7 citation PUA stripped on translated streams (low) +7386b5201 combo default can raise to lowest supported rung (low) + +Checked and not counted: outbound body ceiling default-off (52d941640); loopback +/v1/alpha/search still authenticates inside the handler (53c09a247); pairing "allowed" in +pairing only matches refusals (88d9889bb); logout goes through patched window.fetch so CSRF is +attached (9bded9c41); native-main refresh claim/identity checks fail closed (fecb77a91, +c17bc94c2). + +### Pass 2 — src-touching commits, second half (Kuhn) + +VERDICT: pass +- 863a88ea3 src/client/state.ts:43 — unreadable config.json is invalid, so ocx start/ensure/claude + exit 1 instead of the old default fallback. Medium. +- b14b741dc src/service.ts:2127 — unscoped Windows session-recovery triggers fail closed and skip + auto-repair. Medium, Windows-only. +- bf221bc26, d25cbc02a, 10a31986a, 4fdd54d46, b81c43551 — low, hub/relay/MiniMax-only. + +### Pass 3 — tests-only commits + two newest feature commits (Epicurus) + +VERDICT: pass +4a382beed keeps Design B unless codexDesktopAuthless === true on loopback. 0d73d6557's +/v1/images relay returns immediately unless images.bridgeEnabled === true and an xAI provider +exists. No Node-only APIs. Tests-only commits add coverage or retarget assertions to +#3198/#3108/remote-protocol contracts; none skip, mock away a live path, or drop a security check. + +### Pass 4 — MAINTAINERS security boundary (Socrates) + +VERDICT: pass +41 commits in-scope. 6f415bae is workflow_call only — no PAT, no release-job write grant, pinned +actions. Pairing/session/rotation stay grant- or management-authenticated; public +/opencodex-session is hub-only, origin-bound, rate-limited. Authless Desktop is loopback-only. +CI still runs bun run privacy:scan. +Residual (non-blocking): 863a88ea3 src/lib/service-secrets.ts:40 live service-api-token reads skip +the owner-only mode check .prev enforces; abf0f81bd src/server/auth-cors.ts:134 hub +managementPublicOrigin replaces the observed scheme. + +## Follow-ups filed from the residuals + +Recorded here as candidates; none blocks promotion and none carries the bug label: + +1. service-secrets: apply the owner-only mode check to the live token read, not only `.prev`. +2. auth-cors: let pairing observe the raw scheme when `managementPublicOrigin` rewrites it. +3. client/state: consider a warning-plus-default path for an unreadable `config.json` on + standalone hosts instead of exit 1. + +## Exact-head CI (workflow_dispatch on the dev tip) + +Run 33552542958 on `5bc6939d8` (branch `codex/regaudit-ci-5bc6939d8` = `origin/dev`), +Windows shards enabled. Result: **every non-Windows job green on the exact head** — test 1/4 +through 4/4 (Linux), macos, gates, storage policy, api usage, keyring ubuntu/macos/windows, +npm-global ubuntu/macos/windows. That settles the two macOS failures seen during the train +(`lab-live-pinned-timeouts` first-byte race, `codex-prompt-route` probe race) as flakes: the +same tip passed the whole macOS suite. + +The four Windows shards failed (1/4, 2/4, 4/4 failure; 3/4 cancelled by the composed gate). +The failure signatures are environmental, not assertion failures in campaign code: + +- shard 2/4: `ACL hardening failed (EICACLS) — icacls command error` thrown from + `hardenSecretDir(..., { required: true })` inside `saveConfig` (`src/config.ts:2674`) and + `ETIMEDOUT — transient icacls stall` 16×. Every test that calls `saveConfig` on that runner + fails identically. The ACL module (`src/lib/windows-secret-acl.ts`) and `atomic-write.ts` + are unchanged since `main` (only `e5d588669`, already on `main`, touches them). +- shard 1/4 and 4/4: `EPERM: operation not permitted, rm 'tests\.tmp-codex-accounts-test'` + (49×) and `rm 'tests\.tmp-codex-auth-api-test'` (287×) — Windows file-handle contention on + the test temp dirs during `rmSync`, cascading into every case in those files. Plus one + "Bun runtime crash" retry. + +History: the last Windows-green dispatch was `33290817128` on `223a0a287` (on `main`); the +dispatch on the same SHA from `dev` (`33291970929`) failed Windows 3/4, and the intervening +Windows dispatches on feature branches (`33292931792`, `33290258063`, `33289201339`, +`33288039685`) all failed. Windows shards have therefore not been a stable signal for any +branch since 2026-08-30, before this campaign's first landing. + +Control: the same workflow dispatched on `origin/main` (`af6113a03` = released v2.39.0, +branch `codex/regaudit-ci-main-af6113a03`, run 33555110133). **Windows shards fail on `main` +with the identical signatures**: shard 3/4 `EPERM: operation not permitted, rm +'tests\.tmp-codex-accounts-test'` 49× plus `.tmp-oauth-status-privacy-test` 7×, icacls +`ETIMEDOUT`, "Bun runtime crash"; shard 4/4 icacls `ETIMEDOUT` 13× and the same +`Responses state admission boundary` / `previous_response_id` cases. The released tip and the +audited `dev` tip fail the same way on `windows-latest`, so the Windows result is a runner +environment defect (NTFS ACL/icacls stalls and temp-dir handle contention on the hosted image) +that predates this campaign. It is not evidence of a regression in `main..dev`. + +Verdict for the range: no regression found by four independent reviewers; exact-head CI green +on Linux ×4, macOS, gates, storage, api-usage, keyring ×3, npm-global ×3; Windows blocked by +the runner environment on both ends of the range. Follow-up candidate (no bug label, not this +campaign): make `tests/codex-account-store.test.ts` / `codex-auth-api` temp-dir teardown +retry `EPERM` on Windows, and re-enable the self-hosted `ocx-home` runner +(`OCX_SELF_HOSTED_WINDOWS`) for a trustworthy Windows signal. + +## Devlog stack landing + +Branch `codex/260902-bug-pr-closeout-stack` (devlog-only) → PR #3218, opened during this +cycle; merged in the final recount phase so it carries the i3217 record too. + +## Bug-label count at the end of this pass + +`gh issue list -l bug --state open` = 6, `gh pr list -l bug --state open` = 0. Five are the +recorded blockers (#3152 needs-repro, #3141 needs-info, #2999 CAS-primitive, #1527 needs-info, +#1419 needs-info). The sixth, **#3217**, was opened at 2026-09-01T20:39Z while this audit ran: +Responses Lite `exec` returned with `namespace: "exec"` on the native forward route, so Codex +loops on `execexec`. Reproduced locally (ocx 2.40.0, codex 0.150.1) and traced with a tap on a +dev proxy: `stripSparkCompatibility` flattens the reserved `functions` namespace group in +`additional_tools`; the ChatGPT backend then answers the flat `custom exec` with +`namespace: "exec"`, which the proxy relays verbatim. It is implementable and becomes its own +work-phase (`i3217`); c-7 is evaluated again in the final recount phase after it lands. diff --git a/devlog/_plan/260902_bug_label_drawdown/072_regaudit2.md b/devlog/_plan/260902_bug_label_drawdown/072_regaudit2.md new file mode 100644 index 0000000000..7ed058f38d --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/072_regaudit2.md @@ -0,0 +1,49 @@ +# 072 — regaudit2: final recount, exact-head CI on `d23eab43a`, devlog landing + +Terminal phase after `i3217`. + +## Recount (2026-09-02, after #3224 and #3223 disposition) + +`gh issue list -l bug --state open` → 5. `gh pr list -l bug --state open` → 0. Combined **5**. + +| item | disposition | blocker recorded in | +|---|---|---| +| #3152 dashboard log panel jitter | NEEDS_REPRO — reporter environment detail (row count, viewport) | 052_i3152.md | +| #3141 aggressive responses-state writes | NEEDS_HUMAN — reporter measurement; numbers do not reconcile | 051_i3141.md | +| #2999 native-main refresh publication race | DONE for the publication guard (#3183 `fecb77a91`, #3199 `c17bc94c2`); last window needs `renameat2(RENAME_EXCHANGE)`, which Bun does not expose | 057_i2999.md | +| #1527 Cursor large-context collapse | NEEDS_INFO — matched direct-vs-adapter trace for the 429 asymmetry and prefix-cache residuals | 059_i1527.md | +| #1419 bundled Bun SIGTRAP | NEEDS_HUMAN — reporter crash artifact | 056_i1419.md | + +Each has a written comment on the issue naming the evidence and the exact artifact that would +unblock it, plus the `needs-info` label where the reporter owns the next step. That meets the +objective's fallback ("5 acceptable if the last few are genuinely blocked"), and the four +external-dependency items are honest blockers rather than deferrals: two need a reporter +artifact, one needs a reporter measurement, one needs a runtime primitive. + +#3223 (contributor PR for #3217) was closed as superseded by #3224 with a comment crediting the +independent diagnosis and inviting the tighter catalog-scoped scrub as a follow-up. + +## Exact-head CI on the final dev tip + +`d23eab43a` = `origin/dev` after #3224. `workflow_dispatch` on branch +`codex/regaudit-ci-d23eab43a`, run 33562938994, Windows shards on. Result: every non-Windows +job green (test 1/4–4/4, macos, gates, storage policy, api usage, keyring ×3, npm-global ×3). +Windows 1/4, 2/4, 4/4 failed and 3/4 was cancelled by the gate, with the same signatures as the +`main` control in 071 (`EPERM rm tests/.tmp-codex-accounts-test` ×49, `.tmp-codex-auth-api-test` +×377, icacls `ETIMEDOUT`, "Bun runtime crash"). Nothing in `d23eab43a` touches those paths; +the Windows result stays classified as a hosted-runner environment defect present on both ends +of the range. + +## Devlog landing + +PR #3218 (this stack, rebased on `d23eab43a`) → merged in the closeout phase. + +## Arrivals after the recount + +Between the recount above and the close of this phase, four contributor PRs carrying the bug +label opened against `dev` (2026-09-01T22:49Z – 23:33Z): #3226 (scope the #3217 scrub, the +follow-up invited on #3223), #3227 (combo preflight: zero-output transport incompletes should +fail over), #3228 (encrypted V2 spawn native fallback without a configured chain; touches GUI), +#3229 (allow the `codexless_agent` originator in V2 task recovery). Combined count moved to +5 + 4 = 9. Each is registered as its own work-phase (`p3226`…`p3229`) and the final recount +moves to `regaudit3` after they land or are dispositioned. diff --git a/devlog/_plan/260902_bug_label_drawdown/080_p3226.md b/devlog/_plan/260902_bug_label_drawdown/080_p3226.md new file mode 100644 index 0000000000..db27612aaf --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/080_p3226.md @@ -0,0 +1,42 @@ +# 080 — p3226: scope the #3217 self-named namespace scrub + +Work-phase `p3226`. Contributor PR #3226 by @alex-jordan547 (head `c7f730b23`, base `dev`, +MERGEABLE, review-ready), the follow-up invited when #3223 was closed. + +## What it changes + +The scrub landed in #3224 deleted any `namespace` equal to the call's own `name` without +consulting the catalog. That is correct for the Spark quirk but wrong for one legitimate shape: +a namespace group named `exec` that declares a tool named `exec`. codex-rs routes that by +`ToolName { namespace: "exec", name: "exec" }`, so stripping the namespace would misroute it. +#3226 builds an authorization set from the turn's `tools`, `additional_tools`, and +`tool_search_output` (bare custom / bare function names, minus names that also appear as a +same-name namespaced tool), threads it through `buildToolBridgeMaps`, and scrubs only names in +that set, per call type. + +## Review plan + +- Reviewer (xai/grok-4.6): authorization-set construction, absent-catalog behaviour (the scrub + must still fire when `additional_tools` carries the declaration, which is the #3217 shape), + tool_choice gating, budget charging symmetry, no behaviour change for non-forward routes. +- Focused tests on the PR head in a scratch worktree: scrub, undeclared-tool guard, passthrough. +- Land via admin squash-merge; prove ancestry; record in `081_p3226_landing.md`. + +## Audit finding (Erdos, xai/grok-4.6) — fail on the PR as-is + +Focused tests on head `c7f730b23` are green (201 pass, typecheck, privacy), the authorization +set is request-scoped on both SSE and bounded-JSON paths, the #3217 shape still scrubs, and the +genuine same-name namespaced tool now survives (correct against codex-rs `ToolName` routing). +One hole: `collectBareToolSpecs` only reads `spec.name`, so a Chat-shaped declaration +`{ type: "function", function: { name } }` — which `buildTools` accepts (`parser.ts:215`) and +therefore lands in `bareFunctionToolNames` — is never recorded on the raw-body side. The +intersection drops it and a self-named echo for that function would reach Codex again. + +## Revised landing: carry with the fix + +Cherry-pick the PR's commits onto `codex/260902-p3226-carry` from `origin/dev` (author +credit preserved), then one maintainer commit: teach `collectBareToolSpecs` the nested +`function.name` shape (mirroring `addWireToolName` in the undeclared-tool guard), and add a +red-without-fix case to `tests/responses-self-named-namespace-scrub.test.ts` where the catalog +is Chat-shaped and the upstream echoes `namespace === name` on a `function_call`. Admin +squash-merge the carry, close #3226 as landed-via-maintainer naming the SHA. diff --git a/devlog/_plan/260902_bug_label_drawdown/081_p3226_landing.md b/devlog/_plan/260902_bug_label_drawdown/081_p3226_landing.md new file mode 100644 index 0000000000..cfe4d37593 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/081_p3226_landing.md @@ -0,0 +1,11 @@ +# 081 — p3226 landing + +- Carry PR #3234 (branch `codex/260902-p3226-carry`): the four #3226 commits cherry-picked with + author credit + maintainer commit `1092d4f68` (Chat-shaped `function.name` in + `collectBareToolSpecs`, regression red without it). +- Admin squash-merge → `b732b0d0f` on `dev`; ancestry proven. #3226 closed as landed via + maintainer with the SHA and the addition explained. +- Audit trail: Erdos failed the PR as-is (nested-name hole); plan revised; Chandrasekhar verified + the hole and passed the carry plan. +- Checks: focused set 202 pass / 0 fail (receipt), typecheck, privacy:scan. + diff --git a/devlog/_plan/260902_bug_label_drawdown/082_p3227.md b/devlog/_plan/260902_bug_label_drawdown/082_p3227.md new file mode 100644 index 0000000000..a2edf7e291 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/082_p3227.md @@ -0,0 +1,23 @@ +# 082 — p3227: fail over zero-output incomplete combo streams + +Work-phase `p3227`. Contributor PR #3227 by @RHODIZSECURITY (head `43d23383d`, base `dev`, +draft, MERGEABLE). + +## What it changes + +`preflightComboStreamResponse` only treated `response.failed` as a retryable zero-output +terminal. An upstream answering HTTP 200 with a stream that ends as `response.incomplete` for +a transport reason (`adapter_eof`, `missing_terminal_event`, `upstream_stall_timeout`) was +accepted, so a combo with healthy backups never advanced. The PR adds those three reasons to a +retryable set; semantic incompletes (`max_output_tokens`, `content_filter`) and any incomplete +after output has committed remain accepted. Tests: three unit cases + one server-level e2e +where A ends with `adapter_eof` before output and B wins, with receipts asserting 502 → 200. + +## Review plan + +- Reviewer: confirm the three reasons are the ones the proxy itself mints for transport faults + (not upstream semantics), the output-commit boundary is untouched, and the failed attempt is + accounted as a 502 in log/usage receipts. +- Focused tests on the PR head; typecheck; privacy. +- Admin squash-merge; ancestry; `083_p3227_landing.md`. + diff --git a/devlog/_plan/260902_bug_label_drawdown/083_p3227_landing.md b/devlog/_plan/260902_bug_label_drawdown/083_p3227_landing.md new file mode 100644 index 0000000000..7e029deeaf --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/083_p3227_landing.md @@ -0,0 +1,7 @@ +# 083 — p3227 landing + +- Carry PR #3236 (branch `codex/260902-p3227-carry`): the #3227 commit rebased onto `dev` with author credit. +- Admin squash-merge → `1c8278b4d` on `dev`; ancestry proven. #3227 closed as landed via maintainer. +- Reviewer (xai/grok-4.6) pass: the three reasons are proxy-minted transport faults; commit boundary untouched; 502 attempt accounted. +- Checks: 88 pass / 0 fail on the PR head, after rebase, and on the landed `dev` tip (receipt); typecheck, privacy:scan. + diff --git a/devlog/_plan/260902_bug_label_drawdown/084_p3228.md b/devlog/_plan/260902_bug_label_drawdown/084_p3228.md new file mode 100644 index 0000000000..b547cc089c --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/084_p3228.md @@ -0,0 +1,34 @@ +# 084 — p3228: encrypted V2 spawn native fallback without a configured chain + +Work-phase `p3228`. Contributor PR #3228 by @x3M3x (head `06fe048bd`, draft, base `dev`; +`enforce-target` fails because the description mentions `gui` with no screenshot). + +## What is in the PR + +Two unrelated things: + +1. **The bug fix** (`src/codex/subagent-model-fallback.ts`, 5 lines + 1 test): an encrypted V2 + worker payload needs the native ChatGPT backend, but `applySubagentModelFallback` only + consulted a fallback chain the operator configured. With no chain, a routed sub-agent model + reached the encrypted-task guard and failed with `unreadable_encrypted_agent_task`. The fix + uses `normalizedChain(modelId, config, [], DEFAULT_SUBAGENT_MODELS)` when + `nativeFallbackOnly` and no chain is configured; ordinary routed spawns are unchanged. +2. **A GUI feature** (`gui/src/pages/Subagents.tsx`, `SubagentDelegationSection.tsx`, nine + i18n files, ~130 lines): a fallback-chain editor wired to the existing + `/api/subagent-model-fallback` routes. No screenshot, no issue, not a bug. + +## Disposition + +Land 1 via a carry branch from `origin/dev` (`Co-authored-by` credit, since a partial +cherry-pick is not a git operation). Leave 2 to a separate feature PR with a screenshot, which +the closing comment invites. #3228 closes as landed-partially. + +## Review plan + +- Reviewer: does `DEFAULT_SUBAGENT_MODELS` respect the operator's roster (disabled natives, + `codexAccountNamespaces`)? Is the `nativeFallbackOnly` filter in + `selectAvailableSubagentModel` still the thing that keeps non-forward candidates out? Is the + unit test red without the change? +- `bun test tests/subagent-model-fallback.test.ts`; typecheck; privacy. +- Admin squash-merge; ancestry; `085_p3228_landing.md`. + diff --git a/devlog/_plan/260902_bug_label_drawdown/085_p3228_landing.md b/devlog/_plan/260902_bug_label_drawdown/085_p3228_landing.md new file mode 100644 index 0000000000..c5adaec678 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/085_p3228_landing.md @@ -0,0 +1,8 @@ +# 085 — p3228 landing + +- Carry PR #3239 (branch `codex/260902-p3228-carry`): the source hunks of #3228 with `Co-authored-by` credit; the bundled GUI editor left for a feature PR with a screenshot. +- Admin squash-merge → `744d12d02` on `dev`; ancestry proven. #3228 closed as landed (source half) with the split explained. +- Reviewer Cicero (xai/grok-4.6) pass; local red-green on the carry worktree (test fails without the src hunk, 60 pass with). +- Checks: subagent-model-fallback focused file, typecheck, privacy:scan. +- Non-blocking caveat recorded: entitlement filtering still uses the null initial chain (`core.ts:2945`); the first synthetic candidate `gpt-5.5` is ungated. + diff --git a/devlog/_plan/260902_bug_label_drawdown/086_p3229.md b/devlog/_plan/260902_bug_label_drawdown/086_p3229.md new file mode 100644 index 0000000000..66c9443240 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/086_p3229.md @@ -0,0 +1,23 @@ +# 086 — p3229: admit the Codexless originator in V2 task recovery + +Work-phase `p3229`. Contributor PR #3229 by @iamnomankazi (head `6fb0bbad9`, draft, +24/-0). + +## What it changes + +`CODEX_ORIGINATORS` in `src/server/responses/agent-task-recovery.ts` gains +`codexless_agent`. Without it, an encrypted V2 sub-agent task spawned through Codexless is +refused at recovery admission and fails as `unreadable_encrypted_agent_task`. One security test +asserts the recovery request forwards `originator=codexless_agent`. + +## Why this needs a security look + +The set gates which client originators may enter the recovery path, which then forwards a +credential to `chatgpt.com`. Adding a name must not weaken the checks that follow it (OAuth +issuer, client id, token shape). Review confirms the later checks are untouched and that the +string is the one Codexless actually sends. + +## Landing + +Carry onto `origin/dev` with author credit; focused test; admin squash-merge; +`087_p3229_landing.md`. + diff --git a/devlog/_plan/260902_bug_label_drawdown/087_p3229_landing.md b/devlog/_plan/260902_bug_label_drawdown/087_p3229_landing.md new file mode 100644 index 0000000000..114aa6e73b --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/087_p3229_landing.md @@ -0,0 +1,18 @@ +# 087 — p3229 landing (and the #3239 regression it exposed) + +- Carry PR #3241 (branch `codex/260902-p3229-carry`): the #3229 two-file diff re-applied on the + current tip with `Co-authored-by`. Admin squash-merge → `b54508c8c` on `dev`; ancestry + proven. #3229 closed as landed via maintainer. +- Reviewer (xai/grok-4.6) pass: `CODEX_ORIGINATORS` is admission-only; issuer/client/token/ + account/proxy-secret gates after it unchanged; `codexless_agent` is Codexless's real + `clientInfo.name`. Red-without-fix proven on `1c8278b4d`. +- **Regression caught by this cycle's check**: on `744d12d02` (#3239, the p3228 carry) + `tests/agent-task-recovery-security.test.ts` was 2/13 (13/13 at `1c8278b4d`). The synthesized + `DEFAULT_SUBAGENT_MODELS` chain fired in the first fallback pass, rerouting an unreadable + encrypted spawn to native before `recoverEncryptedAgentTask` ran, so recovery's security + gates never executed. Repaired as work-phase `r3239`: PR #3240 → `7f00d0eee` gates the + synthesized chain on `config.agentTaskRecovery?.enabled !== true`; unit regression red + without the guard; security file 14/14 again. This is exactly the "CI behind the work, repair + as its own cycle" contract — the focused check on the next PR caught it before CI did. +- Checks on the landed tip: recovery security 14 pass / 0 fail; typecheck; privacy. + diff --git a/devlog/_plan/260902_bug_label_drawdown/088_r3239.md b/devlog/_plan/260902_bug_label_drawdown/088_r3239.md new file mode 100644 index 0000000000..b5de222a8d --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/088_r3239.md @@ -0,0 +1,27 @@ +# 088 — r3239: repair the #3239 regression (recovery gates bypassed) + +Work-phase `r3239`. Found by the p3229 focused check, not by CI (dev CI runs were being cancelled +by the merge train). + +## Regression + +`744d12d02` (#3239) synthesized a `DEFAULT_SUBAGENT_MODELS` chain for an unreadable encrypted +spawn when no chain is configured. In `core.ts` the first `applySubagentModelFallback` pass runs +before `recoverEncryptedAgentTask`; with the synthesized chain that pass rerouted the spawn to +native `gpt-5.5`, the route became canonical-forward, recovery was skipped, and recovery's +caller-auth / proxy-secret / token-validity gates never executed. +`tests/agent-task-recovery-security.test.ts`: 13/13 at `1c8278b4d` → 2/13 at `744d12d02`. + +## Fix + +Gate the synthesized chain on `config.agentTaskRecovery?.enabled !== true`. An operator who +enabled recovery chose to decrypt and stay routed; a configured chain keeps its precedence; the +#3239 case (recovery off, no chain) is unchanged. + +## Landing + +PR #3240 → `7f00d0eee` on `dev`. Unit regression red without the guard; security file 14/14. + +Audit (xai/grok-4.6): pass — recovery-on + no chain does not also want the native rescue; a +failed recovery still hits the fail-closed 400, and the post-recovery second pass applies only +a configured chain. diff --git a/devlog/_plan/260902_bug_label_drawdown/089_p3232.md b/devlog/_plan/260902_bug_label_drawdown/089_p3232.md new file mode 100644 index 0000000000..5858b42398 --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/089_p3232.md @@ -0,0 +1,10 @@ +# 089 — p3232: sibling start must not persist its port (merged by maintainer) + +Work-phase `p3232`. PR #3232 (@lidge-jun, bug label) opened at 2026-09-02T01:17Z while this loop +was on p3226 and was merged directly by the maintainer as `261b7e012` (ancestor of +`origin/dev`, verified). No carry or review work was needed from the loop; this phase records +the landing and re-runs the PR's test files on the current tip so the count and the CI verdict +stay honest: `tests/cli-dispatch.test.ts tests/ports.test.ts`. + +Result on the current tip: 46 pass / 0 fail. Audit (xai/grok-4.6): pass — record-only is the +correct disposition. diff --git a/devlog/_plan/260902_bug_label_drawdown/090_regaudit3.md b/devlog/_plan/260902_bug_label_drawdown/090_regaudit3.md new file mode 100644 index 0000000000..2d1dfe40dc --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/090_regaudit3.md @@ -0,0 +1,78 @@ +# 090 — regaudit3: final recount and closeout + +Terminal phase after `p3226`–`p3232` and `r3239`. + +## Recount (2026-09-02, final) + +`gh issue list -l bug --state open` → **4** (#3152, #3141, #2999, #1527), +`gh pr list -l bug --state open` → 0. Combined **4**. + +An earlier pass of this doc read 5: #1419 (bundled Bun SIGTRAP) was closed by the maintainer at +2026-09-02T02:43Z as completed — the bundled Bun moved to 1.4.0, with a reopen invitation if it +recurs. That closure was not made by this loop and not made to lower the count; it is the +platform fix the blocker was waiting for. The four remaining are the recorded blockers from 072, +each with a maintainer comment naming the evidence it needs and `needs-info` where the +reporter owns the next step (#2999 is the runtime-primitive blocker). + +`origin/dev` has since moved past `2cb592174` with feature/docs landings by the maintainer +(#3222, #3230, #3231, #3225); none carries the bug label and none is in this campaign's scope. +The CI verdict below is pinned to `2cb592174`, the last commit this campaign put on `dev`. + +## Landings since regaudit2 (all ancestors of `origin/dev`) + +| item | landing | note | +|---|---|---| +| #3226 → #3234 | `b732b0d0f` | carry + nested `function.name` fix | +| #3227 → #3236 | `1c8278b4d` | carry, author credit | +| #3228 → #3239 | `744d12d02` | **reverted** by #3242 `2cb592174` — see below | +| regression from #3239 → #3240 | `7f00d0eee` | **reverted** by #3242 together with #3239 | +| #3229 → #3241 | `b54508c8c` | carry on the repaired tip | +| #3232 | `261b7e012` | merged by the maintainer directly; verified | + +## Trailing CI + +The `push` runs on `dev` during this train were all cancelled by the next push. The r3239 +regression was caught by the next cycle's focused check, not by CI, and repaired before anything +else landed — which is the point of pairing "CI behind the work" with a focused red-green gate +on every PR. Exact-head `workflow_dispatch` on `b54508c8c` (branch +`codex/regaudit-ci-b54508c8c`, run 33581824312, Windows on): **test 3/4 failed** on +`tests/agent-task-recovery.test.ts` "keeps the disabled fail-fast response byte-identical to +the absent feature" (400 expected, 502 received). Bisect: 19/19 at `1c8278b4d`, 7/19 at +`744d12d02` (#3239), 18/19 at `7f00d0eee` (#3240). The contract that file pins — recovery +absent/disabled ⇒ fail-fast 400 with zero upstream fetches — is a credential-spend boundary: +synthesizing a native chain reroutes a routed spawn to the ChatGPT backend without the operator +opting in. #3240 could not restore that without removing the feature, so **both were reverted** +in #3242 → `2cb592174` (92 pass / 0 fail across the three recovery/fallback files after the +revert). #3228's disposition is corrected on the PR: the reported behaviour is the documented +opt-in, not a bug; a defaults change is a product decision for a feature request. The p3228 +review ran the fallback and security files but not `agent-task-recovery.test.ts` — recorded +as the miss. + +Second dispatch on the reverted tip `2cb592174` (run 33582128589, Windows on): Linux test +1/4–4/4 all green (the `agent-task-recovery` failure is gone), gates, storage, api-usage, +keyring ×3, npm-global ×3 green. Windows 1/2/4 failed with the known runner signatures +(`EPERM rm tests/.tmp-codex-accounts-test` ×49, icacls `ETIMEDOUT`, "Bun runtime crash"); +3/4 cancelled by the gate. macOS failed one case: `native-profile-manager` "preserves exact +auth bytes, encrypts inactive profiles…" at 12.7 s — a file untouched since `#3054` +(2026-08-29, on `main`), which passed in both earlier dispatches (33562938994, 33581824312) +and 49/49 three times locally on this tip; its history is two macOS-timing bounding commits +(`bef2869c7`, `c1be34da4`). Classified as a macOS timing flake; a third dispatch +(run 33584155821) is recorded below to settle it. + +Third dispatch on `2cb592174` (run 33584155821): **macOS green**, Linux 1/4–4/4 green, gates, +storage, api-usage, keyring ×3, npm-global ×3 green. The `native-profile-manager` case is +settled as a macOS timing flake (fail 1 of 3 dispatches on an unchanged file). Windows shards +remain the hosted-runner defect proven on `main` in 071. Verdict for the campaign's last +source commit: green on every platform this repository can currently trust. + +## Devlog landing + +PR #3218 (this stack, rebased on the current `dev` tip) → admin squash-merge; SHA recorded in +the ledger and the goalplan criterion evidence. + +## Criterion c-7 + +Met: **4** open bug-labelled items (≤5 fallback; 3 would have required closing a blocker without +its evidence), all four with recorded, evidence-backed blockers. From 24 at the start of the +campaign: 14 PRs + 10 issues → 0 PRs + 4 issues. 22 landings / closures with SHAs or evidence +comments, one honest revert. diff --git a/devlog/_plan/260902_bug_label_drawdown/091_rv3239.md b/devlog/_plan/260902_bug_label_drawdown/091_rv3239.md new file mode 100644 index 0000000000..a743f9ef9d --- /dev/null +++ b/devlog/_plan/260902_bug_label_drawdown/091_rv3239.md @@ -0,0 +1,30 @@ +# 091 — rv3239: revert the synthesized native chain (#3239, #3240) + +Work-phase `rv3239`. Triggered by the regaudit3 exact-head dispatch (run 33581824312, tip +`b54508c8c`): `test 3/4` failed `tests/agent-task-recovery.test.ts` "keeps the disabled +fail-fast response byte-identical to the absent feature" (400 expected, 502 received). + +## Why revert rather than patch again + +That test pins a credential-spend boundary: with `agentTaskRecovery` absent or disabled, an +encrypted spawn on a routed model must fail fast with a 400 and make zero upstream fetches. +#3239's synthesized chain reroutes that spawn to the native ChatGPT backend — a stored +credential spent on a model the operator never opted into. #3240 fixed the recovery-*on* path +but the recovery-*off* contract cannot hold while the feature exists. The reported behaviour in +#3228 is the documented opt-in (configure a `subagentModelFallback` chain or enable recovery); +changing that default is a product decision, not a bug fix. + +## Landing + +PR #3242 → `2cb592174` on `dev` (pure revert of `7f00d0eee` and `744d12d02`). After the +revert: agent-task-recovery 19/19, agent-task-recovery-security 14/14, subagent-model-fallback +59/59 (92 pass / 0 fail). #3228 corrected on the PR with an apology and the opt-in explained. + +## What the loop got wrong + +The p3228 review ran `subagent-model-fallback` and (via p3229) the security file, but not +`agent-task-recovery.test.ts`, which is the file that owns the fail-fast contract. "Focused +test" has to mean every file that pins the touched behaviour, not only the file the PR edited. + +Audit (xai/grok-4.6): pass — pure revert confirmed byte-identical to `744d12d02^`; revert is the +right call over a third patch. diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/000_plan.md b/devlog/_plan/260902_bug_pr_closeout_stack/000_plan.md new file mode 100644 index 0000000000..f53fb643f1 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/000_plan.md @@ -0,0 +1,60 @@ +# 000 — bug_pr_closeout_stack: Plan + +## Objective + +Close as many open opencodex bugs and pull requests as can be closed with evidence, +in one session, without running the repository-wide local suite. Two mechanisms: + +1. **Merge train** for pull requests that already carry a maintainer-reviewed body + and a green or known-flake-only exact-head CI run. +2. **Stacked implementation** for issues whose defect is fully visible in the tree, + each landing as its own squash merge into `dev`. + +Evidence base collected 2026-09-02 in this worktree: + +- 47 open PRs, 55 open issues (`gh pr list`, `gh issue list`). +- `gh pr checks 3163` — 23/23 pass on head `486b2f99f3182acf055274755ade9c6571203ac9`. +- `gh pr checks 3166` — head `17f01162ad404f1bcee7d7f00998fc0e143365e5`; `test 3/4` red on + `tests/responses-state.test.ts > late async spill completion cannot overwrite the shutdown + fallback` with `ETIMEDOUT` out of `src/responses/spill-store.ts:232` (ACL budget exhausted + under runner load). The PR touches `src/codex/auth-context.ts` only — the failure is a + timing flake in an unrelated subsystem. +- `gh pr checks 2083` — 24/24 pass, `APPROVED`, `CLEAN`; #2986 is its maintainer carry on + current `dev` with an independent security review recorded in the PR body. + +## Loop-spec + +- Loop archetype: verifier-defined (each item has a binary landing proof). +- Write scope: `src/cli/models.ts`, `src/combos/request.ts`, `src/server/responses/core.ts` + (read-only for phase 5), `docs-site/src/content/docs/reference/cli/lifecycle.md`, + `docs-site/src/content/docs/reference/configuration/server.md`, matching `tests/` files, + and this devlog unit. +- Out of scope: releases, promotion to `main`/`preview`, npm publish, deployment, auth or + credential rewrites beyond what a named issue requires, other worktrees. +- Verification policy (user-directed): **no repository-wide local suite**. CI runs behind the + work — each phase pushes, opens its PR, and merges by admin; CI is then tracked and judged + at the end of the train rather than blocking each merge. +- Merge mechanism: `gh pr merge --squash --admin`. + +## Work-phase map (one phase = one full PABCD cycle) + +| WP | Doc | Slice | Depends on | +|----|-----|-------|------------| +| wp0 | 000 | this roadmap; goalplan lock | — | +| wp1 | 010 | land PR #3163 (closes #3156) | wp0 | +| wp2 | 020 | land PR #3166 (closes #3157) | wp1 | +| wp3 | 030 | land carry PR #2986; close #2083 landed-via-maintainer | wp2 | +| wp4 | 040 | implement #3094 — `ocx models new-policy`/`new-arrivals` dispatch | wp3 | +| wp5 | 050 | implement #3108 — combo default reasoning effort reaches the target | wp4 | +| wp6 | 060 | implement #3158 T19/T21 — `/readyz` shape + three hub/remoteGui config keys | wp5 | + +wp4–wp6 are a stack: each branch is cut from the previous one's landed `dev`, so a lower +layer's merge is the upper layer's base (DEV-STACK-01). + +## Accept criteria + +- c-1..c-6: one per work-phase, each requiring a merge SHA proven an ancestor of + `origin/dev` via `git merge-base --is-ancestor FETCH_HEAD`, plus the issue closed. +- c-7: at least three bug/PR items closed with landing proof. +- Final CI judgment: `gh run list --branch dev` green on the last landed `dev` head, or every + remaining red identified as the known `responses-state` spill flake. diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/010_phase1.md b/devlog/_plan/260902_bug_pr_closeout_stack/010_phase1.md new file mode 100644 index 0000000000..5bcda6b7b8 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/010_phase1.md @@ -0,0 +1,34 @@ +# 010 — Phase 1: land PR #3163 (closes #3156) + +## What lands + +PR #3163 `ingw/fix-copilot-context-3156` — head `486b2f99f3182acf055274755ade9c6571203ac9`. + +- MODIFY `src/codex/catalog/provider-fetch.ts` (+49) — read GitHub Copilot's live context + window at `capabilities.limits.max_context_window_tokens`, preserving the existing + metadata precedence and the safe-integer boundary for malformed values. +- MODIFY `tests/codex-catalog.test.ts` — routed catalog regression for accepted, + conflicting, and invalid Copilot payloads. + +No local code is written in this phase; the diff is the contributor's. + +## Why it is landable as-is + +`gh pr checks 3163` reports 23 checks, all pass, on the exact head above. The PR body +carries root cause, precedence reasoning, and per-suite verification counts +(`tests/codex-catalog.test.ts`: 255 pass / 0 fail). + +## Execution + +1. Re-read `gh pr view 3163 --json headRefOid,mergeStateStatus` to confirm no drift. +2. `gh pr merge 3163 --squash --admin --delete-branch`. +3. `git fetch origin dev` and `git merge-base --is-ancestor FETCH_HEAD`. +4. Confirm #3156 auto-closed; `Closes #3156` targets `dev`, which is not the default + branch, so close it by hand if GitHub did not. + +## Verification (C) + +- `gh pr view 3163 --json state,mergeCommit` reports MERGED with a SHA. +- ancestry check exits 0. +- `gh issue view 3156 --json state` reports CLOSED. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/011_wp1_landing.md b/devlog/_plan/260902_bug_pr_closeout_stack/011_wp1_landing.md new file mode 100644 index 0000000000..1a73fb536b --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/011_wp1_landing.md @@ -0,0 +1,29 @@ +# 011 — wp1 landing record: PR #3163 + +## Landed + +- PR: #3163 `ingw/fix-copilot-context-3156` by @Ingwannu +- Head audited: `486b2f99f3182acf055274755ade9c6571203ac9` +- Merge SHA on `dev`: `e236c36239c93f006a706aba3e7c84da167b5dd9` +- Mechanism: `gh pr merge 3163 --squash --admin --delete-branch` +- Closes: #3156 (closed manually — PRs target `dev`, not the default branch, so + GitHub does not auto-close) + +## Evidence at merge time + +`gh pr checks 3163` — 23 checks, every one `pass`, on the audited head. No waived check. + +## Ancestry proof + + git fetch origin dev + git merge-base --is-ancestor e236c36239c93f006a706aba3e7c84da167b5dd9 FETCH_HEAD + # exit 0 + +## What changed in the product + +`src/codex/catalog/provider-fetch.ts` now reads GitHub Copilot's live context window from +`capabilities.limits.max_context_window_tokens`. Before this, that field was unrecognized and +Copilot models fell back to the conservative 128K window. Existing metadata precedence and +the safe-integer rejection of malformed values are unchanged; +`tests/codex-catalog.test.ts` covers accepted, conflicting, and invalid payloads. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/020_phase2.md b/devlog/_plan/260902_bug_pr_closeout_stack/020_phase2.md new file mode 100644 index 0000000000..c692fdfd63 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/020_phase2.md @@ -0,0 +1,48 @@ +# 020 — Phase 2: land PR #3166 (closes #3157) + +## What lands + +PR #3166 `ingw/fix-request-owned-main-pin-3157` — head `17f01162ad404f1bcee7d7f00998fc0e143365e5`. + +- MODIFY `src/codex/auth-context.ts` — honor an effective healthy manual `__main__` pin when + a Pool-mode request carries its own forwardable Codex bearer; validate that caller + credential's account-gated model roster; keep paused or quota-drained mains on the + ordinary Pool promotion path. +- MODIFY `structure/08_openai-provider-tiers.md` — document the request-owned credential and + pin boundary. +- MODIFY `tests/codex-auth-context.test.ts` — 68 passing cases including healthy main at 16% + vs Pool at 100%, drained main vs healthy Pool, and caller entitlement denial with a + model-only detour. + +## The red check + +`test 3/4` fails on `tests/responses-state.test.ts > Responses previous_response_id state > +late async spill completion cannot overwrite the shutdown fallback`: + + error: Response spill ACL budget exhausted + code: "ETIMEDOUT" + at nextSpillHardenDeadlineMs (src/responses/spill-store.ts:232:29) + +That is a wall-clock ACL budget expiring on a loaded runner. The PR's diff does not reach +`src/responses/`. Treat it as an unrelated flake: rerun the failed job, continue the train, +and judge the result at the end rather than blocking the merge on it. + +## Security note + +This is a credential-selection change, so MAINTAINERS.md requires explicit security review. +The PR body records the trust boundary: no caller bearer is persisted, no Pool +affinity/health/entitlement state is written, and the physical main credential is not read. +The merging maintainer accepts that review. + +## Execution + +1. `gh run rerun --failed` for the exact head, then continue and poll later. +2. `gh pr merge 3166 --squash --admin --delete-branch`. +3. ancestry proof plus `gh issue view 3157`. + +## Verification (C) + +- merge SHA is an ancestor of `origin/dev`. +- #3157 closed. +- rerun of the flaked job recorded: green, or still flaking with the same unrelated stack. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/021_wp2_landing.md b/devlog/_plan/260902_bug_pr_closeout_stack/021_wp2_landing.md new file mode 100644 index 0000000000..21334e4703 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/021_wp2_landing.md @@ -0,0 +1,45 @@ +# 021 — wp2 landing record: PR #3166 + +## Landed + +- PR: #3166 `ingw/fix-request-owned-main-pin-3157` by @Ingwannu +- Head audited: `17f01162ad404f1bcee7d7f00998fc0e143365e5` +- Merge SHA on `dev`: `75090d4e0e26637a3db0157edf3090830ba00d52` +- Mechanism: `gh pr merge 3166 --squash --admin --delete-branch` +- Closes: #3157 (closed manually) + +## The flake, and how it was resolved rather than waived + +At first inspection `test 3/4` was red on +`tests/responses-state.test.ts > late async spill completion cannot overwrite the shutdown +fallback` with `ETIMEDOUT` from `src/responses/spill-store.ts:232` — a wall-clock ACL budget +expiring on a loaded runner, in a subsystem this PR does not touch. + +Rather than merge over a red check, the run was re-inspected: run `33527409692` had already +been re-run and reported `completed success`, and `gh pr checks 3166` returned zero `fail` +lines on the same head. The merge went in on a genuinely green rollup. + +## Ancestry proof + + git merge-base --is-ancestor 75090d4e0e26637a3db0157edf3090830ba00d52 origin/dev + # exit 0 + +## What changed in the product + +A Pool-mode request carrying its own forwardable Codex bearer no longer clears a healthy +manual `__main__` pin. Request-owned credentials are excluded from stored-account entitlement +discovery by design, and the shared-selection path had been reading that exclusion as evidence +the pinned main was dead — persisting a Pool account at 100% usage over a main at 16%. + +The fix validates the caller credential's own account-gated roster, gives an unentitled caller +a model-only detour that leaves the shared pin intact, and keeps paused or quota-drained mains +on the ordinary Pool promotion path. + +## Security boundary + +MAINTAINERS.md requires explicit security review for credential-selection changes. The PR +records the boundary: no caller bearer is persisted, no Pool affinity, health, or entitlement +state is written, and the physical main credential is not read. Documented in +`structure/08_openai-provider-tiers.md`; regressions in `tests/codex-auth-context.test.ts` +(68 pass). + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/030_phase3.md b/devlog/_plan/260902_bug_pr_closeout_stack/030_phase3.md new file mode 100644 index 0000000000..3fa81f6640 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/030_phase3.md @@ -0,0 +1,40 @@ +# 030 — Phase 3: land carry PR #2986, close #2083 + +## What lands + +PR #2986 `codex/carry-2083-xai-imagine` — maintainer carry of #2083 by @zhou-zhichao, +8 commits cherry-picked onto `dev` with author credit preserved. + +Surface: `src/images/` (artifacts, fulfill, index, plan, synthetic-tool, xai-client), +`src/responses/parser.ts`, `src/server/images.ts`, five locales of +`docs-site/.../guides/image-bridge.md` and `codex-integration.md`, plus six test files. + +Relays Codex `image_gen` tool calls to xAI Imagine using Grok OAuth, gated behind exact +`images.bridgeEnabled === true`. + +## Why the carry exists + +#2083 is APPROVED with 24/24 green checks, but its head had drifted 35 commits behind +`dev` — past the repository's 10-commit freshness boundary — so the green run no longer +describes what would land. A maintainer cannot push to a contributor branch, hence the carry. + +## Security review status + +Recorded in the PR body, performed on the exact head: credentials pinned to +`https://api.x.ai/v1`, `redirect: "manual"` on the credentialed fetch, no prompt or +credential logging, opt-in gate fails closed with a fixed 400 before any fallback, artifact +reads require API admission plus Origin validation. Verdict PASS WITH NOTES — artifact +authorization is proxy-wide, matching the single-operator trust model. + +## Execution + +1. `gh pr view 2986 --json headRefOid,mergeStateStatus`; rebase onto current `dev` if the + carry fell behind after phases 1-2. +2. `gh pr merge 2986 --squash --admin --delete-branch`. +3. `gh pr close 2083` with a landed-via-maintainer comment naming the merge SHA. + +## Verification (C) + +- merge SHA ancestor of `origin/dev`. +- #2986 MERGED, #2083 CLOSED with the crediting comment. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/031_wp3_disposition.md b/devlog/_plan/260902_bug_pr_closeout_stack/031_wp3_disposition.md new file mode 100644 index 0000000000..067bb9aa43 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/031_wp3_disposition.md @@ -0,0 +1,50 @@ +# 031 — wp3 disposition: PR #2986 / #2083 do NOT land in this train + +## Decision + +**NEEDS_REWORK, not merged.** The roadmap (030_phase3.md) assumed #2986 was a clean carry +awaiting a maintainer merge. Refreshing the live state at execution time contradicted that. + +## Evidence at execution time + +- `gh pr view 2986` — `OPEN`, `mergeStateStatus: BLOCKED`, + head `842170b6f3d076a8274c1cba8824f3e3c56f0bb7`. +- `reviewDecision: CHANGES_REQUESTED`, from maintainer @Ingwannu — not a stale bot nit. +- `git rev-list --count 870a2adb6eaccc9da9ea9832a596e1b2650ab1ea..origin/dev` → **179**. + The PR base is 179 commits behind `dev`, so its green CI describes a tree that no longer + exists — the same freshness problem that caused the carry in the first place. + +## What the maintainer asked for + +Three runtime edge cases, each concrete and each still open: + +1. `src/images/fulfill.ts` resolves `aspect_ratio: "auto"` to `undefined` before calling + `callXaiImages`, so `resolveAspectRatio()` treats the field as absent and derives a ratio + from `size`. An explicit Auto selection therefore stops suppressing size-derived selection. +2. `src/responses/parser.ts` replaces only the *first* unnamespaced `image_gen` when a hosted + declaration arrives. With both an ordinary and a custom root declaration ahead of it, the + second survives and the catalog stays ambiguous. +3. The default downloader in `connectPublicHttps` passes `maxBytes: undefined` to + `pinnedHttpGet`, dropping the `MAX_DOWNLOAD_BYTES` cap when a caller omits a limit. + +Plus a docs correction: the xAI `/v1/images` relay runs only when `bridgeEnabled === true` +**and** `images.provider` is omitted; an explicit image provider owns the route. + +## Why this train does not do it + +Merging over an explicit maintainer `CHANGES_REQUESTED` with `--admin` would spend the +maintainer's review authority to bypass the maintainer. Item 3 is a byte-cap regression on a +credentialless download path — a security-boundary defect, exactly the class +`MAINTAINERS.md` says needs review rather than an override. + +The rework is tractable (four small edits plus a rebase) but it is a different unit of work +from "land a reviewed PR", and it belongs to the author on the same branch, which is what the +maintainer explicitly asked for: *"Please address these on the same branch and rerun the +focused image/parser suites."* + +## Outcome + +- #2986: left open, awaiting author rework. No admin merge. +- #2083: left open. Closing it as `landed-via-maintainer` would be false — nothing landed. +- Train continues to wp4 (#3094), wp5 (#3108), wp6 (#3158 docs). + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/040_phase4.md b/devlog/_plan/260902_bug_pr_closeout_stack/040_phase4.md new file mode 100644 index 0000000000..0c88ba8ebc --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/040_phase4.md @@ -0,0 +1,55 @@ +# 040 — Phase 4: #3094 — ocx models new-policy / new-arrivals are unreachable + +## Defect + +`src/cli/models-runtime.ts:332-333` implements both subcommands: + + else if (sub === "new-policy") action = () => newPolicy(argv, deps); + else if (sub === "new-arrivals") action = () => newArrivals(argv, deps); + +and `src/cli/models-runtime.ts:27-28` lists them in USAGE. But `handleModels` in +`src/cli/models.ts:448` routes only a hardcoded list to the runtime module: + + if (["live", "edit", "enable", "disable", "provider", "selected", "preset", "context", "shadow"].includes(subcommand ?? "")) { + +Neither name is in it, so both fall through to `handleConfiguredModels`, which rejects the +argument: `Unexpected argument(s): new-policy, status`, exit 1. +`docs-site/src/content/docs/guides/model-routing.md:88-89` documents both commands. + +## Root cause, not just symptom + +Two lists name the same set and only one was updated. The fix removes the duplication: +the runtime module owns its subcommand set and `handleModels` consumes it. + +## MODIFY map + +**`src/cli/models-runtime.ts`** — export the set the dispatcher already encodes: + + export const MODELS_RUNTIME_SUBCOMMANDS = [ + "live", "edit", "enable", "disable", "provider", "selected", "preset", + "new-policy", "new-arrivals", "context", "shadow", + ] as const; + +and drive `handleModelsRuntimeCommand`'s guard from it, keeping the existing per-name +action mapping. + +**`src/cli/models.ts`** — replace the literal array with the imported set. The import must +stay lazy if the current dynamic `await import("./models-runtime")` exists to keep the CLI +startup path light; if so, import the constant from a leaf module rather than pulling the +whole runtime eagerly. + +## TESTS + +**NEW `tests/cli-models-runtime-dispatch.test.ts`**: + +1. every name in `MODELS_RUNTIME_SUBCOMMANDS` is routed by `handleModels` to the runtime + module rather than `handleConfiguredModels` — the general form of the defect, so a future + subcommand added without touching the dispatch fails here. +2. `new-policy` and `new-arrivals` specifically reach the runtime handler. +3. an unknown subcommand still falls through to `handleConfiguredModels`. + +## Verification (C) + +- `bun test tests/cli-models-runtime-dispatch.test.ts` focused. No repository-wide suite. +- typecheck and the rest are CI's job, judged at the end of the train. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/050_phase5.md b/devlog/_plan/260902_bug_pr_closeout_stack/050_phase5.md new file mode 100644 index 0000000000..d4396daae3 --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/050_phase5.md @@ -0,0 +1,86 @@ +# 050 — Phase 5: #3108 — combo default reasoning effort arrives as none + +## Reported behaviour + +Combo `combo/0` with default reasoning level `max` routed to `deepseek-v4-pro` sends +`none`; selecting `deepseek-v4-pro` directly with `max` sends `max`. OpenCodex 2.37.0. + +## Mechanism in the tree + +`src/server/responses/core.ts:2277` builds the child body: + + const childBody = concreteComboRequestBody( + body, + pick.target, + comboDefaultEffort(config, comboId), + supportedLadderFor({ provider: targetRoute.provider, modelId: targetRoute.modelId }), + ); + +`src/combos/request.ts:75` then refuses to inject: + + if (!targetReasoningEfforts?.includes(defaultEffort)) { /* debug log */ return clone; } + +So the default is dropped whenever the concrete target's ladder does not literally contain +the configured rung — including when the ladder is `undefined`. The comment calls this +deliberate fail-closed behaviour, but the catalog path disagrees: +`src/codex/catalog/aggregation.ts:168` advertises the combo's default through +`effectiveComboDefault`, which downgrades a too-high request to the nearest supported rung +at or below it (`aggregation.ts:86-93`) instead of dropping it. + +That asymmetry is the defect: the catalog promises `max` or the nearest rung below, the +runtime silently sends nothing, and the provider default — `none` — applies. + +## MODIFY map + +**`src/combos/request.ts`** — reuse the catalog's own resolution instead of exact membership: + + const resolved = targetReasoningEfforts === undefined + ? undefined + : effectiveComboDefault(defaultEffort, targetReasoningEfforts); + if (!resolved) { /* same warn shape */ return clone; } + +then inject `resolved` rather than `defaultEffort`. + +- an unknown (`undefined`) ladder stays fail-closed — that half of the behaviour is correct. +- an explicitly empty ladder still yields `undefined` from `effectiveComboDefault` + (`ranked.length === 0`), so a no-reasoning model is never given an effort. +- a caller-supplied `reasoning.effort` is still untouched; that check runs first. + +## Import boundary — RESOLVED AT AUDIT, the direct import is forbidden + +A-gate audit (independent explorer, grok-4.6) plus direct tracing settled this: +`src/codex/catalog/aggregation.ts` does NOT reach `src/lab/`, so `tests/core-lab-boundary.test.ts` +would stay green — but the import is still wrong for two harder reasons: + +1. **It is a cycle.** `aggregation.ts:22-29` already imports `../../combos`. Adding + `src/combos/request.ts` -> `aggregation.ts` closes the loop. +2. **It drags the catalog plane onto the request path.** `aggregation.ts:1-31` pulls + `node:child_process`, `../../oauth`, `../model-cache`, and + `../../adapters/cursor/live-models`. `src/server/responses/core.ts` imports `src/combos/`, + so every routed request would carry live-discovery and OAuth weight it never uses. + +**Therefore the fallback is the plan, not a contingency.** Lift the ranking helper into +`src/reasoning-effort.ts` — a genuine leaf whose only import is `./types`, and which already +owns `codexEffortRank` (`reasoning-effort.ts:79-81`), the exact primitive the helper needs. + +**MOVE**: `effectiveComboDefault` from `src/codex/catalog/aggregation.ts:80-94` to +`src/reasoning-effort.ts`, renamed `resolveEffortAtOrBelow(configured, supported)` to say what +it does independent of combos. `aggregation.ts` imports it from there (it already imports +`codexEffortRank` out of the same module at line 11) and keeps a local +`effectiveComboDefault` alias only if the existing call site reads better that way. +`src/combos/request.ts` imports the same leaf function. No cycle, no catalog weight. + +## TESTS + +**`tests/combos.test.ts`** already covers `concreteComboRequestBody`. Add: + +1. configured `max`, target ladder `["low","medium","high"]` -> injects `high`. +2. configured `max`, ladder includes `max` -> injects `max` (unchanged). +3. ladder `undefined` -> no injection (fail-closed, unchanged). +4. ladder `[]` -> no injection (unchanged). +5. caller-supplied `reasoning.effort` -> untouched (unchanged). + +## Verification (C) + +- `bun test tests/combos.test.ts` focused. +- CI judged at the end of the train. diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/060_phase6.md b/devlog/_plan/260902_bug_pr_closeout_stack/060_phase6.md new file mode 100644 index 0000000000..0840126d1b --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/060_phase6.md @@ -0,0 +1,51 @@ +# 060 — Phase 6: #3158 T19/T21 documentation debt + +Two of the four remote-hub follow-ups are documentation-only and close in one diff. +T2 and T3 are behaviour gaps and stay open on #3158. + +## T19 — /readyz gained protocol negotiation metadata + +`src/server/index.ts:1170-1178` builds the readiness body as: + + const body = { + service: "opencodex", version: VERSION, uptime: process.uptime(), + pid: process.pid, port: boundPort ?? listenPort, status, + ...readyProtocolMetadata(config, req), + }; + +`src/remote/protocol.ts:46-57` adds `protocol`, `minimumClientProtocol`, and `managementUrl` +— the configured `hub.managementPublicOrigin` when `runtimeRole === "hub"`, otherwise the +observed request origin. + +`docs-site/src/content/docs/reference/cli/lifecycle.md:164` still says the sanitized HTTP +identity is `{service, version, uptime, pid, port, status}`. + +**MODIFY `docs-site/src/content/docs/reference/cli/lifecycle.md`** — extend that sentence to +name the three added fields, say where `managementUrl` comes from in each runtime role, and +keep the CLI JSON shape `{ready, status, pid, port}` explicitly distinct from the HTTP body. + +## T21 — three config keys ship undocumented + +`src/types/config.ts:259,272,284` declare `hub.managementPublicOrigin`, +`remoteGui.allowedTailscaleUsers`, and `remoteGui.allowInsecureHttp`. + +`docs-site/src/content/docs/reference/configuration/server.md:268` mentions the first two in +one prose sentence about `runtimeRole`; `allowInsecureHttp` appears nowhere. The +`guides/remote-hub.md` page shows `ocx config set` examples, but the reference page is the +source of truth for key semantics. + +**MODIFY `docs-site/src/content/docs/reference/configuration/server.md`** — document each key +with type, default when absent, what it gates, and the failure mode of setting it wrong. +`allowInsecureHttp` in particular is a security-relevant opt-out and needs the warning. + +## Locales + +English is the source. Per AGENTS.md, translated locales must not contradict the English +source; the tr/ja/fr/ru/zh-cn pages already carry the `runtimeRole` sentence, so leave them +rather than half-translating. Note the gap in the PR body. + +## Verification (C) + +- `rg` proof that each key name now appears in the reference page. +- the docs build is a CI job (`gates`), judged with the rest of the train. + diff --git a/devlog/_plan/260902_bug_pr_closeout_stack/070_closeout.md b/devlog/_plan/260902_bug_pr_closeout_stack/070_closeout.md new file mode 100644 index 0000000000..8234c1acfc --- /dev/null +++ b/devlog/_plan/260902_bug_pr_closeout_stack/070_closeout.md @@ -0,0 +1,66 @@ +# 070 — closeout: what landed, what did not, and why + +## Landed on `dev` + +| Item | PR | Merge SHA | Issue | +|---|---|---|---| +| Copilot context window | #3163 | `e236c36239c93f006a706aba3e7c84da167b5dd9` | #3156 closed | +| Request-owned main pin | #3166 | `75090d4e0e26637a3db0157edf3090830ba00d52` | #3157 closed | +| `ocx models` dispatch | #3171 | `e92aa336a83c86283b500269a1d55779836114b0` | #3094 closed | +| Combo default effort | #3172 | `7386b52016be7b0246ca941d4e285ec340331431` | #3108 closed | +| Remote-hub reference docs | #3173 | `0d8147c2002e3e4e4adf39a03084d6a6ab18991e` | #3158 T19/T21 | +| Failover e2e assertion | #3175 | `22a643a00b5974fa53b084a04491f60d56ec9ee2` | follow-up to #3108 | + +Every SHA verified with `git merge-base --is-ancestor origin/dev` exiting 0. + +Six pull requests merged, four issues closed. The objective asked for at least three +bug/PR items; ten items moved. + +## Did not land, deliberately + +**#2986 / #2083 — xAI Imagine relay.** The roadmap assumed a clean carry awaiting a merge. +The refresh at execution time said otherwise: `BLOCKED`, `CHANGES_REQUESTED` from maintainer +@Ingwannu, and a base 179 commits behind `dev`. One of the three requested fixes is a +`MAX_DOWNLOAD_BYTES` cap dropped on a credentialless download path — a security-boundary +defect. Admin-merging over that would have spent maintainer authority to bypass the +maintainer. Recorded in `031_wp3_disposition.md`. + +**#3158 T2 and T3.** Behaviour gaps, not documentation. The issue stays open for them. + +## What the loop got wrong, and how it was caught + +Two plan claims did not survive contact: + +1. **The import the plan proposed was unsafe.** 050_phase5.md originally suggested importing + `effectiveComboDefault` from `aggregation.ts` into `request.ts`. The A-gate auditor and + an independent trace both found that closes a cycle (aggregation already imports + `src/combos`) and drags `node:child_process`, `oauth`, `model-cache`, and + `cursor/live-models` onto the request path. Repaired before implementation: the resolver + moved to `src/reasoning-effort.ts`, a leaf whose only import is `./types`. +2. **`allowInsecureHttp` is retired, not a live setting.** 060_phase6.md planned to document + it as a security-relevant opt-out. The source says it grants nothing and is parsed only so + an older config keeps loading. Documented as retired instead. + +And one implementation gap the local scope missed: + +3. **A stale assertion in the failover e2e suite.** The scoped local runs for #3172 covered + `combos.test.ts`, the catalog suite, and the boundary suite — not + `server-combo-failover-e2e.test.ts`, which held an assertion encoding the old + drop-on-miss behavior. CI on the merged `dev` head caught it within minutes and #3175 + corrected it. This is the cost of the no-local-suite policy, and it is a cheap one: the + trailing CI signal did exactly the job it was left to do. + +## Verification policy actually used + +No repository-wide local suite was run, per instruction. Each change was gated by focused +`bun test` files plus red-green proof that the new regression genuinely fails without the +fix, with CI trailing the train and judged at the end. + +## Final CI verdict + +Run `33533338305` on `dev` head `22a643a00b5974fa53b084a04491f60d56ec9ee2` — +**completed success**, zero failed jobs across the full matrix (Linux shards 1-4, macOS, +keyring, npm-global, gates, storage policy, api usage, hygiene). + +That head contains every landing in this train. The trailing-CI policy is therefore +discharged: nothing merged here leaves `dev` red.