From 05e6f1cfe5612102c59ce942f32b300d5c23cff3 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 12:42:02 -0700 Subject: [PATCH] =?UTF-8?q?test:=20phantom-hooks=20heal=20coverage=20?= =?UTF-8?q?=E2=80=94=20incident=20facsimile,=20per-item=20safety,=20lock,?= =?UTF-8?q?=20canonical=20tripwires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gstack-settings-hook-schema-aware: 16 new cases — identity re-point (tag restore), foreign-basename rejection, mixed-entry per-item safety for add-event/remove-source/--all, prune-stale modes incl. bash-prefix + Windows-backslash + spaced-path idempotence, duplicate collapse preferring the tagged twin, plan_tune_hooks:no split, backup-on-change no-churn, fail-closed corrupt-JSON for every mutator, stale-lock takeover, fresh-foreign-lock skip, two-writer concurrency smoke, and an INCIDENT FACSIMILE replaying the exact 2026-08-17 production damage (6/3/2 entries, mixed tags, live-ephemeral Stop) healing to 2/1/1 canonical. - NEW setup-hook-canonical-paths: static tripwires — canonical-only resolver (no $SOURCE_GSTACK_DIR anywhere in it), heal-before-guards ordering, unsuppressed heal output, ${VAR:-0} counter idiom, shared-prelude concatenation at every bun call site, KNOWN_HOOKS completeness vs setup's registrations, uninstall cleanup-before-deletion ordering, defect-class warning present. - setup-plan-tune-hooks-noninteractive: PT_EXPLICIT pins + `gstack-config has` provenance + has-subcommand behavior (env-resolution, malformed keys). - auq-error-fallback-hook: registration + both-teardown wiring (previously untested). - uninstall: behavioral ordering test running the INSTALLED copy from inside the root it deletes. - setup-windows-fallback / gstack-config-key-locale: pins updated for the new HOOK_CMD shape and the third C-locale validator. Co-Authored-By: Claude Fable 5 --- test/auq-error-fallback-hook.test.ts | 33 ++ test/gstack-config-key-locale.test.ts | 4 +- .../gstack-settings-hook-schema-aware.test.ts | 429 ++++++++++++++++++ test/setup-hook-canonical-paths.test.ts | 188 ++++++++ ...tup-plan-tune-hooks-noninteractive.test.ts | 69 +++ test/setup-windows-fallback.test.ts | 6 +- test/uninstall.test.ts | 67 +++ 7 files changed, 793 insertions(+), 3 deletions(-) create mode 100644 test/setup-hook-canonical-paths.test.ts diff --git a/test/auq-error-fallback-hook.test.ts b/test/auq-error-fallback-hook.test.ts index ee6d317f5..21505c04b 100644 --- a/test/auq-error-fallback-hook.test.ts +++ b/test/auq-error-fallback-hook.test.ts @@ -129,3 +129,36 @@ describe('hook integration — invoked as PostToolUse', () => { expect(out.additionalContext).toBeUndefined(); }); }); + +// ---------------------------------------------------------------------- +// Registration + teardown wiring (static). Setup registers this hook under +// its own source tag (sharing plan-tune-cathedral would overwrite the +// question-log entry — same event+matcher); both teardown surfaces +// (--no-team, uninstall) must remove it, which pre-v1.67.2 neither did. +// ---------------------------------------------------------------------- + +import * as fs from 'fs'; + +describe('setup registration + teardown wiring (static)', () => { + const ROOT = path.resolve(__dirname, '..'); + const setupSrc = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + const uninstallSrc = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-uninstall'), 'utf-8'); + + test('setup registers the hook via the canonical resolver under --source auq-error-fallback', () => { + expect(setupSrc).toMatch( + /AUQ_ERROR_FALLBACK_HOOK="\$\(_hook_command_path hosts\/claude\/hooks\/auq-error-fallback-hook/, + ); + expect(setupSrc).toContain('--source auq-error-fallback'); + }); + + test('--no-team tears the hook down', () => { + const idx = setupSrc.indexOf('# Also tear down plan-tune'); + expect(idx).toBeGreaterThan(-1); + const slice = setupSrc.slice(idx, idx + 900); + expect(slice).toContain('remove-source --source auq-error-fallback'); + }); + + test('gstack-uninstall tears the hook down', () => { + expect(uninstallSrc).toContain('remove-source --source auq-error-fallback'); + }); +}); diff --git a/test/gstack-config-key-locale.test.ts b/test/gstack-config-key-locale.test.ts index dbf3c4993..94d60cd6b 100644 --- a/test/gstack-config-key-locale.test.ts +++ b/test/gstack-config-key-locale.test.ts @@ -46,13 +46,13 @@ afterEach(() => { }); describe("gstack-config key validation is locale-independent", () => { - test("both get and set validate ASCII ranges under the C locale", () => { + test("get, has, and set all validate ASCII ranges under the C locale", () => { const source = fs.readFileSync(CONFIG, "utf8"); const guardedValidators = source.match( /LC_ALL=C grep -qE '\^\[a-zA-Z0-9_\]\+\(@\[a-zA-Z0-9\]\+\)\?\$'/g, ); - expect(guardedValidators).toHaveLength(2); + expect(guardedValidators).toHaveLength(3); }); test("round-trips existing keys that contain i", () => { diff --git a/test/gstack-settings-hook-schema-aware.test.ts b/test/gstack-settings-hook-schema-aware.test.ts index 5a5f302ec..878a78f27 100644 --- a/test/gstack-settings-hook-schema-aware.test.ts +++ b/test/gstack-settings-hook-schema-aware.test.ts @@ -349,3 +349,432 @@ describe('list-sources', () => { expect(r.stdout).toMatch(/no settings file/); }); }); + +// ---------------------------------------------------------------------- +// Phantom-hooks heal surface (v1.67.2): KNOWN_HOOKS identity table, +// per-item mutation, prune-stale, mutation lock, fail-closed parse. +// +// Ownership is intrinsic (basename + relpath suffix + event/matcher against +// the fixed table) because Claude Code strips the _gstack_source key when it +// rewrites settings.json — tag-only dedupe is what let every Conductor +// worktree append a fresh dead entry. +// ---------------------------------------------------------------------- + +const AUQ_MATCHER = '(AskUserQuestion|mcp__.*__AskUserQuestion)'; +const HOOK_NAMES = [ + 'question-log-hook', + 'question-preference-hook', + 'auq-error-fallback-hook', + 'timeline-stop-hook', +]; + +/** run() with hermetic gstack-config state (prune-stale consults plan_tune_hooks). */ +function runIso(args: string[], extraEnv: Record = {}) { + try { + const stdout = execSync([SETTINGS_HOOK, ...args].map((s) => `'${s}'`).join(' '), { + env: { + ...process.env, + GSTACK_SETTINGS_FILE: settingsFile, + GSTACK_STATE_ROOT: tmpDir, + ...extraEnv, + }, + encoding: 'utf-8', + timeout: 15000, + }); + return { stdout, stderr: '', exitCode: 0 }; + } catch (e: any) { + return { stdout: e.stdout || '', stderr: e.stderr || '', exitCode: e.status ?? 1 }; + } +} + +/** A fake stable install with executable hooks, under `base`. */ +function mkCanon(base: string, name = 'canon'): string { + const canon = path.join(base, name); + fs.mkdirSync(path.join(canon, 'hosts', 'claude', 'hooks'), { recursive: true }); + fs.mkdirSync(path.join(canon, 'bin'), { recursive: true }); + for (const h of HOOK_NAMES) { + const p = path.join(canon, 'hosts', 'claude', 'hooks', h); + fs.writeFileSync(p, '#!/bin/sh\n'); + fs.chmodSync(p, 0o755); + } + const su = path.join(canon, 'bin', 'gstack-session-update'); + fs.writeFileSync(su, '#!/bin/sh\n'); + fs.chmodSync(su, 0o755); + return canon; +} + +function hookEntry(cmd: string, matcher?: string, src?: string, extraItems: any[] = []) { + const e: any = { hooks: [...extraItems, { type: 'command', command: cmd, timeout: 5 }] }; + if (matcher) e.matcher = matcher; + if (src) e._gstack_source = src; + return e; +} + +function backups(): string[] { + return fs.readdirSync(tmpDir).filter((f) => f.startsWith('settings.json.bak.')); +} + +describe('add-event: per-item identity re-point', () => { + test('tag-stripped stale worktree path is re-pointed in place, tag restored', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { PostToolUse: [hookEntry('/dead/wt/hosts/claude/hooks/question-log-hook', AUQ_MATCHER)] }, + }, null, 2)); + runIso([ + 'add-event', '--event', 'PostToolUse', '--matcher', AUQ_MATCHER, + '--command', `${canon}/hosts/claude/hooks/question-log-hook`, + '--source', 'plan-tune-cathedral', '--timeout', '5', + ]); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe(`${canon}/hosts/claude/hooks/question-log-hook`); + expect(s.hooks.PostToolUse[0]._gstack_source).toBe('plan-tune-cathedral'); + }); + + test('foreign path with a gstack basename is NOT claimed (wrong relpath suffix)', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { PostToolUse: [hookEntry('/home/u/myhooks/question-log-hook', AUQ_MATCHER)] }, + }, null, 2)); + runIso([ + 'add-event', '--event', 'PostToolUse', '--matcher', AUQ_MATCHER, + '--command', `${canon}/hosts/claude/hooks/question-log-hook`, + '--source', 'plan-tune-cathedral', + ]); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(2); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe('/home/u/myhooks/question-log-hook'); + }); + + test('mixed entry: only the gstack item (index > 0) is replaced; the user item survives', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [hookEntry( + '/dead/wt/hosts/claude/hooks/question-log-hook', AUQ_MATCHER, undefined, + [{ type: 'command', command: '/Users/me/my-own-hook' }], + )], + }, + }, null, 2)); + runIso([ + 'add-event', '--event', 'PostToolUse', '--matcher', AUQ_MATCHER, + '--command', `${canon}/hosts/claude/hooks/question-log-hook`, + '--source', 'plan-tune-cathedral', + ]); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + const items = s.hooks.PostToolUse[0].hooks; + expect(items).toHaveLength(2); + expect(items[0].command).toBe('/Users/me/my-own-hook'); + expect(items[1].command).toBe(`${canon}/hosts/claude/hooks/question-log-hook`); + }); +}); + +describe('remove-source: per-item', () => { + test('mixed tagged entry: gstack item removed, user item survives, tag dropped', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [hookEntry( + '/x/hosts/claude/hooks/question-log-hook', AUQ_MATCHER, 'plan-tune-cathedral', + [{ type: 'command', command: '/Users/me/my-own-hook' }], + )], + }, + }, null, 2)); + const r = runIso(['remove-source', '--source', 'plan-tune-cathedral']); + expect(r.stdout).toMatch(/removed 1 hook/); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe('/Users/me/my-own-hook'); + expect(s.hooks.PostToolUse[0]._gstack_source).toBeUndefined(); + }); +}); + +describe('prune-stale', () => { + test('prunes dead gstack items; keeps live gstack and dead non-gstack', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [ + hookEntry(`${canon}/hosts/claude/hooks/question-log-hook`, AUQ_MATCHER), // live gstack + hookEntry('/dead/wt/hosts/claude/hooks/auq-error-fallback-hook', AUQ_MATCHER), // dead gstack + hookEntry('/dead/user/own-hook', AUQ_MATCHER), // dead NON-gstack + ], + }, + }, null, 2)); + const r = runIso(['prune-stale']); + expect(r.stdout).toMatch(/removed 1 gstack hook entries/); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(2); + const cmds = s.hooks.PostToolUse.map((e: any) => e.hooks[0].command); + expect(cmds).toContain(`${canon}/hosts/claude/hooks/question-log-hook`); + expect(cmds).toContain('/dead/user/own-hook'); + }); + + test('no-op run writes no backup and leaves the file byte-identical', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { PreToolUse: [hookEntry('/Users/me/my-own-hook', 'Bash')] }, + }, null, 2) + '\n'); + const before = fs.readFileSync(settingsFile, 'utf-8'); + const r = runIso(['prune-stale']); + expect(r.exitCode).toBe(0); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + expect(backups()).toHaveLength(0); + expect(fs.existsSync(path.join(tmpDir, 'settings.json.bak-latest'))).toBe(false); + }); + + test('--repoint re-points dead AND live items, preserves bash prefix, restores tags', () => { + const canon = mkCanon(tmpDir); + const live = mkCanon(tmpDir, 'live-worktree'); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + Stop: [hookEntry(`${live}/hosts/claude/hooks/timeline-stop-hook`)], // LIVE but ephemeral + PostToolUse: [hookEntry('bash /dead/wt/hosts/claude/hooks/question-log-hook', AUQ_MATCHER)], + }, + }, null, 2)); + const r = runIso(['prune-stale', '--repoint', canon]); + expect(r.stdout).toMatch(/repointed 2/); + const s = settings(); + expect(s.hooks.Stop[0].hooks[0].command).toBe(`${canon}/hosts/claude/hooks/timeline-stop-hook`); + expect(s.hooks.Stop[0]._gstack_source).toBe('gstack-timeline-stop'); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe(`bash ${canon}/hosts/claude/hooks/question-log-hook`); + expect(s.hooks.PostToolUse[0]._gstack_source).toBe('plan-tune-cathedral'); + }); + + test('--repoint collapses exact duplicates preferring the tagged twin', () => { + const canon = mkCanon(tmpDir); + const cmd = `${canon}/hosts/claude/hooks/question-log-hook`; + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [ + hookEntry('/dead/a/hosts/claude/hooks/question-log-hook', AUQ_MATCHER), + hookEntry(cmd, AUQ_MATCHER, 'plan-tune-cathedral'), + ], + }, + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe(cmd); + expect(s.hooks.PostToolUse[0]._gstack_source).toBe('plan-tune-cathedral'); + }); + + test('--repoint never ADDS entries (repair, not registration)', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ theme: 'dark' }, null, 2) + '\n'); + const before = fs.readFileSync(settingsFile, 'utf-8'); + runIso(['prune-stale', '--repoint', canon]); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + }); + + test('Windows backslash path is classified as gstack-owned and pruned when dead', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [hookEntry('C:\\dead\\wt\\hosts\\claude\\hooks\\question-log-hook', AUQ_MATCHER)], + }, + }, null, 2)); + const r = runIso(['prune-stale']); + expect(r.stdout).toMatch(/removed 1/); + expect(settings().hooks).toBeUndefined(); + }); + + test('spaced canonical root produces a quoted command that stays owned (idempotent)', () => { + const spacedBase = path.join(tmpDir, 'My Claude'); + fs.mkdirSync(spacedBase, { recursive: true }); + const canon = mkCanon(spacedBase); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { Stop: [hookEntry('/dead/wt/hosts/claude/hooks/timeline-stop-hook')] }, + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + const s = settings(); + expect(s.hooks.Stop[0].hooks[0].command).toBe(`"${canon}/hosts/claude/hooks/timeline-stop-hook"`); + // Second run: the quoted command is still recognized as ours — no churn. + const before = fs.readFileSync(settingsFile, 'utf-8'); + const r2 = runIso(['prune-stale', '--repoint', canon]); + expect(r2.stdout).toMatch(/removed 0 gstack hook entries \(repointed 0\)/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + }); + + test('--all removes live untagged gstack items, spares user hooks and mixed-entry user items', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [hookEntry(`${canon}/hosts/claude/hooks/question-log-hook`, AUQ_MATCHER)], + Stop: [hookEntry( + `${canon}/hosts/claude/hooks/timeline-stop-hook`, undefined, 'gstack-timeline-stop', + [{ type: 'command', command: '/Users/me/custom-stop-hook' }], + )], + PreCompact: [hookEntry('/Users/me/my-own-hook')], + }, + }, null, 2)); + const r = runIso(['prune-stale', '--all']); + expect(r.stdout).toMatch(/removed 2/); + const s = settings(); + expect(s.hooks.PostToolUse).toBeUndefined(); + expect(s.hooks.Stop[0].hooks).toHaveLength(1); + expect(s.hooks.Stop[0].hooks[0].command).toBe('/Users/me/custom-stop-hook'); + expect(s.hooks.Stop[0]._gstack_source).toBeUndefined(); + expect(s.hooks.PreCompact[0].hooks[0].command).toBe('/Users/me/my-own-hook'); + }); + + test('--all removes tagged single-item legacy strays (no table match)', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { Stop: [hookEntry('/old/install/bin/gstack-verify-gate', undefined, 'gstack-verify-gate')] }, + }, null, 2)); + const r = runIso(['prune-stale', '--all']); + expect(r.stdout).toMatch(/removed 1/); + expect(settings().hooks).toBeUndefined(); + }); + + test('--all and --repoint are mutually exclusive', () => { + const r = runIso(['prune-stale', '--all', '--repoint', '/x']); + expect(r.exitCode).not.toBe(0); + expect(r.stderr).toMatch(/mutually exclusive/); + }); + + test('explicit plan_tune_hooks:no — dead plan-tune pruned, live plan-tune NOT re-pointed, Stop still re-pointed', () => { + const canon = mkCanon(tmpDir); + const live = mkCanon(tmpDir, 'live-worktree'); + execSync(`'${path.join(ROOT, 'bin', 'gstack-config')}' set plan_tune_hooks no`, { + env: { ...process.env, GSTACK_STATE_ROOT: tmpDir }, + }); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [ + hookEntry('/dead/wt/hosts/claude/hooks/question-log-hook', AUQ_MATCHER), // dead plan-tune + hookEntry(`${live}/hosts/claude/hooks/auq-error-fallback-hook`, AUQ_MATCHER), // LIVE plan-tune + ], + Stop: [hookEntry('/dead/wt/hosts/claude/hooks/timeline-stop-hook')], + }, + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + const s = settings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + // Live plan-tune hook left exactly where it was (no re-activation without consent). + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe(`${live}/hosts/claude/hooks/auq-error-fallback-hook`); + // Stop hook is not part of the opt-out — re-pointed to canonical. + expect(s.hooks.Stop[0].hooks[0].command).toBe(`${canon}/hosts/claude/hooks/timeline-stop-hook`); + }); + + test('incident facsimile: the exact live-damage shape heals to canonical', () => { + // Replays the 2026-08-17 production state: 6 PostToolUse / 3 PreToolUse / + // 2 Stop entries; 6 dead (deleted worktrees), tags stripped on some, one + // live-but-ephemeral Stop hook, plus a user hook that must survive. + const canon = mkCanon(tmpDir); + const cebu = mkCanon(tmpDir, 'cebu-v4'); + const dead = (n: string) => `/dead/biarritz-v3/hosts/claude/hooks/${n}`; + const dead2 = (n: string) => `/dead/taipei-v2/hosts/claude/hooks/${n}`; + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + SessionStart: [hookEntry(`${canon}/bin/gstack-session-update`)], + PostToolUse: [ + hookEntry(`${canon}/hosts/claude/hooks/auq-error-fallback-hook`, AUQ_MATCHER), + hookEntry(`${canon}/hosts/claude/hooks/question-log-hook`, AUQ_MATCHER), + hookEntry(dead('question-log-hook'), AUQ_MATCHER), + hookEntry(dead('auq-error-fallback-hook'), AUQ_MATCHER), + hookEntry(dead2('question-log-hook'), AUQ_MATCHER, 'plan-tune-cathedral'), + hookEntry(dead2('auq-error-fallback-hook'), AUQ_MATCHER, 'auq-error-fallback'), + ], + PreToolUse: [ + hookEntry(`${canon}/hosts/claude/hooks/question-preference-hook`, AUQ_MATCHER), + hookEntry(dead('question-preference-hook'), AUQ_MATCHER), + hookEntry(dead2('question-preference-hook'), AUQ_MATCHER, 'plan-tune-cathedral'), + ], + Stop: [ + hookEntry(`${cebu}/hosts/claude/hooks/timeline-stop-hook`), + hookEntry(dead2('timeline-stop-hook'), undefined, 'gstack-timeline-stop'), + ], + PreCompact: [hookEntry('/Users/me/my-own-hook')], + }, + }, null, 2)); + const r = runIso(['prune-stale', '--repoint', canon]); + expect(r.exitCode).toBe(0); + const s = settings(); + expect(s.hooks.SessionStart).toHaveLength(1); + expect(s.hooks.PostToolUse).toHaveLength(2); + expect(s.hooks.PreToolUse).toHaveLength(1); + expect(s.hooks.Stop).toHaveLength(1); + expect(s.hooks.PreCompact[0].hooks[0].command).toBe('/Users/me/my-own-hook'); + for (const ev of ['SessionStart', 'PostToolUse', 'PreToolUse', 'Stop']) { + for (const e of s.hooks[ev]) { + expect(e._gstack_source).toBeDefined(); + for (const it of e.hooks) expect(it.command.startsWith(canon)).toBe(true); + } + } + const postSources = s.hooks.PostToolUse.map((e: any) => e._gstack_source).sort(); + expect(postSources).toEqual(['auq-error-fallback', 'plan-tune-cathedral']); + }); +}); + +describe('fail-closed parse (pre-existing data-loss fix)', () => { + const MUTATORS: string[][] = [ + ['add', '/x/bin/gstack-session-update'], + ['remove', '/x/bin/gstack-session-update'], + ['add-event', '--event', 'Stop', '--command', '/x', '--source', 's'], + ['remove-source', '--source', 'plan-tune-cathedral'], + ['prune-stale'], + ]; + test('every mutator refuses to touch a corrupt settings.json', () => { + for (const args of MUTATORS) { + fs.writeFileSync(settingsFile, '{definitely not json'); + const r = runIso(args); + expect(r.exitCode).not.toBe(0); + expect(r.stderr).toMatch(/refusing to mutate/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe('{definitely not json'); + } + }); +}); + +describe('mutation lock', () => { + test('stale lock (old mtime) is taken over; mutation proceeds', () => { + const lockDir = `${settingsFile}.lock`; + fs.mkdirSync(lockDir); + fs.writeFileSync(path.join(lockDir, 'owner'), 'dead-process'); + const old = new Date(Date.now() - 120_000); + fs.utimesSync(lockDir, old, old); + const r = runIso(['add-event', '--event', 'Stop', '--command', '/x/hosts/claude/hooks/timeline-stop-hook', '--source', 'gstack-timeline-stop']); + expect(r.exitCode).toBe(0); + expect(settings().hooks.Stop).toHaveLength(1); + expect(fs.existsSync(lockDir)).toBe(false); // released after the mutation + }); + + test('fresh foreign lock: mutation skipped with a warning, file untouched', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ theme: 'dark' }, null, 2) + '\n'); + const before = fs.readFileSync(settingsFile, 'utf-8'); + const lockDir = `${settingsFile}.lock`; + fs.mkdirSync(lockDir); + fs.writeFileSync(path.join(lockDir, 'owner'), 'another-live-process'); + // Capture stderr explicitly: on a zero exit, execSync passes stderr + // through to the parent instead of returning it. + const cmd = [SETTINGS_HOOK, 'add-event', '--event', 'Stop', '--command', '/x', '--source', 's'] + .map((s) => `'${s}'`).join(' '); + const out = execSync(`${cmd} 2>&1`, { + env: { + ...process.env, + GSTACK_SETTINGS_FILE: settingsFile, + GSTACK_STATE_ROOT: tmpDir, + GSTACK_SETTINGS_LOCK_TIMEOUT_MS: '300', + }, + encoding: 'utf-8', + timeout: 15000, + }); + expect(out).toMatch(/could not acquire lock/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + expect(fs.existsSync(lockDir)).toBe(true); // foreign lock NOT stolen + }); + + test('two concurrent add-events both land (lock serializes; file stays valid JSON)', () => { + const q = (args: string[]) => + [SETTINGS_HOOK, ...args].map((s) => `'${s}'`).join(' '); + const a = q(['add-event', '--event', 'PreToolUse', '--matcher', AUQ_MATCHER, '--command', '/pre-hook', '--source', 'src-a']); + const b = q(['add-event', '--event', 'PostToolUse', '--matcher', AUQ_MATCHER, '--command', '/post-hook', '--source', 'src-b']); + execSync(`sh -c "${a} & ${b} & wait"`, { + env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile, GSTACK_STATE_ROOT: tmpDir }, + encoding: 'utf-8', + timeout: 20000, + }); + const s = settings(); // throws if the file is corrupt + expect(s.hooks.PreToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse).toHaveLength(1); + }); +}); diff --git a/test/setup-hook-canonical-paths.test.ts b/test/setup-hook-canonical-paths.test.ts new file mode 100644 index 000000000..b67eddfd1 --- /dev/null +++ b/test/setup-hook-canonical-paths.test.ts @@ -0,0 +1,188 @@ +/** + * Canonical-only hook registration (phantom-hooks fix, v1.67.2). + * + * Static tripwires over `setup` and `bin/gstack-settings-hook`. The defect + * class these pin against: hook commands baked from the SETUP-TIME tree + * (`$SOURCE_GSTACK_DIR` = `pwd -P` of the running tree) into the user's + * GLOBAL ~/.claude/settings.json. Conductor worktrees are ephemeral, so every + * deleted workspace left dead hooks erroring on each AskUserQuestion fire. + * + * The contract: + * - hook registration paths come ONLY from `_hook_command_path` (canonical + * install: ${CLAUDE_CONFIG_DIR:-$HOME/.claude}/skills/gstack) — an + * ephemeral tree can never be baked in; missing canonical = skip + log. + * - setup heals BEFORE any tag-presence guard (`prune-stale --repoint`), + * so a dead tagged entry can't block re-registration forever. + * - the heal is visible when it changes anything (no full output + * suppression at the call site). + * - the settings-hook binary's bun scripts share one JS prelude (KNOWN_HOOKS + * identity table + helpers) so the dedupe key and the prune predicate + * cannot drift. + */ + +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const setupSrc = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); +const hookBinSrc = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-settings-hook'), 'utf-8'); +const uninstallSrc = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-uninstall'), 'utf-8'); + +describe('setup: canonical-only hook paths', () => { + test('CANONICAL_GSTACK_ROOT honors CLAUDE_CONFIG_DIR with ~/.claude fallback', () => { + expect(setupSrc).toContain( + 'CANONICAL_GSTACK_ROOT="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/skills/gstack"', + ); + }); + + test('_hook_command_path body never references the running tree', () => { + const start = setupSrc.indexOf('_hook_command_path() {'); + expect(start).toBeGreaterThan(-1); + const end = setupSrc.indexOf('\n}', start); + const body = setupSrc.slice(start, end); + expect(body).not.toContain('SOURCE_GSTACK_DIR'); + expect(body).toContain('CANONICAL_GSTACK_ROOT'); + }); + + test('every hook var routes through _hook_command_path; no raw SOURCE_GSTACK_DIR hook assignment remains', () => { + for (const rel of [ + 'hosts/claude/hooks/question-log-hook', + 'hosts/claude/hooks/question-preference-hook', + 'hosts/claude/hooks/auq-error-fallback-hook', + 'hosts/claude/hooks/timeline-stop-hook', + 'bin/gstack-session-update', + ]) { + expect(setupSrc).toContain(`$(_hook_command_path ${rel}`); + } + // The bug: FOO_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/..." + expect(setupSrc).not.toMatch(/="\$SOURCE_GSTACK_DIR\/hosts\/claude\/hooks\//); + expect(setupSrc).not.toMatch(/HOOK_CMD="(bash )?\$SOURCE_GSTACK_DIR\/bin\/gstack-session-update"/); + }); + + test('SessionStart registers via schema-aware add-event under its identity source', () => { + expect(setupSrc).toMatch( + /add-event --event SessionStart --command "\$HOOK_CMD" --source gstack-session-update/, + ); + }); + + test('every add-event source in setup has a KNOWN_HOOKS table row', () => { + // Future-hook tripwire: a new add-event registration whose hook basename + // is missing from the identity table would be invisible to the healer, + // the --no-team sweep, and uninstall. + const rels = [...setupSrc.matchAll(/_hook_command_path (\S+)/g)].map((m) => m[1]); + expect(rels.length).toBeGreaterThanOrEqual(5); + for (const rel of rels) { + const basename = rel.split('/').pop()!; + expect(hookBinSrc).toContain(`"${basename}":`); + } + }); +}); + +describe('setup: heal-first ordering + visibility', () => { + test('prune-stale --repoint runs before any list-sources guard or add-event registration', () => { + const heal = setupSrc.indexOf('prune-stale --repoint'); + const firstGuard = setupSrc.indexOf('list-sources'); + const firstAdd = setupSrc.indexOf('add-event'); + expect(heal).toBeGreaterThan(-1); + expect(heal).toBeLessThan(firstGuard); + expect(heal).toBeLessThan(firstAdd); + }); + + test('the heal call site is not output-suppressed (zero silent settings mutations)', () => { + const lines = setupSrc.split('\n').filter((l) => l.includes('prune-stale --repoint')); + expect(lines.length).toBeGreaterThanOrEqual(1); + for (const line of lines) { + expect(line).not.toContain('>/dev/null'); + expect(line).not.toContain('2>&1'); + } + // Captured for the change-only summary line. + expect(setupSrc).toMatch(/_HEAL_OUT=\$\("\$SETTINGS_HOOK" prune-stale/); + expect(setupSrc).toContain('healed hook registrations'); + }); + + test('heal counters use the ${VAR:-0} idiom, never `grep -c || echo 0`', () => { + // Prior learning grep-c-double-emit-fail-open: `grep -c ... || echo 0` + // double-emits "0\n0" on no-match and breaks numeric guards open. + expect(setupSrc).toContain('${_HEAL_REMOVED:-0}'); + expect(setupSrc).toContain('${_HEAL_REPOINTED:-0}'); + const healRegion = setupSrc.slice( + setupSrc.indexOf('_HEAL_OUT='), + setupSrc.indexOf('healed hook registrations'), + ); + expect(healRegion).not.toMatch(/grep -c .*\|\| echo 0/); + }); + + test('--no-team teardown includes the auq source and the identity sweep', () => { + const idx = setupSrc.indexOf('# Also tear down plan-tune'); + expect(idx).toBeGreaterThan(-1); + const slice = setupSrc.slice(idx, idx + 900); + expect(slice).toContain('remove-source --source plan-tune-cathedral'); + expect(slice).toContain('remove-source --source auq-error-fallback'); + expect(slice).toContain('remove-source --source gstack-timeline-stop'); + expect(slice).toContain('prune-stale --all'); + }); +}); + +describe('gstack-settings-hook: shared prelude (dedupe key == prune predicate)', () => { + test('every bun script call site uses the shared JS prelude concatenation', () => { + const codeLines = hookBinSrc.split('\n').filter((l) => !l.trim().startsWith('#')); + const bunCalls = codeLines.filter((l) => l.includes('bun -e ')); + const preludeCalls = codeLines.filter((l) => l.includes(`bun -e "$_HOOK_JS_PRELUDE"'`)); + expect(bunCalls.length).toBeGreaterThanOrEqual(6); + expect(preludeCalls.length).toBe(bunCalls.length); + }); + + test('the prelude contains no single quotes (single-quoted shell assignment)', () => { + const start = hookBinSrc.indexOf("_HOOK_JS_PRELUDE='"); + expect(start).toBeGreaterThan(-1); + const end = hookBinSrc.indexOf("\n'", start); + const prelude = hookBinSrc.slice(start + "_HOOK_JS_PRELUDE='".length, end); + expect(prelude).not.toContain("'"); + // And no shell-expansion hazards inside the double-quoted call-site expansion. + expect(prelude).not.toContain('`'); + }); + + test('KNOWN_HOOKS table carries all five identities with source+event+relpath', () => { + for (const [name, source, event] of [ + ['question-log-hook', 'plan-tune-cathedral', 'PostToolUse'], + ['question-preference-hook', 'plan-tune-cathedral', 'PreToolUse'], + ['auq-error-fallback-hook', 'auq-error-fallback', 'PostToolUse'], + ['timeline-stop-hook', 'gstack-timeline-stop', 'Stop'], + ['gstack-session-update', 'gstack-session-update', 'SessionStart'], + ]) { + const rowStart = hookBinSrc.indexOf(`"${name}":`); + expect(rowStart).toBeGreaterThan(-1); + const row = hookBinSrc.slice(rowStart, hookBinSrc.indexOf('}', rowStart)); + expect(row).toContain(`source: "${source}"`); + expect(row).toContain(`event: "${event}"`); + expect(row).toContain('relpath: "'); + } + }); +}); + +describe('gstack-uninstall: hook cleanup runs before install-root deletion', () => { + test('the settings cleanup block precedes every install-root rm -rf', () => { + // Pre-fix bug: SETTINGS_HOOK=$(dirname "$0")/gstack-settings-hook resolved + // INSIDE the install root, which was already deleted by the time cleanup + // ran — a real global uninstall silently orphaned every hook. + const cleanup = uninstallSrc.indexOf('Remove gstack hooks from Claude Code settings'); + const rootDelete = uninstallSrc.indexOf('rm -rf "$CLAUDE_SKILLS/gstack"'); + expect(cleanup).toBeGreaterThan(-1); + expect(rootDelete).toBeGreaterThan(-1); + expect(cleanup).toBeLessThan(rootDelete); + }); + + test('uninstall removes all three sources and sweeps untagged strays', () => { + expect(uninstallSrc).toContain('remove-source --source plan-tune-cathedral'); + expect(uninstallSrc).toContain('remove-source --source auq-error-fallback'); + expect(uninstallSrc).toContain('remove-source --source gstack-timeline-stop'); + expect(uninstallSrc).toContain('prune-stale --all'); + }); +}); + +describe('the defect-class warning is written down where the next author will see it', () => { + test('setup carries the never-register-tree-relative-paths warning', () => { + expect(setupSrc).toMatch(/NEVER register .*SOURCE_GSTACK_DIR.*hook paths/); + }); +}); diff --git a/test/setup-plan-tune-hooks-noninteractive.test.ts b/test/setup-plan-tune-hooks-noninteractive.test.ts index 9a0f03ded..cb4871a41 100644 --- a/test/setup-plan-tune-hooks-noninteractive.test.ts +++ b/test/setup-plan-tune-hooks-noninteractive.test.ts @@ -74,6 +74,75 @@ describe('dev-setup: never silently mutates global settings.json', () => { }); }); +describe('setup: PT_EXPLICIT provenance (Conductor auto-opt-in respects explicit decisions)', () => { + // The phantom-hooks root cause (Bug A): the Conductor auto-opt-in upgraded + // PT_DECISION "prompt" → "yes" even when "prompt" came from dev-setup's + // EXPLICIT --plan-tune-hooks=prompt flag, so every new Conductor workspace + // installed hooks pointing at its ephemeral worktree. + + test('flag and env set PT_EXPLICIT=1', () => { + expect(setupSrc).toContain('PT_EXPLICIT=1'); + const flagIdx = setupSrc.indexOf('PT_DECISION="$PLAN_TUNE_HOOKS_MODE"'); + const explicitIdx = setupSrc.indexOf('PT_EXPLICIT=1', flagIdx); + expect(flagIdx).toBeGreaterThan(-1); + expect(explicitIdx).toBeGreaterThan(flagIdx); + }); + + test('the Conductor auto-opt-in fires only on the true silent fall-through', () => { + expect(setupSrc).toMatch( + /\[ "\$PT_DECISION" = "prompt" \] && \[ "\$PT_EXPLICIT" -eq 0 \] && \{ \[ -n "\$\{CONDUCTOR_WORKSPACE_PATH:-\}" \] \|\| \[ -n "\$\{CONDUCTOR_PORT:-\}" \]; \}/, + ); + }); + + test('config provenance uses gstack-config has (env-resolution-safe), never a hardcoded config grep', () => { + // `gstack-config get` returns the default "prompt" for absent keys, so + // key PRESENCE must come from `has`, which resolves GSTACK_STATE_ROOT / + // GSTACK_HOME / GSTACK_STATE_DIR the same way `get` does. A hardcoded + // grep of ~/.gstack/config.yaml misclassifies under env overrides. + expect(setupSrc).toMatch(/"\$GSTACK_CONFIG" has plan_tune_hooks/); + expect(setupSrc).not.toMatch(/grep -q ["']\^plan_tune_hooks:/); + }); +}); + +describe('gstack-config: has subcommand (key-presence provenance)', () => { + let tmpHome2: string; + let env2: NodeJS.ProcessEnv; + + beforeAll(() => { + tmpHome2 = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-cfg-has-')); + env2 = { ...process.env, GSTACK_STATE_ROOT: tmpHome2 }; + }); + + afterAll(() => { + fs.rmSync(tmpHome2, { recursive: true, force: true }); + }); + + function has(key: string): number { + try { + execSync(`${GSTACK_CONFIG} has '${key}'`, { encoding: 'utf-8', env: env2 }); + return 0; + } catch (e: any) { + return e.status ?? 1; + } + } + + test('absent key exits nonzero even though get returns the default', () => { + expect(has('plan_tune_hooks')).not.toBe(0); + const got = execSync(`${GSTACK_CONFIG} get plan_tune_hooks`, { encoding: 'utf-8', env: env2 }).trim(); + expect(got).toBe('prompt'); // default — indistinguishable from a saved value via get + }); + + test('present key exits 0 through the same STATE_DIR resolution as get', () => { + execSync(`${GSTACK_CONFIG} set plan_tune_hooks no`, { encoding: 'utf-8', env: env2 }); + expect(has('plan_tune_hooks')).toBe(0); + // GSTACK_STATE_ROOT was the writer — a hardcoded ~/.gstack grep would miss it. + }); + + test('rejects malformed keys', () => { + expect(has('bad key$(touch /tmp/pwned)')).not.toBe(0); + }); +}); + describe('gstack-config: plan_tune_hooks key', () => { // Isolate state: gstack-config reads $GSTACK_HOME/config.yaml. Point it at a // fresh temp dir so `get` returns the built-in default rather than whatever diff --git a/test/setup-windows-fallback.test.ts b/test/setup-windows-fallback.test.ts index 142bb7912..d1b895056 100644 --- a/test/setup-windows-fallback.test.ts +++ b/test/setup-windows-fallback.test.ts @@ -61,7 +61,11 @@ describe('setup: _link_or_copy invariant (D7)', () => { const hookEnd = SETUP_SRC.indexOf('\nif [ "$TEAM_MODE" -eq 1 ]', hookStart); const hookSection = SETUP_SRC.slice(hookStart, hookEnd); expect(hookSection).toContain('IS_WINDOWS'); - expect(hookSection).toContain('bash $SOURCE_GSTACK_DIR/bin/gstack-session-update'); + // v1.67.2 phantom-hooks fix: the command comes from the CANONICAL install + // via _hook_command_path (never $SOURCE_GSTACK_DIR — ephemeral trees were + // baked into settings.json), but the Windows bash prefix survives. + expect(hookSection).toContain('HOOK_CMD="bash $SESSION_UPDATE_CMD"'); + expect(hookSection).toContain('_hook_command_path bin/gstack-session-update'); }); }); diff --git a/test/uninstall.test.ts b/test/uninstall.test.ts index 1ed3edf83..9cc5d6c8b 100644 --- a/test/uninstall.test.ts +++ b/test/uninstall.test.ts @@ -231,3 +231,70 @@ describe('gstack-uninstall', () => { }); }); }); + +// ---------------------------------------------------------------------- +// Hook-cleanup ordering (phantom-hooks fix). Pre-v1.67.2, the settings +// cleanup ran AFTER `rm -rf $CLAUDE_SKILLS/gstack` — and SETTINGS_HOOK +// resolves via $(dirname "$0") INSIDE that root, so a real global uninstall +// (running the installed copy) silently orphaned every hook. Prior tests +// masked this by running the uninstaller from the repo checkout. This test +// runs the INSTALLED copy. +// ---------------------------------------------------------------------- + +describe('hook cleanup runs before the install root is deleted', () => { + test('uninstall executed FROM the install root still removes hook entries', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-uninstall-order-')); + try { + const mockHome = path.join(tmp, 'home'); + const installRoot = path.join(mockHome, '.claude', 'skills', 'gstack'); + const installBin = path.join(installRoot, 'bin'); + fs.mkdirSync(installBin, { recursive: true }); + // The installed copies — the uninstaller under test IS the one inside + // the root it deletes. + for (const b of ['gstack-uninstall', 'gstack-settings-hook', 'gstack-session-update', 'gstack-config']) { + const src = path.join(ROOT, 'bin', b); + const dst = path.join(installBin, b); + fs.copyFileSync(src, dst); + fs.chmodSync(dst, 0o755); + } + const settingsFile = path.join(mockHome, '.claude', 'settings.json'); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [{ + matcher: '(AskUserQuestion|mcp__.*__AskUserQuestion)', + _gstack_source: 'auq-error-fallback', + hooks: [{ type: 'command', command: '/dead/wt/hosts/claude/hooks/auq-error-fallback-hook', timeout: 5 }], + }], + Stop: [{ + hooks: [{ type: 'command', command: `${installRoot}/hosts/claude/hooks/timeline-stop-hook`, timeout: 5 }], + }], + PreCompact: [{ hooks: [{ type: 'command', command: '/Users/me/my-own-hook' }] }], + }, + }, null, 2)); + fs.mkdirSync(path.join(mockHome, '.gstack'), { recursive: true }); + + const result = spawnSync('bash', [path.join(installBin, 'gstack-uninstall'), '--force', '--keep-state'], { + stdio: 'pipe', + env: { + ...process.env, + HOME: mockHome, + GSTACK_SETTINGS_FILE: settingsFile, + GSTACK_STATE_ROOT: path.join(mockHome, '.gstack'), + }, + cwd: tmp, + }); + expect(result.status).toBe(0); + + // Install root gone… + expect(fs.existsSync(installRoot)).toBe(false); + // …and the hook entries were still cleaned (cleanup ran BEFORE deletion). + const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8')); + expect(s.hooks?.PostToolUse).toBeUndefined(); + expect(s.hooks?.Stop).toBeUndefined(); + // The user's own hook survives the sweep. + expect(s.hooks?.PreCompact?.[0]?.hooks?.[0]?.command).toBe('/Users/me/my-own-hook'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +});