fix(ship): judge passes an empty merge; pass ids read as lowercase; snapshot --docs takes a comma list (GST-1219)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-docs-candidate

@@ -4,7 +4,7 @@
  *
  * Usage (run inside the repository):
  *   gstack-docs-candidate snapshot --out <candidate.json> --audit-id <id> --mode edit|read-only
- *                                  --base <rev> [--select <path>]... [--docs <path>]... [--exception <docs-audit.json>]
+ *                                  --base <rev> [--select <path>]... [--docs <path>[,<path>...]]... [--exception <docs-audit.json>]
  *   gstack-docs-candidate compare <candidate.json>
  *   gstack-docs-candidate attempt <child-output> --state <attempts.json>
  *   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<string, string | null> {
   const out: Record<string, string | null> = {};
@@ -99,7 +114,7 @@ function one(args: string[], name: string): string {
   return values[0]!;
 }
 
-const USAGE = `usage: gstack-docs-candidate snapshot --out <candidate.json> --audit-id <id> --mode edit|read-only --base <rev> [--select <path>]... [--docs <path>]... [--exception <docs-audit.json>]
+const USAGE = `usage: gstack-docs-candidate snapshot --out <candidate.json> --audit-id <id> --mode edit|read-only --base <rev> [--select <path>]... [--docs <path>[,<path>...]]... [--exception <docs-audit.json>]
        gstack-docs-candidate compare <candidate.json>
        gstack-docs-candidate attempt <child-output> --state <attempts.json>`;
 
@@ -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 });

bin/gstack-review-dispatch

@@ -56,7 +56,8 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
   judge --ledger <f> --merged <merged.jsonl> --judged <judged.jsonl>
       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 <f> [--targeted-out <file>]
       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 <out>/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.
+      <id> 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 <ref>, --out <dir> and --pass <id> (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 <ref>, --out <dir> and --pass <id>\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 <file> and --judged <file>\n'); return EXIT.usage; }
-    const r = recordJudgment(ledger, readJsonl<MergedLine>(merged), readJsonl<JudgedLine>(judged), judged, merged);
+    const lines = readJsonl<MergedLine>(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<JudgedLine>(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;

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.

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 <f>` 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 <b> --out <dir> --pass <id> -- <plan flags>` runs one pass from a
-   staged manifest (`<dir>/pass-manifest.json`): audit readers marked first, then
+   staged manifest (`<dir>/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

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');

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', () => {

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');