fix(tests): the test suite no longer writes to the developer's real home dir (#2275) (#2295)

Running the suite on a machine with Claude Code configured could wire the CodeGraph prompt hook into the developer's own ~/.claude/settings.json: every upgrade test whose fake upgrade succeeded ran the real post-upgrade prompt-hook self-heal, which detected the real global Claude profile and wrote it. Daemon suites likewise left records in the real ~/.codegraph/daemons. On Windows a HOME-only redirect would not have helped, since os.homedir() reads USERPROFILE there. - The prompt-hook writer is now injected through UpgradeDeps (wirePromptHook); the CLI passes the real defaultWirePromptHook and the upgrade tests a recorder. New tests pin that a successful upgrade calls the recorder and that the real writer never touches a configured profile. - A vitest setupFiles entry gives every engine test file a throwaway home: HOME/USERPROFILE (plus HOMEDRIVE/HOMEPATH/APPDATA/LOCALAPPDATA on Windows), XDG_CONFIG_HOME, and GIT_CONFIG_GLOBAL (seeded with a dummy identity) point into it, and CLAUDE_CONFIG_DIR/CODEX_HOME/COPILOT_HOME/ HERMES_HOME are cleared. Spawned CLI/MCP children inherit it. A sandbox test asserts os.homedir(), the Claude profile, the daemon registry, a child process and git's global config all resolve inside it. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-16 20:53:57 +08:00 · 2026-10-02 06:18:46 +00:00
parent e7389b22fb
commit 7bbc1fb7f6
8 changed files with 270 additions and 8 deletions
@@ -181,6 +181,8 @@ Tests live in `__tests__/` and mirror the module they cover. Notable ones beyond

 Tests create temp dirs with `fs.mkdtempSync` and clean up in `afterEach`. They write real files and exercise real SQLite — there is no DB mocking.

+Every engine test file runs in a throwaway home dir (`__tests__/setup-home-sandbox.ts`, a `setupFiles` entry): `HOME`/`USERPROFILE` (+ Windows vars), `XDG_CONFIG_HOME` and `GIT_CONFIG_GLOBAL` point into it, and `CLAUDE_CONFIG_DIR`/`CODEX_HOME`/… are cleared; spawned children inherit it. It's a backstop — still inject writes to global state (e.g. `UpgradeDeps.wirePromptHook`, #2275).
+
 ### Windows-gated tests

 Behavior that differs by platform (path resolution, drive letters, `SENSITIVE_PATHS`, `%APPDATA%` config dirs, CRLF) must be gated, not assumed. Use `it.runIf(process.platform === 'win32')(...)` for Windows-only assertions and `it.runIf(process.platform !== 'win32')(...)` for POSIX-only ones — e.g. `/etc` is sensitive on POSIX but resolves to `C:\etc` (non-existent) on Windows, so an ungated `/etc` assertion fails on Windows. Validate the Windows side for real (see below); don't merge a Windows-gated test you haven't seen run.
