diff --git a/docs/skill-routing-and-contracts.md b/docs/skill-routing-and-contracts.md index 1aa1d67af..ecbb65c93 100644 --- a/docs/skill-routing-and-contracts.md +++ b/docs/skill-routing-and-contracts.md @@ -77,6 +77,8 @@ 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. + 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. Items are checked against one run of the skill: from its last load to the end of the session, or to its next load when `report` re-checks an earlier run. A load is a Skill tool call, a typed `/name` command, or a Read of its `SKILL.md` under a `skills/` directory (reading the source in a gstack checkout is not a load). A run recorded `met`, `met_waived` or `met_after_block` stays that way, so a later unrelated turn in the same session is never blocked by it. diff --git a/lib/skill-contract.ts b/lib/skill-contract.ts index 9963a6756..fb58ca418 100644 --- a/lib/skill-contract.ts +++ b/lib/skill-contract.ts @@ -91,6 +91,8 @@ 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. */ + earlyExit: 'Nothing to review', } as const; /** @@ -429,17 +431,21 @@ export function checkContract(t: Transcript, skill: string, contract: Contract, const results = new Map(); for (const e of run) if (e.kind === 'tool_result') results.set(e.id, e); const waived = load ? waivers(run, skill, results) : new Map(); - const items: ItemResult[] = load ? contract.items.map(item => { - const r = checkItem(item, run); - if (r.met) return { id: item.id, kind: item.kind, status: 'met', detail: r.detail, fix: '' }; - const reason = waived.get(item.id); - return reason ? { id: item.id, kind: item.kind, status: 'waived', detail: r.detail, fix: r.fix, reason } : { id: item.id, kind: item.kind, status: 'missing', detail: r.detail, fix: r.fix }; - }) : []; const lastPrompt = events.map(e => e.kind).lastIndexOf('prompt'); const turn = events.slice(lastPrompt + 1).filter(e => e.kind === 'text' || e.kind === 'tool_use'); 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 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); + if (r.met) return { id: item.id, kind: item.kind, status: 'met', detail: r.detail, fix: '' }; + const reason = waived.get(item.id); + return reason ? { id: item.id, kind: item.kind, status: 'waived', detail: r.detail, fix: r.fix, reason } : { id: item.id, kind: item.kind, status: 'missing', detail: r.detail, fix: r.fix }; + }) : []; const lastParagraph = (finalText?.text ?? '').trim().split(/\n\s*\n/).pop() ?? ''; const pendingQuestion = statusLine === 'NEEDS_CONTEXT' || (final !== undefined && isToolUse(final) && /AskUserQuestion$/.test(final.name)) || (final !== undefined && final.kind === 'text' && /\?[\s*_)"']*$/.test(lastParagraph)); diff --git a/test/skill-contract-stop-hook.test.ts b/test/skill-contract-stop-hook.test.ts index 80393d79b..065fd0cec 100644 --- a/test/skill-contract-stop-hook.test.ts +++ b/test/skill-contract-stop-hook.test.ts @@ -96,6 +96,12 @@ describe('block once, with the pinned text', () => { expect((Object.values(record().runs)[0] as any).outcome).toBe('met'); }); + test('an /investigate run that ends with Status: BLOCKED in its DEBUG REPORT is met, not blocked', () => { + const file = transcript([...J1_TRANSCRIPT.slice(0, -1), say(DEBUG_REPORT.replace('Status: DONE', 'Status: BLOCKED'))]); + expect(stop(file)).toEqual({ code: 0, stdout: '', stderr: '' }); + expect((Object.values(record().runs)[0] as any).outcome).toBe('met'); + }); + test('a met run stays met: a later unrelated edit turn in the same session is not blocked', () => { const file = transcript([...J1_TRANSCRIPT.slice(0, -1), tool('b9', 'Bash', { command: 'bun test' }), result('b9', 'pass'), say(DEBUG_REPORT)]); expect(stop(file).stdout).toBe(''); diff --git a/test/skill-contract.test.ts b/test/skill-contract.test.ts index 0ccdad147..d27377ebe 100644 --- a/test/skill-contract.test.ts +++ b/test/skill-contract.test.ts @@ -114,7 +114,7 @@ describe('checkContract is a pure function of the transcript', () => { expect(check([slash('review'), sm, ...red, report], 'review').items[0].detail).toBe('selected State Machines; no Agent dispatch for State Machines'); expect(check([slash('review'), sm, ...agent('s', 'State Machines specialist'), ...red, report], 'review').met).toBe(true); expect(check([slash('review'), say('Dispatching 2\nspecialists: Testing, Security'), ...red, report], 'review').items[0].detail).toBe('selected Testing, Security; no Agent dispatch for Testing, Security'); - expect(DEFAULT_MATCHERS).toEqual({ specialistSelection: 'Dispatching N specialists:', specialistsSkipped: 'specialists skipped' }); + expect(DEFAULT_MATCHERS).toEqual({ specialistSelection: 'Dispatching N specialists:', specialistsSkipped: 'specialists skipped', earlyExit: 'Nothing to review' }); expect(check([slash('review'), ...red, report], 'review').items[0].detail).toBe('no specialist selection line found'); const fixedLine = say('Dispatching adversarial pass: native'); expect(check([slash('review'), fixedLine, ...agent('d', 'Adversarial pass', 'Find the ways this diff fails.')], 'review').items[1]).toMatchObject({ status: 'met', detail: 'printed "Dispatching adversarial pass: native", then an Agent dispatch' }); @@ -151,6 +151,18 @@ describe('checkContract is a pure function of the transcript', () => { expect(r.missing).toEqual(['debug-report', 'repro-after-fix']); }); + test('an early exit counts as met: /review "Nothing to review" or STATUS: BLOCKED, /investigate Status: BLOCKED in its DEBUG REPORT', () => { + const early = (lines: string[], skill: string) => check(lines, skill); + const nothing = early([slash('review'), say('Nothing to review — no changes against the base branch.')], 'review'); + expect(nothing).toMatchObject({ met: true, follow: true, missing: [] }); + expect(nothing.items[0].detail).toBe('the skill ended the run early (Nothing to review)'); + expect(early([slash('review'), say('Base branch could not be found.\nSTATUS: BLOCKED')], 'review').met).toBe(true); + const blockedReport = DEBUG_REPORT.replace('Status: DONE', 'Status: BLOCKED'); + 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); + }); + test('a Read of a SKILL.md counts as a load only under a skills/ directory, not in a gstack checkout', () => { const loads = (file_path: string) => skillLoads(parseTranscript(tool('r', 'Read', { file_path }))).map(l => l.skill); expect(loads('/home/u/.claude/skills/gstack/investigate/SKILL.md')).toEqual(['investigate']);