From 079b36c7f8d3c8c2e6c5d95dd29f063dec6e4c5a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 31 Aug 2026 21:26:55 +0000 Subject: [PATCH] =?UTF-8?q?feat(setup):=20persistent=20timeline=20Stop=20h?= =?UTF-8?q?ook=20opt-out=20=E2=80=94=20timeline=5Fstop=5Fhook=20config=20g?= =?UTF-8?q?ate=20(#2677)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --no-team is a one-shot teardown, so every later bare ./setup (including the ones /gstack-upgrade runs) re-registered the timeline Stop hook with no way to say 'never'. New gate mirrors the plan_tune_hooks pattern: flag (--timeline-stop-hook/--no-timeline-stop-hook) > env (GSTACK_TIMELINE_STOP_HOOK) > saved config (timeline_stop_hook) > default yes. An explicit flag persists to config so the decision survives upgrades; an explicit 'no' also removes a live registration (reconciliation), so the opt-out works against installs registered by an older setup. --no-team semantics unchanged (NO_TEAM_MODE is never initialized from config). Full gstack-config surface: DEFAULTS entry, header docs, list/defaults enumeration, warn-and-default validation. Fixes #2677 Co-Authored-By: Claude Fable 5 --- bin/gstack-config | 19 +++++- setup | 34 +++++++++- test/setup-timeline-hook-gate.test.ts | 97 +++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 test/setup-timeline-hook-gate.test.ts diff --git a/bin/gstack-config b/bin/gstack-config index cc1c20e6a..d8e005753 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -93,6 +93,14 @@ CONFIG_HEADER='# gstack configuration — edit freely, changes take effect on ne # # Set to true once the privacy gate has asked the user. # # Flip back to false to be re-prompted. # +# ─── Timeline Stop hook ────────────────────────────────────────────── +# timeline_stop_hook: yes # Controls whether ./setup registers the timeline +# # Stop hook (closes dangling session entries). +# # yes — register on every setup (default) +# # no — never register; setup also removes a +# # live registration (persistent opt-out; +# # --no-team stays a one-shot teardown, #2677) +# # ─── Plan-tune hooks ───────────────────────────────────────────────── # plan_tune_hooks: prompt # Controls whether ./setup installs the plan-tune # # Claude Code hooks (PostToolUse capture + @@ -148,6 +156,7 @@ lookup_default() { artifacts_sync_mode) echo "off" ;; artifacts_sync_mode_prompted) echo "false" ;; plan_tune_hooks) echo "prompt" ;; # prompt | yes | no — controls ./setup plan-tune hook install + timeline_stop_hook) echo "yes" ;; # yes | no — controls ./setup timeline Stop hook registration (#2677) redact_repo_visibility) echo "" ;; # empty → fall through to gh/glab detection redact_prepush_hook) echo "false" ;; @@ -396,6 +405,10 @@ case "${1:-}" in echo "Warning: plan_tune_hooks '$VALUE' not recognized. Valid values: prompt, yes, no. Using prompt." >&2 VALUE="prompt" fi + if [ "$KEY" = "timeline_stop_hook" ] && [ "$VALUE" != "yes" ] && [ "$VALUE" != "no" ]; then + echo "Warning: timeline_stop_hook '$VALUE' not recognized. Valid values: yes, no. Using yes." >&2 + VALUE="yes" + fi # codex_reviews controls PAID Codex calls. Unlike the warn-and-default keys above, # an invalid value is REJECTED and the existing setting is left unchanged — a typo # must never silently flip the switch and turn paid Codex calls on or off. @@ -441,7 +454,8 @@ case "${1:-}" in for KEY in proactive routing_declined telemetry auto_upgrade update_check \ skill_prefix checkpoint_mode checkpoint_push explain_level \ codex_reviews gstack_contributor skip_eng_review workspace_root \ - artifacts_sync_mode artifacts_sync_mode_prompted plan_tune_hooks; do + artifacts_sync_mode artifacts_sync_mode_prompted plan_tune_hooks \ + timeline_stop_hook; do VALUE=$(read_config_value "$KEY" || true) SOURCE="default" if [ -n "$VALUE" ]; then @@ -457,7 +471,8 @@ case "${1:-}" in for KEY in proactive routing_declined telemetry auto_upgrade update_check \ skill_prefix checkpoint_mode checkpoint_push explain_level \ codex_reviews gstack_contributor skip_eng_review workspace_root \ - artifacts_sync_mode artifacts_sync_mode_prompted plan_tune_hooks; do + artifacts_sync_mode artifacts_sync_mode_prompted plan_tune_hooks \ + timeline_stop_hook; do printf ' %-24s %s\n' "$KEY:" "$(lookup_default "$KEY")" done ;; diff --git a/setup b/setup index 4e4cce4e3..67a4e1358 100755 --- a/setup +++ b/setup @@ -183,6 +183,7 @@ SKILL_PREFIX_FLAG=0 TEAM_MODE=0 NO_TEAM_MODE=0 PLAN_TUNE_HOOKS_MODE="" # "" = resolve from env/config/prompt; "yes"/"no" = explicit +TIMELINE_STOP_HOOK_MODE="" # "" = resolve from env/config; "yes"/"no" = explicit (#2677) MODEL_OVERRIDE="" MODEL_OVERRIDE_SET=0 while [ $# -gt 0 ]; do @@ -199,6 +200,9 @@ while [ $# -gt 0 ]; do --plan-tune-hooks) PLAN_TUNE_HOOKS_MODE="yes"; shift ;; --no-plan-tune-hooks) PLAN_TUNE_HOOKS_MODE="no"; shift ;; --plan-tune-hooks=*) PLAN_TUNE_HOOKS_MODE="${1#--plan-tune-hooks=}"; shift ;; + --timeline-stop-hook) TIMELINE_STOP_HOOK_MODE="yes"; shift ;; + --no-timeline-stop-hook) TIMELINE_STOP_HOOK_MODE="no"; shift ;; + --timeline-stop-hook=*) TIMELINE_STOP_HOOK_MODE="${1#--timeline-stop-hook=}"; shift ;; -q|--quiet) QUIET=1; shift ;; *) shift ;; esac @@ -2463,7 +2467,35 @@ TIMELINE_STOP_HOOK="$(_hook_command_path hosts/claude/hooks/timeline-stop-hook | 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 +# #2677: PERSISTENT gate, mirroring the plan_tune_hooks pattern. --no-team is +# (and stays) a one-shot teardown — every later bare ./setup, including the +# ones /gstack-upgrade runs, re-registered the hook with no way to say +# "never". Resolution: flag > env (GSTACK_TIMELINE_STOP_HOOK) > saved config +# (timeline_stop_hook) > default yes. An explicit FLAG persists to config so +# the decision survives upgrades; env stays session-scoped. Do NOT initialize +# NO_TEAM_MODE from config — that would silently change --no-team semantics. +if [ -n "$TIMELINE_STOP_HOOK_MODE" ]; then + TL_DECISION="$TIMELINE_STOP_HOOK_MODE" +elif [ -n "${GSTACK_TIMELINE_STOP_HOOK:-}" ]; then + TL_DECISION="${GSTACK_TIMELINE_STOP_HOOK}" +else + TL_DECISION="$("$GSTACK_CONFIG" get timeline_stop_hook 2>/dev/null || true)" +fi +TL_DECISION=$(printf '%s' "$TL_DECISION" | tr '[:upper:]' '[:lower:]' | tr -d '[:space:]') +case "$TL_DECISION" in + n|no|false|skip|off|0) TL_DECISION="no" ;; + *) TL_DECISION="yes" ;; +esac +if [ -n "$TIMELINE_STOP_HOOK_MODE" ]; then + "$GSTACK_CONFIG" set timeline_stop_hook "$TL_DECISION" >/dev/null 2>&1 || true +fi +if [ "$TL_DECISION" = "no" ] && [ -x "$SETTINGS_HOOK" ]; then + # Reconciliation arm: an explicit "no" with a live registration removes it — + # the opt-out works even when the hook was registered by an older setup. + "$SETTINGS_HOOK" remove-source --source gstack-timeline-stop >/dev/null 2>&1 || true + log " timeline Stop hook disabled (timeline_stop_hook=no) — any existing registration removed" +fi +if [ "$NO_TEAM_MODE" -ne 1 ] && [ "$TL_DECISION" != "no" ] && [ -x "$SETTINGS_HOOK" ] && [ -n "$TIMELINE_STOP_HOOK" ]; then if _TL_ENSURE_OUT=$("$SETTINGS_HOOK" ensure-event \ --event Stop \ --command "$TIMELINE_STOP_HOOK" \ diff --git a/test/setup-timeline-hook-gate.test.ts b/test/setup-timeline-hook-gate.test.ts new file mode 100644 index 000000000..cf392d4ac --- /dev/null +++ b/test/setup-timeline-hook-gate.test.ts @@ -0,0 +1,97 @@ +/** + * Timeline Stop hook persistent gate (#2677). + * + * --no-team is a one-shot teardown: every later bare ./setup — including the + * ones /gstack-upgrade runs — re-registered the Stop hook with no way to say + * "never". The gate mirrors the plan_tune_hooks pattern: flag > env + * (GSTACK_TIMELINE_STOP_HOOK) > saved config (timeline_stop_hook) > default + * yes; an explicit FLAG persists to config; an explicit "no" also REMOVES a + * live registration (reconciliation), so the opt-out works against installs + * registered by an older setup. + * + * Static pins on `setup` + real gstack-config runs — driving full ./setup in + * a unit test is disproportionate; the wiring shapes below are the contract. + */ +import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +const ROOT = path.resolve(import.meta.dir, ".."); +const setupSrc = fs.readFileSync(path.join(ROOT, "setup"), "utf-8"); +const CONFIG = path.join(ROOT, "bin", "gstack-config"); + +describe("setup: timeline Stop hook gate (#2677)", () => { + test("flags parse: --timeline-stop-hook / --no-timeline-stop-hook / = form", () => { + expect(setupSrc).toContain('--timeline-stop-hook) TIMELINE_STOP_HOOK_MODE="yes"'); + expect(setupSrc).toContain('--no-timeline-stop-hook) TIMELINE_STOP_HOOK_MODE="no"'); + expect(setupSrc).toContain('--timeline-stop-hook=*) TIMELINE_STOP_HOOK_MODE='); + }); + + test("resolution order: flag > env > config, normalized, default yes", () => { + const block = setupSrc.slice(setupSrc.indexOf("#2677: PERSISTENT gate")); + const flag = block.indexOf('TL_DECISION="$TIMELINE_STOP_HOOK_MODE"'); + const env = block.indexOf('TL_DECISION="${GSTACK_TIMELINE_STOP_HOOK}"'); + const cfg = block.indexOf("get timeline_stop_hook"); + expect(flag).toBeGreaterThan(-1); + expect(env).toBeGreaterThan(flag); + expect(cfg).toBeGreaterThan(env); + // Negative-value normalization uses the shared set. + expect(block).toContain('n|no|false|skip|off|0) TL_DECISION="no"'); + }); + + test("registration guard requires TL_DECISION != no; explicit flag persists to config", () => { + expect(setupSrc).toMatch( + /\[ "\$NO_TEAM_MODE" -ne 1 \] && \[ "\$TL_DECISION" != "no" \] && \[ -x "\$SETTINGS_HOOK" \]/, + ); + expect(setupSrc).toContain('"$GSTACK_CONFIG" set timeline_stop_hook "$TL_DECISION"'); + }); + + test("reconciliation arm: explicit no removes a live registration", () => { + const noArm = setupSrc.indexOf('[ "$TL_DECISION" = "no" ] && [ -x "$SETTINGS_HOOK" ]'); + expect(noArm).toBeGreaterThan(-1); + const arm = setupSrc.slice(noArm, noArm + 400); + expect(arm).toContain("remove-source --source gstack-timeline-stop"); + }); + + test("--no-team semantics unchanged: NO_TEAM_MODE stays a hardcoded initializer", () => { + expect(setupSrc).toContain("NO_TEAM_MODE=0"); + expect(setupSrc).not.toMatch(/NO_TEAM_MODE=\$\(/); + }); +}); + +describe("gstack-config: timeline_stop_hook key surface", () => { + function runConfig(args: string[], home: string): { stdout: string; stderr: string; status: number | null } { + const r = spawnSync(CONFIG, args, { + encoding: "utf-8", + timeout: 15_000, + env: { ...process.env, GSTACK_HOME: home, GSTACK_STATE_ROOT: home }, + }); + return { stdout: r.stdout ?? "", stderr: r.stderr ?? "", status: r.status }; + } + + test("default is yes; set/get round-trips; list and defaults enumerate the key", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-tlhook-")); + try { + expect(runConfig(["get", "timeline_stop_hook"], home).stdout.trim()).toBe("yes"); + expect(runConfig(["defaults"], home).stdout).toContain("timeline_stop_hook:"); + runConfig(["set", "timeline_stop_hook", "no"], home); + expect(runConfig(["get", "timeline_stop_hook"], home).stdout.trim()).toBe("no"); + expect(runConfig(["list"], home).stdout).toContain("timeline_stop_hook:"); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test("malformed values warn and default to yes", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-tlhook-")); + try { + const r = runConfig(["set", "timeline_stop_hook", "banana"], home); + expect(r.stderr).toContain("not recognized"); + expect(runConfig(["get", "timeline_stop_hook"], home).stdout.trim()).toBe("yes"); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); +});