From d374bb8931814cb202098bd5aaf1d261a6236684 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 22 Aug 2026 17:43:11 +0900 Subject: [PATCH] devlog: backlog disposition program roadmap (work-phase 0, docs-only) Opens devlog/_plan/260822_backlog_disposition_program/ as the planning unit for clearing the open PR/issue backlog by explicit per-item disposition. 000 objective, 45-PR inventory captured at unit open, disposition classes, and the dependency-ordered wp0-wp9 map 001 baseline verifier evidence actually run at unit open (tool-argument-integers 24 pass, tsc exit 0), remote host state, and repository authority 002 A-phase audit synthesis: round 1 returned FAIL with 7 blockers, all accepted with zero rebuttals, each re-verified against the tree before disposition 003 live drift at the A gate (45 -> 50 open PRs) and the disposition of competitor PR #2360, which fixes the same issue as wp3 010 wp1 green-and-ready merges, with per-PR verified change maps 020 wp2 changes-requested rebuilds, including the full 16-PR roster the audit found missing 030 wp3 #2316, re-scoped by the audit to a single file after the bare-name alias was shown unreachable behind the bridge authorization guard 040 wp4 #2292 Windows picker, with a bounded subprocess seam 050 wp5 #2221 native main token refresh, with external-writer CAS promoted into acceptance criteria 060 wp6 #1049, recorded as deferred: it needs a crash-safe publisher phase first 070 wp7 Bun 1.4 memory stack retarget, preserving the recorded FAIL verdicts 080 wp8 conflicting and remaining PR disposition Docs only: no production file is touched by this commit, and nothing in the build, typecheck, or test path reads from devlog/. privacy:scan passes. --- .../000_plan.md | 163 ++++++++++ .../001_baseline_verifier_evidence.md | 90 ++++++ .../002_audit_synthesis.md | 155 ++++++++++ .../003_live_drift_and_2360.md | 108 +++++++ .../010_wp1_green_ready_merges.md | 98 ++++++ .../020_wp2_changes_requested_rebuilds.md | 136 ++++++++ .../030_wp3_issue_2316_wait_agent_timeout.md | 243 +++++++++++++++ .../040_wp4_issue_2292_windows_picker.md | 290 ++++++++++++++++++ .../050_wp5_issue_2221_native_main_refresh.md | 208 +++++++++++++ ...6_issue_1049_presubstrate_home_adoption.md | 93 ++++++ .../070_wp7_bun14_memory_stack_retarget.md | 89 ++++++ .../080_wp8_conflicting_and_remaining.md | 63 ++++ 12 files changed, 1736 insertions(+) create mode 100644 devlog/_plan/260822_backlog_disposition_program/000_plan.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/001_baseline_verifier_evidence.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/002_audit_synthesis.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/003_live_drift_and_2360.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/010_wp1_green_ready_merges.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/020_wp2_changes_requested_rebuilds.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/030_wp3_issue_2316_wait_agent_timeout.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/040_wp4_issue_2292_windows_picker.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/050_wp5_issue_2221_native_main_refresh.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/060_wp6_issue_1049_presubstrate_home_adoption.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/070_wp7_bun14_memory_stack_retarget.md create mode 100644 devlog/_plan/260822_backlog_disposition_program/080_wp8_conflicting_and_remaining.md diff --git a/devlog/_plan/260822_backlog_disposition_program/000_plan.md b/devlog/_plan/260822_backlog_disposition_program/000_plan.md new file mode 100644 index 0000000000..a86146f0e9 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/000_plan.md @@ -0,0 +1,163 @@ +# 000 — Backlog disposition program: objective, inventory, and work-phase map + +Unit opened 2026-08-22 against `dev@ced9a85c5` (`origin/dev` identical at open). +Mode: HOTL goal loop, session `01a0287a-f569-7612-982a-f17c7c33d1fe`, +goalplan slug `clear-the-opencodex-open-pr-issue-backlog-by-dis`. + +## Objective + +Give every one of the 45 open pull requests and the 4 named PR-less priority issues an +explicit terminal disposition, and land everything accepted on `dev`. A disposition is +one of: + +| Code | Meaning | +|------|---------| +| **MERGE** | squash-merge the PR head as-is after verification | +| **REBUILD** | the intent is right but the diff is not landable; re-derive it on a fresh `codex/` branch | +| **REIMPLEMENT** | no usable PR exists (issue-only, or the PR is unsalvageable); write it from the spec | +| **CLOSE** | close with a recorded reason (wrong branch, superseded, duplicate, rejected-by-evidence) | + +## Scope boundary + +IN: the working tree, `codex/` branches, `origin/dev`, and GitHub PR/issue state. +OUT: `main` promotion, npm publication, releases and tags, credential/auth files, +security triage written into `devlog/` (scratch space only, per AGENTS.md). + +## Open pull request inventory (45, captured at unit open) + +| PR | State | Base | Head | Mergeable | Review | Size | Author | Title | +|----|-------|------|------|-----------|--------|------|--------|-------| +| #2359 | ready | `dev` | d587a4b4 | MERGEABLE | REVIEW_REQUIRED | 12+/0- (2f) | chilung-cgu | fix(catalog): exclude uncallable OpenCode Go and Zen models | +| #2357 | draft | `main` | 9a5115ad | MERGEABLE | REVIEW_REQUIRED | 81+/5- (3f) | mdwsk88 | [WRONG BRANCH] Add `__omit__` reasoning-effort wire sentinel for per-e | +| #2355 | ready | `dev` | 29e8d7b2 | MERGEABLE | REVIEW_REQUIRED | 427+/4- (20f) | harryzhou2000 | feat(status): warn when config.json diverges from the running proxy (C | +| #2352 | draft | `dev` | 7b07ba60 | MERGEABLE | REVIEW_REQUIRED | 860+/59- (8f) | luvs01 | fix(native): start owned lifecycle after ownership reprobe | +| #2351 | ready | `dev` | 916fc9f2 | MERGEABLE | REVIEW_REQUIRED | 845+/75- (22f) | harryzhou2000 | feat(config): audit persisted config mutations (source, fields, redact | +| #2350 | ready | `dev` | b1b5b071 | MERGEABLE | REVIEW_REQUIRED | 287+/4- (8f) | harryzhou2000 | feat(adapters): annotate present-but-empty tool outputs (DeepSeek defa | +| #2339 | ready | `dev` | e646ad6e | MERGEABLE | REVIEW_REQUIRED | 69+/10- (2f) | luvs01 | fix(google): preserve streaming thought-signature order | +| #2335 | ready | `dev` | 29acc673 | MERGEABLE | REVIEW_REQUIRED | 184+/29- (8f) | luvs01 | perf(tools): resolve tool-choice catalogs in linear time | +| #2326 | draft | `dev` | 794abac4 | MERGEABLE | REVIEW_REQUIRED | 398+/7- (14f) | JasonSujaya | feat(gui): add frontier model shortcuts | +| #2313 | ready | `dev` | befb4df5 | MERGEABLE | REVIEW_REQUIRED | 571+/11- (8f) | olddonkey | fix(responses): scope reasoning replay by conversation and remember pr | +| #2311 | ready | `dev` | 9fbfa19b | MERGEABLE | CHANGES_REQUESTED | 3348+/59- (37f) | goodwilliam0126 | fix(grok): translate native edit tools for Codex | +| #2310 | ready | `dev` | 1acf7343 | MERGEABLE | CHANGES_REQUESTED | 940+/59- (12f) | goodwilliam0126 | fix(responses): repair apply_patch envelopes | +| #2309 | ready | `dev` | 1d5d935b | MERGEABLE | REVIEW_REQUIRED | 63+/4- (4f) | Ingwannu | fix(kiro): accept Codex parallel tool permission | +| #2304 | ready | `codex/bun14-followup-memory-docs` | 9c7f42f8 | MERGEABLE | CHANGES_REQUESTED | 147+/0- (2f) | lidge-jun | test(scripts): smol-worker A/B gate harness (verdict: FAIL, flags not | +| #2303 | ready | `codex/bun14-mem-diagnostics` | 3ee558a2 | MERGEABLE | CHANGES_REQUESTED | 658+/0- (5f) | lidge-jun | feat(scripts): Bun.gc relief evaluation harness (SIGUSR2 GC channel, m | +| #2302 | ready | `codex/bun14-followup-memory-docs` | cac21afb | MERGEABLE | CHANGES_REQUESTED | 56+/5- (4f) | lidge-jun | feat(memory): expose JSC extraMemorySize in system memory API, watchdo | +| #2301 | ready | `dev` | 7a0fb255 | MERGEABLE | CHANGES_REQUESTED | 612+/0- (10f) | lidge-jun | devlog: Bun 1.4 follow-up memory roadmap (research + decade docs) | +| #2299 | draft | `dev` | 48326fc5 | MERGEABLE | CHANGES_REQUESTED | 860+/3- (9f) | abhisheksharma2411 | feat(catalog): operator display labels for live-discovered models | +| #2298 | draft | `dev` | 38888e3d | MERGEABLE | CHANGES_REQUESTED | 74+/0- (3f) | ppvia | fix(claude): warm empty Desktop-3P alias registry on first /v1/message | +| #2280 | ready | `dev` | b857561a | CONFLICTING | CHANGES_REQUESTED | 556+/15- (17f) | cristph | feat(catalog): allow per-model synthetic max suppression | +| #2257 | draft | `dev` | 510f1044 | MERGEABLE | CHANGES_REQUESTED | 1289+/29- (20f) | yansigit | feat(agent): named subagent role catalog | +| #2244 | draft | `dev` | 67acb331 | MERGEABLE | CHANGES_REQUESTED | 913+/0- (9f) | ZSN12 | feat(workbuddy): add experimental desktop OAuth provider | +| #2230 | draft | `dev` | 154fe3be | CONFLICTING | CHANGES_REQUESTED | 1637+/61- (33f) | ppvia | feat(oauth): add Gemini OAuth (Google account) accounts with Code Assi | +| #2222 | draft | `dev` | d54acacd | CONFLICTING | CHANGES_REQUESTED | 1390+/168- (15f) | MarcTCruz | fix(codex): refresh native main account tokens | +| #2215 | draft | `dev` | d85cf057 | MERGEABLE | CHANGES_REQUESTED | 126+/41- (8f) | parkjs101 | docs(sub-agents): describe v2 fork override rule as a prompt conventio | +| #2213 | draft | `dev` | a5afe351 | CONFLICTING | CHANGES_REQUESTED | 494+/101- (18f) | louis-tepe | feat: add Grok direct-first tool projection | +| #2123 | draft | `dev` | 701b51f9 | MERGEABLE | CHANGES_REQUESTED | 495+/32- (3f) | chilung-cgu | feat(quota): add per-account Gem/Cla quota probing for Google Antigrav | +| #2122 | draft | `dev` | fd6e53de | MERGEABLE | CHANGES_REQUESTED | 607+/41- (15f) | chilung-cgu | feat(catalog): config-level retainModels allowlist for authoritative d | +| #2113 | ready | `dev` | 3e17fe58 | MERGEABLE | CHANGES_REQUESTED | 2184+/112- (63f) | cb8010d6 | feat(providers): allow trusted encrypted V2 task passthrough | +| #2083 | draft | `dev` | 06c8d936 | MERGEABLE | APPROVED | 645+/57- (14f) | zhou-zhichao | feat(images): relay Codex image_gen to xAI Imagine with Grok OAuth | +| #2071 | draft | `dev` | e365907b | MERGEABLE | CHANGES_REQUESTED | 2789+/124- (25f) | yansigit | feat(antigravity): CCA host failover and non-retryable image POST | +| #2070 | draft | `dev` | f276d325 | MERGEABLE | CHANGES_REQUESTED | 1608+/76- (14f) | yansigit | feat(antigravity): Claude CCA wire fidelity | +| #2069 | draft | `dev` | e61e2b2e | CONFLICTING | CHANGES_REQUESTED | 1650+/41- (20f) | yansigit | feat(antigravity): process-local account cooldowns | +| #2068 | ready | `dev` | 9dceb40f | MERGEABLE | CHANGES_REQUESTED | 954+/31- (7f) | yansigit | feat(antigravity): live quota RPC and geoblock classification | +| #2050 | draft | `dev` | 52324cef | MERGEABLE | CHANGES_REQUESTED | 528+/72- (46f) | x3M3x | feat(combos): add random, least-used, and reset-window routing strateg | +| #2041 | draft | `dev` | e2460240 | CONFLICTING | REVIEW_REQUIRED | 26+/1- (3f) | yzxcj797 | feat(catalog): durable auto_review_model config override | +| #2033 | draft | `dev` | 6505a525 | MERGEABLE | REVIEW_REQUIRED | 14+/0- (2f) | louis-tepe | Expose web search sidecar enabled status | +| #1905 | ready | `dev` | 0be75f29 | MERGEABLE | CHANGES_REQUESTED | 781+/80- (27f) | luvs01 | feat(codex): add per-model ChatGPT compaction budgets | +| #1829 | ready | `dev` | bf5e67f9 | MERGEABLE | CHANGES_REQUESTED | 2878+/2- (4f) | luvs01 | feat(codex): add durable reset-credit operation ledger | +| #1794 | draft | `dev` | 179a6a31 | CONFLICTING | REVIEW_REQUIRED | 1759+/8- (50f) | riique | feat: recover routed V2 subagents and select OpenRouter endpoints | +| #1769 | draft | `dev` | f87c4acb | MERGEABLE | CHANGES_REQUESTED | 963+/36- (19f) | dbc-hbin | feat(gui): add manual paste fallback for OAuth add-account | +| #1756 | ready | `dev` | e9a04d1e | MERGEABLE | CHANGES_REQUESTED | 850+/116- (17f) | takltc | feat(grok): inject per-model reasoning effort into Grok Build config | +| #1704 | draft | `dev` | c8c4358a | CONFLICTING | REVIEW_REQUIRED | 181+/7- (16f) | lidge-jun | feat(gui): surface per-target quota state in combo workspace (#1702) | +| #1645 | draft | `dev` | 2a760080 | CONFLICTING | CHANGES_REQUESTED | 1425+/151- (68f) | waw4303 | feat(vision): add chat and Google sidecars | +| #1557 | draft | `dev` | 5586e4e0 | CONFLICTING | REVIEW_REQUIRED | 2545+/69- (28f) | LeoWang331 | feat(server): add least-privilege data-plane catalog endpoint | + +## Disposition classes derived from the inventory + +| Class | Count | PRs | +|-------|-------|-----| +| Ready, no changes requested, base `dev` | 8 | #2359 #2355 #2351 #2350 #2339 #2335 #2313 #2309 | +| Changes requested, mergeable | 19 | #2311 #2310 #2301 #2299 #2298 #2257 #2244 #2215 #2123 #2122 #2113 #2071 #2070 #2068 #2050 #1905 #1829 #1769 #1756 | +| Conflicting with `dev` | 10 | #2280 #2230 #2222 #2213 #2069 #2041 #1794 #1704 #1645 #1557 | +| Draft, other | 4 | #2352 #2326 #2083 #2033 | +| Wrong base | 4 | #2357 (`main`) #2304 #2303 #2302 (stack-internal bases) | + +## PR-less priority issues + +| Issue | Priority | Disposition | Decade doc | +|-------|----------|-------------|------------| +| #2316 Grok `wait_agent` `timeout_ms` rejected as `120000.0` | 73 | REIMPLEMENT | `030` | +| #2292 Windows model picker stale after `ocx sync --restart-codex` | 72 | REIMPLEMENT | `040` | +| #2221 native main pool does not refresh expired `auth.json` tokens | 70 | REIMPLEMENT (disposes #2222) | `050` | +| #1049 adopt pre-substrate Codex homes into the write coordinator | 73 | DEFERRED — see `060` | `060` | + +## Work-phase map (dependency-ordered, PHASE-SPLIT-01) + +Ordering is by build dependency, not by effort. The shared-type surface +(`src/types/tools.ts`, `src/server/responses/collaboration.ts`) is touched by both WP1 +(#2335) and WP3 (#2316), so WP1 lands first and WP3 re-verifies its pre-written doc +against the landed tree at its own P. The auth surface (WP5) sits after the catalog and +tool-plumbing phases because a token-refresh regression is only diagnosable on a tree +whose routing layer is already settled. + +| WP | Title | Decade doc | Depends on | +|----|-------|-----------|------------| +| wp0 | Docs-only inventory + roadmap (this cycle) | `000` | — | +| wp1 | Green-and-ready merges | `010` | wp0 | +| wp2 | Changes-requested rebuilds | `020` | wp1 | +| wp3 | #2316 wait_agent timeout_ms | `030` | wp1 (shares tool-choice surface) | +| wp4 | #2292 Windows picker | `040` | wp1 | +| wp5 | #2221 native main token refresh | `050` | wp1 | +| wp6 | #1049 pre-substrate home adoption | `060` | wp5 (auth/coordinator surface) | +| wp7 | Bun 1.4 memory stack retarget | `070` | wp1 | +| wp8 | Conflicting + remaining PR disposition | `080` | wp1–wp7 | + +## Verifier reality check (PLAN-VERIFIER-REAL-01) + +Commands named by the decade docs were run at unit open: + +| Command | Exit | Reads the change target? | +|---------|------|--------------------------| +| `bun test tests/tool-argument-integers.test.ts` | see `001` | yes — imports `src/lib/tool-argument-integers.ts` and `src/bridge.ts` | +| `bun x tsc --noEmit` | see `001` | yes — `tsconfig` includes `src/` | +| `bun test ` per WP | recorded per phase | verified per decade doc | + +Full-suite runs may execute on remote host `lidge` (`~/Developer/opencodex`, bun 1.3.14, +16 cores) when a local run would block the loop; the merge evidence records which host +produced the output. + + + +--- + +# AMENDMENT (A-phase round 1) — corrections to this document + +**Verifier reality table (B5).** The table above overclaimed. Corrected findings, all +re-run against the tree: + +| Command | Real result | Reads target? | +|---------|-------------|---------------| +| `bun test tests/tool-argument-integers.test.ts` | 24 pass / 0 fail, exit 0 | yes | +| `bun x tsc --noEmit` | exit 0 | yes | +| `bun test tests/dispatch-sync.test.ts` | **file does not exist**; real file is `tests/cli-dispatch.test.ts` | n/a | +| `bun test ` | **exits 0 while silently skipping the missing file** | dangerous | + +Every phase's C step therefore prepends `test -f ` for each required new suite and +runs each mandatory regression as its own invocation. A combined invocation is not +accepted as evidence. + +**Work-phase ordering rationale (auditor additional finding).** This document justified +running WP1 before WP3 as a shared-surface dependency on `src/types/tools.ts` / +`src/server/responses/collaboration.ts`. That is factually wrong and is withdrawn: +#2335 touches `src/adapters/anthropic.ts`, `src/adapters/command-code.ts`, +`src/adapters/google.ts`, `src/responses/parser.ts`, `src/types.ts`, +`src/types/tools.ts` — not `collaboration.ts` — and WP3 after its own amendment touches +only `src/lib/tool-argument-integers.ts`. **There is no file overlap.** WP1 still runs +first, for the plain reason that landing already-verified green work before opening new +work keeps the baseline clean and every later phase's evidence interpretable. + +**Disposition coverage (B3).** The 16 changes-requested PRs that had no lane +(#2299 #2298 #2257 #2244 #2215 #2123 #2122 #2113 #2071 #2070 #2068 #2050 #1905 #1829 +#1769 #1756) are dispositioned in `020`. + diff --git a/devlog/_plan/260822_backlog_disposition_program/001_baseline_verifier_evidence.md b/devlog/_plan/260822_backlog_disposition_program/001_baseline_verifier_evidence.md new file mode 100644 index 0000000000..58f34dc448 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/001_baseline_verifier_evidence.md @@ -0,0 +1,90 @@ +# 001 — Baseline verifier evidence at unit open + +Captured 2026-08-22 on `dev@ced9a85c5`, macOS darwin/arm64, before any code change in +this unit. These are the commands the decade docs name; PLAN-VERIFIER-REAL-01 requires +they be RUN, not merely cited. + +## `bun test tests/tool-argument-integers.test.ts` + +``` + 24 pass + 0 fail + 32 expect() calls +Ran 24 tests across 1 file. [96.00ms] +EXIT=0 +``` + +Reads the change target: **yes**. The file imports `src/lib/tool-argument-integers.ts` +and `src/bridge.ts` directly, which are exactly the modules WP3 modifies. + +Baseline behaviours this suite already pins, which WP3 must not break: + +- `never touches number-typed fields` — `'{"temperature":1.0}'` must return byte-identical. +- `leaves fields with no declared schema exactly as received`. +- `#1611 wiring: bridge emits repaired arguments`. + +## `bun x tsc --noEmit` + +``` +EXIT=0 +``` + +Completed in ~0.56s wall (521% CPU, warm). Reads the change target: **yes**, the +`tsconfig` include covers `src/`. + +## Remote suite host + +`ssh lidge` resolves to `lidge-AI-AI`, Linux x86_64, 16 cores, 30 GB RAM, bun 1.3.14 at +`/usr/local/bin/bun`, repository at `~/Developer/opencodex`. At probe time its checkout +was behind `origin/dev` (head `c378435022`); any full-suite run there must fetch and +check out the exact head under test before its output counts as evidence. + +## Repository authority confirmed at open + +``` +gh api repos/lidge-jun/opencodex --jq .permissions +{"admin":true,"maintain":true,"pull":true,"push":true,"triage":true} + +gh api repos/lidge-jun/opencodex/branches/dev/protection +404 Branch not protected +``` + +`dev` carries no branch protection, so merges are gated by this unit's verification +discipline rather than by GitHub. That makes the per-phase C evidence the only real +gate — treat it accordingly. + +## Cross-check: #2320 and `Closes #2316` + +The #2316 triage comment instructed that PR #2320 must not carry `Closes #2316`. +Verified at unit open: **#2320 is MERGED and its body still contains `Closes #2316`**, +yet **issue #2316 is still OPEN**. GitHub auto-closes a linked issue only when the PR +merges into the default branch (`main`); #2320 targeted `dev`, so the link never fired. +The risk is therefore latent, not realised: it would close #2316 spuriously at the next +`dev`→`main` promotion if the issue is still open then. WP3 closes #2316 on its own +merits well before that, which resolves the hazard without editing merged history. + + + +--- + +# AMENDMENT (A-phase round 1, blocker B7) — the auto-close hazard claim is WITHDRAWN + +The section above claimed the merged #2320 "would close #2316 spuriously at the next +`dev`→`main` promotion". That is wrong and is withdrawn. + +Re-verified: the squash commit that landed on `dev` is + +``` +fix(cursor): classify bare 0-token resource_exhausted as context overflow (#2320) +``` + +It contains **no closing keyword**. GitHub ignores closing keywords in a pull request +body when the PR targets a non-default branch: no link is created at all, so there is +nothing for a later promotion to trigger. Promoting an already-created commit cannot +resurrect an ignored keyword. + +The **state facts remain verified and stand**: #2320 is MERGED, its body still contains +`Closes #2316`, and #2316 is OPEN. Only the inferred future hazard was false. A real +hazard would require a *promotion commit that itself carries a closing keyword*, which is +a separate condition and is not present. + diff --git a/devlog/_plan/260822_backlog_disposition_program/002_audit_synthesis.md b/devlog/_plan/260822_backlog_disposition_program/002_audit_synthesis.md new file mode 100644 index 0000000000..9d58e7d61e --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/002_audit_synthesis.md @@ -0,0 +1,155 @@ +# 002 — A-phase audit synthesis (REVIEW-SYNTHESIS-01) + +Round 1 auditor verdict: **FAIL, blockers=7**. Every blocker was independently +re-verified by the main agent against the real tree before disposition. **All seven are +ACCEPTED**; none is rebutted. This document records the root cause of each and the +amendment it forces, per REVIEW-SYNTHESIS-01. + +## B1 (High) — WP3's bare-name alias is unreachable. ACCEPTED, and it removes a change. + +Auditor anchor: `src/bridge.ts:1041` and `src/bridge.ts:1788`. Re-verified verbatim: + +```ts +if (options?.declaredToolNames && !options.declaredToolNames.has(event.name)) { + const failure = responseError(502, "upstream_error", + `routed provider emitted undeclared client tool "${event.name}"; only request-declared tools may be called`); +``` + +The authorization guard runs **before** `closeCurrentToolCall()` reaches +`coerceIntegerToolArguments`. Registering a schema under a bare key while leaving +`declaredToolNames` untouched therefore repairs arguments for a call that was already +rejected with a 502. The proposed wiring test injected `toolParameterSchemas` by hand and +bypassed the guard entirely — false confidence, exactly as the auditor said. + +**Independent finding that settles it.** The issue body for #2316 reports the failing +call as `multi_agent_v1__wait_agent` — the **namespaced** name: + +> On current `dev` (`a228ed741`), Grok-routed Codex App still rejects +> `multi_agent_v1__wait_agent` before the tool runs: +> `failed to parse function arguments: invalid type: floating point \`120000.0\`, expected u64` + +Two things follow. First, the error is raised by **Codex's own deserializer**, which +proves the call *passed* our bridge and reached Codex — so the schema lookup **hit**. +Second, the name is namespaced, so no bare-name miss ever occurred in the report. + +**Defect B does not exist in the reported bug.** WP3 is amended to a single-file change: +`src/lib/tool-argument-integers.ts` only. The `src/server/responses/collaboration.ts` +edit is **struck** from the plan. This makes WP3 smaller and removes the security-adjacent +surface entirely. + +## B2 (High) — alias collision. ACCEPTED, dissolved by B1. + +Auditor anchor: `src/server/responses/collaboration.ts:154`, which requires +`bareNameCounts.get(t.name) !== 1` before admitting a bare alias. The proposal's +"first declaration wins" bypassed that uniqueness policy. Since B1 strikes the alias +change entirely, the collision cannot occur. Recorded because the reasoning must survive: +**if a future unit wants bare-name repair, it must go through `declaredToolNames`, +`toolNsMap`, and `toolParameterSchemas` atomically under the existing uniqueness rule** — +never through the schema map alone. + +## B3 (High) — 16 changes-requested PRs had no disposition. ACCEPTED. + +The inventory counted 19 in that class; WP2 named 2 and WP7 named 1. The remaining 16 +appeared only as inventory rows. A generic acceptance criterion is not a disposition. +Amendment: the table below enters `020` as its dispositioned roster. + +| PR | State | Head | Size | Author | Title | +|----|-------|------|------|--------|-------| +| #2299 | draft | 48326fc5 | 860+/3- (9f) | abhisheksharma2411 | feat(catalog): operator display labels for live-discovered m | +| #2298 | draft | 38888e3d | 74+/0- (3f) | ppvia | fix(claude): warm empty Desktop-3P alias registry on first / | +| #2257 | draft | 510f1044 | 1289+/29- (20f) | yansigit | feat(agent): named subagent role catalog | +| #2244 | draft | 67acb331 | 913+/0- (9f) | ZSN12 | feat(workbuddy): add experimental desktop OAuth provider | +| #2215 | draft | d85cf057 | 126+/41- (8f) | parkjs101 | docs(sub-agents): describe v2 fork override rule as a prompt | +| #2123 | draft | 701b51f9 | 495+/32- (3f) | chilung-cgu | feat(quota): add per-account Gem/Cla quota probing for Googl | +| #2122 | draft | fd6e53de | 607+/41- (15f) | chilung-cgu | feat(catalog): config-level retainModels allowlist for autho | +| #2113 | ready | 3e17fe58 | 2184+/112- (63f) | cb8010d6 | feat(providers): allow trusted encrypted V2 task passthrough | +| #2071 | draft | e365907b | 2789+/124- (25f) | yansigit | feat(antigravity): CCA host failover and non-retryable image | +| #2070 | draft | f276d325 | 1608+/76- (14f) | yansigit | feat(antigravity): Claude CCA wire fidelity | +| #2068 | ready | 9dceb40f | 954+/31- (7f) | yansigit | feat(antigravity): live quota RPC and geoblock classificatio | +| #2050 | draft | 52324cef | 528+/72- (46f) | x3M3x | feat(combos): add random, least-used, and reset-window routi | +| #1905 | ready | 0be75f29 | 781+/80- (27f) | luvs01 | feat(codex): add per-model ChatGPT compaction budgets | +| #1829 | ready | bf5e67f9 | 2878+/2- (4f) | luvs01 | feat(codex): add durable reset-credit operation ledger | +| #1769 | draft | f87c4acb | 963+/36- (19f) | dbc-hbin | feat(gui): add manual paste fallback for OAuth add-account | +| #1756 | ready | e9a04d1e | 850+/116- (17f) | takltc | feat(grok): inject per-model reasoning effort into Grok Buil | + +## B4 (High) — WP5 knowingly permits an external-writer credential clobber. ACCEPTED. + +The research lane wrote: *"A true multi-writer CAS ... was demanded by the owner review; +the lock covers same-machine ocx processes but not Codex CLI writers that ignore our +lock. Flag this residual risk in the PR description."* A PR-description note is not a +mitigation for a credential-clobber race on a file another product writes. + +Amendment: external-writer CAS moves **into WP5 acceptance criteria** — capture file +identity (dev/ino/mtime/size) plus content hash at read, re-compare immediately before +publication, and retry/adopt/refuse on change; with a regression that mutates +`auth.json` between refresh and publish and proves the newer writer survives. WP5 also +remains gated on exact-head maintainer security review per AGENTS.md. + +## B5 (High) — the verifier commands are false-green. ACCEPTED; my `001` claim was wrong. + +Re-verified directly: + +``` +$ ls tests/dispatch-sync.test.ts +ls: tests/dispatch-sync.test.ts: No such file or directory + +$ bun test tests/codex-main-account-refresh.test.ts tests/codex-account-store.test.ts + 26 pass / 0 fail RC=0 +``` + +The second command names a file that does not exist and **still exits 0**, because Bun +silently ignores missing paths when at least one listed file exists. A verifier that +passes while its mandatory regression is absent is worse than no verifier. + +Amendments: (a) WP4's placeholder resolves to the real file — `tests/cli-dispatch.test.ts` +exists, `tests/dispatch-sync.test.ts` does not; (b) every phase's C step prepends an +existence gate `test -f ` for each required new suite, and runs each mandatory +regression as its own invocation so a missing target fails the phase; (c) the +PLAN-VERIFIER-REAL-01 table in `000` is corrected — it currently overclaims. + +## B6 (Medium) — WP4's `execFile` seam cannot carry its own timeout. ACCEPTED. + +`execFile?: (file: string, args: readonly string[]) => Promise<{ stdout: string }>` has +no options parameter, yet the same document mandates `timeout: 10_000` and +`windowsHide: true`. A hung `Get-AppxPackage` probe would wedge `ocx sync` or +`ocx doctor`. Amendment: the seam gains a typed options parameter (`timeout`, +`windowsHide`, abort), with a test proving timeout rejection and process cleanup. + +## B7 (Medium) — my auto-close hazard claim was factually wrong. ACCEPTED. + +`001` claimed the merged #2320 "would close #2316 spuriously at the next dev→main +promotion". Re-verified: the squash commit message is + +``` +fix(cursor): classify bare 0-token resource_exhausted as context overflow (#2320) +``` + +— it contains **no** closing keyword. GitHub ignores closing keywords in a PR body when +the PR targets a non-default branch; no link is ever created, and promoting the existing +commit cannot resurrect an ignored keyword. The state facts stay (#2320 MERGED, body +still says `Closes #2316`, #2316 OPEN); the hazard claim is **withdrawn**. + +## Correction carried from the auditor's additional findings + +`000` justified ordering WP1 before WP3 as a shared-file dependency. Verified false: +#2335 touches `src/adapters/anthropic.ts`, `command-code.ts`, `google.ts`, +`src/responses/parser.ts`, `src/types.ts`, `src/types/tools.ts` — **not** +`collaboration.ts`; and post-B1 WP3 touches only +`src/lib/tool-argument-integers.ts`. There is **no file overlap**. WP1 still runs first +because merging verified-green work before opening new work keeps the baseline clean, but +the stated rationale is corrected to that, rather than a dependency that does not exist. + +## Disposition summary + +| Blocker | Severity | Disposition | Effect on plan | +|---------|----------|-------------|----------------| +| B1 | High | ACCEPTED | WP3 loses the `collaboration.ts` change entirely | +| B2 | High | ACCEPTED | Dissolved by B1; policy recorded for future units | +| B3 | High | ACCEPTED | 16 PRs dispositioned in `020` | +| B4 | High | ACCEPTED | External-writer CAS becomes WP5 acceptance, not a note | +| B5 | High | ACCEPTED | Existence-gated, one-file-per-invocation verifiers | +| B6 | Medium | ACCEPTED | `execFile` seam gains options | +| B7 | Medium | ACCEPTED | False hazard claim withdrawn from `001` | + +Zero rebuttals. Round 2 audit follows on the amended documents. + diff --git a/devlog/_plan/260822_backlog_disposition_program/003_live_drift_and_2360.md b/devlog/_plan/260822_backlog_disposition_program/003_live_drift_and_2360.md new file mode 100644 index 0000000000..10077bc05e --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/003_live_drift_and_2360.md @@ -0,0 +1,108 @@ +# 003 — Live drift at the A gate, and the disposition of competitor PR #2360 + +Round-2 audit returned **VERDICT: PASS** on the amended documents, with one non-blocking +observation: the backlog moved while work-phase 0 was running. This document records the +drift and disposes it, so the roadmap locks against reality rather than against a +snapshot. + +## Drift measured at the A gate + +Open PRs: **45 at unit open → 50 now**. Zero PRs left the open set, so the `000` +inventory is still valid as far as it goes; five arrived, all from one contributor +(`chilung-cgu`) within a twelve-minute window. + +| PR | Base | Head | Size | Created | Title | +|----|------|------|------|---------|-------| +| #2360 | `dev` | `c4e38608` | 137+/12- (4f) | 08-22 08:27 | fix(tools): repair integral float arguments for native integer fields | +| #2361 | `dev` | `b4c0b949` | 70+/2- (2f) | 08-22 08:32 | fix(reasoning): support per-effort field omission sentinel (`__omit__`) | +| #2362 | `dev` | `64e7e62c` | 141+/3- (3f) | 08-22 08:34 | feat(providers): Responses terminal repair escape hatch for custom providers | +| #2363 | `dev` | `299d87f9` | 61+/0- (3f) | 08-22 08:36 | feat(catalog): apply configured auto_review_model override during sync | +| #2364 | `dev` | `33322b41` | 292+/1- (7f) | 08-22 08:39 | feat(providers): model-specific routing for Vercel AI Gateway (draft) | + +Head drift on four already-inventoried PRs: #2351 `916fc9f2→829997d3`, +#2339 `e646ad6e→4fb942d4`, #2311 `9fbfa19b→b7b5c5f1`, #2310 `1acf7343→93b977d3`. +Every phase re-reads its PR's head at its own P; the `000` table is explicitly +"captured at unit open" and is not treated as current at merge time. + +## #2360 competes directly with WP3 + +#2360 fixes issue #2316 — the same issue WP3 exists to fix — and its commit carries +`closes #2316`. It cannot be ignored: two fixes for one issue would either conflict or +double-land. + +**Where it agrees with WP3.** Its core is the same shape the amended `030` specifies: +thread the property key into `coerceValue`, and treat a known Codex-native integer field +as integer-declared even when the schema says `number`. That part is correct, and it is +the whole of Defect A. + +**Where it carries the defects this unit already identified.** + +1. **It re-introduces the `collaboration.ts` alias that `002`/B2 rejected**, and does so + *without* the uniqueness guard: + + ```ts + for (const alias of toolChoiceAliases(t)) { + toolParameterSchemas.set(alias, t.parameters); + } + if (!toolParameterSchemas.has(t.name)) toolParameterSchemas.set(t.name, t.parameters); + ``` + + The existing bare-alias path at `src/server/responses/collaboration.ts:154` deliberately + refuses when `bareNameCounts.get(t.name) !== 1`. This loop bypasses that policy, so with + two namespaced tools sharing a logical name, one tool's schema can be used to repair the + other's arguments. + +2. **`lookupToolParameterSchema` resolves ambiguity by iteration order.** Its fallback + + ```ts + for (const [key, schema] of toolParameterSchemas.entries()) { + if (key.endsWith(`__${toolName}`)) return schema; + } + ``` + + returns the **first** map entry whose key ends with the bare name. With + `a__wait_agent` and `b__wait_agent` both present, which schema wins depends on + insertion order, not on a rule. + +3. **A much broader allowlist:** `timeout_ms`, `yield_time_ms`, `max_tokens`, + `max_output_tokens`, `session_id`, `line`, `start`, `end`, `priority`, `port`. + `030` deliberately scoped WP3 to `timeout_ms` because that is the field the issue + reports and the only one proven against a Rust `u64`. Names like `start`, `end`, + `line`, and `priority` are generic enough to collide with a third-party tool that + legitimately takes a fractional value, and the PR carries no evidence for them. + +4. **It repairs with no schema at all** (`coerceIntegerToolArguments` now proceeds when + `parameters === undefined`). Combined with the broad allowlist, an unknown provider's + `priority: 1.0` would be silently rewritten. The existing guard test survives only + because `undeclared` is not an allowlisted name. + +## Disposition + +**#2360 → REBUILD (absorb core, remove the rest).** Not closed: the contributor found the +same root cause and their key-threading core is right. Not merged as-is: it re-introduces +a collision policy bypass this unit already analysed and rejected, plus an order-dependent +resolver and an unevidenced allowlist. + +WP3's terminal action becomes: land the minimal, evidence-backed fix, credit #2360, and +close it as superseded with these specific reasons recorded — or, if the contributor +prefers, leave it open with the three defects restated. The `closes #2316` keyword on +#2360 stops being a hazard the moment WP3 closes #2316 on its own merits. + +## New work-phase (LOOP-UNIT-CHAIN-01) + +The four remaining new PRs (#2361, #2362, #2363, #2364) are independent of every existing +lane, so they become a new appended work-phase **wp9**, not an excuse to close the goal. +Note #2361 supersedes the wrong-base #2357 (both address #2356), and #2363 addresses +#1225, which is still OPEN — so #2041 (the conflicting `auto_review_model` PR in wp8) +now has a mergeable competitor and must be disposed against it. + +## Reconciliation, restated + +``` +50 open = 45 at unit open + 5 arrived during wp0 + = wp1(5) + wp2(18) + wp3-disposes(#2360) + wp7(4) + wp8(15) + wp9(4) + ... reconciled mechanically at wp8/wp9 C against a LIVE gh pr list +``` + +The final reconciliation runs against a live query, never against this table. + diff --git a/devlog/_plan/260822_backlog_disposition_program/010_wp1_green_ready_merges.md b/devlog/_plan/260822_backlog_disposition_program/010_wp1_green_ready_merges.md new file mode 100644 index 0000000000..439f81a485 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/010_wp1_green_ready_merges.md @@ -0,0 +1,98 @@ +# 010 — WP1: green-and-ready merges + +Five PRs whose base is `dev`, whose CI is green, and against which no reviewer has +requested changes. Each is small enough to verify individually. + +## Merge order (dependency-ordered) + +``` +#2359 catalog exclusions (no shared surface) +#2309 kiro parallel permission (adapter-local + docs) +#2339 google signature order (adapter-local) +#2335 tool-choice linear time (SHARED: src/types/tools.ts) <- last of the small set +#2313 reasoning replay scoping (SHARED: responses core) <- largest, own verification +``` + +#2335 and #2313 touch shared surfaces, so they land after the adapter-local trio and +each gets its own full verification. WP3 (#2316) re-verifies its pre-written doc against +the tree **after** #2335 lands, because both edit `src/types/tools.ts` / +`src/server/responses/collaboration.ts`. + +## Per-PR file change map (verified against the PR heads) + +### #2359 — `fix(catalog): exclude uncallable OpenCode Go and Zen models` +Head `d587a4b4`. 12+/0-, 2 files. Closes #2330. + +- MODIFY `src/codex/catalog/parsing.ts` — four slugs appended to + `ROUTED_MODEL_COMPATIBILITY_EXCLUSIONS`: `opencode-go/grok-4.6`, + `opencode-go/mimo-v2-omni`, `opencode-go/mimo-v2-pro`, + `opencode-free/deepseek-v4-flash-free`. `hy3-preview` retained. +- MODIFY `tests/codex-catalog.test.ts` — four `shouldExposeRoutedModel(...) === false` + assertions plus a positive control (`opencode-go/glm-5.2` stays exposed). + +Matches the maintainer triage exactly: exclusion-set edit only, augmentation contract +(Ox Alpha, `deepseek-v4-flash-vision-exp`) untouched. +Verifier: `bun test tests/codex-catalog.test.ts`. + +### #2309 — `fix(kiro): accept Codex parallel tool permission` +Head `1d5d935b`. 63+/4-, 4 files. Addresses #2308 (priority 72). + +Treats client `parallel_tool_calls: true` as permission, not a wire requirement: Kiro +stays serialized and advertises no parallel capability, but no longer rejects the turn. +Includes the docs-site adapter reference update. +Verifier: the kiro adapter test file. + +### #2339 — `fix(google): preserve streaming thought-signature order` +Head `e646ad6e`. 69+/10-, 2 files. + +- MODIFY `src/adapters/google.ts:964` — `observeAntigravityReplay(...)` return value no + longer feeds `pendingStreamThoughtSig`; observation may scan the whole frame, so it is + used for replay-cache side effects only and the source-order loop keeps sole ownership + of stream carry (it cannot pair backwards). +- MODIFY `tests/google-signature-history-roundtrip.test.ts` — extracts an `sseResponse` + helper, adds an AI Studio provider fixture, and adds two tests: signatures attach only + to function calls that FOLLOW them in the same frame, and cross-frame carry survives. +Verifier: `bun test tests/google-signature-history-roundtrip.test.ts`. + +### #2335 — `perf(tools): resolve tool-choice catalogs in linear time` +Head `29acc673`. 184+/29-, 8 files. **Shared surface.** + +- MODIFY `src/types/tools.ts` — new `createToolChoiceResolver(tools)` compiling one + immutable catalog view (`candidatesByName`, `sourceCandidatesByName`, + `identitiesByTool` WeakMap); `toolAllowedByChoiceFromIndex` extracted; + `toolChoiceCandidates` / `toolAllowedByChoice` / `toolChoiceToolPredicate` re-expressed + over it. Fails closed when catalog objects mutate after compile. +- MODIFY `src/adapters/anthropic.ts`, `src/adapters/command-code.ts`, + `src/adapters/google.ts` — replace hand-rolled `Set` + `toolAllowedByChoice` filters + with `toolChoiceToolPredicate` / the resolver. +- MODIFY `src/responses/parser.ts` — ambiguity check uses `resolver.candidateCount`. +- MODIFY `src/types.ts` — re-export `createToolChoiceResolver`. +- NEW `tests/tool-choice-performance.test.ts` — proves no quadratic candidate replay, + that public lookups rebuild after a mutable caller mutates its catalog, and that a + compiled resolver fails closed when its catalog objects change. +- MODIFY `tests/types-barrel-identity.test.ts` — barrel identity for the new export. +Verifier: `bun test tests/tool-choice-performance.test.ts tests/types-barrel-identity.test.ts` +plus the three adapter suites. + +### #2313 — `fix(responses): scope reasoning replay by conversation and remember proven blob rejections` +Head `befb4df5`. 571+/11-, 8 files. **Shared surface, largest of the set.** + +Two prior approvals from `Ingwannu` (2026-08-21 19:48 and 21:11) both predate the +current head, which was pushed 2026-08-22 03:55. Under the repository's review-readiness +contract a new push resets completion, so the stale approvals are **not** carried into +this merge as evidence. This unit verifies the head itself and records that verification +as the merge basis. + +## Accept criteria + +1. Each PR merges into `dev` with a squash commit whose message names the PR number. +2. `bun x tsc --noEmit` exits 0 after each merge. +3. The focused suite for each PR exits 0 against the merged tree. +4. Every merge commit is reachable from `origin/dev` by SHA. +5. #2330 closes when #2359 lands; #2308 is updated when #2309 lands. + +## Out of scope + +Rebasing any of these branches; touching the changes-requested set (WP2); any `main` +movement. + diff --git a/devlog/_plan/260822_backlog_disposition_program/020_wp2_changes_requested_rebuilds.md b/devlog/_plan/260822_backlog_disposition_program/020_wp2_changes_requested_rebuilds.md new file mode 100644 index 0000000000..f218f84d7b --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/020_wp2_changes_requested_rebuilds.md @@ -0,0 +1,136 @@ +# 020 — WP2: changes-requested rebuilds + +PRs whose intent is accepted but which carry unresolved reviewer blockers. Each is +either fixed forward on a `codex/` branch or closed with a recorded reason. None is +merged on the strength of author self-attestation alone. + +## Inventory and recorded blockers + +### #2310 — `fix(responses): repair apply_patch envelopes` +Head `1acf7343`, 940+/59-, 12 files. Review state CHANGES_REQUESTED. + +Blocker history (from the review threads): +- CodeRabbit, `src/responses/custom-tool-compat.ts:222`, Major: `custom_tool_call.input` + bypassed repair in JSON/SSE. **Author fixed in `134ec8b13`** — a separate exact + wire-name authorization set for native passthrough custom tools; CodeRabbit + acknowledged the fix resolves the reported bypass. +- CodeRabbit, `tests/responses-custom-tool-repair.test.ts:163`, Minor: missing regression + where `repairNames` contains `apply_patch` but a `custom_tool_call` has no `name`; + that payload must stay byte-identical. + +Disposition: **REBUILD-LITE** — add the one missing regression, re-verify, merge. The +Major blocker is already closed; the residual is a single test. + +### #2311 — `fix(grok): translate native edit tools for Codex` +Head `9fbfa19b`, **3348+/59-, 37 files**. Review state CHANGES_REQUESTED. + +Open blockers: +- `src/adapters/grok-structured-edit.ts:206` Minor — `grokShellNeedsGitEscalation` misses + `git -C add`: the flag group `(?:\s+-[^\s]+)*` consumes `-C` but then requires a + subcommand where `/repo` sits, so escalation never fires and Codex fails to write + `.git/index.lock`. Costs one wasted turn (the tool description tells the model to + retry escalated) rather than breaking the operation. +- `src/adapters/grok-structured-edit.ts:260` Major — three near-duplicate helper pairs + redefine the same activation predicate (`isCodexCodeModeExecTool`, + `isBareShellBridgeTool`, `CODEX_SHELL_BRIDGE_TOOL_NAMES` duplicating + `src/adapters/tool-catalog-nudge.ts:44-66`; `grokEditCodexSink` and + `grokCodeModeExecSink` sharing nine of eleven lines). This predicate is the activation + gate for the whole bridge — drift silently changes which turns convert. +- `src/adapters/grok-structured-edit.ts:1020` Major — greedy `('.+')` in the Windows grep + reconstruction backtracks to the LAST quote in the emitted script, capturing the + trailing `'{0}:{1}:{2}'` formatter and corrupting the restored `pattern`. + +Disposition: at 3.3k lines across 37 files with three open correctness blockers on the +default Grok path, this does not merge in this unit on author attestation. Either the +blockers are closed and independently re-verified, or the PR is left open with the +blockers restated. **It is not closed** — the intent is sound and the work is +substantial. + +### #2350 — `feat(adapters): annotate present-but-empty tool outputs (DeepSeek default)` +Head `b1b5b071`, 287+/4-, 8 files. `review-ready` label; unresolved CodeRabbit checkboxes. +CI: all five checks SUCCESS on re-query (the aggregate rollup reported a stale `label` +failure — see 002). + +### #2351 — `feat(config): audit persisted config mutations` +Head `916fc9f2`, 845+/75-, 22 files. + +Recorded blocker: at line 3272 a new save replaces the only pending marker even when +lines 2771-2773 replayed an older marker in the current uncommitted transaction. Crash +sequence: `config.json` at `C1` with marker `P1`; next save inserts the `P1` audit row, +overwrites `P1` with `P2`, then writes `C2`. If the `C2` write fails, the transaction +rolls back the `C1` audit row and the surviving `P2` hash does not match `C1`, so later +reconciliation drops it. This is a durability defect in an audit feature — the exact +class of defect the feature exists to prevent. Must be closed before merge. + +### #2355 — `feat(status): warn when config.json diverges from the running proxy` +Head `29e8d7b2`, 427+/4-, 20 files. CI green on re-query. + +## Accept criteria + +Every PR in this phase ends in one of: merged with its blockers verifiably closed; +left open with the blockers restated in a review comment; or closed with a reason. No +PR in this phase merges while a Major correctness blocker is open. + + + +--- + +# AMENDMENT (A-phase round 1, blocker B3) — the full changes-requested roster + +The round-1 audit found this document dispositioned only #2310 and #2311 out of the 19 +PRs in the changes-requested class (#2301 is handled in `070`). The remaining 16 existed +only as inventory rows in `000`, which is not a disposition. They are entered here. + +## Dispositioned roster + +| PR | State | Head | Size | Author | Disposition | Title | +|----|-------|------|------|--------|-------------|-------| +| #2299 | draft | `48326fc5` | 860+/3- (9f) | abhisheksharma2411 | REBUILD-CANDIDATE | feat(catalog): operator display labels for live-disc | +| #2298 | draft | `38888e3d` | 74+/0- (3f) | ppvia | REBUILD-CANDIDATE | fix(claude): warm empty Desktop-3P alias registry on | +| #2257 | draft | `510f1044` | 1289+/29- (20f) | yansigit | REBUILD-CANDIDATE | feat(agent): named subagent role catalog | +| #2244 | draft | `67acb331` | 913+/0- (9f) | ZSN12 | REBUILD-CANDIDATE | feat(workbuddy): add experimental desktop OAuth prov | +| #2215 | draft | `d85cf057` | 126+/41- (8f) | parkjs101 | REBUILD-CANDIDATE | docs(sub-agents): describe v2 fork override rule as | +| #2123 | draft | `701b51f9` | 495+/32- (3f) | chilung-cgu | REBUILD-CANDIDATE | feat(quota): add per-account Gem/Cla quota probing f | +| #2122 | draft | `fd6e53de` | 607+/41- (15f) | chilung-cgu | REBUILD-CANDIDATE | feat(catalog): config-level retainModels allowlist f | +| #2113 | ready | `3e17fe58` | 2184+/112- (63f) | cb8010d6 | TRIAGE-RESTATE | feat(providers): allow trusted encrypted V2 task pas | +| #2071 | draft | `e365907b` | 2789+/124- (25f) | yansigit | TRIAGE-RESTATE | feat(antigravity): CCA host failover and non-retryab | +| #2070 | draft | `f276d325` | 1608+/76- (14f) | yansigit | TRIAGE-RESTATE | feat(antigravity): Claude CCA wire fidelity | +| #2068 | ready | `9dceb40f` | 954+/31- (7f) | yansigit | REBUILD-CANDIDATE | feat(antigravity): live quota RPC and geoblock class | +| #2050 | draft | `52324cef` | 528+/72- (46f) | x3M3x | REBUILD-CANDIDATE | feat(combos): add random, least-used, and reset-wind | +| #1905 | ready | `0be75f29` | 781+/80- (27f) | luvs01 | REBUILD-CANDIDATE | feat(codex): add per-model ChatGPT compaction budget | +| #1829 | ready | `bf5e67f9` | 2878+/2- (4f) | luvs01 | TRIAGE-RESTATE | feat(codex): add durable reset-credit operation ledg | +| #1769 | draft | `f87c4acb` | 963+/36- (19f) | dbc-hbin | REBUILD-CANDIDATE | feat(gui): add manual paste fallback for OAuth add-a | +| #1756 | ready | `e9a04d1e` | 850+/116- (17f) | takltc | REBUILD-CANDIDATE | feat(grok): inject per-model reasoning effort into G | + +## Disposition rules for this roster + +**REBUILD-CANDIDATE** (diff under ~1500 added lines): the blockers are read in full, and +the PR is either fixed forward and merged after verification, or left open with the +blockers restated in a review comment. Merging requires this unit's own verification — +never author self-attestation. + +**TRIAGE-RESTATE** (#2113 2184+/63f, #2071 2789+/25f, #2070 1608+/14f): a diff of this +size with open reviewer blockers is not landable inside a backlog-clearing pass without +becoming its own review unit. Terminal disposition for this program: **left open with +blockers restated and the review burden named**. Recording that honestly is the +disposition; silently merging or silently closing would both be wrong. + +## Why nothing here is closed for staleness + +Every PR in this roster represents accepted intent with a reviewer objection attached. +Closing them would discard contributor work over process state rather than over evidence. +The terminal outcomes available are merge-after-verification, restate-and-leave-open, or +close-with-a-named-superseding-change — never close-because-old. + +## Reconciliation + +``` +19 changes-requested = 2 (#2310, #2311, body above) + + 1 (#2301, work-phase 7) + + 16 (this roster) +``` + +Combined with WP1 (8), WP7 (4, of which #2301 is counted above), and WP8 (15 conflicting ++ drafts + wrong-base), every one of the 45 open PRs now carries a named disposition +lane. WP8's C step performs the mechanical 45-item reconciliation before the goal closes. + diff --git a/devlog/_plan/260822_backlog_disposition_program/030_wp3_issue_2316_wait_agent_timeout.md b/devlog/_plan/260822_backlog_disposition_program/030_wp3_issue_2316_wait_agent_timeout.md new file mode 100644 index 0000000000..3d1417cbf2 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/030_wp3_issue_2316_wait_agent_timeout.md @@ -0,0 +1,243 @@ +# WP3 — Reimplement #2316: Grok wait_agent timeout_ms integral-float rejection + +> Source: read-only research lane `res-2316` (model `openrouter/stealth-ox-alpha`), dispatched at work-phase 0 against `dev@ced9a85c5`. +> Every file:line pointer below was independently spot-checked by the main agent before the roadmap was locked; verification notes are appended at the end of this document. + +Investigation complete. Both triage claims verified against real code; here is the implementation doc. + +# Issue #2316 — Implementation Doc + +## 1. ROOT CAUSE + +Both claimed defects are real and independently sufficient to reproduce the symptom. + +**Defect A — `number`-typed u64 fields are outside the repair contract.** +[src/lib/tool-argument-integers.ts](/Users/jun/Developer/new/700_projects/opencodex/src/lib/tool-argument-integers.ts) gates all integral-float repair on `integer` being *declared*: + +- L36–41 `declaresInteger`: `if (type === "integer") return true; return Array.isArray(type) && type.includes("integer");` — `"number"` returns false. +- L126–144 `coerceValue`, number branch: L129 `const integerDeclared = declaresInteger(resolved) || branches.some(declaresInteger);` and L141 `if (!integerDeclared || !safelyIntegral(value)) return { value, changed: false };`. So a `120000.0` arriving under `{ "type": "number" }` hits L141 with `integerDeclared === false` and passes through unchanged. L186 `if (!parameters || !args) return args;` also means no schema ⇒ full passthrough. + +Upstream evidence confirms the mismatch: [001_upstream_multiagent_v2_evidence.md:134](/Users/jun/Developer/new/700_projects/opencodex/devlog/_plan/260816_codexrs_multiagent_v2_and_history_perf/001_upstream_multiagent_v2_evidence.md) — V1 `wait_agent` optional field `timeout_ms: number`; :139 V2 same. Rust runtime deserializes `u64` ⇒ serde rejects `120000.0`. + +**Defect B — schema map keyed only by namespaced wire name unless toolChoice names the bare one.** +[src/server/responses/collaboration.ts:121](/Users/jun/Developer/new/700_projects/opencodex/src/server/responses/collaboration.ts): `const wireName = namespacedToolName(t.namespace, t.name)` (`namespace ? \`${namespace}__${name}\` : name`, [src/types/tools.ts:30–32](/Users/jun/Developer/new/700_projects/opencodex/src/types/tools.ts)), then L~117 `toolParameterSchemas.set(wireName, t.parameters)`. The bare-name entry exists only in the second loop (L153–161) guarded by `bareChoiceNames.has(t.name)` — i.e. only when `toolChoice` explicitly selected the tool. With `toolChoice: "auto"` and Grok echoing bare `wait_agent`, the bridge lookups miss: + +- Streaming: [src/bridge.ts:626–629](/Users/jun/Developer/new/700_projects/opencodex/src/bridge.ts) `coerceIntegerToolArguments(currentToolCall.args || "{}", options?.toolParameterSchemas?.get(currentToolCall.name))`. +- Non-streaming: [src/bridge.ts:1656–1659](/Users/jun/Developer/new/700_projects/opencodex/src/bridge.ts) `options?.toolParameterSchemas?.get(currentToolCallName)`. + +Missing key ⇒ `parameters === undefined` ⇒ L186 passthrough ⇒ `120000.0` reaches Codex raw. + +**Existing invariant to preserve:** [tests/tool-argument-integers.test.ts:59–62](/Users/jun/Developer/new/700_projects/opencodex/tests/tool-argument-integers.test.ts) `"never touches number-typed fields"` asserts `'{"temperature":1.0}'` comes back byte-identical. So the fix cannot blanket-coerce all `number` fields; it must be keyed to known Codex-native u64 fields. + +## 2. FILE CHANGE MAP + +### MODIFY [src/lib/tool-argument-integers.ts](/Users/jun/Developer/new/700_projects/opencodex/src/lib/tool-argument-integers.ts) + +**(a)** Add allowlist constant immediately before `function coerceValue(...` (currently L121): + +```diff ++/** ++ * Codex-native fields whose advertised JSON Schema says `number` but whose Rust ++ * runtime deserializes `u64` (#2316). An integral value serialized as `120000.0` ++ * has exactly one faithful reading (`120000`), so it is repaired; a genuinely ++ * fractional value still fails, and ordinary `number` fields like `temperature` ++ * stay untouched. ++ */ ++const U64_NUMBER_FIELDS = new Set(["timeout_ms"]); ++ + function coerceValue(value: unknown, schema: SchemaNode | undefined, root: SchemaNode, depth: number): CoerceResult { +``` + +**(b)** Thread the property key into `coerceValue` and add the number-field repair. Replace L121 signature and insert into the number branch between the `#1938` block (ends L138) and L139's comment: + +```diff +-function coerceValue(value: unknown, schema: SchemaNode | undefined, root: SchemaNode, depth: number): CoerceResult { ++function coerceValue(value: unknown, schema: SchemaNode | undefined, root: SchemaNode, depth: number, key?: string): CoerceResult { + // A hostile or deeply nested schema must not blow the stack. + if (depth > 64) return { value, changed: false }; + const resolved = schema ? resolveRef(schema, root, new Set()) : undefined; + + if (typeof value === "number") { + if (!resolved) return { value, changed: false }; + const branches = compositionBranches(resolved); + const integerDeclared = declaresInteger(resolved) || branches.some(declaresInteger); + if (!integerDeclared && safelyIntegral(value)) { + // Issue #1938: a bare integer in a string-only field has exactly one faithful + // string reading. A field that also accepts a numeric type keeps the number. + const stringDeclared = declaresString(resolved) || branches.some(declaresString); + const numericDeclared = declaresNumeric(resolved) || branches.some(declaresNumeric); + if (stringDeclared && !numericDeclared) { + return { value: String(value), changed: true }; + } + } ++ // #2316: a known Codex-native u64 field advertised as `number` still rejects ++ // integral floats at the Rust boundary. Re-serialize the same JS number so ++ // `120000.0` becomes `120000`; non-integral values keep failing. ++ if ( ++ !integerDeclared && key !== undefined && U64_NUMBER_FIELDS.has(key) ++ && (declaresNumeric(resolved) || branches.some(declaresNumeric)) ++ && safelyIntegral(value) ++ ) { ++ return { value, changed: true }; ++ } + // Not an integer field, already an integer, non-integral, or unrepresentable: + // in every one of those cases the received value is the right thing to keep. + if (!integerDeclared || !safelyIntegral(value)) return { value, changed: false }; +``` + +**(c)** Pass the key at the two recursive call sites in the array branch (L150) and object loop (L167): + +```diff + const next = value.map(entry => { +- const result = coerceValue(entry, itemSchema, root, depth + 1); ++ const result = coerceValue(entry, itemSchema, root, depth + 1); + if (result.changed) changed = true; + return result.value; + }); +``` +(array items keep no key — leave that call as-is.) + +```diff + for (const [key, entry] of Object.entries(object)) { + const childSchema = asSchema(properties?.[key]) ?? additional; +- const result = coerceValue(entry, childSchema, root, depth + 1); ++ const result = coerceValue(entry, childSchema, root, depth + 1, key); + if (result.changed) changed = true; + next[key] = result.value; + } +``` + +Note: `JSON.parse("120000.0") === 120000` and `Number.isInteger(120000) === true`, so "repair" here just means re-stringify; a payload already containing `120000` produces byte-identical output, keeping the "returns original bytes when nothing needs repair" behavior intact for these fields too. + +### MODIFY [src/server/responses/collaboration.ts](/Users/jun/Developer/new/700_projects/opencodex/src/server/responses/collaboration.ts) + +In `buildToolBridgeMaps`, first authorization loop (~L117), register the bare logical name as a schema alias whenever the tool is namespaced. Collision-guarded (first declaration wins): + +```diff +- if (t.parameters && typeof t.parameters === "object") toolParameterSchemas.set(wireName, t.parameters); ++ if (t.parameters && typeof t.parameters === "object") { ++ toolParameterSchemas.set(wireName, t.parameters); ++ // #2316: routed providers can echo the bare logical name instead of the ++ // namespaced wire name even under toolChoice "auto"; expose the schema ++ // under both keys so argument repair still finds it. ++ if (t.namespace && !toolParameterSchemas.has(t.name)) toolParameterSchemas.set(t.name, t.parameters); ++ } +``` + +No change needed in [src/bridge.ts](/Users/jun/Developer/new/700_projects/opencodex/src/bridge.ts) — its `.get(name)` lookups become hits once the map carries the alias. (If another adapter builds `toolParameterSchemas` itself without aliases, it would still miss; none currently do besides this builder.) + +## 3. TEST PLAN + +Extend [tests/tool-argument-integers.test.ts](/Users/jun/Developer/new/700_projects/opencodex/tests/tool-argument-integers.test.ts). + +Add fixture near the top: + +```ts +/** The multi_agent wait shape from the #2316 report: Codex advertises number, runtime wants u64. */ +const WAIT_TIMEOUT_SCHEMA = { + type: "object", + properties: { targets: { type: "array", items: { type: "string" } }, timeout_ms: { type: "number" } }, +}; +``` + +New tests inside the top-level describe (names verbatim): + +1. `test("repairs an integral float in a number-advertised u64 field (#2316)")` — assert `coerceIntegerToolArguments('{"targets":["a"],"timeout_ms":120000.0}', WAIT_TIMEOUT_SCHEMA)` `toBe('{"targets":["a"],"timeout_ms":120000}')`. **Fails before** (returns input unchanged); passes after. +2. `test("leaves a fractional timeout_ms failing")` — `coerceIntegerToolArguments('{"timeout_ms":1.5}', WAIT_TIMEOUT_SCHEMA)` `toBe('{"timeout_ms":1.5}')`. +3. `test("still never touches ordinary number-typed fields")` — existing temperature assertion at L59–62 must remain green unchanged (guards regression). +4. Wiring test in the `#1611 wiring` describe: extend `schemas` map with both `"multi_agent_v1__wait_agent"` and `"wait_agent"` mapped to `WAIT_TIMEOUT_SCHEMA`; add `test("bare-name tool calls find their schema for u64 repair")` — stream events `{ type:"tool_call_start", id:"call_3", name:"wait_agent" }`, delta `'{"timeout_ms":120000.0}'`, end, done, via `bridgeToResponsesSSE(..., { toolParameterSchemas: schemas })`; assert `response.function_call_arguments.done` arguments `toBe('{"timeout_ms":120000}')`. +5. Builder test (new small test or appended): call `buildToolBridgeMaps` from `src/server/responses/collaboration.ts` with a parsed request containing one namespaced tool `multi_agent_v1/wait_agent` with parameters and default/auto choice; assert `maps.toolParameterSchemas.get("wait_agent")` is defined **and** `maps.toolParameterSchemas.get("multi_agent_v1__wait_agent")` is defined. Fails before (bare key undefined under auto choice); passes after. + +## 4. VERIFIER COMMAND + +```bash +bun test tests/tool-argument-integers.test.ts +``` + +Yes — it imports and executes both changed files directly (`src/lib/tool-argument-integers.ts` at L2, `src/bridge.ts` at L3) and will execute `src/server/responses/collaboration.ts` once test 5 imports it. Before opening a review-ready PR, AGENTS.md requires `bun run typecheck` and `bun run test` (this touches shared server config-building code). + +## 5. ACTIVATION SCENARIO + +A test triggers the path by feeding the real bridge an adapter event sequence where `tool_call_start` carries name `wait_agent` (bare) and the args delta carries `{"timeout_ms":120000.0}`, with `options.toolParameterSchemas` built by `buildToolBridgeMaps` under a default/auto tool choice. Observable proof the changed conditional ran: the SSE frame `response.function_call_arguments.done` (streaming) / `output[].arguments` in `buildResponseJSON` (non-streaming) serializes `120000` instead of `120000.0` — i.e. the exact bytes Codex's serde layer would otherwise reject. Unit-level observable: `coerceIntegerToolArguments` returns a different string than its input only for allowlisted keys declaring numeric types. + +## 6. RISK/BLOCKERS + +- None blocking. The design deliberately avoids blanket `number` coercion; `temperature: 1.0` stays untouched because it is not in `U64_NUMBER_FIELDS` (existing test remains the guard). +- The allowlist is hand-maintained: future Codex tools advertising `number` for u64 fields will need entries added. That is the accepted trade-off versus silently rewriting arbitrary floats. +- If Grok emits the v2 namespaced form `collaboration__wait_agent` instead of `multi_agent_v1__wait_agent`, Defect B's alias fix covers any namespace since the bare alias is registered per declared tool regardless of namespace value. +- Per the triage comment, sibling field Cursor `yield_time_ms` (`src/adapters/cursor/tool-definitions.ts:49`, still `type:"number"`) is explicitly out of scope for this ticket — do not widen the change. +- Do not let PR #2320 carry `Closes #2316`; it touches unrelated Cursor error classification. + + +--- + +# AMENDMENT (A-phase round 1, blockers B1+B2) — authoritative over the body above + +The round-1 auditor found, and the main agent independently confirmed, that the +"Defect B" half of the research lane's diagnosis is **wrong for this bug**. This section +overrides the body wherever they disagree. + +## What was struck + +The proposed edit to `src/server/responses/collaboration.ts` (register the bare logical +name as a schema alias) is **removed from this work-phase**. Two independent reasons: + +**1. It is unreachable.** `src/bridge.ts:1041` (streaming) and `src/bridge.ts:1788` +(non-streaming) reject any tool name absent from `declaredToolNames` with a 502 *before* +argument repair runs: + +```ts +if (options?.declaredToolNames && !options.declaredToolNames.has(event.name)) { + const failure = responseError(502, "upstream_error", + `routed provider emitted undeclared client tool "${event.name}"; only request-declared tools may be called`); +``` + +A schema registered under a bare key that is not also in `declaredToolNames` can never be +consulted, because the call is already dead. The proposed wiring test injected the schema +map by hand and bypassed the guard, which is why it looked like it would work. + +**2. The reported bug never involved a bare name.** Issue #2316's body reports the +failing call as `multi_agent_v1__wait_agent` — namespaced — and the error text +(`invalid type: floating point \`120000.0\`, expected u64`) comes from **Codex's own +deserializer**, which only sees the call after it passed our bridge. The schema lookup +hit; the repair simply declined to act because the field declares `number`, not +`integer`. Defect A is the entire bug. + +## Policy recorded for any future unit (B2) + +If bare-name repair is ever genuinely wanted, the alias must be admitted **atomically** +into `declaredToolNames`, `toolNsMap`, and `toolParameterSchemas` together, under the +uniqueness rule that already governs that path at +`src/server/responses/collaboration.ts:154` (`bareNameCounts.get(t.name) !== 1` refuses +the alias). Registering into the schema map alone bypasses a deliberate collision policy +and can hand one tool's schema to a different, legitimately-authorized tool. + +## Amended scope of WP3 + +**One file changes:** `src/lib/tool-argument-integers.ts` (plus its test). + +- Add `U64_NUMBER_FIELDS = new Set(["timeout_ms"])`. +- Thread the property key into `coerceValue` and repair an integral float when the key is + allowlisted and the field declares a numeric type. +- `temperature: 1.0` must remain byte-identical (existing test + `tests/tool-argument-integers.test.ts:59-62` is the guard). +- Fractional values (`1.5`) must still pass through unrepaired and fail upstream. + +## Amended verifier (B5) + +Run as its own invocation, with an existence gate, so a missing file fails the phase: + +```bash +test -f tests/tool-argument-integers.test.ts || exit 1 +bun test tests/tool-argument-integers.test.ts +``` + +Baseline before the change: 24 pass / 0 fail. After: 24 + the new cases, 0 fail. + +## Amended activation scenario + +Unit-level, no bridge injection required: `coerceIntegerToolArguments` with the real +`wait_agent` schema shape (`timeout_ms: {type: "number"}`) returns +`'{"timeout_ms":120000}'` for input `'{"timeout_ms":120000.0}'` — a different string than +its input — while `'{"temperature":1.0}'` under the same call returns byte-identical. + diff --git a/devlog/_plan/260822_backlog_disposition_program/040_wp4_issue_2292_windows_picker.md b/devlog/_plan/260822_backlog_disposition_program/040_wp4_issue_2292_windows_picker.md new file mode 100644 index 0000000000..4c1b22107c --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/040_wp4_issue_2292_windows_picker.md @@ -0,0 +1,290 @@ +# WP4 — Reimplement #2292: Windows model picker stale after ocx sync --restart-codex + +> Source: read-only research lane `res-2292` (model `openrouter/stealth-ox-alpha`), dispatched at work-phase 0 against `dev@ced9a85c5`. +> Every file:line pointer below was independently spot-checked by the main agent before the roadmap was locked; verification notes are appended at the end of this document. + +All pointers verified against `dev@ced9a85c5`. The triage comment is accurate; PR #2293 landed only the standalone PowerShell helper, and the CLI integration it promised is still missing. Here is the diff-level implementation doc. + +--- + +# #2292 — Windows model picker stays stale after `ocx sync --restart-codex` + +## 1. ROOT CAUSE + +**Chain: `ocx sync` → app-server-only kill → Electron shell (`ChatGPT.exe`) never signaled → renderer cache survives → doctor prints false OK.** + +1. [src/cli/dispatch.ts:205](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:205) parses only one flag: + ```ts + const restartCodex = deps.args.slice(1).includes("--restart-codex"); + ``` + and at [src/cli/dispatch.ts:230-232](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:230): + ```ts + if (synced.catalogWritten || synced.cacheSynced) { + afterCatalogWriteHandleAppServers({ restart: restartCodex, log: console }); + } + ``` + `sync-cache` repeats this verbatim at [src/cli/dispatch.ts:261-273](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:261). + +2. [src/codex/app-server-processes.ts:1072-1101](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:1072) — `afterCatalogWriteHandleAppServers` operates only on `listCodexAppServerProcesses(...)` output, i.e. matches of `isCodexAppServerCommandLine` ([src/codex/app-server-processes.ts:247-274](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:247)). That matcher requires token[0] to be a `codex`/`codex.exe`/`codex.cmd`/target-triple executable **and** subcommand `app-server`, or a `codex-code-mode-host` token. `ChatGPT.exe` (the Electron desktop shell) matches neither — its command line contains no `codex` executable token, so it is never listed, never killed, never relaunched. The registry contract even advertises this narrowness: [src/cli/registry.ts:95](/Users/jun/Developer/new/700_projects/opencodex/src/cli/registry.ts:95): `"--restart-codex sends SIGTERM only to matching app-server / code-mode-host processes"`. + +3. Why the picker stays stale: the desktop renderer caches `model/list` + `config/read` (TanStack Query over stdio JSON-RPC) and invalidates only on a `codex-app-server-initialized` event; on Windows MSIX, externally `taskkill`ing the `codex.exe` child does not reliably re-emit that event in the surviving shell (devlog research, `devlog/_plan/260821_260821-windows-picker-full-restart/000_plan.md:24-31`). macOS recovers because the respawned app-server triggers the event; Windows does not. + +4. The false-OK path: `collectCodexAppServerCatalogState` ([src/codex/app-server-processes.ts:774-824](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:774)) compares **app-server start time vs catalog mtime**. The freshly respawned `codex.exe` postdates the catalog, so state = `fresh`, and [src/cli/doctor.ts:1153-1154](/Users/jun/Developer/new/700_projects/opencodex/src/cli/doctor.ts:1153) prints `[OK] Codex app-server model catalog is current with the on-disk catalog.` — while the Electron shell still shows the old picker. All three user-facing hints point at the flag that doesn't fix Windows: `STALE_CODEX_APP_SERVER_HINT` ([src/codex/app-server-processes.ts:19-20](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:19)), `formatStaleCodexAppServerWarning`, and doctor's WARN text (`ocx sync --restart-codex`). + +5. The GUI restart route has the same hole: `performCodexRestart` ([src/codex/app-server-restart-service.ts:78-79](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-restart-service.ts:78)) uses the same `collectCodexAppServerCatalogState` + `restartCodexAppServers`, so a dashboard click also restarts only the app-server. + +6. Windows kill semantics are **not** the cause: `defaultKillCodexAppServer` ([src/codex/app-server-processes.ts:976-998](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:976)) uses `taskkill /PID /T /F`; `/T` covers only that PID's descendants. The Electron shell is the *parent*, so it is intentionally untouched. This asymmetry must be preserved (Unix SIGTERM stays graceful, no SIGKILL escalation — comment at :962-971). + +7. Existing state: `scripts/restart-codex-desktop-app.ps1` exists (from #2293) with package-bound targeting, self-kill guard, `CloseMainWindow` → bounded `taskkill /T /F` → AUMID relaunch — but it is **unreachable from the product**: `package.json` `files` = `['bin','src','gui/dist','assets/...','README.md','AGENTS_INSTALL.md','LICENSE']` — no `scripts/`, and nothing in `src/` references it (`rg 'restart-codex-desktop-app'` hits only the devlog). Also note its constants are hardcoded (`$PackageFamily = "OpenAI.Codex_2p2nqsd0c76g0"`), which the maintainer triage explicitly rejects for the integrated path (AUMID changes per beta build). + +## 2. FILE CHANGE MAP + +Design constraints from the maintainer triage (verified consistent with the code): `--restart-codex` must keep app-server-only matching; the new capability is an opt-in Windows-only `--restart-desktop-app`, run only after an actual write; discovery must be runtime (no hardcoded AUMID); fail closed with an actionable message; Unix/macOS untouched. + +### 2.1 NEW `src/codex/desktop-app-restart.ts` + +New module (keeps `app-server-processes.ts` untouched — the triage warns against rebasing over a moved file). + +```ts +export interface DesktopAppRestartIo { + platform?: NodeJS.Platform; + execFile?: (file: string, args: readonly string[]) => Promise<{ stdout: string }>; + isAlive?: (pid: number) => boolean; + waitExit?: (pid: number, timeoutMs: number) => boolean; + log?: Pick | null; +} + +export interface DesktopAppRestartResult { + attempted: boolean; + stopped: number[]; + surviving: number[]; + relaunch: "started" | "skipped"; + reason?: string; // set when attempted === false or relaunch === "skipped" +} + +export async function restartCodexDesktopApp(io: DesktopAppRestartIo = {}): Promise +``` + +Implementation contract (all via `resolveTrustedWindowsPowerShellExe()` / `resolveTrustedWindowsTaskkillExe()` from `src/lib/windows-elevation.ts:192` — never PATH): + +1. `if ((io.platform ?? process.platform) !== "win32") return { attempted: false, stopped: [], surviving: [], relaunch: "skipped", reason: "windows_only" };` +2. Discover the package at runtime with a single PowerShell probe (one `execFile` call, `timeout: 10_000`, `windowsHide: true`): + ```powershell + $p = Get-AppxPackage -Name OpenAI.Codex; if (-not $p) { $p = Get-AppxPackage -Name OpenAI.CodexBeta } + if (-not $p -or -not $p.InstallLocation) { 'MISS' } else { + '{0}`n{1}`n{2}' -f $p.PackageFamilyName, $p.InstallLocation, "$($p.PackageFamilyName)!App" + } + ``` + Parse `family`, `installLocation`, `aumid`. On `MISS` or unparseable output: return `{ attempted: false, ..., reason: "package_discovery_failed" }` — **do not kill anything** (fail closed per triage). +3. Enumerate candidate roots: + ```powershell + Get-CimInstance Win32_Process -Filter "Name='ChatGPT.exe'" | + Where-Object { $_.ExecutablePath -and $_.ExecutablePath.StartsWith($installLocation, 'OrdinalIgnoreCase') } + ``` + Case-insensitive `StartsWith` handles Windows path casing. Roots = processes whose `ParentProcessId` is not itself a package-tree process (mirrors the .ps1 root logic at `scripts/restart-codex-desktop-app.ps1:33-36`). +4. Self-kill guard: walk `process.ppid` ancestry (or a PowerShell-side ancestry walk like the .ps1 at :44-56); if any root is an ancestor, abort with `reason: "self_ancestry"` and kill nothing. +5. Graceful pass per root: `execFile(powershell, ['-NoProfile','-Command', ` (Get-Process -Id ).CloseMainWindow() `])`, then poll `io.waitExit(pid, 1000)` up to 15 s. If still alive → forced pass: `execFile(resolveTrustedWindowsTaskkillExe(), ['/PID', String(pid), '/T', '/F'])` (same trusted-resolution discipline as `defaultKillCodexAppServer`), then wait up to 5 s. Record `stopped` / `surviving`. +6. Relaunch only if every verified target stopped: `execFile(powershell, ['-NoProfile','-Command', `Start-Process 'shell:AppsFolder\'`])`. If any survivor, skip relaunch and set `reason: "targets_survived"`. +7. Export a test-only seam type so every branch is injectable (`execFile`, `isAlive`, `waitExit`, `platform`), following the `CodexAppServerProcessIo` pattern at [src/codex/app-server-processes.ts:70-91](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:70). + +### 2.2 MODIFY `src/cli/dispatch.ts` + +Symbol: `sync` handler. + +**Before** ([src/cli/dispatch.ts:205](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:205)): +```ts + const restartCodex = deps.args.slice(1).includes("--restart-codex"); +``` +**After**: +```ts + const syncArgs = deps.args.slice(1); + const restartCodex = syncArgs.includes("--restart-codex"); + const restartDesktopApp = syncArgs.includes("--restart-desktop-app"); +``` + +**Before** ([src/cli/dispatch.ts:230-232](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:230)): +```ts + if (synced.catalogWritten || synced.cacheSynced) { + afterCatalogWriteHandleAppServers({ restart: restartCodex, log: console }); + } +``` +**After**: +```ts + if (synced.catalogWritten || synced.cacheSynced) { + afterCatalogWriteHandleAppServers({ restart: restartCodex, log: console }); + if (restartDesktopApp) { + const { restartCodexDesktopApp } = await import("../codex/desktop-app-restart"); + const desktop = await restartCodexDesktopApp({ log: console }); + if (desktop.reason === "windows_only") { + console.error("--restart-desktop-app is supported on Windows only; nothing was stopped."); + } else if (desktop.reason === "package_discovery_failed") { + console.error("Could not identify the OpenAI Codex desktop package; quit and relaunch the desktop app manually to refresh the model picker."); + } else if (desktop.reason === "targets_survived") { + console.error(`Desktop app PID(s) ${desktop.surviving.join(", ")} did not exit; quit the desktop app manually to refresh the model picker.`); + } else if (desktop.relaunch === "started") { + console.log("Codex desktop app restarted; the model picker will re-read the catalog."); + } + } + } +``` + +Symbol: `sync-cache` handler — identical two edits at [src/cli/dispatch.ts:261](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:261) (`const restartCodex = ...` line) and [src/cli/dispatch.ts:270-272](/Users/jun/Developer/new/700_projects/opencodex/src/cli/dispatch.ts:270) (same `if (invalidated.kind === "completed" && invalidated.value)` block, same appended desktop block). + +### 2.3 MODIFY `src/cli/registry.ts` + +Symbols: `sync` and `sync-cache` entries. + +**Before** ([src/cli/registry.ts:91](/Users/jun/Developer/new/700_projects/opencodex/src/cli/registry.ts:91)): +```ts + usage: "ocx sync [--restart-codex]", +``` +**After**: +```ts + usage: "ocx sync [--restart-codex] [--restart-desktop-app]", +``` +and add to `details` after the existing `--restart-codex` line: +```ts + "--restart-desktop-app (Windows only, opt-in) fully restarts the Codex desktop app so its model picker re-reads the catalog; never implied by --restart-codex.", +``` +Same for `sync-cache` at [src/cli/registry.ts:100](/Users/jun/Developer/new/700_projects/opencodex/src/cli/registry.ts:100). + +### 2.4 MODIFY `src/cli/doctor.ts` + +Symbol: the `#857` catalog-state block. + +**Before** ([src/cli/doctor.ts:1153-1154](/Users/jun/Developer/new/700_projects/opencodex/src/cli/doctor.ts:1153)): +```ts + } else if (catalogState.state === "fresh") { + console.log(" [OK] Codex app-server model catalog is current with the on-disk catalog."); +``` +**After** (suppress false OK on Windows when the desktop shell predates the catalog — the shell, not the app-server, owns the picker): +```ts + } else if (catalogState.state === "fresh") { + if (process.platform === "win32" && desktopShellPredatesCatalog()) { + console.log(" [WARN] The Codex desktop app started before the on-disk catalog changed; its model picker may still show the old list. Action: quit and relaunch the desktop app (or run 'ocx sync --restart-desktop-app')"); + } else { + console.log(" [OK] Codex app-server model catalog is current with the on-disk catalog."); + } +``` +Supporting helper (same file or in `desktop-app-restart.ts`, exported for tests): reuse the existing PowerShell CIM machinery pattern — enumerate `ChatGPT.exe` processes under the discovered package `InstallLocation` (runtime discovery identical to §2.1 step 2), take the earliest `CreationDate`, compare against `defaultCatalogMtimeMs()` (the same mtime source `collectCodexAppServerCatalogState` uses at [src/codex/app-server-processes.ts:806](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:806)). On any discovery/read failure return `false` (never manufacture a WARN from guesswork — matches the `unknown`-not-stale doctrine at [src/codex/app-server-processes.ts:789-795](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:789)). + +### 2.5 MODIFY `src/codex/app-server-processes.ts` (one line, hint text only) + +**Before** ([src/codex/app-server-processes.ts:19-20](/Users/jun/Developer/new/700_projects/opencodex/src/codex/app-server-processes.ts:19)): +```ts +export const STALE_CODEX_APP_SERVER_HINT = + "If Codex still shows an older model list, restart its long-lived app-server process after sync (ocx sync --restart-codex)."; +``` +**After**: +```ts +export const STALE_CODEX_APP_SERVER_HINT = + "If Codex still shows an older model list, restart its long-lived app-server process after sync (ocx sync --restart-codex); on Windows the desktop app may also need a full restart (ocx sync --restart-desktop-app)."; +``` +This propagates automatically to `formatStaleCodexAppServerWarning`, `attachStaleAppServerHint`, and the dashboard hint — all read this constant. + +### 2.6 OPTIONAL (defer unless trivial): package the helper + +`package.json` `files` currently omits `scripts/`, so `scripts/restart-codex-desktop-app.ps1` never ships. The CLI-integrated path above does not depend on the .ps1 file (it inlines the same logic via trusted PowerShell), so packaging is not a blocker; if the team wants the standalone script shipped too, add `"scripts"` to `files` — but flag that as a separate decision since it changes the npm payload. + +## 3. TEST PLAN + +### 3.1 NEW `tests/desktop-app-restart.test.ts` + +Inject everything through `DesktopAppRestartIo` — no real processes, no Windows required. + +- `"is a no-op off Windows"` — `platform: "linux"` → `{ attempted: false, relaunch: "skipped", reason: "windows_only" }`, `execFile` never called. +- `"fails closed when package discovery returns nothing"` — `execFile` returns `{ stdout: "MISS" }` → `attempted: false`, no kill call ever issued. +- `"kills only package-tree roots, gracefully first, forced after timeout"` — fake `execFile` script returns package info for discovery and records `taskkill` calls; `isAlive` returns true for 3 polls then false → assert exactly one `CloseMainWindow`-shaped PowerShell call per root, no `taskkill`; then with `waitExit` always false → assert `taskkill /PID /T /F` and **no kill of a ChatGPT.exe whose ExecutablePath is outside the install location** (include an out-of-package decoy process in the CIM fixture output). +- `"refuses to kill its own ancestry"` — CIM fixture lists a root PID present in the injected ancestry chain → `reason: "self_ancestry"`, zero kill calls. +- `"relaunches via the discovered AUMID only after every target stopped"` — all stop → assert `Start-Process 'shell:AppsFolder\'` call and `relaunch: "started"`; one survivor → no relaunch call, `reason: "targets_survived"`. +- `"does not hardcode the beta AUMID"` — discovery fixture returns `OpenAI.Codex_9.9.9.0_hzzzzzzz0!App` → relaunch command contains that exact string. + +### 3.2 MODIFY `tests/codex-app-server-processes.test.ts` + +In the existing `"rejects unrelated processes..."` block ([tests/codex-app-server-processes.test.ts:433-437](/Users/jun/Developer/new/700_projects/opencodex/tests/codex-app-server-processes.test.ts:433)) add the regression the triage demands: + +```ts +expect(isCodexAppServerCommandLine( + '"C:\\Program Files\\WindowsApps\\OpenAI.Codex_2p2nqsd0c76g0\\ChatGPT.exe" --msix')) + .toBe(false); +``` + +**Fails before**: currently `true`? No — it already returns `false` (no `codex` token). This is a **lock-in test**, not a red/green test; the red/green tests are in 3.3/3.4. State that explicitly in the PR. + +### 3.3 MODIFY `tests/dispatch-sync.test.ts` (or the existing sync-arg test file — locate with `rg -l '"sync"' tests/ | head`) + +- `"ocx sync --restart-desktop-app triggers the desktop restart only after a real write"` — inject a fake `syncModelsToCodex` returning `{ catalogWritten: false, cacheSynced: false }` → desktop restart import/mock not invoked; with `catalogWritten: true` → invoked exactly once with the console logger. Also assert `--restart-codex` alone does **not** invoke it (default-off contract). +- `"sync-cache --restart-desktop-app triggers after completed invalidation"` — `invalidated.kind !== "completed"` → not invoked; `completed && value` → invoked. + +### 3.4 MODIFY doctor test (`rg -l 'model catalog is current' tests/`) + +- `"doctor suppresses the fresh OK on Windows when the desktop shell predates the catalog"` — inject `platform: win32`, `desktopShellPredatesCatalog: true` → output contains `[WARN] The Codex desktop app started before` and does **not** contain `[OK] Codex app-server model catalog is current`. This is the precise false-OK regression from the issue: **fails before** the fix (prints OK), **passes after**. +- `"doctor keeps the fresh OK when shell discovery fails"` — helper returns `false` → OK line preserved. + +## 4. VERIFIER COMMAND + +```bash +bun test tests/desktop-app-restart.test.ts tests/codex-app-server-processes.test.ts +bun test tests/dispatch-sync.test.ts # or the located sync-arg test file +bun x tsc --noEmit +``` + +Yes, all of these read the changed files: the new test file imports `src/codex/desktop-app-restart.ts` directly; the dispatch tests exercise `src/cli/dispatch.ts`'s `sync`/`sync-cache` handlers; the process-matcher test reads `src/codex/app-server-processes.ts`; tsc's `tsconfig` includes `src/`. Per repo policy this is a scoped change to CLI dispatch + a new isolated module, so focused checks are correct; run the full `bun run typecheck && bun run test` only before marking the PR review-ready. + +## 5. ACTIVATION SCENARIO + +A test triggers the new path by (a) passing `--restart-desktop-app` in the dispatch handler's `deps.args` (e.g. `["sync", "--restart-desktop-app"]`) with an injected `syncModelsToCodex` stub returning `catalogWritten: true`, and (b) injecting a fake `execFile` in `DesktopAppRestartIo` whose scripted outputs simulate: package discovery → CIM process list → graceful close → exit. The observable proof the conditional path ran is the recorded `execFile` call sequence (discover → close → taskkill only on timeout → `Start-Process shell:AppsFolder\...`) plus the returned `{ attempted: true, relaunch: "started" }` and the `"Codex desktop app restarted..."` console line captured by an injected logger. For doctor, the observable is the WARN line replacing the OK line in captured stdout. On a real Windows machine the end-to-end proof is: `ocx sync --restart-desktop-app` → all package-tree PIDs change (new `ChatGPT.exe` start time) → picker shows the 5 appended models. + +## 6. RISK / BLOCKERS + +- **Not implementable as "widen `--restart-codex`"** — explicitly forbidden by the triage (registry contract at [src/cli/registry.ts:95](/Users/jun/Developer/new/700_projects/opencodex/src/cli/registry.ts:95) advertises app-server-only, and killing the Electron shell interrupts whole conversations, a different consent). The doc above respects that. +- **PowerShell/Appx availability**: `Get-AppxPackage` exists in Windows PowerShell 5.1; pwsh 7 needs the `Appx` module import fallback (the .ps1 does `Import-Module Appx -ErrorAction SilentlyContinue` at :16). The TS path must tolerate discovery failure and fail closed — covered by test 3.1 case 2. +- **MSIX path casing**: handled with `OrdinalIgnoreCase` `StartsWith`; the `PackageFamilyName` is *not* a substring of `InstallLocation`, so match on `InstallLocation` only (the .ps1's approach). +- **`CloseMainWindow` close-to-tray**: the 15 s bounded wait plus forced fallback mirrors the .ps1; the WARN-then-force behavior is asserted in test 3.1 case 3. +- **The hardcoded AUMID in the shipped .ps1** (`scripts/restart-codex-desktop-app.ps1:19-20`) contradicts the triage ("AUMID는 베타 MSIX가 빌드마다 바뀜… 하드코드 금지"). The integrated TS path fixes this by runtime discovery; the standalone script remains a known deviation — worth a follow-up note in the PR, not a blocker for the CLI contract. +- **`desktopShellPredatesCatalog` adds a synchronous PowerShell probe to `ocx doctor` on Windows** — acceptable (doctor is already a cold diagnostics path and the app-server collector there is synchronous too), but keep it behind a short timeout and fail-open to the OK line. +- **No blockers to implementation as specified.** The devlog plan explicitly scoped runtime integration *out* of wp1 and deferred it to "a later unit" (`000_plan.md` OUT section) — this is that unit. + + +--- + +# AMENDMENT (A-phase round 1, blockers B5+B6) + +**B5 — the named verifier does not exist.** `tests/dispatch-sync.test.ts` is not in the +tree. The real dispatch suite is `tests/cli-dispatch.test.ts`. Every mandatory new suite +gets an existence gate and its own invocation: + +```bash +test -f tests/desktop-app-restart.test.ts || exit 1 +bun test tests/desktop-app-restart.test.ts +test -f tests/cli-dispatch.test.ts || exit 1 +bun test tests/cli-dispatch.test.ts +bun test tests/codex-app-server-processes.test.ts +``` + +Rationale: Bun exits 0 on a multi-file invocation when some listed files are missing, as +long as one exists. A single combined command would report green while the regression +that proves the fix never ran. + +**B6 — the `execFile` seam cannot carry its mandated timeout.** The proposed type + +```ts +execFile?: (file: string, args: readonly string[]) => Promise<{ stdout: string }>; +``` + +has no options parameter, yet the same document requires `timeout: 10_000` and +`windowsHide: true`. A hung `Get-AppxPackage` or CIM probe would wedge `ocx sync` and +`ocx doctor` with no bound. Amended seam: + +```ts +execFile?: ( + file: string, + args: readonly string[], + options?: { timeout?: number; windowsHide?: boolean; signal?: AbortSignal }, +) => Promise<{ stdout: string }>; +``` + +Acceptance additions: a test proving the probe rejects on timeout and does not leave a +child process behind, and a test proving a timed-out discovery fails **closed** (kills +nothing, relaunches nothing). + diff --git a/devlog/_plan/260822_backlog_disposition_program/050_wp5_issue_2221_native_main_refresh.md b/devlog/_plan/260822_backlog_disposition_program/050_wp5_issue_2221_native_main_refresh.md new file mode 100644 index 0000000000..4cda893fa6 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/050_wp5_issue_2221_native_main_refresh.md @@ -0,0 +1,208 @@ +# WP5 — Reimplement #2221: native main pool auth.json token refresh (disposes #2222) + +> Source: read-only research lane `res-2221` (model `openrouter/stealth-ox-alpha`), dispatched at work-phase 0 against `dev@ced9a85c5`. +> Every file:line pointer below was independently spot-checked by the main agent before the roadmap was locked; verification notes are appended at the end of this document. + +All evidence gathered from HEAD `ced9a85c5` plus the full PR #2222 diff and issue triage. Report below. + +--- + +# Implementation Doc — Issue #2221 (native `__main__` auth.json token refresh) + +## 1. ROOT CAUSE + +Verified chain at `ced9a85c5`: + +1. [src/codex/main-account.ts:34](/Users/jun/Developer/new/700_projects/opencodex/src/codex/main-account.ts:34) — `getMainAccountToken()` is read-only: + ```ts + /** Read-only main account token from ~/.codex/auth.json, or null when not logged in. */ + export function getMainAccountToken(): ... { + const tokens = readCodexTokens(); + if (!tokens?.access_token) return null; + ``` + It never reads `refresh_token` (`readCodexTokensResult()` at [src/codex/auth-collision.ts:44-58](/Users/jun/Developer/new/700_projects/opencodex/src/codex/auth-collision.ts:44) only parses `access_token/account_id/id_token`) and never calls the token endpoint. + +2. [src/codex/auth-context.ts:515-517](/Users/jun/Developer/new/700_projects/opencodex/src/codex/auth-context.ts:515) — when routing selects `__main__` into rotation: + ```ts + if (accountId === MAIN_CODEX_ACCOUNT_ID) { + // Main account in rotation: inject the read-only auth.json token and fail closed if it vanished. + const token = (options.getMainAccountToken ?? getMainAccountToken)(); + ``` + The expired bearer is injected into `kind: "main-pool"` (line 534) and sent upstream → 401. + +3. Contrast: the stored-pool path at [src/codex/auth-context.ts:548](/Users/jun/Developer/new/700_projects/opencodex/src/codex/auth-context.ts:548) calls `getValidCodexToken(accountId)`, which refreshes under the shared grant file-lock in [src/codex/account-store.ts:446-530](/Users/jun/Developer/new/700_projects/opencodex/src/codex/account-store.ts:446) (lock acquisition, same-grant adoption, POST to `CHATGPT_TOKEN_URL`, generation-CAS save). + +4. Usability gate enforces the gap: [src/codex/account-usability.ts:36-37](/Users/jun/Developer/new/700_projects/opencodex/src/codex/account-usability.ts:36) — + ```ts + // Main account: credential is the read-only ~/.codex/auth.json token (Option A). + return (options.isMainAccountTokenLive ?? isMainAccountTokenLive)(); + ``` + `isMainAccountTokenLive` ([main-account.ts:45-51](/Users/jun/Developer/new/700_projects/opencodex/src/codex/main-account.ts:45)) returns false once JWT `exp < now`, so an expired-but-refreshable main account becomes unroutable while pool accounts keep working. + +5. No reactive path either: neither `/v1/responses` ([src/server/responses/core.ts:3173-3177](/Users/jun/Developer/new/700_projects/opencodex/src/server/responses/core.ts:3173) has only the xai/copilot/kiro OAuth replay) nor `/v1/responses/compact` ([src/server/responses/compact.ts:385](/Users/jun/Developer/new/700_projects/opencodex/src/server/responses/compact.ts:385) calls sync `materializeCodexUpstreamAuth`, defined sync at [auth-context.ts:619](/Users/jun/Developer/new/700_projects/opencodex/src/codex/auth-context.ts:619)) has any native-main 401 refresh/replay branch. The triage comment's pointers are all accurate against this head. + +## 2. VERDICT ON PR #2222 + +**Approach: architecturally correct. Reuse: not directly — clean reimplementation on current `dev` is better.** + +What the PR gets right (matches the issue contract): + +- Pre-request refresh via new `getValidMainAccountToken()` / `forceRefreshMainAccountToken()` in `src/codex/main-account.ts`. +- Shares the pool's existing grant file-lock (`withCodexRefreshFileLock`) keyed by refresh-grant fingerprint — no second lock system. +- Exactly-one 401 replay for Responses (both pre-stream and continuation loops in `core.ts`, `codexMain401ReplayAttempted`) and pre-I/O refresh for compact via async materialization. +- Fail-closed persistence through `atomicWriteFile`, preserving unrelated `auth.json` fields. +- Cross-domain convergence: native-first refresh publishes to same-grant pool rows (`publishFreshCredentialForGrant`), stored-first refresh adopts into `auth.json`. + +Why it must be redone rather than rebased: + +1. **Stale against dev.** Its `account-store.ts` hunks add the `expires_in` finite/negative guards that are *already merged* on current dev ([account-store.ts:508-520](/Users/jun/Developer/new/700_projects/opencodex/src/codex/account-store.ts:508)); GitHub reports `mergeable: CONFLICTING`. Large portions of the diff no longer apply. +2. **Semantic change smuggled in:** `saveCodexAccountCredentialIfGeneration` is rewritten so `refreshGrantFingerprint` never rotates when the refresh token rotates (“logical grant” model). This changes pool-wide invariant behavior and invalidates an existing test expectation (`tests/codex-account-store.test.ts:232` currently asserts the opposite). That deserves its own reviewed decision, not a rider on a bugfix. +3. **Lock machinery rewrite** (reclaim lock, PID liveness, abandon set, quarantine) is ~200 lines of new concurrency code attached to a bugfix; the owner review already flagged stale reclaim-lock handling. This is separable. +4. Maintainer findings stand: missing compact replay at that head was fixed later in the PR, but unsafe test-home isolation (env-var mutation of `CODEX_HOME` without process isolation) remains, plus unrelated `LEARNED_LESSONS.md`. +5. Auth surface ⇒ exact-head maintainer security review regardless; a fresh minimal diff reviews far faster than a 2k-line conflicting one. + +## 3. FILE CHANGE MAP (recommended clean implementation) + +Reuse PR #2222's *shapes*, re-derived against current dev: + +| File | Action | Symbols | +|---|---|---| +| `src/oauth/chatgpt.ts` | MODIFY | Export `CHATGPT_CLIENT_ID`, `CHATGPT_TOKEN_URL`; add exported `ChatGPTTokenResponse` + `refreshChatGPTTokenRaw(rt, {signal})` returning `{access, refresh, expires, accountId, idToken}`; refactor existing `refreshChatGPTToken` to wrap it | +| `src/codex/auth-collision.ts` | MODIFY | Add optional `refresh_token?: string` to `CodexTokens` and parse it in `readCodexTokensResult()` | +| `src/codex/main-account.ts` | MODIFY (core) | Add `mainAccessTokenFresh()`, `isMainAccountCredentialUsable()`, `getValidMainAccountToken({dependencies})`, `forceRefreshMainAccountToken(rejectedAccessToken?, {signal, dependencies})`, `NativeMainRefreshDependencies` | +| `src/codex/account-store.ts` | MODIFY (minimal) | Export `CODEX_REFRESH_SKEW_MS` alias of `REFRESH_SKEW_MS`; export `withCodexRefreshFileLock(lockKey, signal, fn)` (keep the *current* signature — do NOT port the reclaim-lock rewrite); export `findFreshCredentialForGrant` and a narrow `publishFreshCredentialForGrant` | +| `src/codex/account-usability.ts` | MODIFY | Line 37: `(options.isMainAccountTokenLive ?? isMainAccountCredentialUsable)()` + comment update | +| `src/codex/auth-context.ts` | MODIFY | Main branch (~line 515): await `getValidMainAccountToken` behind test seams (`getValidMainAccountToken`, `nativeMainRefreshDependencies` options); release probe leases in the catch like the pool branch below; add `materializeCodexUpstreamAuthAsync()` mirroring `materializeCodexUpstreamAuth` (line 619) but awaiting the refreshed main credential for `kind:"main"` substitution | +| `src/server/responses/core.ts` | MODIFY | Thread `nativeMainRefreshDependencies` through `HandleResponsesOptions`; use async materialization in `resolveResponsesCodexAuth` (~line 1494 area); add `codexMain401ReplayAttempted` + one-replay branches in both recovery loops (next to `oauth401ReplayAttempted` at lines 3096/4501), guarded by `status===401 && authCtx.kind==="main-pool" && usesCodexForwardPoolAuth(...)`; add `nativeMainRefreshFailureResponse()` (401 revoked/expired, 503 transient) | +| `src/server/responses/compact.ts` | MODIFY | Use `materializeCodexUpstreamAuthAsync` at line 385; map refresh errors before other catch arms; pass deps to `handleResponses` for the internal call | + +Core new function (literal target shape, adapted from PR #2222 minus the lock rewrite): + +```ts +// src/codex/main-account.ts (new exports) +export async function forceRefreshMainAccountToken( + rejectedAccessToken?: string, + options: { signal?: AbortSignal; dependencies?: NativeMainRefreshDependencies } = {}, +): Promise<{ accessToken: string; chatgptAccountId: string } | null> { + const initial = mainTokenFromAuthJson(); // parses tokens incl. refresh_token + if (!initial?.refreshToken) return null; + const fp = initial.refreshGrantFingerprint ?? refreshGrantFingerprintForToken(initial.refreshToken); + const signal = AbortSignal.any([options.signal ?? AbortSignal.never(), AbortSignal.timeout(30_000)]); + try { + return await withCodexRefreshFileLock(fp, signal, async () => { + const locked = mainTokenFromAuthJson(); + if (!locked?.refreshToken) return null; + if (rejectedAccessToken && locked.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(locked.accessToken)) { + return { accessToken: locked.accessToken, chatgptAccountId: locked.chatgptAccountId }; + } + const stored = findFreshCredentialForGrant(fp, MAIN_CODEX_ACCOUNT_ID); + if (stored && (!rejectedAccessToken || stored.accessToken !== rejectedAccessToken)) { + persistMainAuthJsonWith(stored); // atomicWriteFile, preserve other fields + return { accessToken: stored.accessToken, chatgptAccountId: stored.chatgptAccountId }; + } + const t = await (options.dependencies?.refreshToken ?? refreshChatGPTTokenRaw)(locked.refreshToken, { signal }); + const cred = { accessToken: t.access, refreshToken: t.refresh || locked.refreshToken, + expiresAt: t.expires, chatgptAccountId: t.accountId ?? locked.chatgptAccountId }; + persistMainAuthJsonWith(cred); + publishFreshCredentialForGrant({ refreshGrantFingerprint: fp, credential: cred, + excludeId: MAIN_CODEX_ACCOUNT_ID }); + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + return { accessToken: cred.accessToken, chatgptAccountId: cred.chatgptAccountId }; + }); + } catch (error) { + const reason = tokenRefreshReason(error); // "expired"|"revoked"|"unknown" + if (reason !== "unknown") markAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + throw error instanceof TokenRefreshError ? error : new TokenRefreshError(reason, "Codex main token refresh failed; reauthenticate the main account."); + } +} + +export async function getValidMainAccountToken( + options: { dependencies?: NativeMainRefreshDependencies } = {}, +) { + const t = mainTokenFromAuthJson(); + if (!t) return null; + if (mainAccessTokenFresh(t.accessToken)) { + return { accessToken: t.accessToken, chatgptAccountId: t.chatgptAccountId }; + } + return forceRefreshMainAccountToken(t.accessToken, options); +} +``` + +Deliberately dropped from PR #2222: the reclaim-lock/PID-liveness/quarantine rewrite of `withCodexRefreshFileLock` (keep current dev implementation), the `refreshGrantFingerprint` non-rotation change in `saveCodexAccountCredentialIfGeneration`, and `LEARNED_LESSONS.md`. Note: dropping the fingerprint-stability change means native→pool publication uses the *current* fingerprint-at-refresh-time semantics; verify `refreshGrantFingerprintForToken(rotated)` still matches pool records written by the same refresh flow (it does today because pool saves store the fingerprint of the credential they saved — confirm with the cross-domain test in §5). + +## 4. TEST PLAN + +New files, adapted from PR #2222 but with proper isolation (spawn-per-test or explicit config-dir seam instead of mutating global `CODEX_HOME`; check how existing tests isolate home — e.g. search `tests/codex-auth-context.test.ts` for the established seam and follow it): + +- `tests/codex-main-account-refresh.test.ts` + - `"keeps an expired credential selectable when its refresh grant is valid"` → `isMainAccountCredentialUsable() === true` with expired access + valid refresh (fails before fix: false). + - `"persists rotated fields atomically preserving unrelated auth fields"`. + - `"marks terminal revoked grants for reauthentication"`. + - `"serializes native refresh behind the shared grant lock"` (hold lock externally, assert zero fetch calls and timeout rejection). + - `"publishes native-first refreshes to stored accounts sharing the grant"` and `"adopts stored-first refreshes into native auth without another refresh"` — these two are the external-writer/CAS-convergence contract tests. +- `tests/responses-native-main-refresh.test.ts` — `"replays one native-main 401 with the refreshed bearer"`: local `Bun.serve` returns 401 then 200; assert `observedBearers === [stale, fresh]` (fails before fix: single stale bearer, 401 response). +- `tests/responses-compact-native-main-refresh.test.ts` — `"substitutes a refreshed native credential before compact upstream I/O"`: assert exactly one observed bearer equal to the fresh token (fails before fix: stale bearer). + +## 5. VERIFIER COMMAND + +``` +bun test tests/codex-main-account-refresh.test.ts tests/responses-native-main-refresh.test.ts tests/responses-compact-native-main-refresh.test.ts tests/codex-account-store.test.ts tests/codex-auth-context.test.ts +``` + +Yes — all five suites import the changed modules directly (`main-account.ts`, `account-store.ts`, `auth-context.ts`, `core.ts`, `compact.ts`). Before review-ready status also run `bun run typecheck` and `bun run test` (auth/account surface ⇒ full suite per AGENTS.md). + +## 6. ACTIVATION SCENARIO + +A test writes `$CODEX_HOME/auth.json` containing an access JWT whose `exp` is in the past plus a valid `refresh_token`, with zero pool accounts, and routes through `handleResponses`/`handleResponsesCompact` with an injected `nativeMainRefreshDependencies.refreshToken` stub. Observable proof the new conditional ran: (a) the upstream stub receives `Bearer ` where `` is only produced by the stubbed refresh endpoint, (b) `auth.json` on disk now contains the rotated `access_token`/`refresh_token`, (c) exactly one 401 is observed by the stub followed by success (Responses path), and (d) `isAccountNeedsReauth("__main__")` stays false on success and flips true when the stub throws `TokenRefreshError("revoked", …)`. + +## 7. RISK / BLOCKERS + +- **Writing `~/.codex/auth.json` races the Codex CLI.** Mitigation: refresh read + write inside the shared grant file-lock, atomic write, preserve unknown fields. A true multi-writer CAS (mtime/content compare before rename) beyond lock scope was demanded by the owner review; the lock covers same-machine ocx processes but not Codex CLI writers that ignore our lock. Flag this residual risk in the PR description. +- **Fingerprint semantics:** publishing native refreshes into pool rows requires stable same-grant identification across refresh-token rotation. PR #2222 solved this by freezing fingerprints; if we drop that change, the cross-domain tests above are the guard — if they fail, the fingerprint-freeze decision must be made explicitly (separate small PR) before this lands. +- **Do not port the reclaim-lock rewrite** in this change; current dev lock is adequate and battle-tested. +- Test-home isolation must use the repo's established seam, not raw `process.env.CODEX_HOME` mutation (hygiene finding against #2222 stands). +- None of this is implementable without touching auth-surface files ⇒ exact-head maintainer security review is mandatory before merge (per MAINTAINERS.md policy referenced in AGENTS.md). + + +--- + +# AMENDMENT (A-phase round 1, blockers B4+B5) + +**B4 — external-writer CAS is acceptance, not a PR note.** The body proposes flagging the +Codex-CLI-writer race "in the PR description". That is not acceptable for a credential +file another product writes concurrently: our grant lock coordinates ocx processes only, +and an atomic rename still clobbers a newer token written between our read and our +publish. Promoted into WP5 acceptance criteria: + +1. Capture file identity (`dev`, `ino`, `mtime`, `size`) **and** a content hash of + `auth.json` at read time. +2. Immediately before publication, re-stat and re-hash. On any change: re-read, and + either adopt the newer credential (if it is fresh) or refuse — never blind-overwrite. +3. Regression: mutate `auth.json` between refresh and publish, and assert the newer + writer's token survives and is the one subsequently used. + +**B5 — verifier existence gates.** Each mandatory new suite runs as its own invocation: + +```bash +test -f tests/codex-main-account-refresh.test.ts || exit 1 +bun test tests/codex-main-account-refresh.test.ts +test -f tests/responses-native-main-refresh.test.ts || exit 1 +bun test tests/responses-native-main-refresh.test.ts +test -f tests/responses-compact-native-main-refresh.test.ts || exit 1 +bun test tests/responses-compact-native-main-refresh.test.ts +bun test tests/codex-account-store.test.ts +bun test tests/codex-auth-context.test.ts +``` + +Evidence that the combined form was false-green at plan time: + +``` +$ bun test tests/codex-main-account-refresh.test.ts tests/codex-account-store.test.ts + 26 pass / 0 fail RC=0 # first file does not exist +``` + +**Security review gate.** This work-phase touches credential handling, so it is subject to +exact-head maintainer security review per AGENTS.md and MAINTAINERS.md. It does not merge +on this unit's verification alone. + diff --git a/devlog/_plan/260822_backlog_disposition_program/060_wp6_issue_1049_presubstrate_home_adoption.md b/devlog/_plan/260822_backlog_disposition_program/060_wp6_issue_1049_presubstrate_home_adoption.md new file mode 100644 index 0000000000..089f8990bc --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/060_wp6_issue_1049_presubstrate_home_adoption.md @@ -0,0 +1,93 @@ +# WP6 — #1049: adopt pre-substrate Codex homes into the write coordinator + +> Source: read-only research lane `res-1049` (model `openrouter/stealth-ox-alpha`), dispatched at work-phase 0 against `dev@ced9a85c5`. +> Every file:line pointer below was independently spot-checked by the main agent before the roadmap was locked; verification notes are appended at the end of this document. + +Research complete — all pointers verified against the tree. Here is the implementation document. + +--- + +# Issue #1049 — Adopt pre-substrate Codex homes into the write coordinator + +## 1. ROOT CAUSE + +The write coordinator covers only clean first applies and homes with an already-valid coordinator. Every install that predates the substrate keeps its old uncoordinated write path. Verified chain: + +**a. The eligibility gate decides before lock acquisition and routes legacy homes around coordination.** [src/codex/inject-coordination.ts:54](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject-coordination.ts:54), returning at [src/codex/inject-coordination.ts:120](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject-coordination.ts:120): + +```ts +return { + kind: "legacy-uncoordinated", + reason: coordinatorIsStableZeroByte + ? "the coordinator is a zero-byte non-authoritative remnant ..." + : residue.kind === "residue" + ? "this home was routed before write coordination existed and has not been adopted yet" + : "the existing native Codex state could not be classified, ...", +}; +``` + +Its own docstring ([lines 21–34](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject-coordination.ts:21)) calls this "a temporary boundary, not a design." + +**b. Both production callers branch on it.** [src/codex/inject.ts:891](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject.ts:891) (`injectCodexConfig`) and [src/codex/inject.ts:1512](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject.ts:1512) (`restoreNativeCodexAsync`); the apply side writes via `applyNativeArtifacts()` unconditionally when `legacy-uncoordinated` ([inject.ts:936–944](/Users/jun/Developer/new/700_projects/opencodex/src/codex/inject.ts:936)), never entering `withCodexWriteLock`. + +**c. Why adoption cannot simply turn the gate off.** `assertInitialStateCanBeCreated` at [src/codex/transition-state.ts:268–280](/Users/jun/Developer/new/700_projects/opencodex/src/codex/transition-state.ts:268): + +```ts +if (classifyNativeRoutedResidue().kind !== "clean") { + throw new CodexCoordinatorLegacyAmbiguousError( + "A missing coordinator row cannot be initialized while native Codex routing residue exists.", + ); +} +``` + +Installing `{0, null}` over routed bytes would erase the evidence of an interrupted transition. The refusal is correct; the missing piece is a *different* row identity (`adoption-pending`), not a relaxation. + +**d. None of the adoption machinery exists in `src/`.** `rg 'adoption-pending' src/ tests/` → zero matches (verified). It exists only as a spec in `devlog/_fin/260804_codex_write_substrate/005_contract.md` (WP10, lines ~706–790; fixtures at :2241–2290). + +**e. Current create path is unsafe for adoption-grade publication.** [transition-state.ts:378–382](/Users/jun/Developer/new/700_projects/opencodex/src/codex/transition-state.ts:378): `database = new Database(finalDatabasePath, { create: true }); if (databaseWasAbsent) { try { chmodSync(finalDatabasePath, 0o600); } ... }` — exactly what the contract forbids ("never opens a missing final path with SQLite create:true", contract line ~708). + +## 2. FILE CHANGE MAP + +The maintainer triage comment (verified against the tree) is explicit that this is **not one diff**: it requires a temp-database publisher + no-clobber publication phase first, then the adoption mode, then positive-authority handoff plumbing. No such primitives exist today — there are no no-clobber link/rename or publication-fsync helpers anywhere in `src/codex/`, and `history-job.ts` has no retained-callback authority plumbing. The honest change map therefore names the required new units rather than pretending copy-paste hunks exist: + +| File | Change | What | +|---|---|---| +| `src/codex/coordinator-publish.ts` | NEW | Complete-v1-temp-database publisher: unique mode-0600 temp in final dir, full schema + singleton committed there, bytes fsynced, atomic no-clobber publish (same-dir exclusive hard link or rename-without-replace; ordinary replace forbidden; EEXIST = lost race → strict existing path after scrubbing own temp), parent-dir fsync after success | +| `src/codex/transition-state.ts` | MODIFY | (Phase A) Replace the `create:true`+chmod open path for absent databases with the publisher, so every clean create is crash-safe too; add `'adoption-pending'` to `DURABLE_HISTORY_STATUSES` and to the `CREATE_TRANSITION_TABLE` CHECK constraints; add WP10 compatibility-row initializer producing exactly the identity specified at contract :727–737 (`native_generation=0, current_tx_id=NULL, history_status='adoption-pending'`, fresh non-empty history_tx_id, intent-derived operation, `authority_kind='wp10-compatibility'`, opaque authority id) | +| `src/codex/inject-coordination.ts` | MODIFY | Narrow `legacy-uncoordinated`: when residue kind is `"residue"` and integration record is missing-or-valid, return a new `{kind:"adopt"}` eligibility instead; indeterminate residue, invalid record, unversioned/rowless DB still refuse (the latter two already do via `initialize()` guards at [transition-state.ts:287–299](/Users/jun/Developer/new/700_projects/opencodex/src/codex/transition-state.ts:287)) | +| `src/codex/inject.ts` | MODIFY | In both call sites, route `kind:"adopt"` through `withCodexWriteLock`; inside the lock, publish the adoption-pending row before invoking `applyNativeArtifacts()` / the restore callback, per the contract's ordering (publish → native callback → conditional transition to pending schedule) | +| `src/codex/history-job.ts` | MODIFY | Positive-authority consumption: accept retained high-level callback, closed intent (`retained-apply` with exact op set / `retained-restore`), consume transaction-bound authorizer exactly once; refuse to dispatch from `adoption-pending` without it | + +Literal copy-paste hunks cannot be supplied because the publisher module (~200–300 lines including kill-boundary seams) does not exist; writing it here would be fabrication, not research. The executing agent should treat the contract sections quoted above as the literal specification. + +## 3. TEST PLAN + +Per contract :2241–2290 plus the maintainer's named cases: + +- **New** `tests/codex-coordinator-adoption.test.ts`: + - `routed config with no coordinator adopts via adoption-pending then applies under the lock` — seed routed config/catalog/history with NO coordinator db; run real `injectCodexConfig`; assert final row exists with `history_status='adoption-pending'` published *before* any artifact mutation (sentinel ordering), then transitions to pending schedule. Fails today (no adoption; inject returns `legacy-uncoordinated` path). + - `indeterminate residue refuses adoption and creates no row` + - `invalid or legacy integration record refuses adoption` + - `existing unversioned or rowless coordinator database refuses and is never adopted` (guards already exist at transition-state.ts:287–299; test pins them against the new adopt path) + - `process death mid-adoption leaves recoverable state, never poisoned` (kill checkpoints per contract) +- **Modify** `tests/codex-transition-state.test.ts`: cross-process no-clobber race — two contestants build valid temps; exactly one publishes; loser gets EEXIST, scrubs only its temp, opens winner as ready. +- **Existing** `tests/codex-coordinator-doctor.test.ts:166–205`: its `legacy-uncoordinated` expectations must be updated to the narrowed boundary (they currently pin the behavior this issue removes). + +## 4. VERIFIER COMMAND + +``` +bun test tests/codex-coordinator-adoption.test.ts tests/codex-transition-state.test.ts tests/codex-coordinator-doctor.test.ts +``` + +All three read the changed files: the first exercises `inject-coordination.ts` + `inject.ts` + the new publisher via the real inject path; the second exercises `transition-state.ts` directly; the third pins the eligibility function. + +## 5. ACTIVATION SCENARIO + +A test seeds a temporary home with routed config/catalog/history (residue classifier returns `residue`) and **no** coordinator database, then calls the real `injectCodexConfig`. Before this issue's fix, `codexWriteCoordinationEligibility` returns `legacy-uncoordinated`, the code skips `withCodexWriteLock` entirely, and no coordinator file ever appears — observable by asserting the coordinator path stays absent and no lock was taken. After adoption lands, the same input returns `adopt`, acquires N, publishes a validated `adoption-pending` database at the final path *before* the native callback mutates artifacts, and the observable proof is (a) the coordinator database now exists with the exact WP10 singleton identity, and (b) a sentinel installed in the native-write path fires strictly after publication but before artifact replacement. + +## 6. RISK / BLOCKERS + +- **Not implementable as a single copy-paste diff.** The maintainer's assessment (comment verified against tree: `grep -r 'adoption-pending' src/` = 0 matches) stands: the prerequisite temp-publisher/no-clobber phase rewrites the create path used by *every* clean install, so its blast radius is the whole installed base. It wants its own reviewable phase, then adoption, then authority plumbing. +- **Crash-safety surface:** publication is the crash boundary; partial implementations risk corrupted user installs, which is why the maintainer deferred rather than half-shipped. +- **#1798/#1802 remain separate acceptance cases** even after this lands (per owner comment); closing them here would be wrong scope. +- The `legacy-uncoordinated` tests in `tests/codex-coordinator-doctor.test.ts:166–205` will need updating in the same change — they currently pin the exact boundary being removed. diff --git a/devlog/_plan/260822_backlog_disposition_program/070_wp7_bun14_memory_stack_retarget.md b/devlog/_plan/260822_backlog_disposition_program/070_wp7_bun14_memory_stack_retarget.md new file mode 100644 index 0000000000..24e85a1677 --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/070_wp7_bun14_memory_stack_retarget.md @@ -0,0 +1,89 @@ +# 070 — WP7: Bun 1.4 memory stack retarget + +Four maintainer-authored PRs forming a stack. The user's instruction is explicit: +**retarget rather than abandon**. This document records the per-PR decision and the +evidence behind it. + +## Stack shape as opened + +``` +dev + └── #2301 codex/bun14-followup-memory-docs (devlog only, 612+/0-, 10 files) + ├── #2302 codex/bun14-mem-diagnostics (runtime, 56+/5-, 4 files) + │ └── #2303 codex/bun14-gc-relief-eval (harness, 658+/0-, 5 files) + └── #2304 codex/bun14-smol-ab (harness, 147+/0-, 2 files) +``` + +Only #2301 targets `dev`; the other three target stack-internal branches, which is why +`dev` Cross-platform CI never ran on the runtime diff. #2302's CI shows `ci` and +`macos` FAIL. + +## The two experimental verdicts are FAIL, and that is the deliverable + +- **#2304 (smol workers): FAIL.** Median peak RSS 447,758,336 B off vs 447,807,488 B on + across 3 runs/arm on Bun 1.4.0 darwin/arm64 — no reduction. Per the audited + pre-landing gate, **no production `smol` flags were landed.** The harness plus the + recorded verdict is the deliverable. +- **#2303 (Bun.gc relief): production hook NOT added.** Phase A is measurement only. + +A FAIL verdict recorded with its evidence is a legitimate outcome, not wasted work — it +is what stops the next person re-running the same experiment. That argues for landing +the *records*, not for closing the PRs silently. + +## Recorded blockers (reviewer `Ingwannu`, exact-head reviews) + +### #2301 — docs parent +1. `000_research.md` dated `2026-08-22` while the review was written 2026-08-21, so a + future date was presented as completed current evidence. + **Status at this unit: MOOT.** Today *is* 2026-08-22, so the date is now simply + correct. The "today" present-tense claim no longer misrepresents anything. This must + be stated explicitly in the merge note rather than silently ignored. +2. `git diff --check origin/dev...HEAD` fails on all ten added Markdown files — extra + blank line at EOF. **Still live**, mechanical, must be fixed. + +### #2302 — runtime diagnostics +1. Targets the docs branch, so no `dev` code CI. Must be retargeted/rebased onto current + `dev` for exact-head CI. Merging it through the docs parent would smuggle a runtime + diff into `dev` without the code gate. **This is the core retarget instruction.** +2. `src/server/management/system-routes.ts` converts a missing/non-numeric + `heapStats().extraMemorySize` into `0`, while watchdog and doctor types correctly + treat it as optional. **"Unavailable" is not the same measurement as zero** — a real + correctness defect in an observability feature. + +### #2303 — GC relief harness (re-reviewed, CHANGES_REQUESTED sustained) +1. Records `rssAfterLoad`/`rssPlus5s`/`rssPlus60s` but **no pre-load baseline**. The + controlling gate is "at least 50% of post-load RSS *growth* is gone", which needs + `rssBeforeLoad`, `postLoadGrowth`, and `recoveryFraction`. The revised verdict divided + recovered bytes by total post-load RSS ("<0.1% of load-height RSS"), which is not the + controlling criterion. The data may well reach the same FAIL, but the report cannot + *prove* the gate without the baseline. +2. Latency cells still serial. +3. Never calls `/api/system/memory`, so the stated `extraMemorySize` dependency on #2302 + is unused. + +### #2304 — smol A/B harness +1. Future-dated measurement record. **Now moot** (see #2301). +2. `payloadMb` and `runs` accept zero, negative, non-numeric, and arbitrarily large + values. `runs=0` reaches `median([])` and writes a structurally incomplete gate; a + huge payload can exhaust the host. Needs finite-positive-integer validation. + +## Disposition + +| PR | Decision | Rationale | +|----|----------|-----------| +| #2301 | **REBUILD → merge** | Blocker 1 moot by date; blocker 2 is a whitespace fix. The research ledger and decade docs are the durable value. | +| #2302 | **RETARGET onto `dev` + fix** | Exactly the user's instruction. Fix the `0`-vs-unavailable defect, retarget to `dev`, obtain real exact-head CI. | +| #2303 | **REBUILD** | Add `rssBeforeLoad`/`postLoadGrowth`/`recoveryFraction` so the harness can prove the gate it cites. Keep production GC hook out. | +| #2304 | **REBUILD → merge** | Add input validation; the FAIL verdict itself is sound and stands. | + +No PR in this stack is closed. The FAIL verdicts are preserved verbatim — a negative +result with evidence is the point of the unit. + +## Accept criteria + +1. #2302 targets `dev` and has exact-head CI, not stack-internal CI. +2. `extraMemorySize` absence is representable as absent, not `0`. +3. #2303's harness records a pre-load baseline and a baseline-relative recovery fraction. +4. #2304 rejects non-finite / non-positive `runs` and `payloadMb`. +5. No production `Bun.gc(true)` call and no production `smol: true` flag is landed. + diff --git a/devlog/_plan/260822_backlog_disposition_program/080_wp8_conflicting_and_remaining.md b/devlog/_plan/260822_backlog_disposition_program/080_wp8_conflicting_and_remaining.md new file mode 100644 index 0000000000..b5e853557e --- /dev/null +++ b/devlog/_plan/260822_backlog_disposition_program/080_wp8_conflicting_and_remaining.md @@ -0,0 +1,63 @@ +# 080 — WP8: conflicting and remaining PR disposition + +The tail of the backlog: ten PRs conflicting with `dev`, four drafts outside the other +lanes, and one on the wrong base. This phase runs last because several items are +superseded by whatever WP1–WP7 land. + +## Wrong base — immediate + +### #2357 — `[WRONG BRANCH] Add __omit__ reasoning-effort wire sentinel` +Base `main`, draft, `enforce-target` failing twice. The author labelled it themselves. +Addresses #2356 (ollama ≥0.32 rejecting high reasoning efforts). + +Disposition: **CLOSE** with a reason directing the author to reopen against `dev`. The +underlying issue #2356 stays open with its triage intact. Closing is correct here — the +branch policy forbids feature PRs against `main`, and a retarget by a maintainer would +rewrite a contributor's PR base without their involvement. + +## Conflicting with `dev` (10) + +| PR | Size | Note | +|----|------|------| +| #2280 | 556+/15- | per-model synthetic max suppression; interacts with #2279 | +| #2230 | 1637+/61- | Gemini OAuth accounts; hygiene-blocked | +| #2222 | 1390+/168- | **superseded by WP5** — see `050` for the verdict | +| #2213 | 494+/101- | Grok direct-first tool projection | +| #2069 | 1650+/41- | antigravity account cooldowns; hygiene-blocked | +| #2041 | 26+/1- | auto_review_model override (tiny, addresses #1225) | +| #1794 | 1759+/8- | routed V2 subagents + OpenRouter endpoints | +| #1704 | 181+/7- | maintainer-authored combo quota GUI | +| #1645 | 1425+/151- | vision sidecars; likely superseded by the landed #2188 chain | +| #1557 | 2545+/69- | least-privilege data-plane catalog endpoint (#809) | + +**Standing instruction from the maintainer triage, which governs this phase:** when a +conflicting PR overlaps the `types.ts`/`config.ts` split, *do not rebase — close and +reopen*. Rebasing a large branch across a file split produces a diff no reviewer can +audit. Each PR here is checked against that rule before any conflict resolution is +attempted. + +#2222 is the clearest case: WP5 (`050`) already recorded that its approach is right but +its diff is stale, it smuggles a `refreshGrantFingerprint` semantic change that +contradicts an existing test, and it attaches a ~200-line lock rewrite to a bugfix. +Its disposition is CLOSE-as-superseded once WP5 lands, crediting the author. + +## Drafts outside other lanes (4) + +| PR | State | Note | +|----|-------|------| +| #2352 | draft, mergeable | native lifecycle after ownership reprobe; 860+/59- | +| #2326 | draft, `enforce-target` fail | GUI frontier shortcuts; needs a screenshot per the PR template | +| #2083 | draft, **APPROVED** | xAI Imagine image relay — approved but never marked ready | +| #2033 | draft, tiny | expose web-search sidecar enabled status, 14+/0- | + +#2083 is notable: it carries an APPROVED review and is mergeable, but sits in draft. It +needs only a ready-for-review transition and exact-head verification. + +## Accept criteria + +1. Every open PR not disposed by WP1–WP7 has a recorded terminal disposition here. +2. No large conflicting branch is force-rebased across the `types.ts`/`config.ts` split. +3. Closures name the reason and, where the work was sound, credit the author and point + at the superseding change. +4. Final `gh pr list` count reconciles against the `000` inventory of 45. +