mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-04 07:47:27 +01:00
feat(hooks): an early exit (STATUS: BLOCKED, Nothing to review) meets the contract (GST-1088 lane A)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
c19a685950
commit
e49c9bee65
4 files changed
+33
-7
No files matched your search
@@ -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.
|
||||
|
||||
+12
-6
@@ -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<string, ToolResult>();
|
||||
for (const e of run) if (e.kind === 'tool_result') results.set(e.id, e);
|
||||
const waived = load ? waivers(run, skill, results) : new Map<string, string>();
|
||||
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));
|
||||
|
||||
@@ -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('');
|
||||
|
||||
@@ -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']);
|
||||
|
||||
Reference in new issue
Block a user