merge: trekexecute converging-review-loops
This commit is contained in:
commit
be9e293267
24 changed files with 3224 additions and 75 deletions
78
CHANGELOG.md
78
CHANGELOG.md
|
|
@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file.
|
|||
|
||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
## Unreleased — plan_version 1.9, the trial-4 findings, session-clean tests
|
||||
## Unreleased — plan_version 1.9, the trial-4 findings, session-clean tests, the converging critic loop
|
||||
|
||||
A new plan version, 1.9, in which every path in a step's `Files:` and on its
|
||||
`Test first:` `File:` line stands first, in backticks, so the reader no longer
|
||||
|
|
@ -15,6 +15,11 @@ failed on the code before it. Suite 2517 (2515 pass, 0 fail, 2 skipped),
|
|||
measured by `npm test` on the third remediation's Step 2 commit
|
||||
(`829da48`), the commit before this section.
|
||||
|
||||
The converging critic loop came after that measurement (decisions 43 and
|
||||
44). From round 2 the plan gate counts a critic round by rule A, and both
|
||||
reviewers state rule B. Suite 2602 (2600 pass, 0 fail, 2 skipped), measured
|
||||
by `npm test` on this branch.
|
||||
|
||||
Nothing here asks anything of users. Older plans warn, they do not fail: a plan
|
||||
below 1.9 is read as in 6.3.1, apart from the fixes under Fixed, and gets one
|
||||
warning per step whose `Files:` names a path that is not backticked.
|
||||
|
|
@ -36,6 +41,13 @@ warning per step whose `Files:` names a path that is not backticked.
|
|||
- two-way: original_severity names what the kept finding asserted — only tests read the field.
|
||||
- two-way: the workflow review fails closed when its coordinator returns nothing, and names every reviewer that did not run in allow_blocked_by — opt-in path only.
|
||||
- two-way: below 1.8, when the plan names no test file, the helper and the executor give red proof the files the helper wrote (operator decision 41) — a revert leaves such a step unproven and unlocked.
|
||||
- two-way: rule A in the plan gate (decision 43) — an older gate ignores the new verdict fields, and a revert counts every round fully.
|
||||
- two-way: rule B and the security key in the reviewers (decision 43) — prose only; a revert restores the old rating.
|
||||
- two-way: plan dedup passes the reviewers' marks and rule-B fields through, and its two fixes — a revert drops the fields and brings back the line-0 collapse and the crash.
|
||||
- two-way: the post-compact hook test's timeout (decision 44) — test-side only.
|
||||
- two-way: the critic-loop matcher needs overlapping texts, and a new finding never takes an old id (operator decision 48) — a revert matches by id or line alone again.
|
||||
- two-way: plan dedup keeps a lowered_from that is not a severity (operator decision 48) — a revert counts it as no lowering again.
|
||||
- two-way: plan dedup ranks a merged pair by what rule B counts (operator decision 48) — a revert brings back the order-dependent merge.
|
||||
|
||||
### Added
|
||||
|
||||
|
|
@ -61,6 +73,28 @@ warning per step whose `Files:` names a path that is not backticked.
|
|||
`brief_lowered_from` on any other finding, a mark on a reviewer finding, a
|
||||
mark or an engine finding below its catalogue tier, or a mark with a missing
|
||||
or unknown rule key.
|
||||
- **The critic loop converges by rule A, counted by the plan gate**
|
||||
(decision 43). Round 1 is unchanged. From round 2 on,
|
||||
`plan-gate.mjs --diff` prints what changed since the text the last round
|
||||
reviewed. Both reviewers get it and mark each previous finding by id. Then
|
||||
`--record` counts the round: a new major that cites a plan line outside
|
||||
that diff counts as minor. A blocker, a security finding and a finding
|
||||
with no plan line count at full severity. A previous finding stays counted
|
||||
until it is marked resolved. The gate keeps each recorded round's text in
|
||||
the git directory, so a snapshot is never committed. A plan outside any git
|
||||
repository keeps none, and its next round counts fully. Every new verdict
|
||||
field is optional, and absent means round-1 behaviour, so an older verdict
|
||||
block gives the same exit codes. Phase 9, Handover 4, the planning
|
||||
orchestrator, CLAUDE.md and the README say so.
|
||||
- **Rule B in both reviewers** (decision 43). plan-critic and scope-guardian
|
||||
state it: a major that a named test, `Regression:` command or gate in the
|
||||
plan will fail on during execution is minor, never a blocker and never a
|
||||
security finding. The gate records each such lowering from round 1 on.
|
||||
Plan dedup passes the reviewers' marks and rule-B fields through. The
|
||||
operator answered three questions on 2026-10-15: rule B applies from round
|
||||
1 and rule A from round 2; both reviewers tag a security finding with the
|
||||
review catalogue's security key; a lowered finding that is re-reported as
|
||||
major stays minor, and only a re-report as blocker counts as blocker.
|
||||
|
||||
### Fixed
|
||||
|
||||
|
|
@ -211,6 +245,31 @@ warning per step whose `Files:` names a path that is not backticked.
|
|||
file from the plan and answers `no test files given`. The helper then runs
|
||||
the same red proof again with `--tests` naming the files it wrote, and the
|
||||
executor gives red proof that same list, so such a step is proven and locked.
|
||||
- **The post-compact hook test survives a slow start** (decision 44). Its
|
||||
hook child had a fixed 5 s timeout and was killed under full-suite load.
|
||||
The timeout now comes from `tests/helpers/timing.mjs`, and a killed hook
|
||||
child reads as a failure instead of exit 0.
|
||||
- **Plan dedup keeps distinct file-level findings apart, and no longer
|
||||
crashes on a bad field** (decision 43). Findings of one rule key on line 0
|
||||
or with no line no longer collapse into one. An empty `file` or a `line`
|
||||
that is not a whole number no longer crashes the finding id; the line
|
||||
becomes null and the finding stands.
|
||||
- **The critic loop no longer folds a new finding into an old one by its id
|
||||
or line alone** (reviews `4ab24b07`, `d03835c4`; operator decision 48,
|
||||
16.10).
|
||||
- What happened: an equal id or the old finding's line was enough. The new
|
||||
text was lost, and a new file-level major could count as an old rule-B
|
||||
minor.
|
||||
- Now the texts must overlap, a finding with a plan line is never matched
|
||||
by id, and a new finding never takes an old finding's id.
|
||||
- **A `lowered_from` that is not a severity now counts as blocker** (reviews
|
||||
`8782602b`, `ebe164e2`; operator decision 48, 16.10). It was read as no
|
||||
lowering. A blank one is still no lowering.
|
||||
- **Plan dedup no longer lets reviewer order lower a blocker** (review
|
||||
`69c9cebd`; operator decision 48, 16.10).
|
||||
- A merge keeps the copy rule B counts highest, on the merged keys.
|
||||
- This also covers a blocker claim merged into a plain major, found during
|
||||
this round.
|
||||
|
||||
### Known issues
|
||||
|
||||
|
|
@ -246,6 +305,23 @@ warning per step whose `Files:` names a path that is not backticked.
|
|||
- Accepted deviation: the 1.9 `File:` selector repeats the list-marker set of
|
||||
`LIST_ITEM` (review `4dce7c74`; operator decision 38, 14.10, decision 27
|
||||
stands).
|
||||
- The line diff's LCS table has no size bound. A rewritten gap of tens of
|
||||
thousands of lines takes seconds to minutes and gigabytes, and can end
|
||||
outside exit 0/1/2. Real plans measure about 1,200 lines (review
|
||||
`8a9b5a17`; deferred by operator decision 48, 16.10).
|
||||
- A previous finding re-reported on its own edited line is counted twice.
|
||||
Such a re-report of a lowered finding counts at its full severity (review
|
||||
`8004ea85`; deferred by operator decision 48, 16.10).
|
||||
- A `line` sent as a numeric string (`"42"`) is read as no line and counted
|
||||
at full severity. The agent templates give `line` as a number (review
|
||||
`c257625e`; deferred by operator decision 48, 16.10).
|
||||
- `npm test` does not run `tests/e2e-critic-diff-mode.sh`. It runs under
|
||||
`## Verification` and in `/trekreview`'s success-criteria check (review
|
||||
`818b1346`; deferred by operator decision 48, 16.10).
|
||||
- The matcher's residual (review `4ab24b07`; operator decision 50, 16.10): a
|
||||
previous finding marked resolved by every raiser and re-reported in other
|
||||
words (content overlap below 0.7) on an unchanged line is a new finding,
|
||||
which rule A can lower to minor.
|
||||
|
||||
## v6.3.1 — 2026-10-11 — Fixes from the release review of 6.3.0, and two verdict-gate holes closed
|
||||
|
||||
|
|
|
|||
|
|
@ -32,7 +32,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). The external agents preload the `researching-sources` skill and are told it by name at spawn | 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.9: the file first on every line — every `Files:` entry and `Test first:` `File:` line opens with its backticked path(s). From 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 |
|
||||
| `/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.9: the file first on every line — every `Files:` entry and `Test first:` `File:` line opens with its backticked path(s). From 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, rounds 2+ in diff mode (rule A, counted by the gate; rule B in the reviewers), 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: refuses a plan whose logged approval does not cover its current sha256 (K2); one fresh `step-implementer` helper per plan step, one controller that reads the diff (spec + quality verdicts), proof of red / floor guard / step check (K9) / `Regression:` (K10) / done line as commands, fix rounds that go up (max 5: helper → plan → operator), architecture rules (K5) + plan-file sweep (K6) after the last step, then its own review (`/simplify` → `/trekreview` → verdict gate, K7; BLOCK keeps the merge into main closed), 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 |
|
||||
|
|
@ -42,6 +42,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`.
|
||||
|
||||
> **The critic loop converges (decision 43).** Round 1 reviews the whole plan.
|
||||
> From round 2 the reviewers get `plan-gate.mjs --diff` and mark each previous
|
||||
> finding. The reviewers state rule B; the gate counts by rule A:
|
||||
> Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.
|
||||
|
||||
> **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.
|
||||
|
|
|
|||
|
|
@ -329,7 +329,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. 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).
|
||||
**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, an open BLOCKER or an unaccepted MAJOR) or 2 (could not run). From round 2 the gate counts by rule A: a new major outside the diff since the last reviewed text counts as minor, never a blocker or a security finding (Handover 4). 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.
|
||||
|
||||
|
|
|
|||
|
|
@ -275,12 +275,25 @@ result), or a behaviour the brief names with no test, is **major** — the tier
|
|||
of `MISSING_TEST` — never minor. Whenever a finding is the same class as a
|
||||
catalogue rule, use that rule's tier.
|
||||
|
||||
Rule B: a major that a named test, `Regression:` command or gate in the plan
|
||||
will fail on during execution is minor — never a blocker, never a security finding.
|
||||
The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong
|
||||
implementation catches nothing, so the finding stays major. Give
|
||||
`lowered_from` and `caught_by` when you lower a finding this way.
|
||||
|
||||
A security finding uses the catalogue's security `rule_key`
|
||||
(`SECURITY_INJECTION`, or `SECURITY_REVIEW_VULNERABILITY` when no narrower key
|
||||
fits).
|
||||
|
||||
Your rating decides what happens next: `/trekplan` records your findings with
|
||||
`plan-gate.mjs --record`, and the plan gate stops the plan while any BLOCKER
|
||||
or MAJOR is open — a MAJOR left after the last round goes to the operator
|
||||
(fix now, or build as is). A finding with no valid severity counts as a blocker. You may be run
|
||||
up to 3 times on successive revisions of the same plan; judge the plan in
|
||||
front of you, and do not repeat a finding the revision has fixed.
|
||||
up to 3 times on successive revisions of the same plan. From round 2 the
|
||||
prompt gives you the diff since the last reviewed text, and the plan's
|
||||
`## Critic Verdict` lists the previous findings. List every previous finding
|
||||
by id in `previous`, with `resolved: true` only when the plan now fixes it.
|
||||
Review the changed text. Cite the exact line, with `line_end` for a range.
|
||||
|
||||
Dimensions 11 and 12 are scored inside the existing weights: 11 under Step
|
||||
quality, 12 under Coverage completeness and Specification quality.
|
||||
|
|
@ -359,8 +372,11 @@ this block inline *is* the hand-off; do not attempt to write a file.
|
|||
```json
|
||||
{
|
||||
"agent": "plan-critic",
|
||||
"previous": [
|
||||
{ "id": "<40-hex id from the current ## Critic Verdict>", "resolved": false }
|
||||
],
|
||||
"findings": [
|
||||
{ "file": "<plan path or source file>", "line": 0, "rule_key": "<dimension/short-id>", "severity": "blocker|major|minor", "text": "<one-line finding>" }
|
||||
{ "file": "<plan path or source file>", "line": 0, "line_end": 0, "rule_key": "<dimension/short-id>", "severity": "blocker|major|minor", "text": "<one-line finding>", "lowered_from": "major", "caught_by": "<the named test, Regression: command or gate>" }
|
||||
]
|
||||
}
|
||||
```
|
||||
|
|
@ -370,6 +386,12 @@ path + the step's line) or the source the finding concerns; `rule_key` is a
|
|||
short, stable id for the finding class (used for exact-match dedup).
|
||||
`severity` is exactly one of `blocker`, `major`, `minor`. Emit
|
||||
`"findings": []` when the plan is clean.
|
||||
`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
|
||||
finding the current `## Critic Verdict` lists; `resolved` is `true` or
|
||||
`false`. `line_end` (optional) is the last line of a cited range, an integer
|
||||
≥ `line`. `lowered_from` and `caught_by` go only on a finding lowered by rule
|
||||
B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`
|
||||
command or gate that will fail on it; omit both otherwise.
|
||||
|
||||
Be specific. Reference exact plan sections, step numbers, and file paths.
|
||||
Never use "generally" or "usually" — cite the specific problem in this specific plan.
|
||||
|
|
|
|||
|
|
@ -440,9 +440,16 @@ The dedup output is piped on to `lib/validators/plan-gate.mjs --record
|
|||
<plan> --reviewed <hash>` (the hash `--hash` printed before the reviewers read
|
||||
the plan), which writes the round's verdict into the plan's `## Critic Verdict`.
|
||||
This is a loop of at most 3 rounds: revise, then review again; after round 3
|
||||
with a BLOCKER open, stop and ask the operator. `plan-gate.mjs <plan>` (K3)
|
||||
exits 0 only for a valid plan whose current text carries a verdict with no
|
||||
open BLOCKER (`commands/trekplan.md` Phase 9 is the full text).
|
||||
with a BLOCKER open, stop and ask the operator. Round 1 reviews the whole
|
||||
plan. Rounds 2+ run in diff mode: `plan-gate.mjs --diff <plan>` prints what
|
||||
changed since the text the last round reviewed, both reviewers get it and
|
||||
mark each previous finding by id, and the gate counts the round by rule A:
|
||||
|
||||
Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.
|
||||
|
||||
`plan-gate.mjs <plan>` (K3) exits 0 only for a valid plan whose current text
|
||||
carries a verdict with no open BLOCKER and no open MAJOR the operator has not
|
||||
accepted (`commands/trekplan.md` Phase 9 is the full text).
|
||||
|
||||
After both complete:
|
||||
- Address all blockers and major issues by revising the plan
|
||||
|
|
|
|||
|
|
@ -94,10 +94,27 @@ result), or a behaviour the brief names with no test, is **major** — the tier
|
|||
of `MISSING_TEST` — never minor. Whenever a finding is the same class as a
|
||||
catalogue rule, use that rule's tier.
|
||||
|
||||
Rule B: a major that a named test, `Regression:` command or gate in the plan
|
||||
will fail on during execution is minor — never a blocker, never a security finding.
|
||||
The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong
|
||||
implementation catches nothing, so the finding stays major. Give
|
||||
`lowered_from` and `caught_by` when you lower a finding this way.
|
||||
|
||||
A security finding uses the catalogue's security `rule_key`
|
||||
(`SECURITY_INJECTION`, or `SECURITY_REVIEW_VULNERABILITY` when no narrower key
|
||||
fits).
|
||||
|
||||
Your findings go through `plan-gate.mjs --record` with plan-critic's; an open
|
||||
blocker or major stops the plan. `severity` is exactly `blocker`, `major` or `minor`;
|
||||
an unrated finding counts as a blocker.
|
||||
|
||||
You may be run up to 3 times on successive revisions of the same plan. From
|
||||
round 2 the prompt gives you the diff since the last reviewed text, and the
|
||||
plan's `## Critic Verdict` lists the previous findings. List every previous
|
||||
finding by id in `previous`, with `resolved: true` only when the plan now
|
||||
fixes it. Review the changed text. Cite the exact line, with `line_end` for a
|
||||
range.
|
||||
|
||||
## Output format
|
||||
|
||||
```
|
||||
|
|
@ -142,8 +159,11 @@ this block inline *is* the hand-off; do not attempt to write a file.
|
|||
```json
|
||||
{
|
||||
"agent": "scope-guardian",
|
||||
"previous": [
|
||||
{ "id": "<40-hex id from the current ## Critic Verdict>", "resolved": false }
|
||||
],
|
||||
"findings": [
|
||||
{ "file": "<plan path or source file>", "line": 0, "rule_key": "<creep|gap|dependency|...>", "severity": "blocker|major|minor", "text": "<one-line finding>" }
|
||||
{ "file": "<plan path or source file>", "line": 0, "line_end": 0, "rule_key": "<creep|gap|dependency|...>", "severity": "blocker|major|minor", "text": "<one-line finding>", "lowered_from": "major", "caught_by": "<the named test, Regression: command or gate>" }
|
||||
]
|
||||
}
|
||||
```
|
||||
|
|
@ -152,6 +172,12 @@ One object per scope-creep / gap / dependency finding. `file`/`line` point at th
|
|||
plan step (or the brief requirement) the finding concerns; `rule_key` is a short,
|
||||
stable id for the finding class (used for exact-match dedup against plan-critic).
|
||||
Emit `"findings": []` when the plan is fully aligned.
|
||||
`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
|
||||
finding the current `## Critic Verdict` lists; `resolved` is `true` or
|
||||
`false`. `line_end` (optional) is the last line of a cited range, an integer
|
||||
≥ `line`. `lowered_from` and `caught_by` go only on a finding lowered by rule
|
||||
B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`
|
||||
command or gate that will fail on it; omit both otherwise.
|
||||
|
||||
## When to use — examples
|
||||
|
||||
|
|
|
|||
|
|
@ -921,10 +921,34 @@ approval.
|
|||
|
||||
Phase 9 is a loop of at most **3 critic rounds**. Each round reviews the plan
|
||||
as it stands, and a command — not this session's judgment — records the
|
||||
verdict and decides whether the plan may go on. The loop ends on a round with
|
||||
no BLOCKER; after round 3 with a BLOCKER still open it stops and asks the
|
||||
verdict and decides whether the plan may go on. A BLOCKER or an open MAJOR
|
||||
stops the plan: revise and run the next round. The loop is done on a round
|
||||
that leaves neither, or when the operator accepts the open MAJORs after the
|
||||
last round; after round 3 with a BLOCKER still open it stops and asks the
|
||||
operator. It never goes on silently.
|
||||
|
||||
**From round 2.** Round 1 is unchanged: the reviewers read the whole plan.
|
||||
From round 2 on, first print what changed since the text the last round
|
||||
reviewed:
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/validators/plan-gate.mjs --diff "{plan_path}"
|
||||
```
|
||||
|
||||
It prints `diff-base: <sha256> (round N)` and each changed hunk with the
|
||||
plan's full-file line numbers. Hand its output to both reviewers, with the
|
||||
round-2 sentence their prompts gain below. When it says `diff-base: none`,
|
||||
the round is a full review: the reviewers read the whole plan as in round 1,
|
||||
and rule A lowers nothing. The gate, not this
|
||||
session, counts the round by rule A:
|
||||
|
||||
Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.
|
||||
|
||||
The reviewers state rule B, and the gate records each lowering they make
|
||||
from round 1 on:
|
||||
|
||||
Rule B: a major that a named test, `Regression:` command or gate in the plan will fail on during execution is minor — never a blocker, never a security finding.
|
||||
|
||||
**One round:**
|
||||
|
||||
First take the hash of the plan the reviewers are about to read, and keep the
|
||||
|
|
@ -947,6 +971,8 @@ End your response with the REQUIRED machine-readable `json` findings block
|
|||
(schema in `agents/plan-critic.md`) so the orchestrator can pipe it to the
|
||||
dedup helper. You have no Write tool — return the block inline, do not write a file.
|
||||
Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)."
|
||||
From round 2 the prompt adds one sentence, followed by the `--diff` output:
|
||||
"Previous findings are in the plan's `## Critic Verdict`; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`."
|
||||
|
||||
**scope-guardian** — scope alignment check.
|
||||
Prompt: "Check this implementation plan against the brief.
|
||||
|
|
@ -958,6 +984,8 @@ plan's Verification section. End your response with the REQUIRED machine-readabl
|
|||
`json` findings block (schema in `agents/scope-guardian.md`). You have no Write
|
||||
tool — return the block inline, do not write a file.
|
||||
Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)."
|
||||
From round 2 the prompt adds one sentence, followed by the `--diff` output:
|
||||
"Previous findings are in the plan's `## Critic Verdict`; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`."
|
||||
|
||||
After both complete, **extract each reviewer's trailing `json` findings block**
|
||||
and pipe both into the dedup helper via **stdin** — the reviewers are read-only
|
||||
|
|
@ -974,6 +1002,15 @@ JSON
|
|||
|
||||
`plan-gate.mjs --record` prints one JSON line: `round`, `verdict`
|
||||
(`BLOCK` = a blocker is open, `REVISE` = majors only, `PASS`) and `counts`.
|
||||
It also prints `diff_base` (the sha256 of the text this round was counted
|
||||
against by rule A, or null), `diff_note` (one line: why a round from round 2
|
||||
on was counted fully), `lowered` (how many findings count below what a
|
||||
reviewer rated) and `snapshot` (`kept: …` or `not kept: <reason>`). The
|
||||
snapshot is the operative text this round reviewed, kept under
|
||||
`<git dir>/voyage/plan-snapshots/` and never committed; the next round's
|
||||
`--diff` reads it. A plan outside any git repository (plan mode) keeps no
|
||||
snapshot (`not kept: no git repository`), so its next round counts fully.
|
||||
A failed snapshot write does not fail the round.
|
||||
Exit 1 means the round cap is spent (see "After round 3"). Exit 2 means the
|
||||
round cannot be recorded: broken input, a reviewer missing from it (an absent
|
||||
review is not a clean one), or a plan that changed after `--hash` — the
|
||||
|
|
@ -986,8 +1023,8 @@ 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`,
|
||||
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
|
||||
shorter text's content words in the longer one), and keeps the copy rule B counts more severe
|
||||
(a refused lowering included). Revise the plan once for the merged set, not twice for the
|
||||
duplicates. Source: research/05 R1 + R2.
|
||||
|
||||
**After each round**, by the recorded verdict:
|
||||
|
|
|
|||
|
|
@ -265,7 +265,7 @@ prose form (relaxed in S26 so the template's prose emission parses).
|
|||
- at least one step has `End-to-end:` with a backticked command — the "run the thing" step;
|
||||
- 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.
|
||||
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. **Optional rule-A fields** (decision 43; `DIFF_FIELDS` in `lib/review/critic-diff.mjs`): each one is optional, and absent means round-1 behaviour, so an older block gives the same exit codes. Top level: `diff_base` (the sha256 the round was counted against), `diff_note` (one line: why a round from round 2 on was counted fully), `resolved` (the ids of previous findings that left the count). Per finding: `op_line` and `op_line_end` (the cited lines in the operative text), `line_end`, `rule_keys` (the union of the keys dedup merged), `original_severity` (what a reviewer rated, set whenever the counted `severity` differs), `lowered_by` (`rule A: outside the diff since round <P>` or `rule B: <caught_by>`), `caught_by` (the test, `Regression:` command or gate a rule-B lowering names), `lowering_refused` (why a rule-B lowering was refused) and `carried_from` (the round a carried finding first appeared). `severity` is always the counted severity. `recordVerdict` called without `base`, `baseNote` and `planPaths` records as before, plus the rule-B record, and a per-finding `rule_keys` from dedup is always passed through (an optional field).
|
||||
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`).
|
||||
|
|
@ -315,6 +315,12 @@ The strongest validation in the entire pipeline. Phase 5.5 (planning-orchestrato
|
|||
|
||||
**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.
|
||||
|
||||
**Rule A and the diff (decision 43).** Round 1 is unchanged. From round 2 on, `--record` counts the round against the operative text the previous round reviewed, and both reviewers mark each previous finding by id (`previous`). The gate counts by this rule, not the session:
|
||||
|
||||
Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.
|
||||
|
||||
Rule B is stated by the reviewers and recorded by the gate from round 1 (`lowered_by`, `caught_by`); it never lowers a blocker or a security finding. `--diff <plan.md>` prints what the next round reviews: `diff-base: <sha256> (round N)` and each hunk in the plan's full-file line numbers, or `diff-base: none — <reason>; review the whole plan` (exit 0; 2 = unreadable plan or bad arguments). `--record` keeps each recorded round's operative text as a snapshot, `<git dir>/voyage/plan-snapshots/<sha256>.md`, named by its own sha256. git never tracks its own directory, so a snapshot is never committed, whatever the repo's `.gitignore` or `artifact_store` says. A plan in no git repository (plan mode) keeps none (`snapshot: "not kept: no git repository"`), and its next round counts fully. A missing, oversized, misnamed or tampered snapshot, or a plan unchanged since the base, also means full counting, said in one `diff_note` line. `--record`'s JSON line adds `diff_base`, `diff_note`, `lowered` (a number) and `snapshot`. Trust limit: anything that can write the plan can also rewrite the previous verdict's `plan_sha256` and plant a matching snapshot. That was already the limit of the verdict block; rule A raises what such a forgery buys, and an unchanged base lowers nothing. The gate matches a previous finding only on overlapping text, and a new finding never takes a previous finding's id: a repeated id gets `<rule_key>#n`.
|
||||
|
||||
**Verification CLIs that read a plan** (0/1/2, JSON on stdout, 2 is never green): `lib/verification/arch-rules.mjs --repo <dir>` runs the target repo's own dependency-cruiser or import-linter (installs nothing; `--prove --rule <name> --plant <file>=<dest>` shows pass → fail on that rule → pass, refusing a destination that exists or resolves outside the repo); `lib/verification/plan-scope-sweep.mjs --plan <plan.md> --base <ref>` lists changed files no step names (`Files:`, `Test first:` File, manifest `expected_paths`) and no process path covers.
|
||||
|
||||
**Versioning:** v1.7 has been stable since v1.8.0 of the plugin (when literal-template + Phase 5.5 self-check were added to fix Opus 4.7 schema drift). v1.6 → v1.7 added the Manifest block (mandatory). v1.7 → v1.8 makes `Test first:` and the "run the thing" step required, adds the optional step fields and the design part, and was written validator-first with tests for both shapes (`tests/validators/plan-validator-v18.test.mjs`); 1.7 plans validate unchanged, so readers need no change (the executor already treats `plan_version ≥ 1.7` as strict). v1.8 → v1.9 gives `Files:` and the `Test first:` `File:` line a grammar, the file first on every line (invariant 8), so no reader has to tell a path from a description in free text; it was written validator-first (`tests/validators/plan-version-1-9.test.mjs`). Only a 1.9 plan is refused for it; 1.8 and 1.7 plans validate as before, with the warning `PLAN_STEP_PATH_UNTICKED`, and the 1.8 rules (invariant 5) bind 1.8 and 1.9 plans alike.
|
||||
|
|
|
|||
465
lib/review/critic-diff.mjs
Normal file
465
lib/review/critic-diff.mjs
Normal file
|
|
@ -0,0 +1,465 @@
|
|||
// lib/review/critic-diff.mjs
|
||||
// Rule A and the rule-B record for the plan gate (decision 43).
|
||||
//
|
||||
// Why: from round 2 on, the critic loop counts a new MAJOR that cites plan text
|
||||
// outside the diff since the last reviewed round as minor (rule A), so a loop of
|
||||
// critic rounds converges on what changed instead of re-litigating text that was
|
||||
// already reviewed. That needs a line diff between two operative plan texts
|
||||
// that is deterministic: the same two texts always give the same hunks, so the
|
||||
// same findings always give the same verdict.
|
||||
//
|
||||
// lineDiff(oldText, newText) -> { inside, oldToNew, hunks }
|
||||
// Lines are 1-based operative lines; CRLF is normalized first.
|
||||
// inside Set of every inserted or changed new line; for a pure-deletion
|
||||
// hunk also the new lines on both sides of the deletion point.
|
||||
// oldToNew oldToNew[o] = the new line an unchanged old line moved to, else
|
||||
// null (index 0 is null).
|
||||
// hunks [{oldStart, oldEnd, newStart, newEnd}], inclusive ranges in order;
|
||||
// an empty side has end = start - 1.
|
||||
// How: trim the common prefix and suffix; anchor on lines that occur exactly
|
||||
// once in both texts, keep the longest increasing subsequence of anchors and
|
||||
// recurse between them; a gap with no unique line uses an LCS table, ties
|
||||
// resolved toward matching the earliest old line.
|
||||
//
|
||||
// countRound({round, previous, base, baseNote, current, planPaths, findings, marks})
|
||||
// -> { findings, resolved, diffBase, diffNote, lowered }
|
||||
// Decides each finding's counted severity for one critic round (RULE_A,
|
||||
// RULE_B below). `current` = {text, sha256, fullToOp, opToFull}, built by the
|
||||
// caller from the plan gate's operativeLines and planHash; `base` = {sha256,
|
||||
// text} is the operative text the previous round reviewed, or null.
|
||||
// Rule A is on only from round 2, with a base whose sha256 is the previous
|
||||
// verdict's plan_sha256 and differs from the plan now. Otherwise the round is
|
||||
// counted as before (each finding at its reported severity, plus the rule-B
|
||||
// record) and `diffNote` says why in one line. With rule A on, a current
|
||||
// finding outside the diff is matched to a previous one only when their
|
||||
// texts overlap (`contentOverlap` ≥ `DEFAULT_THRESHOLD`) and the current one
|
||||
// sits at the previous one's mapped line, or (for a previous finding with no
|
||||
// plan line) has its id, or both texts hold `ANCHOR_MIN_CONTENT_WORDS`
|
||||
// content words in the same file class; a new finding never carries a
|
||||
// previous finding's id: a repeated id gets `<rule_key>#n`, as in dedup.
|
||||
// A previous finding stays counted until every reviewer that raised it marks
|
||||
// it resolved, no reviewer says it is not, and nobody re-reports it.
|
||||
// Carried findings come first (previous order), then new ones (dedup order).
|
||||
//
|
||||
// Trust limit: anything that can write the plan can rewrite the previous
|
||||
// verdict's plan_sha256 and plant a matching base snapshot. That was already
|
||||
// the stated limit of the verdict block; rule A raises what such a forgery
|
||||
// buys. An unchanged base lowers nothing.
|
||||
//
|
||||
// Pure: no I/O, and it never reads the plan gate (one dependency direction).
|
||||
//
|
||||
// Credit: patience-style anchoring (unique-line anchors kept by their longest
|
||||
// increasing subsequence), the idea behind Bram Cohen's patience diff; the code
|
||||
// here is written fresh.
|
||||
|
||||
import { resolve } from 'node:path';
|
||||
import {
|
||||
normalizeSeverity, contentOverlap, contentTokens, DEFAULT_THRESHOLD, ANCHOR_MIN_CONTENT_WORDS, SEVERITIES,
|
||||
isSecurityKey, keysOf, isSecurity, ruleB,
|
||||
} from './plan-review-dedup.mjs';
|
||||
import { computeFindingId } from '../parsers/finding-id.mjs';
|
||||
|
||||
export { isSecurityKey };
|
||||
|
||||
export const RULE_A = 'Rule A: from round 2 on, a new major finding that cites a plan line outside the diff since the last reviewed text counts as minor; a blocker, a security finding and a finding with no plan line count at full severity, and a previous finding stays counted until it is marked resolved.';
|
||||
export const RULE_B = 'Rule B: a major that a named test, `Regression:` command or gate in the plan will fail on during execution is minor — never a blocker, never a security finding.';
|
||||
|
||||
// The optional voyage.plan-verdict/1 fields of decision 43; absent means round-1 behaviour.
|
||||
export const DIFF_FIELDS = Object.freeze([
|
||||
// top level
|
||||
'diff_base', 'diff_note', 'resolved',
|
||||
// per finding
|
||||
'op_line', 'op_line_end', 'line_end', 'rule_keys', 'original_severity', 'lowered_by', 'caught_by',
|
||||
'lowering_refused', 'carried_from',
|
||||
]);
|
||||
|
||||
const toLines = (text) => {
|
||||
const t = String(text ?? '').replace(/\r\n?/g, '\n');
|
||||
if (t === '') return [];
|
||||
const out = t.split('\n');
|
||||
if (out[out.length - 1] === '') out.pop();
|
||||
return out;
|
||||
};
|
||||
|
||||
/** Indexes (into `items`) of a longest strictly increasing subsequence of items[k].b; earliest on ties. */
|
||||
function lisByB(items) {
|
||||
const tails = []; // tails[len-1] = index into items of the smallest tail
|
||||
const prev = new Array(items.length).fill(-1);
|
||||
for (let k = 0; k < items.length; k++) {
|
||||
const v = items[k].b;
|
||||
let lo = 0;
|
||||
let hi = tails.length;
|
||||
while (lo < hi) {
|
||||
const mid = (lo + hi) >> 1;
|
||||
if (items[tails[mid]].b < v) lo = mid + 1; else hi = mid;
|
||||
}
|
||||
if (lo > 0) prev[k] = tails[lo - 1];
|
||||
tails[lo] = k;
|
||||
}
|
||||
const out = [];
|
||||
for (let k = tails.length ? tails[tails.length - 1] : -1; k !== -1; k = prev[k]) out.push(k);
|
||||
return out.reverse();
|
||||
}
|
||||
|
||||
/** LCS over a[aLo..aHi) x b[bLo..bHi); pushes matched [i, j] pairs in order. */
|
||||
function lcsGap(a, aLo, aHi, b, bLo, bHi, pairs) {
|
||||
const n = aHi - aLo;
|
||||
const m = bHi - bLo;
|
||||
const w = m + 1;
|
||||
// L[i*w + j] = LCS length of a[aLo+i..aHi) and b[bLo+j..bHi)
|
||||
const L = new Uint32Array((n + 1) * w);
|
||||
for (let i = n - 1; i >= 0; i--) {
|
||||
for (let j = m - 1; j >= 0; j--) {
|
||||
L[i * w + j] = a[aLo + i] === b[bLo + j]
|
||||
? L[(i + 1) * w + j + 1] + 1
|
||||
: Math.max(L[(i + 1) * w + j], L[i * w + j + 1]);
|
||||
}
|
||||
}
|
||||
let i = 0;
|
||||
let j = 0;
|
||||
while (i < n && j < m) {
|
||||
if (a[aLo + i] === b[bLo + j]) {
|
||||
pairs.push([aLo + i, bLo + j]);
|
||||
i += 1;
|
||||
j += 1;
|
||||
} else if (L[i * w + j + 1] >= L[(i + 1) * w + j]) {
|
||||
j += 1; // keep old line i for the earliest match
|
||||
} else {
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Match a[aLo..aHi) against b[bLo..bHi); pushes matched [i, j] pairs (0-based) in order. */
|
||||
function matchRange(a, aLo, aHi, b, bLo, bHi, pairs) {
|
||||
while (aLo < aHi && bLo < bHi && a[aLo] === b[bLo]) {
|
||||
pairs.push([aLo, bLo]);
|
||||
aLo += 1;
|
||||
bLo += 1;
|
||||
}
|
||||
const tail = [];
|
||||
while (aLo < aHi && bLo < bHi && a[aHi - 1] === b[bHi - 1]) {
|
||||
aHi -= 1;
|
||||
bHi -= 1;
|
||||
tail.push([aHi, bHi]);
|
||||
}
|
||||
if (aLo < aHi && bLo < bHi) {
|
||||
const count = new Map(); // line -> {na, nb, a, b}
|
||||
for (let i = aLo; i < aHi; i++) {
|
||||
const c = count.get(a[i]) ?? { na: 0, nb: 0, a: -1, b: -1 };
|
||||
c.na += 1;
|
||||
c.a = i;
|
||||
count.set(a[i], c);
|
||||
}
|
||||
for (let j = bLo; j < bHi; j++) {
|
||||
const c = count.get(b[j]);
|
||||
if (!c) continue;
|
||||
c.nb += 1;
|
||||
c.b = j;
|
||||
}
|
||||
const anchors = [];
|
||||
for (let i = aLo; i < aHi; i++) {
|
||||
const c = count.get(a[i]);
|
||||
if (c.na === 1 && c.nb === 1) anchors.push({ a: c.a, b: c.b });
|
||||
}
|
||||
if (anchors.length > 0) {
|
||||
let pa = aLo;
|
||||
let pb = bLo;
|
||||
for (const k of lisByB(anchors)) {
|
||||
const { a: ia, b: ib } = anchors[k];
|
||||
matchRange(a, pa, ia, b, pb, ib, pairs);
|
||||
pairs.push([ia, ib]);
|
||||
pa = ia + 1;
|
||||
pb = ib + 1;
|
||||
}
|
||||
matchRange(a, pa, aHi, b, pb, bHi, pairs);
|
||||
} else {
|
||||
lcsGap(a, aLo, aHi, b, bLo, bHi, pairs);
|
||||
}
|
||||
}
|
||||
for (let k = tail.length - 1; k >= 0; k--) pairs.push(tail[k]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Deterministic line diff of two operative texts.
|
||||
* @param {string} oldText
|
||||
* @param {string} newText
|
||||
* @returns {{inside: Set<number>, oldToNew: Array<number|null>, hunks: Array<{oldStart: number, oldEnd: number, newStart: number, newEnd: number}>}}
|
||||
*/
|
||||
export function lineDiff(oldText, newText) {
|
||||
const a = toLines(oldText);
|
||||
const b = toLines(newText);
|
||||
const pairs = [];
|
||||
matchRange(a, 0, a.length, b, 0, b.length, pairs);
|
||||
const oldToNew = new Array(a.length + 1).fill(null);
|
||||
for (const [i, j] of pairs) oldToNew[i + 1] = j + 1;
|
||||
const hunks = [];
|
||||
const inside = new Set();
|
||||
let pi = -1;
|
||||
let pj = -1;
|
||||
for (const [qi, qj] of [...pairs, [a.length, b.length]]) {
|
||||
if (qi > pi + 1 || qj > pj + 1) {
|
||||
const h = { oldStart: pi + 2, oldEnd: qi, newStart: pj + 2, newEnd: qj };
|
||||
hunks.push(h);
|
||||
if (h.newEnd >= h.newStart) {
|
||||
for (let n = h.newStart; n <= h.newEnd; n++) inside.add(n);
|
||||
} else {
|
||||
if (h.newStart - 1 >= 1) inside.add(h.newStart - 1);
|
||||
if (h.newStart <= b.length) inside.add(h.newStart);
|
||||
}
|
||||
}
|
||||
pi = qi;
|
||||
pj = qj;
|
||||
}
|
||||
return { inside, oldToNew, hunks };
|
||||
}
|
||||
|
||||
// ---- counting (rule A, rule B) ----------------------------------------------------
|
||||
|
||||
// Lower index = more severe; an unrated severity counts as blocker.
|
||||
const rankOf = s => SEVERITIES.indexOf(s);
|
||||
const countedOf = s => normalizeSeverity(s) ?? 'blocker';
|
||||
const higher = (a, b) => (rankOf(a) <= rankOf(b) ? a : b);
|
||||
const hasOwn = (o, k) => Object.prototype.hasOwnProperty.call(o, k);
|
||||
|
||||
function union(...lists) {
|
||||
const out = [];
|
||||
for (const list of lists) {
|
||||
for (const x of Array.isArray(list) ? list : []) if (!out.includes(x)) out.push(x);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** A current finding in today's verdict shape, plus the pass-through fields. */
|
||||
function project(f) {
|
||||
const rated = normalizeSeverity(f.severity);
|
||||
const out = {
|
||||
id: f.id ?? null,
|
||||
severity: rated ?? 'blocker',
|
||||
rule_key: f.rule_key ?? null,
|
||||
file: f.file ?? null,
|
||||
line: f.line ?? null,
|
||||
text: typeof f.text === 'string' ? f.text : '',
|
||||
raised_by: Array.isArray(f.raised_by) ? [...f.raised_by] : [],
|
||||
};
|
||||
if (rated === null) out.severity_defaulted = true;
|
||||
if (Number.isInteger(f.line_end)) out.line_end = f.line_end;
|
||||
if (Array.isArray(f.rule_keys)) out.rule_keys = [...f.rule_keys];
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where a current finding sits in the operative text, or null when it has no
|
||||
* plan line (another file, no line, a line in the review log, past the end,
|
||||
* or a `line_end` that does not map).
|
||||
*/
|
||||
function placeOf(f, fullToOp, planSet) {
|
||||
if (typeof f.file !== 'string' || f.file === '' || !planSet.has(resolve(f.file))) return null;
|
||||
const at = l => (Number.isInteger(l) && l >= 1 && l < fullToOp.length ? fullToOp[l] ?? null : null);
|
||||
const start = at(f.line);
|
||||
if (start === null) return null;
|
||||
const hasEnd = f.line_end !== undefined && f.line_end !== null;
|
||||
let end = start;
|
||||
if (hasEnd) {
|
||||
const e = Number.isInteger(f.line_end) && f.line_end >= f.line ? at(f.line_end) : null;
|
||||
if (e === null) return null;
|
||||
end = Math.max(start, e);
|
||||
}
|
||||
return { start, end, hasEnd };
|
||||
}
|
||||
|
||||
const contentSize = t => new Set(contentTokens(typeof t === 'string' ? t : '')).size;
|
||||
|
||||
/** Does current finding `f` (placed at `pl`) sit at previous finding `p`'s mapped line? */
|
||||
function atSpot(pl, p, d) {
|
||||
if (pl === null) return false;
|
||||
if (!hasOwn(p, 'op_line') || !Number.isInteger(p.op_line)) return false;
|
||||
const n = d.oldToNew[p.op_line] ?? null;
|
||||
return n !== null && n >= pl.start && n <= pl.end;
|
||||
}
|
||||
|
||||
/** Does current finding `f` (placed at `pl`, outside the diff) re-report previous finding `p`? */
|
||||
function sameFinding(f, pl, p, d) {
|
||||
if (contentOverlap(f.text, p.text) < DEFAULT_THRESHOLD) return false;
|
||||
if (atSpot(pl, p, d)) return true;
|
||||
const pCites = hasOwn(p, 'op_line');
|
||||
// The id is compared only for a previous finding with no plan line.
|
||||
if (!pCites && typeof f.id === 'string' && f.id === p.id) return true;
|
||||
const sameClass = (pl !== null && pCites) ||
|
||||
(typeof f.file === 'string' && f.file !== '' && f.file === p.file);
|
||||
if (!sameClass) return false;
|
||||
return contentSize(f.text) >= ANCHOR_MIN_CONTENT_WORDS && contentSize(p.text) >= ANCHOR_MIN_CONTENT_WORDS;
|
||||
}
|
||||
|
||||
/** A previous finding leaves the count only on its raisers' true, nobody's false. */
|
||||
function isResolved(p, marks) {
|
||||
const ms = marks.filter(m => m && typeof m.id === 'string' && m.id === p.id);
|
||||
if (!ms.some(m => m.resolved === true)) return false;
|
||||
if (ms.some(m => m.resolved === false)) return false;
|
||||
const raisers = Array.isArray(p.raised_by) ? p.raised_by : [];
|
||||
return raisers.every(r => ms.some(m => m.agent === r && m.resolved === true));
|
||||
}
|
||||
|
||||
/** A previous finding carried into this round, with its matches' severities and keys. */
|
||||
function carry(p, matched, d, current, prevRound) {
|
||||
const pSev = countedOf(p.severity);
|
||||
const orig = normalizeSeverity(p.original_severity);
|
||||
const wasLowered = orig !== null && rankOf(orig) < rankOf(pSev);
|
||||
const keys = union(keysOf(p), ...matched.map(m => keysOf(m.f)));
|
||||
let sev;
|
||||
if (wasLowered) {
|
||||
if (keys.some(isSecurityKey)) sev = matched.reduce((s, m) => higher(s, m.sev), orig);
|
||||
else if (matched.some(m => m.sev === 'blocker')) sev = 'blocker';
|
||||
else sev = pSev;
|
||||
} else {
|
||||
sev = matched.reduce((s, m) => higher(s, m.sev), pSev);
|
||||
}
|
||||
const out = {
|
||||
id: p.id ?? null,
|
||||
severity: sev,
|
||||
rule_key: p.rule_key ?? null,
|
||||
file: p.file ?? null,
|
||||
line: p.line ?? null,
|
||||
text: typeof p.text === 'string' ? p.text : '',
|
||||
raised_by: union(p.raised_by, ...matched.map(m => m.f.raised_by)),
|
||||
};
|
||||
if (p.severity_defaulted === true) out.severity_defaulted = true;
|
||||
if (Number.isInteger(p.line_end)) out.line_end = p.line_end;
|
||||
if (keys.length > 0) out.rule_keys = keys;
|
||||
if (hasOwn(p, 'op_line')) {
|
||||
// A plan line: follow the diff. Its own line edited (or no op_line already):
|
||||
// keep the old line, op_line null, never re-mapped from a stale position.
|
||||
const n = Number.isInteger(p.op_line) ? (d.oldToNew[p.op_line] ?? null) : null;
|
||||
out.op_line = n;
|
||||
if (n !== null) {
|
||||
out.line = current.opToFull[n] ?? out.line;
|
||||
const e = Number.isInteger(p.op_line_end) ? (d.oldToNew[p.op_line_end] ?? null) : null;
|
||||
const eFull = e !== null ? (current.opToFull[e] ?? null) : null;
|
||||
if (eFull !== null) {
|
||||
out.line_end = eFull;
|
||||
out.op_line_end = e;
|
||||
} else {
|
||||
delete out.line_end;
|
||||
}
|
||||
} else if (hasOwn(p, 'op_line_end')) {
|
||||
out.op_line_end = null;
|
||||
}
|
||||
}
|
||||
out.carried_from = Number.isInteger(p.carried_from) ? p.carried_from : prevRound;
|
||||
if (wasLowered) {
|
||||
if (rankOf(sev) > rankOf(orig)) {
|
||||
out.original_severity = orig;
|
||||
if (typeof p.lowered_by === 'string') out.lowered_by = p.lowered_by;
|
||||
if (typeof p.caught_by === 'string') out.caught_by = p.caught_by;
|
||||
}
|
||||
} else {
|
||||
if (orig !== null && orig !== sev) out.original_severity = orig;
|
||||
if (typeof p.lowering_refused === 'string') out.lowering_refused = p.lowering_refused;
|
||||
if (typeof p.caught_by === 'string') out.caught_by = p.caught_by;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** One line: the reason a round is counted fully. */
|
||||
export const oneLine = s => String(s).replace(/\s*[\r\n]+\s*/g, ' ').trim();
|
||||
|
||||
/**
|
||||
* Count one critic round by rule A (from round 2, against the diff since the
|
||||
* text the previous round reviewed) and record rule B (every round).
|
||||
* Pure and deterministic.
|
||||
* @param {{round: number, previous: object|null, base: {sha256: string, text: string}|null,
|
||||
* baseNote?: string|null, current: {text: string, sha256: string, fullToOp: Array<number|null>, opToFull: Array<number|null>},
|
||||
* planPaths: string[], findings: object[], marks: Array<{agent: string, id: string, resolved: boolean}>}} input
|
||||
* @returns {{findings: object[], resolved: string[]|null, diffBase: string|null, diffNote: string|null, lowered: number}}
|
||||
*/
|
||||
export function countRound({ round, previous, base, baseNote, current, planPaths, findings, marks }) {
|
||||
const prevRound = previous && Number.isInteger(previous.round) ? previous.round : null;
|
||||
let diffNote = null;
|
||||
let d = null;
|
||||
if (Number.isInteger(round) && round >= 2 && prevRound !== null) {
|
||||
if (!base || typeof base !== 'object') {
|
||||
diffNote = typeof baseNote === 'string' && baseNote.trim() !== ''
|
||||
? oneLine(baseNote)
|
||||
: `no base snapshot of round ${prevRound}'s reviewed text; full counting`;
|
||||
} else if (typeof base.sha256 !== 'string' || base.sha256 !== previous.plan_sha256) {
|
||||
diffNote = `the base snapshot is not the text round ${prevRound} reviewed; full counting`;
|
||||
} else if (base.sha256 === current.sha256) {
|
||||
diffNote = `the plan has not changed since round ${prevRound}; full counting`;
|
||||
} else if (typeof base.text !== 'string') {
|
||||
diffNote = `the base snapshot of round ${prevRound} has no text; full counting`;
|
||||
} else {
|
||||
d = lineDiff(base.text, current.text);
|
||||
}
|
||||
}
|
||||
const ruleA = d !== null;
|
||||
const planSet = new Set(Array.isArray(planPaths) ? planPaths : []);
|
||||
const fullToOp = Array.isArray(current?.fullToOp) ? current.fullToOp : [];
|
||||
const prevFindings = ruleA && Array.isArray(previous.findings)
|
||||
? previous.findings.filter(p => p && typeof p === 'object')
|
||||
: [];
|
||||
const markList = Array.isArray(marks) ? marks : [];
|
||||
|
||||
const matchedTo = new Map(prevFindings.map(p => [p, []]));
|
||||
// A new finding never carries a previous finding's id (dedup's `<rule_key>#n` scheme).
|
||||
const takenIds = new Set(prevFindings.map(p => p.id).filter(id => typeof id === 'string'));
|
||||
const fresh = [];
|
||||
for (const f of Array.isArray(findings) ? findings : []) {
|
||||
const o = project(f);
|
||||
const b = ruleB(f, o.severity);
|
||||
o.severity = b.severity;
|
||||
Object.assign(o, b.record);
|
||||
const pl = placeOf(f, fullToOp, planSet);
|
||||
let inside = false;
|
||||
if (ruleA && pl) {
|
||||
for (let n = pl.start; n <= pl.end && !inside; n++) inside = d.inside.has(n);
|
||||
}
|
||||
if (ruleA && !inside) {
|
||||
const p = prevFindings.find(q => sameFinding(f, pl, q, d));
|
||||
if (p) {
|
||||
matchedTo.get(p).push({ f, sev: o.severity });
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (pl) {
|
||||
o.op_line = pl.start;
|
||||
if (pl.hasEnd) o.op_line_end = pl.end;
|
||||
}
|
||||
if (ruleA && o.severity === 'major' && pl && !inside && !isSecurity(f)) {
|
||||
o.severity = 'minor';
|
||||
o.original_severity = 'major';
|
||||
o.lowered_by = `rule A: outside the diff since round ${prevRound}`;
|
||||
}
|
||||
if (typeof o.id === 'string' && takenIds.has(o.id)) {
|
||||
let n = 2;
|
||||
let id;
|
||||
while (takenIds.has(id = computeFindingId(String(o.file || 'unknown'), Number.isInteger(o.line) ? o.line : 0, `${o.rule_key ?? 'unknown'}#${n}`))) n += 1;
|
||||
o.id = id;
|
||||
}
|
||||
if (typeof o.id === 'string') takenIds.add(o.id);
|
||||
fresh.push(o);
|
||||
}
|
||||
|
||||
const carried = [];
|
||||
const resolved = [];
|
||||
for (const p of prevFindings) {
|
||||
const matched = matchedTo.get(p);
|
||||
if (matched.length === 0 && isResolved(p, markList)) {
|
||||
resolved.push(p.id);
|
||||
continue;
|
||||
}
|
||||
carried.push(carry(p, matched, d, current, prevRound));
|
||||
}
|
||||
|
||||
const out = [...carried, ...fresh];
|
||||
const lowered = out.filter((x) => {
|
||||
const o = normalizeSeverity(x.original_severity);
|
||||
return o !== null && rankOf(o) < rankOf(x.severity);
|
||||
}).length;
|
||||
return {
|
||||
findings: out,
|
||||
resolved: ruleA ? resolved : null,
|
||||
diffBase: ruleA ? base.sha256 : null,
|
||||
diffNote,
|
||||
lowered,
|
||||
};
|
||||
}
|
||||
|
|
@ -15,11 +15,29 @@
|
|||
// holding ≥ 6 content words, is measured this way; the threshold for
|
||||
// every other pair is unchanged.
|
||||
//
|
||||
// Pass-through for the converging critic loop (2026-10-15). A reviewer's
|
||||
// top-level `previous` marks ({id: 40-hex, resolved: boolean}) come out as
|
||||
// top-level `marks: [{agent, id, resolved}]` in input order; a malformed mark
|
||||
// is dropped, never thrown on, and `sources` keeps exactly {agent, present}.
|
||||
// A finding keeps an integer `line_end` ≥ `line`, rule B's `lowered_from`
|
||||
// (normalized when it is a severity, absent when blank, and kept as given
|
||||
// otherwise, so rule B refuses it and counts it as blocker) and a non-empty
|
||||
// `caught_by`, and carries `rule_keys`: the union
|
||||
// of the keys merged into it, so a security key survives a merge into a
|
||||
// differently-keyed survivor. Two defects are fixed. A `line` that is not a
|
||||
// finite integer becomes null and an empty `file` or `rule_key` becomes null,
|
||||
// with 'unknown' / 0 given to computeFindingId, which threw on them and took
|
||||
// the CLI down. And pass 1 merges equal ids only on a line ≥ 1: file-level
|
||||
// findings (line 0 or none) of one rule_key no longer collapse into one; they
|
||||
// go through the text passes, and a second survivor with a taken id gets
|
||||
// `<rule_key>#n` (n = 2, 3, …) in its id.
|
||||
//
|
||||
// Provenance is preserved on the surviving finding's `raised_by` array.
|
||||
// Severity is kept (normalized to blocker | major | minor, else null) and a
|
||||
// merge keeps the more severe of the two: the plan gate (K3,
|
||||
// Severity is kept (normalized to blocker | major | minor, else null). A merge
|
||||
// keeps the copy rule B counts highest on the merged keys, then the higher
|
||||
// rated severity; on a full tie the survivor keeps its own. The plan gate (K3,
|
||||
// lib/validators/plan-gate.mjs) stops on a BLOCKER, so a merge must never
|
||||
// downgrade one.
|
||||
// downgrade one, in either reviewer order.
|
||||
//
|
||||
// CLI shim — two input modes:
|
||||
// file mode: node lib/review/plan-review-dedup.mjs \
|
||||
|
|
@ -36,8 +54,9 @@
|
|||
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { jaccardSimilarity, meetsThreshold } from '../parsers/jaccard.mjs';
|
||||
import { computeFindingId } from '../parsers/finding-id.mjs';
|
||||
import { computeFindingId, parseFindingId } from '../parsers/finding-id.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
import { getRule } from './rule-catalogue.mjs';
|
||||
|
||||
export const DEFAULT_THRESHOLD = 0.7;
|
||||
export const SEVERITIES = Object.freeze(['blocker', 'major', 'minor']);
|
||||
|
|
@ -51,11 +70,72 @@ export function normalizeSeverity(s) {
|
|||
// Lower rank = more severe; unrated ranks with blocker so a merge never hides it.
|
||||
const rank = s => (s === null ? 0 : SEVERITIES.indexOf(s));
|
||||
|
||||
// ---- rule B (lib/review/critic-diff.mjs imports these back) --------------------
|
||||
|
||||
const higher = (a, b) => (SEVERITIES.indexOf(a) <= SEVERITIES.indexOf(b) ? a : b);
|
||||
|
||||
/** A rule_key whose catalogue entry is in the security category (case and spaces ignored). */
|
||||
export function isSecurityKey(key) {
|
||||
return getRule(String(key).trim().toUpperCase())?.category === 'security';
|
||||
}
|
||||
|
||||
/** The rule keys of a finding: `rule_keys` when it is a list, else its one `rule_key`. */
|
||||
export function keysOf(f) {
|
||||
const keys = Array.isArray(f.rule_keys) ? f.rule_keys : [f.rule_key];
|
||||
return keys.filter(k => typeof k === 'string' && k !== '');
|
||||
}
|
||||
|
||||
export const isSecurity = f => keysOf(f).some(isSecurityKey);
|
||||
|
||||
/**
|
||||
* The rule-B record of one finding at its rated severity.
|
||||
* A null or absent `lowered_from` is no record at all.
|
||||
* @returns {{severity: string, record: object}}
|
||||
*/
|
||||
export function ruleB(f, rated) {
|
||||
if (f.lowered_from === undefined || f.lowered_from === null) return { severity: rated, record: {} };
|
||||
const from = normalizeSeverity(f.lowered_from);
|
||||
const caught = typeof f.caught_by === 'string' && f.caught_by.trim() !== '' ? f.caught_by : null;
|
||||
const security = isSecurity(f);
|
||||
if (from === 'major' && rated === 'minor' && caught && !security) {
|
||||
return { severity: 'minor', record: { original_severity: 'major', lowered_by: `rule B: ${caught}`, caught_by: caught } };
|
||||
}
|
||||
let why;
|
||||
if (from === null) why = 'rule B: lowered_from is not a severity; counted as blocker';
|
||||
else if (from === 'blocker') why = 'rule B never lowers a blocker';
|
||||
else if (from !== 'major') why = `rule B lowers only major to minor, not ${from}`;
|
||||
else if (rated !== 'minor') why = `rule B lowers major to minor only; the finding is rated ${rated}`;
|
||||
else if (security) why = 'rule B never lowers a security finding';
|
||||
else why = 'rule B needs caught_by: the named test, Regression: command or gate that will fail on it';
|
||||
const severity = higher(from ?? 'blocker', rated);
|
||||
const record = { lowering_refused: why };
|
||||
if (caught) record.caught_by = caught;
|
||||
if (severity !== rated) record.original_severity = rated;
|
||||
return { severity, record };
|
||||
}
|
||||
|
||||
// ---- merge ---------------------------------------------------------------------
|
||||
|
||||
function mergeInto(survivor, other) {
|
||||
for (const a of other.raised_by) {
|
||||
if (!survivor.raised_by.includes(a)) survivor.raised_by.push(a);
|
||||
}
|
||||
if (rank(other.severity) < rank(survivor.severity)) survivor.severity = other.severity;
|
||||
const keys = [...(survivor.rule_keys ?? [])];
|
||||
for (const k of other.rule_keys ?? []) if (!keys.includes(k)) keys.push(k);
|
||||
if (keys.length > 0) survivor.rule_keys = keys;
|
||||
// The partner wins when rule B counts it higher on the merged keys, or, on an
|
||||
// equal count, when it is rated higher; the winner brings its severity and
|
||||
// its rule-B record (or none). On a full tie the survivor keeps its own.
|
||||
const counted = x => ruleB({ ...x, rule_keys: keys }, normalizeSeverity(x.severity) ?? 'blocker').severity;
|
||||
const co = rank(counted(other));
|
||||
const cs = rank(counted(survivor));
|
||||
if (co < cs || (co === cs && rank(other.severity) < rank(survivor.severity))) {
|
||||
survivor.severity = other.severity;
|
||||
for (const field of ['lowered_from', 'caught_by']) {
|
||||
if (field in other) survivor[field] = other[field];
|
||||
else delete survivor[field];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -92,6 +172,11 @@ function overlap(a, b) {
|
|||
return n / Math.min(A.size, B.size);
|
||||
}
|
||||
|
||||
/** Content-word overlap of two texts: how much of the shorter one the longer holds (0..1). */
|
||||
export function contentOverlap(a, b) {
|
||||
return overlap(contentTokens(a), contentTokens(b));
|
||||
}
|
||||
|
||||
/** 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 &&
|
||||
|
|
@ -116,33 +201,56 @@ function anchorDuplicate(f, s, threshold) {
|
|||
}
|
||||
|
||||
/**
|
||||
* Normalize a single agent payload into an array of {agent, finding} pairs.
|
||||
* Tolerates missing payload (returns []).
|
||||
* Normalize a single agent payload into its {agent, finding} pairs and its
|
||||
* `previous` array. Tolerates missing payload (no pairs, no previous).
|
||||
*/
|
||||
function normalizeAgentPayload(payload, fallbackAgent) {
|
||||
if (!payload || typeof payload !== 'object') return [];
|
||||
if (!payload || typeof payload !== 'object') return { agent: fallbackAgent, pairs: [], previous: [] };
|
||||
const agent = (typeof payload.agent === 'string' && payload.agent.length > 0)
|
||||
? payload.agent
|
||||
: fallbackAgent;
|
||||
const findings = Array.isArray(payload.findings) ? payload.findings : [];
|
||||
return findings.map(f => ({ agent, finding: f }));
|
||||
const previous = Array.isArray(payload.previous) ? payload.previous : [];
|
||||
return { agent, pairs: findings.map(f => ({ agent, finding: f })), previous };
|
||||
}
|
||||
|
||||
/** A reviewer's mark on a previous finding, or null when it is not {id: 40-hex, resolved: boolean}. */
|
||||
function readMark(m, agent) {
|
||||
if (!m || typeof m !== 'object') return null;
|
||||
if (!parseFindingId(m.id).valid) return null;
|
||||
if (typeof m.resolved !== 'boolean') return null;
|
||||
return { agent, id: m.id, resolved: m.resolved };
|
||||
}
|
||||
|
||||
// The substitutes computeFindingId gets for a null file, line or rule_key.
|
||||
const idOf = (file, line, ruleKey) =>
|
||||
computeFindingId(String(file ?? 'unknown'), line ?? 0, String(ruleKey ?? 'unknown'));
|
||||
|
||||
function annotate(finding, agent) {
|
||||
const id = computeFindingId(
|
||||
String(finding.file ?? 'unknown'),
|
||||
finding.line ?? 0,
|
||||
String(finding.rule_key ?? 'unknown'),
|
||||
);
|
||||
return {
|
||||
id,
|
||||
file: finding.file ?? null,
|
||||
line: finding.line ?? null,
|
||||
rule_key: finding.rule_key ?? null,
|
||||
const file = finding.file === '' ? null : (finding.file ?? null);
|
||||
const line = Number.isInteger(finding.line) ? finding.line : null;
|
||||
const ruleKey = finding.rule_key === '' ? null : (finding.rule_key ?? null);
|
||||
const out = {
|
||||
id: idOf(file, line, ruleKey),
|
||||
file,
|
||||
line,
|
||||
rule_key: ruleKey,
|
||||
severity: normalizeSeverity(finding.severity),
|
||||
text: typeof finding.text === 'string' ? finding.text : '',
|
||||
raised_by: [agent],
|
||||
};
|
||||
if (line !== null && Number.isInteger(finding.line_end) && finding.line_end >= line) {
|
||||
out.line_end = finding.line_end;
|
||||
}
|
||||
const lf = finding.lowered_from;
|
||||
if (lf !== undefined && lf !== null && !(typeof lf === 'string' && lf.trim() === '')) {
|
||||
out.lowered_from = normalizeSeverity(finding.lowered_from) ?? finding.lowered_from;
|
||||
}
|
||||
if (typeof finding.caught_by === 'string' && finding.caught_by.trim() !== '') {
|
||||
out.caught_by = finding.caught_by;
|
||||
}
|
||||
if (typeof ruleKey === 'string') out.rule_keys = [ruleKey];
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -152,6 +260,7 @@ function annotate(finding, agent) {
|
|||
* @param {{ threshold?: number }} [opts]
|
||||
* @returns {{
|
||||
* findings: Array<object>,
|
||||
* marks: Array<{agent: string, id: string, resolved: boolean}>,
|
||||
* sources: Array<{agent: string, present: boolean}>,
|
||||
* dedup_stats: { total_in: number, total_out: number,
|
||||
* exact_id_dups: number, jaccard_dups: number, anchor_dups: number }
|
||||
|
|
@ -161,24 +270,33 @@ export function dedupFindings(sources, opts = {}) {
|
|||
const threshold = typeof opts.threshold === 'number' ? opts.threshold : DEFAULT_THRESHOLD;
|
||||
|
||||
const incoming = [];
|
||||
const marks = [];
|
||||
for (const s of sources) {
|
||||
for (const pair of normalizeAgentPayload(s.payload, s.agent)) {
|
||||
incoming.push(annotate(pair.finding, pair.agent));
|
||||
const { agent, pairs, previous } = normalizeAgentPayload(s.payload, s.agent);
|
||||
for (const pair of pairs) incoming.push(annotate(pair.finding, pair.agent));
|
||||
for (const m of previous) {
|
||||
const mark = readMark(m, agent);
|
||||
if (mark) marks.push(mark);
|
||||
}
|
||||
}
|
||||
|
||||
const total_in = incoming.length;
|
||||
|
||||
// Pass 1 — exact id dedup
|
||||
// Pass 1 — exact id dedup, only on a line ≥ 1. A file-level finding (line 0
|
||||
// or none) shares its id with every other one of its rule_key, so equal ids
|
||||
// there say nothing: those go on to the text passes.
|
||||
const byId = new Map();
|
||||
const pass2 = [];
|
||||
let exact_id_dups = 0;
|
||||
for (const f of incoming) {
|
||||
const existing = byId.get(f.id);
|
||||
const anchored = Number.isInteger(f.line) && f.line > 0;
|
||||
const existing = anchored ? byId.get(f.id) : undefined;
|
||||
if (existing) {
|
||||
mergeInto(existing, f);
|
||||
exact_id_dups += 1;
|
||||
} else {
|
||||
byId.set(f.id, f);
|
||||
if (anchored) byId.set(f.id, f);
|
||||
pass2.push(f);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -187,7 +305,7 @@ export function dedupFindings(sources, opts = {}) {
|
|||
const survivors = [];
|
||||
let jaccard_dups = 0;
|
||||
let anchor_dups = 0;
|
||||
for (const f of byId.values()) {
|
||||
for (const f of pass2) {
|
||||
const tokens = tokenize(f.text);
|
||||
let merged = false;
|
||||
for (const s of survivors) {
|
||||
|
|
@ -208,8 +326,21 @@ export function dedupFindings(sources, opts = {}) {
|
|||
if (!merged) survivors.push(f);
|
||||
}
|
||||
|
||||
// A second survivor with an id already taken gets `<rule_key>#n` in its id.
|
||||
const taken = new Set();
|
||||
for (const f of survivors) {
|
||||
if (taken.has(f.id)) {
|
||||
let n = 2;
|
||||
let id;
|
||||
while (taken.has(id = idOf(f.file, f.line, `${f.rule_key ?? 'unknown'}#${n}`))) n += 1;
|
||||
f.id = id;
|
||||
}
|
||||
taken.add(f.id);
|
||||
}
|
||||
|
||||
return {
|
||||
findings: survivors,
|
||||
marks,
|
||||
// Which reviewers actually delivered a findings block. `findings: []` from
|
||||
// a reviewer that was absent (null payload, misspelled key, no `findings`)
|
||||
// is not a clean review; plan-gate --record refuses it.
|
||||
|
|
|
|||
|
|
@ -20,6 +20,25 @@
|
|||
// 2 = could not run: bad stdin, a reviewer missing from the dedup input,
|
||||
// the plan changed since --hash (the verdict would bless unreviewed text),
|
||||
// an unreadable plan or a failed write. Nothing is written on 1 or 2.
|
||||
// From round 2 on it counts against the snapshot named by the previous
|
||||
// verdict's plan_sha256 (rule A, below); a missing, oversized, misnamed or
|
||||
// tampered snapshot means full counting, said in one `diff_note` line. On
|
||||
// 0 it keeps this round's operative text as a snapshot (tmp + rename)
|
||||
// before the plan is written. Output: one JSON line, which adds
|
||||
// `diff_base`, `diff_note`, `lowered` (a number) and `snapshot` ("kept: ..."
|
||||
// or "not kept: <reason>"; a failed snapshot write does not fail the round).
|
||||
//
|
||||
// node plan-gate.mjs --diff <plan.md>
|
||||
// Prints what the next critic round reviews: `diff-base: <sha256> (round N)`
|
||||
// and each hunk as `@@ lines A-B @@` in the plan's full-file line numbers,
|
||||
// `-` old lines and `+NNN ` new lines; or `diff-base: none — <reason>;
|
||||
// review the whole plan`. 0 = printed; 2 = unreadable plan, bad arguments.
|
||||
//
|
||||
// Snapshots: `<git dir>/voyage/plan-snapshots/<sha256>.md`, the operative text
|
||||
// the round reviewed, named by its own sha256. The git dir is found from the
|
||||
// plan file's own directory only (no cwd fallback); a plan in no repository
|
||||
// keeps none. git never tracks its own directory, so a snapshot is never
|
||||
// committed, whatever the repo's .gitignore or artifact_store says.
|
||||
//
|
||||
// node plan-gate.mjs --accept-open <plan.md>
|
||||
// After the operator's explicit yes to "build as is": records the open
|
||||
|
|
@ -53,12 +72,34 @@
|
|||
// then stop and ask; never silently on. Unrated findings count as blockers.
|
||||
// The verdict is a trace of a process, not a signature: anything that can
|
||||
// write the plan can write the block.
|
||||
//
|
||||
// Rule A (decision 43): from critic round 2 on, `recordVerdict` counts a round
|
||||
// against the diff between the operative text the previous round reviewed (the
|
||||
// base, `opts.base`) and the plan now, through `countRound` in
|
||||
// lib/review/critic-diff.mjs. A new MAJOR that cites a plan line outside that
|
||||
// diff counts as minor; a blocker, a security finding and a finding with no
|
||||
// plan line count at full severity; a previous finding stays counted (carried,
|
||||
// with its id) until every reviewer that raised it marks it resolved and nobody
|
||||
// re-reports it. Rule B (a major a named test, `Regression:` command or gate
|
||||
// will fail on is minor) is recorded from round 1 and never lowers a blocker or
|
||||
// a security finding. Without a usable base (none, not the previous round's
|
||||
// text, or the plan unchanged since it) the round is counted as before, and
|
||||
// `diff_note` says why in one line. Every new verdict field is optional
|
||||
// (DIFF_FIELDS); its absence means round-1 behaviour, so an older block gives
|
||||
// the same exits. Trust limit: anything that can write the plan can also
|
||||
// rewrite the previous verdict's plan_sha256 and plant a matching base; rule A
|
||||
// raises what such a forgery buys, and an unchanged base lowers nothing.
|
||||
|
||||
import { readFileSync, existsSync, writeFileSync, renameSync, rmSync } from 'node:fs';
|
||||
import {
|
||||
readFileSync, existsSync, writeFileSync, renameSync, rmSync, statSync, mkdirSync, realpathSync,
|
||||
} from 'node:fs';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { dirname, join, resolve } from 'node:path';
|
||||
import { validatePlanContent } from './plan-validator.mjs';
|
||||
import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs';
|
||||
import { normalizeSeverity } from '../review/plan-review-dedup.mjs';
|
||||
import { countRound, lineDiff, oneLine } from '../review/critic-diff.mjs';
|
||||
import { resolveRepo } from '../verification/repo-tools.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
export const MAX_ROUNDS = 3;
|
||||
|
|
@ -100,10 +141,53 @@ export function operativeText(text) {
|
|||
.trim() + '\n';
|
||||
}
|
||||
|
||||
export function planHash(text) {
|
||||
return createHash('sha256').update(operativeText(text)).digest('hex');
|
||||
/**
|
||||
* The operative text with its line map, built from the same transforms as
|
||||
* `operativeText` (whose output it returns unchanged).
|
||||
* @returns {{text: string, fullToOp: Array<number|null>, opToFull: Array<number|null>}}
|
||||
* 1-based both ways (index 0 is null). fullToOp[i] is null for a review-log
|
||||
* line, the quality line or a collapsed blank; opToFull[j] is null for an
|
||||
* operative line no file line carries (a blank left where a log section stood).
|
||||
*/
|
||||
export function operativeLines(text) {
|
||||
const src = norm(text);
|
||||
const full = src.split('\n');
|
||||
const excluded = new Set();
|
||||
for (const s of splitSections(src, 2)) {
|
||||
if (!isLogTitle(s.title)) continue;
|
||||
const n = logPart(s.text).log.split('\n').length;
|
||||
for (let i = s.start; i < s.start + n; i++) excluded.add(i);
|
||||
}
|
||||
full.forEach((l, i) => { if (QUALITY_LINE.test(l)) excluded.add(i); });
|
||||
const op = operativeText(text);
|
||||
const opL = op.split('\n');
|
||||
const opCount = opL.length - 1; // the text ends with '\n'; the last split element is not a line
|
||||
const fullToOp = new Array(full.length + 1).fill(null);
|
||||
const opToFull = new Array(opCount + 1).fill(null);
|
||||
let j = 0;
|
||||
for (let i = 0; i < full.length && j < opCount; i++) {
|
||||
if (excluded.has(i)) continue;
|
||||
let l = full[i].replace(/[ \t]+$/, '');
|
||||
if (j === 0) l = l.replace(/^\s+/, '');
|
||||
// Only blanks lose their partner, so a non-blank kept line facing an operative
|
||||
// blank means that blank has no file line: step past it.
|
||||
while (l !== '' && opL[j] === '' && j < opCount - 1) j += 1;
|
||||
const want = opL[j];
|
||||
if (l === want || (j === opCount - 1 && l.trimEnd() === want)) {
|
||||
fullToOp[i + 1] = j + 1;
|
||||
opToFull[j + 1] = i + 1;
|
||||
j += 1;
|
||||
}
|
||||
}
|
||||
return { text: op, fullToOp, opToFull };
|
||||
}
|
||||
|
||||
export function planHash(text) {
|
||||
return sha256(operativeText(text));
|
||||
}
|
||||
|
||||
const sha256 = s => createHash('sha256').update(s).digest('hex');
|
||||
|
||||
/**
|
||||
* @returns {{found: boolean, placeholder: boolean, verdict: object|null, error: string|null}}
|
||||
* placeholder = a `## Critic Verdict` heading with no fenced block at all
|
||||
|
|
@ -177,9 +261,15 @@ function missingReviewers(dedup) {
|
|||
* @param {string} text - the plan
|
||||
* @param {{findings: Array<object>, sources: Array<{agent: string, present: boolean}>}} dedup
|
||||
* plan-review-dedup.mjs output
|
||||
* @param {{reviewed?: string, extraRound?: boolean, maxRounds?: number, now?: string}} [opts]
|
||||
* reviewed = planHash of the text the reviewers read (from --hash)
|
||||
* @returns {{code: 0|1|2, text?: string, verdict?: object, message: string}}
|
||||
* @param {{reviewed?: string, extraRound?: boolean, maxRounds?: number, now?: string,
|
||||
* base?: {sha256: string, text: string}|null, baseNote?: string, planPaths?: string[]}} [opts]
|
||||
* reviewed = planHash of the text the reviewers read (from --hash);
|
||||
* base = the operative text the previous round reviewed, with its sha256;
|
||||
* baseNote = why there is no base (one line, written as diff_note);
|
||||
* planPaths = the absolute forms of the plan path (a finding whose
|
||||
* resolve(file) is one of them cites the plan). Without base, baseNote and
|
||||
* planPaths the round is recorded as before, plus the rule-B record.
|
||||
* @returns {{code: 0|1|2, text?: string, verdict?: object, lowered?: number, message: string}}
|
||||
*/
|
||||
export function recordVerdict(text, dedup, opts = {}) {
|
||||
if (!dedup || typeof dedup !== 'object' || !Array.isArray(dedup.findings)) {
|
||||
|
|
@ -192,7 +282,8 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
if (missing.length > 0) {
|
||||
return { code: 2, message: `reviewer(s) missing from the dedup input: ${missing.join(', ')} — an absent review is not a clean one; rerun the round` };
|
||||
}
|
||||
const current = planHash(text);
|
||||
const lines = operativeLines(text);
|
||||
const current = sha256(lines.text);
|
||||
if (typeof opts.reviewed !== 'string' || !/^[0-9a-f]{64}$/.test(opts.reviewed)) {
|
||||
return { code: 2, message: '--reviewed <sha256> is required: take it with --hash before the reviewers read the plan' };
|
||||
}
|
||||
|
|
@ -216,20 +307,18 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
};
|
||||
}
|
||||
const extra = prevExtra + (capped ? 1 : 0);
|
||||
const findings = dedup.findings.map((f) => {
|
||||
const sev = normalizeSeverity(f.severity);
|
||||
const out = {
|
||||
id: f.id ?? null,
|
||||
severity: sev ?? 'blocker',
|
||||
rule_key: f.rule_key ?? null,
|
||||
file: f.file ?? null,
|
||||
line: f.line ?? null,
|
||||
text: typeof f.text === 'string' ? f.text : '',
|
||||
raised_by: Array.isArray(f.raised_by) ? f.raised_by : [],
|
||||
};
|
||||
if (sev === null) out.severity_defaulted = true;
|
||||
return out;
|
||||
const diffMode = opts.base !== undefined || opts.baseNote !== undefined || opts.planPaths !== undefined;
|
||||
const c = countRound({
|
||||
round,
|
||||
previous: prev.verdict ?? null,
|
||||
base: opts.base ?? null,
|
||||
baseNote: opts.baseNote ?? null,
|
||||
current: { text: lines.text, sha256: current, fullToOp: lines.fullToOp, opToFull: lines.opToFull },
|
||||
planPaths: Array.isArray(opts.planPaths) ? opts.planPaths : [],
|
||||
findings: dedup.findings,
|
||||
marks: Array.isArray(dedup.marks) ? dedup.marks : [],
|
||||
});
|
||||
const findings = c.findings;
|
||||
const counts = { blocker: 0, major: 0, minor: 0 };
|
||||
for (const f of findings) counts[f.severity] += 1;
|
||||
const verdict = {
|
||||
|
|
@ -239,11 +328,17 @@ export function recordVerdict(text, dedup, opts = {}) {
|
|||
extra_rounds: extra,
|
||||
recorded_at: opts.now ?? new Date().toISOString(),
|
||||
plan_sha256: current,
|
||||
verdict: verdictFor(counts),
|
||||
counts,
|
||||
findings,
|
||||
};
|
||||
return { code: 0, text: placeSection(text, renderSection(verdict)), verdict, message: `round ${round} recorded: ${verdict.verdict}` };
|
||||
if (c.diffBase !== null) verdict.diff_base = c.diffBase;
|
||||
if (diffMode && c.diffNote !== null) verdict.diff_note = c.diffNote;
|
||||
verdict.verdict = verdictFor(counts);
|
||||
verdict.counts = counts;
|
||||
verdict.findings = findings;
|
||||
if (c.resolved !== null) verdict.resolved = c.resolved;
|
||||
return {
|
||||
code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered: c.lowered,
|
||||
message: `round ${round} recorded: ${verdict.verdict}`,
|
||||
};
|
||||
}
|
||||
|
||||
const sevOf = f => normalizeSeverity(f.severity);
|
||||
|
|
@ -347,10 +442,112 @@ function out(obj) {
|
|||
process.stdout.write(JSON.stringify(obj) + '\n');
|
||||
}
|
||||
|
||||
const SNAPSHOT_MAX_BYTES = 4 * 1024 * 1024;
|
||||
|
||||
/** `<git dir>/voyage/plan-snapshots` of the repo holding the plan file, or null (no cwd fallback). */
|
||||
function snapshotDir(planPath) {
|
||||
const repo = resolveRepo(dirname(resolve(planPath)));
|
||||
return repo ? join(repo.gitDir, 'voyage', 'plan-snapshots') : null;
|
||||
}
|
||||
|
||||
/** The absolute forms of the plan path: as given, and its realpath. */
|
||||
function planPathsOf(planPath) {
|
||||
const paths = [resolve(planPath)];
|
||||
try {
|
||||
const real = realpathSync(planPath);
|
||||
if (!paths.includes(real)) paths.push(real);
|
||||
} catch { /* the given form only */ }
|
||||
return paths;
|
||||
}
|
||||
|
||||
/**
|
||||
* The snapshot named `sha` in `dir`, or a one-line reason it cannot be used.
|
||||
* The name is checked before any path is built, so a `../` name is never read.
|
||||
* @returns {{base: {sha256: string, text: string}|null, reason: string|null}}
|
||||
*/
|
||||
function readBase(dir, sha) {
|
||||
if (typeof sha !== 'string' || !/^[0-9a-f]{64}$/.test(sha)) {
|
||||
return { base: null, reason: 'the previous verdict\'s plan_sha256 is not a sha256, so no snapshot is read' };
|
||||
}
|
||||
const short = sha.slice(0, 12);
|
||||
const file = join(dir, `${sha}.md`);
|
||||
let st;
|
||||
try { st = statSync(file); } catch (e) {
|
||||
return { base: null, reason: e.code === 'ENOENT' ? `no snapshot of ${short} in the git directory` : `the snapshot of ${short} is unreadable: ${oneLine(e.message)}` };
|
||||
}
|
||||
if (!st.isFile()) return { base: null, reason: `the snapshot of ${short} is not a file` };
|
||||
if (st.size > SNAPSHOT_MAX_BYTES) return { base: null, reason: `the snapshot of ${short} is over 4 MB` };
|
||||
let buf;
|
||||
try { buf = readFileSync(file); } catch (e) {
|
||||
return { base: null, reason: `the snapshot of ${short} is unreadable: ${oneLine(e.message)}` };
|
||||
}
|
||||
if (buf.length > SNAPSHOT_MAX_BYTES) return { base: null, reason: `the snapshot of ${short} is over 4 MB` };
|
||||
if (createHash('sha256').update(buf).digest('hex') !== sha) {
|
||||
return { base: null, reason: `the snapshot of ${short} does not hash to its name (tampered or damaged)` };
|
||||
}
|
||||
return { base: { sha256: sha, text: buf.toString('utf-8') }, reason: null };
|
||||
}
|
||||
|
||||
/**
|
||||
* What the next round counts against: the snapshot of the text the current
|
||||
* verdict's round reviewed. verdict null = no round recorded yet.
|
||||
* @returns {{verdict: object|null, base: {sha256: string, text: string}|null, reason: string|null}}
|
||||
*/
|
||||
function loadBase(text, dir) {
|
||||
const ex = extractVerdict(text);
|
||||
if (!ex.verdict) {
|
||||
return { verdict: null, base: null, reason: ex.error && !ex.placeholder ? `the current verdict is unreadable: ${oneLine(ex.error)}` : 'no critic round has been recorded yet' };
|
||||
}
|
||||
if (dir === null) return { verdict: ex.verdict, base: null, reason: 'the plan is in no git repository, so no snapshot was kept' };
|
||||
const r = readBase(dir, ex.verdict.plan_sha256);
|
||||
return { verdict: ex.verdict, base: r.base, reason: r.reason };
|
||||
}
|
||||
|
||||
/** Keep this round's operative text as `<dir>/<sha>.md` (tmp + rename); returns the `snapshot` field. */
|
||||
function keepSnapshot(dir, text, sha) {
|
||||
if (dir === null) return 'not kept: no git repository';
|
||||
const file = join(dir, `${sha}.md`);
|
||||
const tmp = `${file}.${process.pid}.tmp`;
|
||||
try {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
writeFileSync(tmp, operativeText(text));
|
||||
renameSync(tmp, file);
|
||||
return `kept: ${file}`;
|
||||
} catch (e) {
|
||||
try { rmSync(tmp, { force: true }); } catch { /* nothing to clean */ }
|
||||
return `not kept: ${oneLine(e.message)}`;
|
||||
}
|
||||
}
|
||||
|
||||
/** The `--diff` text: the base round and each hunk in the plan's full-file line numbers. */
|
||||
function diffReport(text, dir) {
|
||||
const none = why => `diff-base: none — ${why}; review the whole plan\n`;
|
||||
const lb = loadBase(text, dir);
|
||||
if (!lb.base) return none(lb.reason);
|
||||
const cur = operativeLines(text);
|
||||
if (lb.base.sha256 === sha256(cur.text)) return none(`the plan has not changed since round ${lb.verdict.round}`);
|
||||
const oldL = lb.base.text.split('\n');
|
||||
const newL = cur.text.split('\n');
|
||||
const newCount = newL.length - 1; // the operative text ends with '\n'
|
||||
const d = lineDiff(lb.base.text, cur.text);
|
||||
const lines = [`diff-base: ${lb.base.sha256} (round ${lb.verdict.round})`];
|
||||
for (const h of d.hunks) {
|
||||
const changed = h.newEnd >= h.newStart;
|
||||
const lo = changed ? h.newStart : Math.max(1, h.newStart - 1);
|
||||
const hi = changed ? h.newEnd : Math.min(newCount, h.newStart);
|
||||
const mapped = [];
|
||||
for (let n = lo; n <= hi; n++) if (cur.opToFull[n] != null) mapped.push(cur.opToFull[n]);
|
||||
lines.push(mapped.length ? `@@ lines ${mapped[0]}-${mapped[mapped.length - 1]} @@` : '@@ lines (no file line) @@');
|
||||
for (let o = h.oldStart; o <= h.oldEnd; o++) lines.push(`-${oldL[o - 1]}`);
|
||||
for (let n = h.newStart; n <= h.newEnd; n++) lines.push(`+${cur.opToFull[n] ?? '?'} ${newL[n - 1]}`);
|
||||
}
|
||||
return `${lines.join('\n')}\n`;
|
||||
}
|
||||
|
||||
if (isMain(import.meta.url)) {
|
||||
const args = process.argv.slice(2);
|
||||
const usage = 'Usage: plan-gate.mjs <plan.md> [--json] | --hash <plan.md> | --record <plan.md> --reviewed <sha256> [--extra-round] < dedup.json | --accept-open <plan.md>';
|
||||
const known = new Set(['--json', '--hash', '--record', '--reviewed', '--extra-round', '--accept-open']);
|
||||
const usage = 'Usage: plan-gate.mjs <plan.md> [--json] | --hash <plan.md> | --diff <plan.md> | --record <plan.md> --reviewed <sha256> [--extra-round] < dedup.json | --accept-open <plan.md>';
|
||||
const known = new Set(['--json', '--hash', '--diff', '--record', '--reviewed', '--extra-round', '--accept-open']);
|
||||
const rIdx = args.indexOf('--reviewed');
|
||||
const reviewed = rIdx >= 0 ? args[rIdx + 1] : undefined;
|
||||
const positional = args.filter((a, i) => !a.startsWith('--') && !(rIdx >= 0 && i === rIdx + 1));
|
||||
|
|
@ -373,6 +570,10 @@ if (isMain(import.meta.url)) {
|
|||
process.stdout.write(`${planHash(text)}\n`);
|
||||
process.exit(0);
|
||||
}
|
||||
if (args.includes('--diff')) {
|
||||
process.stdout.write(diffReport(text, snapshotDir(planPath)));
|
||||
process.exit(0);
|
||||
}
|
||||
if (args.includes('--accept-open')) {
|
||||
const r = acceptOpen(text);
|
||||
if (r.code === 0) {
|
||||
|
|
@ -395,8 +596,18 @@ if (isMain(import.meta.url)) {
|
|||
out({ code: 2, recorded: false, message: `stdin is not JSON: ${e.message}` });
|
||||
process.exit(2);
|
||||
}
|
||||
const r = recordVerdict(text, dedup, { extraRound: args.includes('--extra-round'), reviewed });
|
||||
const dir = snapshotDir(planPath);
|
||||
const opts = { extraRound: args.includes('--extra-round'), reviewed, planPaths: planPathsOf(planPath) };
|
||||
const lb = loadBase(text, dir);
|
||||
if (lb.verdict) {
|
||||
opts.base = lb.base;
|
||||
if (lb.reason) opts.baseNote = `${lb.reason}; full counting`;
|
||||
}
|
||||
const r = recordVerdict(text, dedup, opts);
|
||||
let snapshot = null;
|
||||
if (r.code === 0) {
|
||||
// The snapshot first, then the plan: a verdict never names a text that was not kept on purpose.
|
||||
snapshot = keepSnapshot(dir, text, r.verdict.plan_sha256);
|
||||
const tmp = `${planPath}.plan-gate.tmp`;
|
||||
try {
|
||||
writeFileSync(tmp, r.text);
|
||||
|
|
@ -411,6 +622,8 @@ if (isMain(import.meta.url)) {
|
|||
code: r.code, recorded: r.code === 0, message: r.message,
|
||||
round: r.verdict?.round ?? null, max_rounds: r.verdict?.max_rounds ?? null,
|
||||
verdict: r.verdict?.verdict ?? null, counts: r.verdict?.counts ?? null,
|
||||
diff_base: r.verdict?.diff_base ?? null, diff_note: r.verdict?.diff_note ?? null,
|
||||
lowered: r.code === 0 ? r.lowered : null, snapshot,
|
||||
});
|
||||
process.exit(r.code);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -315,5 +315,10 @@ later waves depend on earlier waves completing first.*
|
|||
--record` appends it after the first critic round (Phase 9) and replaces it on
|
||||
each later round: one fenced json block with round, max_rounds, plan_sha256
|
||||
(the plan minus its review log), verdict BLOCK | REVISE | PASS, and the
|
||||
deduplicated findings. `plan-gate.mjs <plan>` exits 0 only when the plan
|
||||
validates, the block matches the current plan, and no BLOCKER is open. -->
|
||||
deduplicated findings. It may also carry the optional rule-A and rule-B
|
||||
fields (DIFF_FIELDS in lib/review/critic-diff.mjs: diff_base, diff_note,
|
||||
resolved, and per finding op_line, op_line_end, line_end, rule_keys,
|
||||
original_severity, lowered_by, caught_by, lowering_refused, carried_from);
|
||||
absent means round-1 behaviour. `plan-gate.mjs <plan>` exits 0 only when the
|
||||
plan validates, the block matches the current plan, no BLOCKER is open, and
|
||||
every open MAJOR was accepted by the operator. -->
|
||||
|
|
|
|||
168
tests/e2e-critic-diff-mode.sh
Normal file
168
tests/e2e-critic-diff-mode.sh
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
#!/usr/bin/env bash
|
||||
# tests/e2e-critic-diff-mode.sh
|
||||
# End to end for the critic loop's diff mode (decision 43): the plan gate CLI
|
||||
# run the way /trekplan Phase 9 runs it, in a temp git repo.
|
||||
# 1. git init a temp repo and build a 1.8 plan with
|
||||
# tests/helpers/plan-v18-builder.mjs.
|
||||
# 2. --hash, a round-1 reviewer payload (one major) through
|
||||
# lib/review/plan-review-dedup.mjs --stdin into a file, then
|
||||
# --record < file (exit 0, REVISE).
|
||||
# 3. Edit one step line with node -e.
|
||||
# 4. --diff names the edited line (its number found with grep -n).
|
||||
# 5. Save a copy of the plan.
|
||||
# 6. A: round 2 marks the round-1 major resolved and adds a new major on a
|
||||
# line outside the edit (exit 0, PASS); the bare gate exits 0.
|
||||
# 7. Restore the copy. B: the same, with the new major on the edited line
|
||||
# (exit 0, REVISE); the bare gate exits 1 with PLAN_GATE_OPEN_MAJOR.
|
||||
# 8. git status --porcelain --untracked-files=all lists no snapshot; the
|
||||
# round-1 snapshot file exists under .git/voyage/plan-snapshots/.
|
||||
# 9. Round 3 (plan unchanged, counted fully): a minor merged with its copy
|
||||
# that claims a lowering from blocker, and a minor with a lowered_from
|
||||
# that is not a severity (exit 0, BLOCK, two blockers); the bare gate
|
||||
# exits 1 with PLAN_GATE_OPEN_BLOCKER.
|
||||
# Prints "e2e: ok" and exits 0, or names the failed check and exits 1.
|
||||
# bash 3.2 compatible, ASCII only.
|
||||
|
||||
set -u
|
||||
|
||||
cd "$(dirname "$0")/.." || { echo "e2e: FAIL - cannot cd to the repo root" >&2; exit 1; }
|
||||
ROOT="$(pwd)"
|
||||
GATE="$ROOT/lib/validators/plan-gate.mjs"
|
||||
DEDUP="$ROOT/lib/review/plan-review-dedup.mjs"
|
||||
|
||||
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE
|
||||
|
||||
TMP="$(mktemp -d "${TMPDIR:-/tmp}/e2e-critic-diff-mode.XXXXXX")" || { echo "e2e: FAIL - mktemp" >&2; exit 1; }
|
||||
trap 'rm -rf "$TMP"' EXIT
|
||||
|
||||
fail() {
|
||||
echo "e2e: FAIL - $1" >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
REPO="$TMP/repo"
|
||||
PLAN="$REPO/plan.md"
|
||||
|
||||
# gate <args...>: the plan gate run from inside the temp repo (findings cite plan.md).
|
||||
gate() {
|
||||
(cd "$REPO" && node "$GATE" "$@")
|
||||
}
|
||||
|
||||
# round <payload-file>: --hash, dedup --stdin, then --record; prints the record line.
|
||||
round() {
|
||||
h="$(gate --hash plan.md)" || fail "--hash exited non-zero"
|
||||
node "$DEDUP" --stdin < "$1" > "$TMP/dedup.json" || fail "plan-review-dedup --stdin on $1"
|
||||
gate --record plan.md --reviewed "$h" < "$TMP/dedup.json"
|
||||
}
|
||||
|
||||
# ---- 1. a temp git repo and a 1.8 plan ----
|
||||
|
||||
mkdir -p "$REPO" || fail "mkdir $REPO"
|
||||
git -C "$REPO" init -q || fail "git init $REPO"
|
||||
node --input-type=module -e '
|
||||
import { writeFileSync } from "node:fs";
|
||||
import { buildPlan } from "./tests/helpers/plan-v18-builder.mjs";
|
||||
writeFileSync(process.argv[1], buildPlan());
|
||||
' "$PLAN" || fail "could not write the plan with tests/helpers/plan-v18-builder.mjs"
|
||||
|
||||
# ---- 2. round 1: one major ----
|
||||
|
||||
L1="$(grep -n 'Verifies: run1()' "$PLAN" | head -n 1 | cut -d: -f1)"
|
||||
[ -n "$L1" ] || fail "no Verifies line for step 1 in the plan"
|
||||
M1='Step 1 verification never states the expected return value'
|
||||
printf '{"plan_critic":{"findings":[{"file":"plan.md","line":%s,"rule_key":"step-quality","severity":"major","text":"%s"}]},"scope_guardian":{"findings":[]}}\n' \
|
||||
"$L1" "$M1" > "$TMP/r1.json"
|
||||
H1="$(gate --hash plan.md)" || fail "--hash before round 1"
|
||||
out="$(round "$TMP/r1.json")"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "round 1 --record: exit $rc, expected 0: $out"
|
||||
echo "$out" | grep -q '"verdict":"REVISE"' || fail "round 1: no REVISE in: $out"
|
||||
ID1="$(node -e '
|
||||
const t = require("node:fs").readFileSync(process.argv[1], "utf8");
|
||||
const v = JSON.parse(t.match(/```json\n([\s\S]*?)\n```/)[1]);
|
||||
process.stdout.write(v.findings[0].id);
|
||||
' "$PLAN")" || fail "could not read the round-1 finding id"
|
||||
echo "$ID1" | grep -Eq '^[0-9a-f]{40}$' || fail "round-1 finding id is not 40 hex: $ID1"
|
||||
|
||||
# ---- 3. edit one step line ----
|
||||
|
||||
node -e '
|
||||
const fs = require("node:fs");
|
||||
const p = process.argv[1];
|
||||
const t = fs.readFileSync(p, "utf8");
|
||||
const from = "Add `run2()` to `src/mod2.mjs`.";
|
||||
if (!t.includes(from)) process.exit(1);
|
||||
fs.writeFileSync(p, t.replace(from, "Add `run2()` and its error path to `src/mod2.mjs`."));
|
||||
' "$PLAN" || fail "could not edit the Step 2 Changes line"
|
||||
LE="$(grep -n 'its error path' "$PLAN" | head -n 1 | cut -d: -f1)"
|
||||
[ -n "$LE" ] || fail "the edited line is not in the plan"
|
||||
|
||||
# ---- 4. --diff names the edited line ----
|
||||
|
||||
out="$(gate --diff plan.md)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "--diff: exit $rc, expected 0: $out"
|
||||
echo "$out" | head -n 1 | grep -q "^diff-base: $H1 (round 1)\$" || fail "--diff: first line is not the round-1 base: $out"
|
||||
echo "$out" | grep -q "^@@ lines $LE-$LE @@\$" || fail "--diff: no hunk header for line $LE in: $out"
|
||||
echo "$out" | grep -q "^+$LE .*its error path" || fail "--diff: line $LE is not named as a new line in: $out"
|
||||
|
||||
# ---- 5. save a copy ----
|
||||
|
||||
cp "$PLAN" "$TMP/plan-after-edit.md" || fail "could not copy the plan"
|
||||
|
||||
# ---- 6. A: the new major is outside the edit ----
|
||||
|
||||
LO="$(grep -n 'checkout -- src/mod1.mjs' "$PLAN" | head -n 1 | cut -d: -f1)"
|
||||
[ -n "$LO" ] || fail "no On failure line for step 1 in the plan"
|
||||
MA='The rollback named for this step gives no check that the revert actually worked'
|
||||
printf '{"plan_critic":{"previous":[{"id":"%s","resolved":true}],"findings":[{"file":"plan.md","line":%s,"rule_key":"step-quality","severity":"major","text":"%s"}]},"scope_guardian":{"previous":[{"id":"%s","resolved":true}],"findings":[]}}\n' \
|
||||
"$ID1" "$LO" "$MA" "$ID1" > "$TMP/r2a.json"
|
||||
out="$(round "$TMP/r2a.json")"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "A: round 2 --record: exit $rc, expected 0: $out"
|
||||
echo "$out" | grep -q '"verdict":"PASS"' || fail "A: round 2 outside the diff is not PASS: $out"
|
||||
echo "$out" | grep -q "\"diff_base\":\"$H1\"" || fail "A: round 2 did not diff against round 1: $out"
|
||||
out="$(gate plan.md)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "A: the bare gate exit $rc, expected 0: $out"
|
||||
|
||||
# ---- 7. B: the new major is on the edited line ----
|
||||
|
||||
cp "$TMP/plan-after-edit.md" "$PLAN" || fail "could not restore the plan"
|
||||
MB='The error path named here gives callers no thrown type to match on'
|
||||
printf '{"plan_critic":{"previous":[{"id":"%s","resolved":true}],"findings":[{"file":"plan.md","line":%s,"rule_key":"step-quality","severity":"major","text":"%s"}]},"scope_guardian":{"previous":[{"id":"%s","resolved":true}],"findings":[]}}\n' \
|
||||
"$ID1" "$LE" "$MB" "$ID1" > "$TMP/r2b.json"
|
||||
out="$(round "$TMP/r2b.json")"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "B: round 2 --record: exit $rc, expected 0: $out"
|
||||
echo "$out" | grep -q '"verdict":"REVISE"' || fail "B: round 2 inside the diff is not REVISE: $out"
|
||||
out="$(gate plan.md)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 1 ] || fail "B: the bare gate exit $rc, expected 1: $out"
|
||||
echo "$out" | grep -q 'PLAN_GATE_OPEN_MAJOR' || fail "B: no PLAN_GATE_OPEN_MAJOR in: $out"
|
||||
|
||||
# ---- 8. snapshots stay in the git directory ----
|
||||
|
||||
st="$(git -C "$REPO" status --porcelain --untracked-files=all)" || fail "git status in the temp repo"
|
||||
echo "$st" | grep -q 'plan-snapshots' && fail "git status lists a snapshot: $st"
|
||||
[ "$st" = '?? plan.md' ] || fail "git status lists more than the plan: $st"
|
||||
[ -f "$REPO/.git/voyage/plan-snapshots/$H1.md" ] || fail "no round-1 snapshot under .git/voyage/plan-snapshots/"
|
||||
|
||||
# ---- 9. round 3: a refused and a malformed lowering reach the gate as blockers ----
|
||||
|
||||
M9='Step 1 never names the exit code its verify command expects'
|
||||
G9='The revert for step one is never run against a dirty tree'
|
||||
printf '{"plan_critic":{"findings":[{"file":"plan.md","line":%s,"rule_key":"step-quality","severity":"minor","text":"%s"}]},"scope_guardian":{"findings":[{"file":"plan.md","line":%s,"rule_key":"step-quality","severity":"minor","lowered_from":"blocker","caught_by":"npm test","text":"%s"},{"file":"plan.md","line":%s,"rule_key":"gap","severity":"minor","lowered_from":"high","caught_by":"npm test","text":"%s"}]}}\n' \
|
||||
"$L1" "$M9" "$L1" "$M9" "$LO" "$G9" > "$TMP/r3.json"
|
||||
out="$(round "$TMP/r3.json")"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "9: round 3 --record: exit $rc, expected 0: $out"
|
||||
echo "$out" | grep -q '"verdict":"BLOCK"' || fail "9: round 3 is not BLOCK: $out"
|
||||
echo "$out" | grep -q '"blocker":2' || fail "9: round 3 does not count two blockers: $out"
|
||||
out="$(gate plan.md)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 1 ] || fail "9: the bare gate exit $rc, expected 1: $out"
|
||||
echo "$out" | grep -q 'PLAN_GATE_OPEN_BLOCKER' || fail "9: no PLAN_GATE_OPEN_BLOCKER in: $out"
|
||||
|
||||
echo "e2e: ok"
|
||||
exit 0
|
||||
2
tests/fixtures/jsonl-schemas.md
vendored
2
tests/fixtures/jsonl-schemas.md
vendored
|
|
@ -21,7 +21,7 @@
|
|||
|-----------|--------|-------------|----------|---------------|-----|
|
||||
| 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:1137-1161 | 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:1234-1262 | profile, phase_models, parallel_agents, 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:1271-1299 | profile, phase_models, parallel_agents, profile_source | none |
|
||||
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2892-2914 | profile, phase_models, profile_source | none |
|
||||
| trekexecute-stats (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) |
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
|
||||
import { execFile } from 'node:child_process';
|
||||
import { childEnv } from './child-env.mjs';
|
||||
import { HOOK_CHILD_TIMEOUT_MS } from './timing.mjs';
|
||||
|
||||
// How long a hook may run before the helper kills it. The tests check a
|
||||
// hook's answer, not its speed (SC #14 times itself, tests/helpers/timing.mjs),
|
||||
|
|
@ -14,7 +15,8 @@ import { childEnv } from './child-env.mjs';
|
|||
// (post-edit-check) at load 95-177 in four wave sessions while it was 8/8
|
||||
// green alone (order 10, 2026-10-09). hooks/hooks.json sets no timeout for
|
||||
// these hooks, so 5 s was stricter than anything the hook meets in use.
|
||||
export const HOOK_TIMEOUT_MS = 30_000;
|
||||
// The number lives in tests/helpers/timing.mjs (decision 44), so there is one.
|
||||
export const HOOK_TIMEOUT_MS = HOOK_CHILD_TIMEOUT_MS;
|
||||
|
||||
/**
|
||||
* Run a hook script by spawning `node <scriptPath>` and piping `input` to stdin.
|
||||
|
|
|
|||
|
|
@ -35,3 +35,13 @@ export async function fastestWithin(limitMs, measure, tries = 3) {
|
|||
// (2000 -> 15000 ms, 6000 -> 45000 ms) that the floor guard may report as
|
||||
// loosened-threshold: plan step 1 of round 3 asks for it.
|
||||
export const CONTROL_RUN = Object.freeze({ timeoutMs: 15_000, hangMs: 45_000, slowStartMs: 2_500 });
|
||||
|
||||
// How long a hook test may let its hook child run before it is killed
|
||||
// (tests/helpers/hook-helper.mjs reads it as HOOK_TIMEOUT_MS, and
|
||||
// tests/hooks/post-compact-flush.test.mjs uses it directly). A fixed 5 s limit
|
||||
// killed hook children under full-suite load (decision 44, the same class as
|
||||
// review 703d87e1). The limit is raised on purpose and is not a loosened
|
||||
// assertion: the tests check the hook's answer, not its speed. This is the
|
||||
// raise (5000 -> 30000 ms) that the floor guard may report as
|
||||
// loosened-threshold: plan step 1 of the converging-review-loops plan asks for it.
|
||||
export const HOOK_CHILD_TIMEOUT_MS = 30_000;
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import { join, dirname } from 'node:path';
|
|||
import { fileURLToPath } from 'node:url';
|
||||
import { execFile } from 'node:child_process';
|
||||
import { childEnv } from '../helpers/child-env.mjs';
|
||||
import { HOOK_CHILD_TIMEOUT_MS } from '../helpers/timing.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
|
|
@ -23,10 +24,10 @@ function runHookIn(cwd, input = {}) {
|
|||
const child = execFile(
|
||||
'node',
|
||||
[HOOK],
|
||||
{ timeout: 5000, cwd, env: childEnv() },
|
||||
{ timeout: HOOK_CHILD_TIMEOUT_MS, cwd, env: childEnv() },
|
||||
(err, stdout, stderr) => {
|
||||
resolve({
|
||||
code: child.exitCode ?? 0,
|
||||
code: child.exitCode ?? 1,
|
||||
stdout: stdout || '',
|
||||
stderr: stderr || '',
|
||||
});
|
||||
|
|
|
|||
296
tests/lib/critic-diff.test.mjs
Normal file
296
tests/lib/critic-diff.test.mjs
Normal file
|
|
@ -0,0 +1,296 @@
|
|||
// tests/lib/critic-diff.test.mjs
|
||||
// Step 2 of the converging-review-loops plan (decision 43): the operative line
|
||||
// map (`operativeLines` in plan-gate.mjs) and the deterministic line diff
|
||||
// (`lineDiff` in critic-diff.mjs) that rule A counts on.
|
||||
//
|
||||
// - The line map must not move the hash: its text is operativeText's output,
|
||||
// byte for byte, and three fixtures keep a fixed sha256 (taken at the start
|
||||
// point of this step, before operativeLines existed).
|
||||
// - lineDiff places hunks; a step inserted with boilerplate lines that duplicate
|
||||
// the steps around it is marked as the NEW copy, never the old one.
|
||||
// - The two architecture rules: critic-diff never imports plan-gate, and does
|
||||
// no I/O (no fs, no child_process). Checked on the real source (pass) and on
|
||||
// the source with a planted violation (fail), in the same run.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { readFileSync, existsSync } from 'node:fs';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath, pathToFileURL } from 'node:url';
|
||||
import { operativeText, planHash, recordVerdict } from '../../lib/validators/plan-gate.mjs';
|
||||
import * as gate from '../../lib/validators/plan-gate.mjs';
|
||||
import { buildPlan, step } from '../helpers/plan-v18-builder.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const CRITIC_DIFF = join(HERE, '..', '..', 'lib', 'review', 'critic-diff.mjs');
|
||||
|
||||
// Loaded so that each new assertion fails on its own while the module is absent.
|
||||
let diffMod = {};
|
||||
try {
|
||||
diffMod = await import(pathToFileURL(CRITIC_DIFF).href);
|
||||
} catch (e) {
|
||||
if (existsSync(CRITIC_DIFF)) throw e;
|
||||
}
|
||||
|
||||
function operativeLines(text) {
|
||||
assert.equal(typeof gate.operativeLines, 'function', 'plan-gate.mjs exports operativeLines');
|
||||
return gate.operativeLines(text);
|
||||
}
|
||||
|
||||
function lineDiff(oldText, newText) {
|
||||
assert.equal(typeof diffMod.lineDiff, 'function', 'critic-diff.mjs exports lineDiff');
|
||||
return diffMod.lineDiff(oldText, newText);
|
||||
}
|
||||
|
||||
const sha = s => createHash('sha256').update(s).digest('hex');
|
||||
const BOTH = [{ agent: 'plan-critic', present: true }, { agent: 'scope-guardian', present: true }];
|
||||
const lines = t => t.split('\n');
|
||||
const lineNo = (t, pred) => lines(t).findIndex(pred) + 1;
|
||||
|
||||
// ---- fixtures ----------------------------------------------------------------
|
||||
|
||||
/** A buildPlan() plan. */
|
||||
const PLAN_A = buildPlan();
|
||||
|
||||
/** The same plan after a recorded verdict, plus a `## Revisions` section with a `### Step` under it. */
|
||||
function planB() {
|
||||
const base = buildPlan();
|
||||
const r = recordVerdict(base, {
|
||||
findings: [{ id: 'a'.repeat(40), file: 'plan.md', line: 70, rule_key: 'step-quality', severity: 'minor', text: 'nit', raised_by: ['plan-critic'] }],
|
||||
sources: BOTH,
|
||||
}, { reviewed: planHash(base), now: '2026-10-15T00:00:00.000Z' });
|
||||
assert.equal(r.code, 0, r.message);
|
||||
return r.text +
|
||||
'\n## Revisions\n\n| # | Finding | Severity | Resolution |\n|---|---|---|---|\n| 1 | nit | minor | left |\n\n' +
|
||||
'### Step 3: A step placed under the log\n\n- **Files:** `src/late.mjs`\n';
|
||||
}
|
||||
const PLAN_B = planB();
|
||||
|
||||
/** A triple blank line inside a fence, plus CRLF and line-end whitespace. */
|
||||
const PLAN_C = '# Fence plan\r\n\r\n## Context\r\n\r\n```text\r\nalpha \r\n\r\n\r\n\r\nbeta\r\n```\r\n\r\nend \r\n';
|
||||
|
||||
// sha256 of operativeText at this step's start point (c9248bb), before operativeLines existed.
|
||||
const FIXED = {
|
||||
A: '458fe6763d59d3674e9bc882e326e06b81522b625e5443799a7f61099d356c55',
|
||||
B: '57f0c8c7e908014975e12b50e6c16aef2acaccf4e765047ffd4ef33017724475',
|
||||
C: 'c12b130fbc418ff5c0fe570c8bd26ecac69f0117d23fe7cfe93ce68af0e11a3f',
|
||||
};
|
||||
|
||||
// ---- operativeLines: the hash does not move -------------------------------------
|
||||
|
||||
for (const [name, text] of [['A', PLAN_A], ['B', PLAN_B], ['C', PLAN_C]]) {
|
||||
test(`operativeLines(${name}).text is operativeText byte for byte, at the fixed sha256`, () => {
|
||||
assert.equal(planHash(text), FIXED[name], 'planHash is unchanged from the start point');
|
||||
const m = operativeLines(text);
|
||||
assert.equal(m.text, operativeText(text));
|
||||
assert.equal(sha(m.text), FIXED[name]);
|
||||
});
|
||||
}
|
||||
|
||||
test('operativeLines: fullToOp and opToFull are inverse where both are set', () => {
|
||||
for (const text of [PLAN_A, PLAN_B, PLAN_C]) {
|
||||
const m = operativeLines(text);
|
||||
const opCount = m.text.split('\n').length - 1;
|
||||
for (let j = 1; j <= opCount; j++) {
|
||||
const i = m.opToFull[j];
|
||||
if (i !== null) assert.equal(m.fullToOp[i], j, `opToFull[${j}] = ${i} maps back`);
|
||||
}
|
||||
for (let i = 1; i < m.fullToOp.length; i++) {
|
||||
const j = m.fullToOp[i];
|
||||
if (j !== null) assert.equal(m.opToFull[j], i, `fullToOp[${i}] = ${j} maps back`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('operativeLines: ### Step 2 at full line 70 maps to operative line 69 in buildPlan()', () => {
|
||||
const full = lineNo(PLAN_A, l => l.startsWith('### Step 2'));
|
||||
const op = lineNo(operativeText(PLAN_A), l => l.startsWith('### Step 2'));
|
||||
assert.equal(full, 70);
|
||||
assert.equal(op, 69);
|
||||
const m = operativeLines(PLAN_A);
|
||||
assert.equal(m.fullToOp[70], 69);
|
||||
assert.equal(m.opToFull[69], 70);
|
||||
// Every non-blank operative line is reached from the full line that carries its text.
|
||||
const opL = lines(m.text);
|
||||
const fullL = lines(PLAN_A);
|
||||
for (let j = 1; j < opL.length; j++) {
|
||||
if (opL[j - 1] === '') continue;
|
||||
const i = m.opToFull[j];
|
||||
assert.notEqual(i, null, `operative line ${j} "${opL[j - 1]}" has a full line`);
|
||||
assert.equal(fullL[i - 1].replace(/[ \t]+$/, ''), opL[j - 1]);
|
||||
}
|
||||
});
|
||||
|
||||
test('operativeLines: the quality line maps to null', () => {
|
||||
const q = lineNo(PLAN_A, l => l.startsWith('> **Plan quality:'));
|
||||
assert.equal(q, 3);
|
||||
const m = operativeLines(PLAN_A);
|
||||
assert.equal(m.fullToOp[q], null);
|
||||
assert.equal(m.fullToOp[1], 1, '# Test plan stays line 1');
|
||||
});
|
||||
|
||||
test('operativeLines: review-log lines map to null; a ### Step under the log stays mapped', () => {
|
||||
const m = operativeLines(PLAN_B);
|
||||
const fullL = lines(PLAN_B);
|
||||
const verdictAt = lineNo(PLAN_B, l => l === '## Critic Verdict');
|
||||
const jsonAt = lineNo(PLAN_B, l => l === '```json');
|
||||
const revAt = lineNo(PLAN_B, l => l === '## Revisions');
|
||||
const tableAt = lineNo(PLAN_B, l => l === '| 1 | nit | minor | left |');
|
||||
const lateAt = lineNo(PLAN_B, l => l.startsWith('### Step 3: A step placed under the log'));
|
||||
for (const i of [verdictAt, jsonAt, jsonAt + 1, revAt, tableAt]) {
|
||||
assert.ok(i > 0, `fixture line found (${i})`);
|
||||
assert.equal(m.fullToOp[i], null, `log line ${i} "${fullL[i - 1]}" maps to null`);
|
||||
}
|
||||
const opLate = lineNo(m.text, l => l.startsWith('### Step 3: A step placed under the log'));
|
||||
assert.ok(opLate > 0);
|
||||
assert.equal(m.fullToOp[lateAt], opLate);
|
||||
assert.equal(m.opToFull[opLate], lateAt);
|
||||
assert.equal(m.fullToOp[lateAt + 2], opLate + 2, 'the step\'s Files line under the log');
|
||||
});
|
||||
|
||||
test('operativeLines: collapsed blanks (inside a fence, CRLF) map to null', () => {
|
||||
const m = operativeLines(PLAN_C);
|
||||
const alpha = lineNo(PLAN_C, l => l.startsWith('alpha'));
|
||||
const beta = lineNo(PLAN_C, l => l.startsWith('beta'));
|
||||
assert.equal(beta - alpha, 4, 'three blank lines between alpha and beta');
|
||||
const opAlpha = lineNo(m.text, l => l === 'alpha');
|
||||
assert.equal(m.fullToOp[alpha], opAlpha);
|
||||
assert.equal(m.fullToOp[beta], opAlpha + 2, 'one blank survives the collapse');
|
||||
const blanks = [alpha + 1, alpha + 2, alpha + 3].map(i => m.fullToOp[i]);
|
||||
assert.equal(blanks.filter(x => x === null).length, 2, `two of three blanks collapse: ${JSON.stringify(blanks)}`);
|
||||
assert.deepEqual(blanks.filter(x => x !== null), [opAlpha + 1]);
|
||||
const end = lineNo(PLAN_C, l => l.startsWith('end'));
|
||||
assert.equal(m.opToFull[lineNo(m.text, l => l === 'end')], end);
|
||||
});
|
||||
|
||||
test('operativeLines: a log section right after a line with no blank keeps the rest of the map', () => {
|
||||
// Joining the kept sections puts a blank where the log section stood; no full
|
||||
// line carries it. The lines after it must still map.
|
||||
const text = '# P\n\n## Context\n\nalpha\n## Critic Verdict\n\n```json\n{}\n```\n## Tail\n\nomega\n';
|
||||
const m = operativeLines(text);
|
||||
assert.equal(m.text, operativeText(text));
|
||||
const omegaFull = lineNo(text, l => l === 'omega');
|
||||
const omegaOp = lineNo(m.text, l => l === 'omega');
|
||||
assert.equal(m.fullToOp[omegaFull], omegaOp);
|
||||
assert.equal(m.fullToOp[lineNo(text, l => l === '## Tail')], lineNo(m.text, l => l === '## Tail'));
|
||||
assert.equal(m.fullToOp[lineNo(text, l => l === '## Critic Verdict')], null);
|
||||
});
|
||||
|
||||
// ---- lineDiff ------------------------------------------------------------------
|
||||
|
||||
// Hunks are 1-based and inclusive; an empty side has end = start - 1.
|
||||
|
||||
test('lineDiff: identical texts give empty inside and no hunks (CRLF normalized)', () => {
|
||||
const t = 'a\nb\nc\n';
|
||||
for (const other of [t, 'a\r\nb\r\nc\r\n']) {
|
||||
const d = lineDiff(t, other);
|
||||
assert.equal(d.inside.size, 0);
|
||||
assert.deepEqual(d.hunks, []);
|
||||
assert.deepEqual(d.oldToNew.slice(1), [1, 2, 3]);
|
||||
}
|
||||
});
|
||||
|
||||
test('lineDiff: an insert', () => {
|
||||
const d = lineDiff('a\nb\nc\n', 'a\nb\nX\nc\n');
|
||||
assert.deepEqual([...d.inside], [3]);
|
||||
assert.deepEqual(d.hunks, [{ oldStart: 3, oldEnd: 2, newStart: 3, newEnd: 3 }]);
|
||||
assert.deepEqual(d.oldToNew.slice(1), [1, 2, 4]);
|
||||
});
|
||||
|
||||
test('lineDiff: a delete', () => {
|
||||
const d = lineDiff('a\nb\nc\nd\n', 'a\nb\nd\n');
|
||||
assert.deepEqual(d.hunks, [{ oldStart: 3, oldEnd: 3, newStart: 3, newEnd: 2 }]);
|
||||
assert.deepEqual(d.oldToNew.slice(1), [1, 2, null, 3]);
|
||||
});
|
||||
|
||||
test('lineDiff: a replace', () => {
|
||||
const d = lineDiff('a\nb\nc\n', 'a\nX\nY\nc\n');
|
||||
assert.deepEqual([...d.inside].sort((x, y) => x - y), [2, 3]);
|
||||
assert.deepEqual(d.hunks, [{ oldStart: 2, oldEnd: 2, newStart: 2, newEnd: 3 }]);
|
||||
assert.deepEqual(d.oldToNew.slice(1), [1, null, 4]);
|
||||
});
|
||||
|
||||
test('lineDiff: two adjacent hunks stay two', () => {
|
||||
const d = lineDiff('a\nb\nc\nd\ne\n', 'a\nX\nc\nY\ne\n');
|
||||
assert.deepEqual(d.hunks, [
|
||||
{ oldStart: 2, oldEnd: 2, newStart: 2, newEnd: 2 },
|
||||
{ oldStart: 4, oldEnd: 4, newStart: 4, newEnd: 4 },
|
||||
]);
|
||||
assert.deepEqual([...d.inside].sort((x, y) => x - y), [2, 4]);
|
||||
assert.deepEqual(d.oldToNew.slice(1), [1, null, 3, null, 5]);
|
||||
});
|
||||
|
||||
test('lineDiff: oldToNew follows an insert above', () => {
|
||||
const d = lineDiff('a\nb\nc\n', 'X\nY\na\nb\nc\n');
|
||||
assert.deepEqual(d.oldToNew.slice(1), [3, 4, 5]);
|
||||
assert.deepEqual([...d.inside].sort((x, y) => x - y), [1, 2]);
|
||||
assert.deepEqual(d.hunks, [{ oldStart: 1, oldEnd: 0, newStart: 1, newEnd: 2 }]);
|
||||
});
|
||||
|
||||
test('lineDiff: a pure deletion marks both neighbours (one at the end of the text)', () => {
|
||||
const mid = lineDiff('a\nb\nc\nd\n', 'a\nd\n');
|
||||
assert.deepEqual([...mid.inside].sort((x, y) => x - y), [1, 2]);
|
||||
const end = lineDiff('a\nb\nc\n', 'a\nb\n');
|
||||
assert.deepEqual([...end.inside], [2]);
|
||||
assert.deepEqual(end.hunks, [{ oldStart: 3, oldEnd: 3, newStart: 3, newEnd: 2 }]);
|
||||
});
|
||||
|
||||
test('lineDiff: a step inserted with duplicated boilerplate marks the new copy, not the old one', () => {
|
||||
// Title and last line change too, so prefix/suffix trimming cannot do the work:
|
||||
// the boilerplate tail of step 1 and the inserted step 2 share a gap with no unique line.
|
||||
const s1 = step(1);
|
||||
const s2 = step(2);
|
||||
const s3 = step(3);
|
||||
const oldText = ['# Old title', '', s1, '', s3, '', 'old end', ''].join('\n');
|
||||
const newText = ['# New title', '', s1, '', s2, '', s3, '', 'new end', ''].join('\n');
|
||||
const d = lineDiff(oldText, newText);
|
||||
const nl = lines(newText);
|
||||
const s1Start = lineNo(newText, l => l.startsWith('### Step 1:'));
|
||||
const s2Start = lineNo(newText, l => l.startsWith('### Step 2:'));
|
||||
const s3Start = lineNo(newText, l => l.startsWith('### Step 3:'));
|
||||
const last = nl.length - 1; // the final '' after the trailing newline is not a line
|
||||
const expected = [1];
|
||||
for (let i = s2Start; i < s3Start; i++) expected.push(i);
|
||||
expected.push(last);
|
||||
assert.deepEqual([...d.inside].sort((x, y) => x - y), expected);
|
||||
for (let i = s1Start; i < s2Start; i++) assert.ok(!d.inside.has(i), `step 1 line ${i} "${nl[i - 1]}" is not inside`);
|
||||
// Old step 1 keeps its place; old step 3 moves down by step 2 plus its blank.
|
||||
const oS1 = lineNo(oldText, l => l.startsWith('### Step 1:'));
|
||||
const oS3 = lineNo(oldText, l => l.startsWith('### Step 3:'));
|
||||
for (let o = oS1; o < oS3; o++) assert.equal(d.oldToNew[o], o, `old line ${o} unchanged`);
|
||||
const shift = s3Start - oS3;
|
||||
assert.equal(shift, s2Start - s1Start, 'step 2 has as many lines as step 1');
|
||||
for (let o = oS3; o < oS3 + lines(s3).length; o++) assert.equal(d.oldToNew[o], o + shift);
|
||||
});
|
||||
|
||||
test('lineDiff: the same input twice gives deep-equal output', () => {
|
||||
const a = ['# T', '', step(1), '', step(3), ''].join('\n');
|
||||
const b = ['# T', '', step(1), '', step(2), '', step(3), ''].join('\n');
|
||||
assert.deepEqual(lineDiff(a, b), lineDiff(a, b));
|
||||
assert.deepEqual(lineDiff(PLAN_A, operativeText(PLAN_B)), lineDiff(PLAN_A, operativeText(PLAN_B)));
|
||||
});
|
||||
|
||||
// ---- architecture rules ----------------------------------------------------------
|
||||
|
||||
/** Every `from '...'`, `import('...')` and `require('...')` specifier that breaks a rule. */
|
||||
function importViolations(source) {
|
||||
const re = /\bfrom\s*(['"`])([^'"`]+)\1|\bimport\s*\(\s*(['"`])([^'"`]+)\3\s*\)|\brequire\s*\(\s*(['"`])([^'"`]+)\5\s*\)/g;
|
||||
const out = [];
|
||||
for (const m of String(source).matchAll(re)) {
|
||||
const spec = m[2] ?? m[4] ?? m[6];
|
||||
if (/(^|\/)plan-gate\.mjs$/.test(spec) || /^(node:)?(fs|child_process)(\/|$)/.test(spec)) out.push(spec);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
test('architecture rules: critic-diff imports neither plan-gate nor fs/child_process (planted violations caught)', () => {
|
||||
assert.ok(existsSync(CRITIC_DIFF), 'lib/review/critic-diff.mjs exists');
|
||||
const src = readFileSync(CRITIC_DIFF, 'utf-8');
|
||||
assert.deepEqual(importViolations(src), [], 'the real source passes');
|
||||
const plantedGate = `${src}\nimport { planHash } from '../validators/plan-gate.mjs';\n`;
|
||||
assert.deepEqual(importViolations(plantedGate), ['../validators/plan-gate.mjs'], 'a planted plan-gate import fails');
|
||||
const plantedFs = `${src}\nawait import('node:fs');\n`;
|
||||
assert.deepEqual(importViolations(plantedFs), ['node:fs'], 'a planted dynamic fs import fails');
|
||||
assert.deepEqual(importViolations(src), [], 'the real source passes again');
|
||||
});
|
||||
90
tests/lib/critic-severity-rules.test.mjs
Normal file
90
tests/lib/critic-severity-rules.test.mjs
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
// tests/lib/critic-severity-rules.test.mjs
|
||||
// The two plan reviewers (plan-critic, scope-guardian) must state rule B word for
|
||||
// word as lib/review/critic-diff.mjs exports it, keep the 2026-10-08 rule that a
|
||||
// test which cannot fail on a wrong implementation is major, tag security
|
||||
// findings with the catalogue's security keys, and carry the round-2 fields
|
||||
// (`previous`, `line_end`, `lowered_from`, `caught_by`) in their json template.
|
||||
// The name is the brief's; lib/util/test-census.mjs counts this file as a
|
||||
// behaviour test (it does not match PIN_FILE_RE), not as a doc pin.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { RULE_B } from '../../lib/review/critic-diff.mjs';
|
||||
import { RULE_CATALOGUE } from '../../lib/review/rule-catalogue.mjs';
|
||||
|
||||
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
const read = rel => readFileSync(join(ROOT, rel), 'utf-8');
|
||||
const norm = s => s.replace(/\s+/g, ' ');
|
||||
|
||||
const AGENTS = ['agents/plan-critic.md', 'agents/scope-guardian.md'];
|
||||
|
||||
// The one json template an agent file carries: the fenced block after
|
||||
// "Machine-readable findings block".
|
||||
function jsonTemplate(text, rel) {
|
||||
const head = text.indexOf('Machine-readable findings block');
|
||||
assert.ok(head >= 0, `${rel}: machine-readable findings block heading missing`);
|
||||
const m = /```json\n([\s\S]*?)\n```/.exec(text.slice(head));
|
||||
assert.ok(m, `${rel}: no fenced json template after the heading`);
|
||||
return JSON.parse(m[1]);
|
||||
}
|
||||
|
||||
test('the catalogue has exactly two security keys: SECURITY_INJECTION and SECURITY_REVIEW_VULNERABILITY', () => {
|
||||
const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key).sort();
|
||||
assert.deepEqual(keys, ['SECURITY_INJECTION', 'SECURITY_REVIEW_VULNERABILITY']);
|
||||
});
|
||||
|
||||
for (const rel of AGENTS) {
|
||||
test(`${rel}: states rule B verbatim (never a blocker, never a security finding)`, () => {
|
||||
const t = norm(read(rel));
|
||||
assert.ok(t.includes(norm(RULE_B)), `${rel} must contain the RULE_B sentence from critic-diff.mjs`);
|
||||
assert.ok(t.includes('never a blocker, never a security finding'));
|
||||
assert.ok(
|
||||
t.includes('The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong implementation catches nothing, so the finding stays major.'),
|
||||
`${rel} must say the 2026-10-08 rule still holds for rule B`,
|
||||
);
|
||||
assert.ok(t.includes('Give `lowered_from` and `caught_by` when you lower a finding this way.'));
|
||||
assert.ok(!/\brule A\b/i.test(t), `${rel} must not state rule A (plan Design § Gotchas)`);
|
||||
});
|
||||
|
||||
test(`${rel}: keeps the 2026-10-08 rule (cannot fail on a wrong implementation is major; catalogue tier)`, () => {
|
||||
const raw = read(rel);
|
||||
assert.match(raw, /cannot fail on a wrong\s+implementation[\s\S]{0,200}\*\*major\*\*/);
|
||||
assert.ok(norm(raw).includes("Whenever a finding is the same class as a catalogue rule, use that rule's tier"));
|
||||
});
|
||||
|
||||
test(`${rel}: a security finding uses the catalogue's security rule_key`, () => {
|
||||
const t = norm(read(rel));
|
||||
const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key);
|
||||
for (const k of keys) assert.ok(t.includes(`\`${k}\``), `${rel} must name \`${k}\``);
|
||||
assert.ok(t.includes("A security finding uses the catalogue's security `rule_key`"));
|
||||
});
|
||||
|
||||
test(`${rel}: round 2+ lists every previous finding by id and cites the exact line`, () => {
|
||||
const t = norm(read(rel));
|
||||
assert.ok(t.includes('From round 2 the prompt gives you the diff since the last reviewed text'), `${rel}: round-2 diff sentence missing`);
|
||||
assert.ok(t.includes('List every previous finding by id in `previous`, with `resolved: true` only when the plan now fixes it.'));
|
||||
assert.ok(t.includes('Cite the exact line, with `line_end` for a range.'));
|
||||
});
|
||||
|
||||
test(`${rel}: the json template carries previous, line_end, lowered_from and caught_by beside the old fields`, () => {
|
||||
const tpl = jsonTemplate(read(rel), rel);
|
||||
assert.equal(typeof tpl.agent, 'string');
|
||||
assert.ok(Array.isArray(tpl.findings) && tpl.findings.length >= 1, 'findings array with one example');
|
||||
assert.ok(Array.isArray(tpl.previous) && tpl.previous.length >= 1, 'previous array with one example');
|
||||
assert.ok('id' in tpl.previous[0] && 'resolved' in tpl.previous[0], 'previous entries carry id and resolved');
|
||||
const f = tpl.findings[0];
|
||||
for (const k of ['file', 'line', 'rule_key', 'severity', 'text', 'line_end', 'lowered_from', 'caught_by']) {
|
||||
assert.ok(k in f, `finding template lacks "${k}"`);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test('plan-critic no longer tells the critic to judge only the plan in front of it', () => {
|
||||
assert.ok(
|
||||
!norm(read('agents/plan-critic.md')).includes('judge the plan in front of you'),
|
||||
'the round-2 paragraph replaces "judge the plan in front of you"',
|
||||
);
|
||||
});
|
||||
|
|
@ -14,6 +14,10 @@ import { dirname, join } from 'node:path';
|
|||
import { tmpdir } from 'node:os';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { dedupFindings, tokenize, DEFAULT_THRESHOLD } from '../../lib/review/plan-review-dedup.mjs';
|
||||
// Namespace import: a missing export reads as undefined and fails an assertion,
|
||||
// it does not crash the whole file at link time.
|
||||
import * as dedupModule from '../../lib/review/plan-review-dedup.mjs';
|
||||
import { computeFindingId } from '../../lib/parsers/finding-id.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const SHIM = join(HERE, '..', '..', 'lib', 'review', 'plan-review-dedup.mjs');
|
||||
|
|
@ -196,3 +200,223 @@ test('CLI --stdin exits non-zero on malformed input (loud, not a silent empty me
|
|||
'malformed --stdin must fail loudly so a broken hand-off cannot hide behind an empty merge',
|
||||
);
|
||||
});
|
||||
|
||||
// --- Converging critic loop, Step 3 — the reviewers' marks and rule-B fields
|
||||
// pass through dedup, and two dedup defects are fixed (line-0 collapse,
|
||||
// a bad line / empty file / empty rule_key crashing the id) ---
|
||||
|
||||
const HEX_A = 'a'.repeat(40);
|
||||
const HEX_B = '0123456789abcdef0123456789abcdef01234567';
|
||||
const HEX_C = 'f'.repeat(40);
|
||||
|
||||
function runStdin(obj) {
|
||||
return JSON.parse(execFileSync(process.execPath, [SHIM, '--stdin'], {
|
||||
input: JSON.stringify(obj), encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'],
|
||||
}));
|
||||
}
|
||||
|
||||
test('the new per-finding fields survive dedupFindings (line_end, lowered_from, caught_by, rule_keys)', () => {
|
||||
const r = dedupFindings([{ agent: 'plan-critic', payload: { findings: [
|
||||
{ file: 'plan.md', line: 3, line_end: 5, rule_key: 'PC1', severity: 'minor', lowered_from: 'Major', caught_by: 'tests/lib/x.test.mjs', text: 'first finding about the token check' },
|
||||
] } }]);
|
||||
const f = r.findings[0];
|
||||
assert.equal(f.line_end, 5);
|
||||
assert.equal(f.lowered_from, 'major', 'lowered_from goes through normalizeSeverity');
|
||||
assert.equal(f.caught_by, 'tests/lib/x.test.mjs');
|
||||
assert.deepEqual(f.rule_keys, ['PC1']);
|
||||
assert.equal(f.severity, 'minor');
|
||||
});
|
||||
|
||||
test('unusable per-finding fields are dropped: line_end below line or not an integer, empty caught_by, no rule_key', () => {
|
||||
const one = finding => dedupFindings([{ agent: 'plan-critic', payload: { findings: [finding] } }]).findings[0];
|
||||
assert.equal('line_end' in one({ file: 'p', line: 3, line_end: 2, rule_key: 'k', text: 't' }), false, 'line_end < line');
|
||||
assert.equal('line_end' in one({ file: 'p', line: 3, line_end: 4.5, rule_key: 'k', text: 't' }), false, 'line_end not an integer');
|
||||
assert.equal('line_end' in one({ file: 'p', line: 3, line_end: '5', rule_key: 'k', text: 't' }), false, 'line_end a string');
|
||||
assert.equal(one({ file: 'p', line: 3, line_end: 3, rule_key: 'k', text: 't' }).line_end, 3, 'line_end equal to line is kept');
|
||||
assert.equal('caught_by' in one({ file: 'p', line: 3, rule_key: 'k', caught_by: '', text: 't' }), false, 'empty caught_by');
|
||||
assert.equal('caught_by' in one({ file: 'p', line: 3, rule_key: 'k', caught_by: 7, text: 't' }), false, 'caught_by not a string');
|
||||
assert.equal('lowered_from' in one({ file: 'p', line: 3, rule_key: 'k', text: 't' }), false, 'no lowered_from in, none out');
|
||||
assert.equal('rule_keys' in one({ file: 'p', line: 3, text: 't' }), false, 'no rule_key, no rule_keys');
|
||||
});
|
||||
|
||||
test('CLI --stdin passes marks per reviewer and the new fields through; sources stays {agent, present}', () => {
|
||||
const parsed = runStdin({
|
||||
plan_critic: {
|
||||
agent: 'plan-critic',
|
||||
previous: [{ id: HEX_A, resolved: true }, { id: HEX_B, resolved: false, note: 'extra field' }],
|
||||
findings: [{ file: 'plan.md', line: 8, line_end: 9, rule_key: 'PC1', severity: 'minor', lowered_from: 'major', caught_by: 'npm test', text: 'alpha finding about the snapshot path' }],
|
||||
},
|
||||
scope_guardian: {
|
||||
agent: 'scope-guardian',
|
||||
previous: [{ id: HEX_A, resolved: false }],
|
||||
findings: [],
|
||||
},
|
||||
});
|
||||
assert.deepEqual(parsed.marks, [
|
||||
{ agent: 'plan-critic', id: HEX_A, resolved: true },
|
||||
{ agent: 'plan-critic', id: HEX_B, resolved: false },
|
||||
{ agent: 'scope-guardian', id: HEX_A, resolved: false },
|
||||
]);
|
||||
assert.deepEqual(parsed.sources, [
|
||||
{ agent: 'plan-critic', present: true },
|
||||
{ agent: 'scope-guardian', present: true },
|
||||
]);
|
||||
const f = parsed.findings[0];
|
||||
assert.equal(f.line_end, 9);
|
||||
assert.equal(f.lowered_from, 'major');
|
||||
assert.equal(f.caught_by, 'npm test');
|
||||
assert.deepEqual(f.rule_keys, ['PC1']);
|
||||
});
|
||||
|
||||
test('a malformed mark is dropped, never thrown on; marks keep input order; no previous gives []', () => {
|
||||
const r = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { agent: 'plan-critic', findings: [], previous: [
|
||||
null,
|
||||
'not-an-object',
|
||||
{ id: 'A'.repeat(40), resolved: true }, // upper-case hex
|
||||
{ id: 'a'.repeat(39), resolved: true }, // 39 hex
|
||||
{ id: HEX_A, resolved: 'true' }, // resolved not boolean
|
||||
{ id: HEX_A, resolved: 1 },
|
||||
{ id: 123, resolved: true }, // id not a string
|
||||
{ resolved: true }, // no id
|
||||
{ id: HEX_C, resolved: false },
|
||||
{ id: HEX_B, resolved: true },
|
||||
] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [], previous: 'not-an-array' } },
|
||||
{ agent: 'third', payload: { findings: [] } },
|
||||
]);
|
||||
assert.deepEqual(r.marks, [
|
||||
{ agent: 'plan-critic', id: HEX_C, resolved: false },
|
||||
{ agent: 'plan-critic', id: HEX_B, resolved: true },
|
||||
]);
|
||||
assert.deepEqual(r.sources, [
|
||||
{ agent: 'plan-critic', present: true },
|
||||
{ agent: 'scope-guardian', present: true },
|
||||
{ agent: 'third', present: true },
|
||||
]);
|
||||
const none = dedupFindings([{ agent: 'plan-critic', payload: null }]);
|
||||
assert.deepEqual(none.marks, []);
|
||||
});
|
||||
|
||||
test('a merge takes the union of rule_keys: a security-keyed finding merged into a differently-keyed survivor keeps its key', () => {
|
||||
const text = 'the write path builds a shell command from the plan text without quoting';
|
||||
const r = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [{ file: 'plan.md', line: 10, rule_key: 'PLAN_EXECUTE_DRIFT', severity: 'major', text }] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [{ file: 'plan.md', line: 30, rule_key: 'SECURITY_INJECTION', severity: 'major', text }] } },
|
||||
]);
|
||||
assert.equal(r.findings.length, 1);
|
||||
assert.equal(r.findings[0].rule_key, 'PLAN_EXECUTE_DRIFT', 'the survivor keeps its own rule_key');
|
||||
assert.deepEqual(r.findings[0].rule_keys, ['PLAN_EXECUTE_DRIFT', 'SECURITY_INJECTION']);
|
||||
const same = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [{ file: 'plan.md', line: 10, rule_key: 'PC1', text }] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [{ file: 'plan.md', line: 10, rule_key: 'PC1', text }] } },
|
||||
]);
|
||||
assert.deepEqual(same.findings[0].rule_keys, ['PC1'], 'a key is held once');
|
||||
});
|
||||
|
||||
test('a merge with a more severe partner takes its severity and its lowered_from/caught_by, or drops them', () => {
|
||||
const at = (severity, extra = {}) => ({ file: 'plan.md', line: 42, rule_key: 'PC1', severity, text: 'step four lacks a verifiable acceptance criterion', ...extra });
|
||||
// Partner more severe, partner has no rule-B fields: the survivor's are dropped.
|
||||
const dropped = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [at('minor', { lowered_from: 'major', caught_by: 'test a' })] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [at('blocker')] } },
|
||||
]).findings;
|
||||
assert.equal(dropped.length, 1);
|
||||
assert.equal(dropped[0].severity, 'blocker');
|
||||
assert.equal('lowered_from' in dropped[0], false);
|
||||
assert.equal('caught_by' in dropped[0], false);
|
||||
// Partner more severe with its own rule-B fields: they are taken.
|
||||
const taken = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [at('minor')] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [at('major', { lowered_from: 'blocker', caught_by: 'gate b' })] } },
|
||||
]).findings[0];
|
||||
assert.equal(taken.severity, 'major');
|
||||
assert.equal(taken.lowered_from, 'blocker');
|
||||
assert.equal(taken.caught_by, 'gate b');
|
||||
// Equal severities: the survivor keeps its own.
|
||||
const kept = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [at('minor', { lowered_from: 'major', caught_by: 'test a' })] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [at('minor', { lowered_from: 'major', caught_by: 'test b' })] } },
|
||||
]).findings[0];
|
||||
assert.equal(kept.caught_by, 'test a');
|
||||
const keptAlone = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [at('minor', { lowered_from: 'major', caught_by: 'test a' })] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [at('minor')] } },
|
||||
]).findings[0];
|
||||
assert.equal(keptAlone.lowered_from, 'major');
|
||||
assert.equal(keptAlone.caught_by, 'test a');
|
||||
});
|
||||
|
||||
test('two line-0 findings with one rule_key and different texts stay two, with distinct ids', () => {
|
||||
const r = dedupFindings([{ agent: 'plan-critic', payload: { findings: [
|
||||
{ file: 'plan.md', line: 0, rule_key: 'PC1', text: 'the plan never names a rollback for the migration' },
|
||||
{ file: 'plan.md', line: 0, rule_key: 'PC1', text: 'no step owns the changelog entry for this release' },
|
||||
{ file: 'plan.md', line: 0, rule_key: 'PC1', text: 'the brief success criterion four has no command' },
|
||||
] } }]);
|
||||
assert.equal(r.findings.length, 3);
|
||||
assert.equal(r.dedup_stats.exact_id_dups, 0);
|
||||
assert.deepEqual(r.findings.map(f => f.id), [
|
||||
computeFindingId('plan.md', 0, 'PC1'),
|
||||
computeFindingId('plan.md', 0, 'PC1#2'),
|
||||
computeFindingId('plan.md', 0, 'PC1#3'),
|
||||
]);
|
||||
// The same line-0 finding from two reviewers still merges (through the text).
|
||||
const same = dedupFindings([
|
||||
{ agent: 'plan-critic', payload: { findings: [{ file: 'plan.md', line: 0, rule_key: 'PC1', text: 'the plan never names a rollback for the migration' }] } },
|
||||
{ agent: 'scope-guardian', payload: { findings: [{ file: 'plan.md', line: 0, rule_key: 'PC1', text: 'the plan never names a rollback for the migration' }] } },
|
||||
]);
|
||||
assert.equal(same.findings.length, 1);
|
||||
assert.deepEqual(same.findings[0].raised_by, ['plan-critic', 'scope-guardian']);
|
||||
});
|
||||
|
||||
test('a line that is not a finite integer becomes null; an empty file or rule_key becomes null; nothing throws', () => {
|
||||
const one = finding => dedupFindings([{ agent: 'plan-critic', payload: { findings: [finding] } }]).findings[0];
|
||||
for (const line of [[3, 5], '7', 2.5, Infinity, NaN, { n: 1 }]) {
|
||||
const f = one({ file: 'plan.md', line, rule_key: 'PC1', text: 't' });
|
||||
assert.equal(f.line, null, `line ${JSON.stringify(line)} → null`);
|
||||
assert.equal(f.id, computeFindingId('plan.md', 0, 'PC1'));
|
||||
}
|
||||
const noFile = one({ file: '', line: 4, rule_key: 'PC2', text: 't' });
|
||||
assert.equal(noFile.file, null);
|
||||
assert.equal(noFile.id, computeFindingId('unknown', 4, 'PC2'));
|
||||
const noKey = one({ file: 'plan.md', line: 7, rule_key: '', text: 't' });
|
||||
assert.equal(noKey.rule_key, null);
|
||||
assert.equal('rule_keys' in noKey, false);
|
||||
assert.equal(noKey.id, computeFindingId('plan.md', 7, 'unknown'));
|
||||
});
|
||||
|
||||
test('CLI --stdin: line [3, 5], file "" and rule_key "" no longer crash it (exit 0, line null)', () => {
|
||||
const parsed = runStdin({
|
||||
plan_critic: { agent: 'plan-critic', findings: [
|
||||
{ file: 'plan.md', line: [3, 5], rule_key: 'PC1', text: 'a finding cited with a line range array' },
|
||||
{ file: '', line: 4, rule_key: 'PC2', text: 'a finding with an empty file string' },
|
||||
{ file: 'plan.md', line: 7, rule_key: '', text: 'a finding with an empty rule key here' },
|
||||
] },
|
||||
});
|
||||
assert.equal(parsed.findings.length, 3);
|
||||
assert.equal(parsed.findings[0].line, null);
|
||||
assert.equal(parsed.findings[1].file, null);
|
||||
assert.equal(parsed.findings[2].rule_key, null);
|
||||
});
|
||||
|
||||
test('one reviewer, one file, line [3, 5], one rule_key, different texts: two findings with distinct ids, CLI exit 0', () => {
|
||||
const parsed = runStdin({
|
||||
plan_critic: { agent: 'plan-critic', findings: [
|
||||
{ file: 'plan.md', line: [3, 5], rule_key: 'PC1', text: 'the snapshot directory is never created before the write' },
|
||||
{ file: 'plan.md', line: [3, 5], rule_key: 'PC1', text: 'verify command runs the whole suite instead of one file' },
|
||||
] },
|
||||
});
|
||||
assert.equal(parsed.findings.length, 2);
|
||||
assert.deepEqual(parsed.findings.map(f => f.id), [
|
||||
computeFindingId('plan.md', 0, 'PC1'),
|
||||
computeFindingId('plan.md', 0, 'PC1#2'),
|
||||
]);
|
||||
assert.deepEqual(parsed.findings.map(f => f.line), [null, null]);
|
||||
});
|
||||
|
||||
test('contentOverlap is exported and gives 1 for a shorter text held by a longer one', () => {
|
||||
assert.equal(typeof dedupModule.contentOverlap, 'function', 'contentOverlap is exported');
|
||||
assert.equal(dedupModule.contentOverlap('The token is checked.', 'The token is not checked before the write.'), 1);
|
||||
assert.equal(dedupModule.contentOverlap('alpha beta', 'alpha gamma'), 0.5);
|
||||
assert.equal(dedupModule.contentOverlap('', 'alpha'), 0);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -289,7 +289,7 @@ test('the template\'s own example steps validate as 1.9 with none of the new pat
|
|||
// lines the release check accepts. The release check is imported as a module
|
||||
// namespace, so a missing parseDoors export fails an assertion, not the load.
|
||||
const releaseCheck = await import('../../scripts/release-check.mjs');
|
||||
const UNRELEASED = '## Unreleased — plan_version 1.9, the trial-4 findings, session-clean tests';
|
||||
const UNRELEASED = '## Unreleased — plan_version 1.9, the trial-4 findings, session-clean tests, the converging critic loop';
|
||||
|
||||
function firstChangelogSection() {
|
||||
const lines = read('CHANGELOG.md').split('\n');
|
||||
|
|
|
|||
147
tests/lib/review-loop-docs.test.mjs
Normal file
147
tests/lib/review-loop-docs.test.mjs
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
// tests/lib/review-loop-docs.test.mjs
|
||||
// Prose pins for the converging critic loop (decision 43): /trekplan Phase 9,
|
||||
// Handover 4, CLAUDE.md and the agent files must state rule A and rule B in the
|
||||
// exact words lib/review/critic-diff.mjs exports, name the optional verdict
|
||||
// fields, and no longer say the loop ends on a round with no BLOCKER.
|
||||
// Texts are compared with every run of whitespace read as one space, so a
|
||||
// sentence wrapped over two lines still counts.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { RULE_A, RULE_B, DIFF_FIELDS } from '../../lib/review/critic-diff.mjs';
|
||||
|
||||
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
||||
const read = rel => readFileSync(join(ROOT, rel), 'utf-8');
|
||||
const flat = text => String(text).replace(/\s+/g, ' ');
|
||||
|
||||
function phase(text, n) {
|
||||
const start = text.indexOf(`\n## Phase ${n} `);
|
||||
assert.ok(start >= 0, `## Phase ${n} missing`);
|
||||
const end = text.indexOf('\n## Phase ', start + 1);
|
||||
return text.slice(start, end < 0 ? undefined : end);
|
||||
}
|
||||
|
||||
function handover4() {
|
||||
const contract = read('docs/HANDOVER-CONTRACTS.md');
|
||||
const start = contract.indexOf('## Handover 4');
|
||||
const end = contract.indexOf('## Handover 5');
|
||||
assert.ok(start >= 0 && end > start, 'Handover 4 and Handover 5 headings found in order');
|
||||
return contract.slice(start, end);
|
||||
}
|
||||
|
||||
const P9 = flat(phase(read('commands/trekplan.md'), 9));
|
||||
|
||||
test('RULE_A stands verbatim in Phase 9, Handover 4, CLAUDE.md and the planning orchestrator', () => {
|
||||
const rule = flat(RULE_A);
|
||||
assert.ok(P9.includes(rule), 'Phase 9 of commands/trekplan.md states RULE_A');
|
||||
assert.ok(flat(handover4()).includes(rule), 'Handover 4 of docs/HANDOVER-CONTRACTS.md states RULE_A');
|
||||
assert.ok(flat(read('CLAUDE.md')).includes(rule), 'CLAUDE.md states RULE_A');
|
||||
assert.ok(flat(read('agents/planning-orchestrator.md')).includes(rule), 'agents/planning-orchestrator.md states RULE_A');
|
||||
});
|
||||
|
||||
test('RULE_B stands verbatim in Phase 9 and in both reviewer agent files', () => {
|
||||
const rule = flat(RULE_B);
|
||||
assert.ok(P9.includes(rule), 'Phase 9 of commands/trekplan.md states RULE_B');
|
||||
for (const rel of ['agents/plan-critic.md', 'agents/scope-guardian.md']) {
|
||||
assert.ok(flat(read(rel)).includes(rule), `${rel} states RULE_B`);
|
||||
}
|
||||
});
|
||||
|
||||
test('Handover 4 names every DIFF_FIELDS entry and says absent means round-1 behaviour', () => {
|
||||
const h4 = handover4();
|
||||
assert.ok(DIFF_FIELDS.length > 0, 'DIFF_FIELDS lists the optional fields');
|
||||
const missing = DIFF_FIELDS.filter(f => !h4.includes(`\`${f}\``));
|
||||
assert.deepEqual(missing, [], `Handover 4 does not name (backticked): ${missing.join(', ')}`);
|
||||
assert.ok(flat(h4).includes('absent means round-1 behaviour'), 'Handover 4 says "absent means round-1 behaviour"');
|
||||
});
|
||||
|
||||
test('Phase 9 names plan-gate --diff and says round 1 is unchanged', () => {
|
||||
assert.ok(P9.includes('plan-gate.mjs --diff'), 'Phase 9 runs plan-gate.mjs --diff');
|
||||
assert.match(P9, /round 1 is unchanged/i, 'Phase 9 says "round 1 is unchanged"');
|
||||
});
|
||||
|
||||
test('Phase 9 no longer says the loop ends on a round with no BLOCKER', () => {
|
||||
assert.ok(!P9.includes('ends on a round with no BLOCKER'), 'the stale "ends on a round with no BLOCKER" is gone');
|
||||
});
|
||||
|
||||
// The narrow round of the A/B review (operator decisions 48 and 50): the
|
||||
// CHANGELOG's open Unreleased section records it, Phase 9 says what a merge
|
||||
// keeps, and Handover 4 states the matcher's overlap rule and the distinct id.
|
||||
// The release check is imported as a module namespace, so a missing parseDoors
|
||||
// export fails an assertion, not the load.
|
||||
const releaseCheck = await import('../../scripts/release-check.mjs');
|
||||
|
||||
// The body of the open `## Unreleased` section, up to the next `## ` heading.
|
||||
function unreleasedBody() {
|
||||
const lines = read('CHANGELOG.md').split('\n');
|
||||
const start = lines.findIndex(l => /^## Unreleased\b/.test(l));
|
||||
assert.ok(start >= 0, 'CHANGELOG.md has a "## Unreleased" section');
|
||||
let end = lines.length;
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
if (/^## /.test(lines[i])) { end = i; break; }
|
||||
}
|
||||
return lines.slice(start + 1, end).join('\n');
|
||||
}
|
||||
|
||||
// A `### <name>` body of the Unreleased section, up to the next heading.
|
||||
function subsection(body, name) {
|
||||
const lines = body.split('\n');
|
||||
const start = lines.findIndex(l => l.replace(/\s+$/, '') === `### ${name}`);
|
||||
assert.ok(start >= 0, `the Unreleased section has a "### ${name}" heading`);
|
||||
let end = lines.length;
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
if (/^###? /.test(lines[i])) { end = i; break; }
|
||||
}
|
||||
return lines.slice(start + 1, end).join('\n');
|
||||
}
|
||||
|
||||
// The top-level bullets of a slice: each `- ` line at column 0 with its
|
||||
// wrapped and nested lines, whitespace read as one space.
|
||||
function topBullets(slice) {
|
||||
const out = [];
|
||||
for (const line of slice.split('\n')) {
|
||||
if (/^- /.test(line)) out.push(line);
|
||||
else if (out.length && line.trim() !== '') out[out.length - 1] += `\n${line}`;
|
||||
}
|
||||
return out.map(flat);
|
||||
}
|
||||
|
||||
test('CHANGELOG: the narrow round of operator decisions 48 and 50', () => {
|
||||
assert.equal(typeof releaseCheck.parseDoors, 'function', 'scripts/release-check.mjs exports parseDoors');
|
||||
const body = unreleasedBody();
|
||||
const d = releaseCheck.parseDoors(body);
|
||||
assert.equal(d.found, true, 'the Unreleased section has a "### Doors" heading');
|
||||
assert.deepEqual(d.bad, [], 'every top-level door line reads "- one-way: ..." or "- two-way: ..."');
|
||||
const doors48 = d.entries.filter(e => /operator\s+decision\s+48\b/.test(flat(e.text)));
|
||||
assert.ok(doors48.length >= 3,
|
||||
`at least three door lines name operator decision 48: ${JSON.stringify(d.entries.map(e => e.text))}`);
|
||||
|
||||
const fixed = flat(subsection(body, 'Fixed'));
|
||||
for (const id of ['4ab24b07', 'd03835c4', '69c9cebd', '8782602b', 'ebe164e2']) {
|
||||
assert.ok(fixed.includes(id), `the Fixed slice names review ${id}`);
|
||||
}
|
||||
|
||||
const known = topBullets(subsection(body, 'Known issues'));
|
||||
for (const id of ['8a9b5a17', '8004ea85', 'c257625e', '818b1346']) {
|
||||
const hits = known.filter(b => b.includes(id));
|
||||
assert.equal(hits.length, 1, `exactly one Known-issues bullet names ${id}: ${JSON.stringify(hits)}`);
|
||||
assert.match(hits[0], /operator\s+decision\s+48\b/, `the ${id} Known-issues bullet names operator decision 48: ${JSON.stringify(hits[0])}`);
|
||||
}
|
||||
const residual = known.filter(b => b.includes('4ab24b07'));
|
||||
assert.equal(residual.length, 1, `exactly one Known-issues bullet names 4ab24b07: ${JSON.stringify(residual)}`);
|
||||
assert.match(residual[0], /operator\s+decision\s+50\b/, `the 4ab24b07 Known-issues bullet names operator decision 50: ${JSON.stringify(residual[0])}`);
|
||||
});
|
||||
|
||||
test('Phase 9 says the merge keeps the copy rule B counts more severe', () => {
|
||||
assert.ok(P9.includes('keeps the copy rule B counts more severe'), 'Phase 9 says "keeps the copy rule B counts more severe"');
|
||||
assert.ok(!P9.includes('keeps the more severe of the two severities'), 'the stale "keeps the more severe of the two severities" is gone');
|
||||
});
|
||||
|
||||
test('Handover 4 states the overlap match and the distinct id', () => {
|
||||
const h4 = flat(handover4());
|
||||
assert.ok(h4.includes('only on overlapping text'), 'Handover 4 says the gate matches "only on overlapping text"');
|
||||
assert.ok(h4.includes('`<rule_key>#n`'), 'Handover 4 names `<rule_key>#n`');
|
||||
});
|
||||
|
|
@ -12,9 +12,10 @@
|
|||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { fastestWithin } from '../helpers/timing.mjs';
|
||||
import * as timing from '../helpers/timing.mjs';
|
||||
import { HOOK_TIMEOUT_MS, runHookWithEnv } from '../helpers/hook-helper.mjs';
|
||||
|
|
@ -81,3 +82,24 @@ test('red proof control run (703d87e1): the timeout leaves headroom over a slow
|
|||
assert.ok(c.hangMs >= c.timeoutMs + 30000, `the hang outlasts the timeout by 30 s: ${seen}`);
|
||||
assert.ok(c.hangMs < 2 ** 31 - 1, `the hang fits a real timer (below 2^31-1 ms): ${seen}`);
|
||||
});
|
||||
|
||||
// The post-compact hook test (tests/hooks/post-compact-flush.test.mjs) ran its
|
||||
// hook child with a fixed 5 s limit and was killed under full-suite load
|
||||
// (decision 44, the same class as 703d87e1). Its limit now comes from
|
||||
// tests/helpers/timing.mjs (HOOK_CHILD_TIMEOUT_MS), shared with the hook
|
||||
// helper. The fallback 5000 is the record of the limit before decision 44,
|
||||
// taken only when the export is absent.
|
||||
test('post-compact-flush (decision 44): its child timeout comes from tests/helpers/timing.mjs', () => {
|
||||
const hookTest = readFileSync(
|
||||
join(dirname(fileURLToPath(import.meta.url)), '..', 'hooks', 'post-compact-flush.test.mjs'),
|
||||
'utf8',
|
||||
);
|
||||
assert.ok(hookTest.includes("'../helpers/timing.mjs'"), 'the hook test imports from ../helpers/timing.mjs');
|
||||
assert.ok(hookTest.includes('timeout: HOOK_CHILD_TIMEOUT_MS'), 'the hook test uses timeout: HOOK_CHILD_TIMEOUT_MS');
|
||||
assert.doesNotMatch(hookTest, /\b5_?000\b/, 'the hook test holds no 5000 literal');
|
||||
const limit = timing.HOOK_CHILD_TIMEOUT_MS ?? 5000;
|
||||
assert.ok(limit > 5000, `the shared child timeout is above the old 5 s: ${limit}`);
|
||||
assert.equal(HOOK_TIMEOUT_MS, timing.HOOK_CHILD_TIMEOUT_MS, 'the hook helper and the hook test share one number');
|
||||
assert.ok(hookTest.includes('child.exitCode ?? 1'), 'a killed child reads as non-zero');
|
||||
assert.ok(!hookTest.includes('?? 0'), 'no killed child reads as exit 0');
|
||||
});
|
||||
|
|
|
|||
1196
tests/validators/plan-gate-diff-mode.test.mjs
Normal file
1196
tests/validators/plan-gate-diff-mode.test.mjs
Normal file
File diff suppressed because it is too large
Load diff
Loading…
Add table
Add a link
Reference in a new issue