From 4d0e7b7c2a417d7ac1a8937483c28c4242e78163 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 09:35:36 -0700 Subject: [PATCH] =?UTF-8?q?feat(hooks):=20Stop=20hook=20closes=20dangling?= =?UTF-8?q?=20timeline=20entries=20=E2=80=94=20fail-open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preamble writes event:'started' to the project timeline at every skill start, but the matching 'completed' write lives in prose at the END of the skill workflow — unenforceable. An interrupted session, a context blowout, or an agent that simply stops leaked started > completed forever, and the leak was unrepairable after the fact (observed live in #2553). New hosts/claude/hooks/timeline-stop-hook (+ .ts, question-log-hook shim pattern): on Claude Code's Stop event it appends event:'completed' with outcome 'unknown' and source 'stop-hook' for every 'started' entry in the project timeline that has no matching completion. setup registers it via gstack-settings-hook add-event (Stop was already an accepted event) under its own source tag, idempotently; --no-team and gstack-uninstall remove it. FAIL-OPEN contract (F5), pinned by tests: ALWAYS exits 0 — corrupt timeline (bad lines skipped individually, valid ones still repaired), missing timeline, garbage/empty stdin, bun missing from PATH (the shim '|| true's), and an over-cap timeline (10MB skip) all repair nothing and block nothing; errors land in ~/.gstack/hook-errors.log best-effort. The write path is append-only with a ~2s internal budget, and a second Stop is a no-op (already-closed entries never re-close). Correlation is project-scoped by design — the preamble's session id is shell-local, so a concurrent same-project session's entry may close early as a traceable source:'stop-hook' row rather than a silent leak; the header documents the trade-off. Fixes #2553 Co-Authored-By: Claude Fable 5 --- bin/gstack-uninstall | 4 + hosts/claude/hooks/timeline-stop-hook | 10 ++ hosts/claude/hooks/timeline-stop-hook.ts | 162 +++++++++++++++++++ setup | 24 ++- test/timeline-stop-hook.test.ts | 197 +++++++++++++++++++++++ 5 files changed, 396 insertions(+), 1 deletion(-) create mode 100755 hosts/claude/hooks/timeline-stop-hook create mode 100755 hosts/claude/hooks/timeline-stop-hook.ts create mode 100644 test/timeline-stop-hook.test.ts diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall index f5199f293..181b3813a 100755 --- a/bin/gstack-uninstall +++ b/bin/gstack-uninstall @@ -318,6 +318,10 @@ if [ -x "$SETTINGS_HOOK" ]; then 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 ──────────────────────────────────── diff --git a/hosts/claude/hooks/timeline-stop-hook b/hosts/claude/hooks/timeline-stop-hook new file mode 100755 index 000000000..2beb36b74 --- /dev/null +++ b/hosts/claude/hooks/timeline-stop-hook @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +# Bash shim — Claude Code hooks run `command` strings via /bin/sh, so this +# wrapper makes the TypeScript hook executable via bun. Settings.json +# references this file directly. +# +# FAIL-OPEN (F5): a Stop-event telemetry repair must never block the session. +# Every failure path — bun missing, script crash — still exits 0. +HERE="$(cd "$(dirname "$0")" && pwd)" || exit 0 +bun "$HERE/timeline-stop-hook.ts" || true +exit 0 diff --git a/hosts/claude/hooks/timeline-stop-hook.ts b/hosts/claude/hooks/timeline-stop-hook.ts new file mode 100755 index 000000000..1b0b3f189 --- /dev/null +++ b/hosts/claude/hooks/timeline-stop-hook.ts @@ -0,0 +1,162 @@ +#!/usr/bin/env bun +/** + * Stop hook: close dangling "started" timeline entries (#2553). + * + * The preamble writes {"skill":X,"event":"started",...} at every skill start; + * the completion write lives in prose at the END of the skill workflow and is + * unenforceable — an interrupted session, a context blowout, or an agent that + * simply stops leaves started > completed forever, and the leak is + * unrepairable after the fact. This hook runs on Claude Code's Stop event and + * appends event:"completed" (outcome "unknown", source "stop-hook") for every + * "started" entry in the project's timeline that has no matching "completed". + * + * FAIL-OPEN CONTRACT (F5) — a telemetry repair must never block a session: + * - ALWAYS exits 0, whatever happens (corrupt timeline, missing file, bad + * stdin, unreadable slug). Errors go to ~/.gstack/hook-errors.log, + * best-effort. + * - Internal time budget (~2s): work is bounded up front — the timeline is + * skipped entirely over a size cap, and the deadline is re-checked before + * the write. Claude Code's own hook timeout is the outer belt. + * - Append-only: never rewrites timeline.jsonl. + * + * Correlation limits, on purpose: the preamble's session id is a shell-local + * "$$-epoch", not the Claude session id this hook receives, so entries can't + * be attributed to THIS session specifically. Closing every dangling entry in + * the project is the repair semantics #2553 asks for; a concurrent session + * mid-skill in the same project may get its entry closed early, which shows + * up as a traceable source:"stop-hook" completion rather than a silent leak. + */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { runBin } from './spawn-bin'; + +const DEADLINE_MS = 2000; +const MAX_TIMELINE_BYTES = 10 * 1024 * 1024; +const startedAt = Date.now(); + +function stateRoot(): string { + return process.env.GSTACK_HOME || path.join(os.homedir(), '.gstack'); +} + +function logHookError(msg: string): void { + try { + const root = stateRoot(); + fs.mkdirSync(root, { recursive: true }); + fs.appendFileSync( + path.join(root, 'hook-errors.log'), + `${new Date().toISOString()} timeline-stop-hook: ${msg}\n`, + ); + } catch { + // best-effort; never block the session because logging failed + } +} + +interface TimelineEntry { + skill?: string; + event?: string; + session?: string; + branch?: string; + ts?: string; +} + +function main(): void { + let cwd = process.cwd(); + try { + const stdin = fs.readFileSync(0, 'utf8'); + if (stdin.trim()) { + const payload = JSON.parse(stdin) as { cwd?: string }; + if (payload.cwd && fs.existsSync(payload.cwd)) cwd = payload.cwd; + } + } catch { + // Bad/absent stdin: fall through with process.cwd() — repair is still valid. + } + + // Resolve the project slug the same way the preamble did (GSTACK_PROJECT_SLUG + // override, project-root walk, remote-derived slug). + let slug = ''; + try { + const r = runBin('gstack-slug', [], { cwd, encoding: 'utf8', timeout: DEADLINE_MS }); + const m = (r.stdout ?? '').toString().match(/^SLUG=([A-Za-z0-9._-]+)$/m); + if (m) slug = m[1]; + } catch { + // fall through + } + if (!slug) { + logHookError('could not resolve project slug — nothing repaired'); + return; + } + + const timelinePath = path.join(stateRoot(), 'projects', slug, 'timeline.jsonl'); + let stat: fs.Stats; + try { + stat = fs.statSync(timelinePath); + } catch { + return; // no timeline — nothing to repair + } + if (stat.size === 0) return; + if (stat.size > MAX_TIMELINE_BYTES) { + logHookError(`timeline over size cap (${stat.size} bytes) — skipped (fail-open)`); + return; + } + + let raw: string; + try { + raw = fs.readFileSync(timelinePath, 'utf8'); + } catch (err) { + logHookError(`could not read timeline: ${err instanceof Error ? err.message : String(err)}`); + return; + } + + // Corrupt LINES are skipped individually; a fully corrupt file repairs nothing. + const started = new Map(); + const completed = new Set(); + for (const line of raw.split('\n')) { + if (!line.trim()) continue; + let entry: TimelineEntry; + try { + entry = JSON.parse(line) as TimelineEntry; + } catch { + continue; + } + if (!entry || typeof entry.skill !== 'string') continue; + const key = `${entry.skill}\u0000${entry.session ?? ''}`; + if (entry.event === 'started' && !started.has(key)) started.set(key, entry); + if (entry.event === 'completed') completed.add(key); + } + + const dangling = [...started.entries()].filter(([key]) => !completed.has(key)); + if (dangling.length === 0) return; + + if (Date.now() - startedAt > DEADLINE_MS) { + logHookError('internal 2s budget exhausted before write — skipped (fail-open)'); + return; + } + + const now = new Date().toISOString(); + const lines = dangling + .map(([, entry]) => + JSON.stringify({ + skill: entry.skill, + event: 'completed', + ...(entry.branch ? { branch: entry.branch } : {}), + outcome: 'unknown', + source: 'stop-hook', + ...(entry.session ? { session: entry.session } : {}), + ts: now, + }), + ) + .join('\n'); + try { + fs.appendFileSync(timelinePath, lines + '\n'); + } catch (err) { + logHookError(`could not append completions: ${err instanceof Error ? err.message : String(err)}`); + } +} + +try { + main(); +} catch (err) { + logHookError(`unexpected: ${err instanceof Error ? (err.stack ?? err.message) : String(err)}`); +} +process.exit(0); diff --git a/setup b/setup index ad4075848..638196550 100755 --- a/setup +++ b/setup @@ -1997,9 +1997,31 @@ if [ "$NO_TEAM_MODE" -ne 1 ] \ fi fi -# Also tear down plan-tune hooks on --no-team (matches the existing pattern). +# ─── Timeline Stop hook (#2553) ────────────────────────────────────────────── +# The preamble writes event:"started" to the project timeline at every skill +# start; the completion write lives in end-of-workflow prose and is +# unenforceable — interrupted sessions leaked started > completed forever. +# Register a Stop-event hook that closes dangling entries. FAIL-OPEN contract +# (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 + if ! "$SETTINGS_HOOK" list-sources 2>/dev/null | grep -q "gstack-timeline-stop"; then + if "$SETTINGS_HOOK" add-event \ + --event Stop \ + --command "$TIMELINE_STOP_HOOK" \ + --source gstack-timeline-stop \ + --timeout 5 >/dev/null 2>&1; then + log " registered Stop hook: session timeline entries now close even when a skill is interrupted (backup: settings.json.bak.; remove: $SETTINGS_HOOK remove-source --source gstack-timeline-stop)" + fi + fi +fi + +# Also tear down plan-tune + timeline hooks on --no-team (matches the existing pattern). 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 fi # ─── Redact pre-push guard consent (#1946) ─────────────────────────────────── diff --git a/test/timeline-stop-hook.test.ts b/test/timeline-stop-hook.test.ts new file mode 100644 index 000000000..f2d0b3d5b --- /dev/null +++ b/test/timeline-stop-hook.test.ts @@ -0,0 +1,197 @@ +/** + * Timeline Stop hook (#2553) — fail-open contract (F5). + * + * The preamble writes event:"started" at every skill start; the completion + * write is end-of-workflow prose and unenforceable, so interrupted sessions + * leaked started > completed forever. The Stop hook closes dangling entries. + * + * Contract under test: ALWAYS exits 0 (corrupt timeline, missing timeline, + * garbage stdin), append-only, and the normal path appends event:"completed" + * with outcome "unknown" + source "stop-hook" for every un-closed "started". + */ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const HOOK = path.join(ROOT, 'hosts', 'claude', 'hooks', 'timeline-stop-hook'); + +const SLUG = 'stop-hook-test-project'; + +let tmpHome: string; +let projectDir: string; +let timelinePath: string; + +beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-stop-hook-home-')); + projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-stop-hook-proj-')); + fs.mkdirSync(path.join(tmpHome, 'projects', SLUG), { recursive: true }); + timelinePath = path.join(tmpHome, 'projects', SLUG, 'timeline.jsonl'); +}); + +afterEach(() => { + fs.rmSync(tmpHome, { recursive: true, force: true }); + fs.rmSync(projectDir, { recursive: true, force: true }); +}); + +function runHook(stdin: string): { exitCode: number; stdout: string; stderr: string } { + const r = spawnSync('bash', [HOOK], { + input: stdin, + encoding: 'utf-8', + env: { + ...process.env, + GSTACK_HOME: tmpHome, + GSTACK_PROJECT_SLUG: SLUG, // deterministic slug, no git required + }, + timeout: 15_000, + }); + return { exitCode: r.status ?? 1, stdout: r.stdout, stderr: r.stderr }; +} + +function stopPayload(): string { + return JSON.stringify({ + session_id: 'sess-abc', + hook_event_name: 'Stop', + cwd: projectDir, + }); +} + +function timelineEntries(): any[] { + if (!fs.existsSync(timelinePath)) return []; + return fs + .readFileSync(timelinePath, 'utf-8') + .split('\n') + .filter((l) => l.trim()) + .map((l) => { + try { + return JSON.parse(l); + } catch { + return { __corrupt: l }; + } + }); +} + +describe('timeline-stop-hook (#2553, F5 fail-open)', () => { + test('normal path: closes dangling started entries, leaves closed pairs alone', () => { + fs.writeFileSync( + timelinePath, + [ + JSON.stringify({ skill: 'review', event: 'started', branch: 'main', session: '11-1' }), + JSON.stringify({ skill: 'ship', event: 'started', session: '22-2' }), + JSON.stringify({ skill: 'ship', event: 'completed', session: '22-2', outcome: 'success' }), + ].join('\n') + '\n', + ); + + const r = runHook(stopPayload()); + expect(r.exitCode).toBe(0); + + const entries = timelineEntries(); + // Append-only: the three originals survive verbatim in order. + expect(entries[0]).toMatchObject({ skill: 'review', event: 'started' }); + expect(entries[2]).toMatchObject({ skill: 'ship', event: 'completed', outcome: 'success' }); + + const repairs = entries.filter((e) => e.source === 'stop-hook'); + expect(repairs).toHaveLength(1); + expect(repairs[0]).toMatchObject({ + skill: 'review', + event: 'completed', + outcome: 'unknown', + branch: 'main', + session: '11-1', + }); + expect(typeof repairs[0].ts).toBe('string'); + }); + + test('idempotent: a second Stop appends nothing new', () => { + fs.writeFileSync( + timelinePath, + JSON.stringify({ skill: 'qa', event: 'started', session: '33-3' }) + '\n', + ); + expect(runHook(stopPayload()).exitCode).toBe(0); + const afterFirst = timelineEntries().length; + expect(runHook(stopPayload()).exitCode).toBe(0); + expect(timelineEntries().length).toBe(afterFirst); + }); + + test('exit 0 on missing timeline (nothing written, nothing created)', () => { + const r = runHook(stopPayload()); + expect(r.exitCode).toBe(0); + expect(fs.existsSync(timelinePath)).toBe(false); + }); + + test('exit 0 on a corrupt timeline; corrupt lines are skipped, valid ones still repaired', () => { + fs.writeFileSync( + timelinePath, + [ + 'this is not json at all {{{', + JSON.stringify({ skill: 'qa', event: 'started', session: '44-4' }), + '{"half": "an object"', + ].join('\n') + '\n', + ); + const r = runHook(stopPayload()); + expect(r.exitCode).toBe(0); + const repairs = timelineEntries().filter((e) => e.source === 'stop-hook'); + expect(repairs).toHaveLength(1); + expect(repairs[0].skill).toBe('qa'); + }); + + test('exit 0 on a FULLY corrupt timeline (no valid entries → no write)', () => { + const garbage = 'garbage\n{{{\n'; + fs.writeFileSync(timelinePath, garbage); + const r = runHook(stopPayload()); + expect(r.exitCode).toBe(0); + expect(fs.readFileSync(timelinePath, 'utf-8')).toBe(garbage); + }); + + test('exit 0 on garbage stdin', () => { + fs.writeFileSync( + timelinePath, + JSON.stringify({ skill: 'qa', event: 'started', session: '55-5' }) + '\n', + ); + const r = runHook('not json'); + expect(r.exitCode).toBe(0); + }); + + test('exit 0 on empty stdin', () => { + expect(runHook('').exitCode).toBe(0); + }); + + test('oversized timeline is skipped, untouched, and still exits 0 (fail-open size cap)', () => { + const line = JSON.stringify({ skill: 'qa', event: 'started', session: '66-6' }) + '\n'; + const filler = '#'.repeat(1024 * 1024); + fs.writeFileSync(timelinePath, line + filler.repeat(11)); + const sizeBefore = fs.statSync(timelinePath).size; + const r = runHook(stopPayload()); + expect(r.exitCode).toBe(0); + expect(fs.statSync(timelinePath).size).toBe(sizeBefore); + }); +}); + +describe('timeline-stop-hook wiring', () => { + test('setup registers the Stop hook with its own source tag and tears it down on --no-team', () => { + const setup = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + expect(setup).toContain('--event Stop'); + expect(setup).toContain('--source gstack-timeline-stop'); + expect(setup).toContain('hosts/claude/hooks/timeline-stop-hook'); + // --no-team teardown removes it alongside the plan-tune hooks. + const teardown = setup.slice(setup.indexOf('# Also tear down plan-tune')); + expect(teardown).toContain('remove-source --source gstack-timeline-stop'); + }); + + test('gstack-uninstall removes the Stop hook registration', () => { + const uninstall = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-uninstall'), 'utf-8'); + expect(uninstall).toContain('remove-source --source gstack-timeline-stop'); + }); + + test('the bash shim is fail-open: exits 0 even when bun is unavailable', () => { + const r = spawnSync('bash', [HOOK], { + input: '{}', + encoding: 'utf-8', + env: { HOME: tmpHome, PATH: '/usr/bin:/bin', GSTACK_HOME: tmpHome }, + timeout: 15_000, + }); + expect(r.status).toBe(0); + }); +});