devlog: open the Windows stability program unit

devlog/_plan/260817_windows_stability_program/000_problem_model.md

@@ -0,0 +1,68 @@
+# 000 — Windows stability: why "806/806 green" is not "stable"
+
+Unit opened 2026-08-17, after v2.24.2 shipped.
+
+## The gap this unit exists to close
+
+The Windows campaign that preceded this unit took the local Bun suite from 53+
+failures to 806/806 across 15 commits. That was real work on real defects — an
+empty-string `LocalApplicationData`, unfinalized SQLite statements holding a
+file open against unlink, TOML escapes doubling backslashes, per-process
+identity lookups costing ~510ms each.
+
+None of it proves the product is stable for a Windows user, and the reason is
+structural rather than rhetorical: **the suite that went green is not a gate.**
+
+```yaml
+# .github/workflows/ci.yml:547-552
+platform-windows:
+  name: windows ${{ matrix.shard }}/4
+  needs: select-windows-runner
+  if: github.event_name == 'workflow_dispatch'
+```
+
+Windows runs only when a maintainer asks by hand. The aggregation job at
+`.github/workflows/ci.yml:747-783` accepts `skipped` as an outcome, and
+`.github/workflows/release.yml:181-201` requires a successful **push-event**
+CI run before publishing. Since `platform-windows` always skips on push, a
+release satisfies its own gate having executed zero Windows tests.
+
+Issue #1059 tracks exactly this and is still open. Its stated end condition is
+Windows restored as a required gate. The failure counts quoted there are now
+stale in our favour; the workflow contract has not caught up.
+
+## Evidence base for this unit
+
+Three independent GPT-5 Pro audits were run on 2026-08-17 against a zip of the
+v2.24.2 tree (`src/`, `tests/`, `scripts/`, `.github/`, `structure/`), each
+with the GitHub connector attached and a distinct brief:
+
+| Chat | Perspective | Conversation |
+|---|---|---|
+| P1 | Platform primitives: handles, locking, atomic publication, paths, ACLs | `chatgpt.com/c/6a82ebc4-48d4-83ee-a223-a6fc5a9556e5` |
+| P2 | Runtime and distribution: install, spawn, service lifecycle, update, ports | `chatgpt.com/c/6a82ec28-86b4-83e8-86e4-a5477b6a9d91` |
+| P3 | User-visible failure modes, diagnostics, and CI coverage | `chatgpt.com/c/6a82ec41-6b0c-83ee-93ab-3a96010a543f` |
+
+Every finding carried into `001` was **re-verified against the working tree in
+this session**. Claims that could not be reproduced locally were dropped rather
+than recorded. That rule matters here because two of the three audits also
+correctly identified defects as *already fixed* (#1843 elevation argv, #31
+passthrough segfault) — an audit that cannot tell live from historical is not
+usable as a roadmap input.
+
+## What changed in the problem model
+
+The pre-campaign model was "Windows has many small filesystem bugs." The
+evidence no longer supports that as the dominant class. The surviving defects
+cluster into three shapes:
+
+1. **Synchronous Windows subprocesses on the request path.** `icacls` and
+   PowerShell/CIM calls that block Bun's event loop. This is invisible to a
+   test suite that never measures latency under concurrency.
+2. **Lifecycle operations that are not transactional.** Update and native
+   service migration both destroy working state before proving the replacement.
+3. **Invariants enforced by prose or by a single-file test, so they drift.**
+   The `-WindowStyle Hidden` case in `001` is the clearest example.
+
+None of those three are things a per-file unit test naturally catches, which is
+why 806 green files and an unhappy user base are consistent with each other.

devlog/_plan/260817_windows_stability_program/001_verified_findings.md

