Repository navigation
fix(test): isolate service state from the shared startup home - #7161
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
…-isolation # Conflicts: # scripts/test-layout/layout.json
# Conflicts: # scripts/test-layout/layout.json # structure/ops/test-sandbox-cleanup.md # tests/fixtures/test-layout-expected.json
|
Maintainer integration into
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
Parallel
bun testworkers all share one startup home. Bun cachesos.homedir()at process start, so the preload's per-workerHOMEnever reaches it, and service-state's legacy default-home mirror (<startup home>/.opencodex/service-state.json) was shared by every worker and by serial cases that switchOPENCODEX_HOMEfixtures. A foreign record left there broke service SQLite-home binding and diagnostics tests depending on file order.src/service/state.tsresolves service-state reads and writes only within the selectedOPENCODEX_HOME, including captured-home lookups. The existing real-home filter stays. Production keeps its mirror writes and legacy migration reads unchanged.tests/service/service-state-isolation.test.tsplants a foreign startup-home record in a child process, movesHOMEthe way the preload does, and checks isolated reads, writes, and a fixture switch with the guard armed, plus unchanged mirror behavior with it disarmed. The boundary is documented instructure/ops/test-sandbox-cleanup.mdandstructure/runtime.md.Why a guarded branch in
state.tsand not a per-worker HOME in the runner. On Bun 1.4.3,homedir()keeps the startup value afterprocess.env.HOMEchanges, and replacingos.homedirfrom a preload does not reach namedimport { homedir }bindings (verified directly). Bun spawns--parallelworkers itself, soscripts/test.tshas no per-worker spawn hook. Giving each worker its own HOME would mean replacing Bun's worker pool with runner-owned sharding. That is a much larger change, and it would still miss serial cases inside one worker that switch fixtures. The guarded branch also extends an existing precedent:serviceStatePaths()already dropped the mirror under the guard when it named the real home.Other
homedir()consumers were surveyed. Most are read-only probes, accept an injected home, or already preferprocess.env.HOME(claude/gateway-cache.ts). The one with the same shape is the owner registry (<homedir>/.opencodex/ocx-homes). ItsOCX_OWNER_REGISTRY_DIRseam is set by the suites that use it, and no failure points at it, so it is left as a follow-up.Verification
service-state-isolation,service-sqlite-home,service,service-ownership-state,ci-workflows/test-home-guard): 290 pass, 0 fail serially and with--parallel=4.test-layout,test-layout-tooling,file-size-ratchet): 36 pass.bun run typecheck,bun run structure:checkandbun run privacy:scanpass.bun run test:changed: 30,198 pass, 68 skip, 5 fail, 4 unhandled errors across 1,350 files, all in three CLI files. To attribute them,bun scripts/test.ts --parallel=1ontests/cli/cli-access-audio-live.test.ts,tests/cli/cli-headless-parity.test.tsandtests/cli/sibling-home-client-sync.test.tswas repeated on this head and on a cleanorigin/dev(7640a942b) worktree. Both trees fail the same cases every round: the threelive audio readiness5s timeouts and the 30ssecondary ensure parent preserves shared Grok, Codex and Claude agent bytestimeout (165/4 here; 165/4 and 166/3 on dev). They are pre-existing.cli-headless-parity, the source of the earlier varying combo and Raycast assertions, passed on every run in both trees. The Raycast failure body readauth required, which appears nowhere insrc/and which the test's fake server never sends, so a foreign local listener answered on that port during a load spike. Neither touches service state.test:changedconcurrently (load average above 40). Broader and non-macOS coverage is left to the manualci.ymldispatch withlane=allon headf5ccb650d, queued behind a repository-wide Actions backlog. This PR stays draft until it completes green.Checklist