Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions devlog/_plan/261004_fixture_isolation_stabilization/000_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Repair fixture ownership without changing production lease safety

The release test fixtures can share a wrapper-startup service authority, and one fixture kills its holder before releasing a young lock. A directory-blind injection snapshot also depends on the installed Codex binary. Repair the two fixtures while preserving the actual runtime and original assertions. Historical failures elsewhere receive controlled attribution, not a waiver from isolated passes or CI.

Reader: coordinator deciding whether this bounded repair can join the final regression candidate.

- Class/archetype: C3 fixture lifecycle and isolation; satisfy-spec, one PABCD cycle.
- Trigger/goal: delegated stabilization on dev dd9a980ec071285e628a35d8cdefaea785b8916b; publish a small test-only repair PR with exact-head evidence.
- Scope: tests/update/update-restart-lease.test.ts and tests/codex-integration/injection-model-suggest-routes.test.ts, plus this unit. Inherited leaf implementer owns lease fixture; main owns injection fixture and integration. Historical explorer writes only ignored scratch. Independent reviewer checks the integrated patch.
- Non-goals: production/runtime fixes, lease grace/reclaim changes, deleted/weakened assertions, skips/retry-as-fix, other B/E repairs, user-home/live-service/account changes, broad local suite/import graph, final all-platform CI, merge/release.
- Verifier: original focused files, holder→successor order, bounded forced-cleanup negative, case-owned authority checks, directory mutation sensitivity and cache cleanup; one bounded combined runner control; typecheck/privacy/structure/file-size plus applicable PR CI. No repeated GUI build/test: coordinator supplies its separate current-tree 43/0 proof.
- Stop: DONE means both fixtures repaired, focused/control evidence and independent review complete, own PR published with successful applicable exact-head CI, and truthful attribution/limits report. Unexplained historical producer identity remains explicitly unresolved; it is not silently counted as passing.
- Artifact: this numbered unit and ignored .tmp/release-stabilization/fixture-followup/report.md with raw command/HEAD/cleanup receipts.
- Resource/authority: existing tools, own checkout/branch, inherited leaves, shared user test lock. Owned TMP paths retain the existing shared runtime lock path. No token/time budget or new paid resources. Commands are bounded; no OCX_TEST_NO_QUEUE or foreign-lock removal.
- Escalation: a separately verified product defect, new write scope, or actual safety/access blocker. Main decides scope; coordinator merges.

## Evidence and design decisions

Baseline controlled original lease trio on dd9a980e: 1 pass / 2 fail, 8 assertions, 14.27s, same young-lock timeout signature. The coordinator separately traced the dead holder and toggled cooperative release: 1/2 to 3/0 without changing original assertions. Prior injection evidence identifies installed bundled discovery creating codex/tmp before the initial snapshot. Raw traces and hypotheses remain in ignored scratch.

Architect: D1 case-scoped parent homedir spy and same child startup home, preserving real-home guard; D2 stdin EOF release/readiness/ack; D3 bounded reaping/drains and failed graceful release stays failure; D4 existing runtime/catalog cache seams plus recursive snapshot; D5 exactly two fixture files and no runtime edits; D6 original sequence and adversarial isolation controls. Main accepts all six. See 010 for concrete changes. Same-architect reflection and independent A audit precede edits.

Historical explorer owns five remaining victim files (16 named failures). Controlled producer/victim/off/on experiments may establish equivalent contamination sufficiency; historical adjacency alone cannot identify the original writer. Source equivalence and isolated passes remain narrower evidence. No fixture expansion is authorized from that report alone.

Architect reflection identified one accepted omission: injection environment capture must be per-case, not module-evaluation time. 010 now requires beforeEach capture, unconditional owner-cache reset and failure-safe env/root cleanup. Combined runner control explicitly uses one worker and serial cases. No design expansion.

Implementation integration: all7 original lease expectations remain; The implementer repaired missing first-case port config using the existing second-case setup after observing an existing default-port proxy, then obtained12/0. Main added an awaited/bounded stdin end result so asynchronous pipe-close failure cannot escape teardown. Injection9/0 includes deterministic EISDIR red proof, recursive byte/directory observation and a stale module-capture control. No production source changes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Make each fixture own its state and child lifetime

Depends on: approved 000 plan. No public type, stored schema, production dependency or runtime behavior changes. MODIFY only the two test files and this unit; no new layout registration.

## Lease fixture: executor scope

In tests/update/update-restart-lease.test.ts:

