From 7bbb730da5ff8f79af9fdaf8f7583ae9cf2ff082 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 17 Oct 2026 19:15:18 +0000 Subject: [PATCH] fix(review-army): run the state-machine detector in the scope block and check its exit status (W3-5) Specialist 9 named gstack-state-matrix detect only in selection prose, and small diffs keep only State Machines, so agents ran an undeclared command (paid gate run 39002269281). The generated scope block now runs the detector with its exit status checked: on failure it keeps the detector's diagnostics and sets SCOPE_STATE_MACHINE=unknown, and selection dispatches State Machines when the signal is true or unknown. The check step becomes a declared heredoc block. The clarity test requires every non-diff-scope signal to come from a detect helper the scope block runs; a free test runs the block against a failing detector stub; the captured standalone call stays outside the caller interface. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- review/sections/review-army.md | 8 +++++- scripts/resolvers/review-army.ts | 8 +++++- ship/sections/review-army.md | 8 +++++- test/qa-exploratory-callers.test.ts | 14 ++++++++++ test/review-workflow-clarity.test.ts | 41 ++++++++++++++++++++++++++-- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/review/sections/review-army.md b/review/sections/review-army.md index 10f990caa..827029a6f 100644 --- a/review/sections/review-army.md +++ b/review/sections/review-army.md @@ -5,6 +5,7 @@ ### Detect stack and scope ```bash +if _SM=$(~/.claude/skills/gstack/bin/gstack-state-matrix detect --base ); then source <(printf '%s\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) || true set | grep '^SCOPE_' # Detect stack for specialist context @@ -53,7 +54,12 @@ Based on the scope signals above, select which specialists to dispatch. 6. **API Contract** — if SCOPE_API=true. Read `~/.claude/skills/gstack/review/specialists/api-contract.md` 7. **Design** — if SCOPE_FRONTEND=true. Use the existing design review checklist at `~/.claude/skills/gstack/review/design-checklist.md` and run the mechanical pass at the top of that checklist (the user-installed design detector, when present) before the LLM items 8. **Simplification** — if DIFF_LINES > 100. Read `~/.claude/skills/gstack/review/specialists/simplification.md`. Advisory-only lens: hunts unrequested structure (hand-rolled stdlib, one-implementation abstractions, dependencies duplicating platform features), never coverage. -9. **State Machines** — if `~/.claude/skills/gstack/bin/gstack-state-matrix detect --base ` says SCOPE_STATE_MACHINE=true: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then pipe output to `check -`. +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown (printed by the scope block): read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then check its complete output: +```bash +~/.claude/skills/gstack/bin/gstack-state-matrix check - <<'STATE_MACHINES_OUTPUT' + +STATE_MACHINES_OUTPUT +``` ### Adaptive gating diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index 49faaf5d9..18cd35797 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -24,6 +24,7 @@ function generateSpecialistSelection(ctx: TemplateContext): string { ### Detect stack and scope \`\`\`bash +if _SM=$(${ctx.paths.binDir}/gstack-state-matrix detect --base ); then source <(printf '%s\\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi source <(${ctx.paths.binDir}/gstack-diff-scope 2>/dev/null) || true set | grep '^SCOPE_' # Detect stack for specialist context @@ -72,7 +73,12 @@ Based on the scope signals above, select which specialists to dispatch. 6. **API Contract** — if SCOPE_API=true. Read \`${ctx.paths.skillRoot}/review/specialists/api-contract.md\` 7. **Design** — if SCOPE_FRONTEND=true. Use the existing design review checklist at \`${ctx.paths.skillRoot}/review/design-checklist.md\` and run the mechanical pass at the top of that checklist (the user-installed design detector, when present) before the LLM items 8. **Simplification** — if DIFF_LINES > 100. Read \`${ctx.paths.skillRoot}/review/specialists/simplification.md\`. Advisory-only lens: hunts unrequested structure (hand-rolled stdlib, one-implementation abstractions, dependencies duplicating platform features), never coverage. -9. **State Machines** — if \`${ctx.paths.binDir}/gstack-state-matrix detect --base \` says SCOPE_STATE_MACHINE=true: read \`${ctx.paths.skillRoot}/review/specialists/state-machines.md\`, then pipe output to \`check -\`. +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown (printed by the scope block): read \`${ctx.paths.skillRoot}/review/specialists/state-machines.md\`, then check its complete output: +\`\`\`bash +${ctx.paths.binDir}/gstack-state-matrix check - <<'STATE_MACHINES_OUTPUT' + +STATE_MACHINES_OUTPUT +\`\`\` ### Adaptive gating diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 106f92ef9..a767ce649 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -267,6 +267,7 @@ distinct defects stay separate and neither pass substitutes for the other. ### Detect stack and scope ```bash +if _SM=$(~/.claude/skills/gstack/bin/gstack-state-matrix detect --base ); then source <(printf '%s\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) || true set | grep '^SCOPE_' # Detect stack for specialist context @@ -315,7 +316,12 @@ Based on the scope signals above, select which specialists to dispatch. 6. **API Contract** — if SCOPE_API=true. Read `~/.claude/skills/gstack/review/specialists/api-contract.md` 7. **Design** — if SCOPE_FRONTEND=true. Use the existing design review checklist at `~/.claude/skills/gstack/review/design-checklist.md` and run the mechanical pass at the top of that checklist (the user-installed design detector, when present) before the LLM items 8. **Simplification** — if DIFF_LINES > 100. Read `~/.claude/skills/gstack/review/specialists/simplification.md`. Advisory-only lens: hunts unrequested structure (hand-rolled stdlib, one-implementation abstractions, dependencies duplicating platform features), never coverage. -9. **State Machines** — if `~/.claude/skills/gstack/bin/gstack-state-matrix detect --base ` says SCOPE_STATE_MACHINE=true: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then pipe output to `check -`. +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown (printed by the scope block): read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then check its complete output: +```bash +~/.claude/skills/gstack/bin/gstack-state-matrix check - <<'STATE_MACHINES_OUTPUT' + +STATE_MACHINES_OUTPUT +``` ### Adaptive gating diff --git a/test/qa-exploratory-callers.test.ts b/test/qa-exploratory-callers.test.ts index beac16aa7..8dacfcc56 100644 --- a/test/qa-exploratory-callers.test.ts +++ b/test/qa-exploratory-callers.test.ts @@ -196,6 +196,20 @@ describe('caller native-event observer controls', () => { } }); + test('captured 39002269281 standalone state-machine detect stays outside the caller interface; the generated scope block runs it', async () => { + // PR gate run 39002269281 ship-exploratory-plan-checks, native Bash call 17: the selection prose named the detector, so the + // actor ran it alone. The detector now runs inside the generated scope block, which is the declared form. + const fixture = createQaCallerFixture('ship-exploratory-plan-checks', { installRuntime: false }); + try { + const standalone = `${fixture.runtime}/bin/gstack-state-matrix detect --base main`; + expect(qaCallerCommandAllowed(standalone, fixture.workflowCommands)).toBe(false); + const block = fixture.workflowCommands.find(command => command.includes('/bin/gstack-state-matrix detect --base main')); + expect(block).toBeDefined(); + expect(block).toContain('/gstack-diff-scope main 2>/dev/null) || true'); + expect(qaCallerCommandAllowed(block!, fixture.workflowCommands)).toBe(true); + } finally { await fixture.close(); fs.rmSync(fixture.root, { recursive: true, force: true }); } + }); + test.each(['pending-result', 'same-event', 'same-message-id'])('method Read %s cannot authorize a dependent probe', ordering => { const observed = evidence(); const events = observed.result.transcript as any[]; diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index c356d70fa..5206895e5 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -1,5 +1,7 @@ import { expect, test } from 'bun:test'; -import { readFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { generateReviewArmy } from '../scripts/resolvers/review-army'; import { generateCrossReviewDedup, generateSharedCodeReuse, generateScopeDrift } from '../scripts/resolvers/review-scope'; @@ -371,13 +373,46 @@ for (const skillName of ['review', 'ship']) { const flags = between(readFileSync(join(root, 'bin/gstack-diff-scope'), 'utf8'), '_print_flags() {', '}'); for (const signal of used) { if (flags.includes(`echo "${signal}=`)) continue; - // A signal the scope block does not print must come from the helper its own selection line runs. - const helper = /bin\/(gstack-[a-z-]+) detect\b/.exec(selection.split('\n').find(line => line.includes(signal))!)?.[1]; + // PR gate run 39002269281: a signal gstack-diff-scope does not print comes from a detect helper the scope + // block itself runs, never from a command named only in selection prose. + const helper = /bin\/(gstack-[a-z-]+) detect\b/.exec(block.split('\n').find(line => line.includes(signal))!)?.[1]; expect(helper, signal).toBeDefined(); expect(readFileSync(join(root, 'bin', helper!), 'utf8'), signal).toContain(`${signal}=\${`); + expect(selection, signal).not.toMatch(/\bdetect\b/); } }); + test(`${skillName} clarity: the state-machine signal checks the detector's exit status and fails toward dispatch`, () => { + const block = between(army, '### Detect stack and scope', '### Read specialist hit rates'); + expect(block).toContain(`if _SM=$(${HOST_PATHS.claude.binDir}/gstack-state-matrix detect --base ); then source <(printf '%s\\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi\n`); + expect(block.indexOf('SCOPE_STATE_MACHINE=unknown')).toBeLessThan(block.indexOf("set | grep '^SCOPE_'")); + const selection = between(army, '### Select specialists', '### Adaptive gating').replace(/\s+/g, ' '); + expect(selection).toContain('9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown (printed by the scope block)'); + expect(selection).toContain("gstack-state-matrix check - <<'STATE_MACHINES_OUTPUT' STATE_MACHINES_OUTPUT"); + expect(selection).not.toContain('pipe output'); + }); + + test(`${skillName} clarity: the scope block run against a failing detector prints unknown and keeps its diagnostics`, () => { + const stubs = mkdtempSync(join(realpathSync(tmpdir()), 'scope-block-')); + try { + const block = /\`\`\`bash\n([\s\S]*?)\n\`\`\`/.exec(between(generateReviewArmy({ ...ctx, paths: { ...ctx.paths, binDir: stubs } }), + '### Detect stack and scope', '### Read specialist hit rates'))![1].replaceAll('', 'main'); + writeFileSync(join(stubs, 'gstack-diff-scope'), '#!/bin/sh\necho SCOPE_FRONTEND=false\n', { mode: 0o755 }); + const run = (detector: string) => { + writeFileSync(join(stubs, 'gstack-state-matrix'), `#!/bin/sh\n${detector}\n`, { mode: 0o755 }); + return spawnSync('bash', ['-c', block], { cwd: stubs, encoding: 'utf8', timeout: 10_000 }); + }; + const failed = run("echo SCOPE_STATE_MACHINE=false; echo '# no merge base with origin/main; run: git fetch origin main'; echo 'detector crashed' >&2; exit 2"); + expect(failed.stdout).toMatch(/^SCOPE_STATE_MACHINE=unknown$/m); + expect(failed.stdout).not.toMatch(/^SCOPE_STATE_MACHINE=false$/m); + expect(failed.stdout).toContain('# no merge base with origin/main; run: git fetch origin main'); + expect(failed.stderr).toContain('detector crashed'); + const detected = run("echo SCOPE_STATE_MACHINE=true; echo '# state-machine: src/takes.ts:4 proposal,accept'"); + expect(detected.stdout).toMatch(/^SCOPE_STATE_MACHINE=true$/m); + expect(detected.stdout).toMatch(/^SCOPE_FRONTEND=false$/m); + } finally { rmSync(stubs, { recursive: true, force: true }); } + }); + test(`${skillName} clarity: terminal failure permits independent work but never certifies coverage`, () => { expectMentions(flat, [['confirm', 'finished', 'stopped']], 'flat'); expectMentions(flat, [['does not', 'termination', 'timeout']], 'flat');