Merge remote-tracking branch 'origin/main' into phantom-askuserquestion-hooks

# Conflicts:
#	CHANGELOG.md
#	TODOS.md
#	VERSION
#	bin/gstack-settings-hook
#	package.json
#	setup

test/timeline-stop-hook.test.ts

@@ -225,6 +225,8 @@ describe('timeline-stop-hook (#2553, F5 fail-open)', () => {
 });
 
 describe('timeline-stop-hook wiring', () => {
+  const SETTINGS_HOOK = path.join(ROOT, 'bin', 'gstack-settings-hook');
+
   test('setup registers the Stop hook with its own source tag and tears it down on --no-team', () => {
     const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
     expect(setup).toContain('--event Stop');
@@ -235,6 +237,240 @@ describe('timeline-stop-hook wiring', () => {
     expect(teardown).toContain('remove-source --source gstack-timeline-stop');
   });
 
+  test('setup surfaces a settings-hook refusal instead of swallowing it', () => {
+    // The hardened settings-hook refuses to rewrite a corrupt settings.json
+    // (exit 1). Both setup call sites (ALREADY_INSTALLED plan-tune re-point,
+    // timeline ensure-event) must stay non-fatal but PRINT the failure.
+    const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
+    const warnings = setup.match(/settings hook update failed/g) || [];
+    expect(warnings.length).toBeGreaterThanOrEqual(2);
+    // The old swallow patterns are gone (the --no-team remove-source teardown
+    // legitimately keeps its 2>/dev/null; only the ensure-event registration
+    // must surface stderr).
+    expect(setup).not.toContain('_install_plan_tune_hooks >/dev/null 2>&1 || true');
+    expect(setup).not.toMatch(/ensure-event[\s\S]{0,220}--source gstack-timeline-stop[\s\S]{0,40}2>\/dev\/null/);
+  });
+
+  test('setup routes the Stop hook through ensure-event, not presence-only dedup', () => {
+    const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
+    // ensure-event registers when missing AND re-points a stale path in place.
+    expect(setup).toMatch(/ensure-event[\s\S]{0,220}--source gstack-timeline-stop/);
+    // The old guard skipped registration whenever the source tag was merely
+    // PRESENT, so a stale absolute path (deleted dev worktree) was never
+    // re-pointed on a setup re-run.
+    expect(setup).not.toMatch(/list-sources 2>\/dev\/null \| grep -q "gstack-timeline-stop"/);
+  });
+
+  test('hook path resolution is canonical-only: global install or skip, never the worktree', () => {
+    // Drive setup's _hook_command_path directly: canonical install present →
+    // that path (survives deleting the worktree setup ran from). Absent →
+    // non-zero and NO output — registration is skipped with a log line; the
+    // running tree's path is NEVER baked into settings.json (the SOURCE
+    // fallback was the phantom-hooks defect and is deliberately gone).
+    const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
+    const fn = setup.match(/_hook_command_path\(\) \{[\s\S]*?\n\}/);
+    expect(fn).not.toBeNull();
+
+    const fakeHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hookpath-'));
+    try {
+      const canonicalRoot = path.join(fakeHome, '.claude', 'skills', 'gstack');
+      const globalHook = path.join(canonicalRoot, 'hosts', 'claude', 'hooks', 'timeline-stop-hook');
+      fs.mkdirSync(path.dirname(globalHook), { recursive: true });
+      fs.writeFileSync(globalHook, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
+      const env = {
+        ...process.env,
+        HOME: fakeHome,
+        SOURCE_GSTACK_DIR: '/some/dev/worktree',
+        CANONICAL_GSTACK_ROOT: canonicalRoot,
+      };
+
+      const withGlobal = spawnSync(
+        'bash',
+        ['-c', `${fn![0]}\n_hook_command_path hosts/claude/hooks/timeline-stop-hook`],
+        { env, encoding: 'utf-8', timeout: 10_000 },
+      );
+      expect(withGlobal.status).toBe(0);
+      expect(withGlobal.stdout.trim()).toBe(globalHook);
+
+      // No canonical install → the resolver FAILS (caller logs a visible
+      // skip); it never falls back to the setup-time tree.
+      fs.rmSync(globalHook);
+      const withoutGlobal = spawnSync(
+        'bash',
+        ['-c', `${fn![0]}\n_hook_command_path hosts/claude/hooks/timeline-stop-hook`],
+        { env, encoding: 'utf-8', timeout: 10_000 },
+      );
+      expect(withoutGlobal.status).not.toBe(0);
+      expect(withoutGlobal.stdout.trim()).toBe('');
+    } finally {
+      fs.rmSync(fakeHome, { recursive: true, force: true });
+    }
+  });
+
+  test('ensure-event re-points a stale absolute path and leaves exactly one registration', () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-'));
+    try {
+      const settingsFile = path.join(dir, 'settings.json');
+      fs.writeFileSync(settingsFile, JSON.stringify({
+        hooks: {
+          Stop: [{
+            _gstack_source: 'gstack-timeline-stop',
+            hooks: [{ type: 'command', command: '/deleted/worktree/hosts/claude/hooks/timeline-stop-hook', timeout: 5 }],
+          }],
+        },
+      }, null, 2) + '\n');
+
+      const r = spawnSync('bash', [
+        SETTINGS_HOOK, 'ensure-event',
+        '--event', 'Stop',
+        '--command', HOOK,
+        '--source', 'gstack-timeline-stop',
+        '--timeout', '5',
+      ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
+
+      expect(r.status).toBe(0);
+      expect(r.stdout).toContain('re-pointed');
+      const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8'));
+      expect(s.hooks.Stop).toHaveLength(1); // replaced in place — never two
+      expect(s.hooks.Stop[0].hooks[0].command).toBe(HOOK);
+      expect(s.hooks.Stop[0]._gstack_source).toBe('gstack-timeline-stop');
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test('ensure-event is a true no-op when the registration already matches (no write, no backup churn)', () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-noop-'));
+    try {
+      const settingsFile = path.join(dir, 'settings.json');
+      const args = [
+        SETTINGS_HOOK, 'ensure-event',
+        '--event', 'Stop',
+        '--command', HOOK,
+        '--source', 'gstack-timeline-stop',
+        '--timeout', '5',
+      ];
+      const env = { ...process.env, GSTACK_SETTINGS_FILE: settingsFile };
+      const first = spawnSync('bash', args, { env, encoding: 'utf-8', timeout: 15_000 });
+      expect(first.status).toBe(0);
+      const bytesAfterFirst = fs.readFileSync(settingsFile, 'utf-8');
+
+      const second = spawnSync('bash', args, { env, encoding: 'utf-8', timeout: 15_000 });
+      expect(second.status).toBe(0);
+      expect(second.stdout).toContain('unchanged');
+      expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(bytesAfterFirst);
+      // Re-running ./setup must not accumulate settings.json.bak.<ts> files.
+      const baks = fs.readdirSync(dir).filter((f) => f.includes('.bak'));
+      expect(baks).toEqual([]);
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test('corrupt settings.json: ensure-event refuses (exit 3) and never rewrites the file', () => {
+    // The old catch{} folded an unparseable EXISTING settings.json into {}
+    // and the atomic write replaced the user's permissions/env/other hooks
+    // with just ours. Now: loud stderr error, fail-closed exit 3 (the
+    // settings-hook parse-refusal code), file byte-identical.
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-corrupt-'));
+    try {
+      const settingsFile = path.join(dir, 'settings.json');
+      const corrupt = '{ "permissions": { "allow": ["Bash(npm:*)"] }, INVALID';
+      fs.writeFileSync(settingsFile, corrupt);
+
+      const r = spawnSync('bash', [
+        SETTINGS_HOOK, 'ensure-event',
+        '--event', 'Stop',
+        '--command', HOOK,
+        '--source', 'gstack-timeline-stop',
+        '--timeout', '5',
+      ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
+
+      expect(r.status).toBe(3);
+      expect(r.stderr).toContain('not valid JSON');
+      // Never rewritten — the corrupt bytes (and whatever the user can still
+      // salvage from them) survive verbatim.
+      expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(corrupt);
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test('a matcher change updates the tagged entry in place — still exactly one registration', () => {
+    // Identity key is (event, source): an existing gstack entry with a STALE
+    // matcher must be updated, never joined by a second entry (the old key
+    // included the matcher, so any future matcher change would duplicate).
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-matcher-'));
+    try {
+      const settingsFile = path.join(dir, 'settings.json');
+      fs.writeFileSync(settingsFile, JSON.stringify({
+        hooks: {
+          PreToolUse: [{
+            _gstack_source: 'gstack-plan-tune',
+            matcher: 'OldMatcher',
+            hooks: [{ type: 'command', command: '/old/path/hook', timeout: 5 }],
+          }],
+        },
+      }, null, 2) + '\n');
+
+      const r = spawnSync('bash', [
+        SETTINGS_HOOK, 'ensure-event',
+        '--event', 'PreToolUse',
+        '--command', '/new/path/hook',
+        '--source', 'gstack-plan-tune',
+        '--matcher', 'NewMatcher',
+        '--timeout', '5',
+      ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
+
+      expect(r.status).toBe(0);
+      const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8'));
+      expect(s.hooks.PreToolUse).toHaveLength(1); // updated in place — never two
+      expect(s.hooks.PreToolUse[0].matcher).toBe('NewMatcher');
+      expect(s.hooks.PreToolUse[0].hooks[0].command).toBe('/new/path/hook');
+      expect(s.hooks.PreToolUse[0]._gstack_source).toBe('gstack-plan-tune');
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test('a failed update leaves exactly one registration — never zero, never two', () => {
+    // Root can write through 0o555 directories, so the failure injection
+    // (read-only dir) does not bind there; the invariant is still covered by
+    // the atomic tmp+rename pinned in the re-point test above.
+    if (typeof process.getuid === 'function' && process.getuid() === 0) return;
+
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-ensure-fail-'));
+    try {
+      const settingsFile = path.join(dir, 'settings.json');
+      fs.writeFileSync(settingsFile, JSON.stringify({
+        hooks: {
+          Stop: [{
+            _gstack_source: 'gstack-timeline-stop',
+            hooks: [{ type: 'command', command: '/stale/path/timeline-stop-hook', timeout: 5 }],
+          }],
+        },
+      }, null, 2) + '\n');
+
+      fs.chmodSync(dir, 0o555); // every write path (backup, tmp, rename) fails
+      const r = spawnSync('bash', [
+        SETTINGS_HOOK, 'ensure-event',
+        '--event', 'Stop',
+        '--command', HOOK,
+        '--source', 'gstack-timeline-stop',
+        '--timeout', '5',
+      ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 });
+      fs.chmodSync(dir, 0o755);
+
+      expect(r.status).not.toBe(0); // the failure is loud, not swallowed
+      const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8'));
+      expect(s.hooks.Stop).toHaveLength(1); // old registration intact
+      expect(s.hooks.Stop[0].hooks[0].command).toBe('/stale/path/timeline-stop-hook');
+    } finally {
+      try { fs.chmodSync(dir, 0o755); } catch {}
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
   test('gstack-uninstall removes the Stop hook registration', () => {
     const uninstall = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-uninstall'), 'utf-8');
     expect(uninstall).toContain('remove-source --source gstack-timeline-stop');