1. MODIFY node:os import to namespace form and add Bun test spyOn. Before each case, capture environment and arrange a restorable homedir spy. sandbox() sets the spy to its case home before calling serviceStatePaths(). Replace platform-specific child HOME/USERPROFILE selection with box.home on every platform. Preserve the original OCX_REAL_HOME protection and token exclusion. Assert every parent candidate and the selected authority are within box.root, using path-relative containment; retain actual child-authority equality before service-child behavior. Pin/restore CODEX_SQLITE_HOME to the case if inherited override could escape fixture scope.
2. MODIFY child bookkeeping to track actual Bun subprocesses and their drain completion; keep fake Node ChildProcess only at the production injection seam. Do not cast Bun objects to Node types for cleanup. Preserve childLogs behavior and original assertions.
3. REPLACE the lease holder's `await Bun.sleep(13_000); lease.release();` with explicit stdin EOF protocol. Child acquires the real lease, writes a case-owned ready marker, then waits on its piped stdin. Closing stdin requests release; child calls release, writes a distinct released acknowledgment and exits normally. Parent keeps stdin open throughout the existing fail-closed assertions. The synchronous runService hook polls the ready marker under its existing bound, verifies holder ownership, then returns. Production reacquire 10s and stale-grace 30s remain unchanged.
4. MODIFY teardown: request cooperative release for holders, require acknowledgment/exit0/exact lock absence before root removal, and await output drains. Bound every wait with existing watchdog/budget helpers and clear timers. If cooperation fails, kill/reap only that tracked fixture child under a separate bound, settle/cancel its drains while collecting every observed rejection by drain index, exclude only the exact cleanup-owned cancellation object (not an error name), preserve all genuine and graceful failures and restore env/spies in finally. Remove an ordinary proven-owned root only after every child is reaped, drains settled and the full cleanup protocol succeeded; retain it with diagnostics on an unexpected cleanup failure. An explicit negative control may dispose its owned root after reaping/draining only after verifying and consuming exactly its deliberately expected failures. Never unlink/backdate a lock to obtain green.
5. ADD behavior controls in the same file: two cases yield different contained authority paths; parent/child authority matches with no service command; successful EOF release removes the exact lock and leaves no live child before root cleanup; ignored EOF/missing acknowledgment reaches bounded fallback and is reported as failed graceful cleanup, not success. Preserve all seven existing behavioral cases and assertions. Original holder→two successors sequence is the regression oracle.

## Injection snapshot: main scope

In tests/codex-integration/injection-model-suggest-routes.test.ts:

1. MOVE environment capture from the module-level saved object into beforeEach, immediately before each case overrides. Capture every variable this fixture changes, then unconditionally reset both caches and restore that per-case snapshot through failure-safe cleanup before removing its owned root. ADD imports for existing reset/setCodexRuntimeResolveCacheForTests and reset/setBundledCatalogCacheForTests. After case environment setup and before first listCatalogNativeSlugs(), seed both with the same synthetic command/version, source fallback and `{ discoverAlternatives: false }` runtime key, following codex-convergence-account-selectors.test.ts:217. Use a supported native fixture row. This removes dependence on launching installed Codex while keeping the real catalog/listing/handler and writer paths. Reset both owners unconditionally before restoring environment/removing the case root. Do not introduce a module mock of the handler or snapshot it away.
2. REPLACE the one-level readdir/readFile snapshot with recursive lstat-based owned-tree traversal. Record directory markers (including empty directories) and exact regular-file bytes. Reject symlinks/nonregular entries explicitly rather than following them or ignoring a subtree. Retain only the actual top-level `ocx/codex-runtime.json` cache exclusion; nested same-basename files and codex-home files remain observable. Preserve `expect(snapshot()).toEqual(before)` and every existing route/model/effort/error assertion.
3. ADD fixture controls: unchanged pre-existing nested and empty directories snapshot successfully; nested file-byte mutation, empty-directory addition/removal and file/directory replacement change the snapshot; same-basename nested cache paths are not exempt. A safe owned symlink/junction fixture proves no traversal. Assert cache reset leaves no prior seeded memo after cleanup (matching the same runtime cache key). Seed immediately before synchronous fixture catalog use; no live model/discovery subprocess is required.

## Verification and boundaries

- Existing wrapped baseline trio already reproduced 1/2 on dd9a980e; do not duplicate baseline or launch the original unsafe service child before authority isolation.
- After repair: wrapped original lease trio, then complete original lease file plus new controls; complete injection file with original assertions plus directory/cache controls. Meaningful combined run uses only these two files and `--parallel=1` with no concurrent cases; retain observed runner order rather than assuming argv determines order. Explorer's no-service producer→victim→off/on control classifies other historical symptoms separately.
- Run `bun run typecheck`, `bun run privacy:scan`, `bun run structure:check`, relevant file-size guard and `git diff --check`. Test commands use .tmp/release-stabilization/fixture-followup/run.py to preserve shared user lock and own TMP, HEAD and child-exit receipts. No broad suite/import graph, no duplicate management-integration GUI test/build, no manual final lane=all/all-platform dispatch.
- Verify `git diff dd9a980e -- src` is empty and no original expectation was removed/weakened. Runtime lease tests remain real; no source guard bypass or altered timeout is accepted.
- Independent code/security reviewer checks fixture path ownership, guard retention, fallback failure propagation, restoration and process cleanup. Hosted per-PR CI must execute applicable jobs at the published head; platform absence/skips remain limits.
- No live service-manager operation or user-home mutation. Only case-owned transient test children/listeners permitted by the original tests after containment assertions pass. No Windows-native claim from macOS execution alone.
Loading
Loading