@@ -0,0 +1,66 @@
+/**
+ * Give every engine test file a throwaway home directory (#2275).
+ *
+ * The suite exercises code that writes to the user's GLOBAL state: the Claude
+ * prompt hook in `~/.claude/settings.json`, the daemon registry under
+ * `~/.codegraph/daemons`, every agent target's global config. A test that
+ * forgot to redirect the home dir wrote to the developer's real one — a
+ * successful fake `codegraph upgrade` wired the prompt hook into their own
+ * Claude profile, and the daemon suites left thousands of registry records
+ * behind. CI never noticed: its runners have no Claude profile to edit.
+ *
+ * So the home dir is redirected here, before any test module loads, rather
+ * than file by file. Everything that resolves a home dir is covered:
+ * `os.homedir()` reads HOME on POSIX but USERPROFILE on Windows; git on
+ * Windows falls back to HOMEDRIVE+HOMEPATH; the per-agent overrides
+ * (CLAUDE_CONFIG_DIR, CODEX_HOME, ...) win over either, so they are cleared;
+ * and git's global config is pinned to a seeded file, so a test's `git
+ * commit` neither reads the developer's identity, signing or hooks nor
+ * depends on them. Spawned CLI / MCP processes inherit all of it.
+ *
+ * Tests that redirect the home dir themselves (spying on `os.homedir`, or
+ * setting HOME and restoring it) keep working: they restore to this sandbox.
+ */
+import { afterAll } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
+// Resolved, so a comparison against a realpath'd cwd (macOS `/var` →
+// `/private/var`) sees the same home everything else does.
+const home = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-home-')));
+
+process.env.HOME = home;
+process.env.USERPROFILE = home;
+if (process.platform === 'win32') {
+  const drive = /^([A-Za-z]:)(\\.*)$/.exec(home);
+  if (drive) {
+    process.env.HOMEDRIVE = drive[1];
+    process.env.HOMEPATH = drive[2];
+  }
+  process.env.APPDATA = path.join(home, 'AppData', 'Roaming');
+  process.env.LOCALAPPDATA = path.join(home, 'AppData', 'Local');
+} else {
+  // Only ever set on Windows in real life; on POSIX a stray one would point
+  // the opencode/Copilot legacy-path logic somewhere real.
+  delete process.env.APPDATA;
+  delete process.env.LOCALAPPDATA;
+}
+process.env.XDG_CONFIG_HOME = path.join(home, '.config');
+for (const override of ['CLAUDE_CONFIG_DIR', 'CODEX_HOME', 'COPILOT_HOME', 'HERMES_HOME']) {
+  delete process.env[override];
+}
+
+// Both spellings: GIT_CONFIG_GLOBAL for git >= 2.32, and `$HOME/.gitconfig`
+// (the same file) for older ones.
+const gitConfig = path.join(home, '.gitconfig');
+fs.writeFileSync(gitConfig, '[user]\n\tname = CodeGraph Test\n\temail = test@codegraph.invalid\n');
+process.env.GIT_CONFIG_GLOBAL = gitConfig;
+
+afterAll(() => {
+  try {
+    fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 });
+  } catch {
+    /* a straggling child still holding a file there; it's in the temp dir */
+  }
+});
@@ -0,0 +1,65 @@
+/**
+ * The suite's home-dir sandbox (`setup-home-sandbox.ts`, #2275) is in force:
+ * nothing a test runs — in-process or spawned — resolves the developer's real
+ * home, Claude profile, daemon registry or git identity.
+ */
+import { describe, it, expect } from 'vitest';
+import { execFileSync } from 'child_process';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { claudeTarget } from '../src/installer/targets/claude';
+import { getRegistryDir } from '../src/mcp/daemon-registry';
+
+const tmpRoot = fs.realpathSync(os.tmpdir());
+const under = (p: string, dir: string): boolean => path.resolve(p).startsWith(dir + path.sep);
+/** The sandbox home — asserted to be one, so the checks below can't pass on a real home. */
+function sandboxHome(): string {
+  const home = os.homedir();
+  expect(under(home, tmpRoot), home).toBe(true);
+  return home;
+}
+
+describe('test home sandbox', () => {
+  it('os.homedir() is a throwaway dir under the temp root, not the real home', () => {
+    const home = sandboxHome();
+    expect(path.basename(home)).toMatch(/^codegraph-test-home-/);
+    // The account's profile dir, which ignores HOME / USERPROFILE.
+    expect(home).not.toBe(os.userInfo().homedir);
+  });
+
+  it('the global Claude profile and the daemon registry resolve inside it', () => {
+    const home = sandboxHome();
+    expect(process.env.CLAUDE_CONFIG_DIR).toBeUndefined();
+    expect(under(claudeTarget.detect('global').configPath, home)).toBe(true);
+    expect(under(getRegistryDir(), home)).toBe(true);
+  });
+
+  it('a spawned child sees the same home', () => {
+    const childHome = execFileSync(process.execPath, ['-p', 'require("os").homedir()'], {
+      encoding: 'utf-8',
+      windowsHide: true,
+    }).trim();
+    expect(childHome).toBe(sandboxHome());
+  });
+
+  it("git's global config is the seeded sandbox file", () => {
+    const gitConfig = process.env.GIT_CONFIG_GLOBAL ?? '';
+    expect(under(gitConfig, sandboxHome())).toBe(true);
+    const name = execFileSync('git', ['config', '--global', '--get', 'user.name'], {
+      encoding: 'utf-8',
+      windowsHide: true,
+    }).trim();
+    expect(name).toBe('CodeGraph Test');
+  });
+
+  // os.homedir() reads USERPROFILE there, and git falls back to
+  // HOMEDRIVE+HOMEPATH — a HOME-only sandbox would not hold.
+  it.runIf(process.platform === 'win32')('covers the Windows home variables', () => {
+    const home = sandboxHome();
+    expect(process.env.USERPROFILE).toBe(home);
+    expect(`${process.env.HOMEDRIVE}${process.env.HOMEPATH}`).toBe(home);
+    expect(under(process.env.APPDATA ?? '', home)).toBe(true);
+    expect(under(process.env.LOCALAPPDATA ?? '', home)).toBe(true);
+  });
+});
@@ -14,6 +14,7 @@ import {
  reindexAdvisory,
  runUpgrade,
  verifyResolvedVersion,
+  defaultWirePromptHook,
  buildWindowsUpgradeScript,
  NPM_PACKAGE,
  type InstallMethod,
@@ -230,13 +231,15 @@ interface Calls {
  captures: Array<{ cmd: string; args: string[] }>;
  logs: string[];
  errors: string[];
+  /** How many times the upgrade asked deps to wire the Claude prompt hook. */
+  promptHookWires: number;
 }

 function makeDeps(
  overrides: Partial<UpgradeDeps> & { method: InstallMethod; currentVersion: string },
  runExit = 0
 ): { deps: UpgradeDeps; calls: Calls } {
-  const calls: Calls = { runs: [], captures: [], logs: [], errors: [] };
+  const calls: Calls = { runs: [], captures: [], logs: [], errors: [], promptHookWires: 0 };
  const deps: UpgradeDeps = {
    currentVersion: overrides.currentVersion,
    method: overrides.method,
@@ -252,6 +255,12 @@ function makeDeps(
      return overrides.capture ? overrides.capture(cmd, args) : null;
    },
    hasCommand: overrides.hasCommand ?? ((c) => c === 'curl'),
+    // A recorder, never the real writer: that one edits the GLOBAL Claude
+    // profile, which here would be the developer's own (#2275).
+    wirePromptHook: async () => {
+      calls.promptHookWires += 1;
+      return overrides.wirePromptHook ? overrides.wirePromptHook() : false;
+    },
    log: (m) => calls.logs.push(m),
    warn: (m) => calls.logs.push(m),
    error: (m) => calls.errors.push(m),
@@ -473,6 +482,104 @@ describe('runUpgrade beta signup offer', () => {
  });
 });

+// ---------------------------------------------------------------------------
+// Post-upgrade prompt-hook self-heal — only ever through deps (#2275). The
+// real writer edits the GLOBAL Claude profile; while the upgrade called it
+// directly, every successful fake upgrade in this file could wire the hook
+// into the developer's own ~/.claude/settings.json.
+// ---------------------------------------------------------------------------
+
+describe('post-upgrade prompt-hook self-heal', () => {
+  // A configured global Claude profile the REAL writer would act on, so an
+  // upgrade that bypasses deps shows up as a settings.json written here.
+  const KEYS = ['CLAUDE_CONFIG_DIR', 'CODEGRAPH_NO_PROMPT_HOOK', 'CODEGRAPH_PROMPT_HOOK'] as const;
+  const saved: Partial<Record<(typeof KEYS)[number], string | undefined>> = {};
+  let profile: string;
+
+  function configureProfile(): void {
+    fs.writeFileSync(
+      path.join(profile, '.claude.json'),
+      JSON.stringify({ mcpServers: { codegraph: { command: 'codegraph', args: ['serve', '--mcp'] } } }),
+    );
+  }
+
+  beforeEach(() => {
+    for (const k of KEYS) saved[k] = process.env[k];
+    delete process.env.CODEGRAPH_NO_PROMPT_HOOK;
+    delete process.env.CODEGRAPH_PROMPT_HOOK;
+    profile = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-upgrade-claude-'));
+    process.env.CLAUDE_CONFIG_DIR = profile;
+  });
+  afterEach(() => {
+    for (const k of KEYS) {
+      if (saved[k] === undefined) delete process.env[k];
+      else process.env[k] = saved[k];
+    }
+    fs.rmSync(profile, { recursive: true, force: true });
+  });
+
+  it('a successful upgrade wires the hook through deps, never the real writer', async () => {
+    configureProfile();
+    const { deps, calls } = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' });
+    expect(await runUpgrade({}, deps)).toBe(0);
+    expect(fs.existsSync(path.join(profile, 'settings.json')), 'the real writer ran').toBe(false);
+    expect(calls.promptHookWires).toBe(1);
+  });
+
+  it('notes the hook only when the writer changed something', async () => {
+    const quiet = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' });
+    expect(await runUpgrade({}, quiet.deps)).toBe(0);
+    expect(quiet.calls.logs.join('\n')).not.toMatch(/front-load hook/);
+
+    const wired = makeDeps({
+      method: { kind: 'npm', scope: 'global' },
+      currentVersion: '0.9.8',
+      wirePromptHook: async () => true,
+    });
+    expect(await runUpgrade({}, wired.deps)).toBe(0);
+    expect(wired.calls.logs.join('\n')).toMatch(/Enabled the CodeGraph front-load hook/);
+  });
+
+  it('the kill-switch skips the writer entirely', async () => {
+    process.env.CODEGRAPH_NO_PROMPT_HOOK = '1';
+    const { deps, calls } = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' });
+    expect(await runUpgrade({}, deps)).toBe(0);
+    expect(calls.promptHookWires).toBe(0);
+  });
+
+  it('does not wire on --check, when up to date, or when the upgrade fails', async () => {
+    const check = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' });
+    expect(await runUpgrade({ check: true }, check.deps)).toBe(0);
+    const current = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.9' });
+    expect(await runUpgrade({}, current.deps)).toBe(0);
+    const failed = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }, 1);
+    expect(await runUpgrade({}, failed.deps)).toBe(1);
+    for (const { calls } of [check, current, failed]) expect(calls.promptHookWires).toBe(0);
+  });
+
+  it('a throwing writer never fails the upgrade', async () => {
+    const { deps } = makeDeps({
+      method: { kind: 'npm', scope: 'global' },
+      currentVersion: '0.9.8',
+      wirePromptHook: async () => { throw new Error('EACCES'); },
+    });
+    expect(await runUpgrade({}, deps)).toBe(0);
+  });
+
+  // The production writer itself, pointed at the temp profile above.
+  it('the real writer wires a configured profile once, and leaves an unconfigured one alone', async () => {
+    const settings = path.join(profile, 'settings.json');
+    expect(await defaultWirePromptHook()).toBe(false);
+    expect(fs.existsSync(settings)).toBe(false);
+
+    configureProfile();
+    expect(await defaultWirePromptHook()).toBe(true);
+    const written = JSON.parse(fs.readFileSync(settings, 'utf-8'));
+    expect(JSON.stringify(written.hooks.UserPromptSubmit)).toMatch(/prompt-hook/);
+    expect(await defaultWirePromptHook()).toBe(false); // idempotent
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Post-upgrade self-heal of installed agent surfaces
 // ---------------------------------------------------------------------------
@@ -2859,6 +2859,7 @@ program
        run: up.defaultRun,
        capture: up.defaultCapture,
        hasCommand: up.hasCommand,
+        wirePromptHook: up.defaultWirePromptHook,
        log: (m: string) => console.log(m),
        warn: (m: string) => warn(m),
        error: (m: string) => error(m),
@@ -297,6 +297,15 @@ export interface UpgradeDeps {
  warn: (msg: string) => void;
  error: (msg: string) => void;
  platform: NodeJS.Platform;
+  /**
+   * Wire Claude Code's front-load prompt hook into the GLOBAL Claude profile
+   * when that profile already has CodeGraph configured; resolves true when it
+   * changed the settings file. That file is the user's real
+   * `~/.claude/settings.json`, so the writer is injected like every other side
+   * effect here — the CLI passes {@link defaultWirePromptHook}, unit tests a
+   * recorder (#2275: tests that reached the real one rewrote the developer's).
+   */
+  wirePromptHook: () => Promise<boolean>;
  /**
   * Offer the one-time CodeGraph Pro beta opt-in after a successful update
   * (see installer/beta-signup — self-gating: TTY only, and silent forever
@@ -528,10 +537,7 @@ function selfHealInstalledSurfaces(deps: UpgradeDeps): void {
 */
 async function selfHealPromptHook(deps: UpgradeDeps): Promise<void> {
  if (process.env.CODEGRAPH_NO_PROMPT_HOOK === '1' || process.env.CODEGRAPH_PROMPT_HOOK === '0') return;
-  const { claudeTarget, writePromptHookEntry } = await import('../installer/targets/claude');
-  if (!claudeTarget.detect('global').alreadyConfigured) return;
-  const res = writePromptHookEntry('global');
-  if (res.action === 'created' || res.action === 'updated') {
+  if (await deps.wirePromptHook()) {
    deps.log(
      c.dim('Enabled the CodeGraph front-load hook for Claude Code (structural prompts). Disable any time: CODEGRAPH_NO_PROMPT_HOOK=1'),
    );
@@ -714,3 +720,15 @@ export function defaultCapture(cmd: string, args: string[]): { code: number; std
  if (r.error) return null;
  return { code: r.status ?? -1, stdout: r.stdout ?? '' };
 }
+
+/**
+ * The production `UpgradeDeps.wirePromptHook`: writes the hook only when the
+ * global Claude profile already carries CodeGraph's MCP entry, and leaves the
+ * file byte-for-byte alone once the hook is there.
+ */
+export async function defaultWirePromptHook(): Promise<boolean> {
+  const { claudeTarget, writePromptHookEntry } = await import('../installer/targets/claude');
+  if (!claudeTarget.detect('global').alreadyConfigured) return false;
+  const res = writePromptHookEntry('global');
+  return res.action === 'created' || res.action === 'updated';
+}
@@ -14,6 +14,9 @@ export default defineConfig({
    include: ['__tests__/**/*.test.ts'],
    // Suites that spawn the built CLI need a current dist/ (#1879).
    globalSetup: ['./__tests__/global-setup-dist.ts'],
+    // A throwaway home dir (and git global config) per test file, so nothing
+    // the suite runs can write to the developer's real one (#2275).
+    setupFiles: ['./__tests__/setup-home-sandbox.ts'],
    /**
     * Several MCP integration tests (mcp-daemon, mcp-initialize, mcp-ppid-watchdog,
     * mcp-roots) spawn `dist/bin/codegraph.js serve --mcp` with `process.execPath`
@@ -20,9 +20,9 @@ import { defineWorkspace } from 'vitest/config';
 * browser builds of `web-tree-sitter` and friends, and the failures that
 * causes look nothing like their cause.
 *
- * The engine project `extends` the shared base, so the env vars and Node guard
- * in `vitest.config.mts` still apply to every engine test. The ui project does
- * not — see the note on it.
+ * The engine project `extends` the shared base, so the env vars, Node guard and
+ * home-dir sandbox in `vitest.config.mts` still apply to every engine test. The
+ * ui project does not — see the note on it.
 */
 export default defineWorkspace([
  {