@@ -0,0 +1,205 @@
+# 001 — Verified findings
+
+Every entry below was reproduced against the working tree at `474584bcd` on
+2026-08-17. Line numbers are from that tree. Findings the audits raised that
+could not be reproduced are listed at the bottom under "Not carried".
+
+Ranked by user impact.
+
+---
+
+## F1 — `src/service.ts:2361` uses the exact PowerShell argv the codebase forbids
+
+`killWindowsServiceWrapperProcesses()` in `src/service.ts` spawns:
+
+```ts
+// src/service.ts:2360-2363
+spawnSync(resolveTrustedWindowsPowerShellExe(), [
+  "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden",
+  "-Command", ps,
+], { stdio: "ignore", timeout: 5000, windowsHide: true });
+```
+
+The codebase already knows this is wrong. `src/codex/user-identity.ts:222-224`:
+
+> Do not add PowerShell's `-WindowStyle Hidden` here: Bun 1.3.14 can fail that
+> direct CLI combination before the SID command executes (#1589); the
+> process-level `windowsHide` flag is sufficient.
+
+**Why it survived.** The regression test is scoped to one file:
+
+```ts
+// tests/windows-deploy-close-regressions.test.ts:43
+expect(src).not.toContain('["-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", ps]');
+```
+
+`src` there is `read("src/update/job.ts")` (line 13). `src/service.ts` is never
+checked. A search of `src/` finds exactly one surviving production occurrence
+of that CLI pair: `src/service.ts:2361`.
+
+**User-visible consequence.** `stopServiceIfInstalled()` calls this function
+because `schtasks /end` can leave the `wscript.exe`/`cmd.exe` wrapper alive,
+which then respawns the proxy. The call ignores `spawnSync`'s exit status and
+swallows errors, so under #1589 wrapper termination silently does nothing:
+`ocx stop`, restart, and update appear to succeed and do not stick.
+
+Severity: high. Fix cost: one line. Phase 010.
+
+---
+
+## F2 — The wrapper killer exists twice and the copies have drifted apart
+
+Two implementations of the same operation:
+
+```ts
+// src/service.ts:2337-2358 — canonical token matching scoped to THIS home
+//   (paths built 2340-2341; token boundaries enforced 2350-2355)
+// src/update/job.ts:1377-1383 (the bare -like match is line 1383)
+"$pats = @('opencodex-service.cmd','opencodex-service-launcher.vbs');"
+...
+"foreach ($p in $pats) { if ($c -like ('*' + $p + '*')) { return $true } };"
+```
+
+The updater copy matches a bare filename anywhere in a command line. Two
+OpenCodex homes under one Windows account means a dashboard update for home A
+can terminate home B's scheduler wrapper. Any unrelated process whose command
+line contains either filename also matches.
+
+Cited precisely: the updater's bare match is `src/update/job.ts:1383`; the service copy builds canonical paths at `src/service.ts:2340-2341` and enforces token boundaries at `:2350-2355`.
+
+The drift is already measurable and runs in both directions: `update/job.ts`
+received the #1589 argv cleanup that `service.ts` missed (F1); `service.ts`
+received canonical path scoping that `update/job.ts` missed. Two copies, two
+different half-fixes.
+
+Severity: high (cross-installation process kill). Phase 020.
+
+---
+
+## F3 — Windows is not a gate, and the release gate cannot see that
+
+```yaml
+# .github/workflows/ci.yml:547-552
+if: github.event_name == 'workflow_dispatch'
+```
+
+The aggregation job accepts `skipped` (`ci.yml:769-772` — the jq filter keeps
+only jobs that are neither `success` nor `skipped`). The release preflight
+(`release.yml:181-201`) demands a successful **push-event** `ci.yml` run —
+deliberately narrower than "any successful run for this SHA" — but
+`platform-windows` never runs on push. So the general release preflight does not require `platform-windows`, and a
+release can publish without it having run.
+
+One qualification, because the stronger claim is not true: releases that touch
+`src/service.ts`, `src/cli/index.ts`, `package.json` and a few others separately
+require a green `service-lifecycle.yml` (`release.yml:224-241`, enforced at
+235-239), and that
+workflow does include a Windows job. Windows is therefore not entirely absent
+from release gating - it is absent from the *suite* gate, and present only as a
+lifecycle smoke test for service-shaped changes.
+
+Severity: high, and it is the multiplier on every other finding — without it,
+each fix below is one careless merge away from regressing. Phases 060 and 070.
+
+---
+
+## F4 — Durable publishers do not share the Windows retry primitive
+
+`src/config.ts:102-123` knows about Windows sharing violations:
+
+```ts
+const transientWindowsError = io.platform === "win32"
+  && (code === "EBUSY" || code === "EPERM" || code === "EACCES");
+if (!transientWindowsError || attempt >= 2) throw error;
+io.sleep(25 * (attempt + 1));
+```
+
+Two retries, 25ms then 50ms: about 75ms of total tolerance. Other durable
+publishers do not call it at all and use raw `renameSync`:
+
+- `src/codex/prompt-journal.ts` — publishes a journal holding full
+  `config.toml` bytes
+- `src/lib/config-ownership.ts` — publishes the uninstall ownership manifest
+
+These are fail-safe, not corrupting: they throw rather than publish a partial
+file. But under a real-time scanner or a sync client holding the target, they
+turn a recoverable hiccup into a user-visible operational failure.
+
+The 75ms envelope is itself a watch item, not yet a defect — we have no field
+telemetry showing Defender or OneDrive holding files longer. Instrument before
+widening. Phase 030 makes the primitive shared; Phase 031 adds the counters.
+
+---
+
+## F5 — `chmod` is load-bearing where it does nothing
+
+`src/config.ts` calls `chmodSync(target, 0o600)` at lines 221, 316, 450, 1713,
+2683 and 3942, and `chmodSync(dir, 0o700)` at 1704, 2632, each wrapped in
+`catch { /* platform may ignore chmod */ }`. The 3942 site sits inside
+`backupInvalidConfig` (declared at 3937), which copies the whole config
+including whatever secrets it held. On Windows the call is a no-op:
+the ACL is what protects the file, and `src/lib/windows-secret-acl.ts` is what
+sets it.
+
+Where both run, the file is protected. The audit work needed here is an
+inventory: every path that writes a credential, token, or OAuth refresh token,
+and whether the Windows ACL path is reached on that specific write or only the
+`chmod`. `src/service.ts:1983` states the ACL is authoritative, but says so about an
+elevation staging directory specifically. That is evidence for the principle,
+not evidence about any credential writer's coverage - each inventory row needs
+its own citation.
+
+Treated as **unproven** until the inventory is done. Phase 040. Per AGENTS.md,
+if that inventory turns up a live exposure the writeup goes to scratch space,
+not into this directory.
+
+---
+
+## F6 — The service wrapper retries a deterministic crash forever
+
+```bat
+:: src/service.ts:1556-1563
+"%OCX_BUN%" "%OCX_CLI%" start ...
+if %ERRORLEVEL% NEQ 0 (
+  ... restarting in 5s
+  ping -n 6 127.0.0.1 >nul
+  goto loop
+)
+```
+
+A proxy that starts successfully and then crashes deterministically is
+relaunched every five seconds indefinitely. #1877 deliberately fixed only the
+missing-executable case, on the reasoning that a flat "N failures then stop"
+ceiling would break recovery from intermittent faults. That reasoning is sound;
+the conclusion does not have to be an unbounded fixed-interval loop.
+
+Capped exponential backoff with a health-reset — 5s, 15s, 30s, 60s, reset after
+sustained uptime — preserves recovery and stops the log storm. Phase 050.
+
+---
+
+## F7 — Windows CI never proves crash-restart
+
+`.github/workflows/service-lifecycle.yml:104-135` kills the systemd MainPID,
+waits for a different PID, and asserts `/healthz`. The Windows job
+(`windows-schtasks`, line 239) only covers install, health, clean `ocx stop`,
+uninstall. The restart path F6 describes has no coverage on the platform where
+it is implemented in batch. Phase 051.
+
+---
+
+## Not carried
+
+Raised by the audits, deliberately excluded:
+
+- **#1843 elevated `Start-Process` argv** — already fixed; PR #1860 merged and
+  present in the tree.
+- **#31 passthrough SSE segfault** — fixed via `body.tee()`.
+- **Bun replacing its own running executable during update** —
+  `src/update/index.ts:152-155` documents that the plain-Node launcher handles
+  npm self-update before Bun starts.
+- **Synchronous `icacls`/CIM on the request path (#1852, #1298; PR #1876)** —
+  both P1 and P3 rate this their top runtime issue and the reasoning is
+  persuasive, but it is a latency property this session did not measure. It
+  belongs to the open PR, not to this unit. Recorded here so the next cycle
+  starts from it rather than rediscovering it.

devlog/_plan/260817_windows_stability_program/002_sequencing.md

@@ -0,0 +1,94 @@
+# 002 — Sequencing and what this unit deliberately does not do
+
+The first draft of this document claimed a long dependency chain. A plan audit
+(round `r1-20260817113441`) showed most of it was file-overlap dressed up as
+dependency, and one link was backwards. This is the corrected version; the
+reasoning is kept because "why we thought these were dependencies" is the more
+useful record.
+
+## Real dependencies
+
+Only two links are structural:
+
+```mermaid
+graph LR
+  A["030 shared replace primitive"] --> B["031 retry telemetry"]
+  C["060 stage 1 - run non-gating"] --> D["070 flakiness policy"]
+```
+
+`030 → 031` because there is nothing to instrument until the primitive exists.
+`060 stage 1 → 070` because the flakiness policy is calibrated on the failure
+data stage 1 produces.
+
+Everything else is schedulable now.
+
+## Start immediately, in parallel
+
+- **060 stage 1** — highest priority despite its number. It only makes Windows
+  *run*; it blocks nothing, and every later phase wants its data. Its one
+  prerequisite is the runner-policy decision inside 060, which is a decision to
+  make rather than work to schedule.
+- **010** — one line plus a widened guard.
+- **051** — crash-restart already exists, so it is testable today. Landing it
+  before 050 gives the timing change a baseline.
+- **040** — independent inventory, produces a document.
+
+## Ordering preferences that are not dependencies
+
+Stated so nobody mistakes them for blockers:
+
+- **010 before 020** was originally justified as "otherwise the fix is written
+  twice". That is wrong: deduplicating first moves one flawed implementation,
+  and 010 then fixes it once. Either order works. Prefer 010 first only because
+  it is trivial and unblocks nothing else.
+- **020 before 030** is people-not-colliding in `service.ts` and `job.ts`.
+- **010/020 before 050** is the same, all three touch `src/service.ts`.
+- **050 before 051** was fake, and worse, it produced an impossible verification
+  claim — 051 now says plainly that it cannot verify 050's backoff.
+- **"everything before 060"** was false. None of F1, F2, F4, F5 or F6 makes the
+  suite red today. What is true is narrower: **060 stages 3 and 4** should wait
+  for the fixes, because that is when a Windows failure starts costing someone
+  a merge or a release.
+- **080** is simplest to add once 060 stage 1 has a Windows leg running, but it
+  is not blocked by it; it starts non-gating and does not wait for stage 3.
+
+Each phase header states its own dependency line. Where a header says "sequence
+around" another phase, that is collision avoidance in shared files — `002` is
+authoritative on what is structural, and only the two links above are.
+
+## Out of scope for this unit
+
+**The synchronous-subprocess latency class.** Both P1 and P3 rank
+`icacls`/PowerShell-CIM on the request path as the top runtime problem
+(#1852, #1298, PR #1876). It is excluded because this session measured nothing —
+no latency numbers, no event-loop traces. Carrying it would put an unverified
+claim beside seven verified ones and devalue all of them.
+
+The audit accepted that exclusion as honest and then made the sharper point:
+because `000` itself names this the leading runtime class, finishing this unit
+**cannot** establish "Windows is stable". It establishes a reliability and CI
+baseline while the highest-ranked risk stays open in #1876. That is the accurate
+claim and the one to make in any release note.
+
+**Update transactionality.** #1849 is open and the design work — stage outside
+the live tree, verify, switch, retire the backup — is larger than any phase
+here. Separate unit.
+
+**Branch protection.** 060 cannot make Windows block a merge; `dev` has no
+protection and `MAINTAINERS.md:121` and `:125` record that enforcing anything
+that way is an unmade decision. Configuring it is a maintainer call, not a
+phase.
+
+## Definition of done for the unit
+
+- 010, 020, 030, 031, 050, 051 landed, each guard driven red before it counts.
+- 060 through stage 4, so a release preflight cannot pass on a push run where
+  Windows silently skipped.
+- 060's runner policy explicitly resolved rather than left implicit.
+- 070's nightly running, quarantine list open and reviewed each release.
+- 080 items 1-5 landed; items 6 and 7 landed or documented as not achievable.
+- 040's inventory complete, with any live exposure handled entirely in scratch
+  per AGENTS.md and nothing about it written here.
+
+Until 060 stage 4 is done, every fix in this unit is one careless merge from
+regressing. That is the point of the unit.