test(server): isolate the vitest suite from the host's ~/.codex - #1053
Merged
Conversation
The Codex token harvester resolves its rollout directory from CODEX_HOME and falls back to the host's real ~/.codex (src/agents/codex-sessions.ts:11). Any server test that harvests a codex agent then recursively walks that tree and reads the head of every rollout file. On this machine that is 5.7 GB / ~11.7k files, and the self-hosted CI runner is the same machine — so it is not a "works on CI" situation. PR #1041 patched the one test whose 30s budget it blew by swapping CODEX_HOME inside that test, but stopAgent harvests fire-and-forget (manager.ts:1173), so codex agents stopped by other tests still walked the real directory. The file was still spending ~24s on host I/O. Point CODEX_HOME at an empty directory for the whole server suite via vitest.config.ts `test.env`, and drop the now-redundant per-test override. apps/server/test/db/agent-manager.test.ts alone: 55.5s with one 30s timeout before, 7.5s and 129/129 after. Full server suite: 3130 passed in 40.4s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review 1107 item 2612: a fixed name under os.tmpdir() is not guaranteed to be
empty — mkdirSync(..., { recursive: true }) silently reuses an existing
directory or follows an existing symlink, so a stale or planted `sessions` tree
would put host-controlled data back in the harvester's path.
mkdtempSync creates a fresh 0700 directory with a random suffix on every run, so
it can never adopt an existing tree, and concurrent runs no longer share one.
Removed on process exit.
Verified: zero `dispatch-server-vitest-codex-*` directories in TMPDIR before and
after a run; agent-manager.test.ts 129/129 in 8.1s; full server suite 3130
passed in 32.2s; pnpm run check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two files.
apps/server/vitest.config.tsgains atest.envblock pointingCODEX_HOMEat an empty directory underos.tmpdir()for the whole server suite;apps/server/test/db/agent-manager.test.tsdrops the per-testCODEX_HOMEswap that is now redundant.Why this is tech debt
codexSessionsDir()(apps/server/src/agents/codex-sessions.ts:11) resolvesCODEX_HOME || ~/.codex, anddiscoverCodexRolloutFilesthen recursively walks that tree reading the first 20 lines of every rollout.jsonllooking for the[dispatch:<agentId>]tag. So any server test that harvests a codex agent reads the developer's real Codex history. On this machine that is 5.7 GB / ~11,700 files. The self-hosted CI runner is the same machine (studio.local), so CI reads the same directory — this was never a "works on CI" situation.AgentManager.harvestAgentTokensalready opens withif (!this.runtime.tracksSessions()) return;and the comment there says the guard "keeps inert dev/test servers from scanning session history that belongs to the host environment". The unit suite constructs a manager whose runtime does track sessions, so it bypasses that guard — the intent was already recorded in the code, just not enforced for tests.PR #1041 patched the single test whose 30s budget it blew by swapping
CODEX_HOMEinside that one test. That fixed the named symptom but not the class:stopAgentharvests fire-and-forget (apps/server/src/agents/manager.ts:1173), so every codex agent stopped by another test in the file still walked the real directory in the background, and no per-test override can wrap an un-awaited call. The file was still burning ~24s on host I/O after #1041.Proof (the wall clock is the assertion — no new test)
apps/server/test/db/agent-manager.test.ts, same DB, same machine:main)should skip session ownership logic for non-claude agentstest times out at 30000msThat third row is the non-vacuity check: I reverted only
vitest.config.tsand re-ran, and the exact test named in the flake report failed. Restored, and it passes in 7.5s.Full server suite with the change: 3130 passed / 9 skipped, 40.4s.
pnpm run checkclean. E2E: 195 passed / 13 skipped.Explicitly NOT changed
claudeProjectRoot()(token-harvester.ts:60) is hardcoded toos.homedir()/.claude/projectswith no env knob, so isolating it would mean either overridingHOMEfor the whole suite (huge blast radius — git, docker, pg all read it) or adding a new env knob to production code, which is a behavior change rather than debt cleanup. It is also bounded: itreaddirs one encoded directory for the agent's cwd rather than walking a tree, and on this machine~/.claude/projects/-tmpdoes not exist, so it is a no-op today. Recorded on the backlog instead.apps/server/test/codex-sessions.test.ts. It sets its ownCODEX_HOMEper test (including one case that deliberatelydeletes it to assert the~/.codexfallback). Those are fixtures under test, not host leakage, and they still pass unchanged — the config value is a default that a test is free to override.apps/server/test/token-harvester.test.ts:431, same reasoning: it pointsCODEX_HOMEat its own fixture tree.testTimeout. Deliberately untouched — raising it would have hidden a suite doing gigabytes of host I/O on every run.The one arguable call
Removing #1041's per-test
CODEX_HOMEwrapper rather than leaving it as belt-and-braces. I removed it because its comment ("without this the harvester walks the real ~/.codex") would become false, and a stale comment asserting a defense that has moved elsewhere is the debt this job exists to remove. See the reviewer question below.Next run
Backlog: the
publishUiEvent: (event: unknown) => voidlooseness across eight route dep types, together with theagent.upsert/snapshothasStreamwire understatement — they are the same job.🤖 Generated with Claude Code