mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
feat(setup): persistent timeline Stop hook opt-out — timeline_stop_hook config gate (#2677)
--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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7ed5e87bba
commit
079b36c7f8
+17
-2
@@ -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
|
||||
;;
|
||||
|
||||
@@ -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" \
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user