diff --git a/CHANGELOG.md b/CHANGELOG.md index e74cb7561..ff11c3f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,56 @@ # Changelog +## [1.68.1.0] - 2026-08-18 + +**Phantom hook errors are dead. Your settings.json now heals itself** +**on every setup, and no ephemeral path can ever be baked in again.** + +If you work in Conductor workspaces or git worktrees, you have probably seen it: `PostToolUse:AskUserQuestion hook error ... No such file or directory` spraying on every question, pointing at a workspace you deleted last week. The cause was a three-part failure. Setup baked the running tree's physical path into your global `~/.claude/settings.json`, the Conductor auto-opt-in overrode the exact flag `bin/dev-setup` passes to prevent that, and the dedupe tag gstack relied on gets stripped by Claude Code itself, so every new workspace appended a fresh dead entry instead of replacing the old one. + +All three are fixed at the root. Hook registration is now canonical-only: commands point at the stable `~/.claude/skills/gstack` install or are not registered at all. Ownership is decided by a fixed identity table in `bin/gstack-settings-hook`, per hook item, so it survives tag-stripping and can never claim a hook you wrote yourself. And every `./setup` run now heals first: `gstack-settings-hook prune-stale --repoint` removes dead gstack entries, re-points stale ones, restores stripped tags, and collapses duplicates, printing one line only when it changed something. + +### The numbers that matter + +Source: the 2026-08-17 incident on a real dev box, replayed byte-for-byte as the `incident facsimile` test in `test/gstack-settings-hook-schema-aware.test.ts`. + +| Metric | Before | After | Δ | +|--------|--------|-------|---| +| Hook entries in settings.json | 11 (6 dead) | 5, all canonical | −6 dead | +| Error lines per AskUserQuestion | 4 | 0 | −4 | +| Hook processes spawned per question that do nothing | 4 | 0 | −4 | +| Traced code paths under test | — | 53 of 61 (87%) | new | + +The healer also fixes damage you could not see: a corrupt settings.json is never overwritten (every mutator now fails closed instead of clobbering it with `{}`), a user-tightened 0600 file keeps its mode across rewrites (settings.json can carry API keys), concurrent setups can no longer rename a half-written temp file into place, and uninstall now cleans hooks BEFORE deleting the install root, which previously made cleanup silently no-op in exactly the case it existed for. + +### What this means for you + +Run `./setup` (or `/gstack-upgrade`) once and the errors stop, on every machine, with a printed receipt of what was healed and a backup beside the file. New workspaces can never reintroduce them. If you ever want everything gone, `gstack-uninstall` now actually removes every gstack hook, including the ones an older version orphaned. + +### Itemized changes + +### Added +- `gstack-settings-hook prune-stale [--repoint ] [--all]`: self-healing for hook registrations. Dead gstack entries pruned, stale paths re-pointed at the stable install, stripped `_gstack_source` tags restored from the identity table, exact duplicates and within-entry twins collapsed. Runs automatically at the start of every `./setup`; `--all` is the complete teardown sweep used by uninstall and `--no-team`. +- `gstack-config has `: key-presence check through the same state-dir resolution as `get` (which returns defaults for absent keys), so consent logic can tell a recorded decision from a default. +- KNOWN_HOOKS identity table covering all six gstack hooks (plan-tune trio, timeline Stop, session update, verify-gate), shared by registration dedupe and the healer so the two can never drift. +- A mutation lock around every settings.json write: mkdir-based with an owner token, ownership-checked release, and atomic stale-lock takeover. Backups get unique names and rotate (10 kept); `rollback` validates its pointer and restores atomically. + +### Changed +- Hook registration is canonical-only. Setup never writes a running-tree path into global settings; if the stable install is missing a hook, it skips with a visible log line instead. The Conductor auto-opt-in for AskUserQuestion reliability hooks now respects explicit decisions (flag, env, or a recorded config key) and fires only on the true silent fall-through. +- `add-event` is the single quoting authority: registered commands are normalized once (whitespace and shell metacharacters escaped), so a spaced or `$`-bearing install path produces a working hook from the first registration. Windows gets the required `bash ` prefix on all hooks, not just SessionStart, and MSYS-form paths no longer read as dead to the healer. +- All settings.json mutators are per-item: a hook you co-located in the same entry as a gstack hook survives every gstack operation, including uninstall, and gstack never tags an entry that contains your items. +- Teardown paths (`gstack-uninstall`, `./setup --no-team`) run hook cleanup before any deletion, sweep untagged strays by identity, and keep stderr attached so a skipped cleanup is loud, never silent. + +### Fixed +- Deleted Conductor workspaces and worktrees no longer leave dead hooks erroring on every AskUserQuestion, session start, and stop event. +- A corrupt settings.json is preserved and reported (exit 3) instead of being replaced with an empty object by the next hook operation. +- settings.json file mode is preserved across rewrites; fresh files are created 0600. +- Liveness checks treat only provable absence as dead, so an unmounted volume or permission blip cannot prune a working hook. +- A vacuous test in the banner-tripwire check executed its script through JSON-as-shell-quoting, silently littering a `2nelsen` artifact in the repo root on every suite run while asserting nothing; it now passes the script as argv and asserts both branches. + +### For contributors +- 60+ new or updated test cases across 8 files, including the incident facsimile, a two-writer concurrency smoke, an uninstall test that runs the installed copy from inside the root it deletes, held-lock teardown visibility, quoting round-trips, and static tripwires pinning canonical-only registration, heal-first ordering, matcher-literal parity, and the shared-prelude call sites. +- The review pipeline for this release (five specialists plus red team plus two Codex passes) contributed 14 verified hardening fixes; rejected findings are documented in the PR. + ## [1.68.0.0] - 2026-08-18 **The next tracker wave: 16 verified fixes in, 90 stale PRs and 21 issues out.** diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5584644b9..77e188ae8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -431,7 +431,7 @@ If you're using [Conductor](https://conductor.build) to run multiple Claude Code When Conductor creates a new workspace, `bin/dev-setup` runs automatically. It detects the main worktree (via `git worktree list`), copies your `.env` so API keys carry over, and sets up dev mode — no manual steps needed. -`bin/dev-setup` runs `./setup` fully non-interactively (it passes `--plan-tune-hooks=prompt` and closes stdin), so a forwarded Conductor TTY can never hang on a hidden setup prompt. It also never installs the plan-tune Claude Code hooks, which means a throwaway workspace can't rewrite your global `~/.claude/settings.json` to point at an ephemeral worktree path. To install the plan-tune hooks deliberately, run `./setup --plan-tune-hooks` outside dev-setup (or `gstack-config set plan_tune_hooks yes`). +`bin/dev-setup` runs `./setup` fully non-interactively (it passes `--plan-tune-hooks=prompt` and closes stdin), so a forwarded Conductor TTY can never hang on a hidden setup prompt. It also never installs the plan-tune Claude Code hooks, which means a throwaway workspace can't rewrite your global `~/.claude/settings.json` to point at an ephemeral worktree path. To install the plan-tune hooks deliberately, run `./setup --plan-tune-hooks` outside dev-setup (or `gstack-config set plan_tune_hooks yes`). The explicit flag counts as an explicit decision: setup's Conductor auto-opt-in for AskUserQuestion hooks fires only on the true silent fall-through (no flag, no `GSTACK_PLAN_TUNE_HOOKS` env var, no `plan_tune_hooks` key literally present in config, checked via `gstack-config has`), so it can never override dev-setup into installing hooks. One stated repair exception: setup's heal-first pass (`gstack-settings-hook prune-stale --repoint`) may prune dead gstack hook entries and re-point existing ones at the stable `~/.claude/skills/gstack` install. That is strictly convergent repair, never a new registration, and registration itself is canonical-only, so an ephemeral tree path can never be baked into settings.json. **First-time setup:** Put your `ANTHROPIC_API_KEY` in `.env` in the main repo (see `.env.example`). Every Conductor workspace inherits it automatically. diff --git a/README.md b/README.md index f284f8c65..7b2b9305e 100644 --- a/README.md +++ b/README.md @@ -272,6 +272,14 @@ a session). Skip it with `./setup --no-team`, remove it with `gstack-settings-hook remove-source --source gstack-timeline-stop`; `gstack-uninstall` removes it too. +Hook registration is canonical-only: every hook command points at the stable +`~/.claude/skills/gstack` install, never the tree setup ran from, so deleting +a worktree or Conductor workspace can't leave dead hooks erroring in your +sessions. Every `./setup` run also heals first: `gstack-settings-hook +prune-stale --repoint` removes dead gstack hook entries, re-points stale ones +at the stable install, and collapses duplicates, printing one line (and +writing a backup beside the file) only when it changed something. + ### Continuous checkpoint mode (opt-in, local by default) Set `gstack-config set checkpoint_mode continuous` and skills auto-commit your work as you go with a `WIP:` prefix plus a structured `[gstack-context]` body (decisions, remaining work, failed approaches). Survives crashes and context switches. `/context-restore` reads those commits to reconstruct session state. `/ship` filter-squashes WIP commits before the PR (preserving non-WIP commits) so bisect stays clean. Push is opt-in via `checkpoint_push=true` — default is local-only so you don't trigger CI on every WIP commit. @@ -399,9 +407,13 @@ rm -rf .gstack .gstack-worktrees .claude/skills/gstack 2>/dev/null rm -rf .agents/skills/gstack* .factory/skills/gstack* 2>/dev/null ``` -Manual removal leaves the gstack Stop hook entry behind in `~/.claude/settings.json` -(the uninstall script removes it for you). Edit that file and delete the hook whose -command path ends in `hosts/claude/hooks/timeline-stop-hook`. +Manual removal leaves gstack's hook entries behind in `~/.claude/settings.json` +(the uninstall script removes all of them for you, including entries whose +`_gstack_source` tag was stripped). Edit that file and delete every hook whose +command path points into `.claude/skills/gstack/`: the SessionStart auto-update +hook, the AskUserQuestion PreToolUse/PostToolUse hooks, and the Stop hooks +(session timeline, plus verify-gate if you opted in). Left in place, they error +on every matching event once the install directory is gone. ### Clean up CLAUDE.md diff --git a/TODOS.md b/TODOS.md index 026a7bc8c..742ecf796 100644 --- a/TODOS.md +++ b/TODOS.md @@ -58,6 +58,19 @@ global-path registration + re-point). Remaining: doesn't handle the coordinate. Needs hit-test-aware routing + real-device verification. Effort M. (Related: the multi-window rewrite has no static pins — see the test-gap backlog below.) +- **setup:1601 CLAUDE_CONFIG_DIR alignment** — the skills installer hardcodes + `$HOME/.claude/skills` while settings.json and hook registration honor + `CLAUDE_CONFIG_DIR`; users with the override get a split-brain install. + Mitigated in v1.68.1 (canonical-root fallback to the home path so hooks + still register), but the installer itself should honor the override. + **Priority:** P3. Effort S. +- **Centralize plan_tune_hooks bool parsing + gstack-config key validation** — + the `n|no|false|skip|off|0` negative-value set is triplicated + (gstack-settings-hook prune-stale, setup heal note, setup PT_DECISION) and + gstack-config carries three verbatim copies of the key-validation block + (get/has/set). Extract a `gstack-config` bool helper + `validate_key()`; + update the locale pin test. Filed via /ship review army (maintainability). + **Priority:** P3. Effort S. - **Accepted threat-model notes (documented, no action planned):** redact-prepush treats content pushed to ANY private remote as already-left (accident-only threat model); a parcel-shaped twin within 400 chars can @@ -2773,6 +2786,21 @@ needs one paid run to validate, so it didn't ride the ship. ## Completed +### ✅ DONE (v1.68.1.0): Stop-hook registration pins the setup-time absolute path + +**Priority:** P1 (was filed Effort S, scoped to the Stop hook — shipped as the full defect class) + +**What:** Registering hooks from a dev worktree baked that worktree's physical +path into global settings.json; deleting the worktree left dead hooks erroring +on every AskUserQuestion/session stop. Fixed for ALL gstack hooks, not just +Stop: canonical-only registration via `_hook_command_path`, a KNOWN_HOOKS +identity table in `gstack-settings-hook` (survives Claude Code stripping +`_gstack_source` tags), a `prune-stale [--repoint|--all]` self-healer that +runs heal-first on every `./setup`, per-item mutation safety, a mutation lock, +fail-closed parse, and complete uninstall/no-team teardown. + +**Completed:** v1.68.1.0 (2026-08-18) + ### ✅ DONE (v1.66.0.0): Free suite exit code is untrustworthy — in-process force-exits mask failures **Priority:** P1 diff --git a/VERSION b/VERSION index 1193b2477..0569dac5b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.68.0.0 +1.68.1.0 diff --git a/bin/dev-setup b/bin/dev-setup index 00a286706..f661b593b 100755 --- a/bin/dev-setup +++ b/bin/dev-setup @@ -68,10 +68,17 @@ fi # GSTACK_PLAN_TUNE_HOOKS=yes would still resolve to "install" and rewrite the # user's global ~/.claude/settings.json to point at THIS ephemeral worktree — # which breaks once the workspace is deleted. The flag has highest precedence, -# so it pins resolution to "prompt", and closed stdin then makes prompt-mode a -# no-op skip (no install, no decline marker). A dev workspace must never mutate -# global settings.json. To install the hooks, run `./setup --plan-tune-hooks` -# directly (outside dev-setup). Saved prefix/other config preferences still apply. +# so it pins resolution to "prompt" (setup's PT_EXPLICIT provenance keeps the +# Conductor auto-opt-in from overriding an explicit flag), and closed stdin +# then makes prompt-mode a no-op skip (no install, no decline marker). +# +# A dev workspace never ADDS hooks to global settings.json. One stated repair +# exception: setup's heal-first pass may PRUNE dead gstack hook entries and +# RE-POINT existing ones at the stable ~/.claude/skills/gstack install — +# strictly convergent repair, never a new registration, and hook registration +# itself is canonical-only (an ephemeral tree path can never be baked in). +# To install the hooks, run `./setup --plan-tune-hooks` directly (outside +# dev-setup). Saved prefix/other config preferences still apply. # # GSTACK_SKIP_GBRAIN_REGEN=1 is passed INLINE (not exported) so it scopes to # exactly this nested setup call and can't leak into any other setup path. It diff --git a/bin/gstack-config b/bin/gstack-config index 5d02c9feb..6c23b5d8d 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -3,6 +3,9 @@ # # Usage: # gstack-config get — read a config value (falls back to DEFAULTS) +# gstack-config has — exit 0 iff the key is literally present in the +# config file (get returns DEFAULTS for absent keys, +# so callers that need provenance use this instead) # gstack-config set — write a config value # gstack-config list — show all config (values + defaults) # gstack-config defaults — show just the defaults table @@ -326,6 +329,14 @@ case "${1:-}" in fi printf '%s' "$VALUE" ;; + has) + KEY="${2:?Usage: gstack-config has }" + if ! printf '%s' "$KEY" | LC_ALL=C grep -qE '^[a-zA-Z0-9_]+(@[a-zA-Z0-9]+)?$'; then + echo "Error: key must contain only alphanumeric characters, underscores, and an optional @ suffix" >&2 + exit 1 + fi + grep -qE "^${KEY}:" "$CONFIG_FILE" 2>/dev/null + ;; set) KEY="${2:?Usage: gstack-config set }" VALUE="${3:?Usage: gstack-config set }" diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index 057c7207b..a658048fa 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -1,39 +1,59 @@ #!/usr/bin/env bash # gstack-settings-hook — manage Claude Code hooks in ~/.claude/settings.json # -# Two shapes: +# Three shapes: # -# 1. Legacy (SessionStart only — used by setup --team and gstack-uninstall): +# 1. Legacy (SessionStart only — kept so old installs still clean up): # gstack-settings-hook add # adds SessionStart hook -# gstack-settings-hook remove # removes matching SessionStart hook +# gstack-settings-hook remove # removes gstack-session-update items +# # (the arg is accepted for +# # interface compat; matching is by +# # the gstack-session-update basename) # -# 2. Schema-aware (plan-tune cathedral T3 — supports PreToolUse + PostToolUse): -# gstack-settings-hook add-event --event \ +# 2. Schema-aware (plan-tune cathedral T3): +# gstack-settings-hook add-event --event \ # --command --source [--matcher ] [--timeout ] # gstack-settings-hook ensure-event --event ... --command ... --source ... [--matcher ...] [--timeout ] # gstack-settings-hook remove-source --source # gstack-settings-hook diff-event --event ... --command ... --source ... [--matcher ...] -# gstack-settings-hook rollback # restore latest backup +# gstack-settings-hook rollback # restore latest backup (single-step undo) # gstack-settings-hook list-sources # show all gstack-tagged hook entries # -# ensure-event is the update-in-place verb: same flags as add-event, but it -# first compares the REGISTERED payload for (event, matcher, source) against -# the requested one. Identical → no write, no backup ("unchanged"). Different -# → the single matching entry is replaced via one atomic tmp+rename, so a -# failed update can never leave zero or two registrations. This is what heals -# a stale absolute hook path (e.g. a deleted dev worktree) baked into -# settings.json by an earlier setup — presence-only dedup never re-pointed it. +# 3. Self-heal (phantom-hooks fix): +# gstack-settings-hook prune-stale # prune dead gstack hook items +# gstack-settings-hook prune-stale --repoint # re-point gstack items at , prune still-dead +# gstack-settings-hook prune-stale --all # remove ALL gstack hook items (uninstall sweep) # -# Every add-event/remove-source writes a backup to ~/.claude/settings.json.bak. -# before mutating (Codex correction — silent settings.json mutation is wrong); -# ensure-event backs up only when it actually mutates, so a no-op re-run of -# ./setup doesn't churn backup files. +# ensure-event is the update-in-place verb: same flags as add-event, but keyed +# on (event, source) — any entry carrying our source tag for the event is THE +# registration to compare/update, so a matcher change re-points in place +# instead of pushing a second entry, and duplicate same-source twins from the +# old matcher-keyed dedup collapse to one (reported on stderr). Identical +# payload → no write, no backup ("unchanged"); a re-run of ./setup stays a +# true no-op. This heals a stale absolute hook path (e.g. a deleted dev +# worktree) baked into settings.json by an earlier setup. # -# Dedup: legacy `add`/`remove` dedupe by the historical `gstack-session-update` -# substring. Schema-aware `add-event` dedupes by (event, matcher, _gstack_source) so -# multiple gstack registrations (plan-tune, ...) don't collide. +# Ownership model (KNOWN_HOOKS identity table): a hook ITEM is gstack-owned iff +# its command basename + relpath suffix + event (+ matcher where the table row +# defines one) match a table row. Entry-level `_gstack_source` tags are +# best-effort metadata — Claude Code strips unknown keys when it rewrites +# settings.json, so identity is intrinsic (the table), never tag-only. A tag +# NEVER claims foreign items: in a tagged multi-item entry, unrecognized items +# are always preserved; only a tagged SINGLE-item entry with no table match is +# treated as an owned legacy stray. # -# Writes atomically: .tmp + rename to prevent corruption on crash/disk-full. +# Mutation safety: +# - every mutation runs under a mkdir lock (.lock/) with an owner +# token; release is ownership-checked; stale locks (>30s) are taken over +# via atomic rename. On lock give-up the mutation is SKIPPED with a warning +# (the next setup retries — the system is convergent). +# - parse failure fails CLOSED: a corrupt settings.json is never overwritten +# (only ENOENT starts fresh). Exit 3. +# - backup-on-change: a backup (unique name, .bak-latest pointer) is written +# only when the file content actually changes. No-op mutations are silent +# on disk. `rollback` is a single-step undo of the last real mutation. +# - writes are atomic: unique tmp file + rename (a fixed tmp name would let +# two concurrent writers rename a half-written file into place). set -euo pipefail ACTION="${1:-}" @@ -48,6 +68,7 @@ Usage: gstack-settings-hook ensure-event --event --command --source [--matcher ] [--timeout ] gstack-settings-hook remove-source --source gstack-settings-hook diff-event --event --command --source [--matcher ] [--timeout ] + gstack-settings-hook prune-stale [--repoint ] [--all] gstack-settings-hook rollback gstack-settings-hook list-sources EOF @@ -59,41 +80,258 @@ if ! command -v bun >/dev/null 2>&1; then exit 1 fi -backup_settings() { - if [ -f "$SETTINGS_FILE" ]; then - local ts - ts=$(date +%Y%m%d-%H%M%S) - cp "$SETTINGS_FILE" "$SETTINGS_FILE.bak.$ts" - echo "$SETTINGS_FILE.bak.$ts" > "$SETTINGS_FILE.bak-latest" +# ─── Shared JS prelude ──────────────────────────────────────────────── +# Single source of truth for the KNOWN_HOOKS identity table and the +# ownership/liveness/IO helpers, interpolated into EVERY bun -e script as +# bun -e "$_HOOK_JS_PRELUDE"' ' +# so the dedupe key and the prune predicate cannot drift. The prelude MUST NOT +# contain single quotes (this assignment is single-quoted). +_HOOK_JS_PRELUDE=' +// Umbrella fail-closed guard: bun in -e mode swallows uncaught exceptions +// thrown after a require() call and exits 0 (verified on bun 1.3.13; +// uncaughtException handlers never fire in -e mode either). Every script body +// below runs inside gsMain so a runtime throw becomes a LOUD exit 4 instead +// of a silent success that reports a mutation as clean. +function gsMain(fn) { + try { + fn(); + } catch (e) { + process.stderr.write("gstack-settings-hook: internal error (" + (e && e.message) + ") -- refusing to mutate\n"); + process.exit(4); + } +} +var KNOWN_HOOKS = { + "question-log-hook": { source: "plan-tune-cathedral", event: "PostToolUse", matcher: "(AskUserQuestion|mcp__.*__AskUserQuestion)", relpath: "hosts/claude/hooks/question-log-hook" }, + "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-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) { + var s = String(c == null ? "" : c).trim(); + if (s.indexOf("bash ") === 0) s = s.slice(5).trim(); + if (s.length >= 2 && s.charAt(0) === "\"" && s.charAt(s.length - 1) === "\"") { + // Unescape the gsQuoteCmd form so a re-pointed escaped command is still + // recognized as ours on later passes (identity round-trips). + s = s.slice(1, -1).replace(/\\([\\"$\x60])/g, "$1"); + } + // Separator normalization is Windows-only (a rare-but-legal Unix path + // containing a backslash must not be rewritten and mis-stat-ed). + if (process.platform === "win32" || /^[A-Za-z]:[\\\/]/.test(s)) { + s = s.replace(/\\/g, "/"); + } + return s; +} +function gsBaseOf(c) { var p = gsStripWrap(c); return p.split("/").pop(); } +function gsOwnedRow(cmd, event, matcher) { + var p = gsStripWrap(cmd); + var b = p.split("/").pop(); + // hasOwnProperty guard: a foreign hook basename like "toString" or + // "constructor" must not resolve to an inherited Object.prototype member. + var row = Object.prototype.hasOwnProperty.call(KNOWN_HOOKS, b) ? KNOWN_HOOKS[b] : null; + if (!row) return null; + if (p !== row.relpath && p.slice(-(row.relpath.length + 1)) !== "/" + row.relpath) return null; + if (row.event !== event) return null; + 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 = gsWinPath(gsStripWrap(cmd)); + if (!p) return false; + try { + if (process.platform === "win32") return fs.existsSync(p); + var st = fs.statSync(p); + if (!st.isFile()) return false; + fs.accessSync(p, fs.constants.X_OK); + return true; + } catch (e) { + // Only provable absence counts as dead. EACCES/EIO/unmounted-volume + // errors are transient unreachability -- pruning on those would be a + // one-way ratchet, so conservatively treat the item as alive. + var code = e && e.code; + return !(code === "ENOENT" || code === "ENOTDIR"); + } +} +function gsQuoteCmd(target, hadBash) { + // Shell-metacharacter hardening: the command string is executed by a shell + // when Claude Code fires the hook, so a path containing $, backtick (x60 -- + // written as an escape so the prelude itself stays backtick-free), or a + // quote must be neutralized, not just space-wrapped. + var needsQuote = /[\s$\x60"\\]/.test(target); + var quoted = needsQuote + ? "\"" + target.replace(/[\\"$\x60]/g, function (ch) { return "\\" + ch; }) + "\"" + : target; + return (hadBash ? "bash " : "") + quoted; +} +function gsRotateBackups(settingsPath, keep) { + // Backup files are change-gated but unbounded across months of setups -- + // keep the most recent N so ~/.claude does not accumulate forever. + var fs = require("fs"); + var path = require("path"); + try { + var dir = path.dirname(settingsPath); + var base = path.basename(settingsPath) + ".bak."; + var baks = fs.readdirSync(dir) + .filter(function (f) { return f.indexOf(base) === 0; }) + .map(function (f) { + var full = path.join(dir, f); + var m = 0; + try { m = fs.statSync(full).mtimeMs; } catch (e3) {} + return { full: full, m: m }; + }) + .sort(function (a, b) { return a.m - b.m; }); + for (var i = 0; i < baks.length - keep; i++) { + try { fs.unlinkSync(baks[i].full); } catch (e2) {} + } + } catch (e) {} +} +function gsLoadSettings(path) { + var fs = require("fs"); + var raw = null; + try { raw = fs.readFileSync(path, "utf8"); } + catch (e) { + if (e && e.code === "ENOENT") return { settings: {}, existed: false }; + process.stderr.write("gstack-settings-hook: cannot read " + path + ": " + e.message + " -- refusing to mutate\n"); + process.exit(3); + } + try { return { settings: JSON.parse(raw), existed: true }; } + catch (e) { + process.stderr.write("gstack-settings-hook: " + path + " is not valid JSON (" + e.message + ") -- refusing to mutate; fix or restore it (.bak files / rollback)\n"); + process.exit(3); + } +} +function gsWriteIfChanged(path, beforeText, settings, existed) { + var fs = require("fs"); + var afterText = JSON.stringify(settings, null, 2); + if (afterText === beforeText) return false; + // Preserve the live file mode across the tmp+rename (settings.json can + // carry API keys in its env block -- a user-tightened 0600 must never be + // silently broadened to the default 0644). Fresh files start 0600. + var mode = 0o600; + if (existed) { + try { mode = fs.statSync(path).mode & 0o777; } catch (e) {} + fs.copyFileSync(path, process.env.GSTACK_BACKUP_PATH); + fs.writeFileSync(process.env.GSTACK_BAK_LATEST, process.env.GSTACK_BACKUP_PATH + "\n"); + gsRotateBackups(path, 10); + } + var tmp = process.env.GSTACK_TMP_PATH; + fs.writeFileSync(tmp, afterText + "\n"); + try { fs.chmodSync(tmp, mode); } catch (e) {} + fs.renameSync(tmp, path); + return true; +} +' + +# ─── Mutation lock ──────────────────────────────────────────────────── +# Accepted tradeoffs (adversarial-reviewed): (1) the lock serializes gstack +# writers only -- Claude Code rewrites settings.json without honoring it, so a +# lost update against a live session remains possible (convergent: the next +# heal repairs); (2) stale takeover is mtime-based -- a holder legitimately +# slower than the stale window can be stolen from, and a fresh crash stalls +# callers for the give-up window. PID-aware takeover was considered and +# deferred (owner file already carries $$ if it becomes worth it). +_LOCK_DIR="$SETTINGS_FILE.lock" +_LOCK_TOKEN="" + +_release_lock() { + if [ -n "$_LOCK_TOKEN" ] && [ -d "$_LOCK_DIR" ]; then + # Ownership-checked: never remove a lock another process re-acquired + # after a stale takeover. + _OWNER_CONTENT="$(cat "$_LOCK_DIR/owner" 2>/dev/null || true)" + if [ "$_OWNER_CONTENT" = "$_LOCK_TOKEN" ] || [ -z "$_OWNER_CONTENT" ]; then + rm -rf "$_LOCK_DIR" 2>/dev/null || true + fi fi + _LOCK_TOKEN="" } -# --- legacy SessionStart add/remove (backwards compat) ----------------- +_acquire_lock() { + # GSTACK_SETTINGS_LOCK_TIMEOUT_MS: test-only override for the give-up + # (a contention test should not stall the suite for 10 real seconds). + local waited_ms=0 token stale mtime now + local give_up_ms="${GSTACK_SETTINGS_LOCK_TIMEOUT_MS:-10000}" + local stale_after_s=30 # lock older than this belongs to a crashed holder + local poll_ms=50 # retry cadence; sleep below derives from this + token="$$-$RANDOM$RANDOM" + while :; do + if mkdir "$_LOCK_DIR" 2>/dev/null; then + printf '%s\n' "$token" > "$_LOCK_DIR/owner" 2>/dev/null || true + _LOCK_TOKEN="$token" + trap _release_lock EXIT + trap 'exit 129' INT + trap 'exit 143' TERM + return 0 + fi + # mkdir failed but no lock dir exists: NOT contention (unwritable parent, + # read-only fs, missing directory) -- waiting cannot help, so give up + # loudly now instead of spinning out the full timeout. (Tiny race: a + # contender could acquire+release between our mkdir and this check; that + # transient reads as an environment failure and the next run converges.) + if [ ! -e "$_LOCK_DIR" ]; then + echo "gstack-settings-hook: cannot create lock $_LOCK_DIR (unwritable parent?) -- skipping this mutation, exit 5" >&2 + return 1 + fi + # Stale takeover: atomic rename means exactly one contender wins; the + # loser loops and re-contends against the winner's fresh mkdir. + # GNU stat (-c %Y) first: on Linux, BSD-style `stat -f %m` prints a + # multi-line FILESYSTEM block to stdout before failing, and the || chain + # would capture that garbage alongside the real epoch. BSD stat rejects + # -c with no stdout, so macOS falls through cleanly. The numeric guard + # below makes any residual garbage inert (no takeover, normal give-up) + # instead of an arithmetic abort under set -e. + mtime=$(stat -c %Y "$_LOCK_DIR" 2>/dev/null || stat -f %m "$_LOCK_DIR" 2>/dev/null || echo "") + case "$mtime" in *[!0-9]*) mtime="" ;; esac + now=$(date +%s) + if [ -n "$mtime" ] && [ $(( now - mtime )) -gt "$stale_after_s" ]; then + stale="$_LOCK_DIR.stale.$$-$RANDOM" + if mv "$_LOCK_DIR" "$stale" 2>/dev/null; then rm -rf "$stale" 2>/dev/null || true; fi + continue + fi + if [ "$waited_ms" -ge "$give_up_ms" ]; then + echo "gstack-settings-hook: could not acquire lock $_LOCK_DIR -- skipping this mutation, exit 5 (the next setup retries it)" >&2 + return 1 + fi + sleep "$(printf '0.%03d' "$poll_ms")" + waited_ms=$(( waited_ms + poll_ms )) + done +} + +# Per-invocation unique backup + tmp paths, exported for gsWriteIfChanged. +_mutation_env() { + GSTACK_BACKUP_PATH="$SETTINGS_FILE.bak.$(date +%Y%m%d-%H%M%S).$$.$RANDOM" + GSTACK_BAK_LATEST="$SETTINGS_FILE.bak-latest" + GSTACK_TMP_PATH="$SETTINGS_FILE.tmp.$$.$RANDOM" + export GSTACK_BACKUP_PATH GSTACK_BAK_LATEST GSTACK_TMP_PATH +} case "$ACTION" in + # --- legacy SessionStart add/remove (backwards compat) ----------------- add) HOOK_CMD="${2:-}" if [ -z "$HOOK_CMD" ]; then echo "Usage: gstack-settings-hook add " >&2 exit 1 fi - backup_settings - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun -e ' - const fs = require("fs"); + _acquire_lock || exit 5 + _mutation_env + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const hookCmd = process.env.GSTACK_HOOK_CMD; - let settings = {}; - // An EXISTING file that does not parse must never be rewritten: the - // old catch{} folded it to {} and the atomic write below replaced the - // user permissions/env/other hooks with just ours. Refuse loudly. - if (fs.existsSync(settingsPath)) { - try { settings = JSON.parse(fs.readFileSync(settingsPath, "utf8")); } - catch (e) { - console.error("error: " + settingsPath + " exists but is not valid JSON (" + - (e && e.message ? e.message : e) + "); refusing to rewrite it. Fix or move the file, then re-run."); - process.exit(1); - } - } + const loaded = gsLoadSettings(settingsPath); + const settings = loaded.settings; + const before = JSON.stringify(settings, null, 2); if (!settings.hooks) settings.hooks = {}; if (!settings.hooks.SessionStart) settings.hooks.SessionStart = []; const exists = settings.hooks.SessionStart.some(entry => @@ -104,9 +342,8 @@ case "$ACTION" in hooks: [{ type: "command", command: hookCmd }] }); } - const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp - fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); - fs.renameSync(tmp, settingsPath); + gsWriteIfChanged(settingsPath, before, settings, loaded.existed); + }); ' ;; @@ -117,23 +354,34 @@ case "$ACTION" in exit 1 fi [ -f "$SETTINGS_FILE" ] || exit 1 - backup_settings - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e ' - const fs = require("fs"); + _acquire_lock || exit 5 + _mutation_env + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; - let settings = {}; - try { settings = JSON.parse(fs.readFileSync(settingsPath, "utf8")); } catch { process.exit(0); } + const loaded = gsLoadSettings(settingsPath); + const settings = loaded.settings; + const before = JSON.stringify(settings, null, 2); if (settings.hooks && settings.hooks.SessionStart) { - settings.hooks.SessionStart = settings.hooks.SessionStart.filter(entry => - !(entry.hooks && entry.hooks.some(h => h.command && h.command.includes("gstack-session-update"))) - ); + // Item-aware: remove only matching hook items; foreign items in the + // same entry survive; an entry is dropped ONLY when this pass emptied + // it. Malformed/foreign entries (hooks absent, non-array, or already + // empty) are preserved verbatim -- they are not ours to judge. + settings.hooks.SessionStart = settings.hooks.SessionStart + .filter(entry => { + if (!Array.isArray(entry.hooks)) return true; + const beforeLen = entry.hooks.length; + entry.hooks = entry.hooks.filter(h => + !(h && h.command && h.command.includes("gstack-session-update")) + ); + if (entry.hooks.length === 0 && beforeLen > 0) return false; + return true; + }); if (settings.hooks.SessionStart.length === 0) delete settings.hooks.SessionStart; if (Object.keys(settings.hooks).length === 0) delete settings.hooks; } - const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp - fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); - fs.renameSync(tmp, settingsPath); - ' 2>/dev/null + gsWriteIfChanged(settingsPath, before, settings, loaded.existed); + }); + ' ;; add-event|diff-event|ensure-event) @@ -161,13 +409,15 @@ case "$ACTION" in SessionStart|PreToolUse|PostToolUse|UserPromptSubmit|Stop|Notification) ;; *) echo "invalid --event '$EVENT'; must be one of SessionStart|PreToolUse|PostToolUse|UserPromptSubmit|Stop|Notification" >&2; exit 1 ;; esac - if [ "$ACTION" = "add-event" ]; then - backup_settings - fi DIFF_ONLY="" - if [ "$ACTION" = "diff-event" ]; then DIFF_ONLY=1; fi ENSURE="" - if [ "$ACTION" = "ensure-event" ]; then ENSURE=1; fi + if [ "$ACTION" = "diff-event" ]; then + DIFF_ONLY=1 + else + [ "$ACTION" = "ensure-event" ] && ENSURE=1 + _acquire_lock || exit 5 + fi + _mutation_env GSTACK_SETTINGS_PATH="$SETTINGS_FILE" \ GSTACK_EVENT="$EVENT" \ GSTACK_COMMAND="$COMMAND" \ @@ -176,8 +426,7 @@ case "$ACTION" in GSTACK_TIMEOUT="$TIMEOUT" \ GSTACK_DIFF_ONLY="$DIFF_ONLY" \ GSTACK_ENSURE="$ENSURE" \ - bun -e ' - const fs = require("fs"); + bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const event = process.env.GSTACK_EVENT; const cmd = process.env.GSTACK_COMMAND; @@ -187,64 +436,98 @@ case "$ACTION" in const diffOnly = process.env.GSTACK_DIFF_ONLY === "1"; const ensure = process.env.GSTACK_ENSURE === "1"; - let settings = {}; - // An EXISTING file that does not parse must never be rewritten: the - // old catch{} folded it to {} and the atomic write below replaced the - // user permissions/env/other hooks with just ours. Refuse loudly. - if (fs.existsSync(settingsPath)) { - try { settings = JSON.parse(fs.readFileSync(settingsPath, "utf8")); } - catch (e) { - console.error("error: " + settingsPath + " exists but is not valid JSON (" + - (e && e.message ? e.message : e) + "); refusing to rewrite it. Fix or move the file, then re-run."); - process.exit(1); - } - } - + const loaded = gsLoadSettings(settingsPath); + const settings = loaded.settings; const before = JSON.stringify(settings, null, 2); if (!settings.hooks) settings.hooks = {}; if (!settings.hooks[event]) settings.hooks[event] = []; - // Identity key is (event, source): any existing entry carrying OUR - // source tag for this event IS the entry to compare/update — a matcher - // change must update it in place, never push a SECOND gstack entry - // (the old key included the matcher, so a future matcher change would - // have duplicated the registration). Untagged legacy entries are still - // adopted when both matcher and command line up. - const matchesEntry = (entry) => { - if (entry._gstack_source === source) return true; - const sameMatcher = (entry.matcher || "") === matcher; - const sameCommand = entry.hooks && entry.hooks[0] && entry.hooks[0].command === cmd; - return sameMatcher && sameCommand; - }; - - // Collect ALL matches, not just the first: pre-existing installs can - // carry two entries with the same (event, _gstack_source) from the old - // matcher-keyed dedup. `.find()` updated only the first and left the - // stale twin running forever. Keep ONE canonical entry (the first), - // remove the rest in the same atomic write. - const matched = settings.hooks[event].filter(matchesEntry); - let existing = matched.length > 0 ? matched[0] : undefined; - let collapsed = 0; - if (matched.length > 1) { - const extras = new Set(matched.slice(1)); - settings.hooks[event] = settings.hooks[event].filter((e) => !extras.has(e)); - collapsed = matched.length - 1; - } - 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; } - if (existing) { - existing.hooks = [hookEntry]; - existing._gstack_source = source; - // Keep the matcher current too — under the (event, source) key the - // matched entry may carry a stale matcher. - if (matcher) existing.matcher = matcher; - else delete existing.matcher; - } else { + // Identity is item-level and two-layer: + // - a same-event entry carrying OUR source tag is ours even under a + // DIFFERENT matcher (a matcher change must re-point in place, never + // push a second registration -- the old matcher-keyed dedupe + // duplicated the entry on every matcher change), and + // - a same-matcher entry containing our exact command or a table-owned + // item with our basename is OUR registration under a stale path + // (Claude Code strips _gstack_source on its own rewrites, so + // tag-based dedupe degrades). + // A tag never claims foreign items: in a multi-item entry only the + // identified item is touched; entries where no item can be identified + // as ours are left alone entirely. + const ourItemIdx = (entry) => { + if (!Array.isArray(entry.hooks)) return -1; + 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(cmdNorm); + }); + } + if (idx < 0 && entry._gstack_source === source && entry.hooks.length === 1 && entry.hooks[0] && entry.hooks[0].command) idx = 0; + return idx; + }; + const cands = []; + for (const entry of settings.hooks[event]) { + const idx = ourItemIdx(entry); + if (idx < 0) continue; + if (entry._gstack_source === source || (entry.matcher || "") === matcher) cands.push({ entry: entry, idx: idx }); + } + + let placed = false; + let collapsed = 0; + if (cands.length > 0) { + const primary = cands[0]; + if ((primary.entry.matcher || "") === matcher) { + primary.entry.hooks[primary.idx] = hookEntry; + // Mixed-version ratchet guard: tag ONLY single-item entries (the + // item we just placed). Old gstack versions in sibling worktrees do + // entry-level ownership (remove-source deletes the whole tagged + // entry; add-event clobbers entry.hooks wholesale) -- a tag on a + // mixed entry hands them permission to destroy the user items in it. + if (primary.entry.hooks.length === 1) primary.entry._gstack_source = source; + else delete primary.entry._gstack_source; + placed = true; + } else if (primary.entry.hooks.length === 1) { + // Tagged single-item entry under a stale matcher: the entry is + // exclusively ours, so re-point payload AND matcher in place. + primary.entry.hooks = [hookEntry]; + primary.entry._gstack_source = source; + if (matcher) primary.entry.matcher = matcher; + else delete primary.entry.matcher; + placed = true; + } else { + // Our item sits in a MIXED entry under a different matcher: the + // entry-level matcher also governs the foreign siblings, so pull + // our item out and register separately below. + primary.entry.hooks.splice(primary.idx, 1); + delete primary.entry._gstack_source; + } + // Duplicate registrations (e.g. same-source twins from the old + // matcher-keyed dedupe): remove OUR item from every other candidate. + for (let i = 1; i < cands.length; i++) { + cands[i].entry.hooks.splice(cands[i].idx, 1); + delete cands[i].entry._gstack_source; + collapsed++; + } + // Drop only entries WE emptied; started-empty foreign entries are + // never candidates, so they are preserved verbatim. + settings.hooks[event] = settings.hooks[event].filter(e => + !(Array.isArray(e.hooks) && e.hooks.length === 0 && cands.some(c => c.entry === e))); + } + if (!placed) { const newEntry = { _gstack_source: source, hooks: [hookEntry] }; if (matcher) newEntry.matcher = matcher; settings.hooks[event].push(newEntry); @@ -261,48 +544,22 @@ case "$ACTION" in } if (ensure && before === after) { - // Registered payload already matches the canonical one — no write, no - // backup, no churn. Re-running ./setup stays a true no-op. + // Registered payload already matches the canonical one -- no write, + // no backup, no churn. Re-running ./setup stays a true no-op. console.log("OK: " + event + " hook unchanged (source: " + source + ")"); process.exit(0); } - try { - if (ensure && fs.existsSync(settingsPath)) { - // Mirrors backup_settings (bash) — but only when a write actually - // happens, so a no-op ensure-event never creates backup files. - const d = new Date(); - const pad = (n) => String(n).padStart(2, "0"); - const ts = "" + d.getFullYear() + pad(d.getMonth() + 1) + pad(d.getDate()) + - "-" + pad(d.getHours()) + pad(d.getMinutes()) + pad(d.getSeconds()); - fs.copyFileSync(settingsPath, settingsPath + ".bak." + ts); - fs.writeFileSync(settingsPath + ".bak-latest", settingsPath + ".bak." + ts + "\n"); - } - - // Atomic tmp+rename: the settings file is either the old JSON (with - // the old single registration) or the new JSON (with the replaced - // one) — a failed update can never leave zero or two registrations. - // Per-process tmp suffix: a fixed settings.json.tmp let two parallel - // writers consume one another. (No apostrophes here: this JS lives - // inside a bash single-quoted string.) - const tmp = settingsPath + ".tmp." + process.pid; - fs.writeFileSync(tmp, after + "\n"); - fs.renameSync(tmp, settingsPath); - } catch (e) { - // Explicit catch + exit 1: bun -e has been observed (1.3.13) to turn - // an uncaught sync fs error into a SILENT exit 0, which would let a - // failed update masquerade as success to the caller. - console.error("error: could not update " + settingsPath + ": " + (e && e.message ? e.message : e)); - process.exit(1); - } + gsWriteIfChanged(settingsPath, before, settings, loaded.existed); if (collapsed > 0) { console.error("collapsed " + collapsed + " duplicate (event, source) hook entr" + (collapsed === 1 ? "y" : "ies") + " for " + event + " (source: " + source + ")"); } - if (ensure && existing) { + if (ensure && cands.length > 0) { console.log("OK: " + event + " hook re-pointed (source: " + source + ")"); } else { console.log("OK: " + event + " hook registered (source: " + source + ")"); } + }); ' ;; @@ -320,26 +577,199 @@ case "$ACTION" in exit 1 fi [ -f "$SETTINGS_FILE" ] || exit 0 - backup_settings - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_SOURCE="$SOURCE" bun -e ' - const fs = require("fs"); + _acquire_lock || exit 5 + _mutation_env + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_SOURCE="$SOURCE" bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const source = process.env.GSTACK_SOURCE; - let settings = {}; - try { settings = JSON.parse(fs.readFileSync(settingsPath, "utf8")); } catch { process.exit(0); } - if (!settings.hooks) { process.exit(0); } + const loaded = gsLoadSettings(settingsPath); + const settings = loaded.settings; + if (!settings.hooks) { console.log("OK: removed 0 hook entry/entries tagged source=" + source); process.exit(0); } + const before = JSON.stringify(settings, null, 2); let removed = 0; for (const event of Object.keys(settings.hooks)) { - const before = settings.hooks[event].length; - settings.hooks[event] = settings.hooks[event].filter(entry => entry._gstack_source !== source); - removed += before - settings.hooks[event].length; + const kept = []; + for (const entry of settings.hooks[event]) { + if (entry._gstack_source !== source) { kept.push(entry); continue; } + if (!Array.isArray(entry.hooks) || entry.hooks.length === 0) { removed++; continue; } + // Item-aware: remove table-owned items (or the single item of a + // tagged legacy-stray entry); foreign items in a tagged multi-item + // entry are preserved and the tag is dropped with the last owned item. + const single = entry.hooks.length === 1; + const remain = entry.hooks.filter(h => { + // Command-less items cannot be ours (gstack only writes + // type:command items) -- preserve them. + const owned = (h && h.command) + ? gsOwnedRow(h.command, event, entry.matcher || "") !== null + : false; + // The single-item stray claim requires a command item (gstack + // never writes command-less items). + if (owned || (single && h && h.command)) { removed++; return false; } + return true; + }); + if (remain.length === 0) continue; + entry.hooks = remain; + delete entry._gstack_source; + kept.push(entry); + } + settings.hooks[event] = kept; if (settings.hooks[event].length === 0) delete settings.hooks[event]; } if (Object.keys(settings.hooks).length === 0) delete settings.hooks; - const tmp = settingsPath + ".tmp." + process.pid; // per-process: parallel writers must not share a tmp - fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n"); - fs.renameSync(tmp, settingsPath); + gsWriteIfChanged(settingsPath, before, settings, loaded.existed); console.log("OK: removed " + removed + " hook entry/entries tagged source=" + source); + }); + ' + ;; + + prune-stale) + REPOINT_ROOT="" + PRUNE_ALL="" + shift + while [ $# -gt 0 ]; do + case "$1" in + --repoint) REPOINT_ROOT="$2"; shift 2 ;; + --all) PRUNE_ALL=1; shift ;; + *) echo "unknown flag: $1" >&2; exit 1 ;; + esac + done + if [ -n "$REPOINT_ROOT" ] && [ -n "$PRUNE_ALL" ]; then + echo "prune-stale: --repoint and --all are mutually exclusive" >&2 + exit 1 + fi + if [ ! -f "$SETTINGS_FILE" ]; then + echo "OK: removed 0 gstack hook entries (repointed 0)" + exit 0 + fi + # Explicit plan_tune_hooks opt-out: dead plan-tune items are still pruned, + # but live ones are never re-pointed (re-activation needs consent; removal + # of live ones is --no-team/uninstall territory). + GSTACK_PT_OPTOUT=0 + # The opt-out lookup is repoint/heal-only — the --all sweep never + # re-points, and uninstall must not depend on a sibling gstack-config. + if [ -z "$PRUNE_ALL" ]; then + _CFG_BIN="$(cd "$(dirname "$0")" && pwd)/gstack-config" + if [ -x "$_CFG_BIN" ] && "$_CFG_BIN" has plan_tune_hooks 2>/dev/null; then + _PT_VAL=$("$_CFG_BIN" get plan_tune_hooks 2>/dev/null || true) + _PT_VAL=$(printf '%s' "$_PT_VAL" | tr '[:upper:]' '[:lower:]' | tr -d '[:space:]') + case "$_PT_VAL" in + n|no|false|skip|off|0) GSTACK_PT_OPTOUT=1 ;; + esac + fi + fi + _acquire_lock || exit 5 + _mutation_env + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" \ + GSTACK_REPOINT_ROOT="$REPOINT_ROOT" \ + GSTACK_PRUNE_ALL="$PRUNE_ALL" \ + GSTACK_PT_OPTOUT="$GSTACK_PT_OPTOUT" \ + GSTACK_SWEEP_EXCLUDE_SOURCES="${GSTACK_SWEEP_EXCLUDE_SOURCES:-}" \ + bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + const settingsPath = process.env.GSTACK_SETTINGS_PATH; + const root = (process.env.GSTACK_REPOINT_ROOT || "").replace(/\/+$/, ""); + const all = process.env.GSTACK_PRUNE_ALL === "1"; + const ptOptout = process.env.GSTACK_PT_OPTOUT === "1"; + const PT_SOURCES = { "plan-tune-cathedral": true, "auq-error-fallback": true }; + // Sources a sweep must leave alone (e.g. `setup --no-team` excludes the + // user-registered verify-gate hook: turning team mode off must not + // delete an unrelated opt-in whose binary still exists). + const sweepExclude = {}; + (process.env.GSTACK_SWEEP_EXCLUDE_SOURCES || "").split(",").forEach(function (sName) { + if (sName.trim()) sweepExclude[sName.trim()] = true; + }); + + const loaded = gsLoadSettings(settingsPath); + const settings = loaded.settings; + const before = JSON.stringify(settings, null, 2); + let removed = 0; + let repointed = 0; + + if (settings.hooks) { + for (const event of Object.keys(settings.hooks)) { + const rebuilt = []; + for (const entry of settings.hooks[event]) { + if (!Array.isArray(entry.hooks)) { rebuilt.push(entry); continue; } + const matcher = entry.matcher || ""; + const wasSingle = entry.hooks.length === 1; + 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 && !sweepExclude[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; + // A tagged SINGLE-item entry with no table match is an owned + // legacy stray (command items only -- gstack never writes + // command-less items); tags never claim items in multi-item entries. + const stray = !row && !!cmdRaw && !!entry._gstack_source && wasSingle; + if (!row && !stray) { remain.push(h); continue; } // foreign: never touched + if (all) { + if ((row && sweepExclude[row.source]) || (!row && entry._gstack_source && sweepExclude[entry._gstack_source])) { remain.push(h); continue; } + removed++; continue; + } + if (row && root && !(ptOptout && PT_SOURCES[row.source])) { + const target = root + "/" + row.relpath; + if (gsIsAlive(target)) { + const newCmd = gsQuoteCmd(target, gsHadBashPrefix(cmdRaw)); + if (h.command !== newCmd) { h.command = newCmd; repointed++; } + // Within-entry twin collapse: two dead copies of the same + // hook re-point to the same canonical command -- keeping + // both would fire the hook twice per event, forever. + if (seenInEntry.has(newCmd)) { removed++; continue; } + seenInEntry.add(newCmd); + if (wasSingle) entry._gstack_source = row.source; // tag restore + remain.push(h); + continue; + } + } + // No re-point target (or plan-tune opt-out): keep live, prune dead. + if (gsIsAlive(cmdRaw)) remain.push(h); else { removed++; } + } + if (remain.length === 0) continue; // entry emptied → dropped + entry.hooks = remain; + // Tag hygiene: a tag must never sit on an entry containing foreign + // items (old gstack versions in sibling worktrees treat tags as + // entry-level ownership and would destroy the user items). Drop + // the tag from any mixed entry; single-item strays keep theirs. + if (entry._gstack_source && entry.hooks.length > 0 + && !entry.hooks.every(h => h && h.command && gsOwnedRow(h.command, event, matcher)) + && !(entry.hooks.length === 1 && wasSingle)) { + delete entry._gstack_source; + } + rebuilt.push(entry); + } + // Collapse exact duplicates among FULLY-owned entries (same matcher, + // same item commands). Prefer the tagged twin so stripped tags heal. + const seen = new Map(); + const out = []; + for (const entry of rebuilt) { + const items = Array.isArray(entry.hooks) ? entry.hooks : []; + const fullyOwned = items.length > 0 && items.every(h => + h && h.command && gsOwnedRow(h.command, event, entry.matcher || "")); + if (!fullyOwned) { out.push(entry); continue; } + const key = JSON.stringify([entry.matcher || ""].concat(items.map(function (h) { return h.command; }).sort())); + const at = seen.get(key); + if (at === undefined) { seen.set(key, out.length); out.push(entry); } + else { + if (!out[at]._gstack_source && entry._gstack_source) out[at] = entry; + removed++; + } + } + settings.hooks[event] = out; + if (out.length === 0) delete settings.hooks[event]; + } + if (settings.hooks && Object.keys(settings.hooks).length === 0) delete settings.hooks; + } + + gsWriteIfChanged(settingsPath, before, settings, loaded.existed); + console.log("OK: removed " + removed + " gstack hook entries (repointed " + repointed + ")"); + }); ' ;; @@ -349,20 +779,46 @@ case "$ACTION" in exit 1 fi LATEST=$(cat "$SETTINGS_FILE.bak-latest") + LATEST=$(printf '%s' "$LATEST" | tr -d '\n') + # Defense in depth: only ever restore a sibling settings.json.bak.* file + # (a corrupted/hostile pointer must not install an arbitrary file as the + # live settings.json). + case "$LATEST" in + "$SETTINGS_FILE".bak.*) ;; + *) + echo "rollback: pointer target $LATEST is not a $SETTINGS_FILE.bak.* file -- refusing" >&2 + exit 1 + ;; + esac + case "${LATEST#"$SETTINGS_FILE".bak.}" in + */*) + echo "rollback: pointer suffix contains a path separator -- refusing" >&2 + exit 1 + ;; + esac if [ ! -f "$LATEST" ]; then echo "rollback: pointer references missing backup $LATEST" >&2 exit 1 fi - cp "$LATEST" "$SETTINGS_FILE" + _acquire_lock || exit 1 + _RB_TMP="$SETTINGS_FILE.tmp.$$.$RANDOM" + cp "$LATEST" "$_RB_TMP" + mv "$_RB_TMP" "$SETTINGS_FILE" echo "OK: restored $SETTINGS_FILE from $LATEST" ;; list-sources) [ -f "$SETTINGS_FILE" ] || { echo "(no settings file)"; exit 0; } - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e ' + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const fs = require("fs"); let settings = {}; - try { settings = JSON.parse(fs.readFileSync(process.env.GSTACK_SETTINGS_PATH, "utf8")); } catch { process.exit(0); } + try { settings = JSON.parse(fs.readFileSync(process.env.GSTACK_SETTINGS_PATH, "utf8")); } + catch (e) { + // Read-only surface: report loudly (setup guards read this output and + // must not mistake corrupt-file for no-hooks) but exit 0. + process.stderr.write("gstack-settings-hook: settings.json unparseable (" + e.message + ") -- fix or rollback\n"); + process.exit(0); + } const hooks = settings.hooks || {}; let any = false; for (const event of Object.keys(hooks)) { @@ -374,6 +830,7 @@ case "$ACTION" in } } if (!any) console.log("(no gstack-tagged hooks)"); + }); ' ;; diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall index 9f83386d7..d79e0f387 100755 --- a/bin/gstack-uninstall +++ b/bin/gstack-uninstall @@ -132,6 +132,39 @@ if [ -d "$STATE_DIR/projects" ]; then done < <(find "$STATE_DIR/projects" -name browse.json -path '*/.gstack/*' 2>/dev/null || true) fi +# ─── Remove gstack hooks from Claude Code settings ────────── +# MUST run BEFORE any install-root deletion: SETTINGS_HOOK resolves inside the +# install being removed — in a real global uninstall, running this after +# `rm -rf ~/.claude/skills/gstack` silently no-ops and orphans every hook. +SETTINGS_HOOK="$(dirname "$0")/gstack-settings-hook" +SESSION_UPDATE="$(dirname "$0")/gstack-session-update" +if [ -x "$SETTINGS_HOOK" ]; then + "$SETTINGS_HOOK" remove "$SESSION_UPDATE" && REMOVED+=("SessionStart hook") || true + # Cathedral T8 cleanup: also remove plan-tune PreToolUse + PostToolUse hooks. + if "$SETTINGS_HOOK" remove-source --source plan-tune-cathedral | grep -q "removed [1-9]"; then + REMOVED+=("plan-tune cathedral hooks") + fi + # AskUserQuestion error-fallback hook (registered by setup; previously never + # torn down). + if "$SETTINGS_HOOK" remove-source --source auq-error-fallback | grep -q "removed [1-9]"; then + REMOVED+=("AskUserQuestion error-fallback hook") + fi + # Timeline Stop hook (#2553). + 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. + if "$SETTINGS_HOOK" prune-stale --all | grep -q "removed [1-9]"; then + REMOVED+=("stray gstack hook entries") + fi +fi + # ─── Remove global Claude skills ──────────────────────────── CLAUDE_SKILLS="$HOME/.claude/skills" @@ -334,21 +367,6 @@ if [ -n "$_GIT_ROOT" ]; then fi fi -# ─── Remove SessionStart hook from Claude Code settings ───── -SETTINGS_HOOK="$(dirname "$0")/gstack-settings-hook" -SESSION_UPDATE="$(dirname "$0")/gstack-session-update" -if [ -x "$SETTINGS_HOOK" ]; then - "$SETTINGS_HOOK" remove "$SESSION_UPDATE" 2>/dev/null && REMOVED+=("SessionStart hook") || true - # Cathedral T8 cleanup: also remove plan-tune PreToolUse + PostToolUse hooks. - if "$SETTINGS_HOOK" remove-source --source plan-tune-cathedral 2>/dev/null | grep -q "removed [1-9]"; then - REMOVED+=("plan-tune cathedral hooks") - fi - # Timeline Stop hook (#2553). - if "$SETTINGS_HOOK" remove-source --source gstack-timeline-stop 2>/dev/null | grep -q "removed [1-9]"; then - REMOVED+=("timeline Stop hook") - fi -fi - # ─── Remove global state ──────────────────────────────────── if [ "$KEEP_STATE" -eq 0 ] && [ -d "$STATE_DIR" ]; then rm -rf "$STATE_DIR" diff --git a/package.json b/package.json index 3f7519086..43b7145b7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.68.0", + "version": "1.68.1", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/setup b/setup index 0e932efbf..8c5d6c077 100755 --- a/setup +++ b/setup @@ -1920,27 +1920,113 @@ rm -f /tmp/gstack-latest-version # 10. Team mode: register/unregister SessionStart hook SETTINGS_HOOK="$SOURCE_GSTACK_DIR/bin/gstack-settings-hook" + +# ─── Canonical hook paths + self-heal (phantom-hooks fix) ───────────────────── +# Hook commands written to GLOBAL settings.json must survive deletion of the +# tree setup ran from: SOURCE_GSTACK_DIR is `pwd -P` of the running tree, which +# for Conductor workspaces / manual worktrees / temp clones is EPHEMERAL — +# baking it produced dead hooks erroring on every AskUserQuestion until v1.67. +# Hook registration is therefore CANONICAL-ONLY: the stable install path below, +# or no registration at all. By this point setup has already installed/linked +# the canonical tree, so a missing canonical hook means "don't register", never +# "fall back to the running tree". The canonical path is symlink-preserving, so +# re-pointing ~/.claude/skills/gstack at a new clone heals every hook with zero +# settings writes. Repo-local --local installs don't register global Claude +# hooks (by design). +# +# 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. +_hook_command_path() { + if [ -x "$CANONICAL_GSTACK_ROOT/$1" ]; then + printf '%s\n' "$CANONICAL_GSTACK_ROOT/$1" + return 0 + fi + return 1 +} + +# Heal-first: prune dead gstack hook entries and re-point survivors at the +# stable install BEFORE any tag-presence guard below (a dead entry carrying the +# tag otherwise blocks re-registration forever — the missing-Stop-hook failure +# mode). Runs on EVERY setup, including --no-team, so upgrades self-heal +# without migrations. One log line only when something actually changed; stderr +# passes through uncaptured (zero silent failures). +if [ -x "$SETTINGS_HOOK" ]; then + if [ -d "$CANONICAL_GSTACK_ROOT" ]; then + _HEAL_OUT=$("$SETTINGS_HOOK" prune-stale --repoint "$CANONICAL_GSTACK_ROOT" || true) + else + _HEAL_OUT=$("$SETTINGS_HOOK" prune-stale || true) + fi + _HEAL_REMOVED=$(printf '%s' "$_HEAL_OUT" | sed -n 's/^OK: removed \([0-9]*\).*/\1/p') + _HEAL_REPOINTED=$(printf '%s' "$_HEAL_OUT" | sed -n 's/.*repointed \([0-9]*\).*/\1/p') + if [ "${_HEAL_REMOVED:-0}" -gt 0 ] 2>/dev/null || [ "${_HEAL_REPOINTED:-0}" -gt 0 ] 2>/dev/null; then + log " healed hook registrations: removed ${_HEAL_REMOVED:-0}, repointed ${_HEAL_REPOINTED:-0} (backup: settings.json.bak.; note: later registrations in this run move the rollback pointer — restore the heal's own .bak file directly if needed)" + fi + # Explicit opt-out + live plan-tune hooks is a contradiction worth surfacing: + # the heal honors the opt-out (dead plan-tune entries pruned, never + # re-pointed) but live hooks stay until the user removes them. + if "$GSTACK_CONFIG" has plan_tune_hooks 2>/dev/null; then + _PT_CFG_VAL=$("$GSTACK_CONFIG" get plan_tune_hooks 2>/dev/null || true) + case "$(printf '%s' "$_PT_CFG_VAL" | tr '[:upper:]' '[:lower:]' | tr -d '[:space:]')" in + n|no|false|skip|off|0) + if "$SETTINGS_HOOK" list-sources 2>/dev/null | grep -q "plan-tune-cathedral"; then + log " note: plan_tune_hooks is 'no' in config but live plan-tune hooks exist — remove with ./setup --no-team or $SETTINGS_HOOK remove-source --source plan-tune-cathedral" + fi + ;; + esac + fi +fi + # On Windows (Git Bash / MSYS2 / Cygwin), extensionless scripts can't be # launched directly by the OS — the file-association dialog appears instead. # Prefix with 'bash' so Claude Code's hook runner invokes Git Bash explicitly. -if [ "$IS_WINDOWS" -eq 1 ]; then - HOOK_CMD="bash $SOURCE_GSTACK_DIR/bin/gstack-session-update" -else - HOOK_CMD="$SOURCE_GSTACK_DIR/bin/gstack-session-update" +# Paths with whitespace are quoted so the hook command survives shell parsing. +SESSION_UPDATE_CMD="$(_hook_command_path bin/gstack-session-update || true)" +HOOK_CMD="" +if [ -n "$SESSION_UPDATE_CMD" ]; then + # 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 + HOOK_CMD="$SESSION_UPDATE_CMD" + fi fi if [ "$TEAM_MODE" -eq 1 ]; then "$GSTACK_CONFIG" set auto_upgrade true 2>/dev/null || true "$GSTACK_CONFIG" set team_mode true 2>/dev/null || true - # Register SessionStart hook in Claude Code settings - if [ -x "$SETTINGS_HOOK" ]; then - "$SETTINGS_HOOK" add "$HOOK_CMD" 2>/dev/null || true + # Register SessionStart hook in Claude Code settings (schema-aware: the + # legacy `add` action's substring dedupe bypasses the KNOWN_HOOKS identity + # system; add-event re-points stale paths in place instead of appending). + # stderr stays attached (zero silent settings mutations — a fail-closed + # parse error or lock give-up must reach the user). + if [ -x "$SETTINGS_HOOK" ] && [ -n "$HOOK_CMD" ]; then + "$SETTINGS_HOOK" add-event --event SessionStart --command "$HOOK_CMD" --source gstack-session-update >/dev/null || true + elif [ -z "$HOOK_CMD" ]; then + log " SessionStart hook not registered: bin/gstack-session-update missing at $CANONICAL_GSTACK_ROOT (no stable install)" fi log "" - log "Team mode enabled: gstack will auto-update at the start of each Claude Code session." - log " Hook: $HOOK_CMD" + if [ -n "$HOOK_CMD" ]; then + log "Team mode enabled: gstack will auto-update at the start of each Claude Code session." + log " Hook: $HOOK_CMD" + else + log "Team mode enabled (auto-update hook pending a stable install — re-run ./setup after installing globally)." + fi log " To disable: ./setup --no-team" log "" log "Bootstrap your repo:" @@ -2045,22 +2131,13 @@ if [ -x "$DETECT_BIN" ]; then fi fi -# Hook commands registered into ~/.claude/settings.json must survive deletion -# of the directory setup ran from. A dev-worktree setup used to bake -# $SOURCE_GSTACK_DIR's absolute path into the registration; deleting that -# worktree left a dead hook erroring on every trigger. Prefer the global -# install (~/.claude/skills/gstack — a persistent checkout, or a stable -# symlink) and fall back to the setup-time source tree only when no global -# install exists yet (first install from a fresh clone). -_hook_install_path() { - local rel="$1" - local global_hook="$HOME/.claude/skills/gstack/$rel" - if [ -x "$global_hook" ]; then - printf '%s' "$global_hook" - else - printf '%s' "$SOURCE_GSTACK_DIR/$rel" - fi -} +# Hook path resolution is CANONICAL-ONLY via the resolver defined near +# CANONICAL_GSTACK_ROOT above (_hook_command_path): a hook command registered into +# ~/.claude/settings.json must survive deletion of the directory setup ran +# from, and no heuristic can enumerate every ephemeral tree (manual worktrees, +# temp clones, CI checkouts) — so there is deliberately NO fallback to +# $SOURCE_GSTACK_DIR here. A missing canonical hook means "skip registration +# with a log line", never "bake the running tree's path". # 11. Plan-tune cathedral hook install (T8). # @@ -2072,16 +2149,31 @@ _hook_install_path() { # Idempotent via _gstack_source tag = 'plan-tune-cathedral'. If both hooks # already registered under that tag, the install skips the consent prompt and # only refreshes the registered command paths in place (ensure-event is a -# no-op when they already match — see the stale-path note above). -PLAN_TUNE_LOG_HOOK="$(_hook_install_path hosts/claude/hooks/question-log-hook)" -PLAN_TUNE_PREF_HOOK="$(_hook_install_path hosts/claude/hooks/question-preference-hook)" -AUQ_ERROR_FALLBACK_HOOK="$(_hook_install_path hosts/claude/hooks/auq-error-fallback-hook)" +# no-op when they already match). +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, +# never a baked worktree path. +if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] \ + && { [ -z "$PLAN_TUNE_LOG_HOOK" ] || [ -z "$PLAN_TUNE_PREF_HOOK" ]; }; then + log " AskUserQuestion hooks not registered: hooks missing at $CANONICAL_GSTACK_ROOT (no stable install)" +fi + if [ "$NO_TEAM_MODE" -ne 1 ] \ && [ -x "$SETTINGS_HOOK" ] \ - && [ -x "$PLAN_TUNE_LOG_HOOK" ] \ - && [ -x "$PLAN_TUNE_PREF_HOOK" ]; then + && [ -n "$PLAN_TUNE_LOG_HOOK" ] \ + && [ -n "$PLAN_TUNE_PREF_HOOK" ]; then # Already installed? Require BOTH the plan-tune source AND the AUQ-error-fallback # source — so an existing install that predates the fallback hook re-runs the @@ -2101,9 +2193,25 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ # This guarantees scripted/workspace setups (conductor, CI) are never # interactive: pass --no-plan-tune-hooks (or --plan-tune-hooks) and the # block runs to completion with no `read`. - PT_DECISION="$PLAN_TUNE_HOOKS_MODE" - [ -z "$PT_DECISION" ] && PT_DECISION="${GSTACK_PLAN_TUNE_HOOKS:-}" - [ -z "$PT_DECISION" ] && PT_DECISION="$("$GSTACK_CONFIG" get plan_tune_hooks 2>/dev/null || true)" + # PT_EXPLICIT provenance: an EXPLICIT decision (CLI flag, env var, or a key + # literally present in the config file) must never be overridden by the + # Conductor auto-opt-in below. `gstack-config get` returns the default + # "prompt" for absent keys, so provenance uses `gstack-config has` (which + # resolves GSTACK_STATE_ROOT/GSTACK_HOME/GSTACK_STATE_DIR the same way get + # does — never grep a hardcoded ~/.gstack/config.yaml). + PT_EXPLICIT=0 + if [ -n "$PLAN_TUNE_HOOKS_MODE" ]; then + PT_DECISION="$PLAN_TUNE_HOOKS_MODE" + PT_EXPLICIT=1 + elif [ -n "${GSTACK_PLAN_TUNE_HOOKS:-}" ]; then + PT_DECISION="${GSTACK_PLAN_TUNE_HOOKS}" + PT_EXPLICIT=1 + else + PT_DECISION="$("$GSTACK_CONFIG" get plan_tune_hooks 2>/dev/null || true)" + if "$GSTACK_CONFIG" has plan_tune_hooks 2>/dev/null; then + PT_EXPLICIT=1 + fi + fi # Normalize: strip whitespace + lowercase so "YES", "Yes", " yes" from a flag # or env var all resolve correctly (an unrecognized opt-in must NOT silently # downgrade to skip). Unknown values fall through to "prompt". @@ -2120,7 +2228,11 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ # falls through to "prompt" → the non-interactive skip below, leaving Conductor # users without that backstop. Treat Conductor as an implicit opt-in — but # only on the silent fall-through, never overriding an explicit --no-plan-tune-hooks. - if [ "$PT_DECISION" = "prompt" ] && { [ -n "${CONDUCTOR_WORKSPACE_PATH:-}" ] || [ -n "${CONDUCTOR_PORT:-}" ]; }; then + # Only the true silent fall-through auto-opts-in. An explicit + # --plan-tune-hooks=prompt (bin/dev-setup passes exactly this so ephemeral + # workspace setups never install) stays "prompt" — this was the bug that + # baked worktree hook paths into every Conductor user's settings.json. + if [ "$PT_DECISION" = "prompt" ] && [ "$PT_EXPLICIT" -eq 0 ] && { [ -n "${CONDUCTOR_WORKSPACE_PATH:-}" ] || [ -n "${CONDUCTOR_PORT:-}" ]; }; then PT_DECISION="yes" _PT_CONDUCTOR_AUTO=1 fi @@ -2129,18 +2241,22 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ # ensure-event (not add-event): registers when missing, RE-POINTS a stale # command path in place when the registration differs, and is a true no-op # (no write, no backup churn) when it already matches. + # Returns non-zero if ANY registration was skipped (lock contention or a + # fail-closed settings error) so callers log honestly instead of claiming + # success for a mutation that never happened. + local _pt_install_rc=0 "$SETTINGS_HOOK" ensure-event \ --event PostToolUse \ --matcher '(AskUserQuestion|mcp__.*__AskUserQuestion)' \ --command "$PLAN_TUNE_LOG_HOOK" \ --source plan-tune-cathedral \ - --timeout 5 + --timeout 5 || _pt_install_rc=1 "$SETTINGS_HOOK" ensure-event \ --event PreToolUse \ --matcher '(AskUserQuestion|mcp__.*__AskUserQuestion)' \ --command "$PLAN_TUNE_PREF_HOOK" \ --source plan-tune-cathedral \ - --timeout 5 + --timeout 5 || _pt_install_rc=1 # AskUserQuestion-failure prose-fallback reliability hook (OV3:B). Fires only when # an AskUserQuestion call returns an error/missing result; inert on success and # inert if the platform doesn't invoke PostToolUse on tool errors. MUST use its @@ -2148,14 +2264,15 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ # REPLACES the entry's hooks, so sharing 'plan-tune-cathedral' would overwrite the # question-log capture hook (same event+matcher). A distinct source = a second # PostToolUse entry; both run in parallel. - if [ -x "$AUQ_ERROR_FALLBACK_HOOK" ]; then + if [ -n "$AUQ_ERROR_FALLBACK_HOOK" ]; then "$SETTINGS_HOOK" ensure-event \ --event PostToolUse \ --matcher '(AskUserQuestion|mcp__.*__AskUserQuestion)' \ --command "$AUQ_ERROR_FALLBACK_HOOK" \ --source auq-error-fallback \ - --timeout 5 + --timeout 5 || _pt_install_rc=1 fi + return $_pt_install_rc } if [ "$ALREADY_INSTALLED" -eq 1 ]; then @@ -2172,13 +2289,17 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ log "Plan-tune hooks already installed. Run \`$SETTINGS_HOOK list-sources\` to inspect." elif [ "$PT_DECISION" = "yes" ]; then # Explicit opt-in (flag / env / config) or Conductor implicit opt-in. Non-interactive. - _install_plan_tune_hooks - log "" - if [ "${_PT_CONDUCTOR_AUTO:-0}" -eq 1 ]; then - log "AskUserQuestion reliability hooks installed (Conductor detected): decisions" - log "render as a prose brief instead of the flaky AskUserQuestion tool. Inspect with /plan-tune." + if _install_plan_tune_hooks; then + log "" + if [ "${_PT_CONDUCTOR_AUTO:-0}" -eq 1 ]; then + log "AskUserQuestion reliability hooks installed (Conductor detected): decisions" + log "render as a prose brief instead of the flaky AskUserQuestion tool. Inspect with /plan-tune." + else + log "Plan-tune hooks installed. Run /plan-tune anytime to inspect." + fi else - log "Plan-tune hooks installed. Run /plan-tune anytime to inspect." + log "" + log " warning: some AskUserQuestion hooks were NOT registered (settings lock contention or a settings error above) — re-run ./setup to complete." fi touch "$PLAN_TUNE_INSTALL_MARKER" elif [ "$PT_DECISION" = "no" ]; then @@ -2223,9 +2344,13 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ read -t "$_PT_PROMPT_TIMEOUT" -r PLAN_TUNE_INSTALL_REPLY /dev/null || PLAN_TUNE_INSTALL_REPLY="" case "$PLAN_TUNE_INSTALL_REPLY" in y|Y) - _install_plan_tune_hooks - log "" - log "Plan-tune hooks installed. Run /plan-tune anytime to inspect." + if _install_plan_tune_hooks; then + log "" + log "Plan-tune hooks installed. Run /plan-tune anytime to inspect." + else + log "" + log " warning: some AskUserQuestion hooks were NOT registered (settings lock contention or a settings error above) — re-run ./setup to complete." + fi touch "$PLAN_TUNE_INSTALL_MARKER" ;; n|N) @@ -2262,15 +2387,18 @@ fi # (F5): the hook always exits 0 and repairs best-effort — it can never block # a session. Removed by --no-team and gstack-uninstall. # -# The command path prefers the global install (see _hook_install_path): a -# dev-worktree setup used to bake its own absolute dir into settings.json, so -# deleting the worktree left a dead hook erroring on every session stop — and -# the old presence-only dedup (list-sources | grep) never re-pointed it on a -# re-run. ensure-event registers when missing, replaces a stale path in place -# (one atomic write — never zero or two registrations), and no-ops when the +# The command path is canonical-only (see _hook_command_path): a dev-worktree +# setup used to bake its own absolute dir into settings.json, so deleting the +# worktree left a dead hook erroring on every session stop — and the old +# presence-only dedup (list-sources | grep) never re-pointed it on a re-run. +# ensure-event registers when missing, replaces a stale path in place (one +# atomic write — never zero or two registrations), and no-ops when the # registration already matches. -TIMELINE_STOP_HOOK="$(_hook_install_path hosts/claude/hooks/timeline-stop-hook)" -if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -x "$TIMELINE_STOP_HOOK" ]; then +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 +if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -n "$TIMELINE_STOP_HOOK" ]; then if _TL_ENSURE_OUT=$("$SETTINGS_HOOK" ensure-event \ --event Stop \ --command "$TIMELINE_STOP_HOOK" \ @@ -2289,16 +2417,26 @@ if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -x "$TIMELINE_STOP_ esac else # Non-fatal to setup, but never silent: the hardened settings-hook refuses - # to rewrite a corrupt settings.json (exit 1), and swallowing that refusal - # left the Stop hook unregistered with no signal. + # to mutate a corrupt settings.json (exit 3) or under a held lock (exit 5), + # and swallowing that refusal left the Stop hook unregistered with no signal. log " warning: settings hook update failed: $(printf '%s\n' "$_TL_ENSURE_OUT" | head -1) — run $SETTINGS_HOOK manually" fi fi # Also tear down plan-tune + timeline hooks on --no-team (matches the existing pattern). +# Tag-only remove-source misses untagged entries (Claude Code strips +# _gstack_source), so the identity sweep (prune-stale --all) finishes the job. +# stderr stays attached on every call: a lock give-up or fail-closed parse +# error during TEARDOWN must be visible — "the next setup retries" does not +# apply when the user is turning the hooks off. if [ "$NO_TEAM_MODE" -eq 1 ] && [ -x "$SETTINGS_HOOK" ]; then - "$SETTINGS_HOOK" remove-source --source plan-tune-cathedral 2>/dev/null || true - "$SETTINGS_HOOK" remove-source --source gstack-timeline-stop 2>/dev/null || true + "$SETTINGS_HOOK" remove-source --source plan-tune-cathedral >/dev/null || true + "$SETTINGS_HOOK" remove-source --source auq-error-fallback >/dev/null || true + "$SETTINGS_HOOK" remove-source --source gstack-timeline-stop >/dev/null || true + # verify-gate is a user-registered opt-in unrelated to team mode -- turning + # team mode off must not delete it (uninstall still sweeps it, correctly, + # because there the binary itself is being removed). + GSTACK_SWEEP_EXCLUDE_SOURCES="verify-gate" "$SETTINGS_HOOK" prune-stale --all >/dev/null || true fi # ─── Redact pre-push guard consent (#1946) ─────────────────────────────────── 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/binding-template-drift.test.ts b/test/binding-template-drift.test.ts index 3fd99115d..08888400c 100644 --- a/test/binding-template-drift.test.ts +++ b/test/binding-template-drift.test.ts @@ -68,21 +68,47 @@ describe('content-binding template drift', () => { // Functional: execute the template's tripwire block against a 0-banner // original and a 1-banner outgoing body — the ABORT branch must fire. + // + // Pass the script as an ARGV element (spawnSync array form), never by + // interpolating JSON.stringify into a shell line: JSON escaping is not + // shell escaping. Inside shell double quotes a JSON "\n" stays a literal + // backslash-n, which collapsed this multi-line script onto one line where + // `then\n` became the command word `thenn` and `>&2\nelse\n` became the + // redirect `>&2nelsen` — silently littering a `2nelsen` file (containing + // "bash: thenn: command not found") in the repo root on every suite run, + // while the old not-contains assertion passed vacuously because ALL + // output had been redirected into that file. const block = body.match(/_ORIG_BANNERS=\$\(grep[\s\S]*?fi\n/); expect(block).not.toBeNull(); const fs = require('fs'); const os = require('os'); const path = require('path'); - const { execSync } = require('child_process'); + const { spawnSync } = require('child_process'); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-banner-')); try { - fs.writeFileSync(path.join(dir, 'orig.md'), 'clean body\n'); - fs.writeFileSync(path.join(dir, 'new.md'), 'body with UNTRUSTED TRACKER CONTENT banner leak\n'); - const script = block![0] - .replaceAll('/tmp/gstack-pr-body-orig-$$.md', path.join(dir, 'orig.md')) - .replaceAll('/tmp/gstack-pr-body-$$.md', path.join(dir, 'new.md')); - const out = execSync(`bash -c ${JSON.stringify(script + '; true')}`, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); - expect(out).not.toContain('banner tripwire clean'); + const scriptFor = (origContent: string, newContent: string) => { + fs.writeFileSync(path.join(dir, 'orig.md'), origContent); + fs.writeFileSync(path.join(dir, 'new.md'), newContent); + return block![0] + .replaceAll('/tmp/gstack-pr-body-orig-$$.md', path.join(dir, 'orig.md')) + .replaceAll('/tmp/gstack-pr-body-$$.md', path.join(dir, 'new.md')); + }; + + // Banner leaked into the outgoing body → the ABORT branch fires, loudly. + const abort = spawnSync('bash', ['-c', scriptFor( + 'clean body\n', + 'body with UNTRUSTED TRACKER CONTENT banner leak\n', + )], { encoding: 'utf-8' }); + expect(abort.stderr).toContain('ABORT: envelope banner leaked'); + expect(abort.stdout).not.toContain('banner tripwire clean'); + + // No banner delta → the clean branch fires. + const clean = spawnSync('bash', ['-c', scriptFor( + 'clean body\n', + 'also clean body\n', + )], { encoding: 'utf-8' }); + expect(clean.stdout).toContain('banner tripwire clean'); + expect(clean.stderr).not.toContain('ABORT'); } finally { fs.rmSync(dir, { recursive: true, force: true }); } 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 cc6f4f2bb..a6dbb252a 100644 --- a/test/gstack-settings-hook-schema-aware.test.ts +++ b/test/gstack-settings-hook-schema-aware.test.ts @@ -420,3 +420,719 @@ 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('legacy remove: per-item (regression)', () => { + test('mixed SessionStart entry: user item survives in place, gstack item removed', () => { + // REGRESSION pin: the pre-v1.67.2 legacy `remove` dropped the ENTIRE + // entry when any item matched gstack-session-update, destroying a user's + // co-located hook. The rewrite filters per-item; this is the only test of + // that branch (team-mode.test.ts covers single-item entries only). + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + SessionStart: [{ + hooks: [ + { type: 'command', command: '/Users/me/my-own-session-hook' }, + { type: 'command', command: '/old/install/bin/gstack-session-update' }, + ], + }], + }, + }, null, 2)); + const r = runIso(['remove', '/old/install/bin/gstack-session-update']); + expect(r.exitCode).toBe(0); + const s = settings(); + expect(s.hooks.SessionStart).toHaveLength(1); + expect(s.hooks.SessionStart[0].hooks).toHaveLength(1); + expect(s.hooks.SessionStart[0].hooks[0].command).toBe('/Users/me/my-own-session-hook'); + }); +}); + +describe('review-army hardening (specialist findings)', () => { + test('legacy remove preserves malformed/foreign entries it never touched', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + SessionStart: [ + { comment: 'no hooks array at all' }, + { hooks: 'not-an-array' }, + { hooks: [] }, + { hooks: [{ type: 'command', command: '/x/bin/gstack-session-update' }] }, + ], + }, + }, null, 2)); + runIso(['remove', '/x/bin/gstack-session-update']); + const s = settings(); + // Only the entry we emptied is gone; the three malformed/foreign ones stay. + expect(s.hooks.SessionStart).toHaveLength(3); + }); + + test('add-event never tags a mixed entry (old-version ratchet guard)', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [{ + matcher: AUQ_MATCHER, + _gstack_source: 'plan-tune-cathedral', + hooks: [ + { type: 'command', command: '/Users/me/my-own-hook' }, + { type: 'command', command: '/dead/wt/hosts/claude/hooks/question-log-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 e = s.hooks.PostToolUse[0]; + expect(e.hooks).toHaveLength(2); + expect(e.hooks[0].command).toBe('/Users/me/my-own-hook'); + // A tag on a mixed entry hands old-version remove-source permission to + // destroy the user's item — it must be gone. + expect(e._gstack_source).toBeUndefined(); + }); + + test('two dead twins of one hook in ONE entry collapse to a single item after --repoint', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [{ + matcher: AUQ_MATCHER, + hooks: [ + { type: 'command', command: '/dead/a/hosts/claude/hooks/question-log-hook' }, + { type: 'command', command: '/dead/b/hosts/claude/hooks/question-log-hook' }, + ], + }], + }, + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + const items = settings().hooks.PostToolUse[0].hooks; + expect(items).toHaveLength(1); // pre-fix: two identical items → hook fires twice per event + expect(items[0].command).toBe(`${canon}/hosts/claude/hooks/question-log-hook`); + }); + + test('a 0600 settings.json keeps its mode across mutations (API keys stay private)', () => { + fs.writeFileSync(settingsFile, JSON.stringify({ env: { SECRET: 'x' } }, null, 2)); + fs.chmodSync(settingsFile, 0o600); + runIso(['add-event', '--event', 'Stop', '--command', '/x/hosts/claude/hooks/timeline-stop-hook', '--source', 'gstack-timeline-stop']); + const mode = fs.statSync(settingsFile).mode & 0o777; + expect(mode).toBe(0o600); + }); + + test('a canonical root containing $ is escaped in the registered command', () => { + const trickyBase = path.join(tmpDir, 'weird$dir'); + fs.mkdirSync(trickyBase, { recursive: true }); + const canon = mkCanon(trickyBase); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { Stop: [hookEntry('/dead/wt/hosts/claude/hooks/timeline-stop-hook')] }, + }, null, 2)); + runIso(['prune-stale', '--repoint', canon]); + const cmd = settings().hooks.Stop[0].hooks[0].command; + expect(cmd.startsWith('"')).toBe(true); + expect(cmd).toContain('\\$'); // $ neutralized — shell must not expand it at hook-fire time + // Idempotent: the escaped command is still recognized as ours. + 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('backups rotate: at most 10 .bak files survive repeated mutations', () => { + for (let i = 0; i < 13; i++) { + runIso(['add-event', '--event', 'Stop', '--command', `/x/hosts/claude/hooks/timeline-stop-hook-${i}`, '--source', 'gstack-timeline-stop']); + } + expect(backups().length).toBeLessThanOrEqual(10); + // The rollback pointer still resolves to an existing backup. + const latest = fs.readFileSync(path.join(tmpDir, 'settings.json.bak-latest'), 'utf-8').trim(); + 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'); + fs.writeFileSync(evil, JSON.stringify({ hooks: { Stop: [{ hooks: [{ type: 'command', command: '/evil' }] }] } })); + fs.writeFileSync(path.join(tmpDir, 'settings.json.bak-latest'), evil + '\n'); + const r = runIso(['rollback']); + expect(r.exitCode).not.toBe(0); + expect(r.stderr).toMatch(/refusing/); + expect(settings().a).toBe(1); + }); +}); + +describe('ownership negatives', () => { + test('owned basename+relpath under the WRONG matcher stays foreign (not re-pointed)', () => { + const canon = mkCanon(tmpDir); + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + PostToolUse: [hookEntry('/dead/wt/hosts/claude/hooks/question-log-hook', 'Bash')], + }, + }, null, 2)); + 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('prune-stale on an absent settings file exits 0 with removed 0', () => { + const r = runIso(['prune-stale', '--repoint', '/nonexistent-root']); + expect(r.exitCode).toBe(0); + expect(r.stdout).toMatch(/removed 0 gstack hook entries \(repointed 0\)/); + expect(fs.existsSync(settingsFile)).toBe(false); + }); +}); + +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 loudly (exit 5), 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'); + const r = runIso( + ['add-event', '--event', 'Stop', '--command', '/x', '--source', 's'], + { GSTACK_SETTINGS_LOCK_TIMEOUT_MS: '300' }, + ); + expect(r.exitCode).toBe(5); // loud give-up, not silent skip + expect(r.stderr).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); + }); +}); + +describe('gstack-settings-hook adversarial hardening', () => { + let tmpDir: string; + let settingsFile: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-settings-adv-')); + settingsFile = path.join(tmpDir, 'settings.json'); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function runAdv(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 }; + } + } + + const advSettings = (): any => JSON.parse(fs.readFileSync(settingsFile, 'utf-8')); + + test('wrong-shape hooks value fails LOUD (exit 4), file untouched', () => { + // bun -e swallows uncaught exceptions after a require() and exits 0 + // (verified on bun 1.3.13) -- without the gsMain umbrella this exact + // input produced a silent exit-0 no-op that reported clean. + const raw = JSON.stringify({ hooks: { Stop: { bogus: 'shape' } } }, null, 2) + '\n'; + fs.writeFileSync(settingsFile, raw); + const r = runAdv(['prune-stale', '--all']); + expect(r.exitCode).toBe(4); + expect(r.stderr).toMatch(/internal error/); + expect(r.stderr).toMatch(/refusing to mutate/); + expect(fs.readFileSync(settingsFile, 'utf-8')).toBe(raw); + }); + + test('foreign hook whose basename collides with Object.prototype survives --all', () => { + // KNOWN_HOOKS["toString"] returns an inherited member without the + // hasOwnProperty guard -- pre-fix, this threw mid-scan and turned the + // sweep into a silent no-op. + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + Stop: [ + { hooks: [{ type: 'command', command: '/usr/local/bin/toString' }] }, + { _gstack_source: 'gstack-timeline-stop', hooks: [{ type: 'command', command: '/x/hosts/claude/hooks/timeline-stop-hook' }] }, + ], + }, + }, null, 2) + '\n'); + const r = runAdv(['prune-stale', '--all']); + expect(r.exitCode).toBe(0); + const s = advSettings(); + expect(s.hooks.Stop).toHaveLength(1); // gstack entry swept... + expect(s.hooks.Stop[0].hooks[0].command).toBe('/usr/local/bin/toString'); // ...foreign one kept + }); + + test('GSTACK_SWEEP_EXCLUDE_SOURCES preserves verify-gate during an --all sweep', () => { + // `setup --no-team` sweeps team hooks but must not delete the + // user-registered verify-gate opt-in whose binary still exists. + fs.writeFileSync(settingsFile, JSON.stringify({ + hooks: { + Stop: [ + { _gstack_source: 'verify-gate', hooks: [{ type: 'command', command: '/x/bin/gstack-verify-gate' }] }, + { _gstack_source: 'gstack-timeline-stop', hooks: [{ type: 'command', command: '/x/hosts/claude/hooks/timeline-stop-hook' }] }, + ], + }, + }, null, 2) + '\n'); + const r = runAdv(['prune-stale', '--all'], { GSTACK_SWEEP_EXCLUDE_SOURCES: 'verify-gate' }); + expect(r.exitCode).toBe(0); + expect(r.stdout).toMatch(/removed 1 /); + const s = advSettings(); + expect(s.hooks.Stop).toHaveLength(1); + expect(s.hooks.Stop[0]._gstack_source).toBe('verify-gate'); + }); +}); diff --git a/test/setup-hook-canonical-paths.test.ts b/test/setup-hook-canonical-paths.test.ts new file mode 100644 index 000000000..b5f4b8a9b --- /dev/null +++ b/test/setup-hook-canonical-paths.test.ts @@ -0,0 +1,216 @@ +/** + * 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/); + }); +}); + +describe('matcher-literal drift tripwire (review-army)', () => { + test("every --matcher literal in setup equals its KNOWN_HOOKS row's matcher", () => { + // gsOwnedRow requires an EXACT matcher match — if setup's registration + // matcher drifts from the table row, identity re-pointing/pruning silently + // stops recognizing the hook and the phantom-duplicate class returns. + const rowMatcher = (name: string) => { + const rowStart = hookBinSrc.indexOf(`"${name}":`); + expect(rowStart).toBeGreaterThan(-1); + const row = hookBinSrc.slice(rowStart, hookBinSrc.indexOf('}', rowStart)); + return row.match(/matcher: "([^"]*)"/)![1]; + }; + const pairs: Array<[string, string]> = [ + ['question-log-hook', '(AskUserQuestion|mcp__.*__AskUserQuestion)'], + ['question-preference-hook', '(AskUserQuestion|mcp__.*__AskUserQuestion)'], + ['auq-error-fallback-hook', '(AskUserQuestion|mcp__.*__AskUserQuestion)'], + ]; + for (const [name, expected] of pairs) { + expect(rowMatcher(name)).toBe(expected); + } + // And setup registers those hooks with exactly that matcher literal. + const matcherLiterals = [...setupSrc.matchAll(/--matcher '([^']+)'/g)].map((m) => m[1]); + expect(matcherLiterals.length).toBeGreaterThanOrEqual(3); + for (const lit of matcherLiterals) { + expect(lit).toBe('(AskUserQuestion|mcp__.*__AskUserQuestion)'); + } + }); +}); 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/timeline-stop-hook.test.ts b/test/timeline-stop-hook.test.ts index 7a20eda1b..4d49a06d6 100644 --- a/test/timeline-stop-hook.test.ts +++ b/test/timeline-stop-hook.test.ts @@ -261,37 +261,47 @@ describe('timeline-stop-hook wiring', () => { expect(setup).not.toMatch(/list-sources 2>\/dev\/null \| grep -q "gstack-timeline-stop"/); }); - test('fresh register prefers the global-install hook path when present', () => { - // Drive setup's _hook_install_path directly: global install present → the - // registration survives deleting the worktree setup ran from. + 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_install_path\(\) \{[\s\S]*?\n\}/); + 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 globalHook = path.join( - fakeHome, '.claude', 'skills', 'gstack', 'hosts', 'claude', 'hooks', 'timeline-stop-hook', - ); + 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' }; + 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_install_path hosts/claude/hooks/timeline-stop-hook`], + ['-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 global install (fresh first install from a clone) → setup-time path. + // 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_install_path hosts/claude/hooks/timeline-stop-hook`], + ['-c', `${fn![0]}\n_hook_command_path hosts/claude/hooks/timeline-stop-hook`], { env, encoding: 'utf-8', timeout: 10_000 }, ); - expect(withoutGlobal.stdout.trim()).toBe('/some/dev/worktree/hosts/claude/hooks/timeline-stop-hook'); + expect(withoutGlobal.status).not.toBe(0); + expect(withoutGlobal.stdout.trim()).toBe(''); } finally { fs.rmSync(fakeHome, { recursive: true, force: true }); } @@ -357,10 +367,11 @@ describe('timeline-stop-hook wiring', () => { } }); - test('corrupt settings.json: ensure-event refuses (exit 1) and never rewrites the file', () => { + 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, exit 1, file byte-identical. + // 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'); @@ -375,7 +386,7 @@ describe('timeline-stop-hook wiring', () => { '--timeout', '5', ], { env: { ...process.env, GSTACK_SETTINGS_FILE: settingsFile }, encoding: 'utf-8', timeout: 15_000 }); - expect(r.status).toBe(1); + 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. diff --git a/test/uninstall.test.ts b/test/uninstall.test.ts index 1ed3edf83..96e2e689e 100644 --- a/test/uninstall.test.ts +++ b/test/uninstall.test.ts @@ -231,3 +231,126 @@ 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 }); + } + // 30s: the copied uninstaller spawns several bun -e children; on a loaded + // box their cold starts blow bun's default 5s per-test timeout. + }, 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-')); + 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: { + Stop: [{ + _gstack_source: 'gstack-timeline-stop', + hooks: [{ type: 'command', command: `${installRoot}/hosts/claude/hooks/timeline-stop-hook` }], + }], + }, + }, null, 2)); + fs.mkdirSync(path.join(mockHome, '.gstack'), { recursive: true }); + // A fresh foreign lock: pre-fix, every cleanup call silently skipped and + // uninstall reported clean while orphaning the hooks forever. + fs.mkdirSync(`${settingsFile}.lock`); + fs.writeFileSync(path.join(`${settingsFile}.lock`, 'owner'), 'another-live-process'); + + 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'), + GSTACK_SETTINGS_LOCK_TIMEOUT_MS: '300', + }, + cwd: tmp, + }); + const stderr = result.stderr.toString(); + const s = JSON.parse(fs.readFileSync(settingsFile, 'utf-8')); + const cleaned = s.hooks?.Stop === undefined; + // Either the sweep still happened, or the user SEES why it didn't. + expect(cleaned || /could not acquire lock/.test(stderr)).toBe(true); + expect(/could not acquire lock/.test(stderr)).toBe(true); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + // 30s: several settings-hook calls each wait out the 300ms lock give-up, + // and their bun -e cold starts stack up under load — the default 5s + // per-test budget is too tight on a busy box. + }, 30000); +}); diff --git a/test/verify-gate.test.ts b/test/verify-gate.test.ts index 99bf9670b..922b7e8bb 100644 --- a/test/verify-gate.test.ts +++ b/test/verify-gate.test.ts @@ -349,7 +349,17 @@ describe('opt-in contract (adapted from the fork: NOT registered by default)', ( const gate = fs.readFileSync(GATE, 'utf-8'); test('./setup does NOT register the gate — a Stop hook running the verify command after every turn is opt-in', () => { - expect(setup).not.toContain('verify-gate'); + // The only permitted mentions are comments and the --no-team sweep + // EXCLUSION (protecting a user-registered gate from team-mode teardown). + // A registration (add-event) referencing the gate stays banned. + const mentions = setup.split('\n').filter((l) => l.includes('verify-gate')); + expect(mentions.length).toBeGreaterThan(0); // the exclusion itself is pinned + for (const line of mentions) { + const t = line.trim(); + const allowed = t.startsWith('#') || t.includes('GSTACK_SWEEP_EXCLUDE_SOURCES="verify-gate"'); + expect(allowed).toBe(true); + expect(t).not.toContain('add-event'); + } }); test('the bin documents its own registration and removal commands', () => {