fix(hooks): review round 2 notes: check commands behind wrappers count, the shell test builtin does not; met_after_block is final (GST-1088 lane A)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

docs/skill-routing-and-contracts.md

@@ -73,11 +73,11 @@ Every item has an `id` (lowercase letters, digits, `.`, `_`, `-`) and a `kind`,
 | kind | Fields | Met when |
 |---|---|---|
 | `report_labels` | `heading`, `labels[]` | The last assistant message containing the heading has a `Label: value` line for every label, and no value is empty or a placeholder (`[...]`, `<...>`, TBD, TODO, `...`, or a template like `DONE \| BLOCKED`) |
-| `check_after_last_edit` | optional `match` (regular expression) | No Edit, Write, MultiEdit or NotebookEdit happened, or a Bash test or repro command ran after the last one. By default a command counts when, with quoted text removed, one of its parts (split on `&&`, `\|\|`, `;`, `\|`) starts with a test runner, build tool or interpreter (bun, npm, pytest, go, cargo, make, node, python, curl, ...) or a script whose name says test, spec, repro, check, verify or lint. `git commit --no-verify` and `cat test/a.test.ts` do not count |
+| `check_after_last_edit` | optional `match` (regular expression) | No Edit, Write, MultiEdit or NotebookEdit happened, or a Bash test or repro command ran after the last one. By default a command counts when, with quoted text removed, one of its parts (split on `&&`, `\|\|`, `;`, `\|`) starts with a test runner, build tool or interpreter (bun, npm, pytest, go, cargo, make, node, python, curl, ...) or a script whose name says test, spec, repro, check, verify or lint, directly or behind a wrapper (`bundle exec rspec`, `uv run pytest`, `docker compose run app pytest`, `xvfb-run npm test`, `bash scripts/test.sh`). `git commit --no-verify`, `cat test/a.test.ts` and the shell's `test -f` do not count |
 | `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:`. A "specialists skipped" line means zero specialists were selected, which passes |
 | `agent_dispatch` | `match` (regular expression, case-insensitive) | An Agent dispatch's description or prompt matches |
 
