merge: trial-5 fixes 1, 4 and 7 before v6.5 (decision 59)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

agents/review-coordinator.md

@@ -76,7 +76,11 @@ re-reading, and fresh finding generation are all forbidden.
 ### Pass 1 — Dedup by `(file, line, rule_key)` triplet
 
 Two findings collide when their `(file, line, rule_key)` triplets are
-identical. When findings collide:
+identical, with one exception: **one reviewer never duplicates itself.** Two
+findings from the same reviewer on one triplet under different titles are two
+findings (an engine's catch-all `rule_key` makes this common): keep both rows,
+in input order. A finding from another reviewer joins the row whose title it
+shares, else the first one. When findings collide:
 - Rank the colliding findings and keep the first:
   1. the higher catalogue tier: the severity `lib/review/rule-catalogue.mjs`
      gives the `rule_key`, or the finding's own severity when the key is
@@ -420,8 +424,9 @@ for the ID list.
   where n is its 1-based position in the JSON block's `findings` array.
   `/trekreview` Phase 7 runs `lib/review/fill-finding-ids.mjs`, which
   replaces each placeholder with the id computed over `(file, line,
-  rule_key)` (`lib/parsers/finding-id.mjs`) and refuses a placeholder with
-  no finding behind it.
+  rule_key)` (`lib/parsers/finding-id.mjs`; the k-th row on one triplet
+  gets it over `rule_key#k`) and refuses a placeholder with no finding
+  behind it.
 - **Suppressed findings are accountable.** When you remove a finding via
   Pass 2 or Pass 3, log it in `## Suppressed Findings` with the reason and
   its fate (`[dropped]` / `[unverified]`), and list every `[unverified]` one

commands/trekbrief.md

@@ -820,7 +820,8 @@ If the validator returns errors, report them to the user and offer to
 re-enter Phase 4 with the validator's hints in scope. If only warnings,
 note them in the final report — except `BRIEF_SC_NOT_RUNNABLE`: a success
 criterion whose command `/trekreview` will never run (the product's own
-binary, `jq`, `grep`, a pipe). Name each one to the user in plain words and
+binary, `jq`, `grep`, a pipe, or a command that stands after a flag or path
+in backticks). Name each one to the user in plain words and
 offer to rewrite it as a script under `tests/` that the criterion runs
 (`bash tests/<script>.sh` exits 0) before the brief is approved; if the user
 keeps it, it stays a criterion a human judges, and the report says so. The

commands/trekexecute.md

@@ -1522,8 +1522,21 @@ and the skill to work by: `skill: test-first` for a build round,
 
 **A step at `fix_rounds` 5 is never built again** (Sub-step F): skip the
 launch and run only the checks (B, D1, D2, D3, D4, E) on whatever the operator
-changed. A step at `fix_rounds` 4 on `--resume` was re-planned: launch a fresh
-`build` helper for the revised step.
+changed. A step at `fix_rounds` 4 on `--resume` was re-planned: first park
+the earlier attempt's new files, which the revert left on disk untracked, so a
+stale file can never pass the revised step's checks (voyage-lekerepo trial 5,
+finding 4):
+```bash
+node ${CLAUDE_PLUGIN_ROOT}/lib/util/park-attempt-files.mjs --diff {report dir}/step-{N}-upward.diff --step {N} --attempt {attempts}
+```
+It moves every file the attempt created that is still untracked to the git
+dir (`voyage/parked/step-{N}-attempt-{attempts}/`, outside the work tree, so
+Sub-step C.2 never registers it again): moved, never deleted, never over a file
+already parked. Name the `parked` paths and every `kept` path with its reason
+in the step report. Exit 1 means a file the attempt made is still in the tree
+(a header it could not read, a move that failed): stop and name it; exit 2 is
+a stop too (the tree was not checked). Then launch a fresh `build` helper for
+the revised step.
 
 - **Never pass the Agent tool's `name` parameter.** A named spawn becomes an
   `in_process_teammate`: it does the work, but its result never reaches this
