From ae706a616893fad00af69ac041fa2eca47861819 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 17:53:57 +0000 Subject: [PATCH] fix(uninstall): named gstack-memorable arm, vendor-consent notice, honest kept config The identity sweep already removed the Memorable bridge hook as an unnamed stray. It now has a named arm like every other source, so the summary says what went, and says plainly that Memorable's own consent (if the user granted it) is theirs to revoke. Under --keep-state the kept config is set memorable_recall=off so it never claims a hook that is gone. The canonical-paths pins cover the sixth KNOWN_HOOKS row and the new uninstall source. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-uninstall | 10 ++++++ test/setup-hook-canonical-paths.test.ts | 6 ++-- test/uninstall.test.ts | 43 +++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall index d79e0f387..c532a2ac1 100755 --- a/bin/gstack-uninstall +++ b/bin/gstack-uninstall @@ -157,6 +157,16 @@ if [ -x "$SETTINGS_HOOK" ]; then if "$SETTINGS_HOOK" remove-source --source verify-gate | grep -q "removed [1-9]"; then REMOVED+=("verification Stop hook") fi + # Memorable recall bridge (opt-in via bin/gstack-memorable; user-registered, + # ours to sweep). gstack removes only its own hook entry: the vendor's + # consent, if the user granted it, is theirs (`memorable disable|forget`). + if "$SETTINGS_HOOK" remove-source --source gstack-memorable | grep -q "removed [1-9]"; then + REMOVED+=("Memorable UserPromptSubmit hook (Memorable's own consent is unchanged: memorable disable | memorable forget)") + fi + # A kept config must never say memorable_recall=on with no hook behind it. + if [ "$KEEP_STATE" -eq 1 ] && [ -x "$(dirname "$0")/gstack-config" ]; then + "$(dirname "$0")/gstack-config" set memorable_recall off >/dev/null 2>&1 || true + fi # Identity sweep for untagged strays (Claude Code strips _gstack_source # tags; pre-v1.67 setups baked worktree paths). Removes every gstack-owned # hook item, live or dead — the binaries they point at are being deleted. diff --git a/test/setup-hook-canonical-paths.test.ts b/test/setup-hook-canonical-paths.test.ts index b5f4b8a9b..db26047e0 100644 --- a/test/setup-hook-canonical-paths.test.ts +++ b/test/setup-hook-canonical-paths.test.ts @@ -143,13 +143,14 @@ describe('gstack-settings-hook: shared prelude (dedupe key == prune predicate)', expect(prelude).not.toContain('`'); }); - test('KNOWN_HOOKS table carries all five identities with source+event+relpath', () => { + test('KNOWN_HOOKS table carries all six 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'], + ['memorable-user-prompt-hook', 'gstack-memorable', 'UserPromptSubmit'], ]) { const rowStart = hookBinSrc.indexOf(`"${name}":`); expect(rowStart).toBeGreaterThan(-1); @@ -173,10 +174,11 @@ describe('gstack-uninstall: hook cleanup runs before install-root deletion', () expect(cleanup).toBeLessThan(rootDelete); }); - test('uninstall removes all three sources and sweeps untagged strays', () => { + test('uninstall removes every named source 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('remove-source --source gstack-memorable'); expect(uninstallSrc).toContain('prune-stale --all'); }); }); diff --git a/test/uninstall.test.ts b/test/uninstall.test.ts index 1590c4894..d41bcee03 100644 --- a/test/uninstall.test.ts +++ b/test/uninstall.test.ts @@ -309,6 +309,49 @@ describe('hook cleanup runs before the install root is deleted', () => { }, 30000); }); +describe('the Memorable bridge hook is removed by name and the kept config is left honest', () => { + test('a tag-stripped memorable entry is removed, reported, and memorable_recall is set off under --keep-state', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-uninstall-memo-')); + 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 }); + for (const b of ['gstack-uninstall', 'gstack-settings-hook', 'gstack-session-update', 'gstack-config']) { + const dst = path.join(installBin, b); + fs.copyFileSync(path.join(ROOT, 'bin', b), dst); + fs.chmodSync(dst, 0o755); + } + const settingsFile = path.join(mockHome, '.claude', 'settings.json'); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: `${installRoot}/hosts/claude/hooks/memorable-user-prompt-hook`, timeout: 5 }] }, + { hooks: [{ type: 'command', command: '"/Users/me/.memorable/bin/memorable" hook user-prompt' }] }, + ], + }, + }, null, 2)); + const stateRoot = path.join(mockHome, '.gstack'); + fs.mkdirSync(stateRoot, { recursive: true }); + const env = { ...process.env, HOME: mockHome, GSTACK_SETTINGS_FILE: settingsFile, GSTACK_STATE_ROOT: stateRoot }; + spawnSync('bash', [path.join(installBin, 'gstack-config'), 'set', 'memorable_recall', 'on'], { env, timeout: 20_000 }); + + const result = spawnSync('bash', [path.join(installBin, 'gstack-uninstall'), '--force', '--keep-state'], { + stdio: 'pipe', timeout: 30_000, env, cwd: tmp, encoding: 'utf-8', + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('Memorable UserPromptSubmit hook'); + const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8')); + // gstack's entry gone, the vendor's own entry untouched + expect(s.hooks.UserPromptSubmit).toHaveLength(1); + expect(s.hooks.UserPromptSubmit[0].hooks[0].command).toContain('.memorable/bin/memorable'); + expect(fs.readFileSync(path.join(stateRoot, 'config.yaml'), 'utf-8')).toMatch(/memorable_recall: off/); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, 30000); +}); + describe('hook cleanup under lock contention is loud, never silent (review-army)', () => { test('a held foreign lock during uninstall surfaces the give-up warning on stderr', () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-uninstall-lock-'));