From ea9c5f7ee26c5ec0c1abe75627f5f4ca770e163c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 17 Mar 2026 11:11:34 -0400 Subject: [PATCH] fix: headless mode exits early on progress notifications containing 'complete' (#879) (#888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isTerminalNotification() used broad substring matching against ['complete', 'stopped', 'blocked']. Any notification containing these words triggered early exit — including progress messages like: 'All slices are complete — nothing to discuss.' 'Override(s) resolved — rewrite-docs completed.' 'Skipped 5+ completed units. Yielding to UI before continuing.' Fix: Replace substring matching with prefix matching against the actual stop signals emitted by stopAuto(): 'Auto-mode stopped...' 'Step-mode stopped...' These are the ONLY notifications that indicate auto-mode has genuinely terminated. All other notifications (slice completion, override resolution, skip yielding) are progress events and must not trigger exit. Also tighten isBlockedNotification to match 'blocked:' (with colon) instead of bare 'blocked' to avoid false positives from unrelated messages. Added 15 regression tests covering: - All real terminal notification variants - 6 false-positive cases from the issue report - Non-notify event rejection - Blocked detection with and without colon --- src/headless.ts | 21 +++++- src/tests/headless-detection.test.ts | 101 +++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 src/tests/headless-detection.test.ts diff --git a/src/headless.ts b/src/headless.ts index 1130d303d..78e6934bf 100644 --- a/src/headless.ts +++ b/src/headless.ts @@ -188,7 +188,20 @@ function formatProgress(event: Record, verbose: boolean): strin // Completion Detection // --------------------------------------------------------------------------- -const TERMINAL_KEYWORDS = ['complete', 'stopped', 'blocked'] +/** + * Detect genuine auto-mode termination notifications. + * + * Only matches the actual stop signals emitted by stopAuto(): + * "Auto-mode stopped..." + * "Step-mode stopped..." + * + * Does NOT match progress notifications that happen to contain words like + * "complete" or "stopped" (e.g., "Override resolved — rewrite-docs completed", + * "All slices are complete — nothing to discuss", "Skipped 5+ completed units"). + * + * Blocked detection is separate — checked via isBlockedNotification. + */ +const TERMINAL_PREFIXES = ['auto-mode stopped', 'step-mode stopped'] const IDLE_TIMEOUT_MS = 15_000 // new-milestone is a long-running creative task where the LLM may pause // between tool calls (e.g. after mkdir, before writing files). Use a @@ -198,12 +211,14 @@ const NEW_MILESTONE_IDLE_TIMEOUT_MS = 120_000 function isTerminalNotification(event: Record): boolean { if (event.type !== 'extension_ui_request' || event.method !== 'notify') return false const message = String(event.message ?? '').toLowerCase() - return TERMINAL_KEYWORDS.some((kw) => message.includes(kw)) + return TERMINAL_PREFIXES.some((prefix) => message.startsWith(prefix)) } function isBlockedNotification(event: Record): boolean { if (event.type !== 'extension_ui_request' || event.method !== 'notify') return false - return String(event.message ?? '').toLowerCase().includes('blocked') + const message = String(event.message ?? '').toLowerCase() + // Blocked notifications come through stopAuto as "Auto-mode stopped (Blocked: ...)" + return message.includes('blocked:') } function isMilestoneReadyNotification(event: Record): boolean { diff --git a/src/tests/headless-detection.test.ts b/src/tests/headless-detection.test.ts new file mode 100644 index 000000000..71bb82bac --- /dev/null +++ b/src/tests/headless-detection.test.ts @@ -0,0 +1,101 @@ +/** + * Tests for headless completion detection. + * + * Verifies that isTerminalNotification only matches actual auto-mode stop + * signals and does not false-positive on progress notifications that + * happen to contain words like "complete" or "stopped". + */ + +import test from "node:test"; +import assert from "node:assert/strict"; + +// Import the module to get access to the functions via a dynamic import +// since headless.ts has side-effect-free detection functions but no exports. +// We'll test by extracting the logic inline. + +// ─── Extracted detection logic (mirrors headless.ts) ──────────────────────── + +const TERMINAL_PREFIXES = ['auto-mode stopped', 'step-mode stopped'] + +function isTerminalNotification(event: Record): boolean { + if (event.type !== 'extension_ui_request' || event.method !== 'notify') return false + const message = String(event.message ?? '').toLowerCase() + return TERMINAL_PREFIXES.some((prefix) => message.startsWith(prefix)) +} + +function isBlockedNotification(event: Record): boolean { + if (event.type !== 'extension_ui_request' || event.method !== 'notify') return false + const message = String(event.message ?? '').toLowerCase() + return message.includes('blocked:') +} + +function makeNotify(message: string): Record { + return { type: 'extension_ui_request', method: 'notify', message } +} + +// ─── isTerminalNotification ───────────────────────────────────────────────── + +test("detects 'Auto-mode stopped.' as terminal", () => { + assert.ok(isTerminalNotification(makeNotify("Auto-mode stopped."))) +}) + +test("detects 'Auto-mode stopped (All milestones complete).' as terminal", () => { + assert.ok(isTerminalNotification(makeNotify("Auto-mode stopped (All milestones complete). Session: $0.42 · 15K tokens · 8 units"))) +}) + +test("detects 'Auto-mode stopped (Blocked: missing API key).' as terminal", () => { + assert.ok(isTerminalNotification(makeNotify("Auto-mode stopped (Blocked: missing API key)."))) +}) + +test("detects 'Auto-mode stopped (Milestone M001 complete).' as terminal", () => { + assert.ok(isTerminalNotification(makeNotify("Auto-mode stopped (Milestone M001 complete)."))) +}) + +test("detects 'Step-mode stopped.' as terminal", () => { + assert.ok(isTerminalNotification(makeNotify("Step-mode stopped."))) +}) + +// ─── False positives that previously triggered early exit (#879) ──────────── + +test("does NOT match 'All slices are complete — nothing to discuss.'", () => { + assert.ok(!isTerminalNotification(makeNotify("All slices are complete — nothing to discuss."))) +}) + +test("does NOT match 'Override(s) resolved — rewrite-docs completed.'", () => { + assert.ok(!isTerminalNotification(makeNotify("Override(s) resolved — rewrite-docs completed."))) +}) + +test("does NOT match 'Skipped 5+ completed units. Yielding to UI before continuing.'", () => { + assert.ok(!isTerminalNotification(makeNotify("Skipped 5+ completed units. Yielding to UI before continuing."))) +}) + +test("does NOT match 'Cannot dispatch reassess-roadmap: no completed slices.'", () => { + assert.ok(!isTerminalNotification(makeNotify("Cannot dispatch reassess-roadmap: no completed slices."))) +}) + +test("does NOT match 'Committed: feat(S03): complete task implementation'", () => { + assert.ok(!isTerminalNotification(makeNotify("Committed: feat(S03): complete task implementation"))) +}) + +test("does NOT match 'Post-hook: applied 3 fix(es).'", () => { + assert.ok(!isTerminalNotification(makeNotify("Post-hook: applied 3 fix(es)."))) +}) + +test("does NOT match non-notify events", () => { + assert.ok(!isTerminalNotification({ type: 'agent_end' })) + assert.ok(!isTerminalNotification({ type: 'extension_ui_request', method: 'select', message: 'Auto-mode stopped.' })) +}) + +// ─── isBlockedNotification ────────────────────────────────────────────────── + +test("detects blocked notification with 'Blocked:' prefix", () => { + assert.ok(isBlockedNotification(makeNotify("Auto-mode stopped (Blocked: missing API key)."))) +}) + +test("detects inline 'Blocked:' message", () => { + assert.ok(isBlockedNotification(makeNotify("Blocked: no active milestone. Fix and run /gsd auto."))) +}) + +test("does NOT match 'blocked' without colon (avoids false positives)", () => { + assert.ok(!isBlockedNotification(makeNotify("The request was blocked by the firewall"))) +})