fix: the second lekerepo trial's faults that made a result wrong or misleading

voyage-lekerepo trial 2 (2026-10-08), report section A; the operator decided
they are fixed before the next release:
- A1: a /code-review finding whose category is simplification, efficiency,
  style or readability enters as CODE_REVIEW_IMPROVEMENT (MINOR), not MAJOR.
- A2: recommended_action is the engine's own fix when transcribed, otherwise
  a test-first step; never the failure scenario dressed as a remedy.
- A3: a /code-review finding outside the exact range it was given is declined
  and listed, but no longer holds ALLOW back; /security-review's still does.
- A4: the manifest must_contain recipe uses grep -E -e ... -- so a pattern
  like --sep is not read as an option.
- A5: indented sub-points belong to their criterion (counted), and a
  criterion with sub-points and no command warns BRIEF_SC_SUBPOINTS.
- A6: a brief requiring bash 3.2 whose criterion runs plain bash warns
  BRIEF_SC_BASH_NOT_PINNED; /trekbrief proposes /bin/bash.
- A7: the plan template's and /trekplan's checkpoints stage the step's files.

Tests fail on 5f9faee first (tests/lib/trial2-fixes.test.mjs, 11 of 14).

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

agents/review-coordinator.md

@@ -49,15 +49,18 @@ You will receive a prompt containing:
   mode, only the latter), plus the engine payloads from Claude Code's own
   review: `claude-code-review` (`/code-review`) and `claude-security-review`
   (`/security-review`, not in `quick`). Engine findings carry the catalogue
-  keys `CODE_REVIEW_DEFECT` / `SECURITY_REVIEW_VULNERABILITY` (or a narrower
+  keys `CODE_REVIEW_DEFECT` / `CODE_REVIEW_IMPROVEMENT` (MINOR: simplification,
-  key) and are treated exactly like any reviewer's findings.
+  efficiency, style, readability) / `SECURITY_REVIEW_VULNERABILITY` (or a
+  narrower key) and are treated exactly like any reviewer's findings.
 - **Expected reviewers** — the names that were due, engines included, and
   each engine recorded as not run with its reason. A name that did not report
   forbids `ALLOW` (Pass 4).
 - **Engine findings outside the review scope** — the `out_of_scope` list from
-  `lib/review/engine-findings.mjs`. Each one is `unverified`
+  `lib/review/engine-findings.mjs`. List each one under `## Declined to Judge`
   (`engine:out-of-scope`): a real claim on a file this review did not cover.
-  List it under `## Declined to Judge`; it forbids `ALLOW`.
+  One from `/security-review` is `unverified` and forbids `ALLOW` (its base is
+  wider than this review). One from `/code-review` does not: it was given
+  exactly this range, so it does not hold `ALLOW` back.
 - **Triage map** — `{file → deep-review|summary-only|skip, reason}` from
   the /trekreview triage gate.
 - **Brief metadata** — `task`, `slug`, `project_dir`, `brief_path` from

commands/trekbrief.md

@@ -332,7 +332,9 @@ sentence with your recommended answer first.
   testable conditions — commands to run, observations, or metrics."
   A command the review can run is one test-runner call (`npm test`,
   `node --test <file>`, `bash tests/<script>.sh`); propose a script under
