merge(wave2): 9 rettepakke, Regression and On failure step rules
Order 9 of wave 2: contracts K10 (Regression: step field) and K11 (On failure: action words) in the plan validator, and the minor points B8-B12 and C13-C17 from the second trial. Branch suite 2043 (2041/0/2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
e39a588bbb
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
15 changed files with 675 additions and 24 deletions
|
|
@ -31,7 +31,7 @@ Voyage — a contract-driven Claude Code pipeline: brief, research, plan, execut
|
|||
|---------|-------------|--------------------|
|
||||
| `/trekbrief` | Brief — one question round sized by class (`ceremony: small\|bounded\|architectural`, heaviest in doubt, said aloud, only moved up) produces a task brief with explicit research plan; ends with the operator explicitly approving `## Intent` + `## Goal` («høres bra ut» is not approval), logged in the target repo's hash-chained approval log and committed (the brief too when `artifact_store: git`) (`--approve <dir>` does only the approval, then writes the trekbrief record); optionally orchestrates the pipeline | session |
|
||||
| `/trekresearch` | Research — deep local + external research, produces structured research brief. Opt-in `--engine {swarm\|deep-research}` delegates the external phase to Claude Code's built-in `/deep-research` workflow (swarm default) | session |
|
||||
| `/trekplan` | Plan — brief-reviewer, explore, plan, review. Requires `--brief` or `--project`; halts unless the brief's intent is approved (hash-bound, stale on edit). Auto-discovers `architecture/overview.md` if present. Plan 1.8: `Test first:` per step + one `End-to-end:` step; a design part for `ceremony: architectural` (absent = architectural); critic loop of max 3 rounds, recorded in the plan, and `plan-gate.mjs` (0 go / 1 stop / 2 could not run) decides | session |
|
||||
| `/trekplan` | Plan — brief-reviewer, explore, plan, review. Requires `--brief` or `--project`; halts unless the brief's intent is approved (hash-bound, stale on edit). Auto-discovers `architecture/overview.md` if present. Plan 1.8: `Test first:` per step + one `End-to-end:` step; a design part for `ceremony: architectural` (absent = architectural); critic loop of max 3 rounds, recorded in the plan, and `plan-gate.mjs` (0 go / 1 stop / 2 could not run) decides; open minors after it are listed for the operator to pick | session |
|
||||
| `/trekexecute` | Execute — disciplined plan/session-spec executor: one fresh `step-implementer` helper per plan step, one controller that reads the diff (spec + quality verdicts), proof of red / floor guard / done line as commands, fix rounds that go up (max 5: helper → plan → operator), closed stop list + decision log in the final report | session |
|
||||
| `/trekreview` | Review — a gate: Claude Code's `/code-review` + `/security-review` as engine, then Voyage's brief control (which stops); writes `review.md` with a required verdict that `lib/review/verdict-gate.mjs` decides on (0 ALLOW/WARN · 1 BLOCK · 2 never green); proposes recurring finding classes as project memory, line by line (Handover 6) | session |
|
||||
| `/trekcontinue` | Continue — resumes the next session of a multi-session voyage project. Reads `.session-state.local.json` (Handover 7) and immediately begins executing | session |
|
||||
|
|
@ -41,6 +41,11 @@ Voyage — a contract-driven Claude Code pipeline: brief, research, plan, execut
|
|||
|
||||
Full flag reference for each command (modes, `--gates`, `--profile`, breaking changes): see `docs/command-modes.md`.
|
||||
|
||||
> **Plan step fields K10/K11 (any plan_version).** Optional `Regression:` = ONE
|
||||
> allowlisted command, unpiped, run green before the checkpoint (`Verify:` stays
|
||||
> the red-first test); `On failure:` opens with revert | retry | skip | escalate.
|
||||
> `plan-validator.mjs` refuses the first, warns on the second → Handover 4.
|
||||
|
||||
> **STORM bounded loop — default-off, env-gated.** `/trekresearch` Phase 4.5
|
||||
> (dimension discovery, under the existing `maxDimensions: 8` ceiling) and
|
||||
> Phase 5 (bounded multi-turn follow-up) run only at `effort: high` **and** only
|
||||
|
|
|
|||
|
|
@ -297,7 +297,8 @@ Every plan includes:
|
|||
Every implementation step includes:
|
||||
- **Test first:** — the test the step makes pass (required from `plan_version 1.8`)
|
||||
- **Interface in:** / **Interface out:** / **Review focus:** — optional: what the step consumes and produces, and the untested inputs it pins to a test
|
||||
- **On failure:** — what to do when verification fails (revert / retry / skip / escalate)
|
||||
- **Regression:** — optional: one runnable suite command (`npm test`, no pipe, no `&&`) run green before the checkpoint, while `Verify:` stays the step's own red-first test; the validator refuses one that can never run
|
||||
- **On failure:** — what to do when verification fails; it opens with `revert`, `retry`, `skip` or `escalate`, or the validator warns
|
||||
- **Checkpoint:** — git commit after success
|
||||
- **Manifest:** — the objective completion predicate (Hard Rule 17)
|
||||
|
||||
|
|
@ -305,7 +306,7 @@ One step per plan is the "run the thing" step (`End-to-end:`): the product run t
|
|||
|
||||
Exploration uses 6–8 agents in parallel (architecture-mapper, dependency-tracer, task-finder, test-strategist, git-historian, risk-assessor, plus convention-scanner on medium+ codebases and research-scout when unfamiliar tech is detected). Adversarial review then runs `brief-reviewer`, `plan-critic` (12 dimensions, no-placeholder enforcement, manifest audit, interfaces and slices, design and architecture rules), and `scope-guardian` (creep + gap detection, ceremony match). Per-agent details in [`agents/`](agents/).
|
||||
|
||||
**The plan gate.** Each critic round is written into the plan by `lib/validators/plan-gate.mjs --record`; `plan-gate.mjs <plan.md>` then exits 0 (go on), 1 (stop: invalid plan, no verdict, a verdict the plan has changed since, or an open BLOCKER) or 2 (could not run). After 3 rounds with a BLOCKER open, `/trekplan` stops and asks; it never continues silently, and "execute" is offered only when the gate exits 0. In Claude Code's plan mode the plan is written to Claude Code's plan file and approved through its own approval prompt (that `/trekplan` runs cleanly inside plan mode is not verified yet).
|
||||
**The plan gate.** Each critic round is written into the plan by `lib/validators/plan-gate.mjs --record`; `plan-gate.mjs <plan.md>` then exits 0 (go on), 1 (stop: invalid plan, no verdict, a verdict the plan has changed since, or an open BLOCKER) or 2 (could not run). After 3 rounds with a BLOCKER open, `/trekplan` stops and asks; it never continues silently, and "execute" is offered only when the gate exits 0. Minors still open after the last round are listed numbered in the summary; the ones you pick are fixed and get one more round, the rest are carried. In Claude Code's plan mode the plan is written to Claude Code's plan file and approved through its own approval prompt (that `/trekplan` runs cleanly inside plan mode is not verified yet).
|
||||
|
||||
**Checks the plan hands on** (0 / 1 / 2, and 2 = "not measured", never green): `lib/verification/arch-rules.mjs --repo <dir>` runs the target repo's own dependency-cruiser or import-linter — Voyage installs nothing — and `--prove` shows a rule pass, fail on a planted violation, and pass again; `lib/verification/plan-scope-sweep.mjs --plan <plan.md> --base <ref>` lists changed files that no step names.
|
||||
|
||||
|
|
|
|||
|
|
@ -15,8 +15,15 @@ in flight. This helps the planner avoid conflicts and build on recent work.
|
|||
|
||||
## Input
|
||||
|
||||
You receive a task description and optionally a list of task-relevant files (from
|
||||
the task-finder agent). Focus your analysis on code areas related to the task.
|
||||
You receive a task description, the brief's path, and optionally a list of
|
||||
task-relevant files (from the task-finder agent). Focus your analysis on code
|
||||
areas related to the task.
|
||||
|
||||
**Read the brief first.** Its Constraints and Non-Goals may freeze files ("the
|
||||
existing test files stay unchanged"). Those files are fixed for this task: report
|
||||
their history like any other file, but never propose or recommend changing one
|
||||
— not in Recommendations, not as a conflict fix. A git-historian once proposed
|
||||
editing a test file the brief froze (voyage-lekerepo trial 2, 2026-10-08).
|
||||
|
||||
## Your analysis process
|
||||
|
||||
|
|
@ -56,6 +63,11 @@ Look for:
|
|||
- Work-in-progress that touches the same files
|
||||
- Feature branches that should be merged first
|
||||
|
||||
Run `git remote -v` and name each remote exactly as it shows it: the URL's host
|
||||
for a network remote, "a local repository at <path>" for a local path or bare
|
||||
repository. Never name a hosting service the URL does not show (a local bare
|
||||
repository was once called Forgejo, voyage-lekerepo trial 2). No remote: say so.
|
||||
|
||||
### 6. Uncommitted state
|
||||
|
||||
Run `git status --short` to check for:
|
||||
|
|
@ -88,6 +100,12 @@ Run `git status --short` to check for:
|
|||
|--------|-----------|-----------|-------------------|
|
||||
| `feature/auth-v2` | 1d ago | Yes | Touches same auth module |
|
||||
|
||||
### Remote
|
||||
- `origin` — {host from `git remote -v`, or "a local repository at <path>", or "no remote"}
|
||||
|
||||
### Frozen by the brief
|
||||
- `path/to/file` — {the brief's words}; history reported, no change recommended
|
||||
|
||||
### Recommendations
|
||||
- {Any timing or sequencing advice based on git state}
|
||||
- {Files to watch for conflicts}
|
||||
|
|
@ -97,7 +115,10 @@ Run `git status --short` to check for:
|
|||
## Rules
|
||||
|
||||
- **Only analyze git history.** Do not read file contents for code analysis — other
|
||||
agents handle that.
|
||||
agents handle that. The brief is the one file you read, for what it freezes.
|
||||
- **Frozen files stay frozen.** Never recommend changing a file the brief keeps
|
||||
unchanged.
|
||||
- **Name the remote from `git remote -v`**, never by guess.
|
||||
- **Focus on the task.** Do not produce a full repository history report. Only
|
||||
report what is relevant to planning the specific task.
|
||||
- **Flag risks explicitly.** Hot files, concurrent branches, and recent refactors
|
||||
|
|
|
|||
|
|
@ -140,6 +140,13 @@ questions, which defeats the purpose.
|
|||
the product the way a user does — not a unit test renamed? A plan whose only
|
||||
end-to-end check is a unit test is a **blocker**: its result could only be
|
||||
"not verified end to end".
|
||||
- `Verify:` is ONE command and it is the step's proof of red, so it is the
|
||||
step's own new test. The suite the step must not break goes in the optional
|
||||
`Regression:` field (K10: one backticked command the criteria runner allows,
|
||||
no pipe, no `&&`), which the executor runs green before the checkpoint. Do
|
||||
not demand `npm test` in `Verify:` — that test is not red before the step.
|
||||
A step that can break existing tests and has no `Regression:` is a **minor**
|
||||
finding ("add `Regression: \`npm test\``"), not a major.
|
||||
|
||||
### 9. Headless readiness
|
||||
|
||||
|
|
|
|||
|
|
@ -648,7 +648,7 @@ relevant to this task. This is best-effort and environment-dependent: if no memo
|
|||
is available, pass nothing — the reviewer scores the memory-alignment dimension
|
||||
`5` (N/A) when no context is supplied.
|
||||
|
||||
Launch the `brief-reviewer` agent (foreground, blocking) with the prompt:
|
||||
Launch the `brief-reviewer` agent with the prompt below, then wait for its result before Step 4d: the Agent tool runs it in the background and tells this session when it is done (voyage-lekerepo trial 2, 2026-10-08), and no gate, revision or guess at the verdict comes before the reviewer's own output.
|
||||
|
||||
> **Never pass the Agent tool's `name` parameter — at this or any other spawn
|
||||
> site in this command.** `name` does not label a subagent, it changes its kind:
|
||||
|
|
|
|||
|
|
@ -414,7 +414,7 @@ Determine codebase scale to calibrate agent turns (not agent count).
|
|||
|
||||
Run via Bash:
|
||||
```
|
||||
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" -o -name "*.py" -o -name "*.go" -o -name "*.rs" -o -name "*.java" -o -name "*.rb" -o -name "*.c" -o -name "*.cpp" -o -name "*.h" -o -name "*.cs" -o -name "*.swift" -o -name "*.kt" -o -name "*.sh" -o -name "*.md" \) -not -path "*/node_modules/*" -not -path "*/.git/*" -not -path "*/vendor/*" -not -path "*/dist/*" -not -path "*/build/*" | wc -l
|
||||
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.mts" -o -name "*.cts" -o -name "*.js" -o -name "*.jsx" -o -name "*.mjs" -o -name "*.cjs" -o -name "*.py" -o -name "*.go" -o -name "*.rs" -o -name "*.java" -o -name "*.rb" -o -name "*.c" -o -name "*.cpp" -o -name "*.h" -o -name "*.cs" -o -name "*.swift" -o -name "*.kt" -o -name "*.sh" -o -name "*.md" \) -not -path "*/node_modules/*" -not -path "*/.git/*" -not -path "*/vendor/*" -not -path "*/dist/*" -not -path "*/build/*" | wc -l
|
||||
```
|
||||
|
||||
Classify:
|
||||
|
|
@ -523,8 +523,11 @@ Prompt: "Analyze the test infrastructure and design a test strategy for this tas
|
|||
Discover existing patterns and identify coverage gaps."
|
||||
|
||||
**git-historian** — recent changes, code ownership, hot files, active branches.
|
||||
Prompt: "Analyze git history relevant to this task: {task}. Report recent changes,
|
||||
ownership, hot files, and active branches that may affect planning."
|
||||
Prompt: "Analyze git history relevant to this task: {task}. Brief file:
|
||||
{brief_path} — read its Constraints and Non-Goals for files it keeps unchanged,
|
||||
and never recommend changing one. Report recent changes, ownership, hot files,
|
||||
and active branches that may affect planning. Name the remote as `git remote -v`
|
||||
shows it."
|
||||
|
||||
### Launch for medium+ codebases (50+ files):
|
||||
|
||||
|
|
@ -628,6 +631,7 @@ Do NOT write this synthesis to disk. It is internal working context only.
|
|||
> - Verifies: valid token attaches user; invalid token returns 401; missing header returns 401
|
||||
> - Pattern: `src/middleware/cors.test.ts` (follow this style)
|
||||
> - **Verify:** `npm test -- jwt.test.ts` → expected: `3 passing`
|
||||
> - **Regression:** `npm test` → expected: exit 0
|
||||
> - **On failure:** revert — `git checkout -- src/middleware/jwt.ts src/middleware/jwt.test.ts`
|
||||
> - **Checkpoint:** `git add src/middleware/jwt.ts src/middleware/jwt.test.ts && git commit -m "feat(auth): add JWT verification middleware"`
|
||||
> - **Manifest:**
|
||||
|
|
@ -650,7 +654,7 @@ Do NOT write this synthesis to disk. It is internal working context only.
|
|||
> **plan_version 1.8 additions** (the validator enforces them; 1.7 plans stay
|
||||
> valid): `Test first:` with `File:` and `Verifies:` on EVERY step; exactly the
|
||||
> optional fields `Interface in:`, `Interface out:`, `Review focus:` (never
|
||||
> empty when present); at least one step with `End-to-end:` and a backticked
|
||||
> empty when present), plus `Regression:` below; at least one step with `End-to-end:` and a backticked
|
||||
> command — the "run the thing" step; the header line carries
|
||||
> `` `plan_version: 1.8` · `ceremony: <class>` ``; an architectural plan carries
|
||||
> `## Design` (see "Design part" below); and every check the run executes can
|
||||
|
|
@ -661,6 +665,19 @@ Do NOT write this synthesis to disk. It is internal working context only.
|
|||
> `PLAN_STEP_VERIFY_NOT_RUNNABLE`, `PLAN_END_TO_END_NOT_RUNNABLE`,
|
||||
> `PLAN_END_TO_END_NOT_VERIFIED`).
|
||||
>
|
||||
> **Two step rules at every plan_version** (no version bump; a plan without
|
||||
> the field is unchanged): the optional `Regression:` field (K10) holds ONE
|
||||
> backticked command the criteria runner allows, with no pipe and no `&&` —
|
||||
> usually `npm test`, the suite the step must not break. `Verify:` stays the
|
||||
> step's own test, the one that is red first; `Regression:` is what the
|
||||
> executor runs green before the checkpoint, so a step whose red test is a
|
||||
> script can still keep the frozen suite green. Write it on a step that can
|
||||
> break existing tests; leave it out otherwise. A `Regression:` that can never
|
||||
> run is `PLAN_STEP_REGRESSION_NOT_RUNNABLE`. And `On failure:` (K11) opens with
|
||||
> one of `revert`, `retry`, `skip`, `escalate` — the words `/trekexecute`
|
||||
> reads — then the specifics; any other opening is the warning
|
||||
> `PLAN_STEP_ON_FAILURE_UNKNOWN`.
|
||||
>
|
||||
> **Validator self-check (mandatory after writing `plan.md`):** run
|
||||
> `node ${CLAUDE_PLUGIN_ROOT}/lib/validators/plan-validator.mjs --strict --json {plan_path}`
|
||||
> and re-revise the plan if it fails. The validator is the source of truth for
|
||||
|
|
@ -703,7 +720,10 @@ Write the plan following the template structure. The plan MUST include:
|
|||
- **Review focus:** — inputs the brief implies but no test exercises yet,
|
||||
each pinned to a test; and any Global Constraint the step could break,
|
||||
quoted verbatim.
|
||||
- **On failure:** — recovery action (revert/retry/skip/escalate)
|
||||
- **Regression:** (optional, K10) — the one suite command run green before
|
||||
the checkpoint, on a step that can break existing tests
|
||||
- **On failure:** — opens with the recovery action (`revert`, `retry`,
|
||||
`skip` or `escalate`, K11), then the specifics
|
||||
- **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
|
||||
|
|
@ -897,7 +917,8 @@ loudly instead of collapsing into a silently-empty merge (the historical Phase-9
|
|||
defect, where the read-only reviewers never wrote the files and the dedup ran on
|
||||
nothing). The merged array attributes each finding to `[plan-critic, scope-guardian]`
|
||||
when both reviewers raised the same issue (exact match on `file:line:rule_key`,
|
||||
or Jaccard ≥ 0.7 on text tokens), and keeps the more severe of the two
|
||||
Jaccard ≥ 0.7 on text tokens, or — same file and same line — most of the
|
||||
shorter text's content words in the longer one), and keeps the more severe of the two
|
||||
severities. Revise the plan once for the merged set, not twice for the
|
||||
duplicates. Source: research/05 R1 + R2.
|
||||
|
||||
|
|
@ -909,7 +930,11 @@ duplicates. Source: research/05 R1 + R2.
|
|||
re-reviewed.
|
||||
- **PASS** (minors only, or clean): fill in the quality header and
|
||||
`## Plan Quality Score` from plan-critic's score. These and `## Revisions`
|
||||
are the review log and do not stale the verdict. Go to Phase 10.
|
||||
are the review log and do not stale the verdict. Do not edit a step for the
|
||||
open minors now — that stales the verdict the round just earned. Go to
|
||||
Phase 10, which lists the minors for the operator to pick from (operator
|
||||
decision 2026-10-08: a real weakness among the minors after the last round
|
||||
used to be carried with no way to a fix).
|
||||
- **REVISE after the last round**: the gate reports
|
||||
`PLAN_GATE_OPEN_MAJOR_FINAL`. Stop and ask the operator with
|
||||
AskUserQuestion — each open major in plain words, recommendation first
|
||||
|
|
@ -984,9 +1009,11 @@ Present a summary to the user:
|
|||
**Plan critic:** {Summary — blockers/majors/minors found, how addressed}
|
||||
**Scope guardian:** {Summary — creep/gaps found, how addressed}
|
||||
**Open majors:** {list with "accepted by the operator" for each, or "none"}
|
||||
**Open minors:** {numbered from the last recorded verdict — "1. {plain words} ({file}:{line})" — or "none"}
|
||||
**Plan gate (K3):** {PASS | STOP — the reasons}
|
||||
|
||||
You can:
|
||||
- Say which open minors to fix ("fix 1 and 3")
|
||||
- Ask questions or request changes to refine the plan
|
||||
- Say **"execute"** to start implementing ← only when the gate exited 0
|
||||
- Say **"save"** to keep the plan for later
|
||||
|
|
@ -1006,9 +1033,19 @@ If the user asks questions or requests changes:
|
|||
- Update the plan file in-place
|
||||
- Show what changed
|
||||
- A change outside the review log makes the verdict stale: run one Phase 9
|
||||
round on the changed plan (it counts toward the 3), then re-run the gate
|
||||
round on the changed plan (it counts toward the 3), then re-run the gate.
|
||||
When the 3 rounds are spent, record that round with `--extra-round`: the
|
||||
operator's request for the change is their yes, for this one round.
|
||||
- Re-present the summary
|
||||
|
||||
**Picking open minors.** The summary lists the open minors numbered. Nothing
|
||||
is fixed unless the operator picks: minors they do not name are carried, as
|
||||
before. When they pick some ("fix 1 and 3"), revise the plan for exactly those,
|
||||
add rows to `## Revisions`, and run one Phase 9 round on the revised plan —
|
||||
with `--extra-round` when the 3 rounds are spent; the operator's pick is their
|
||||
yes for that one round. The new round's verdict decides as in Phase 9 (a new
|
||||
major after the last round goes to the operator again).
|
||||
|
||||
### In plan mode
|
||||
|
||||
When this session is in Claude Code's plan mode, the plan sits in Claude
|
||||
|
|
@ -1280,7 +1317,8 @@ Standard and low effort: do NOT emit the section at all.
|
|||
- **Bounded critic loop**: at most 3 critic rounds (Phase 9). After the third
|
||||
with a BLOCKER open, stop and ask the operator; never continue silently, and
|
||||
never write or edit the `## Critic Verdict` block by hand — only
|
||||
`plan-gate.mjs --record` writes it.
|
||||
`plan-gate.mjs --record` writes it. Open minors are listed numbered in the
|
||||
Phase 10 summary; only the ones the operator picks are fixed, with one round.
|
||||
- **Plan stands alone**: The plan file must be understandable without access
|
||||
to the exploration results. Include all necessary context.
|
||||
- **Honesty**: If exploration reveals the task is trivial (single file, obvious
|
||||
|
|
|
|||
|
|
@ -266,6 +266,9 @@ prose form (relaxed in S26 so the template's prose emission parses).
|
|||
- the optional step fields `Interface in:`, `Interface out:`, `Review focus:` are never empty when present (consumers tolerate their absence);
|
||||
- `ceremony` is one of the three classes; an `architectural` plan has `## Design` with the subsections Architecture, Data Flow, Interfaces, Data Models, Error Handling, Module Map, Architecture Rules, Requirements Check, plus `## Test Strategy`; the module map (mermaid) has no cycle; every architecture rule says "checked by `<command>`" (with a Proof column naming an existing step) or "advisory — `.claude/rules/<file>.md`".
|
||||
6. **`## Critic Verdict`** (written by `lib/validators/plan-gate.mjs --record`, never by hand): one fenced `json` block, schema `voyage.plan-verdict/1` — `round`, `max_rounds` (3), `extra_rounds`, `plan_sha256`, `verdict` (`BLOCK` | `REVISE` | `PASS`), `counts`, `findings` (deduplicated, each with `severity`). `plan_sha256` hashes the plan minus its review log (the `> **Plan quality:` line and the sections Critic Verdict, Revisions, Plan Quality Score, Adversarial Pass 2), with CRLF and line-end whitespace normalized. The verdict sits inside the plan because Claude Code's plan approval hands a hook the plan file's text and nothing beside it.
|
||||
7. **Every plan_version, no version bump** (a plan without the field validates unchanged; voyage-lekerepo trial 2, 2026-10-08):
|
||||
- **K10 — `Regression:`** (optional step field): ONE backticked command the criteria runner's allowlist runs, with no pipe and no `&&` (first backticked span, as for `Verify:`). The executor runs it green before the step's checkpoint; a step without it runs nothing new. `Verify:` stays the step's own red-first test. One that can never run is `PLAN_STEP_REGRESSION_NOT_RUNNABLE` (an error; a warning under `--soft`).
|
||||
- **K11 — `On failure:`** opens with one of `revert`, `retry`, `skip`, `escalate` (case-insensitive; a leading `**`, `_`, backtick or `-` is ignored, other leading characters are not; `onFailureAction` in `lib/parsers/plan-schema.mjs`), the words `/trekexecute` reads; any other opening is the warning `PLAN_STEP_ON_FAILURE_UNKNOWN`. A missing `On failure:` is unchanged (the executor defaults to `escalate`).
|
||||
|
||||
**Validation strategy:**
|
||||
|
||||
|
|
@ -295,6 +298,8 @@ The strongest validation in the entire pipeline. Phase 5.5 (planning-orchestrato
|
|||
| `PLAN_MODULE_MAP_EMPTY` / `PLAN_MODULE_CYCLE` | 1.8 architectural: no mermaid module map, or a cycle in it (all `mermaid` blocks of `### Module Map` form one map) | Draw it; break the cycle |
|
||||
| `PLAN_MODULE_MAP_UNPARSED` | 1.8 architectural: a module-map line that looks like a link but gives no edge | Write it as `a --> b` |
|
||||
| `PLAN_ARCH_RULE_NO_ENFORCER` / `PLAN_ARCH_RULE_NO_PROOF` | 1.8 architectural: a rule without an enforcer, or a checked rule without an existing proving step | Name the enforcer; name the step |
|
||||
| `PLAN_STEP_REGRESSION_NOT_RUNNABLE` | K10, any version: a step's `Regression:` is not one command the allowlist runs (a pipe, `&&`, no backticks, an unknown runner) | One backticked runner call (`npm test`); a wider check goes in `bash tests/<script>.sh` |
|
||||
| `PLAN_STEP_ON_FAILURE_UNKNOWN` | K11, any version, warning: `On failure:` does not open with revert / retry / skip / escalate | Open with the word, then the specifics |
|
||||
|
||||
**The plan gate (K3).** `node lib/validators/plan-gate.mjs <plan.md> [--json]` decides whether a plan may go on: **0** = it validates strict, its `## Critic Verdict` matches the current text, no BLOCKER is open, and every open MAJOR was accepted by the operator; **1** = stop, with `reasons` (`PLAN_GATE_INVALID_PLAN`, `PLAN_GATE_VERSION` — below plan_version 1.8, `PLAN_GATE_NO_VERDICT`, `PLAN_GATE_VERDICT_MALFORMED`, `PLAN_GATE_VERDICT_STALE`, `PLAN_GATE_OPEN_BLOCKER`, `PLAN_GATE_ROUNDS_EXHAUSTED`, `PLAN_GATE_OPEN_MAJOR` — rounds left: revise, `PLAN_GATE_OPEN_MAJOR_FINAL` — after the last round: ask the operator) and `warnings` (`PLAN_GATE_OPEN_MAJOR` for majors the operator accepted); **2** = could not read the plan. `--hash <plan.md>` prints the operative hash; Phase 9 takes it before the reviewers read the plan. `--record <plan.md> --reviewed <hash>` reads plan-review-dedup output on stdin (which lists the reviewers that delivered, `sources`) and writes the next round; exit 2 when a reviewer is missing or the plan changed since `--hash`; it refuses round 4 (exit 1, stop and ask) unless `--extra-round` follows the operator's yes, and refuses that flag before the cap is spent. Unrated findings count as blockers. The review log is excluded from the hash only up to its first other heading, so a step placed under it is hashed. `--accept-open <plan.md>` records the operator's "build as is" for the current round's open MAJORs (`accepted_open` in the verdict block, bound to the round and the plan hash; only after the last round; never for a BLOCKER; exit 1 when rounds are left, the verdict is stale, a blocker is open or no major is). Operator decision 2026-10-08: open majors used to be warnings, and a weakness the critics named was built and came back from `/trekreview` as MAJOR. plan-critic and scope-guardian rate on the review catalogue's tiers. The verdict is a trace of the review, not a signature.
|
||||
|
||||
|
|
|
|||
|
|
@ -263,6 +263,24 @@ export function parseStepFields(body) {
|
|||
return fields;
|
||||
}
|
||||
|
||||
// ---- `On failure:` (K11) ---------------------------------------------------
|
||||
|
||||
// The recovery words /trekexecute reads from a step's `On failure:` ("Parse
|
||||
// steps"). Free text that opens with none of them left the executor to guess:
|
||||
// "retry once; if still failing, revert ... and STOP" met a ladder of fix rounds
|
||||
// (voyage-lekerepo trial 2, 2026-10-08).
|
||||
export const ON_FAILURE_ACTIONS = Object.freeze(['revert', 'retry', 'skip', 'escalate']);
|
||||
|
||||
/**
|
||||
* The recovery word an `On failure:` text opens with, or null. Case-insensitive;
|
||||
* leading markdown (`**`, backticks, a sub-bullet's `-`) is stripped.
|
||||
*/
|
||||
export function onFailureAction(text) {
|
||||
const s = String(text ?? '').trim().replace(/^[*_`\s-]+/, '').toLowerCase();
|
||||
const m = s.match(/^([a-z]+)\b/);
|
||||
return m && ON_FAILURE_ACTIONS.includes(m[1]) ? m[1] : null;
|
||||
}
|
||||
|
||||
// ---- sections (fence-aware) ------------------------------------------------
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -134,9 +134,50 @@ function checkLineRange(finding, index, repoRoot, errors) {
|
|||
));
|
||||
}
|
||||
|
||||
// The same check for a `path:line` (or `path:a-b`) a reviewer writes in prose:
|
||||
// a finding's title/detail/recommended_action/brief_ref, and the text around
|
||||
// the JSON block. code-correctness-reviewer wrote "`tests/contract.sh:119`
|
||||
// relies on git" about a 33-line file, and nothing looked (voyage-lekerepo
|
||||
// trial 2, 2026-10-08). In a finding's fields a line past the end of a file
|
||||
// that exists is an error (the re-ask can fix the block); in the prose around
|
||||
// the block it is a warning (see validateReviewerOutput). A path absent from
|
||||
// the tree is not judged. The path needs an extension, so `localhost:8080` and
|
||||
// `host.example:443` are no citation of anything in the tree; a path glued to
|
||||
// a `\` or `:` (a Windows path) is not read as one relative to the root.
|
||||
const PROSE_CITE = /(?<![\w./\\:-])((?:\.{0,2}\/)?[\w.-]+(?:\/[\w.-]+)*\.[A-Za-z0-9]+):(\d+)(?:[-–—](\d+))?(?![\w])/g;
|
||||
const PROSE_FIELDS = Object.freeze(['title', 'detail', 'recommended_action', 'brief_ref']);
|
||||
|
||||
function checkProseCitations(text, where, repoRoot, errors, seen) {
|
||||
if (typeof text !== 'string' || text === '') return;
|
||||
PROSE_CITE.lastIndex = 0;
|
||||
let m;
|
||||
while ((m = PROSE_CITE.exec(text)) !== null) {
|
||||
const line = Math.max(Number(m[2]), m[3] === undefined ? 0 : Number(m[3]));
|
||||
if (line <= 0) continue;
|
||||
const n = lineCount(repoRoot, m[1]);
|
||||
if (n === null || line <= n) continue;
|
||||
const key = `${where}\u0000${m[0]}`;
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
errors.push(issue(
|
||||
'FINDING_PROSE_LINE_OUT_OF_RANGE',
|
||||
`${where} cites ${m[0]}, but ${m[1]} has ${n} lines`,
|
||||
'Cite the line number in the file itself (Read it), not the line in the diff or patch, or drop the line number.',
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
function checkFindingProse(finding, index, repoRoot, errors, seen) {
|
||||
if (!finding || typeof finding !== 'object') return;
|
||||
for (const field of PROSE_FIELDS) {
|
||||
checkProseCitations(finding[field], `findings[${index}].${field}`, repoRoot, errors, seen);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {unknown} payload
|
||||
* @param {{ repoRoot?: string }} [opts] - with a repo root, cited lines are checked against the files
|
||||
* @param {{ repoRoot?: string }} [opts] - with a repo root, cited lines are checked against the files:
|
||||
* each finding's `file:line`, and every `path:line` in its prose fields
|
||||
*/
|
||||
export function validateFindings(payload, opts = {}) {
|
||||
if (payload === null || typeof payload !== 'object' || Array.isArray(payload)) {
|
||||
|
|
@ -155,9 +196,13 @@ export function validateFindings(payload, opts = {}) {
|
|||
return { valid: false, errors, warnings, parsed: payload };
|
||||
}
|
||||
|
||||
const seen = new Set();
|
||||
for (let i = 0; i < payload.findings.length; i++) {
|
||||
validateFinding(payload.findings[i], i, errors);
|
||||
if (opts.repoRoot) checkLineRange(payload.findings[i], i, opts.repoRoot, errors);
|
||||
if (opts.repoRoot) {
|
||||
checkLineRange(payload.findings[i], i, opts.repoRoot, errors);
|
||||
checkFindingProse(payload.findings[i], i, opts.repoRoot, errors, seen);
|
||||
}
|
||||
}
|
||||
|
||||
return { valid: errors.length === 0, errors, warnings, parsed: payload };
|
||||
|
|
@ -185,7 +230,17 @@ export function validateReviewerOutput(rawText, opts = {}) {
|
|||
} catch (e) {
|
||||
return fail(issue('FINDINGS_PARSE_ERROR', `Reviewer JSON block did not parse: ${e.message}`));
|
||||
}
|
||||
return validateFindings(parsed, opts);
|
||||
const r = validateFindings(parsed, opts);
|
||||
if (!opts.repoRoot) return r;
|
||||
// The prose around the block (the text outside the LAST json fence; the
|
||||
// block itself was checked field by field above) is a WARNING, not an error:
|
||||
// the bounded re-ask asks for the JSON block only, which cannot correct that
|
||||
// prose, and nothing downstream reads it — an error there could only stop a
|
||||
// review over text no one uses (independent review, 2026-10-08).
|
||||
const at = rawText.lastIndexOf(block);
|
||||
const found = [];
|
||||
checkProseCitations(rawText.slice(0, at) + rawText.slice(at + block.length), 'reviewer prose', opts.repoRoot, found, new Set());
|
||||
return found.length === 0 ? r : { ...r, warnings: [...r.warnings, ...found] };
|
||||
}
|
||||
|
||||
// ---- CLI shim ----------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -3,9 +3,17 @@
|
|||
// merges plan-critic + scope-guardian findings into a single deduplicated
|
||||
// stream, preserving provenance (which agent originally raised each finding).
|
||||
//
|
||||
// Two dedup signals:
|
||||
// Three dedup signals:
|
||||
// 1. Exact match — identical computeFindingId(file:line:rule_key) → merge.
|
||||
// 2. Jaccard ≥ 0.7 on text-token sets → merge (catches near-duplicates).
|
||||
// 3. Same anchor — the same file and the same line (≥ 1), and the content
|
||||
// words of the shorter text mostly in the longer one (overlap ≥ the same
|
||||
// 0.7) → merge. Two reviewers naming one defect at one place word it
|
||||
// differently and pick their own rule_key: trial 2's pair on plan.md:248
|
||||
// measured Jaccard 0.44 and stayed two (voyage-lekerepo, 2026-10-08).
|
||||
// Only an anchored pair from two different reviewers, the shorter text
|
||||
// holding ≥ 6 content words, is measured this way; the threshold for
|
||||
// every other pair is unchanged.
|
||||
//
|
||||
// Provenance is preserved on the surviving finding's `raised_by` array.
|
||||
// Severity is kept (normalized to blocker | major | minor, else null) and a
|
||||
|
|
@ -59,6 +67,54 @@ export function tokenize(text) {
|
|||
return text.toLowerCase().split(/\W+/).filter(t => t.length > 0);
|
||||
}
|
||||
|
||||
// Words that carry no content of a finding.
|
||||
const STOP_WORDS = new Set([
|
||||
'a', 'an', 'the', 'and', 'or', 'but', 'of', 'in', 'on', 'at', 'to', 'for', 'with', 'by', 'from', 'as',
|
||||
'is', 'are', 'was', 'were', 'be', 'been', 'it', 'its', 'this', 'that', 'these', 'those', 'only', 'both',
|
||||
'where', 'which', 'while', 'when', 'than', 'then', 'so', 'if', 'also', 'each', 'every', 'all', 'any',
|
||||
'unlike', 'own',
|
||||
]);
|
||||
|
||||
/** Content words of a finding: tokens minus stop words and single letters, a plural -s folded. */
|
||||
export function contentTokens(text) {
|
||||
return tokenize(text)
|
||||
.filter(t => t.length > 1 && !STOP_WORDS.has(t))
|
||||
.map(t => (t.length > 3 && t.endsWith('s') && !t.endsWith('ss') ? t.slice(0, -1) : t));
|
||||
}
|
||||
|
||||
/** |A ∩ B| / min(|A|, |B|): how much of the shorter text the longer one holds. */
|
||||
function overlap(a, b) {
|
||||
const A = new Set(a);
|
||||
const B = new Set(b);
|
||||
if (A.size === 0 || B.size === 0) return 0;
|
||||
let n = 0;
|
||||
for (const x of A) if (B.has(x)) n += 1;
|
||||
return n / Math.min(A.size, B.size);
|
||||
}
|
||||
|
||||
/** Same file and same line ≥ 1: line 0 (file-scoped) and a missing line anchor nothing. */
|
||||
function sameAnchor(a, b) {
|
||||
return typeof a.file === 'string' && a.file !== '' && a.file === b.file &&
|
||||
Number.isInteger(a.line) && a.line > 0 && a.line === b.line;
|
||||
}
|
||||
|
||||
// Overlap measures the shorter text against the longer, so a short text is
|
||||
// held by almost any longer one on its line ("The token is checked." inside
|
||||
// "The token is not checked before the write." scores 1.0). The anchor rule
|
||||
// therefore merges only a pair from two different reviewers — one reviewer's
|
||||
// two findings on one line are two findings by its own count — and only when
|
||||
// the shorter text names enough to be compared (independent review, 2026-10-08).
|
||||
export const ANCHOR_MIN_CONTENT_WORDS = 6;
|
||||
|
||||
function anchorDuplicate(f, s, threshold) {
|
||||
if (!sameAnchor(f, s)) return false;
|
||||
if (f.raised_by.some(a => s.raised_by.includes(a))) return false;
|
||||
const a = new Set(contentTokens(f.text));
|
||||
const b = new Set(contentTokens(s.text));
|
||||
if (Math.min(a.size, b.size) < ANCHOR_MIN_CONTENT_WORDS) return false;
|
||||
return meetsThreshold(overlap([...a], [...b]), threshold);
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize a single agent payload into an array of {agent, finding} pairs.
|
||||
* Tolerates missing payload (returns []).
|
||||
|
|
@ -98,7 +154,7 @@ function annotate(finding, agent) {
|
|||
* findings: Array<object>,
|
||||
* sources: Array<{agent: string, present: boolean}>,
|
||||
* dedup_stats: { total_in: number, total_out: number,
|
||||
* exact_id_dups: number, jaccard_dups: number }
|
||||
* exact_id_dups: number, jaccard_dups: number, anchor_dups: number }
|
||||
* }}
|
||||
*/
|
||||
export function dedupFindings(sources, opts = {}) {
|
||||
|
|
@ -126,9 +182,11 @@ export function dedupFindings(sources, opts = {}) {
|
|||
}
|
||||
}
|
||||
|
||||
// Pass 2 — jaccard on text tokens; merge near-duplicates
|
||||
// Pass 2 — jaccard on text tokens; merge near-duplicates. Pass 3 rides in
|
||||
// the same loop: an anchored pair that Jaccard misses is measured by overlap.
|
||||
const survivors = [];
|
||||
let jaccard_dups = 0;
|
||||
let anchor_dups = 0;
|
||||
for (const f of byId.values()) {
|
||||
const tokens = tokenize(f.text);
|
||||
let merged = false;
|
||||
|
|
@ -140,6 +198,12 @@ export function dedupFindings(sources, opts = {}) {
|
|||
merged = true;
|
||||
break;
|
||||
}
|
||||
if (anchorDuplicate(f, s, threshold)) {
|
||||
mergeInto(s, f);
|
||||
anchor_dups += 1;
|
||||
merged = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!merged) survivors.push(f);
|
||||
}
|
||||
|
|
@ -158,6 +222,7 @@ export function dedupFindings(sources, opts = {}) {
|
|||
total_out: survivors.length,
|
||||
exact_id_dups,
|
||||
jaccard_dups,
|
||||
anchor_dups,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,14 @@
|
|||
// criteria runner's allowlist, and each `End-to-end:` command also stands
|
||||
// under `## Verification`, where Phase 7 runs it.
|
||||
// Strict mode reports these as errors; --soft downgrades them to warnings.
|
||||
//
|
||||
// Every plan version (the fields are new; a plan without them is unchanged):
|
||||
// - K10: the optional step field `Regression:` holds ONE command the
|
||||
// criteria runner's allowlist runs (first backticked span, no pipe, no
|
||||
// `&&`); the executor runs it green before the step's checkpoint. One that
|
||||
// can never run is an error (a warning under --soft).
|
||||
// - K11: `On failure:` opens with revert | retry | skip | escalate, the words
|
||||
// /trekexecute reads; any other opening is a warning.
|
||||
|
||||
import { readFileSync, existsSync, realpathSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
|
@ -29,6 +37,7 @@ import {
|
|||
sliceSteps, validatePlanHeadings, extractPlanVersion, extractPlanCeremony,
|
||||
parseStepFields, findSection, splitSections, findModuleCycle, parseMermaid, fencedBlocks,
|
||||
versionAtLeast, PLAN_VERSION_CURRENT, PLAN_VERSIONS_SUPPORTED, CEREMONY_VALUES,
|
||||
ON_FAILURE_ACTIONS, onFailureAction,
|
||||
} from '../parsers/plan-schema.mjs';
|
||||
import { validateAllManifests } from '../parsers/manifest-yaml.mjs';
|
||||
import { parseDocument } from '../util/frontmatter.mjs';
|
||||
|
|
@ -125,6 +134,36 @@ function checkRunnable(text, sections, opts) {
|
|||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* K10 and K11. `Verify:` is both the step's one test command and its proof of
|
||||
* red, so a step whose red test is a shell script could not also run the frozen
|
||||
* suite before its checkpoint; the plan wrote "run npm test before the
|
||||
* checkpoint" in prose that nothing enforced (voyage-lekerepo trial 2,
|
||||
* 2026-10-08). `Regression:` is that command, checked like `Verify:`.
|
||||
*/
|
||||
function checkRecovery(sections, opts) {
|
||||
const errors = [];
|
||||
const warnings = [];
|
||||
for (const s of sections) {
|
||||
const f = parseStepFields(s.body);
|
||||
if (f.has('Regression')) {
|
||||
const r = runnableCheck(firstCommand(f.get('Regression').text), opts);
|
||||
if (!r.runnable) {
|
||||
errors.push(issue('PLAN_STEP_REGRESSION_NOT_RUNNABLE',
|
||||
`Step ${s.n}: \`Regression:\` is not one runnable command (${r.reason})`,
|
||||
'Backtick ONE command the criteria runner allows, unpiped and without `&&` (`npm test`); ' +
|
||||
`the executor runs it green before the checkpoint. ${WAY_IN}`));
|
||||
}
|
||||
}
|
||||
if (f.has('On failure') && onFailureAction(f.get('On failure').text) === null) {
|
||||
warnings.push(issue('PLAN_STEP_ON_FAILURE_UNKNOWN',
|
||||
`Step ${s.n}: \`On failure:\` does not open with a recovery word the executor reads`,
|
||||
`Open with one of ${ON_FAILURE_ACTIONS.join(' | ')}, then the specifics (\`retry once, then escalate\`).`));
|
||||
}
|
||||
}
|
||||
return { errors, warnings };
|
||||
}
|
||||
|
||||
/** Table body rows (skips the header and the |---| separator). */
|
||||
function tableRows(text) {
|
||||
const rows = text.split('\n').filter(l => l.trim().startsWith('|'));
|
||||
|
|
@ -230,6 +269,11 @@ export function validatePlanContent(rawText, opts = {}) {
|
|||
warnings.push(issue('PLAN_VERSION_MISMATCH', `plan_version=${planVersion}, current target is ${PLAN_VERSION_CURRENT}`));
|
||||
}
|
||||
|
||||
const recovery = checkRecovery(sections, { cwd: opts.cwd });
|
||||
if (strict) errors.push(...recovery.errors);
|
||||
else warnings.push(...recovery.errors);
|
||||
warnings.push(...recovery.warnings);
|
||||
|
||||
const ceremony = extractPlanCeremony(text);
|
||||
if (versionAtLeast(planVersion, '1.8')) {
|
||||
const v18 = [...checkSteps(sections), ...checkRunnable(text, sections, { cwd: opts.cwd })];
|
||||
|
|
|
|||
|
|
@ -610,7 +610,10 @@ export function formatCriteriaEvidence(rep) {
|
|||
for (const r of rep.results) {
|
||||
const cmd = r.command ? `\`${cell(r.command)}\`` : '(none)';
|
||||
const exit = r.exitCode === null ? '—' : `exit ${r.exitCode}`;
|
||||
const evidence = r.status === 'passed' ? '—' : cell(firstLine(r.output)) || '—';
|
||||
// The first line of output on every row, a PASS too: /trekreview tells
|
||||
// the reviewer it gets "the command, exit code and first line of output",
|
||||
// and a PASS row said "—" (voyage-lekerepo trial 2, 2026-10-08).
|
||||
const evidence = cell(firstLine(r.output)) || '—';
|
||||
out.push(`| ${r.label} | ${cmd} | ${MARK[r.status]} | ${exit} | ${evidence} |`);
|
||||
}
|
||||
const s = rep.summary;
|
||||
|
|
|
|||
|
|
@ -78,7 +78,12 @@ Optional per-step fields: `Interface in:` (what the step consumes, and which
|
|||
earlier step or existing code produces it), `Interface out:` (what it produces
|
||||
for later steps), `Review focus:` (inputs the brief implies but no test
|
||||
exercises yet, each pinned to a test; and any Global Constraint the step could
|
||||
break, quoted verbatim). A field that is present is never empty.
|
||||
break, quoted verbatim), and `Regression:` (ONE backticked command the criteria
|
||||
runner allows, no pipe and no `&&` — usually `npm test` — that the executor
|
||||
runs green before the checkpoint, on a step that can break existing tests;
|
||||
`Verify:` stays the step's own red-first test). A field that is present is
|
||||
never empty. `On failure:` opens with `revert`, `retry`, `skip` or `escalate`,
|
||||
then the specifics.
|
||||
|
||||
### Step 1: {description}
|
||||
|
||||
|
|
@ -92,6 +97,7 @@ break, quoted verbatim). A field that is present is never empty.
|
|||
- Verifies: {what the test checks}
|
||||
- Pattern: `path/to/existing-test.ts` *(follow this style)*
|
||||
- **Verify:** `{exact command}` → expected: `{output}`
|
||||
- **Regression:** `{npm test}` → expected: exit 0 *(optional)*
|
||||
- **On failure:** {revert | retry | skip | escalate} — {specific instructions}
|
||||
- **Checkpoint:** `git add {the step's Files and its test files} && git commit -m "{conventional commit message}"`
|
||||
- **Manifest:**
|
||||
|
|
|
|||
130
tests/lib/trial2-minor-prose-pins.test.mjs
Normal file
130
tests/lib/trial2-minor-prose-pins.test.mjs
Normal file
|
|
@ -0,0 +1,130 @@
|
|||
// tests/lib/trial2-minor-prose-pins.test.mjs
|
||||
// Prose pins for the minor points of the second voyage-lekerepo trial
|
||||
// (2026-10-08, report parts B and C) whose fix is text a session reads.
|
||||
// Counted as doc pins by lib/util/test-census.mjs (the file name).
|
||||
// B8 minors left after the last critic round have a way to be fixed
|
||||
// (operator decision 2026-10-08: listed in the summary, the operator picks).
|
||||
// B9 K10 `Regression:` is described where plans are written and reviewed.
|
||||
// B10 K11 the four `On failure:` words are named where plans are written.
|
||||
// B12 /trekbrief Step 4c no longer claims a foreground, blocking spawn.
|
||||
// C13 the codebase-size count in /trekplan Phase 4 counts .mjs/.cjs/.mts/.cts.
|
||||
// C17 git-historian leaves the brief's frozen files alone and names the
|
||||
// remote from `git remote -v`.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFileSync, mkdtempSync, writeFileSync, rmSync } 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';
|
||||
import { findSection } from '../../lib/parsers/plan-schema.mjs';
|
||||
|
||||
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
const read = (p) => readFileSync(join(ROOT, p), 'utf8');
|
||||
|
||||
/** The text of a level-2 section, up to the next `## ` heading outside a fence. */
|
||||
function section(text, heading) {
|
||||
const s = findSection(text, heading.replace(/^## /, ''), 2);
|
||||
assert.ok(s, `missing ${heading}`);
|
||||
return s.text;
|
||||
}
|
||||
|
||||
// --- B8 ------------------------------------------------------------------------
|
||||
|
||||
test('B8: a PASS with minors sends them to the Phase 10 summary, numbered, for the operator to pick', () => {
|
||||
const plan = read('commands/trekplan.md');
|
||||
const p9 = section(plan, '## Phase 9 — Adversarial review (bounded critic loop)');
|
||||
assert.match(p9, /\*\*PASS\*\*[\s\S]{0,600}minors[\s\S]{0,300}Phase 10/i);
|
||||
const p10 = section(plan, '## Phase 10 — Present and refine');
|
||||
assert.match(p10, /\*\*Open minors:\*\*/);
|
||||
assert.match(p10, /numbered/i);
|
||||
});
|
||||
|
||||
test('B8: fixing picked minors after the last round runs one more round with --extra-round, the pick being the yes', () => {
|
||||
const p10 = section(read('commands/trekplan.md'), '## Phase 10 — Present and refine');
|
||||
assert.match(p10, /--extra-round/);
|
||||
assert.match(p10, /pick(ed|s)?\s+(is|counts as)\s+(the|their)\s+yes/i);
|
||||
});
|
||||
|
||||
// --- B9 / B10 --------------------------------------------------------------------
|
||||
|
||||
test('B9: /trekplan Phase 8 names `Regression:` as an allowed optional field and says what it is for', () => {
|
||||
const p8 = section(read('commands/trekplan.md'), '## Phase 8 — Deep planning');
|
||||
assert.match(p8, /`Regression:`/);
|
||||
assert.match(p8, /PLAN_STEP_REGRESSION_NOT_RUNNABLE/);
|
||||
assert.match(p8, /\*\*Regression:\*\* `npm test`/, 'the canonical example shows the field');
|
||||
assert.match(p8, /before (the|its) checkpoint/i);
|
||||
});
|
||||
|
||||
test('B9: the plan template shows `Regression:` as optional', () => {
|
||||
const t = read('templates/plan-template.md');
|
||||
assert.match(t, /- \*\*Regression:\*\* `[^\n]*\*\(optional\)\*/);
|
||||
assert.match(t, /`Regression:` \(ONE backticked command/);
|
||||
});
|
||||
|
||||
test('B9: plan-critic knows the field and does not demand the suite in `Verify:`', () => {
|
||||
const c = read('agents/plan-critic.md');
|
||||
assert.match(c, /`Regression:`/);
|
||||
assert.match(c, /(never|do\s+not)\s+(demand|require)\s+`npm test`\s+in\s+`Verify:`/i);
|
||||
});
|
||||
|
||||
test('B9/B10: Handover 4 documents `Regression:` (K10) and the `On failure:` words (K11)', () => {
|
||||
const h4 = section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 4 — `plan.md` → execute');
|
||||
for (const code of ['PLAN_STEP_REGRESSION_NOT_RUNNABLE', 'PLAN_STEP_ON_FAILURE_UNKNOWN']) assert.match(h4, new RegExp(code));
|
||||
assert.match(h4, /K10/);
|
||||
assert.match(h4, /K11/);
|
||||
assert.match(h4, /revert[^\n]*retry[^\n]*skip[^\n]*escalate/);
|
||||
});
|
||||
|
||||
test('B10: /trekplan Phase 8 says `On failure:` opens with one of the four words', () => {
|
||||
const p8 = section(read('commands/trekplan.md'), '## Phase 8 — Deep planning');
|
||||
assert.match(p8, /On failure:\*\*[^\n]*(opens|starts) with/i);
|
||||
assert.match(p8, /PLAN_STEP_ON_FAILURE_UNKNOWN/);
|
||||
});
|
||||
|
||||
// --- B12 -------------------------------------------------------------------------
|
||||
|
||||
test('B12: /trekbrief Step 4c does not call the brief-reviewer spawn foreground or blocking, and waits for it', () => {
|
||||
const t = read('commands/trekbrief.md');
|
||||
const start = t.indexOf('**Step 4c — Launch brief-reviewer**');
|
||||
const end = t.indexOf('**Step 4d — Parse JSON scores**');
|
||||
assert.ok(start !== -1 && end > start);
|
||||
const step = t.slice(start, end);
|
||||
assert.doesNotMatch(step, /foreground, blocking/);
|
||||
assert.match(step, /wait\s+for\s+(its|the agent's)\s+(result|completion)/i);
|
||||
assert.match(step, /(background|asynchronous)/i, 'says how the spawn actually runs');
|
||||
});
|
||||
|
||||
// --- C13 -------------------------------------------------------------------------
|
||||
|
||||
test('C13: the Phase 4 size count counts .mjs, .cjs, .mts and .cts files', () => {
|
||||
const p4 = section(read('commands/trekplan.md'), '## Phase 4 — Codebase sizing');
|
||||
const cmd = p4.match(/```\n(find [^\n]+)\n```/);
|
||||
assert.ok(cmd, 'the find command stands in a fence of its own');
|
||||
const dir = mkdtempSync(join(tmpdir(), 'voyage-sizing-'));
|
||||
try {
|
||||
for (const f of ['a.mjs', 'b.cjs', 'c.mts', 'd.cts', 'e.ts', 'skip.txt']) writeFileSync(join(dir, f), 'x\n');
|
||||
const r = spawnSync('/bin/sh', ['-c', cmd[1]], { cwd: dir, encoding: 'utf8' });
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.equal(r.stdout.trim(), '5');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// --- C17 -------------------------------------------------------------------------
|
||||
|
||||
test('C17: git-historian reads the brief, never proposes changing a file it freezes', () => {
|
||||
const g = read('agents/git-historian.md');
|
||||
assert.match(g, /brief/i);
|
||||
assert.match(g, /(never|do not) (propose|suggest|recommend)[^\n]*(change|edit)[^\n]*(frozen|unchanged)|(frozen|unchanged)[^\n]*(never|do not) (propose|suggest|recommend)/i);
|
||||
const p5 = section(read('commands/trekplan.md'), '## Phase 5 — Parallel exploration (specialized agents + research)');
|
||||
assert.match(p5, /\*\*git-historian\*\*[\s\S]{0,400}\{brief_path\}/, 'the spawn hands it the brief');
|
||||
});
|
||||
|
||||
test('C17: git-historian names the remote from `git remote -v`, never by guess', () => {
|
||||
const g = read('agents/git-historian.md');
|
||||
assert.match(g, /`git remote -v`/);
|
||||
assert.match(g, /local (path|repository|bare)/i);
|
||||
});
|
||||
253
tests/lib/trial2-minor.test.mjs
Normal file
253
tests/lib/trial2-minor.test.mjs
Normal file
|
|
@ -0,0 +1,253 @@
|
|||
// tests/lib/trial2-minor.test.mjs
|
||||
// The minor points of the second voyage-lekerepo trial (2026-10-08, report
|
||||
// parts B and C) that are behaviour, not prose. Each test was red before its
|
||||
// fix.
|
||||
// B9 K10 — optional step field `Regression:`: one runnable command, or the
|
||||
// validator refuses it.
|
||||
// B10 K11 — `On failure:` opens with revert | retry | skip | escalate, or the
|
||||
// validator warns.
|
||||
// B11 two findings on the same file:line with the same content merge (the
|
||||
// trial's pair measured Jaccard 0.44 against a 0.7 threshold).
|
||||
// C14 `--evidence` shows the first output line on a PASS row, not "—".
|
||||
// C16 a file:line cited in a reviewer's prose is checked against the file.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
|
||||
import { onFailureAction, ON_FAILURE_ACTIONS } from '../../lib/parsers/plan-schema.mjs';
|
||||
import { dedupFindings, DEFAULT_THRESHOLD } from '../../lib/review/plan-review-dedup.mjs';
|
||||
import { formatCriteriaEvidence } from '../../lib/verification/criteria-runner.mjs';
|
||||
import { validateFindings, validateReviewerOutput } from '../../lib/review/findings-schema.mjs';
|
||||
import { buildPlan } from '../helpers/plan-v18-builder.mjs';
|
||||
|
||||
const codes = (list) => list.map((e) => e.code);
|
||||
|
||||
// Step 1's Verify line in the builder's plan; the new field goes right under it.
|
||||
const STEP1_VERIFY = '- **Verify:** `node --test tests/mod1.test.mjs` → expected: pass';
|
||||
const withRegression = (line, plan = buildPlan()) => plan.replace(STEP1_VERIFY, `${STEP1_VERIFY}\n${line}`);
|
||||
const withOnFailure = (text, plan = buildPlan()) =>
|
||||
plan.replace(/^- \*\*On failure:\*\* .*$/m, `- **On failure:** ${text}`);
|
||||
|
||||
// --- B9: K10 `Regression:` ----------------------------------------------------
|
||||
|
||||
test('K10: `Regression: npm test | tee x` is refused — one command, unpiped', () => {
|
||||
const r = validatePlanContent(withRegression('- **Regression:** `npm test | tee x` → expected: exit 0'));
|
||||
assert.equal(r.valid, false);
|
||||
const e = r.errors.find((x) => x.code === 'PLAN_STEP_REGRESSION_NOT_RUNNABLE');
|
||||
assert.ok(e, codes(r.errors).join(','));
|
||||
assert.match(e.message, /Step 1/);
|
||||
assert.match(e.message, /shell operator/);
|
||||
});
|
||||
|
||||
test('K10: `Regression: npm test` is accepted', () => {
|
||||
const r = validatePlanContent(withRegression('- **Regression:** `npm test` → expected: exit 0'));
|
||||
assert.deepEqual(codes(r.errors), []);
|
||||
assert.ok(!codes(r.warnings).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'));
|
||||
});
|
||||
|
||||
test('K10: without the field the plan validates exactly as before', () => {
|
||||
const r = validatePlanContent(buildPlan());
|
||||
assert.equal(r.valid, true, codes(r.errors).join(','));
|
||||
assert.deepEqual(codes(r.warnings), []);
|
||||
});
|
||||
|
||||
test('K10: `npm test && bash tests/e2e.sh`, a bare word, or no backticks is refused', () => {
|
||||
for (const line of [
|
||||
'- **Regression:** `npm test && bash tests/e2e.sh`',
|
||||
'- **Regression:** `./run-all-tests`',
|
||||
'- **Regression:** npm test',
|
||||
'- **Regression:**',
|
||||
]) {
|
||||
const r = validatePlanContent(withRegression(line));
|
||||
assert.ok(codes(r.errors).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'), `${line}: ${codes(r.errors).join(',')}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('K10: --soft reports an unrunnable Regression as a warning, not an error', () => {
|
||||
const r = validatePlanContent(withRegression('- **Regression:** `npm test | tee x`'), { strict: false });
|
||||
assert.equal(r.valid, true, codes(r.errors).join(','));
|
||||
assert.ok(codes(r.warnings).includes('PLAN_STEP_REGRESSION_NOT_RUNNABLE'));
|
||||
});
|
||||
|
||||
// --- B10: K11 `On failure:` ---------------------------------------------------
|
||||
|
||||
test('K11: `On failure: try again` warns, and the plan stays valid', () => {
|
||||
const r = validatePlanContent(withOnFailure('try again'));
|
||||
assert.equal(r.valid, true, codes(r.errors).join(','));
|
||||
const w = r.warnings.find((x) => x.code === 'PLAN_STEP_ON_FAILURE_UNKNOWN');
|
||||
assert.ok(w, codes(r.warnings).join(','));
|
||||
assert.match(w.message, /Step 1/);
|
||||
for (const word of ON_FAILURE_ACTIONS) assert.match(w.hint, new RegExp(word));
|
||||
});
|
||||
|
||||
test('K11: `On failure: retry once, then escalate` gives no warning', () => {
|
||||
const r = validatePlanContent(withOnFailure('retry once, then escalate'));
|
||||
assert.ok(!codes(r.warnings).includes('PLAN_STEP_ON_FAILURE_UNKNOWN'), codes(r.warnings).join(','));
|
||||
});
|
||||
|
||||
test('K11: the four words, in any case and with markdown around them, are read; a longer word is not', () => {
|
||||
assert.deepEqual([...ON_FAILURE_ACTIONS], ['revert', 'retry', 'skip', 'escalate']);
|
||||
assert.equal(onFailureAction('revert — `git checkout -- x`'), 'revert');
|
||||
assert.equal(onFailureAction('Escalate: stop the run'), 'escalate');
|
||||
assert.equal(onFailureAction('**skip** this step'), 'skip');
|
||||
assert.equal(onFailureAction('`retry` once'), 'retry');
|
||||
assert.equal(onFailureAction('retrying is fine'), null);
|
||||
assert.equal(onFailureAction('try again'), null);
|
||||
assert.equal(onFailureAction(''), null);
|
||||
});
|
||||
|
||||
// --- B11: same file:line, same content ---------------------------------------
|
||||
|
||||
// The two findings from trial 2's round 1, verbatim (plan.md:248).
|
||||
const PLAN_MD = '.claude/projects/2026-10-08-slug-sep-option/plan.md';
|
||||
const CRITIC_248 = {
|
||||
severity: 'minor', rule_key: 'verification/contract-brief-form-missing', file: PLAN_MD, line: 248,
|
||||
text: "Verification runs `bash tests/e2e.sh` in the brief's own form but not `bash tests/contract.sh`, the form brief SC4 names.",
|
||||
};
|
||||
const GUARDIAN_248 = {
|
||||
severity: 'minor', rule_key: 'gap-verification-command-form', file: PLAN_MD, line: 248,
|
||||
text: "Brief SC4 runs 'bash tests/contract.sh'; Verification lists only '/bin/bash tests/contract.sh' (unlike e2e, where both forms are listed) - add the brief's form",
|
||||
};
|
||||
const sources = (a, b) => [
|
||||
{ agent: 'plan-critic', payload: { findings: a } },
|
||||
{ agent: 'scope-guardian', payload: { findings: b } },
|
||||
];
|
||||
|
||||
test('B11: the two trial-2 findings on plan.md:248 merge into one, raised by both', () => {
|
||||
const r = dedupFindings(sources([CRITIC_248], [GUARDIAN_248]));
|
||||
assert.equal(r.findings.length, 1, JSON.stringify(r.findings, null, 2));
|
||||
assert.deepEqual(r.findings[0].raised_by, ['plan-critic', 'scope-guardian']);
|
||||
assert.equal(r.dedup_stats.anchor_dups, 1);
|
||||
});
|
||||
|
||||
test('B11: two different findings on the same line stay two', () => {
|
||||
const other = {
|
||||
severity: 'minor', rule_key: 'verification/no-negative-case', file: PLAN_MD, line: 248,
|
||||
text: 'No criterion feeds an invalid separator such as a letter and checks the exit code.',
|
||||
};
|
||||
const r = dedupFindings(sources([CRITIC_248], [other]));
|
||||
assert.equal(r.findings.length, 2);
|
||||
});
|
||||
|
||||
test('B11: a short finding held by a longer one on the same line stays its own (independent review)', () => {
|
||||
const at = (text, severity = 'minor') => ({ severity, rule_key: `k-${text.length}`, file: PLAN_MD, line: 40, text });
|
||||
for (const [a, b] of [
|
||||
['The token is checked.', 'The token is not checked before the write.'],
|
||||
['Step 3 test is missing.', 'Step 3 is missing a checkpoint; the test file already exists.'],
|
||||
['Verify runs npm test.', 'Verify must not run npm test, it is green before the step.'],
|
||||
]) {
|
||||
assert.equal(dedupFindings(sources([at(a)], [at(b)])).findings.length, 2, `${a} | ${b}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('B11: one reviewer\'s two findings on one line are never merged by the anchor rule', () => {
|
||||
const r = dedupFindings([{ agent: 'plan-critic', payload: { findings: [CRITIC_248, { ...GUARDIAN_248, rule_key: 'other' }] } }]);
|
||||
assert.equal(r.findings.length, 2);
|
||||
assert.equal(r.dedup_stats.anchor_dups, 0);
|
||||
});
|
||||
|
||||
test('B11: the same content on another line is not merged by the anchor rule; the threshold elsewhere is unchanged', () => {
|
||||
assert.equal(DEFAULT_THRESHOLD, 0.7);
|
||||
const r = dedupFindings(sources([CRITIC_248], [{ ...GUARDIAN_248, line: 249 }]));
|
||||
assert.equal(r.findings.length, 2);
|
||||
const noLine = dedupFindings(sources([{ ...CRITIC_248, line: 0 }], [{ ...GUARDIAN_248, line: 0 }]));
|
||||
assert.equal(noLine.findings.length, 2, 'line 0 is file-scoped, not an anchor');
|
||||
});
|
||||
|
||||
// --- C14: --evidence on a PASS row ---------------------------------------------
|
||||
|
||||
const evidenceRep = (results) => ({
|
||||
source: 'brief.md',
|
||||
heading: '## Success Criteria',
|
||||
error: null,
|
||||
results,
|
||||
summary: { passed: 1, failed: 0, blocked: 0, unrunnable: 0, total: 1, ok: true },
|
||||
});
|
||||
|
||||
test('C14: --evidence shows the first line of output on a passed criterion', () => {
|
||||
const block = formatCriteriaEvidence(evidenceRep([
|
||||
{ label: 'SC1', text: 't', command: 'npm test', status: 'passed', exitCode: 0, output: '\n# tests 12\n# pass 12\n' },
|
||||
]));
|
||||
assert.match(block, /\| SC1 \| `npm test` \| PASS \| exit 0 \| # tests 12 \|/);
|
||||
});
|
||||
|
||||
test('C14: a passed criterion with no output still says "—"', () => {
|
||||
const block = formatCriteriaEvidence(evidenceRep([
|
||||
{ label: 'SC1', text: 't', command: 'npm test', status: 'passed', exitCode: 0, output: '' },
|
||||
]));
|
||||
assert.match(block, /\| SC1 \| `npm test` \| PASS \| exit 0 \| — \|/);
|
||||
});
|
||||
|
||||
// --- C16: file:line in prose --------------------------------------------------
|
||||
|
||||
function repoWith33() {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'voyage-prose-cite-'));
|
||||
mkdirSync(join(dir, 'tests'));
|
||||
writeFileSync(join(dir, 'tests', 'x.sh'), Array.from({ length: 33 }, (_, i) => `echo ${i + 1}`).join('\n') + '\n');
|
||||
return dir;
|
||||
}
|
||||
|
||||
const finding = (over = {}) => ({
|
||||
id: '0123456789abcdef0123456789abcdef01234567',
|
||||
severity: 'MAJOR', rule_key: 'UNIMPLEMENTED_CRITERION', file: 'tests/x.sh', line: 3,
|
||||
title: 'A title', detail: 'A detail.', recommended_action: 'Do one thing.', ...over,
|
||||
});
|
||||
|
||||
test('C16: a finding whose prose cites `tests/x.sh:119` in a 33-line file is refused', () => {
|
||||
const dir = repoWith33();
|
||||
try {
|
||||
const r = validateFindings(
|
||||
{ reviewer: 'code-correctness-reviewer', findings: [finding({ detail: 'The check at `tests/x.sh:119` relies on git.' })] },
|
||||
{ repoRoot: dir },
|
||||
);
|
||||
assert.equal(r.valid, false);
|
||||
const e = r.errors.find((x) => x.code === 'FINDING_PROSE_LINE_OUT_OF_RANGE');
|
||||
assert.ok(e, JSON.stringify(r.errors));
|
||||
assert.match(e.message, /tests\/x\.sh:119/);
|
||||
assert.match(e.message, /33 lines/);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('C16: the same citation in prose outside the JSON block is a warning, never silent (a JSON-only re-ask cannot fix prose)', () => {
|
||||
const dir = repoWith33();
|
||||
try {
|
||||
const raw = [
|
||||
'### Risk noted but not raised as a finding',
|
||||
'',
|
||||
'`tests/x.sh:119` relies on `git` and on a fixed commit.',
|
||||
'',
|
||||
'```json',
|
||||
JSON.stringify({ reviewer: 'code-correctness-reviewer', findings: [] }),
|
||||
'```',
|
||||
].join('\n');
|
||||
const r = validateReviewerOutput(raw, { repoRoot: dir });
|
||||
assert.equal(r.valid, true, JSON.stringify(r.errors));
|
||||
const w = r.warnings.find((x) => x.code === 'FINDING_PROSE_LINE_OUT_OF_RANGE');
|
||||
assert.ok(w, JSON.stringify(r.warnings));
|
||||
assert.match(w.message, /reviewer prose cites `?tests\/x\.sh:119/);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('C16: a line that exists, a range ending in the file, a file absent from the tree and no repo root all pass', () => {
|
||||
const dir = repoWith33();
|
||||
try {
|
||||
const ok = (detail, opts = { repoRoot: dir }) =>
|
||||
validateFindings({ reviewer: 'r', findings: [finding({ detail })] }, opts).valid;
|
||||
assert.equal(ok('See tests/x.sh:33 and tests/x.sh:10-12.'), true);
|
||||
assert.equal(ok('See src/absent.mjs:900.'), true);
|
||||
assert.equal(ok('See tests/x.sh:119.', {}), true);
|
||||
assert.equal(ok('See tests/x.sh:30-40.'), false, 'a range that runs past the end is out of range');
|
||||
assert.equal(ok('Served on http://localhost:8080 and example.com:443.'), true);
|
||||
assert.equal(ok('On Windows: C:\\work\\tests\\x.sh:119 and tests\\x.sh:119.'), true, 'a Windows path is not the root\'s file');
|
||||
assert.equal(ok('See tests/x.sh:30\u201440.'), false, 'an em-dash range is read as a range');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue