mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
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>
This commit is contained in:
1 parent
939ace2231
commit
7bbb730da5
5 files changed
+73
-6
No files matched your search
@@ -5,6 +5,7 @@
|
||||
### Detect stack and scope
|
||||
|
||||
```bash
|
||||
if _SM=$(~/.claude/skills/gstack/bin/gstack-state-matrix detect --base <base>); then source <(printf '%s\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi
|
||||
source <(~/.claude/skills/gstack/bin/gstack-diff-scope <base> 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 <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'
|
||||
<the specialist's complete output>
|
||||
STATE_MACHINES_OUTPUT
|
||||
```
|
||||
|
||||
### Adaptive gating
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ function generateSpecialistSelection(ctx: TemplateContext): string {
|
||||
### Detect stack and scope
|
||||
|
||||
\`\`\`bash
|
||||
if _SM=$(${ctx.paths.binDir}/gstack-state-matrix detect --base <base>); then source <(printf '%s\\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi
|
||||
source <(${ctx.paths.binDir}/gstack-diff-scope <base> 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 <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'
|
||||
<the specialist's complete output>
|
||||
STATE_MACHINES_OUTPUT
|
||||
\`\`\`
|
||||
|
||||
### Adaptive gating
|
||||
|
||||
|
||||
@@ -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 <base>); then source <(printf '%s\n' "$_SM"); else echo "$_SM" | grep '^#'; SCOPE_STATE_MACHINE=unknown; fi
|
||||
source <(~/.claude/skills/gstack/bin/gstack-diff-scope <base> 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 <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'
|
||||
<the specialist's complete output>
|
||||
STATE_MACHINES_OUTPUT
|
||||
```
|
||||
|
||||
### Adaptive gating
|
||||
|
||||
|
||||
@@ -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[];
|
||||
|
||||
@@ -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 <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' <the specialist's complete 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('<base>', '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');
|
||||
|
||||
Reference in new issue
Block a user