-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` stays met, so a later unrelated turn in the same session is never blocked by it.
+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.
 
 ### Worked example
 

lib/skill-contract.ts

@@ -253,16 +253,23 @@ export function skillLoads(t: Transcript): SkillLoad[] {
 export const STATUS_RE = /^[ \t>*_-]*(?:\*\*|__)?(?:STATUS|Status)(?:\*\*|__)?[ \t]*(?:\*\*|__)?:[ \t]*(?:\*\*|__)?[ \t]*(DONE_WITH_CONCERNS|DONE|BLOCKED|NEEDS_CONTEXT)\b/m;
 const EDIT_TOOLS = /^(?:Edit|Write|MultiEdit|NotebookEdit)$/;
 const AGENT_TOOLS = /^(?:Agent|Task)$/;
-const CHECK_PROGRAM = /^(?:pytest|jest|vitest|mocha|rspec|phpunit|ctest|tox|nox|tsc|make|cargo|go|bun|bunx|npm|npx|pnpm|yarn|node|deno|python3?|ruby|rake|mvn|gradlew?|dotnet|swift|xcodebuild|curl)$|test|spec|repro|check|verify|lint/i;
+const CHECK_RUNNER = /^(?:pytest|jest|vitest|mocha|rspec|phpunit|ctest|tox|nox|tsc|make|cargo|go|bun|bunx|npm|npx|pnpm|yarn|node|deno|python3?|ruby|rake|mvn|gradlew?|dotnet|swift|xcodebuild|curl|mix|bazel|just|dart|flutter|rails)$/i;
+const CHECK_SCRIPT = /(?:^|[-_.])(?:tests?|spec|repro|check|verify|lint)(?:[-_.]|$)/i;
+const COMMAND_WRAPPER = /^(?:bundle|uv|poetry|pipenv|docker|podman|xvfb-run|bash|sh|zsh)$/;
 const COMMAND_PREFIX = /^(?:env|sudo|time|nice|timeout|command|exec|\w+=\S*|-\S*|\d+[smh]?)$/;
+const isCheckProgram = (word: string) => { const name = word.split(/[\\/]/).pop() ?? ''; return CHECK_RUNNER.test(name) || (name !== 'test' && CHECK_SCRIPT.test(name)); };
 const SAFE_SPECIALIST = /^[A-Za-z0-9][A-Za-z0-9 _.-]{0,39}$/;
 
-/** A test or repro run: some segment of the command (split on && || ; | and newlines) starts with a test runner or a test, check or repro script. */
+/**
+ * A test or repro run: some segment of the command (split on && || ; | and newlines) starts with a test runner or a
+ * test, check or repro script, directly or behind a wrapper (bundle exec, uv run, docker compose run, xvfb-run, bash).
+ */
 export function isCheckCommand(cmd: string): boolean {
   return cmd.split(/&&|\|\||[;|\n]/).some(segment => {
     const words = segment.trim().replace(/^[({]\s*/, '').split(/\s+/);
     while (words.length > 1 && COMMAND_PREFIX.test(words[0])) words.shift();
-    return CHECK_PROGRAM.test(words[0].split(/[\\/]/).pop() ?? '');
+    if (isCheckProgram(words[0])) return true;
+    return COMMAND_WRAPPER.test(words[0].split(/[\\/]/).pop() ?? '') && words.slice(1, 6).some(isCheckProgram);
   });
 }
 const DEFAULT_SELECTION_RE = /Dispatching\s+\d+\s+specialists?:\s*(?<names>[^\n]*)/i;
@@ -507,10 +514,10 @@ export function writeRecord(file: string, rec: SessionRecord): void {
 
 /**
  * Block at most once per skill run: an unmet contract blocks unless this run was already blocked, which makes it a miss.
- * A run recorded met (never blocked) is final: later unrelated turns in the session never reopen it.
+ * A run recorded met, met_waived or met_after_block is final: later unrelated turns in the session never reopen it.
  */
 export function nextOutcome(prev: RunRecord | undefined, result: CheckResult): { outcome: Outcome; block: boolean } {
-  if (prev && !prev.blocked_at && (prev.outcome === 'met' || prev.outcome === 'met_waived')) return { outcome: prev.outcome, block: false };
+  if (prev && (prev.outcome === 'met' || prev.outcome === 'met_waived' || prev.outcome === 'met_after_block')) return { outcome: prev.outcome, block: false };
   if (result.met) return { outcome: prev?.blocked_at ? 'met_after_block' : result.waived.length ? 'met_waived' : 'met', block: false };
   return prev?.blocked_at ? { outcome: 'miss', block: false } : { outcome: 'blocked', block: true };
 }

test/skill-contract-stop-hook.test.ts

@@ -77,6 +77,9 @@ describe('block once, with the pinned text', () => {
     fs.appendFileSync(file, [tool('b9', 'Bash', { command: 'bun test test/auth.test.ts' }), result('b9', 'pass'), say(DEBUG_REPORT)].join('\n') + '\n');
     expect(stop(file, {}, { stop_hook_active: true }).stdout).toBe('');
     expect((Object.values(record().runs)[0] as any).outcome).toBe('met_after_block');
+    fs.appendFileSync(file, [user('also fix the README typo'), tool('e8', 'Edit', { file_path: 'README.md', old_string: 'teh', new_string: 'the' }), result('e8', 'updated'), say('Fixed.')].join('\n') + '\n');
+    expect(stop(file)).toEqual({ code: 0, stdout: '', stderr: '' });
+    expect((Object.values(record().runs)[0] as any).outcome).toBe('met_after_block');
   });
 
   test('still unmet on the stop_hook_active Stop after our block: a miss, never a second block', () => {

test/skill-contract.test.ts

@@ -149,9 +149,10 @@ describe('checkContract is a pure function of the transcript', () => {
   });
 
   test('check_after_last_edit: a command counts by the program it runs, not by a word inside it', () => {
-    for (const cmd of ['bun test', 'cd repo && npm test', 'FOO=1 timeout 60 pytest -x', './scripts/run-tests.sh', 'node repro.js | tail -5', 'bin/gstack-verify'])
+    for (const cmd of ['bun test', 'cd repo && npm test', 'FOO=1 timeout 60 pytest -x', './scripts/run-tests.sh', 'node repro.js | tail -5', 'bin/gstack-verify',
+      'bundle exec rspec', 'uv run pytest', 'poetry run pytest', 'docker compose run app pytest', 'xvfb-run -a npm test', 'bash scripts/test.sh', 'mix test', 'bazel test //...'])
       expect([cmd, isCheckCommand(cmd)]).toEqual([cmd, true]);
-    for (const cmd of ['git commit -m wip --no-verify', 'git add test/a.test.ts', 'cat test/a.test.ts', 'git diff --check', 'ls go'])
+    for (const cmd of ['git commit -m wip --no-verify', 'git add test/a.test.ts', 'cat test/a.test.ts', 'git diff --check', 'ls go', 'test -f README.md', './scripts/fetch-latest.sh', 'bash scripts/deploy.sh'])
       expect([cmd, isCheckCommand(cmd)]).toEqual([cmd, false]);
   });