diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index 1cd62d41f..69902289d 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -81,7 +81,8 @@ var KNOWN_HOOKS = { "question-preference-hook": { source: "plan-tune-cathedral", event: "PreToolUse", matcher: "(AskUserQuestion|mcp__.*__AskUserQuestion)", relpath: "hosts/claude/hooks/question-preference-hook" }, "auq-error-fallback-hook": { source: "auq-error-fallback", event: "PostToolUse", matcher: "(AskUserQuestion|mcp__.*__AskUserQuestion)", relpath: "hosts/claude/hooks/auq-error-fallback-hook" }, "timeline-stop-hook": { source: "gstack-timeline-stop", event: "Stop", matcher: "", relpath: "hosts/claude/hooks/timeline-stop-hook" }, - "gstack-session-update": { source: "gstack-session-update", event: "SessionStart", matcher: "", relpath: "bin/gstack-session-update" } + "gstack-session-update": { source: "gstack-session-update", event: "SessionStart", matcher: "", relpath: "bin/gstack-session-update" }, + "gstack-verify-gate": { source: "verify-gate", event: "Stop", matcher: "", relpath: "bin/gstack-verify-gate" } }; function gsHadBashPrefix(c) { return String(c == null ? "" : c).trim().indexOf("bash ") === 0; } function gsStripWrap(c) { @@ -105,9 +106,18 @@ function gsOwnedRow(cmd, event, matcher) { if (row.matcher && (matcher || "") !== row.matcher) return null; return row; } +function gsWinPath(p) { + // Git Bash writes MSYS-form paths (/c/Users/...) into settings.json, but + // native bun resolves them drive-relative (C:\c\Users\...) -- translate for + // fs calls only; stored commands keep the form the firing shell expects. + if (process.platform === "win32" && /^\/[A-Za-z]\//.test(p)) { + return p.charAt(1) + ":" + p.slice(2); + } + return p; +} function gsIsAlive(cmd) { var fs = require("fs"); - var p = gsStripWrap(cmd); + var p = gsWinPath(gsStripWrap(cmd)); if (!p) return false; try { if (process.platform === "win32") return fs.existsSync(p); @@ -364,7 +374,12 @@ case "$ACTION" in if (!settings.hooks) settings.hooks = {}; if (!settings.hooks[event]) settings.hooks[event] = []; - const hookEntry = { type: "command", command: cmd }; + // add-event is the single quoting authority: normalize the command + // through the same round-trip the healer uses so metachar paths are + // registered in the escaped-quoted form from the start (a caller-side + // quoting step would drift per call site). + const cmdNorm = gsQuoteCmd(gsStripWrap(cmd), gsHadBashPrefix(cmd)); + const hookEntry = { type: "command", command: cmdNorm }; if (timeoutRaw) { const n = Number(timeoutRaw); if (Number.isFinite(n) && n > 0) hookEntry.timeout = n; @@ -379,12 +394,12 @@ case "$ACTION" in for (const entry of settings.hooks[event]) { if ((entry.matcher || "") !== matcher) continue; if (!Array.isArray(entry.hooks)) continue; - let idx = entry.hooks.findIndex(h => h && h.command === cmd); + let idx = entry.hooks.findIndex(h => h && h.command === cmdNorm); if (idx < 0) { idx = entry.hooks.findIndex(h => { if (!h || !h.command) return false; const row = gsOwnedRow(h.command, event, entry.matcher || ""); - return !!row && gsBaseOf(h.command) === gsBaseOf(cmd); + return !!row && gsBaseOf(h.command) === gsBaseOf(cmdNorm); }); } if (idx < 0 && entry._gstack_source === source && entry.hooks.length === 1) { @@ -542,6 +557,14 @@ case "$ACTION" in let removedHere = 0; const remain = []; const seenInEntry = new Set(); + if (entry.hooks.length === 0) { + // Started-empty entries are foreign data we never touched -- + // preserve them (only a gstack-tagged empty entry is claimable, + // and only by the --all sweep). + if (all && entry._gstack_source) { removed++; continue; } + rebuilt.push(entry); + continue; + } for (const h of entry.hooks) { const cmdRaw = h && h.command; const row = cmdRaw ? gsOwnedRow(cmdRaw, event, matcher) : null; diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall index d131fc71d..d79e0f387 100755 --- a/bin/gstack-uninstall +++ b/bin/gstack-uninstall @@ -153,6 +153,10 @@ if [ -x "$SETTINGS_HOOK" ]; then if "$SETTINGS_HOOK" remove-source --source gstack-timeline-stop | grep -q "removed [1-9]"; then REMOVED+=("timeline Stop hook") fi + # Verification stop hook (opt-in via README; user-registered, ours to sweep). + if "$SETTINGS_HOOK" remove-source --source verify-gate | grep -q "removed [1-9]"; then + REMOVED+=("verification Stop hook") + 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/setup b/setup index 0a4435d81..947300724 100755 --- a/setup +++ b/setup @@ -1870,6 +1870,14 @@ SETTINGS_HOOK="$SOURCE_GSTACK_DIR/bin/gstack-settings-hook" # WARNING for future code AND migrations (the v1.58.0.0.sh defect class): # NEVER register ${SCRIPT_DIR}/$SOURCE_GSTACK_DIR-relative hook paths. CANONICAL_GSTACK_ROOT="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/skills/gstack" +# Split-brain guard: the installer currently hardcodes $HOME/.claude/skills +# (setup:1601 TODO), so a CLAUDE_CONFIG_DIR override can name a root that was +# never installed. Fall back to where the install actually lives — both are +# stable, neither is the running tree, so canonical-only still holds. +if [ ! -x "$CANONICAL_GSTACK_ROOT/bin/gstack-session-update" ] \ + && [ -x "$HOME/.claude/skills/gstack/bin/gstack-session-update" ]; then + CANONICAL_GSTACK_ROOT="$HOME/.claude/skills/gstack" +fi # Echo the canonical path for a hook (repo-relative arg); fails when the hook # is not executable at the canonical install — callers must skip + log. @@ -1920,14 +1928,9 @@ fi SESSION_UPDATE_CMD="$(_hook_command_path bin/gstack-session-update || true)" HOOK_CMD="" if [ -n "$SESSION_UPDATE_CMD" ]; then - # The registered command is executed by a shell when Claude Code fires the - # hook — neutralize shell metacharacters, not just whitespace (mirrors - # gsQuoteCmd in gstack-settings-hook). - case "$SESSION_UPDATE_CMD" in - *[\ \"\$\`\\]*) - SESSION_UPDATE_CMD="\"$(printf '%s' "$SESSION_UPDATE_CMD" | sed 's/[\\"$`]/\\&/g')\"" - ;; - esac + # No caller-side quoting: add-event is the single quoting authority — it + # normalizes every registered command through the same gsQuoteCmd round-trip + # the healer uses, so metachar/space paths cannot drift per call site. if [ "$IS_WINDOWS" -eq 1 ]; then HOOK_CMD="bash $SESSION_UPDATE_CMD" else @@ -2073,6 +2076,14 @@ fi PLAN_TUNE_LOG_HOOK="$(_hook_command_path hosts/claude/hooks/question-log-hook || true)" PLAN_TUNE_PREF_HOOK="$(_hook_command_path hosts/claude/hooks/question-preference-hook || true)" AUQ_ERROR_FALLBACK_HOOK="$(_hook_command_path hosts/claude/hooks/auq-error-fallback-hook || true)" +# Windows: extensionless bash shims need the explicit 'bash ' prefix (same +# rationale as HOOK_CMD above — the OS file-association dialog otherwise). +# KNOWN_HOOKS identity round-trips the prefix, so healing preserves it. +if [ "$IS_WINDOWS" -eq 1 ]; then + [ -n "$PLAN_TUNE_LOG_HOOK" ] && PLAN_TUNE_LOG_HOOK="bash $PLAN_TUNE_LOG_HOOK" + [ -n "$PLAN_TUNE_PREF_HOOK" ] && PLAN_TUNE_PREF_HOOK="bash $PLAN_TUNE_PREF_HOOK" + [ -n "$AUQ_ERROR_FALLBACK_HOOK" ] && AUQ_ERROR_FALLBACK_HOOK="bash $AUQ_ERROR_FALLBACK_HOOK" +fi PLAN_TUNE_INSTALL_MARKER="$HOME/.gstack/.plan-tune-hooks-prompted" # Canonical-only: an ephemeral tree with no stable install gets a visible skip, @@ -2275,13 +2286,18 @@ fi # a session. Idempotent via the (event, source) dedup in gstack-settings-hook; # removed by --no-team and gstack-uninstall. TIMELINE_STOP_HOOK="$(_hook_command_path hosts/claude/hooks/timeline-stop-hook || true)" +if [ "$IS_WINDOWS" -eq 1 ] && [ -n "$TIMELINE_STOP_HOOK" ]; then + TIMELINE_STOP_HOOK="bash $TIMELINE_STOP_HOOK" +fi +# stderr stays attached on both calls below (zero silent settings mutations; +# the list-sources corrupt-file warning must reach the user too). if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -n "$TIMELINE_STOP_HOOK" ]; then - if ! "$SETTINGS_HOOK" list-sources 2>/dev/null | grep -q "gstack-timeline-stop"; then + if ! "$SETTINGS_HOOK" list-sources | grep -q "gstack-timeline-stop"; then if "$SETTINGS_HOOK" add-event \ --event Stop \ --command "$TIMELINE_STOP_HOOK" \ --source gstack-timeline-stop \ - --timeout 5 >/dev/null 2>&1; then + --timeout 5 >/dev/null; then log " registered Stop hook: session timeline entries now close even when a skill is interrupted (backup: settings.json.bak.; remove: $SETTINGS_HOOK remove-source --source gstack-timeline-stop)" fi fi diff --git a/test/gstack-settings-hook-schema-aware.test.ts b/test/gstack-settings-hook-schema-aware.test.ts index f8e0cfc62..e5f6873e6 100644 --- a/test/gstack-settings-hook-schema-aware.test.ts +++ b/test/gstack-settings-hook-schema-aware.test.ts @@ -597,6 +597,56 @@ describe('review-army hardening (specialist findings)', () => { expect(fs.existsSync(latest)).toBe(true); }); + test('tag-stripped verify-gate entry is table-owned: healed by --repoint, swept by --all', () => { + // Red-team catch: verify-gate is a README-documented opt-in Stop hook. + // Without a KNOWN_HOOKS row, a tag-stripped entry survived uninstall and + // errored at the end of EVERY turn after the install root was deleted. + const canon = mkCanon(tmpDir); + const vg = path.join(canon, 'bin', 'gstack-verify-gate'); + fs.writeFileSync(vg, '#!/bin/sh\n'); + fs.chmodSync(vg, 0o755); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { Stop: [hookEntry('/dead/install/bin/gstack-verify-gate')] }, // tag STRIPPED + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + let s = settings(); + expect(s.hooks.Stop[0].hooks[0].command).toBe(vg); + expect(s.hooks.Stop[0]._gstack_source).toBe('verify-gate'); + const r = runIso(['prune-stale', '--all']); + expect(r.stdout).toMatch(/removed 1/); + expect(settings().hooks).toBeUndefined(); + }); + + test('a foreign entry that STARTED empty survives prune-stale untouched', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { PreToolUse: [{ matcher: 'Bash', hooks: [] }] }, + }, null, 2) + '\n'); + const before = fs.readFileSync(settingsFile, 'utf-8'); + const r = runIso(['prune-stale', '--repoint', mkCanon(tmpDir, 'c2')]); + expect(r.stdout).toMatch(/removed 0 gstack hook entries \(repointed 0\)/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + }); + + test('add-event is the quoting authority: spaced canonical path stored escaped-quoted, healer idempotent', () => { + // Red-team catch (empirically verified pre-fix): setup registered raw + // paths and the very next heal rewrote them — fresh installs shipped a + // form the codebase itself considered wrong. + const spacedBase = path.join(tmpDir, 'canon root'); + fs.mkdirSync(spacedBase, { recursive: true }); + const canon = mkCanon(spacedBase); + runIso([ + 'add-event', '--event', 'PostToolUse', '--matcher', AUQ_MATCHER, + '--command', `${canon}/hosts/claude/hooks/question-log-hook`, + '--source', 'plan-tune-cathedral', '--timeout', '5', + ]); + const stored = settings().hooks.PostToolUse[0].hooks[0].command; + expect(stored).toBe(`"${canon}/hosts/claude/hooks/question-log-hook"`); + const before = fs.readFileSync(settingsFile, 'utf-8'); + const r = runIso(['prune-stale', '--repoint', canon]); + expect(r.stdout).toMatch(/removed 0 gstack hook entries \(repointed 0\)/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(before); + }); + test('rollback refuses a pointer that names a non-backup file', () => { fs.writeFileSync(settingsFile, JSON.stringify({ a: 1 }, null, 2)); const evil = path.join(tmpDir, 'evil.json');