test(agent-setup): remove module mocks that leaked a temp home into other files

Merged
SA
1mo ago
+137-129

Part of removing cross-file test leaks so suites do not depend on bun test --isolate.

The leak

agent-wrappers.test.ts replaced ./paths, ./notify-hook and node:os with mock.module, which is process-wide in bun. Without --isolate, disabled-skills.test.ts and disabled-agent-hooks.test.ts then resolved the leaker's temp directory and read another test's state file. This is the "treats a missing or corrupt file as nothing disabled" failure seen on CI since September.

Fix

  • The test sets SUPERSET_HOME_DIR and HOME to its temp root in beforeAll and restores both in afterAll. The real ./paths and ./notify-hook work unchanged. All 84 tests are kept.
  • The workarounds for this leak (a comment and an rmSync) are removed from the two victim files.

Production change

bun reads $HOME once at startup, so setting it in a test does not change os.homedir(). paths.ts gets resolveUserHomeDir(): os.homedir() on Windows, process.env.HOME || os.homedir() elsewhere. Thirteen agent-wrappers-*.ts files call it where they called os.homedir(). Under Node and Electron this is what os.homedir() already does on POSIX. Under bun the only difference is that a later change to HOME is followed.

Two test bugs in notify-hook.test.ts

  • "agent identity precedence" (5 tests) failed inside a Superset terminal: the helper spread process.env, so the terminal's own SUPERSET_AGENT_LAUNCH_ID reached the hook. It now drops ambient SUPERSET_* variables.
  • The fake host bound the wildcard address while the hook calls 127.0.0.1; another local process on the same port could take the request. It now binds 127.0.0.1.

Verification

bun 1.4.2, bun test --randomize --seed N, seeds 1-12: 384 pass, 0 fail without --isolate and with it, except one --isolate run of seed 1 where 3 tests in agent-wrappers-pi.test.ts failed and then passed on six reruns. That file is not changed here; it polls up to 1 s for a detached script to write a file. Repo typecheck and biome pass.

Summary by cubic

Removes the module mocks in agent-wrappers.test.ts that leaked a temp home directory into disabled-skills.test.ts and disabled-agent-hooks.test.ts, causing intermittent "treats a missing file as nothing disabled" CI failures since September. The suite now sets SUPERSET_HOME_DIR and HOME in beforeAll and restores them in afterAll, so it uses the real ./paths and ./notify-hook instead of mock.module replacements.

Because bun reads $HOME once at startup, paths.ts gains resolveUserHomeDir() which reads HOME per call, matching Node's behavior. All 13 agent-wrappers-*.ts files now use it instead of os.homedir(), and the two victim files' leak workarounds are removed.

notify-hook.test.ts also fixes two test bugs: it no longer passes ambient SUPERSET_* variables to the hook, and its fake host now binds 127.0.0.1 instead of the wildcard address. All 84 tests are kept, and the suite passes with and without --isolate.

Summary by CodeRabbit

  • Bug Fixes
    • Agent configuration and extension paths now consistently respect the HOME environment variable on non-Windows systems, improving file discovery when it differs from the system-reported home directory.
    • Windows continues to use the system-reported home directory.
  • Tests
    • Improved test isolation so inherited environment settings are less likely to affect results.