mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(setup): canonical-only hook registration, heal-first, PT_EXPLICIT consent provenance
Three root causes of the phantom-AskUserQuestion-hooks class, all in the
registration path:
- Bug A: the Conductor auto-opt-in upgraded PT_DECISION "prompt" -> "yes"
even when "prompt" was dev-setup's EXPLICIT --plan-tune-hooks=prompt pin,
so every new Conductor workspace installed hooks. PT_EXPLICIT (flag/env/
config-key-presence via `gstack-config has`) now gates the auto-opt-in to
the true silent fall-through.
- Bug B: hook commands were baked from $SOURCE_GSTACK_DIR (`pwd -P` of the
running tree — ephemeral for worktrees). Registration is now CANONICAL-ONLY
via _hook_command_path (${CLAUDE_CONFIG_DIR:-$HOME/.claude}/skills/gstack);
missing canonical hook = skip + log, never a baked tree path. SessionStart
moves to schema-aware add-event under its identity source; whitespace paths
are quoted.
- Bug C: nothing ever pruned, and dead tagged entries blocked the
"already installed" guards forever. Setup now heals FIRST on every run
(prune-stale --repoint at the stable install), surfaces a one-line summary
only when something changed, surfaces the plan_tune_hooks:no-vs-live-hooks
contradiction, and --no-team tears down all three sources plus an identity
sweep for untagged strays.
dev-setup's no-mutation guarantee gains its stated repair exception (prune
dead / re-point existing, never ADD).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
cec156c9f1
commit
c988cb7ae6
+11
-4
@@ -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
|
||||
|
||||
@@ -1853,22 +1853,94 @@ 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"
|
||||
|
||||
# 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.<ts>; undo: $SETTINGS_HOOK rollback)"
|
||||
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
|
||||
case "$SESSION_UPDATE_CMD" in
|
||||
*" "*) SESSION_UPDATE_CMD="\"$SESSION_UPDATE_CMD\"" ;;
|
||||
esac
|
||||
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).
|
||||
if [ -x "$SETTINGS_HOOK" ] && [ -n "$HOOK_CMD" ]; then
|
||||
"$SETTINGS_HOOK" add-event --event SessionStart --command "$HOOK_CMD" --source gstack-session-update >/dev/null 2>&1 || 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 ""
|
||||
@@ -1987,15 +2059,22 @@ fi
|
||||
#
|
||||
# Idempotent via _gstack_source tag = 'plan-tune-cathedral'. If both hooks
|
||||
# already registered under that tag, the install is a no-op (no prompt).
|
||||
PLAN_TUNE_LOG_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/question-log-hook"
|
||||
PLAN_TUNE_PREF_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/question-preference-hook"
|
||||
AUQ_ERROR_FALLBACK_HOOK="$SOURCE_GSTACK_DIR/hosts/claude/hooks/auq-error-fallback-hook"
|
||||
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)"
|
||||
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
|
||||
@@ -2015,9 +2094,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".
|
||||
@@ -2034,7 +2129,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
|
||||
@@ -2059,7 +2158,7 @@ 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" add-event \
|
||||
--event PostToolUse \
|
||||
--matcher '(AskUserQuestion|mcp__.*__AskUserQuestion)' \
|
||||
@@ -2164,8 +2263,8 @@ fi
|
||||
# (F5): the hook always exits 0 and repairs best-effort — it can never block
|
||||
# a session. Idempotent via the (event, source) dedup in gstack-settings-hook;
|
||||
# removed by --no-team and gstack-uninstall.
|
||||
TIMELINE_STOP_HOOK="$SOURCE_GSTACK_DIR/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 [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -n "$TIMELINE_STOP_HOOK" ]; then
|
||||
if ! "$SETTINGS_HOOK" list-sources 2>/dev/null | grep -q "gstack-timeline-stop"; then
|
||||
if "$SETTINGS_HOOK" add-event \
|
||||
--event Stop \
|
||||
@@ -2178,9 +2277,13 @@ if [ "$NO_TEAM_MODE" -ne 1 ] && [ -x "$SETTINGS_HOOK" ] && [ -x "$TIMELINE_STOP_
|
||||
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.
|
||||
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 auq-error-fallback 2>/dev/null || true
|
||||
"$SETTINGS_HOOK" remove-source --source gstack-timeline-stop 2>/dev/null || true
|
||||
"$SETTINGS_HOOK" prune-stale --all >/dev/null 2>&1 || true
|
||||
fi
|
||||
|
||||
# ─── Redact pre-push guard consent (#1946) ───────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user