docs(devlog): close wp2 planning after four audit rounds and record the wp3 review (#3036)

wp2 (#3023) is planned to an implementable state and closed as a planning cycle. It
does not claim an implementation, because there is none; that is registered as wp6.

Four audit rounds, four correctness holes, all in the same place: what a
deduplicated ensure operation is allowed to answer for. The flight key grew one term
per round, each because a reviewer produced a concrete cross-answering sequence.

Round 8's is the one worth remembering. Candidate set, client version, mutation epoch
and identities can all be unchanged while an entry expires mid-flight, so a second
caller computes an identical key, joins, and the resolver's now is fixed from the
flight's start, meaning the account it came to refresh stays a cache hit. ocx export,
the surface #3023 actually reported, then returns short rows having refreshed nothing.

Final key: (candidate set, client version, mutation epoch, identity vector, workset).
The identity vector exists because a mutation epoch cannot fence external auth.json
writers, so a caller holding a new identity could otherwise join an old-identity
flight.

Also corrected: the negative-memo TTL is pinned at 5000 ms, its publication is fenced
on the captured identity vector, and expiry is measured from the absence observation
rather than settlement, since a flight that spent 30s in a credential refresh must not
hand out evidence treated as 5s fresh. Three regressions the draft claimed as red are
already green and no longer claimed. "Zero credential validation" is restated as zero
token refresh and zero network, because a pure cache read needs a cross-process
credential-generation signal that does not exist.

Wait policy settled: one whole-ensure flight, caught fail-closed and never aborted by
a management timeout, sidecar joining at 0 ms, management surfaces waiting up to
3000 ms from the flight's start. MODEL_ROSTER_TIMEOUT_MS is not a total bound.

Records the wp1 shipped outcome (#3035, merged as 4bdc0f6) and the wp3 review, which
returned FAIL: the drain, budget split and ordering are right, but supersession reached
the state tracking and not the writer, so an abandoned writer could still publish to
the filesystem and orphan a temp. Still in repair, not part of this change.

No source change.

Author: lidge-jun

devlog/_plan/260831_prio70_entitlement_and_spill_train/008_audit_round5_wp2_plan.md

@@ -0,0 +1,86 @@
+# 008 — audit round 5: wp2 plan re-audit after wp1 landed
+
+Auditor: sol-high subagent, read-only, run against `codex/prio70-train-260831` at
+`1b6b36b96` (wp1 merged into the branch). Verdict **FAIL** — five substantive
+corrections plus a stale-citation sweep. All are folded into `020`.
+
+## Findings
+
+1. **Citations shifted by wp1** (+45 lines or so in `model-entitlements.ts`).
+   `:223` → `:323-325`; `:455` → `:524-526`, `:542-552`; `:500-506` → `:556-563`;
+   `:539-550` → `:391-419`, `:592-602`; `:630` → `:686-693`; expiry `:509` → `:673`.
+   Also `auth-api.ts:2015` → `:2016`, `server/index.ts:1155` → `:1158-1164`,
+   `export-command.ts:169` → `:181`, `sidecar/candidates.ts:29` → `:35-40`.
+   Still valid: `account-store.ts:131`, `model-rows.ts:50`,
+   `model-routes.ts:352-354`, `Models.tsx:402-417`.
+
+2. **"Zero credential validation in the steady state" is unachievable as written.**
+   The cache key is `(accountId, clientVersion)` (`:323-325`), but freshness also
+   compares `credentialIdentity` (`:517-522`), and obtaining the current identity
+   reads `auth.json` or the account store (`:378-389`). A pure cache read would need
+   a cross-process credential-generation signal that does not exist. Redefined
+   target: **zero token refresh and zero network**, via an identity-only read, with
+   the full `accountCredentialSnapshot` (`:391-419`) reserved for entries that are
+   missing, stale, or identity-mismatched.
+
+3. **The invalidation hook list was incomplete**, and one entry was wrong.
+   `auth-api.ts:2015` is the branch, not the save (`:2016`), and hooking it misses
+   new-account login at `:546`. Also required: pool CAS refresh
+   (`account-store.ts:195-218`, called at `:705`, `:790`), owner/alias refresh
+   (`:252-309`, called at `:858`), deletion/tombstone (`:312-320` via
+   `account-lifecycle.ts:158`), and main-token refresh
+   (`main-account.ts:133-163`, `:219`). External replacement of
+   `~/.codex/auth.json` cannot be hooked at all, so a bounded identity probe has to
+   remain. Active-account switch (`auth-api.ts:1630-1641`) is config-only and needs
+   no clear. One cycle-free mutation epoch at every successful commit and tombstone,
+   rather than a hook per call site.
+
+4. **Awaiting in `listManagementModelRows` is structurally safe; rejecting is not.**
+   The function is already async, every production caller awaits it, and
+   `nativeModelRows` stays synchronous (`src/codex/catalog/metadata.ts:414`). But a
+   rejection degrades sidecar to auth slots (`src/sidecar/candidates.ts:35-61`) and
+   makes client-config answer 503 (`model-routes.ts:393-409`). The ensure must be
+   non-throwing at its boundary, with a regression for each of those two outcomes.
+
+5. **The 8-second figure is not a total bound.** `MODEL_ROSTER_TIMEOUT_MS` starts
+   inside the roster fetch (`:438-445`); credential refresh can spend 30 seconds
+   first (`main-account.ts:188-190`, `account-store.ts:743-746`). And flight dedup
+   begins only *after* credential enumeration (`:524-552`), so abandoning a wait
+   avoids duplicate roster fetches but not repeated credential work. The whole
+   ensure operation has to be deduplicated, with the deadline anchored to the
+   flight's start.
+
+6. **Three of the four regression claims were false reds.** Fresh-zero-refetch,
+   TTL+1-single-fetch, and unconfirmed-failure-caching are already green through
+   `:517-552`, and the empty/all-filtered 15s behaviour is already covered by wp1's
+   own tests (`tests/codex-model-entitlements.test.ts:672-749`). The genuine wp2
+   reds are the **credential-read count** and the **logged-out negative memo**. The
+   `/api/models` and client-config assertions are red only if the fixture supplies a
+   usable credential and upstream roster — and client-config's expected fetch count
+   of 1 is red because the current count is **0**, which is the defect itself. The
+   CLI case is red only after its stub (`tests/cli-export-command.test.ts:51-57`) is
+   replaced with the real handler. Baseline across the four files: 93 pass, 0 fail.
+
+7. wp1 removed the concept the plan still named: there is no "confirmed-empty
+   entry" any more (`:465-478`). The correct noun is "an empty or all-filtered
+   **unconfirmed** entry within `MODEL_ROSTER_FAILURE_TTL_MS`".
+
+## Open question — settled
+
+The plan's default (wait out the refresh) is wrong, and my proposed flat 3s
+per-waiter deadline was also wrong: sequential sidecar calls
+(`src/server/management/config-routes.ts:589-593`) would accumulate up to 6s.
+
+**Decision.** One whole-ensure flight carrying `{ startedAt, promise }`, caught
+fail-closed, and **never aborted by a management timeout** — the upstream work
+always runs to completion so the cache is populated for the next caller.
+
+- Sidecar candidate paths join with a **0 ms** wait. They never stall; they read
+  whatever is already cached.
+- `/api/models`, `/api/client-config`, integrations and `ocx export` wait up to
+  **3000 ms**, measured from the flight's original `startedAt` rather than from each
+  waiter's own arrival.
+
+That keeps the useful one-shot window for `ocx export` — which is the surface
+#3023 actually reported — while a dashboard poll never stalls and converges within
+one 5s cycle.

devlog/_plan/260831_prio70_entitlement_and_spill_train/009_audit_rounds6to8_wp2_and_wp3_review.md

@@ -0,0 +1,84 @@
+# 009 — audit rounds 6-8: wp2 plan, and the wp3 implementation review
+
+Two independent sol-high reviewers, both read-only. Recorded together because they
+ran concurrently.
+
+## wp2 plan — rounds 6, 7 and 8 (all FAIL, all folded into `020`)
+
+**Round 6.**
+
+1. The whole-ensure flight was underspecified: a bare `{ startedAt, promise }` can
+   cross-answer different candidate sets, client versions or credential epochs, while
+   the per-entry dedup it wraps is keyed by account, identity and version
+   (`:524-552`). Fixed by giving the flight a real key.
+2. The mutation-hook set was still incomplete: `native-profile-manager.ts:1307`,
+   `:1335`, `:1449` write canonical `auth.json` during switch, rollback and recovery
+   (transitions at `:1318`, `:1453`). These are OpenCodex's own writes, so they belong
+   in the epoch.
+3. The two wait policies could not be expressed by `ensureCodexEntitlementFreshness(config)`
+   as written, because sidecar reaches the same `listManagementModelRows`
+   (`src/sidecar/candidates.ts:40`). Needs an options path.
+4. The credential-read regression was **false-green**: "zero token refreshes and zero
+   fetches" is already satisfied today while the full credential snapshot still runs
+   (`:592-601`). Must assert zero `accountCredentialSnapshot` calls.
+5. The negative-memo TTL was never pinned, and "converges within one 5s cycle"
+   contradicted the acknowledged 30s credential refresh.
+
+**Round 7.**
+
+1. The epoch alone cannot fence external `auth.json` writers — the document says so
+   itself — so a caller holding a new identity could still join an old-identity
+   flight. The flight key now carries the identity vector
+   `(accountId, credentialIdentity | null)`.
+2. The negative memo could still be published against a login that landed mid-flight.
+   Publication is now fenced on the captured identity vector, and expiry is measured
+   from the **absence observation**, not from settlement — a flight that spent 30s in
+   a credential refresh must not hand out evidence that is treated as 5s fresh.
+
+**Round 8 (final).**
+
+The key still omitted the **workset**. Candidate set, version, epoch and identities
+can be unchanged while an entry expires mid-flight: a flight that started when B was
+fresh refreshes only A, B expires, a second caller computes the same key and joins,
+and the resolver's `now` is fixed from flight start (`:586`) so B stays a cache hit
+(`:517-522`). `ocx export` — the surface #3023 actually reported — then returns short
+rows having refreshed nothing. The key now includes normalized
+`needsRefreshAccountIds`, or joining requires a subset relationship.
+
+wp2 is planned but **not implemented**. The document is now implementable; the work
+itself is the next cycle.
+
+## wp3 implementation review — FAIL, one high finding
+
+Reviewed commit `135f22932` (on top of Ingwannu's `aec717722`).
+
+**Confirmed correct, no change needed:** drain ordering before the 2 MiB snapshot
+exclusion (`state.ts:1236-1247` vs `:1133`); the stable-fixed-point loop with no bare
+`Promise.race` (`:443-450`); the budget split `B=5000`/`R=4000` with the fallback
+receiving `R`; the shared ACL deadline reaching both the directory and temp hardeners
+(`spill-store.ts:197-227`, `:440-452`); test 2c's six `icacls` calls and its
+max-timeout assertion, which would genuinely detect a fresh 30s window. No
+MUST-NOT-CHANGE violation, no payload or token logging.
+
+**HIGH — the abandoned writer can still publish, and can orphan a temp.** The async
+writer creates and fills its temp *before* awaiting file ACL hardening
+(`spill-store.ts:505-511`). At cap expiry the fallback marks the job cancelled and
+drops its tracking (`state.ts:417-422`) but takes no ownership of that temp; the drain
+returns (`:446-448`) and shutdown exits (`management-api.ts:278-280`,
+`cli/index.ts:365-370`). A resuming writer publishes *first* (`spill-store.ts:515-529`)
+and only then hits the cancellation check (`state.ts:257-260`). State overwrite
+prevention is airtight; filesystem publication and cleanup are merely narrow. The
+commit's own test encoded the violation as expected, asserting **two** publications
+(`tests/responses-state.test.ts:999-1000`).
+
+**MEDIUM — the fixed point has no regression behind it.** The shutdown tests queue a
+single job (`:823-891`), so a one-shot tail await would pass them. The loop is right;
+nothing proves it stays right.
+
+Both sent back for repair before wp3 lands.
+
+## Residual risk carried
+
+A real Windows host is still needed to prove NTFS inheritance, `icacls` timeout
+behaviour, and unlink semantics while `icacls` holds a path. Everything above was
+exercised through the repository's injected Windows/ACL runners.