diff --git a/lib/checks.ts b/lib/checks.ts index 2495ef40f..f789228de 100644 --- a/lib/checks.ts +++ b/lib/checks.ts @@ -32,7 +32,8 @@ import { resolveBaseBranch } from './ship-policy'; export interface Check { name: string; command: string; source: string; body?: string } export type Verdict = 'PASS' | 'FAIL' | 'ZERO_RUN' | 'UNMAPPED' | 'TOO_BROAD' | 'ENV' | 'TIMEOUT' | 'INCOMPLETE'; export interface Outcome { name: string; verdict: Verdict; ms: number; detail: string; lines: string[]; log?: string; notCovered?: string[] } -export interface Ledger { tree: string; set: string; at: string; results: Array>; notCovered: string[] } +/** basis: the base commit and policy the selection was made against, so a moved base or changed policy is not READY. */ +export interface Ledger { tree: string; set: string; basis: string; at: string; results: Array>; notCovered: string[] } export const TESTS = 'tests'; export const MAX_LOCAL_TESTS = 300; @@ -52,7 +53,7 @@ export function declaredVerify(claudeMd: string): string | null { return null; } -/** The '## Health Stack' section (/health's own format): '- key: command' rows. 'test' is left to pregate. */ +/** The '## Health Stack' section (/health's own format): '- key: command' rows. 'test' and 'tests' are left to pregate; a repeated key gets -2, -3 so every check has its own name. */ export function healthStack(claudeMd: string): Check[] { const lines = claudeMd.split('\n'); const start = lines.findIndex(l => /^##\s+Health Stack\s*$/.test(l)); @@ -61,7 +62,11 @@ export function healthStack(claudeMd: string): Check[] { for (const line of lines.slice(start + 1)) { if (/^#{1,6}\s/.test(line)) break; const m = /^\s*[-*]\s*([A-Za-z0-9_-]+):\s*(.+?)\s*$/.exec(line); - if (m && m[1].toLowerCase() !== 'test') checks.push({ name: m[1].toLowerCase(), command: m[2].replace(/`/g, ''), source: 'CLAUDE.md Health Stack' }); + if (!m || ['test', TESTS].includes(m[1].toLowerCase())) continue; + const key = m[1].toLowerCase(); + let name = key; + for (let n = 2; checks.some(c => c.name === name); n++) name = `${key}-${n}`; + checks.push({ name, command: m[2].replace(/`/g, ''), source: 'CLAUDE.md Health Stack' }); } return checks; } @@ -95,8 +100,8 @@ export function manifestChecks(root: string): Check[] { if (/\bruff\b/.test(pyproject)) checks.push({ name: 'lint', command: 'ruff check .', source: 'pyproject.toml' }); if (/\bmypy\b/.test(pyproject)) checks.push({ name: 'typecheck', command: 'mypy .', source: 'pyproject.toml' }); } - if (fs.existsSync(path.join(root, 'Cargo.toml'))) checks.push({ name: 'build', command: 'cargo check --all-targets', source: 'Cargo.toml' }); - if (fs.existsSync(path.join(root, 'go.mod'))) checks.push({ name: 'build', command: 'go vet ./...', source: 'go.mod' }); + if (fs.existsSync(path.join(root, 'Cargo.toml'))) checks.push({ name: 'build', command: 'cargo check --all-targets', source: 'Cargo.toml' }, { name: 'lint', command: 'cargo clippy --all-targets', source: 'Cargo.toml' }); + if (fs.existsSync(path.join(root, 'go.mod'))) checks.push({ name: 'vet', command: 'go vet ./...', source: 'go.mod' }, { name: 'build', command: 'go build ./...', source: 'go.mod' }); const seen = new Set(); return checks.filter(c => (seen.has(c.name) ? false : (seen.add(c.name), true))); } @@ -105,14 +110,28 @@ export function manifestChecks(root: string): Check[] { export function detectChecks(root: string): Check[] { const claude = read(path.join(root, 'CLAUDE.md')) ?? ''; const declared = declaredVerify(claude); - if (declared) return [{ name: 'verify', command: declared, source: 'CLAUDE.md gstack:verify:' }]; const stack = healthStack(claude); - return stack.length ? stack : manifestChecks(root); + const checks = declared ? [{ name: 'verify', command: declared, source: 'CLAUDE.md gstack:verify:' }] : stack.length ? stack : manifestChecks(root); + let scripts: Record = {}; + try { scripts = JSON.parse(read(path.join(root, 'package.json')) ?? '{}').scripts ?? {}; } catch { /* no scripts */ } + return checks.map(c => { + const script = /^(?:npm|pnpm|yarn|bun)\s+run\s+([\w:.-]+)/.exec(c.command)?.[1]; + return c.body === undefined && script && scripts[script] ? { ...c, body: scripts[script] } : c; + }); } /** Every command a run could execute, read without executing any: the checks, the pregate runner and each selection manifest command. */ -export function commandSet(checks: readonly Check[], config: Pick): string { - return sha256(JSON.stringify({ checks: checks.map(c => [c.name, c.command, c.body ?? '']), runner: config.runner ?? 'detected', selection: Object.entries(config.selection ?? {}).sort(([a], [b]) => a.localeCompare(b)) })); +export function commandSet(root: string, checks: readonly Check[], config: Pick): string { + return sha256(JSON.stringify({ checks: checks.map(c => [c.name, c.command, c.body ?? '']), runner: config.runner ?? detectedRunnerInputs(root), selection: Object.entries(config.selection ?? {}).sort(([a], [b]) => a.localeCompare(b)) })); +} + +/** With no declared runner pregate picks one from lockfiles, test dependencies and the test script, so those are what trust covers. */ +function detectedRunnerInputs(root: string): string[] { + let pkg: { scripts?: Record; dependencies?: Record; devDependencies?: Record } = {}; + try { pkg = JSON.parse(read(path.join(root, 'package.json')) ?? '{}'); } catch { /* none */ } + const deps = Object.keys({ ...pkg.dependencies, ...pkg.devDependencies }).filter(d => /^(vitest|jest|mocha|ava|@playwright\/test|pytest)$/.test(d)).sort(); + const locks = ['bun.lock', 'bun.lockb', 'pnpm-lock.yaml', 'yarn.lock', 'package-lock.json'].filter(f => fs.existsSync(path.join(root, f))); + return ['detected', ...locks, ...deps, pkg.scripts?.test ?? '']; } const realRoot = (root: string) => { try { return fs.realpathSync(root); } catch { return path.resolve(root); } }; @@ -144,7 +163,7 @@ export function recordTrust(root: string, set: string, checks: readonly Check[], export function ledgerPath(root: string, env: NodeJS.ProcessEnv = process.env): string { const state = resolveStateRoot(env); - return path.join(state, 'projects', slugFromEnvironment(state, root), 'verify', 'last-run.json'); + return path.join(state, 'projects', slugFromEnvironment(state, root), 'verify', sha256(realRoot(root)).slice(0, 12), 'last-run.json'); } export function readLedger(file: string): Ledger | null { @@ -168,6 +187,7 @@ export function testVerdict(r: CheckResult): Pick sha256(JSON.stringify([ctx.baseRef, ctx.baseSha, ctx.configLabel, ctx.configDigest ?? ''])); /** The pregate context /verify runs under: the base policy, tests local whatever the core count, no remote gate, quiet progress. */ export function load(root: string, base: string | undefined, timeoutMs: number, env: NodeJS.ProcessEnv = process.env): Loaded | { refused: string } { @@ -175,7 +195,7 @@ export function load(root: string, base: string | undefined, timeoutMs: number, const loaded = buildContext({ repoRoot: root, base: b, policyFrom: 'base', allowRepoCommands: true, timeoutMs, cores: Number.MAX_SAFE_INTEGER, testBackend: 'local', remote: null, userConfigPath: defaultUserConfigPath(root), env, progress: () => {} }); if ('refused' in loaded) return { refused: describeRefusal(loaded.refused, b, 'gstack-checks').line }; const checks = detectChecks(root); - return { ctx: loaded.ctx, checks, set: commandSet(checks, loaded.ctx.config) }; + return { ctx: loaded.ctx, checks, set: commandSet(root, checks, loaded.ctx.config) }; } async function runCommand(ctx: PregateContext, check: Check, dir: string, timeoutMs: number): Promise { @@ -218,11 +238,14 @@ export async function runChecks(l: Loaded, o: RunOptions): Promise<{ outcomes: O const moved = 'error' in after || after.tree !== l.ctx.wtree; const final = moved ? outcomes.map(x => (x.verdict === 'PASS' ? { ...x, verdict: 'INCOMPLETE' as Verdict, detail: 'the working tree changed during the run; rerun' } : x)) : outcomes; const prior = readLedger(file); - const same = !!prior && prior.tree === l.ctx.wtree && prior.set === l.set; + const basis = basisOf(l.ctx); + const same = !!prior && prior.tree === l.ctx.wtree && prior.set === l.set && prior.basis === basis; const keep = same ? prior!.results.filter(p => !final.some(f => f.name === p.name)) : []; const tests = final.find(f => f.name === TESTS); - const ledger: Ledger = { tree: l.ctx.wtree, set: l.set, at: new Date().toISOString(), results: [...keep, ...final.map(f => ({ name: f.name, verdict: f.verdict }))], notCovered: tests ? tests.notCovered ?? [] : same ? prior!.notCovered : [] }; - fs.writeFileSync(file, JSON.stringify(ledger, null, 2) + '\n', { mode: 0o600 }); + const ledger: Ledger = { tree: l.ctx.wtree, set: l.set, basis, at: new Date().toISOString(), results: [...keep, ...final.map(f => ({ name: f.name, verdict: f.verdict }))], notCovered: tests ? tests.notCovered ?? [] : same ? prior!.notCovered : [] }; + const tmp = `${file}.tmp.${process.pid}`; + fs.writeFileSync(tmp, JSON.stringify(ledger, null, 2) + '\n', { mode: 0o600 }); + fs.renameSync(tmp, file); return { outcomes: final, ledger }; } @@ -233,8 +256,9 @@ export function status(l: Loaded, ledger: Ledger | null): { ready: boolean; line if (!ledger) return notReady('no gstack-checks run recorded; run gstack-checks run'); if (ledger.tree !== l.ctx.wtree) return notReady(`the last run was at ${ledger.tree.slice(0, 12)}; run gstack-checks run`); if (ledger.set !== l.set) return notReady('the check set changed since the last run; run gstack-checks run'); + if (ledger.basis !== basisOf(l.ctx)) return notReady(`the base or pregate policy changed since the last run (now ${l.ctx.configLabel}); run gstack-checks run`); const needed = [...l.checks.map(c => c.name), TESTS]; - const gaps = needed.map(n => [n, ledger.results.find(r => r.name === n)?.verdict ?? 'not run'] as const).filter(([, v]) => v !== 'PASS'); + const gaps = needed.map(n => [n, ledger.results.filter(r => r.name === n).map(r => r.verdict).find(v => v !== 'PASS') ?? (ledger.results.some(r => r.name === n) ? 'PASS' : 'not run')] as const).filter(([, v]) => v !== 'PASS'); if (gaps.length) return notReady(gaps.map(([n, v]) => `${n}=${v}`).join(' ')); return { ready: true, line: `READY_FOR_SHIP tree=${tree} checks=${needed.join(',')} not_covered=${ledger.notCovered.join(',') || 'none'}` }; } diff --git a/test/gstack-checks.test.ts b/test/gstack-checks.test.ts index afecf4c52..2e7b54399 100644 --- a/test/gstack-checks.test.ts +++ b/test/gstack-checks.test.ts @@ -58,6 +58,8 @@ describe('detection', () => { test('Health Stack rows and manifest checks', () => { expect(healthStack('# x\n## Health Stack\n\n- typecheck: tsc --noEmit\n- test: bun test\n- shell: shellcheck *.sh\n## Next\n- lint: nope\n').map(c => [c.name, c.command])) .toEqual([['typecheck', 'tsc --noEmit'], ['shell', 'shellcheck *.sh']]); + // A repeated key keeps both checks under their own names, and a tests row is pregate's, so status never reads one verdict for two checks. + expect(healthStack('## Health Stack\n- lint: eslint .\n- lint: stylelint "**/*.css"\n- tests: npm test\n').map(c => c.name)).toEqual(['lint', 'lint-2']); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-checks-manifest-')); dirs.push(dir); fs.writeFileSync(path.join(dir, 'tsconfig.json'), '{}'); @@ -66,7 +68,7 @@ describe('detection', () => { fs.writeFileSync(path.join(dir, 'bun.lock'), ''); fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ scripts: { lint: 'eslint .' }, devDependencies: { typescript: '5' } })); fs.writeFileSync(path.join(dir, 'go.mod'), 'module x\n'); - expect(manifestChecks(dir).map(c => c.command)).toEqual(['bunx tsc --noEmit', 'bun run lint', 'go vet ./...']); + expect(manifestChecks(dir).map(c => c.command)).toEqual(['bunx tsc --noEmit', 'bun run lint', 'go vet ./...', 'go build ./...']); const py = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-checks-py-')); dirs.push(py); expect(manifestChecks(py)).toEqual([]); @@ -113,6 +115,7 @@ describe('gstack-checks on a fixture repo', () => { // A changed check command needs consent again. write(s.repo.work, 'package.json', JSON.stringify({ name: 'fixture', scripts: { typecheck: 'node scripts/typecheck.js --strict' } }) + '\n'); expect(run(s).status).toBe(3); + expect(s.checks(['status']).out).toStartWith('NOT_READY'); s.repo.git('checkout', '-q', '--', 'package.json'); // A selection manifest command changed on the base needs consent before anything executes. s.repo.git('checkout', '-q', 'main'); @@ -154,6 +157,28 @@ describe('gstack-checks on a fixture repo', () => { expect(s.checks(['status']).out).toContain('the last run was at'); }, 60_000); + // Value: protects=two --only runs at one tree add up to READY, and a tree that changes during a run is never green; bad arguments + // run nothing; fails_when=a partial rerun drops the other verdicts, a run that edited the tree reads PASS, or an unknown --only name + // is ignored; why_new=the ledger merge and tree re-check are new; seam=a declared check that writes a file; no_claim=signals + test('partial runs, a tree moved mid-run and argument errors', () => { + const s = setup(); + expect(s.checks(['trust']).status).toBe(0); + commit(s.repo, { 'lib/util.js': 'exports.add = (a, b) => b + a;\n' }); + expect(run(s, '--only', 'typecheck').status).toBe(0); + expect(s.checks(['status']).out).toContain('tests=not run'); + expect(run(s, '--only', 'tests').status).toBe(0); + expect(s.checks(['status']).out).toStartWith('READY_FOR_SHIP'); + expect(run(s, '--only', 'nope').status).toBe(64); + expect(run(s, '--timeout', '0').status).toBe(64); + expect(s.checks(['--base']).status).toBe(64); + write(s.repo.work, 'CLAUDE.md', 'gstack:verify: node -e "require(\'fs\').writeFileSync(\'touched.txt\', \'x\')"\n'); + commit(s.repo, {}); + expect(s.checks(['trust']).status).toBe(0); + const moved = run(s, '--only', 'verify'); + expect(moved.out).toContain('CHECK verify INCOMPLETE'); + expect(moved.status).toBe(1); + }, 60_000); + // Value: protects=a selection that runs zero tests is not green; fails_when=exit 0 with no test counted reads PASS; why_new=the // verdict is new; seam=pregate's runner summary (a bats stub printing an empty TAP plan); no_claim=node --test, which counts // each file as a test, so an empty file there is not a zero run diff --git a/verify/SKILL.md b/verify/SKILL.md index 0242bacba..ea9e4c937 100644 --- a/verify/SKILL.md +++ b/verify/SKILL.md @@ -154,14 +154,15 @@ reviews, bumps a version, edits CHANGELOG, pushes, or opens a PR: that is /ship. ## 1. See what will run +Each Bash call is a fresh shell, so every block sets `C` again. + ```bash -C="$HOME/.claude/skills/gstack/bin/gstack-checks" -"$C" detect +C="$HOME/.claude/skills/gstack/bin/gstack-checks"; "$C" detect ``` `detect` reads files only. It lists each check with its source, the pregate -test selection, and `TRUST trusted|untrusted`. `CHECKS: none detected` with -no pregate policy means there is nothing to run: say so and stop. Never invent a +test selection, and `TRUST trusted|untrusted`. `CHECKS: none` means the repo declares no check, +pregate policy or test file: there is nothing to run, so say so and stop. Never invent a command the repo does not declare. When `untrusted`: in an interactive session, show the user the listed commands @@ -171,7 +172,7 @@ and ask before `"$C" trust`. In a session launched to run repo commands, pass ## 2. Loop: run, fix, run ```bash -"$C" run +C="$HOME/.claude/skills/gstack/bin/gstack-checks"; "$C" run ``` One line per check: `CHECK `, at most 40 failing lines @@ -201,7 +202,7 @@ change goes to /investigate. ## 3. Hand off ```bash -"$C" status +C="$HOME/.claude/skills/gstack/bin/gstack-checks"; "$C" status ``` `status` reruns nothing. Only `READY_FOR_SHIP tree= ... not_covered=` diff --git a/verify/SKILL.md.tmpl b/verify/SKILL.md.tmpl index cadd32a8c..8f21e5625 100644 --- a/verify/SKILL.md.tmpl +++ b/verify/SKILL.md.tmpl @@ -30,14 +30,15 @@ reviews, bumps a version, edits CHANGELOG, pushes, or opens a PR: that is /ship. ## 1. See what will run +Each Bash call is a fresh shell, so every block sets `C` again. + ```bash -C="{{RUNTIME_ROOT}}/bin/gstack-checks" -"$C" detect +C="{{RUNTIME_ROOT}}/bin/gstack-checks"; "$C" detect ``` `detect` reads files only. It lists each check with its source, the pregate -test selection, and `TRUST trusted|untrusted`. `CHECKS: none detected` with -no pregate policy means there is nothing to run: say so and stop. Never invent a +test selection, and `TRUST trusted|untrusted`. `CHECKS: none` means the repo declares no check, +pregate policy or test file: there is nothing to run, so say so and stop. Never invent a command the repo does not declare. When `untrusted`: in an interactive session, show the user the listed commands @@ -47,7 +48,7 @@ and ask before `"$C" trust`. In a session launched to run repo commands, pass ## 2. Loop: run, fix, run ```bash -"$C" run +C="{{RUNTIME_ROOT}}/bin/gstack-checks"; "$C" run ``` One line per check: `CHECK `, at most 40 failing lines @@ -77,7 +78,7 @@ change goes to /investigate. ## 3. Hand off ```bash -"$C" status +C="{{RUNTIME_ROOT}}/bin/gstack-checks"; "$C" status ``` `status` reruns nothing. Only `READY_FOR_SHIP tree= ... not_covered=`