diff --git a/AGENTS.md b/AGENTS.md index 2abe06c4..0f5e9a88 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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. diff --git a/__tests__/setup-home-sandbox.ts b/__tests__/setup-home-sandbox.ts new file mode 100644 index 00000000..5acb4739 --- /dev/null +++ b/__tests__/setup-home-sandbox.ts @@ -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 */ + } +}); diff --git a/__tests__/test-home-sandbox.test.ts b/__tests__/test-home-sandbox.test.ts new file mode 100644 index 00000000..f5ebb7a7 --- /dev/null +++ b/__tests__/test-home-sandbox.test.ts @@ -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); + }); +}); diff --git a/__tests__/upgrade.test.ts b/__tests__/upgrade.test.ts index 244957af..e1b05e4d 100644 --- a/__tests__/upgrade.test.ts +++ b/__tests__/upgrade.test.ts @@ -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 & { 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> = {}; + 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 // --------------------------------------------------------------------------- diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 64078dfa..e5f8eacf 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -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), diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index 625e45b8..76354378 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -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; /** * 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 { 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 { + 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'; +} diff --git a/vitest.config.mts b/vitest.config.mts index ae02ba84..96fb2573 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -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` diff --git a/vitest.workspace.mts b/vitest.workspace.mts index c37fae14..9abdc721 100644 --- a/vitest.workspace.mts +++ b/vitest.workspace.mts @@ -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([ {