-  `tests/` for anything else (the product's own binary, a pipe, `jq`).
+  `tests/` for anything else (the product's own binary, a pipe, `jq`). When
+  the brief requires bash 3.2, propose `/bin/bash tests/<script>.sh`: plain
+  `bash` is whichever bash comes first on PATH, which may be newer.
 - _Sharpen_ (criterion is vague): "'{quoted criterion}' is subjective.
   Which command, observation, or metric would prove this is met?"
 - _Quantify_ (performance/quality claim): "You mentioned it should be
@@ -830,7 +832,11 @@ 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
 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.
+keeps it, it stays a criterion a human judges, and the report says so. The
+same goes for `BRIEF_SC_SUBPOINTS` (a criterion with sub-points and no command
+of its own: offer one script that checks them all, or one criterion per
+sub-point) and `BRIEF_SC_BASH_NOT_PINNED` (the brief requires bash 3.2 but a
+criterion runs plain `bash`: offer `/bin/bash`).
 
 **Build the operator-annotation HTML, then print the report.** After the
 brief is validated, run `scripts/annotate.mjs` to produce a self-contained

commands/trekexecute.md

@@ -1521,7 +1521,8 @@ holds, regardless of Verify exit code.
    `.sh` file that appears in `git diff --name-only {start_sha}` (safety
    net for unlisted scripts), run `bash -n {path}`. Non-zero exit = fail.
 5. **must_contain patterns:** for each `{path, pattern}` pair, run
-   `grep -E "{pattern}" {path}`. No match = fail.
+   `grep -E -e "{pattern}" -- {path}` (`-e` and `--`: a pattern such as `--sep`
+   is never read as an option). No match = fail.
 
 **On manifest failure:**
 

commands/trekplan.md

@@ -629,7 +629,7 @@ Do NOT write this synthesis to disk. It is internal working context only.
 >   - Pattern: `src/middleware/cors.test.ts` (follow this style)
 > - **Verify:** `npm test -- jwt.test.ts` → expected: `3 passing`
 > - **On failure:** revert — `git checkout -- src/middleware/jwt.ts src/middleware/jwt.test.ts`
-> - **Checkpoint:** `git commit -m "feat(auth): add JWT verification middleware"`
+> - **Checkpoint:** `git add src/middleware/jwt.ts src/middleware/jwt.test.ts && git commit -m "feat(auth): add JWT verification middleware"`
 > - **Manifest:**
 >   ```yaml
 >   manifest:
@@ -704,7 +704,7 @@ Write the plan following the template structure. The plan MUST include:
      each pinned to a test; and any Global Constraint the step could break,
      quoted verbatim.
    - **On failure:** — recovery action (revert/retry/skip/escalate)
-   - **Checkpoint:** — git commit command after success
+   - **Checkpoint:** — `git add` of the step's Files and test files, then the git commit, after success
 
    Steps are vertical slices: each cuts a narrow but complete path through the
    layers it touches and is verifiable on its own. Never "all the tests, then

commands/trekreview.md

@@ -307,8 +307,11 @@ code, never posts anywhere, and never uploads the repo to a cloud review.
 later in this conversation. Do not start Phase 6 before both engines have
 delivered or have been recorded as not run. For each engine that delivered,
 transcribe every finding — file, line (0 when none), the one-sentence summary,
-the failure scenario, the category if given, and a narrower catalogue
+the failure scenario, the category if given (it decides the tier:
-`rule_key` only when one plainly fits — into
+`simplification`, `efficiency`, `style` and `readability` enter as MINOR), the
+fix the engine proposes (`fix`) only when its text states one — never write one
+the engine did not give — and a narrower catalogue `rule_key` only when one
+plainly fits — into
 `{project_dir}/.review-engine-{code-review|security-review}.json` as
 `{"engine": "...", "findings": [...]}`. Transcribe; never re-judge, merge or
 drop an engine finding on the way. Then convert it:
@@ -325,9 +328,12 @@ Exit 0 prints a reviewer payload (`claude-code-review` /
 `claude-security-review`) for Phase 6 plus `out_of_scope`: engine findings on
 files outside the review's SHA range (`/security-review` diffs against
 origin's default branch, which can be wider). Pass `out_of_scope` to the
-coordinator as well: each one is `unverified` (`engine:out-of-scope`), is
+coordinator as well: each one is listed under `## Declined to Judge`
-listed under `## Declined to Judge` and in Coverage, and forbids `ALLOW` —
+(`engine:out-of-scope`) and in Coverage — never silently dropped. One from
-never silently dropped. Absolute paths under the repo root are made relative
+`/security-review` is `unverified` and forbids `ALLOW`: its base is wider than
+this review and nobody here judged it. One from `/code-review` does not hold
+`ALLOW` back: that engine was given exactly this range, so a citation outside
+it is a remark about code this review did not change. Absolute paths under the repo root are made relative
 before the scope match (`--repo-root`). Exit 1 = the transcription is
 malformed, or the scope list is empty: fix it from the engine's own text (or
 the scope from Phase 3) and re-run; never feed a payload that did not pass.

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. |
+| 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`). |
 
 **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.
 
@@ -131,6 +131,8 @@ Optional but standard sections: `## Non-Goals`, `## Constraints`, `## Preference
 - `BRIEF_INVALID_MODEL` → strict halt; model ∉ `BASE_ALLOWED_MODELS` (currently `[sonnet, opus, fable]`).
 - `BRIEF_SIGNALS_MUTUALLY_EXCLUSIVE` → strict halt; cannot set both `phase_signals` and `phase_signals_partial: true`.
 - `BRIEF_SC_NOT_RUNNABLE` → warning in both modes; `/trekbrief` names each one and offers a script under `tests/` the criterion runs (`bash tests/<script>.sh`).
+- `BRIEF_SC_SUBPOINTS` → warning in both modes; `/trekbrief` offers one script that checks all sub-points, or one criterion per sub-point.
+- `BRIEF_SC_BASH_NOT_PINNED` → warning in both modes; `/trekbrief` offers `/bin/bash tests/<script>.sh` so a bash 3.2 requirement is measured under the system bash.
 
 **Compatibility direction of the v5.9 allowlist widening (`fable`):** enum
 widening is safe for new readers of old data, not old readers of new data.
@@ -421,7 +423,7 @@ Optional but standard sections: `## Findings (BLOCKER)`, `## Findings (MAJOR)`,
 Since 1.1 (ordre 5):
 - `## Remediation Steps` — required at `review_version` ≥ 1.1 when there is a BLOCKER: one `### Step R<n>` per BLOCKER carrying `Source finding: <id>` (strict error `REVIEW_MISSING_REMEDIATION_STEP`, soft warning). Steps are appended and numbered, never reworded across iterations (Spec Kit `/converge`).
 - `## Declined to Judge` — required at `review_version` ≥ 1.1 (strict error, soft warning; 1.0 reviews exempt): every finding set aside without a ruling, with its reason (including engine findings outside the review scope, `engine:out-of-scope`), plus `Trimmed: N`. **No finding is struck for length**: only MINOR/SUGGESTION text is trimmed, and counted (`trimmed` in the JSON block); a long BLOCKER/MAJOR keeps every character.
-- Findings from Claude Code's own `/code-review` and `/security-review` enter under the catalogue keys `CODE_REVIEW_DEFECT` (MAJOR) and `SECURITY_REVIEW_VULNERABILITY` (BLOCKER), or a narrower key, with `reviewer` `claude-code-review` / `claude-security-review`.
+- Findings from Claude Code's own `/code-review` and `/security-review` enter under the catalogue keys `CODE_REVIEW_DEFECT` (MAJOR), `CODE_REVIEW_IMPROVEMENT` (MINOR: a `/code-review` finding whose category is simplification, efficiency, style or readability) and `SECURITY_REVIEW_VULNERABILITY` (BLOCKER), or a narrower key, with `reviewer` `claude-code-review` / `claude-security-review`.
 
 **Validation strategy:**
 

lib/review/coordinator-contract.mjs

@@ -386,15 +386,22 @@ export function runContract(reviewerPayloads, options = {}) {
   const judged = judgeFilter(deduped);
   const reasoned = reasonablenessFilter(judged.kept);
   // Engine findings on files outside the review's SHA range (engine-findings.mjs
-  // `out_of_scope`): real claims nobody here judged, so they forbid ALLOW.
+  // `out_of_scope`). /security-review diffs against origin's default branch,
+  // wider than this review: a real claim nobody here judged, so it forbids
+  // ALLOW. /code-review was given exactly this range, so its citation outside
+  // it is about code the review did not change: declined, listed, never
+  // dropped — but it does not lock the verdict on WARN (voyage-lekerepo
+  // trial 2, 2026-10-08: one remark on a frozen file held ALLOW back for good).
   const outOfScope = (options.engineOutOfScope ?? [])
     .map((f) => ({ ...f, suppressed_reason: 'engine:out-of-scope' }));
-  const unverified = [...judged.unverified, ...reasoned.unverified, ...outOfScope];
+  const outOfScopeBlocking = outOfScope.filter((f) => f.engine !== 'code-review');
+  const outOfScopeNoted = outOfScope.filter((f) => f.engine === 'code-review');
+  const unverified = [...judged.unverified, ...reasoned.unverified, ...outOfScopeBlocking];
   // What the coordinator set aside without ruling on it, one line each, so a
   // reader sees every finding that did not reach a verdict tier. Borrowed:
   // "Declined to judge … nothing you set aside is dropped silently"
   // (Superpowers `skills/requesting-code-review/code-reviewer.md:43-48`, MIT).
-  const declined_to_judge = unverified.map((f) => ({
+  const declined_to_judge = [...unverified, ...outOfScopeNoted].map((f) => ({
     file: f.file ?? null,
     line: f.line ?? null,
     rule_key: f.rule_key ?? null,

lib/review/engine-findings.mjs

@@ -14,13 +14,22 @@
 //   { "engine": "code-review" | "security-review",
 //     "findings": [ { "file": "...", "line": 12, "summary": "...",
 //                     "failure_scenario": "...", "category": "correctness",
+//                     "fix": "<optional: the fix the engine itself proposed>",
 //                     "rule_key": "<optional narrower catalogue key>" } ] }
 //
 // Output: { reviewer, findings: [...catalogue-conformant...], out_of_scope: [...] }
 //   - rule_key: the given one when it is in the catalogue AND at least as
 //     severe as the engine's own key, else CODE_REVIEW_DEFECT (MAJOR) /
 //     SECURITY_REVIEW_VULNERABILITY (BLOCKER). A "narrower" key can never
-//     lower an engine finding's tier.
+//     lower an engine finding's tier. One exception, decided by the engine's
+//     own category and not by the transcriber: a /code-review finding in an
+//     IMPROVEMENT_CATEGORIES category is CODE_REVIEW_IMPROVEMENT (MINOR), since
+//     /code-review reports no severity and a simplification proposal is not a
+//     MAJOR defect (voyage-lekerepo trial 2, 2026-10-08).
+//   - recommended_action: the engine's own `fix` when the transcription
+//     carries one; otherwise a test-first step built from the failure
+//     scenario. Never the scenario dressed as a remedy (trial 2: /trekplan
+//     would have taken failure scenarios as step goals).
 //   - File paths are made repo-relative (an absolute path under `repoRoot`
 //     loses that prefix) before they are compared with the scope.
 //   - A finding on a file outside the review scope is NOT dropped silently: it
@@ -69,6 +78,9 @@ function parseLine(v) {
 
 const tier = (key) => SEVERITY_VALUES.indexOf(getRule(key).severity);
 
+/** /code-review categories that name an improvement, not a defect. */
+export const IMPROVEMENT_CATEGORIES = Object.freeze(new Set(['simplification', 'efficiency', 'style', 'readability']));
+
 /**
  * Convert one engine's transcribed findings into a coordinator payload.
  * @param {{ engine: string, findings: object[] }} input
@@ -96,8 +108,12 @@ export function toReviewerPayload(input, opts = {}) {
     const line = parseLine(f.line);
     const narrower = typeof f.rule_key === 'string' && RULE_KEYS.has(f.rule_key)
       && tier(f.rule_key) <= tier(spec.rule_key);
-    const rule_key = narrower ? f.rule_key : spec.rule_key;
+    const improvement = input.engine === 'code-review'
+      && IMPROVEMENT_CATEGORIES.has(String(f.category ?? '').trim().toLowerCase());
+    const engineKey = improvement ? 'CODE_REVIEW_IMPROVEMENT' : spec.rule_key;
+    const rule_key = narrower ? f.rule_key : engineKey;
     const scenario = String(f.failure_scenario ?? '').trim();
+    const fix = String(f.fix ?? '').trim();
     const loc = line > 0 ? `${file}:${line}` : file;
     const out = {
       id: computeFindingId(file, line, rule_key),
@@ -112,9 +128,10 @@ export function toReviewerPayload(input, opts = {}) {
       // An action, not the title again: `Resolve: <title>` read as "restating
       // the title" to the coordinator's Actionability filter, and 8 of 8
       // engine findings were set aside (voyage-lekerepo trial, 2026-10-06).
-      recommended_action: scenario
+      recommended_action: fix
-        ? `Fix ${loc} so this no longer happens: ${scenario}`
+        || (scenario
-        : `Fix the defect at ${loc} that ${input.engine === 'security-review' ? '/security-review' : '/code-review'} reported`,
+          ? `Write a test that reproduces this at ${loc} (${scenario}), then change ${loc} until it passes`
+          : `Write a test that shows the defect at ${loc} that ${input.engine === 'security-review' ? '/security-review' : '/code-review'} reported, then change ${loc} until it passes`),
       engine: input.engine,
     };
     if (scope && !scope.has(file)) out_of_scope.push(out);

lib/review/rule-catalogue.mjs

@@ -1,19 +1,23 @@
 // lib/review/rule-catalogue.mjs
 // Canonical rule catalogue for /trekreview.
 //
-// 14 rule keys, 4-tier severity (matches brief contract).
+// 15 rule keys, 4-tier severity (matches brief contract).
 // llm-security 5-tier alignment is a v1.1 candidate.
 //
 // Ordre 5 (review_version 1.1) added the two engine keys: findings from Claude
 // Code's own `/code-review` and `/security-review` enter the coordinator under
-// them when no narrower key fits (wrap, not a new engine).
+// them when no narrower key fits (wrap, not a new engine). A third engine key,
+// CODE_REVIEW_IMPROVEMENT (MINOR), takes a /code-review finding whose category
+// is an improvement rather than a defect (simplification, efficiency, style,
+// readability): the second voyage-lekerepo trial (2026-10-08) saw a pure
+// simplification proposal enter as MAJOR and drive the verdict.
 //
 // `memory_route` decides where a finding class that recurs across two
 // review.md files is proposed as project memory (P5, lib/review/recurrence.mjs):
 //   'check' - mechanical: a deterministic check can catch it;
 //   'rule'  - judgement: a line in the target repo's `.claude/rules/` file;
 //   null    - never proposed: review hygiene (about the review itself, not the
-//             target code), or a catch-all key (the two engine keys) whose
+//             target code), or a catch-all key (the three engine keys) whose
 //             members share nothing but their source.
 // Routing borrowed from Pocock `skills/in-progress/retro/SKILL.md:19` (MIT).
 
@@ -120,6 +124,13 @@ export const RULE_CATALOGUE = Object.freeze([
     description: "A defect reported by Claude Code's /code-review that no narrower rule_key covers.",
     memory_route: null,
   }),
+  Object.freeze({
+    rule_key: 'CODE_REVIEW_IMPROVEMENT',
+    severity: 'MINOR',
+    category: 'maintenance',
+    description: "An improvement (simplification, efficiency, style, readability) reported by Claude Code's /code-review, not a defect.",
+    memory_route: null,
+  }),
   Object.freeze({
     rule_key: 'SECURITY_REVIEW_VULNERABILITY',
     severity: 'BLOCKER',

lib/validators/brief-validator.mjs

@@ -270,8 +270,28 @@ export function validateBriefContent(text, opts = {}) {
   // judges by reading, and that stays allowed. Prose-only criteria (no
   // command) are not flagged.
   if (fm.type !== 'trekreview') {
+    // bash 3.2 required but measured under whatever `bash` is first on PATH:
+    // on a Mac with a newer bash installed that is 5.x, so the constraint is
+    // never measured (voyage-lekerepo trial 2, 2026-10-08).
+    const constraints = (extractSection(body, 'Constraints') ?? '') + (extractSection(body, 'Non-Functional Requirements') ?? '');
+    const needsBash32 = /\bbash\s*3\.2\b/i.test(constraints);
     for (const c of parseSuccessCriteria(body)) {
+      if (c.subpoints && !c.command) {
+        warnings.push(issue(
+          'BRIEF_SC_SUBPOINTS',
+          `Success criterion ${c.label} has ${c.subpoints} sub-point(s) and no command of its own: the review measures none of them`,
+          'Give the criterion one command that checks all its sub-points (`bash tests/<script>.sh` exits 0), ' +
+            'or write each sub-point as its own criterion with its own command.',
+        ));
+      }
       if (!c.command) continue;
+      if (needsBash32 && /^bash\s/.test(c.command)) {
+        warnings.push(issue(
+          'BRIEF_SC_BASH_NOT_PINNED',
+          `Success criterion ${c.label} runs \`${c.command}\`, but the brief requires bash 3.2: \`bash\` is whichever bash comes first on PATH, which may be newer`,
+          `Write it as \`/bin/bash ${c.command.replace(/^bash\s+/, '')}\` (/bin/bash tests/<script>.sh) so the check runs under the system bash.`,
+        ));
+      }
       const r = runnableCheck(c, { cwd: opts.cwd });
       if (r.runnable) continue;
       warnings.push(issue(

lib/verification/criteria-runner.mjs

@@ -164,6 +164,12 @@ function parseSection(markdown, heading, prefix) {
   const criteria = [];
   const push = (text, command, reason) =>
     criteria.push({ label: `${prefix}${criteria.length + 1}`, text, command, reason });
+  // A bullet indented deeper than the section's first bullet is a sub-point of
+  // the criterion above it, never a criterion of its own: one criterion with
+  // ten sub-points became SC4..SC13, each `not-a-command` (voyage-lekerepo
+  // trial 2, 2026-10-08). The parent counts them in `subpoints`.
+  let baseIndent = null;
+  let lastBullet = null;
   for (const line of lines) {
     if (line.marker) continue;
     if (line.fenced) {
@@ -175,10 +181,17 @@ function parseSection(markdown, heading, prefix) {
     }
     const bullet = line.text.match(BULLET);
     if (!bullet) continue;
+    const indent = line.text.match(/^\s*/)[0].replace(/\t/g, '    ').length;
+    if (baseIndent === null) baseIndent = indent;
+    if (indent > baseIndent && lastBullet) {
+      lastBullet.subpoints = (lastBullet.subpoints ?? 0) + 1;
+      continue;
+    }
     const text = bullet[1].trim();
     if (text === '') continue;
     const { command, reason } = firstCommand(text);
     push(text, command, reason);
+    lastBullet = criteria[criteria.length - 1];
   }
   return criteria;
 }

scripts/trekreview-armB.workflow.mjs

@@ -40,7 +40,7 @@ const RULE_KEYS = [
   'MISSING_BRIEF_REF', 'UNIMPLEMENTED_CRITERION', 'SCOPE_CREEP_BUILT', 'NON_GOAL_VIOLATED',
   'MISSING_TEST', 'SECURITY_INJECTION', 'PLACEHOLDER_IN_CODE', 'MISSING_ERROR_HANDLING',
   'UNDECLARED_DEPENDENCY', 'PLAN_EXECUTE_DRIFT', 'BROKEN_SUCCESS_CRITERION', 'COVERAGE_SILENT_SKIP',
-  'CODE_REVIEW_DEFECT', 'SECURITY_REVIEW_VULNERABILITY',
+  'CODE_REVIEW_DEFECT', 'CODE_REVIEW_IMPROVEMENT', 'SECURITY_REVIEW_VULNERABILITY',
 ];
 const SEVERITIES = ['BLOCKER', 'MAJOR', 'MINOR', 'SUGGESTION'];
 

templates/plan-template.md

@@ -93,7 +93,7 @@ break, quoted verbatim). A field that is present is never empty.
   - Pattern: `path/to/existing-test.ts` *(follow this style)*
 - **Verify:** `{exact command}` → expected: `{output}`
 - **On failure:** {revert | retry | skip | escalate} — {specific instructions}
-- **Checkpoint:** `git commit -m "{conventional commit message}"`
+- **Checkpoint:** `git add {the step's Files and its test files} && git commit -m "{conventional commit message}"`
 - **Manifest:**
   ```yaml
   manifest:
@@ -119,7 +119,7 @@ break, quoted verbatim). A field that is present is never empty.
 - **End-to-end:** `{a command the criteria runner allows that runs the product the way a user does — the app, the CLI, the simulator; usually bash tests/e2e.sh}` → expected: {what the user sees} *(the same command also stands under ## Verification)*
 - **Verify:** `{exact command}` → expected: `{output}`
 - **On failure:** {revert | retry | skip | escalate} — {specific instructions}
-- **Checkpoint:** `git commit -m "{conventional commit message}"`
+- **Checkpoint:** `git add {the step's Files and its test files} && git commit -m "{conventional commit message}"`
 - **Manifest:**
   ```yaml
   manifest:

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:1124-1139 | 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:1130-1145 | 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:1115-1135 | 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:2343-2365 | 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:2344-2366 | 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:628-632 | profile, phase_models, profile_source | none |

tests/lib/doc-consistency.test.mjs

@@ -269,12 +269,12 @@ test('HANDOVER-CONTRACTS.md Handover 7 § Lifecycle names --cleanup and produced
 // file" (commands/trekcontinue.md exists, so that loop already asserts the
 // /trekcontinue reference). Single-command existence pin → redundant.
 
-test('rule-catalogue has exactly 14 entries', async () => {
+test('rule-catalogue has exactly 15 entries', async () => {
   const mod = await import('../../lib/review/rule-catalogue.mjs');
   assert.strictEqual(
     mod.RULE_CATALOGUE.length,
-    14,
+    15,
-    'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 14 (12 v1.0 baseline + 2 engine keys, ordre 5)',
+    'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 15 (12 v1.0 baseline + 2 engine keys, ordre 5, + CODE_REVIEW_IMPROVEMENT, lekerepo trial 2)',
   );
 });
 

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

@@ -36,7 +36,9 @@ test('engine finding: recommended_action is an imperative built from the failure
   const f = r.payload.findings[0];
   assert.doesNotMatch(f.recommended_action, /^Resolve:/);
   assert.notEqual(f.recommended_action.replace(/^\w+\s*/, ''), f.title);
-  assert.match(f.recommended_action, /^Fix /);
+  // Trial 2 (2026-10-08): the action is a test-first step, not the scenario
+  // dressed as a remedy (tests/lib/trial2-fixes.test.mjs, A2).
+  assert.match(f.recommended_action, /^Write a test /);
   assert.match(f.recommended_action, /bin\/slug\.mjs:12/);
   assert.match(f.recommended_action, /prints `x---max`/);
 });
@@ -44,7 +46,7 @@ test('engine finding: recommended_action is an imperative built from the failure
 test('engine finding without a failure scenario still gets an action that is not the title', () => {
   const r = toReviewerPayload({ engine: 'security-review', findings: [{ file: 'a.mjs', line: 0, summary: 'token logged' }] });
   const f = r.payload.findings[0];
-  assert.match(f.recommended_action, /^Fix /);
+  assert.match(f.recommended_action, /^Write a test /);
   assert.notEqual(f.recommended_action, `Resolve: ${f.title}`);
 });
 

tests/lib/rule-catalogue.test.mjs

@@ -39,13 +39,14 @@ test('RULE_CATALOGUE — all category values within enum', () => {
   }
 });
 
-test('RULE_KEYS.size === RULE_CATALOGUE.length (== 14) — pinned by doc-consistency', () => {
+test('RULE_KEYS.size === RULE_CATALOGUE.length (== 15) — pinned by doc-consistency', () => {
   assert.equal(RULE_KEYS.size, RULE_CATALOGUE.length);
-  assert.equal(RULE_CATALOGUE.length, 14);
+  assert.equal(RULE_CATALOGUE.length, 15);
 });
 
 test('RULE_CATALOGUE — the two Claude Code engine keys exist at their tiers (ordre 5)', () => {
   assert.equal(getRule('CODE_REVIEW_DEFECT')?.severity, 'MAJOR');
+  assert.equal(getRule('CODE_REVIEW_IMPROVEMENT')?.severity, 'MINOR');
   assert.equal(getRule('SECURITY_REVIEW_VULNERABILITY')?.severity, 'BLOCKER');
 });
 

tests/lib/trial2-fixes.test.mjs

@@ -0,0 +1,172 @@
+// tests/lib/trial2-fixes.test.mjs
+// The voyage-lekerepo second end-to-end trial (2026-10-08), report section A:
+// faults that make a result wrong or misleading. The operator decided they are
+// fixed before the next release. Each test fails on main 5f9faee.
+
+import { test } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { spawnSync } from 'node:child_process';
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { toReviewerPayload } from '../../lib/review/engine-findings.mjs';
+import { runContract } from '../../lib/review/coordinator-contract.mjs';
+import { getRule } from '../../lib/review/rule-catalogue.mjs';
+import { parseSuccessCriteria } from '../../lib/verification/criteria-runner.mjs';
+import { validateBriefContent } from '../../lib/validators/brief-validator.mjs';
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const ROOT = join(HERE, '..', '..');
+const read = (p) => readFileSync(join(ROOT, p), 'utf8');
+const codes = (list) => list.map((e) => e.code);
+
+const engineFinding = (extra = {}) => ({
+  file: 'src/slug.mjs', line: 9,
+  summary: 'A two-step detour where one split/filter/join would do',
+  failure_scenario: 'slug("a--b") walks the string twice',
+  ...extra,
+});
+
+// --- A1. an improvement from /code-review is not a MAJOR defect --------------
+
+test('A1: a /code-review simplification or efficiency finding is MINOR, a correctness finding stays MAJOR', () => {
+  const r = toReviewerPayload({ engine: 'code-review', findings: [
+    engineFinding({ category: 'simplification' }),
+    engineFinding({ line: 10, category: 'efficiency' }),
+    engineFinding({ line: 11, category: 'correctness' }),
+    engineFinding({ line: 12 }),
+  ] });
+  assert.equal(r.ok, true, r.errors.join('; '));
+  const sev = r.payload.findings.map((f) => f.severity);
+  assert.deepEqual(sev, ['MINOR', 'MINOR', 'MAJOR', 'MAJOR']);
+  for (const f of r.payload.findings) assert.equal(f.severity, getRule(f.rule_key).severity, 'severity is fixed by rule_key');
+});
+
+test('A1: /security-review keeps its tier whatever the category', () => {
+  const r = toReviewerPayload({ engine: 'security-review', findings: [engineFinding({ category: 'simplification' })] });
+  assert.equal(r.payload.findings[0].severity, 'BLOCKER');
+});
+
+// --- A2. recommended_action is a remedy, not the failure scenario -----------
+
+test('A2: the engine\'s own fix text becomes the recommended action', () => {
+  const r = toReviewerPayload({ engine: 'code-review', findings: [engineFinding({ fix: 'Use one split/filter/join pass' })] });
+  assert.equal(r.payload.findings[0].recommended_action, 'Use one split/filter/join pass');
+});
+
+test('A2: without a fix text the action is a step: reproduce the scenario in a test, then change the location', () => {
+  const r = toReviewerPayload({ engine: 'code-review', findings: [engineFinding()] });
+  const a = r.payload.findings[0].recommended_action;
+  assert.doesNotMatch(a, /so this no longer happens/);
+  assert.match(a, /^Write a test that reproduces/);
+  assert.match(a, /src\/slug\.mjs:9/);
+});
+
+test('A2: /trekreview asks the transcription for the engine\'s fix text and forbids inventing one', () => {
+  const md = read('commands/trekreview.md');
+  const p = md.slice(md.indexOf('## Phase 4.7'), md.indexOf('## Phase 5'));
+  assert.match(p, /the\s+fix\s+the\s+engine\s+proposes\s+\(`fix`\)/);
+  assert.match(p, /never\s+write\s+one\s+the\s+engine\s+did\s+not\s+give/);
+});
+
+// --- A3. an out-of-range /code-review finding does not lock the verdict -----
+
+test('A3: a /code-review finding outside the exact range it was given is declined, and does not hold ALLOW back', () => {
+  const out = { ...engineFinding({ file: 'tests/cli.test.mjs', line: 1 }), engine: 'code-review', rule_key: 'CODE_REVIEW_DEFECT', severity: 'MAJOR', title: 'No CLI test for --sep' };
+  const c = runContract([], { engineOutOfScope: [out] });
+  assert.equal(c.verdict, 'ALLOW', JSON.stringify(c.allow_blocked_by));
+  assert.equal(c.declined_to_judge.length, 1);
+  assert.equal(c.declined_to_judge[0].reason, 'engine:out-of-scope');
+});
+
+test('A3: a /security-review finding outside the range still forbids ALLOW (its base is wider and not ours)', () => {
+  const out = { ...engineFinding({ file: 'lib/auth.mjs' }), engine: 'security-review', rule_key: 'SECURITY_REVIEW_VULNERABILITY', severity: 'BLOCKER', title: 'Token logged' };
+  const c = runContract([], { engineOutOfScope: [out] });
+  assert.notEqual(c.verdict, 'ALLOW');
+});
+
+// --- A4. the manifest's must_contain check survives a pattern that starts with "-"
+
+test('A4: the must_contain recipe passes the pattern with -e and ends options with --', () => {
+  const md = read('commands/trekexecute.md');
+  assert.match(md, /`grep -E -e "\{pattern\}" -- \{path\}`/);
+  assert.doesNotMatch(md, /`grep -E "\{pattern\}" \{path\}`/);
+  const dir = mkdtempSync(join(tmpdir(), 'voyage-grep-'));
+  try {
+    writeFileSync(join(dir, 'README.md'), 'Use --sep to choose the separator.\n');
+    const r = spawnSync('/bin/bash', ['-c', 'grep -E -e "--sep" -- README.md'], { cwd: dir, encoding: 'utf8' });
+    assert.equal(r.status, 0, r.stderr);
+  } finally { rmSync(dir, { recursive: true, force: true }); }
+});
+
+// --- A5. sub-points belong to their criterion -------------------------------
+
+function brief(sections) {
+  return [
+    '---', 'type: trekbrief', 'brief_version: "2.2"', 'task: t', 'slug: s',
+    'research_topics: 0', 'research_status: skipped', 'framing: new-direction', 'phase_signals_partial: true', '---', '',
+    '## TL;DR', '', 'A toy.', '', '## Intent', '', 'Why.', '', '## Goal', '', 'What.', '',
+    ...sections,
+  ].join('\n');
+}
+
+const NESTED = [
+  '## Success Criteria', '',
+  '- `npm test` exits 0.',
+  '- The separator is honoured:',
+  '  - `-` joins words',
+  '  - `_` joins words',
+  '  - `.` joins words',
+  '- `bash tests/e2e.sh` exits 0.', '',
+];
+
+test('A5: indented sub-points are part of their criterion, not criteria of their own', () => {
+  const sc = parseSuccessCriteria(brief(NESTED));
+  assert.deepEqual(sc.map((c) => c.label), ['SC1', 'SC2', 'SC3']);
+  assert.equal(sc[1].subpoints, 3);
+  assert.equal(sc[2].command, 'bash tests/e2e.sh');
+});
+
+test('A5: a criterion with sub-points and no command of its own is a warning that names it', () => {
+  const r = validateBriefContent(brief(NESTED));
+  const w = r.warnings.filter((x) => x.code === 'BRIEF_SC_SUBPOINTS');
+  assert.equal(w.length, 1, JSON.stringify(r.warnings));
+  assert.match(w[0].message, /SC2/);
+});
+
+// --- A6. a bash 3.2 constraint is measured under bash 3.2 --------------------
+
+const BASH32 = ['## Constraints', '', '- Scripts must run under bash 3.2 (macOS system bash).', ''];
+
+test('A6: a brief that requires bash 3.2 and runs a criterion with plain `bash` gets a warning pointing at /bin/bash', () => {
+  const r = validateBriefContent(brief([...BASH32, '## Success Criteria', '', '- `bash tests/contract.sh` exits 0.', '']));
+  const w = r.warnings.filter((x) => x.code === 'BRIEF_SC_BASH_NOT_PINNED');
+  assert.equal(w.length, 1, JSON.stringify(r.warnings));
+  assert.match(w[0].hint, /\/bin\/bash tests\//);
+});
+
+test('A6: `/bin/bash`, or no bash 3.2 constraint, raises no such warning', () => {
+  const pinned = validateBriefContent(brief([...BASH32, '## Success Criteria', '', '- `/bin/bash tests/contract.sh` exits 0.', '']));
+  assert.ok(!codes(pinned.warnings).includes('BRIEF_SC_BASH_NOT_PINNED'));
+  const free = validateBriefContent(brief(['## Success Criteria', '', '- `bash tests/contract.sh` exits 0.', '']));
+  assert.ok(!codes(free.warnings).includes('BRIEF_SC_BASH_NOT_PINNED'));
+});
+
+test('A6: /trekbrief proposes /bin/bash when the brief requires bash 3.2', () => {
+  const md = read('commands/trekbrief.md');
+  assert.match(md, /requires\s+bash\s+3\.2[^`]*`\/bin\/bash tests\/<script>\.sh`/);
+  assert.match(md, /BRIEF_SC_BASH_NOT_PINNED/);
+  assert.match(md, /BRIEF_SC_SUBPOINTS/);
+});
+
+// --- A7. the plan template's checkpoint stages what it commits --------------
+
+test('A7: every checkpoint the plan template and /trekplan\'s example show stages the step\'s files first', () => {
+  const lines = [
+    ...read('templates/plan-template.md').split('\n'),
+    ...read('commands/trekplan.md').split('\n'),
+  ].filter((l) => /\*\*Checkpoint:\*\*/.test(l) && /`git [^`]*commit -m /.test(l));
+  assert.ok(lines.length >= 3, `found ${lines.length}`);
+  for (const l of lines) assert.match(l, /`git add [^`]+ && git commit -m /, l);
+});