diff --git a/bin/gstack-docs-candidate b/bin/gstack-docs-candidate index a2a0a5fbc..bb6d34173 100755 --- a/bin/gstack-docs-candidate +++ b/bin/gstack-docs-candidate @@ -4,7 +4,7 @@ * * Usage (run inside the repository): * gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only - * --base [--select ]... [--docs ]... [--exception ] + * --base [--select ]... [--docs [,...]]... [--exception ] * gstack-docs-candidate compare * gstack-docs-candidate attempt --state * gstack-docs-candidate [snapshot|compare] --help @@ -15,7 +15,10 @@ * selected path, docs path and dirty/untracked path. Without --select, the * selected paths are the committed, staged, unstaged and untracked paths. * --docs takes files or directories (directories expand to their tracked and - * untracked files). It prints a short summary; the JSON file is the record. + * untracked files): repeat the flag, or give a comma-separated list (each entry + * trimmed). A value that names an existing file or directory is one path, commas + * and spaces included; any other value holding whitespace is refused, because a + * space-separated list cannot be told from a path with a space. It prints a short summary; the JSON file is the record. * --exception names the blocked .gstack/tmp/docs-audit.json an owner accepted * (Step 14.5 Blocked recovery): the record then carries that exception, so * Step 16 can tell an accepted risk from a skipped audit. A --base that is HEAD @@ -72,6 +75,18 @@ function expand(listed: string[], paths: string[]): string[] { return [...out].sort(); } +/** --docs values as paths: a value naming an existing file or directory is one path; any other splits on commas, and an entry with whitespace is refused. */ +function docsPaths(listed: string[], values: string[]): string[] { + const exists = (p: string) => { + const rel = p.replace(/^\.\//, '').replace(/\/+$/, ''); + return fs.existsSync(rel) || listed.some(f => f === rel || f.startsWith(rel + '/')); + }; + return values.flatMap(v => (exists(v) ? [v] : v.split(',').map(p => p.trim()).filter(Boolean))).map(p => { + if (/\s/.test(p) && !exists(p)) throw new Error(`--docs ${JSON.stringify(p)} names no file or directory and holds whitespace: repeat --docs for each path, or separate paths with commas`); + return p; + }); +} + /** Git blob ids of working-tree content, computed in-process (any path bytes, nothing written); null when absent. */ function hashes(paths: string[]): Record { const out: Record = {}; @@ -99,7 +114,7 @@ function one(args: string[], name: string): string { return values[0]!; } -const USAGE = `usage: gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only --base [--select ]... [--docs ]... [--exception ] +const USAGE = `usage: gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only --base [--select ]... [--docs [,...]]... [--exception ] gstack-docs-candidate compare gstack-docs-candidate attempt --state `; @@ -149,7 +164,7 @@ function main(argv: string[]): void { const listed = nul(git(['ls-files', '-z', '--cached', '--others', '--exclude-standard'])); const selected = flag(args, '--select').length ? expand(listed, flag(args, '--select')) : [...new Set([...now.committed, ...now.staged, ...now.unstaged, ...now.untracked])].sort(); - const docs = expand(listed, flag(args, '--docs')); + const docs = expand(listed, docsPaths(listed, flag(args, '--docs'))); const tracked = [...new Set([...selected, ...docs, ...now.staged, ...now.unstaged, ...now.untracked])].sort(); const record = { schema_version: 1, audit_id: one(args, '--audit-id'), mode, base, ...now, selected, docs, hashes: hashes(tracked), ...(exception ? { exception } : {}) }; fs.writeFileSync(out, JSON.stringify(record, null, 2) + '\n', { mode: 0o600 }); diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index b5b00be12..c5a8c3554 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -56,7 +56,8 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free judge --ledger --merged --judged judged lines {"id","severity","reason"?,"duplicate_of"?}: every merged id kept at its severity or above (a duplicate folds into one at least as severe); added and promoted findings need a reason. JUDGMENT: ok …, or - JUDGMENT_REJECTED: dropped=… downgraded=… (exit 1) + JUDGMENT_REJECTED: dropped=… downgraded=… (exit 1). An empty merged.jsonl needs no judged file: judge writes an + empty one and prints JUDGMENT: ok nothing to judge; a merge with findings and no judged file is refused (exit 2) verify --ledger [--targeted-out ] BRIEF, OUTPUT and JUDGE rows from the ledger alone, then VERIFY: clean (exit 0), VERIFY: dirty (exit 1) or VERIFY: no brief records (exit 3: this run's ledger kept no prompts). A ledger with a targeted brief also prints @@ -68,6 +69,7 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free runs gstack-review-plan and writes /pass-manifest.json: stage first (audit readers marked first in --extras), specialists (with the other --extras readers), red-team, voices (Step 11). Renders first and specialists now; prints the plan, MANIFEST: and one READER: line per reader. A plan whose NEXT is not full, scoped or fix-check prints PASS: none. + is letters, digits and dashes, read as lowercase (P1 is p1): the manifest, reader titles and a resume use that form. A fix-check reader (stage specialists) renders from decision 3.5's --fix-list, --hunks, --tests and --callers files. --diff-base sets every reader's base, a scoped pass's Red Team included (the targeted reader keeps the merge base). An existing manifest for the same --pass is reused (resumed). --extras: [{"name","prompt_file","first"?,"model"?}] @@ -184,8 +186,9 @@ function ceiling(context: string, base: string, outDir: string, readers: Readonl } function pass(): number { - const base = flag('--base'), out = flag('--out'), id = flag('--pass'); - if (!base || !out || !id || !/^[a-z0-9-]+$/.test(id)) { process.stderr.write('gstack-review-dispatch pass: needs --base , --out and --pass (lowercase letters, digits, dashes)\n'); return EXIT.usage; } + const base = flag('--base'), out = flag('--out'), given = flag('--pass'), id = given?.toLowerCase(); + if (!base || !out || !id) { process.stderr.write('gstack-review-dispatch pass: needs --base , --out and --pass \n'); return EXIT.usage; } + if (!/^[a-z0-9-]+$/.test(id)) { process.stderr.write(`gstack-review-dispatch pass: --pass ${JSON.stringify(given)} is not a pass id: use letters, digits and dashes (read as lowercase, so P1 is p1)\n`); return EXIT.usage; } const file = manifestPath(out); if (fs.existsSync(file)) { const m = readManifest(file); @@ -413,7 +416,15 @@ function main(): number { const merged = flag('--merged'); const judged = flag('--judged'); if (!merged || !judged) { process.stderr.write('gstack-review-dispatch judge: needs --merged and --judged \n'); return EXIT.usage; } - const r = recordJudgment(ledger, readJsonl(merged), readJsonl(judged), judged, merged); + const lines = readJsonl(merged); + const nothing = !lines.length && !fs.existsSync(judged); + if (lines.length && !fs.existsSync(judged)) { + process.stderr.write(`gstack-review-dispatch judge: ${judged} does not exist; ${merged} holds ${lines.length} finding${lines.length === 1 ? '' : 's'}, so lead judgment writes one judged line per finding first\n`); + return EXIT.usage; + } + if (nothing) fs.writeFileSync(judged, '', { mode: 0o600 }); + const r = recordJudgment(ledger, lines, readJsonl(judged), judged, merged); + if (nothing && r.ok) { process.stdout.write(`JUDGMENT: ok nothing to judge (${merged} holds no findings; wrote an empty ${judged})\n`); return EXIT.ok; } process.stdout.write(r.ok ? `JUDGMENT: ok added=${r.added.length} promoted=${r.promoted.length} folded=${r.folded.length}${r.dismissed.length ? ` dismissed=${r.dismissed.length}` : ''}\n` : `JUDGMENT_REJECTED: dropped=${JSON.stringify(r.dropped)} downgraded=${JSON.stringify(r.downgraded)} unexplained=${JSON.stringify(r.unexplained)} invalid=${JSON.stringify(r.invalid)} (lead judgment may add or promote a finding, never drop or downgrade one; a summary_candidate line may be lowered with a reason)\n`); return r.ok ? EXIT.ok : EXIT.fail; diff --git a/changelog.d/gst-1219-ship-papercuts.md b/changelog.d/gst-1219-ship-papercuts.md new file mode 100644 index 000000000..f65490ab8 --- /dev/null +++ b/changelog.d/gst-1219-ship-papercuts.md @@ -0,0 +1 @@ +- **Three /ship papercuts from gstack-evals#191 are fixed (GST-1219).** `gstack-review-dispatch judge` no longer crashes when every reader came back clean: an empty `merged.jsonl` with no `judged.jsonl` now writes an empty judged set and prints `JUDGMENT: ok nothing to judge`, while a merge with findings and no judged file is refused with the finding count. `gstack-review-dispatch pass --pass` reads its id as lowercase, so `--pass P1` runs pass `p1`; an id with any character other than letters, digits and dashes gets its own error naming that rule instead of the missing-flag message. `gstack-docs-candidate snapshot --docs` takes a comma-separated list as well as repeated flags; a value that names an existing path is kept whole, and a space-separated list is refused with a pointer to commas or repeated flags. diff --git a/docs/review-policy.md b/docs/review-policy.md index bc1bee4fc..93789399e 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -652,6 +652,10 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: finding, lowers its severity, folds it into a less severe duplicate, or adds or promotes one without a reason (`JUDGMENT_REJECTED`). A merged file missing any id the ledger collected is a drop. Each addition and promotion is logged with its reason. + When every reader came back clean, `merged.jsonl` is empty and there is nothing to + judge: `judge` writes an empty `judged.jsonl` itself and prints + `JUDGMENT: ok nothing to judge`. A merge that holds findings with no `judged.jsonl` + is refused (exit 2), naming the finding count. 5. `verify --ledger ` replays all of it from the run's ledger: rendered against dispatched hashes per render (each render is one dispatch of its reader, so a two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as @@ -680,7 +684,8 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: recorded judged set, covering every collected id. A ledger from a run that kept no prompts prints `VERIFY: no brief records` (exit 3), never clean. 6. `pass --base --out --pass -- ` runs one pass from a - staged manifest (`/pass-manifest.json`): audit readers marked first, then + staged manifest (`/pass-manifest.json`). The pass id is letters, digits and + dashes, read as lowercase (`P1` is `p1`); any other character is refused. Audit readers marked first, then the specialists with the other audit readers, then the Red Team, then Step 11's voices. A QA extra is refused before any reader is created: Step 9.2.1's probes are the parent's. `--diff-base` sets every reader's base, a scoped pass's Red diff --git a/test/gstack-docs-candidate.test.ts b/test/gstack-docs-candidate.test.ts index 296d3d60b..aa2c68686 100644 --- a/test/gstack-docs-candidate.test.ts +++ b/test/gstack-docs-candidate.test.ts @@ -92,6 +92,28 @@ describe('gstack-docs-candidate', () => { expect(fs.existsSync(path.join(dir, 'inside.json'))).toBe(false); }); + // Value: protects=--docs takes a comma-separated list as well as repeated flags, a path that exists is taken whole even with a space or a comma in it, and a space-separated list is refused instead of recorded as one missing path; fails_when=--docs "a, b" records one path "a, b" (gstack-evals#191 /ship), a single path or an existing spaced path stops working, or "a b" is silently recorded; + // why_new=each --docs value was one path; seam=gstack-docs-candidate snapshot; no_claim=--select, which still takes one path per flag + test('--docs splits a comma-separated list; an existing path is taken whole; a space-separated list is refused', () => { + const { dir, root, record } = repo(); + fs.writeFileSync(path.join(dir, 'my notes.md'), 'n\n'); + fs.writeFileSync(path.join(dir, 'a,b.md'), 'c\n'); + const docs = (...values: string[]) => { + const r = run(dir, 'snapshot', '--out', record, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', ...values.flatMap(v => ['--docs', v])); + expect(r.status, r.stderr).toBe(0); + return JSON.parse(fs.readFileSync(record, 'utf8')).docs; + }; + expect(docs('handbook')).toEqual(['handbook/guide.md']); + expect(docs('handbook, app.ts')).toEqual(['app.ts', 'handbook/guide.md']); + expect(docs('handbook,app.ts,', 'my notes.md')).toEqual(['app.ts', 'handbook/guide.md', 'my notes.md']); + expect(docs('a,b.md')).toEqual(['a,b.md']); + fs.rmSync(record); + const spaced = run(dir, 'snapshot', '--out', record, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'handbook app.ts'); + expect([spaced.status, spaced.stderr.trim()]).toEqual([1, 'gstack-docs-candidate: --docs "handbook app.ts" names no file or directory and holds whitespace: repeat --docs for each path, or separate paths with commas']); + expect(fs.existsSync(record)).toBe(false); + expect(fs.readdirSync(root)).toEqual(['repo']); + }); + test('--help prints the usage it names in its errors', () => { const { dir } = repo(); const help = run(dir, '--help'); diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index fa5e40f54..662d1ddc5 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -249,6 +249,31 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect([tampered.status, tampered.stdout.includes('OUTPUT outside-voice MISMATCH')]).toEqual([1, true]); }); + // Value: protects=a pass whose readers all came back clean records "nothing to judge" without a hand-written judged.jsonl, and a pass with findings still needs lead judgment's file; fails_when=judge crashes on the absent judged set of an empty merge (gstack-evals#191 /ship), or an absent judged set passes a merge that holds findings; + // why_new=judge always read --judged; seam=judge and verify; no_claim=that Step 9.4 runs judge at all on an empty merge + test('judge on an empty merge records nothing to judge without a judged set; a merge with findings still needs one', () => { + const p = pass(); + run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]); + expect(run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('none.out', 'NO FINDINGS\n'), '--merged', p.merged]).status).toBe(0); + expect(fs.readFileSync(p.merged, 'utf8')).toBe(''); + const judged = path.join(p.out, 'judged.jsonl'); + const empty = run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', judged]); + expect([empty.status, empty.stderr, empty.stdout.trim()]).toEqual([0, '', `JUDGMENT: ok nothing to judge (${p.merged} holds no findings; wrote an empty ${judged})`]); + expect(fs.readFileSync(judged, 'utf8')).toBe(''); + const v = run(['verify', '--ledger', p.ledger]); + expect([v.status, v.stdout.includes('JUDGE - OK'), v.stdout.includes('VERIFY: clean briefs=1 outputs=1')]).toEqual([0, true, true]); + const q = pass(); + run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', q.file]); + const finding = q.w('spec.out', '{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"x","specialist":"testing"}\n'); + expect(run(['collect', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--output', finding, '--merged', q.merged]).status).toBe(0); + const missing = path.join(q.out, 'judged.jsonl'); + const needed = run(['judge', '--ledger', q.ledger, '--merged', q.merged, '--judged', missing]); + expect([needed.status, needed.stdout]).toEqual([2, '']); + expect(needed.stderr.trim()).toBe(`gstack-review-dispatch judge: ${missing} does not exist; ${q.merged} holds 1 finding, so lead judgment writes one judged line per finding first`); + expect(fs.existsSync(missing)).toBe(false); + expect(fs.readFileSync(q.ledger, 'utf8')).not.toContain('"event":"judge"'); + }); + // Value: protects=verify is clean only when every collected output came from a brief rendered and sent verbatim, nothing was collected after the judgment, and the judged set still passes as recorded; a fold keeps a standalone survivor; fails_when=outputs from hand-written briefs, a finding collected after judge, a judged.jsonl edited after judge, a self-fold or a fold cycle still read clean (/ship #3648 pass-1 Red Team); // why_new=the first gate trusted the last verdict and any duplicate_of; seam=verify and judge; no_claim=the host's dispatch test('verify is dirty for an unrendered reader, an output after judge and a judged set edited after judge; self-folds and fold cycles are drops', () => { diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 3eadafe57..d642c27ec 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -675,6 +675,29 @@ describe('Capy pass program (fake host, real bins)', () => { expect(bin(dir, env, ['pass', '--base', 'main', '--out', path.join(dir, '.state/run/d2'), '--pass', 'x16b', '--audit-only']).status).not.toBe(0); }, 240_000); + // Value: protects=a pass id typed in capitals is read as its lowercase id, so the manifest, its titles and a resume agree, and an id with other characters is still refused naming the rule; fails_when=--pass P1 is refused as a missing flag (gstack-evals#191 /ship), a capitalized resume of p1 reads as another pass, or p_1 is accepted; + // why_new=the id was matched case-sensitively and its refusal read as a missing flag; seam=gstack-review-dispatch pass; no_claim=the reviewer plan, which --audit-only skips + test('pass reads its id as lowercase; an id with other characters is still refused with the rule', () => { + const { dir, env } = repo(); + const out = path.join(dir, '.state/run/docs'); + fs.mkdirSync(out, { recursive: true }); + fs.writeFileSync(path.join(out, 'docs.prompt.md'), 'Run Step 14.5 as written.\n'); + const extras = path.join(dir, '.state/extras.json'); + fs.writeFileSync(extras, JSON.stringify([{ name: 'docs', prompt_file: path.join(out, 'docs.prompt.md'), first: true }])); + const args = (id: string) => ['pass', '--base', 'main', '--out', out, '--pass', id, '--audit-only', '--extras', extras]; + const upper = bin(dir, env, args('Docs-P1')); + expect(upper.status, upper.stderr).toBe(0); + expect(upper.stdout).toContain(`MANIFEST: file=${path.join(out, 'pass-manifest.json')} pass=docs-p1 readers=1 audit-only`); + const m = JSON.parse(fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8')); + expect([m.pass, m.readers[0].title]).toEqual(['docs-p1', 'ship docs-p1 audit/docs']); + const resumed = bin(dir, env, args('docs-p1')); + expect([resumed.status, resumed.stdout.includes('pass=docs-p1 readers=1 resumed')]).toEqual([0, true]); + for (const bad of ['p_1', 'p 1', 'p1/']) { + const r = bin(dir, env, args(bad)); + expect([r.status, r.stderr.trim()]).toEqual([2, `gstack-review-dispatch pass: --pass ${JSON.stringify(bad)} is not a pass id: use letters, digits and dashes (read as lowercase, so P1 is p1)`]); + } + }, 120_000); + test('Step 11 notes its own growth from its first dispatch, not from the pass\'s starting reading', async () => { const { dir, env } = repo(); const rec = path.join(dir, '.state/rec.md');