From 5c7d678bb98cc8bf7c0996045b2b5ebc4e1b1419 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 03:33:02 +0000 Subject: [PATCH] fix(hooks): early exit only from the final message, line-anchored (GST-1088 lane A) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/skill-routing-and-contracts.md | 2 +- lib/skill-contract.ts | 8 ++++---- test/skill-contract.test.ts | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/skill-routing-and-contracts.md b/docs/skill-routing-and-contracts.md index ecbb65c93..f0eb59b84 100644 --- a/docs/skill-routing-and-contracts.md +++ b/docs/skill-routing-and-contracts.md @@ -77,7 +77,7 @@ Every item has an `id` (lowercase letters, digits, `.`, `_`, `-`) and a `kind`, | `selected_specialists_dispatched` | optional `marker`, `empty_marker` | The selection line was printed in an assistant message or a Bash result (never a fetched page or file), its names are plain (letters, digits, spaces, `_`, `.`, `-`, at most 40 characters), and each specialist it names has an Agent dispatch that mentions it. The marker is literal text in which a whole-word N stands for a number; the default is `Dispatching N specialists:`. N = 0 (`Dispatching 0 specialists: (small diff)`) or a "specialists skipped" line means zero specialists were selected, which passes. Text in parentheses is not a name (`Dispatching 1 specialists: State Machines (small diff)` selects State Machines) | | `agent_dispatch` | `match` (regular expression, case-insensitive), optional `marker` | The `marker` line (literal text, like /review's `Dispatching adversarial pass: native`) was printed and an Agent dispatch follows it; otherwise, as the fallback, an Agent dispatch's description or prompt matches `match` | -A run the skill ended early counts as met: a final `STATUS: BLOCKED` (for /investigate, `Status: BLOCKED` in its DEBUG REPORT) or a "Nothing to review" line from /review. +A run the skill ended early counts as met: a final `STATUS: BLOCKED` (for /investigate, `Status: BLOCKED` in its DEBUG REPORT) or a final message with a line that starts "Nothing to review" (/review's early exit). The checker's default marker strings live in one table, `DEFAULT_MATCHERS` in `lib/skill-contract.ts`; a skill's own `marker`, `empty_marker` or `match` overrides them. diff --git a/lib/skill-contract.ts b/lib/skill-contract.ts index fb58ca418..bd93e34cb 100644 --- a/lib/skill-contract.ts +++ b/lib/skill-contract.ts @@ -91,7 +91,7 @@ export const DEFAULT_MATCHERS = { specialistSelection: 'Dispatching N specialists:', /** selected_specialists_dispatched: a line that means no specialist was selected. */ specialistsSkipped: 'specialists skipped', - /** Any contract: a line that means the skill ended the whole run early, so its contract counts as met. A final STATUS: BLOCKED does too. */ + /** Any contract: a line of the turn's final message, starting with this, means the skill ended the run early (met). A final STATUS: BLOCKED does too. */ earlyExit: 'Nothing to review', } as const; @@ -436,9 +436,9 @@ export function checkContract(t: Transcript, skill: string, contract: Contract, const final = turn[turn.length - 1]; const finalText = [...turn].reverse().find(e => e.kind === 'text') as { text: string } | undefined; const statusLine = finalText ? STATUS_RE.exec(finalText.text)?.[1] ?? null : null; - const exitMarker = markerPattern(DEFAULT_MATCHERS.earlyExit); - const earlyExit = !load ? null : statusLine === 'BLOCKED' ? 'STATUS: BLOCKED' - : run.some(e => e.kind === 'text' && exitMarker.test(e.text)) ? DEFAULT_MATCHERS.earlyExit : null; + const exitMarker = new RegExp(`^[ \\t>*_-]*${markerPattern(DEFAULT_MATCHERS.earlyExit).source}`, 'im'); + const endedEarly = !!finalText && run.includes(finalText as TranscriptEvent) && exitMarker.test(finalText.text); + const earlyExit = !load ? null : statusLine === 'BLOCKED' ? 'STATUS: BLOCKED' : endedEarly ? DEFAULT_MATCHERS.earlyExit : null; const items: ItemResult[] = load ? contract.items.map(item => { if (earlyExit) return { id: item.id, kind: item.kind, status: 'met', detail: `the skill ended the run early (${earlyExit})`, fix: '' }; const r = checkItem(item, run); diff --git a/test/skill-contract.test.ts b/test/skill-contract.test.ts index d27377ebe..79b6d8960 100644 --- a/test/skill-contract.test.ts +++ b/test/skill-contract.test.ts @@ -161,6 +161,8 @@ describe('checkContract is a pure function of the transcript', () => { expect(early([...J1_TRANSCRIPT.slice(0, -1), say(blockedReport)], 'investigate')).toMatchObject({ met: true, statusLine: 'BLOCKED' }); expect(early([...J1_TRANSCRIPT.slice(0, -1), say(DEBUG_REPORT)], 'investigate').missing).toEqual(['repro-after-fix']); expect(early([slash('review'), say('I would print STATUS: BLOCKED if the base were missing, but it is here. Done.')], 'review').met).toBe(false); + expect(early([slash('review'), say('Nothing to review under docs/; checking lib/ next.'), say('Done.')], 'review').met).toBe(false); + expect(early([...J1_TRANSCRIPT.slice(0, -1), say('There is nothing to review in the docs folder. Fixed it.')], 'investigate').met).toBe(false); }); test('a Read of a SKILL.md counts as a load only under a skills/ directory, not in a gstack checkout', () => {