@@ -1839,7 +1852,7 @@ session's effort, Sub-step E).
 | 4 | the plan | **After three failed fix rounds the question is the architecture, not a fourth fix.** The step goes back to planning (below). Sub-step B sends three answers here directly, with no fix round: a `"not-run"` caused by the `Verify:` command, a second `plan-unreadable` (the executor's own call still wrong after one correction), and a `not-red-at-base` with `tests_only: true` (a step of only tests). Stop class 4 |
 | 5 | the operator | A step already sent to the plan (`fix_rounds` 4) that fails its control again after re-planning goes to the operator. The executor never builds this step again: on a later `--resume` it only runs the checks (B, D1, D2, D3, D4, E) on whatever the operator changed. If they fail again the step stays at 5 — there is no sixth round, `fix_rounds` never exceeds 5 — and the run stops again. Stop class 4 |
 
-**Back to planning** (`fix_rounds = 4`): save `git diff {start_sha}` to
+**Back to planning** (`fix_rounds = 4`): save `git diff --no-ext-diff --no-renames --src-prefix=a/ --dst-prefix=b/ {start_sha}` (one form whatever the user's git config) to
 `{report dir}/step-{N}-upward.diff` and write `{report dir}/step-{N}-upward.md`:
 the step verbatim, every round's findings and verdicts, and the question for
 the planner — which assumption in the step (its Changes, its Interface, its
@@ -1862,7 +1875,9 @@ directly (Sub-step B), and so does a second `plan-unreadable` after one correcte
   git rm --cached -q -- {the step's new files: registered with --intent-to-add since start_sha}
   ```
   The new files stay on disk, untracked, and `step-{N}-upward.md` names them —
-  nothing the step made is deleted, and none of it is left registered.
+  nothing the step made is deleted, and none of it is left registered. A
+  `--resume` after re-planning parks them in the git dir before the fresh
+  build (Sub-step C).
   When the guard refuses the restore (it is destructive git: in the permission
   mode that shows no prompts the guard refuses it, exit 2, "no one to ask"), or
   the operator declines it, nothing was restored: write in `step-{N}-upward.md`

docs/HANDOVER-CONTRACTS.md

@@ -100,7 +100,7 @@ Optional but standard sections: `## Non-Goals`, `## Constraints`, `## Preference
 | **v5.1 sequencing gate** | every read | `brief_version ≥ 2.1` requires `phase_signals` (list) OR `phase_signals_partial: true` — error `BRIEF_V51_MISSING_SIGNALS` on miss. Validator-only enforcement; commands surface, don't re-enforce. |
 | **v5.5 framing gate** | every read | `framing` enum-checked on any version when present (`BRIEF_INVALID_FRAMING`); at `brief_version ≥ 2.2` a missing `framing` → `BRIEF_MISSING_FRAMING` and a missing `## TL;DR` → `BRIEF_MISSING_SECTION` (strict) / warning (soft). `trekreview` briefs are exempt. |
 | Body sections | strict only | All `BRIEF_BODY_SECTIONS` present (`## TL;DR` added at ≥ 2.2) |
-| Runnable criteria | every read | A `## Success Criteria` bullet whose first backticked span is a command the review's criteria runner never runs (the product's own binary, `jq`, `grep`, a pipe) → warning `BRIEF_SC_NOT_RUNNABLE`, never an error: a criterion a human judges stays allowed. Prose-only criteria are not flagged. `trekreview` briefs are exempt. No schema change. Indented sub-bullets belong to the criterion above them (counted, never criteria of their own); a criterion with sub-points and no command → warning `BRIEF_SC_SUBPOINTS`. A brief whose Constraints or NFRs require bash 3.2 and a criterion that runs plain `bash` → warning `BRIEF_SC_BASH_NOT_PINNED` (use `/bin/bash`). |
+| Runnable criteria | every read | A `## Success Criteria` bullet whose first backticked span is a command the review's criteria runner never runs (the product's own binary, `jq`, `grep`, a pipe), or whose first span is not a command while a later span is one (the runner reads only the first, voyage-lekerepo trial 5) → warning `BRIEF_SC_NOT_RUNNABLE`, never an error: a criterion a human judges stays allowed. Prose-only criteria, also those that only name a flag or a path, are not flagged. `trekreview` briefs are exempt. No schema change. Indented sub-bullets belong to the criterion above them (counted, never criteria of their own); a criterion with sub-points and no command → warning `BRIEF_SC_SUBPOINTS`. A brief whose Constraints or NFRs require bash 3.2 and a criterion that runs plain `bash` → warning `BRIEF_SC_BASH_NOT_PINNED` (use `/bin/bash`). |
 
 **State machine** detail: a brief that says it has research topics but skipped them must explicitly admit it (via `brief_quality: partial`). This is the most common failure mode the validator catches.
 

lib/parsers/finding-id.mjs

@@ -1,7 +1,8 @@
 // lib/parsers/finding-id.mjs
 // Stable finding-ID for /trekreview v1.0.
 //
-// id = sha1(file:line:rule_key) → 40-char hex.
+// id = sha1(file:line:rule_key) → 40-char hex; a second finding on the same
+// triplet gets it over `rule_key#2` (computeFindingIds).
 // Same input always produces same output (determinism floor SC4).
 // node:crypto is built-in (zero-deps invariant).
 
@@ -43,6 +44,36 @@ export function computeFindingId(filePath, line, ruleKey) {
   return createHash('sha1').update(composite).digest('hex');
 }
 
+/**
+ * The ids of a list of findings, in order. The first finding on a
+ * (file, line, rule_key) gets computeFindingId of it, as always; the k-th
+ * (k >= 2) gets it over `${rule_key}#k`, the form critic-diff.mjs already uses,
+ * so two different findings one reviewer put on one triplet keep a row each
+ * (voyage-lekerepo trial 5, finding 7).
+ * @param {Array<{file: string, line: number|string, rule_key: string}>} findings
+ * @param {(index: number, error: Error) => void} [onError] - when given, a
+ *   finding whose id cannot be computed gets null and is reported here;
+ *   without it the TypeError is thrown
+ * @returns {Array<string|null>} one id per finding
+ */
+export function computeFindingIds(findings, onError) {
+  const seen = new Map();
+  return findings.map((f, i) => {
+    const key = `${f?.file}:${f?.line}:${f?.rule_key}`;
+    const k = (seen.get(key) ?? 0) + 1;
+    seen.set(key, k);
+    try {
+      // A missing rule_key must still throw, never become "undefined#2".
+      const ruleKey = k === 1 || typeof f?.rule_key !== 'string' || f.rule_key === '' ? f?.rule_key : `${f.rule_key}#${k}`;
+      return computeFindingId(f?.file, f?.line, ruleKey);
+    } catch (e) {
+      if (!onError) throw e;
+      onError(i, e);
+      return null;
+    }
+  });
+}
+
 /**
  * Validate a finding-ID's shape (40-char lowercase hex).
  * @param {string} id

lib/review/coordinator-contract.mjs

@@ -12,7 +12,8 @@
 //     contract being mirrored, so only the deterministic "recommended_action is
 //     present-and-non-empty when supplied" half is kept.
 //   - Nothing about ids beyond the triplet: `computeFindingId(file, line,
-//     rule_key)` is the one id. The LLM coordinator writes `@@ID_<n>@@`
+//     rule_key)` is the one id, over `rule_key#k` for the k-th row on one
+//     triplet (computeFindingIds). The LLM coordinator writes `@@ID_<n>@@`
 //     placeholders and lib/review/fill-finding-ids.mjs computes them (the
 //     old doc named a 4-tuple with `title` the code never computed).
 //
@@ -32,13 +33,13 @@
 // classifySuppression). No LLM, no network, no time, no randomness.
 //
 // Reuses: SEVERITY_VALUES / RULE_KEYS / getRule (rule-catalogue.mjs),
-// computeFindingId (finding-id.mjs, triplet), validateFindings
+// computeFindingIds (finding-id.mjs, triplet), validateFindings
 // (findings-schema.mjs). Triplet key format mirrors
 // scripts/bakeoff-armA-merge.mjs `dedupByTriplet`; raised_by provenance mirrors
 // lib/review/plan-review-dedup.mjs.
 
 import { SEVERITY_VALUES, RULE_KEYS, getRule } from './rule-catalogue.mjs';
-import { computeFindingId } from '../parsers/finding-id.mjs';
+import { computeFindingIds } from '../parsers/finding-id.mjs';
 import { validateFindings } from './findings-schema.mjs';
 import { ENGINES } from './engine-findings.mjs';
 import { isMain } from '../util/is-main.mjs';
@@ -259,14 +260,28 @@ export function ingest(reviewerPayloads) {
  * @returns {object[]}
  */
 export function dedupByTriplet(findings) {
-  const groups = new Map();
+  // One reviewer never duplicates itself: its second finding on a triplet under
+  // another title is another finding and opens a group of its own, which gets
+  // its own row and the id over `rule_key#2` (voyage-lekerepo trial 5,
+  // finding 7). Another reviewer's finding joins the group whose title it
+  // shares, else the first one it can join.
+  const tripletGroups = new Map();
   for (const f of findings) {
     const key = tripletKey(f);
-    if (!groups.has(key)) groups.set(key, []);
-    groups.get(key).push(f);
+    if (!tripletGroups.has(key)) tripletGroups.set(key, []);
+    const subs = tripletGroups.get(key);
+    const fits = (g) => g.every((m) => m.reviewer !== f.reviewer || sameTitle(m, f));
+    let sub = subs.find((g) => fits(g) && g.some((m) => sameTitle(m, f) && m.title !== undefined && f.title !== undefined))
+      ?? subs.find(fits);
+    if (!sub) {
+      sub = [];
+      subs.push(sub);
+    }
+    sub.push(f);
   }
+  const groups = [...tripletGroups.values()].flat();
   const out = [];
-  for (const group of groups.values()) {
+  for (const group of groups) {
     // Ranked as the JSDoc above says. A brief-conflict mark is not carried over
     // from a group member to the survivor: when the survivor is unmarked, the
     // group stays unmarked and is judged at its full severity. For a catalogue
@@ -307,17 +322,24 @@ export function dedupByTriplet(findings) {
         detail += `\nAlso flagged by ${o.reviewer ?? 'unknown'}: ${o.title ?? o.rule_key}.`;
       }
     }
-    const id = computeFindingId(survivor.file, survivor.line, survivor.rule_key);
     // When the group raised the survivor, Pass 3 still needs what the survivor
     // itself asserted for `original_severity` (review 08deae68).
     const raised = severityRank(severity) < severityRank(survivor.severity);
     out.push({
-      ...survivor, ...(severity !== undefined ? { severity } : {}), id,
+      ...survivor, ...(severity !== undefined ? { severity } : {}),
       ...(detail !== undefined ? { detail } : {}), raised_by,
       ...(raised ? { pass1_raised_from: survivor.severity } : {}),
     });
   }
-  return out;
+  // One id per row here; runContract counts the written ids again over the
+  // final list (after Pass 2 and 3), the way fill-finding-ids counts review.md.
+  const ids = computeFindingIds(out);
+  return out.map((f, i) => ({ ...f, id: ids[i] }));
+}
+
+function sameTitle(a, b) {
+  if (a.title === undefined || b.title === undefined) return true;
+  return String(a.title).trim().toLowerCase() === String(b.title).trim().toLowerCase();
 }
 
 /**
@@ -578,8 +600,13 @@ export function runContract(reviewerPayloads, options = {}) {
     reason: f.suppressed_reason,
   }));
   // A trimmed finding that Pass 3 later removed is accounted for there, not here.
-  const keptSet = new Set(reasoned.kept.map((f) => tripletKey(f)));
-  const trimmed_count = judged.trimmed.filter((f) => keptSet.has(tripletKey(f))).length;
+  // Matched on Pass 1's id, which is one per row (a triplet can hold two rows).
+  const keptSet = new Set(reasoned.kept.map((f) => f.id));
+  const trimmed_count = judged.trimmed.filter((f) => keptSet.has(f.id)).length;
+  // The ids written are counted over the final list, as fill-finding-ids counts
+  // review.md's JSON: a row whose triplet sibling was set aside gets the plain id.
+  const finalIds = computeFindingIds(reasoned.kept);
+  const kept = reasoned.kept.map((f, i) => ({ ...f, id: finalIds[i] }));
 
   // A reviewer counts as REPORTED only when a payload carrying its name
   // validated. `validateFindings` merely warns on a missing `reviewer`, so a
@@ -611,7 +638,7 @@ export function runContract(reviewerPayloads, options = {}) {
   return {
     verdict,
     counts,
-    findings: reasoned.kept,
+    findings: kept,
     suppressed: [...judgedDropped, ...judgedUnverified, ...reasoned.dropped, ...reasoned.unverified],
     unverified,
     skipped,

lib/review/engine-findings.mjs

@@ -44,7 +44,7 @@
 //   exit 0 = payload on stdout; 1 = input invalid (errors on stderr); 2 = could not run.
 
 import { readFileSync, existsSync } from 'node:fs';
-import { computeFindingId } from '../parsers/finding-id.mjs';
+import { computeFindingIds } from '../parsers/finding-id.mjs';
 import { RULE_KEYS, getRule, SEVERITY_VALUES } from './rule-catalogue.mjs';
 import { validateFindings } from './findings-schema.mjs';
 import { isMain } from '../util/is-main.mjs';
@@ -117,7 +117,7 @@ export function toReviewerPayload(input, opts = {}) {
     const fix = String(f.fix ?? '').trim();
     const loc = line > 0 ? `${file}:${line}` : file;
     const out = {
-      id: computeFindingId(file, line, rule_key),
+      id: null,
       severity: getRule(rule_key).severity,
       rule_key,
       file,
@@ -140,6 +140,10 @@ export function toReviewerPayload(input, opts = {}) {
   });
   if (errors.length > 0) return { ok: false, errors };
 
+  // Two defects the engine put on one line under one key keep an id each, the
+  // second over `rule_key#2` (computeFindingIds; voyage-lekerepo trial 5, finding 7).
+  const ids = computeFindingIds([...findings, ...out_of_scope]);
+  [...findings, ...out_of_scope].forEach((f, i) => { f.id = ids[i]; });
   const payload = { reviewer: spec.reviewer, findings };
   const v = validateFindings(payload);
   if (!v.valid) return { ok: false, errors: v.errors.map((e) => `${e.code}: ${e.message}`) };

lib/review/fidelity-diff.mjs

@@ -14,7 +14,7 @@
 
 import { parseDocument } from '../util/frontmatter.mjs';
 import { jaccardSimilarity } from '../parsers/jaccard.mjs';
-import { computeFindingId } from '../parsers/finding-id.mjs';
+import { computeFindingIds } from '../parsers/finding-id.mjs';
 import { extractFindingsBlock } from './findings-schema.mjs';
 import { isMain } from '../util/is-main.mjs';
 
@@ -126,14 +126,13 @@ export function normalizeArmOutput(arm) {
   const findings = (arm && Array.isArray(arm.findings)) ? arm.findings : [];
   const findingIds = [];
   const details = [];
-  for (const f of findings) {
+  // The k-th row on one triplet gets the id over `rule_key#k`, as review.md does.
+  const ids = computeFindingIds(findings.map((f) => ({ file: f.file, line: f.line, rule_key: f.rule_key ?? f.rule ?? null })), () => {});
+  for (const [i, f] of findings.entries()) {
     const file = f.file;
     const rule_key = f.rule_key ?? f.rule ?? null;
     const line = f.line;
-    let id = null;
-    if (typeof file === 'string' && file.length > 0 && rule_key && line !== null && line !== undefined) {
-      try { id = computeFindingId(file, line, rule_key); } catch { id = null; }
-    }
+    const id = ids[i];
     if (id) findingIds.push(id);
     details.push({ id, severity: f.severity ?? null, rule_key, file: file ?? null, line: line ?? null });
   }

lib/review/fill-finding-ids.mjs

@@ -7,7 +7,8 @@
 // step no document named. Now the contract says it: the coordinator writes
 // `@@ID_<n>@@` wherever finding n's id belongs (n = 1-based position in the
 // trailing JSON block's `findings` array), and this tool replaces every
-// placeholder with computeFindingId(file, line, rule_key) of that finding.
+// placeholder with computeFindingId(file, line, rule_key) of that finding
+// (`rule_key#k` for the k-th finding on one triplet: computeFindingIds).
 //
 // Fail-closed: a placeholder with no JSON finding behind it, or a 40-hex id
 // in the JSON block that is not the computed one, is an error -- never a guess.
@@ -19,7 +20,7 @@
 //   2 = could not run (unreadable file, no JSON block).
 
 import { readFileSync, writeFileSync, renameSync } from 'node:fs';
-import { computeFindingId } from '../parsers/finding-id.mjs';
+import { computeFindingIds } from '../parsers/finding-id.mjs';
 import { extractFindingsBlock } from './findings-schema.mjs';
 import { isMain } from '../util/is-main.mjs';
 
@@ -41,13 +42,8 @@ export function fillFindingIds(text) {
   }
   const findings = Array.isArray(parsed?.findings) ? parsed.findings : [];
   const errors = [];
-  const ids = findings.map((f, i) => {
-    try {
-      return computeFindingId(f.file, f.line, f.rule_key);
-    } catch (e) {
-      errors.push(`findings[${i}]: cannot compute an id (${e.message})`);
-      return null;
-    }
+  const ids = computeFindingIds(findings, (i, e) => {
+    errors.push(`findings[${i}]: cannot compute an id (${e.message})`);
   });
   findings.forEach((f, i) => {
     if (typeof f.id === 'string' && HEX40.test(f.id) && ids[i] && f.id !== ids[i]) {

lib/util/park-attempt-files.mjs

@@ -0,0 +1,137 @@
+#!/usr/bin/env node
+// lib/util/park-attempt-files.mjs
+// A step sent back to the plan (`fix_rounds` 4) is reverted with its new files
+// left on disk, untracked: nothing the step made is deleted. On --resume the
+// re-planned step is built fresh, but the stale files were still there, and in
+// voyage-lekerepo trial 5 (finding 4) an old tests/batch.sh holding the
+// rejected line would have passed Verify, the manifest and the success
+// criterion. Only the plan critic caught it.
+//
+// Before the fresh build the executor parks them: every file the attempt
+// created (a `new file mode` entry in step-{N}-upward.diff) that is still
+// untracked on disk moves to <git dir>/voyage/parked/step-<N>-attempt-<k>/
+// under its own path. The git dir, like the test lock's, is outside the work
+// tree, so a parked file never comes back as a new file of the next build.
+// Moved, never deleted, and never over a file already parked there.
+//
+// Usage:
+//   node lib/util/park-attempt-files.mjs --diff <step-N-upward.diff> --step <N> --attempt <k> [--cwd <repo>]
+// Prints {"parked": [paths], "kept": [{"path", "reason", "blocking"}], "to": dir}.
+// Exit: 0 = no file the attempt made is left in the tree; 1 = one still is
+// (a header it could not read, a move that failed, a target already taken):
+// the run stops; 2 = could not run (bad usage, unreadable diff, not a
+// repository). Neither 1 nor 2 is ever "the tree is clean".
+
+import { readFileSync, mkdirSync, renameSync, lstatSync } from 'node:fs';
+import { join, dirname, resolve, relative, isAbsolute } from 'node:path';
+import { spawnSync } from 'node:child_process';
+import { isMain } from './is-main.mjs';
+
+/**
+ * What a unified git diff creates: `paths` read from a `diff --git a/X b/X`
+ * header followed by `new file mode`, and `unread`, the raw headers of new
+ * files whose header is not that form (another prefix, a quoted path).
+ */
+export function newFilesInDiff(diffText) {
+  const paths = [];
+  const unread = [];
+  let header = null;
+  for (const line of String(diffText).split('\n')) {
+    if (line.startsWith('diff --git ')) {
+      header = line;
+      continue;
+    }
+    if (header !== null && /^new file mode \d+$/.test(line)) {
+      // Both names are the same for a new file, so a space in it is read too.
+      const m = header.match(/^diff --git a\/(.+) b\/\1$/);
+      if (m) paths.push(m[1]);
+      else unread.push(header);
+      header = null;
+    }
+  }
+  return { paths, unread };
+}
+
+function lexists(abs) {
+  try {
+    lstatSync(abs);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * @param {{ diffText: string, step: string|number, attempt: string|number, cwd?: string }} args
+ * @returns {{ parked: string[], kept: Array<{path: string, reason: string, blocking: boolean}>, to: string }}
+ */
+export function parkAttemptFiles({ diffText, step, attempt, cwd = process.cwd() }) {
+  const root = resolve(cwd);
+  const gitDir = spawnSync('git', ['rev-parse', '--absolute-git-dir'], { cwd: root, encoding: 'utf8' }).stdout.trim();
+  const to = join(gitDir, 'voyage', 'parked', `step-${step}-attempt-${attempt}`);
+  const trackedPaths = new Set(
+    spawnSync('git', ['ls-files', '-z'], { cwd: root, encoding: 'utf8' }).stdout.split('\0').filter(Boolean),
+  );
+  const { paths, unread } = newFilesInDiff(diffText);
+  const parked = [];
+  const kept = unread.map((h) => ({ path: h, reason: 'diff header not read: move this file out by hand', blocking: true }));
+  for (const path of paths) {
+    const abs = resolve(root, path);
+    const rel = relative(root, abs);
+    if (isAbsolute(path) || rel === '' || rel.startsWith('..') || isAbsolute(rel) || rel.split(/[\\/]/)[0] === '.git') {
+      kept.push({ path, reason: 'outside the repository: refused', blocking: false });
+      continue;
+    }
+    if (!lexists(abs)) {
+      kept.push({ path, reason: 'not on disk: nothing to park', blocking: false });
+      continue;
+    }
+    if (trackedPaths.has(rel.split('\\').join('/'))) {
+      kept.push({ path, reason: 'tracked since the attempt: not the executor\'s to move', blocking: false });
+      continue;
+    }
+    const target = join(to, rel);
+    if (lexists(target)) {
+      kept.push({ path, reason: `already parked at ${target}: never overwritten, move it by hand`, blocking: true });
+      continue;
+    }
+    try {
+      mkdirSync(dirname(target), { recursive: true });
+      renameSync(abs, target);
+      parked.push(rel);
+    } catch (e) {
+      kept.push({ path, reason: `could not move: ${e.message}`, blocking: true });
+    }
+  }
+  return { parked, kept, to };
+}
+
+if (isMain(import.meta.url)) {
+  const argv = process.argv.slice(2);
+  const opt = (name) => {
+    const i = argv.indexOf(name);
+    return i === -1 ? undefined : argv[i + 1];
+  };
+  const diff = opt('--diff');
+  const step = opt('--step');
+  const attempt = opt('--attempt');
+  const cwd = opt('--cwd') ?? process.cwd();
+  if (!diff || !/^[0-9]+$/.test(step ?? '') || !/^[0-9]+$/.test(attempt ?? '')) {
+    process.stderr.write('Usage: park-attempt-files.mjs --diff <step-N-upward.diff> --step <N> --attempt <k> [--cwd <repo>]\n');
+    process.exit(2);
+  }
+  let diffText;
+  try {
+    diffText = readFileSync(diff, 'utf8');
+  } catch (e) {
+    process.stderr.write(`park-attempt-files: cannot read ${diff}: ${e.message}\n`);
+    process.exit(2);
+  }
+  if (spawnSync('git', ['rev-parse', '--git-dir'], { cwd, encoding: 'utf8' }).status !== 0) {
+    process.stderr.write(`park-attempt-files: ${cwd} is not a git repository\n`);
+    process.exit(2);
+  }
+  const result = parkAttemptFiles({ diffText, step, attempt, cwd });
+  process.stdout.write(JSON.stringify(result, null, 2) + '\n');
+  process.exit(result.kept.some((k) => k.blocking) ? 1 : 0);
+}

lib/validators/brief-validator.mjs

@@ -10,7 +10,7 @@ import { readFileSync, existsSync } from 'node:fs';
 import { parseDocument } from '../util/frontmatter.mjs';
 import { issue, ok, fail } from '../util/result.mjs';
 import { BASE_ALLOWED_MODELS } from './profile-validator.mjs';
-import { parseSuccessCriteria, runnableCheck } from '../verification/criteria-runner.mjs';
+import { parseSuccessCriteria, runnableCheck, firstCommand } from '../verification/criteria-runner.mjs';
 import { isMain } from '../util/is-main.mjs';
 
 export const BRIEF_REQUIRED_FRONTMATTER = ['type', 'brief_version', 'task', 'slug', 'research_topics', 'research_status'];
@@ -286,6 +286,23 @@ export function validateBriefContent(text, opts = {}) {
             'or write each sub-point as its own criterion with its own command.',
         ));
       }
+      // A criterion whose first backticked span is not a command but whose
+      // later span is one (`--stdin` named before `/bin/bash tests/contract.sh`):
+      // the runner reads only the first span and reports it unrunnable. A
+      // criterion that only names a flag or a path stays prose and unflagged
+      // (HANDOVER-CONTRACTS Handover 1; voyage-lekerepo trial 5, finding 1).
+      if (!c.command && c.reason === 'not-a-command' && !c.subpoints) {
+        const spans = [...c.text.matchAll(/`([^`]+)`/g)].map((m) => m[1]);
+        const later = spans.slice(1).find((span) => firstCommand(`\`${span}\``).command);
+        if (later !== undefined) {
+          warnings.push(issue(
+            'BRIEF_SC_NOT_RUNNABLE',
+            `Success criterion ${c.label}'s first backticked span is \`${spans[0]}\`, not a command, so the review's criteria runner never reaches \`${later}\` (not-a-command)`,
+            'The runner runs the first backticked span: put the command first and name anything else in plain words ' +
+              '(voyage:writing-criteria), or it is reported NOT RUN at review.',
+          ));
+        }
+      }
       if (!c.command) continue;
       const bare = c.command.replace(/^(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)+/, '');
       if (needsBash32 && /^bash\s/.test(bare)) {

scripts/trekreview-armB.workflow.mjs

@@ -126,16 +126,33 @@ const VERDICT_SCHEMA = {
 // the workflow has no conformance rule, so otherwise the first-seen one stays.
 // The survivor carries the highest severity any finding in its group asserted,
 // so an engine's mark never lowers what a reviewer agent asserted.
-function dedupByTriplet(findings) {
-  const groups = new Map();
+// As in coordinator-contract Pass 1, one source never duplicates itself: its
+// second finding on a triplet under another title opens a group of its own
+// (voyage-lekerepo trial 5, finding 7). `sourceOf` names the reviewer or engine.
+const sameTitle = (a, b) => a.title === undefined || b.title === undefined ||
+  String(a.title).trim().toLowerCase() === String(b.title).trim().toLowerCase();
+const sourceOf = new Map();
+function dedupByTriplet(findings, { perSource = true } = {}) {
+  const byTriplet = new Map();
   for (const f of findings) {
     const key = tripletOf(f);
-    if (!groups.has(key)) groups.set(key, []);
-    groups.get(key).push(f);
+    if (!byTriplet.has(key)) byTriplet.set(key, []);
+    const subs = byTriplet.get(key);
+    const src = sourceOf.get(f) ?? null;
+    const fits = (g) => g.every((m) => (sourceOf.get(m) ?? null) !== src || sameTitle(m, f));
+    let sub = !perSource ? subs[0]
+      : subs.find((g) => fits(g) && g.some((m) => m.title !== undefined && f.title !== undefined && sameTitle(m, f)))
+        ?? subs.find(fits);
+    if (!sub) {
+      sub = [];
+      subs.push(sub);
+    }
+    sub.push(f);
   }
+  const groups = [...byTriplet.values()].flat();
   const tier = (f) => sevRank(tierOf(f.rule_key) ?? f.severity);
   const out = [];
-  for (const group of groups.values()) {
+  for (const group of groups) {
     let survivor = group[0];
     for (const f of group.slice(1)) {
       const tf = tier(f);
@@ -248,9 +265,15 @@ const withEngine = (f, eng) => {
   const { engine: _dropped, ...rest } = f;
   return eng ? { ...rest, engine: eng } : rest;
 };
+const tagged = (f, src) => {
+  if (f && typeof f === 'object') sourceOf.set(f, src);
+  return f;
+};
 const allFindings = [
-  ...reviewerResults.flatMap((r) => (Array.isArray(r?.findings) ? r.findings : []).map((f) => withEngine(f, null))),
-  ...enginePayloads.flatMap((p) => (Array.isArray(p?.findings) ? p.findings : []).map((f) => withEngine(f, engineOf(p?.reviewer)))),
+  ...reviewerResults.flatMap((r, i) => (Array.isArray(r?.findings) ? r.findings : [])
+    .map((f) => tagged(withEngine(f, null), `reviewer:${r?.reviewer ?? i}`))),
+  ...enginePayloads.flatMap((p) => (Array.isArray(p?.findings) ? p.findings : [])
+    .map((f) => tagged(withEngine(f, engineOf(p?.reviewer)), `engine:${p?.reviewer}`))),
 ];
 const merged = dedupByTriplet(allFindings);
 log(`Arm B: ${reviewerResults.length} reviewer(s); ${allFindings.length} raw → ${merged.length} after triplet-dedup`);
@@ -333,9 +356,10 @@ for (const f of returned) {
 }
 // (c) the put-back (decision 23).
 const returnedTriplets = new Set(returned.filter((f) => f && typeof f === 'object').map(tripletOf));
-// Engine findings only, so dedupByTriplet keeps the higher severity, else the first seen.
+// Engine findings only, so dedupByTriplet keeps the higher severity, else the
+// first seen: one copy per triplet (decision 23), so not split per source here.
 const putBack = dedupByTriplet(allFindings.filter((f) =>
-  f && typeof f === 'object' && f.engine && !returnedTriplets.has(tripletOf(f))));
+  f && typeof f === 'object' && f.engine && !returnedTriplets.has(tripletOf(f))), { perSource: false });
 for (const f of putBack) {
   const { brief_lowered_from: _lowered, ...copy } = f;
   returned.push(copy);

tests/fixtures/jsonl-schemas.md

@@ -19,10 +19,10 @@
 
 | schema_id | fields | writer_path | line_ref | v4.1 additive | PII |
 |-----------|--------|-------------|----------|---------------|-----|
-| trekbrief-stats | ts, task, slug, mode, interview_turns, review_iterations, brief_quality, research_topics, auto_research, auto_result, intent_approved, project_dir, order_id, order_claimed_at, order_reason | commands/trekbrief.md (orchestrator-emit Phase 7; order_* added by lib/stats/stats-append.mjs) | trekbrief.md:1127-1151 | profile, phase_models, profile_source | none |
+| trekbrief-stats | ts, task, slug, mode, interview_turns, review_iterations, brief_quality, research_topics, auto_research, auto_result, intent_approved, project_dir, order_id, order_claimed_at, order_reason | commands/trekbrief.md (orchestrator-emit Phase 7; order_* added by lib/stats/stats-append.mjs) | trekbrief.md:1128-1152 | profile, phase_models, profile_source | none |
 | trekresearch-stats | ts, question, mode, scope, engine, slug, project_dir, brief_path, dimensions, dimensions_baseline, dimensions_baseline_preserved, effort, conv_turns, empty_turns, unique_sources, agents_local, agents_external, gemini_used, confidence, contradictions, open_questions | lib/stats/stats-append.mjs (trekresearch.md Stats tracking) | trekresearch.md:782-808 | profile, phase_models, parallel_agents, external_research_enabled, profile_source | none |
 | trekplan-stats | ts, task, mode, slug, brief_path, project_dir, codebase_size, codebase_files, agents_deployed, deep_dives, research_briefs_used, research_scout_used, critic_verdict, guardian_verdict, outcome | lib/stats/stats-append.mjs (trekplan.md Phase 12) | trekplan.md:1277-1305 | profile, phase_models, parallel_agents, profile_source | none |
-| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2892-2914 | profile, phase_models, profile_source | none |
+| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2907-2929 | profile, phase_models, profile_source | none |
 | trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:78-99 | payload.profile, payload.phase_models, payload.profile_source | none |
 | trekexecute-stats (PostToolUse Bash) | ts, session_id, command_excerpt, duration_ms, success | hooks/scripts/post-bash-stats.mjs (Bash PostToolUse) | post-bash-stats.mjs:42-54 | none (hook is plugin-level, not profile-aware) | command_excerpt (CWE-212) |
 | trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | lib/stats/stats-append.mjs (trekreview.md Phase 8) | trekreview.md:636-686 | profile, phase_models, profile_source | none |

tests/lib/coordinator-contract.test.mjs

@@ -9,6 +9,7 @@ import { strict as assert } from 'node:assert';
 import { readFileSync } from 'node:fs';
 import { join, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
+import { computeFindingId } from '../../lib/parsers/finding-id.mjs';
 import {
   severityRank,
   ingest,
@@ -58,6 +59,21 @@ test('dedupByTriplet — survivor is the highest-severity finding in the group',
   assert.equal(out[0].severity, 'BLOCKER');
 });
 
+test('dedupByTriplet — two DIFFERENT findings from one reviewer on one triplet stay two rows with their own ids (trial 5, finding 7)', () => {
+  // /code-review gave both the catch-all key at bin/slug.mjs:46; one merged row lost the second finding's action.
+  const findings = [
+    { file: 'bin/slug.mjs', line: 46, rule_key: 'CODE_REVIEW_DEFECT', severity: 'MAJOR', title: 'EAGAIN on a non-blocking stdin', recommended_action: 'Read stdin with a stream.', reviewer: 'claude-code-review' },
+    { file: 'bin/slug.mjs', line: 46, rule_key: 'CODE_REVIEW_DEFECT', severity: 'MAJOR', title: 'A lone \\r does not split lines', recommended_action: 'Split on \\r too.', reviewer: 'claude-code-review' },
+    { file: 'bin/slug.mjs', line: 46, rule_key: 'CODE_REVIEW_DEFECT', severity: 'MAJOR', title: 'EAGAIN on a non-blocking stdin', recommended_action: 'Read stdin with a stream.', reviewer: 'code-correctness-reviewer' },
+  ];
+  const out = dedupByTriplet(findings);
+  assert.equal(out.length, 2, JSON.stringify(out.map((f) => f.title)));
+  assert.deepEqual(out.map((f) => f.recommended_action), ['Read stdin with a stream.', 'Split on \\r too.']);
+  assert.equal(out[0].id, computeFindingId('bin/slug.mjs', 46, 'CODE_REVIEW_DEFECT'));
+  assert.equal(out[1].id, computeFindingId('bin/slug.mjs', 46, 'CODE_REVIEW_DEFECT#2'));
+  assert.deepEqual([...out[0].raised_by].sort(), ['claude-code-review', 'code-correctness-reviewer']);
+});
+
 test('dedupByTriplet — severity tie breaks toward the conformance reviewer', () => {
   const findings = [
     { file: 'x.mjs', line: 1, rule_key: 'NON_GOAL_VIOLATED', severity: 'BLOCKER', title: 'corr', reviewer: 'correctness' },
@@ -403,3 +419,23 @@ test('runContract — a BLOCKER with a 900-character detail survives and yields
   assert.equal(result.findings[0].detail, detail, 'its detail is not cut');
   assert.equal(result.unverified.length, 0);
 });
+
+test('the coordinator prose says one reviewer never duplicates itself, as dedupByTriplet does (trial 5, finding 7)', () => {
+  const prose = readFileSync(join(ROOT, 'agents/review-coordinator.md'), 'utf8');
+  const pass1 = prose.slice(prose.indexOf('### Pass 1'), prose.indexOf('### Pass 2'));
+  assert.match(pass1, /one reviewer never duplicates itself/i);
+  assert.match(pass1, /different titles are two\s+findings/);
+  assert.match(prose, /k-th row on one triplet\s+gets it over `rule_key#k`/);
+});
+
+test('runContract — the #k id counts rows of one triplet in the final list, so a row whose sibling was set aside gets the plain id (review of b47cabd)', () => {
+  const long = 'x'.repeat(120);
+  const result = runContract([{ reviewer: 'claude-code-review', findings: [
+    { file: 'bin/slug.mjs', line: 46, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: `EAGAIN ${long}`, detail: 'd', recommended_action: '' },
+    { file: 'bin/slug.mjs', line: 46, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: `Lone CR ${long}`, detail: 'd', recommended_action: 'Split on CR too.' },
+  ] }]);
+  assert.equal(result.findings.length, 1, JSON.stringify(result.findings.map((f) => f.title)));
+  assert.match(result.findings[0].title, /^Lone CR/);
+  assert.equal(result.findings[0].id, computeFindingId('bin/slug.mjs', 46, 'MISSING_ERROR_HANDLING'), 'what fill-finding-ids computes for a review.md holding this row alone');
+  assert.equal(result.trimmed_count, 1, 'the trimmed row that Pass 2 set aside is not counted');
+});

tests/lib/fidelity-diff.test.mjs

@@ -164,3 +164,12 @@ test('fidelityDiffStructured — same triplet, different severity → severity m
   assert.equal(d.severityMismatches.length, 1);
   assert.equal(d.equivalent, false);
 });
+
+test('normalizeArmOutput — two rows on one triplet keep two ids, the second over #2, as review.md writes them (review of b47cabd)', () => {
+  const row = { file: 'bin/slug.mjs', line: 46, rule_key: 'CODE_REVIEW_DEFECT', severity: 'MAJOR' };
+  const n = normalizeArmOutput({ verdict: 'WARN', findings: [row, { ...row }] });
+  assert.deepEqual(n.findingIds, [
+    computeFindingId('bin/slug.mjs', 46, 'CODE_REVIEW_DEFECT'),
+    computeFindingId('bin/slug.mjs', 46, 'CODE_REVIEW_DEFECT#2'),
+  ]);
+});

tests/lib/park-attempt-files.test.mjs

@@ -0,0 +1,196 @@
+// tests/lib/park-attempt-files.test.mjs
+// A step sent back to the plan leaves its new files on disk; before the fresh
+// build of the re-planned step they are moved into the report dir, never
+// deleted (voyage-lekerepo trial 5, finding 4).
+
+import { test } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, symlinkSync } from 'node:fs';
+import { join, dirname } from 'node:path';
+import { tmpdir } from 'node:os';
+import { fileURLToPath } from 'node:url';
+import { spawnSync } from 'node:child_process';
+
+const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
+const CLI = join(ROOT, 'lib/util/park-attempt-files.mjs');
+
+function git(cwd, ...args) {
+  const r = spawnSync('git', args, { cwd, encoding: 'utf8' });
+  assert.equal(r.status, 0, r.stderr);
+  return r.stdout;
+}
+
+function repo() {
+  const dir = mkdtempSync(join(tmpdir(), 'voyage-park-'));
+  git(dir, 'init', '-q');
+  git(dir, 'config', 'user.email', 't@example.invalid');
+  git(dir, 'config', 'user.name', 't');
+  mkdirSync(join(dir, 'bin'));
+  writeFileSync(join(dir, 'bin/slug.mjs'), 'old\n');
+  git(dir, 'add', 'bin/slug.mjs');
+  git(dir, 'commit', '-q', '-m', 'base');
+  return dir;
+}
+
+// The shape `git diff {start_sha}` gives with the new files registered --intent-to-add.
+const DIFF = [
+  'diff --git a/bin/slug.mjs b/bin/slug.mjs',
+  'index 3367afd..e69de29 100644',
+  '--- a/bin/slug.mjs',
+  '+++ b/bin/slug.mjs',
+  '@@ -1 +1 @@',
+  '-old',
+  '+new',
+  'diff --git a/tests/batch.sh b/tests/batch.sh',
+  'new file mode 100755',
+  'index 0000000..1111111',
+  '--- /dev/null',
+  '+++ b/tests/batch.sh',
+  '@@ -0,0 +1 @@',
+  '+cat',
+  'diff --git a/tests/cli-stdin.test.mjs b/tests/cli-stdin.test.mjs',
+  'new file mode 100644',
+  '--- /dev/null',
+  '+++ b/tests/cli-stdin.test.mjs',
+  '@@ -0,0 +1 @@',
+  '+t',
+  '',
+].join('\n');
+
+function run(dir, diff = DIFF) {
+  writeFileSync(join(dir, '.git', 'upward.diff'), diff);
+  return spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', dir], { encoding: 'utf8' });
+}
+
+// Parked outside the work tree, in the git dir, so a later `git ls-files --others` never lists them.
+const parkedPath = (dir, path) => join(dir, '.git', 'voyage', 'parked', 'step-1-attempt-1', path);
+
+test('park: the step\'s untracked new files move into the git dir with their paths; tracked files stay', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    writeFileSync(join(dir, 'tests/batch.sh'), 'stale | cat\n');
+    writeFileSync(join(dir, 'tests/cli-stdin.test.mjs'), 'stale test\n');
+    writeFileSync(join(dir, 'bin/slug.mjs'), 'new\n');
+    const r = run(dir);
+    assert.equal(r.status, 0, r.stderr);
+    const out = JSON.parse(r.stdout);
+    assert.deepEqual(out.parked, ['tests/batch.sh', 'tests/cli-stdin.test.mjs']);
+    assert.equal(existsSync(join(dir, 'tests/batch.sh')), false, 'the stale file is out of the tree');
+    assert.equal(readFileSync(parkedPath(dir, 'tests/batch.sh'), 'utf8'), 'stale | cat\n', 'and kept, never deleted');
+    assert.equal(git(dir, 'ls-files', '--others', '--exclude-standard').trim(), '', 'nothing parked shows up as a new file');
+    assert.equal(readFileSync(join(dir, 'bin/slug.mjs'), 'utf8'), 'new\n', 'a tracked file is not the helper\'s to move');
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: a new file that is tracked now, or already gone, is kept and named, never moved', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    writeFileSync(join(dir, 'tests/batch.sh'), 'committed by the operator\n');
+    git(dir, 'add', 'tests/batch.sh');
+    git(dir, 'commit', '-q', '-m', 'operator');
+    const r = run(dir);
+    assert.equal(r.status, 0, r.stderr);
+    const out = JSON.parse(r.stdout);
+    assert.deepEqual(out.parked, []);
+    assert.deepEqual(out.kept.map((k) => k.path), ['tests/batch.sh', 'tests/cli-stdin.test.mjs']);
+    assert.match(out.kept[0].reason, /tracked/);
+    assert.match(out.kept[1].reason, /not on disk/);
+    assert.equal(existsSync(join(dir, 'tests/batch.sh')), true);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: a path that leaves the repository is refused, never moved', () => {
+  const dir = repo();
+  try {
+    const r = run(dir, DIFF.replaceAll('tests/batch.sh', '../outside.sh'));
+    assert.equal(r.status, 0, r.stderr);
+    const out = JSON.parse(r.stdout);
+    assert.ok(out.kept.some((k) => k.path === '../outside.sh' && /outside the repository/.test(k.reason)), r.stdout);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: a path with a space is read and parked', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    writeFileSync(join(dir, 'tests/my batch.sh'), 'stale\n');
+    const r = run(dir, DIFF.replaceAll('tests/batch.sh', 'tests/my batch.sh'));
+    assert.equal(r.status, 0, r.stderr);
+    assert.ok(JSON.parse(r.stdout).parked.includes('tests/my batch.sh'), r.stdout);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: a new file under a header it cannot read is named, and exit 1 stops the run (review of 3238327)', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    writeFileSync(join(dir, 'tests/batch.sh'), 'stale\n');
+    const r = run(dir, DIFF.replace('diff --git a/tests/batch.sh b/tests/batch.sh', 'diff --git c/tests/batch.sh w/tests/batch.sh'));
+    assert.equal(r.status, 1, r.stdout);
+    const out = JSON.parse(r.stdout);
+    assert.ok(out.kept.some((k) => /header not read/.test(k.reason)), r.stdout);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: an attempt file still on disk because its target exists is exit 1, and the parked copy is never overwritten', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    writeFileSync(join(dir, 'tests/batch.sh'), 'second\n');
+    mkdirSync(dirname(parkedPath(dir, 'tests/batch.sh')), { recursive: true });
+    writeFileSync(parkedPath(dir, 'tests/batch.sh'), 'first\n');
+    const r = run(dir);
+    assert.equal(r.status, 1, r.stdout);
+    assert.equal(readFileSync(parkedPath(dir, 'tests/batch.sh'), 'utf8'), 'first\n');
+    assert.equal(existsSync(join(dir, 'tests/batch.sh')), true);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: a symlink the attempt made is parked, not called absent', () => {
+  const dir = repo();
+  try {
+    mkdirSync(join(dir, 'tests'));
+    symlinkSync('nowhere', join(dir, 'tests/batch.sh'));
+    const r = run(dir, DIFF.replace('new file mode 100755', 'new file mode 120000'));
+    assert.equal(r.status, 0, r.stdout);
+    assert.ok(JSON.parse(r.stdout).parked.includes('tests/batch.sh'), r.stdout);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('park: an unreadable diff or missing arguments exit 2 and move nothing', () => {
+  const dir = repo();
+  try {
+    const r = spawnSync(process.execPath, [CLI, '--diff', join(dir, 'nope.diff'), '--step', '1', '--attempt', '1', '--cwd', dir], { encoding: 'utf8' });
+    assert.equal(r.status, 2);
+    assert.equal(spawnSync(process.execPath, [CLI], { encoding: 'utf8' }).status, 2);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('trekexecute: a re-planned step parks the earlier attempt\'s new files before the fresh build', () => {
+  const md = readFileSync(join(ROOT, 'commands/trekexecute.md'), 'utf8');
+  const at = md.indexOf('A step at `fix_rounds` 4 on `--resume` was re-planned');
+  assert.ok(at > 0);
+  const para = md.slice(at, at + 1600);
+  assert.match(para, /lib\/util\/park-attempt-files\.mjs/);
+  assert.match(para, /step-\{N\}-upward\.diff/);
+  assert.match(para, /never deleted/);
+  assert.match(para, /Exit 1 means a file the attempt made is still in the tree/);
+});

tests/lib/review-findings-kept.test.mjs

@@ -145,6 +145,17 @@ test('fill-finding-ids: every @@ID_n@@ becomes the id computed over (file, line,
   assert.match(r.text, new RegExp(`"id": "${id1}"`));
 });
 
+test('fill-finding-ids: a second finding on the same (file, line, rule_key) gets its own id, `#2` on the rule (trial 5, finding 7)', () => {
+  const G = { ...F1, title: 'A lone \\r does not split lines' };
+  const r = fillFindingIds(review([F1, G]));
+  assert.equal(r.ok, true, r.errors.join('\n'));
+  const id1 = computeFindingId(F1.file, F1.line, F1.rule_key);
+  const id2 = computeFindingId(F1.file, F1.line, `${F1.rule_key}#2`);
+  assert.notEqual(id1, id2);
+  assert.match(r.text, new RegExp(`findings:\\n  - ${id1}\\n  - ${id2}\\n`));
+  assert.equal(fillFindingIds(r.text).ok, true, 'the filled review checks out again');
+});
+
 test('fill-finding-ids: a placeholder with no JSON finding behind it is an error, not a guess', () => {
   const r = fillFindingIds(review([F1], 'See also @@ID_7@@.'));
   assert.equal(r.ok, false);
@@ -186,3 +197,17 @@ test('the coordinator writes @@ID_n@@ placeholders and the doc names the three-f
   assert.match(doc, /@@ID_<n>@@/);
   assert.match(read('commands/trekreview.md'), /fill-finding-ids\.mjs/);
 });
+
+test('engine findings: two different defects on one line under the catch-all key get two ids, the second over #2 (review of b47cabd)', () => {
+  const r = toReviewerPayload({
+    engine: 'code-review',
+    findings: [
+      { file: 'bin/slug.mjs', line: 46, summary: 'EAGAIN on a non-blocking stdin' },
+      { file: 'bin/slug.mjs', line: 46, summary: 'A lone CR does not split lines' },
+    ],
+  });
+  assert.equal(r.ok, true, r.errors?.join('\n'));
+  const [a, b] = r.payload.findings;
+  assert.equal(a.id, computeFindingId('bin/slug.mjs', 46, a.rule_key));
+  assert.equal(b.id, computeFindingId('bin/slug.mjs', 46, `${b.rule_key}#2`));
+});

tests/scripts/trekreview-armb-workflow.test.mjs

@@ -627,3 +627,13 @@ test('decision 23 — Handover 6 states the dedup rank on both paths, the put-ba
   assert.match(h6, PUT_BACK);
   assert.match(h6, FLOOR_B);
 });
+
+test('armB — two different /code-review findings on one triplet stay two rows for the coordinator (trial 5, finding 7)', async () => {
+  const c = await floorCase({
+    codeFindings: [F('CODE_REVIEW_DEFECT', 'MAJOR', { title: 'EAGAIN on stdin' }), F('CODE_REVIEW_DEFECT', 'MAJOR', { title: 'A lone CR does not split lines' })],
+    reviewerFindings: [F('CODE_REVIEW_DEFECT', 'MAJOR', { title: 'EAGAIN on stdin' })],
+    returned: [F('CODE_REVIEW_DEFECT', 'MAJOR')],
+    verdict: 'WARN',
+  });
+  assert.deepEqual(c.merged.map((f) => f.title).sort(), ['A lone CR does not split lines', 'EAGAIN on stdin']);
+});

tests/validators/runnable-checks.test.mjs

@@ -191,6 +191,28 @@ test('brief: a success criterion whose command the runner can never run is a war
   assert.match(w[0].hint, /bash tests\//);
 });
 
+test('brief: a criterion whose first backticked span is not a command is a warning, as the runner reports it (trial 5, finding 1)', () => {
+  const r = validateBriefContent(brief([
+    '- The contract holds, now that the usage line names `--stdin` and the README too: `/bin/bash tests/contract.sh` exits 0',
+    '- `npm test` exits 0.',
+  ]));
+  assert.equal(r.valid, true, JSON.stringify(r.errors));
+  const w = r.warnings.filter((x) => x.code === 'BRIEF_SC_NOT_RUNNABLE');
+  assert.equal(w.length, 1, JSON.stringify(r.warnings));
+  assert.match(w[0].message, /SC1's first backticked span is `--stdin`/);
+  assert.match(w[0].message, /\/bin\/bash tests\/contract\.sh/);
+  assert.match(w[0].message, /not-a-command/);
+  assert.match(w[0].hint, /first backticked span/);
+});
+
+test('brief: a prose criterion that names a flag or a path, with no command after it, stays unflagged (HANDOVER-CONTRACTS)', () => {
+  const r = validateBriefContent(brief([
+    '- The `--verbose` flag is documented.',
+    '- `docs/install.md` explains the install steps.',
+  ]));
+  assert.ok(!codes(r.warnings).includes('BRIEF_SC_NOT_RUNNABLE'), JSON.stringify(r.warnings));
+});
+
 test('brief: runnable and prose-only criteria raise no runnability warning', () => {
   const r = validateBriefContent(brief(['- `npm test` exits 0.', '- `bash tests/e2e.sh` exits 0.', '- The output reads well.']));
   assert.ok(!codes(r.warnings).includes('BRIEF_SC_NOT_RUNNABLE'), JSON.stringify(r.warnings));