From 47f45434656e8b1344e7d67e0368a51a135acf3d Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 18 Oct 2026 00:13:11 +0000 Subject: [PATCH] fix(review-army): write the state-machines output to a fresh private file from gstack-state-matrix outfile /review delta on 51f1e02 (Claude F2): dc739be wrote the specialist output to a fixed .gstack/tmp/state-machines.md with no exclude. On a second review Claude Code's Write refuses an existing unread file, so check could validate the previous run's output, and git add -A could commit it. gstack-state-matrix outfile creates .gstack/tmp (0700) at the repo top, adds /.gstack/tmp/ to info/exclude once, and prints a new empty 0600 file per call; the selection prose has the agent Read that file, write the full output there and check that path. The prose moved the check command inline, so gstack-review shrinks and its pins are lowered (cursor 141010->140637, factory 144792->144416, kiro 140816->140449, opencode 143258->142872, slate 140913->140543). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-state-matrix | 27 ++++++++++++++++++- review/sections/review-army.md | 5 +--- scripts/resolvers/review-army.ts | 5 +--- ship/sections/review-army.md | 5 +--- test/fixtures/skill-md-size-allowlist.json | 10 +++---- test/review-workflow-clarity.test.ts | 7 +++-- test/state-matrix.test.ts | 31 ++++++++++++++++++++++ 7 files changed, 70 insertions(+), 20 deletions(-) diff --git a/bin/gstack-state-matrix b/bin/gstack-state-matrix index f4d039b33..92a20fbbd 100755 --- a/bin/gstack-state-matrix +++ b/bin/gstack-state-matrix @@ -4,16 +4,22 @@ * * gstack-state-matrix detect [--base ] [--diff-base ] * gstack-state-matrix check [--repo ] [--expect ] [--json] + * gstack-state-matrix outfile * * `detect` prints `SCOPE_STATE_MACHINE=true|false` (shell-safe for * `source <(...)`, with `# state-machine:` comment lines naming the hits) from * the lines the branch adds since its merge base with origin/. * `check` reads the state-machines specialist's output and prints * `STATE_MATRIX_GAP:` per gap and `STATE_MATRIX_CHECK: ok | incomplete gaps=` - * last; exit 0 ok, 1 incomplete (STATE_MATRIX_INCOMPLETE). Logic: lib/state-matrix.ts. + * last; exit 0 ok, 1 incomplete (STATE_MATRIX_INCOMPLETE). `outfile` creates and + * prints a new empty 0600 file in the repo's private `.gstack/tmp/` (0700, kept + * out of git through info/exclude) for the specialist's output, so a later + * review never checks or commits an earlier run's file. Logic: lib/state-matrix.ts. */ import { spawnSync } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; import * as fs from 'node:fs'; +import * as path from 'node:path'; import { addedLines } from '../lib/banned-terms'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { resultLine } from '../lib/result-codes'; @@ -25,6 +31,7 @@ const HELP = `gstack-state-matrix — kind × owner action × resulting row for detect [--base ] [--diff-base ] SCOPE_STATE_MACHINE=true|false for the branch's added code lines check [--repo ] [--expect ] [--json] gaps in the state-machines specialist's output (review/specialists/state-machines.md) + outfile print a new empty private file for that output ${exitTableHelp()}`; @@ -64,11 +71,29 @@ function check(rest: string[]): number { return EXIT.ok; } +function outfile(): number { + const root = git(['rev-parse', '--show-toplevel']); + const exclude = git(['rev-parse', '--git-path', 'info/exclude']); + if (!root.ok || !exclude.ok) { process.stderr.write('gstack-state-matrix outfile: not inside a git repository\n'); return EXIT.usage; } + const dir = path.join(root.out.trim(), '.gstack', 'tmp'); + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + fs.chmodSync(dir, 0o700); + const excludeFile = path.resolve(exclude.out.trim()); + fs.mkdirSync(path.dirname(excludeFile), { recursive: true }); + const excluded = fs.existsSync(excludeFile) ? fs.readFileSync(excludeFile, 'utf8') : ''; + if (!excluded.split('\n').includes('/.gstack/tmp/')) fs.appendFileSync(excludeFile, `${excluded && !excluded.endsWith('\n') ? '\n' : ''}/.gstack/tmp/\n`); + const file = path.join(dir, `state-machines.${randomBytes(6).toString('hex')}.md`); + fs.closeSync(fs.openSync(file, 'wx', 0o600)); + process.stdout.write(file + '\n'); + return EXIT.ok; +} + function main(argv: string[]): number { const [cmd, ...rest] = argv; if (!cmd || cmd === '--help' || cmd === '-h') { process.stdout.write(HELP + '\n'); return cmd ? EXIT.ok : EXIT.usage; } if (cmd === 'detect') return detect(rest); if (cmd === 'check') return check(rest); + if (cmd === 'outfile') return outfile(); process.stderr.write(`gstack-state-matrix: unknown command ${cmd}\n${HELP}\n`); return EXIT.usage; } diff --git a/review/sections/review-army.md b/review/sections/review-army.md index 0e1b20417..2043282e0 100644 --- a/review/sections/review-army.md +++ b/review/sections/review-army.md @@ -51,10 +51,7 @@ 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 SCOPE_STATE_MACHINE is true or unknown: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then write its full output to `.gstack/tmp/state-machines.md` and check it: -```bash -~/.claude/skills/gstack/bin/gstack-state-matrix check .gstack/tmp/state-machines.md -``` +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, run `~/.claude/skills/gstack/bin/gstack-state-matrix outfile`, Read the file it prints, write the full output there, then run `~/.claude/skills/gstack/bin/gstack-state-matrix check ` on it. ### Adaptive gating diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index a12fcb134..4601341fd 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -70,10 +70,7 @@ 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 SCOPE_STATE_MACHINE is true or unknown: read \`${ctx.paths.skillRoot}/review/specialists/state-machines.md\`, then write its full output to \`.gstack/tmp/state-machines.md\` and check it: -\`\`\`bash -${ctx.paths.binDir}/gstack-state-matrix check .gstack/tmp/state-machines.md -\`\`\` +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown: read \`${ctx.paths.skillRoot}/review/specialists/state-machines.md\`, run \`${ctx.paths.binDir}/gstack-state-matrix outfile\`, Read the file it prints, write the full output there, then run \`${ctx.paths.binDir}/gstack-state-matrix check \` on it. ### Adaptive gating diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 0efb11a24..dab702dd8 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -313,10 +313,7 @@ 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 SCOPE_STATE_MACHINE is true or unknown: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, then write its full output to `.gstack/tmp/state-machines.md` and check it: -```bash -~/.claude/skills/gstack/bin/gstack-state-matrix check .gstack/tmp/state-machines.md -``` +9. **State Machines** — if SCOPE_STATE_MACHINE is true or unknown: read `~/.claude/skills/gstack/review/specialists/state-machines.md`, run `~/.claude/skills/gstack/bin/gstack-state-matrix outfile`, Read the file it prints, write the full output there, then run `~/.claude/skills/gstack/bin/gstack-state-matrix check ` on it. ### Adaptive gating diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index 9f5980083..b51d87adb 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -37,7 +37,7 @@ "cursor/gstack-plan-design-review": 138935, "cursor/gstack-plan-devex-review": 134558, "cursor/gstack-plan-eng-review": 151523, - "cursor/gstack-review": 141010, + "cursor/gstack-review": 140637, "factory/gstack-autoplan": 131397, "factory/gstack-codex": 100523, "factory/gstack-design-consultation": 108523, @@ -47,7 +47,7 @@ "factory/gstack-plan-design-review": 141716, "factory/gstack-plan-devex-review": 137927, "factory/gstack-plan-eng-review": 154637, - "factory/gstack-review": 144792, + "factory/gstack-review": 144416, "gbrain/gstack-autoplan": 129650, "gbrain/gstack-design-review": 132733, "gbrain/gstack-land-and-deploy": 116039, @@ -67,7 +67,7 @@ "kiro/gstack-plan-design-review": 138775, "kiro/gstack-plan-devex-review": 134398, "kiro/gstack-plan-eng-review": 151373, - "kiro/gstack-review": 140816, + "kiro/gstack-review": 140449, "openclaw/gstack-autoplan": 129355, "openclaw/gstack-design-review": 131538, "openclaw/gstack-land-and-deploy": 115627, @@ -82,7 +82,7 @@ "opencode/gstack-plan-design-review": 140705, "opencode/gstack-plan-devex-review": 136779, "opencode/gstack-plan-eng-review": 153275, - "opencode/gstack-review": 143258, + "opencode/gstack-review": 142872, "slate/gstack-autoplan": 128994, "slate/gstack-design-consultation": 106996, "slate/gstack-design-review": 142272, @@ -90,6 +90,6 @@ "slate/gstack-plan-design-review": 138855, "slate/gstack-plan-devex-review": 134478, "slate/gstack-plan-eng-review": 151448, - "slate/gstack-review": 140913 + "slate/gstack-review": 140543 } } diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index 127575c14..c68b71d1a 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -388,8 +388,11 @@ for (const skillName of ['review', 'ship']) { 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: read'); - expect(selection).toContain('then write its full output to `.gstack/tmp/state-machines.md` and check it:'); - expect(selection).toContain('gstack-state-matrix check .gstack/tmp/state-machines.md '); + // /review delta on 51f1e02: a fixed .gstack/tmp/state-machines.md let a second review check the previous run's + // output (Write refuses an unread existing file) and commit it; each run now checks the new file outfile prints. + expect(selection).toContain('/gstack-state-matrix outfile`, Read the file it prints, write the full output there, then run `'); + expect(selection).toContain('/gstack-state-matrix check ` on it.'); + expect(selection).not.toContain('.gstack/tmp/state-machines.md'); expect(selection).not.toContain('<<'); expect(selection).not.toContain('pipe output'); }); diff --git a/test/state-matrix.test.ts b/test/state-matrix.test.ts index 66f0ee6a5..2b288e6cb 100644 --- a/test/state-matrix.test.ts +++ b/test/state-matrix.test.ts @@ -70,6 +70,37 @@ describe('gstack-state-matrix detect', () => { }); }); +describe('gstack-state-matrix outfile', () => { + test('each call prints a new empty private file under an excluded .gstack/tmp, so a rerun never checks or commits the last output', () => { + const r = repo(); + const sub = path.join(r.work, 'src'); + fs.mkdirSync(sub, { recursive: true }); + const first = run(sub, ['outfile']); + expect(first.status).toBe(EXIT.ok); + const a = first.out.trim(); + fs.writeFileSync(a, 'previous run output\n'); + const b = run(r.work, ['outfile']).out.trim(); + const tmp = path.join(fs.realpathSync(r.work), '.gstack', 'tmp'); + for (const file of [a, b]) expect(path.dirname(fs.realpathSync(file))).toBe(tmp); + expect(b).not.toBe(a); + expect(fs.readFileSync(b, 'utf8')).toBe(''); + expect(fs.statSync(b).mode & 0o777).toBe(0o600); + expect(fs.statSync(tmp).mode & 0o777).toBe(0o700); + const exclude = fs.readFileSync(path.join(r.work, '.git', 'info', 'exclude'), 'utf8'); + expect(exclude.split('\n').filter(l => l === '/.gstack/tmp/')).toHaveLength(1); + expect(spawnSync('git', ['status', '--porcelain', '--untracked-files=all'], { cwd: r.work, encoding: 'utf8', timeout: 30_000 }).stdout).not.toContain('.gstack'); + }); + + test('outside a git repository it refuses with a usage exit and prints no path', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sm-outfile-')); + try { + const r = run(dir, ['outfile']); + expect(r.status).toBe(EXIT.usage); + expect(r.out).toBe(''); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); +}); + describe('gstack-state-matrix check on the captured #6792 review', () => { test('the capture passes: every kind × action has a row, real tests, and each untested cell is CRITICAL; the destructive cells are named', () => { const r = repo();