The minor points (B8-B12, C13, C14, C16, C17) of the second voyage-lekerepo
trial (2026-10-08), each verified against the code and started with a test
that failed first.
- K10: optional step field `Regression:` - one command the criteria runner
allows, unpiped; plan-validator refuses one that can never run
(PLAN_STEP_REGRESSION_NOT_RUNNABLE). Any plan_version, no bump.
- K11: `On failure:` opens with revert | retry | skip | escalate, or the
validator warns (PLAN_STEP_ON_FAILURE_UNKNOWN); onFailureAction in
plan-schema.
- B8 (operator decision 2026-10-08): minors open after the last critic round
are listed numbered in the Phase 10 summary; the ones the operator picks are
fixed and get one more round (--extra-round when the 3 are spent).
- B11: plan-review dedup merges two reviewers' findings on the same file:line
with the same content (trial pair: Jaccard 0.44, overlap 0.83); the Jaccard
threshold for every other pair is unchanged.
- B12: /trekbrief Step 4c says the reviewer runs in the background and is
waited for, not "foreground, blocking".
- C13: the Phase 4 size count includes .mjs, .cjs, .mts, .cts.
- C14: --evidence shows the first output line on PASS rows.
- C16: findings-schema checks path:line citations in finding prose (error) and
in the prose around the block (warning) against the file.
- C17: git-historian gets the brief, never proposes changing a frozen file,
and names the remote from `git remote -v`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The pre-compact hook read only a checkpoint that started with `git commit`,
so every plan in the new `git add … && git commit` form rebuilt nothing, and
an empty match also skipped the session-state refresh. It now finds the
commit anywhere in the span and always refreshes; first test for the hook.
- A sub-bullet that carries its own command stays a criterion (folding it
left the command unrun); the shallowest bullet sets the base indent.
- Only a MINOR/SUGGESTION /code-review finding outside the range is released
from forbidding ALLOW: a MAJOR one may be a break cited at an unchanged
caller.
- `reuse` is an improvement category; the session-spec template stages too;
a negated bash 3.2 constraint, an env prefix and `bash -c` are handled.
- Stale comment, Declined-to-Judge wording and rule-key counts brought in line.
Tests fail on 7c11c2a first (9 of 24).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
voyage-lekerepo trial 2 (2026-10-08), report section A; the operator decided
they are fixed before the next release:
- A1: a /code-review finding whose category is simplification, efficiency,
style or readability enters as CODE_REVIEW_IMPROVEMENT (MINOR), not MAJOR.
- A2: recommended_action is the engine's own fix when transcribed, otherwise
a test-first step; never the failure scenario dressed as a remedy.
- A3: a /code-review finding outside the exact range it was given is declined
and listed, but no longer holds ALLOW back; /security-review's still does.
- A4: the manifest must_contain recipe uses grep -E -e ... -- so a pattern
like --sep is not read as an option.
- A5: indented sub-points belong to their criterion (counted), and a
criterion with sub-points and no command warns BRIEF_SC_SUBPOINTS.
- A6: a brief requiring bash 3.2 whose criterion runs plain bash warns
BRIEF_SC_BASH_NOT_PINNED; /trekbrief proposes /bin/bash.
- A7: the plan template's and /trekplan's checkpoints stage the step's files.
Tests fail on 5f9faee first (tests/lib/trial2-fixes.test.mjs, 11 of 14).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Independent read-only review of c56f852..2d29966 (2026-10-08), each defect
reproduced by the reviewer; each new test fails on that code.
- test-lock green ran ANY allowlisted command: `green --cmd "git status"`
marked a red step green and the stop guard let it stop. The lock now
stores the step's test command (`lock --cmd`); `green` runs only that
one (a different --cmd, or a lock without one, exits 2). trekexecute D1
calls `green` with no command, which also removes the quoting hazard.
- plan-gate --accept-open worked at round 1 and passed the gate; it now
refuses while rounds are left.
- findings-schema --repo-root swallowed a following flag, or took a root
that does not exist, and silently skipped the line check: exit 2.
- brief-review-stamp hashed only the body, so a changed `framing` or
`research_topics` stayed "current"; it now covers the reviewed
frontmatter too (only the post-review stamps are left out).
- red-proof red_kind read an assertion message naming SyntaxError as a
load error; the signatures are now specific.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
voyage-lekerepo end-to-end trial (2026-10-06), report points 1, 5, 11:
- engine-findings: recommended_action is "Fix <file:line> so this no longer
happens: <failure scenario>", not "Resolve: <title>" (8 of 8 engine
findings were set aside as actionability:empty); the coordinator never
sets an engine finding aside for Actionability.
- findings-schema --repo-root: a cited line past the end of an existing
file is FINDING_LINE_OUT_OF_RANGE, so Phase 5's bounded re-ask fixes it
before the coordinator sees it. The reviewers' instruction said "the
actual line number from the diff"; it now says the file's own line,
never the patch's.
- fill-finding-ids.mjs: the coordinator (no shell) writes @@ID_<n>@@ and
trekreview Phase 7 computes them over (file, line, rule_key); an
unresolved placeholder or a wrong 40-hex id fails. The 4-field id with
`title` the docs named is gone; the code always used three.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Wave 1, order 3 (P3 plan mode as the start, P2 design as its own part, P9
plan side). Contracts K3-K6 for the later merges.
- plan_version 1.8 (1.7 plans stay valid): Test first on every step, one
End-to-end ("run the thing") step, optional Interface in / Interface out /
Review focus fields, a ceremony class in the header (absent =
architectural), and for architectural plans a ## Design part: module map
without cycles, architecture rules that name their enforcer and the step
that proves them, a requirements check asked as keep / change.
- K3 lib/validators/plan-gate.mjs: the critic verdict is recorded INSIDE the
plan (Claude Code's plan approval hands a hook only the plan file's text),
bound to the hash taken before the reviewers read it; 0 go / 1 stop / 2
could not run; at most 3 critic rounds, then stop and ask.
- K5 lib/verification/arch-rules.mjs: runs the target repo's own
dependency-cruiser or import-linter (installs nothing; 2 = not measured,
never green); --prove --rule shows pass -> fail on a planted violation of
that rule -> pass. Verified against dependency-cruiser 18.5.0 and
import-linter 2.15; madge 8.0.0 left out (missed a planted .mjs cycle).
- K6 lib/verification/plan-scope-sweep.mjs: changed files no step names.
- /trekplan Phases 8-10: design part, bounded critic loop, the gate before
"execute", a plan-mode path through ExitPlanMode (not yet run end to end
inside plan mode). plan-critic 10 -> 12 dimensions; scope-guardian checks
the ceremony.
- Fixed on the way: plan-validator and plan-review-dedup used the
file://argv[1] main guard (no-op under a symlink or a space); CRLF plans
failed every manifest; the dedup dropped finding severity.
Borrowed, credited in place: obra/superpowers, Kiro (docs), addyosmani/
agent-skills, mattpocock/skills, GitHub Spec Kit, bashebr/ai-native-sdlc,
Boris Cherny's public threads.
Suite 1497 (1495 / 0 / 2), 86 new tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Operator decision 2026-10-06. Before, a WARN that existed only because a
reviewer or engine never reported (e.g. /security-review without an origin
remote) mapped to verdict-gate exit 0, so a review nobody performed in full
was green. Now `missing-reviewer:` / `unattributable-payload` in the JSON
block's allow_blocked_by is exit 2 (could not decide, never green); a WARN
for a MAJOR or a set-aside finding still passes with its reasons. A 1.1
review must carry allow_blocked_by.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An adversarial read of d88245c reproduced 15 problems; all fixed here, each
with a test that fails on the old code.
- verdict-gate: a greener verdict must be backed by the LAST fence being a
`json` block of the right shape, with no BLOCKER in the JSON (any case),
in `## Findings (BLOCKER)`, or a differing finding-id list; two verdict
lines and more than one file argument are exit 2. ({}, null, lowercase
"blocker", a trailing ```jsonc and a later json fence all gave exit 0.)
- engine-findings: a transcribed "narrower" key can no longer lower an
engine tier; absolute paths under --repo-root match the scope; "12" lines
parse; an empty scope is an error; out-of-scope engine findings are
unverified (`engine:out-of-scope`) and forbid ALLOW.
- recurrence: the catch-all engine keys and keys normalised from unknown
ones are never proposed; the same unfixed finding in two iterations is not
a recurrence; proposals are validated before they are offered; \r/U+2028
and damaged markers are refused; writes go through a symlinked CLAUDE.md.
Prose points --repo at the session directory, not the git toplevel.
- Workflow substrate: engines are counted from what arrived, and the
verdict's floor is recomputed from the coordinator's findings; now tested
by running the script with stubbed Workflow globals. Two literal NUL bytes
written as \u0000 (git saw the file as binary).
- Trimming keeps Pass 1's "Also flagged by" provenance; `## Declined to
Judge` and remediation steps are required from review_version 1.1 only
(N-1); the zero-diff review now carries the JSON block the gate needs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ordre 5 (wave 1, P11 + P5), review_version 1.1.
- K7: `verdict: ALLOW | WARN | BLOCK` is required in review.md (strict
error, soft warning for 1.0 reviews). New CLI lib/review/verdict-gate.mjs:
0 = ALLOW/WARN, 1 = BLOCK, 2 = could not decide (never green); a verdict
greener than the file's own BLOCKERs or JSON block is exit 2. Realpath
main guard (the old file:// guard exits 0 under a symlink - measured).
- No finding is struck for length: succinctness trims only MINOR/SUGGESTION
text (catalogue tier), keeps BLOCKER/MAJOR whole, counts trims; the
coordinator lists what it declined to judge.
- Claude Code's /code-review and /security-review run as the engine via the
Skill tool (never --fix/--comment/--post/ultra); lib/review/engine-findings.mjs
turns their findings into a reviewer payload; two catalogue keys
(CODE_REVIEW_DEFECT, SECURITY_REVIEW_VULNERABILITY), catalogue 12 -> 14.
An engine that did not run forbids ALLOW.
- Every BLOCKER is written as a numbered `### Step R<n>` with its source
finding (strict error REVIEW_MISSING_REMEDIATION_STEP).
- P5: lib/review/recurrence.mjs - a finding class in two review.md files
(current + git history, no new store) becomes one proposal, routed by the
catalogue's memory_route; written only between Voyage markers in
.claude/rules/voyage-review-lessons.md under a size budget, CLAUDE.md gets
a pointer only; the operator approves every line (Phase 8.7).
- Workflow substrate: catalogue enum in step; a missing reviewer or engine
forbids ALLOW there too (it returned ALLOW on zero findings before).
- Fixtures review-run-A/B said WARN over a BLOCKER; now BLOCK.
Borrowed and credited in code: Superpowers (declined-to-judge), ai-native-sdlc
(cap only nits; mistake/fix/check), Spec Kit (managed block; converge steps),
Pocock (retro routing), Kiro (verdict file + gate; evidence per lesson),
Cherny (engine -> brief control order; review -> memory; budget).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lib/review/gold-scorer.mjs held its (file, rule_key) separator as a literal NUL byte
inside a string literal. Git's binary heuristic therefore classed the whole file as
binary. Every diff of the scorer read "Binary files differ", `--numstat` printed "- -",
and plain grep matched nothing in it. The separator is now written as the escape
sequence '\x00', which is the same one-character string at runtime.
Proof that the change is behaviour-neutral:
- A snapshot script recorded every number the scorer produces, before and after:
- the committed run-perfect fixture through the coordinator contract;
- empty, half, spurious (keys containing spaces and colons), duplicated and empty-gold
runs;
- all 9 verdict pairs.
`diff` of the two outputs is empty (141 lines each).
- tests/lib/gold-scorer.test.mjs + tests/lib/gold-eval.test.mjs: 13/13 before and after.
- `git diff --no-index --numstat /dev/null <file>`: the new file reads "89 0" (text); the
old blob, as a control, reads "- -" (binary).
End-state gate D-06: open -> closed (defects 4 -> 3 of 7). Suite 1059 (1057/0/2), run
on a clean export of the index.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up defect in the reviewer accounting added by e2aec01, found by review
and confirmed by probe before fixing.
validateFindings only WARNS on a missing `reviewer` field, so a payload can
fail schema while carrying no name. ingest then records `reviewer: null`, and
runContract turned that null into the literal reviewer name "unnamed reviewer".
MEASURED before the fix:
runContract([{findings:[{file:'x.mjs',line:1,rule_key:'NOPE',severity:'MAJOR'}]}],
{expectedReviewers:['code-correctness-reviewer']})
-> missing_reviewers = ["unnamed reviewer", "code-correctness-reviewer"]
One failure, two entries, one of them an agent nobody launched. The
`reported.delete(s.reviewer)` line was also inert for that case, since a null
name was never in the set to begin with.
Fix: skipped payloads are split by whether they carry a name. Named ones go to
missing_reviewers as before; anonymous ones increment the new
`unattributable_payloads` count, which forbids ALLOW on its own - so stripping
a reviewer name from a payload cannot restore ALLOW, and the floor does not
depend on the caller passing expectedReviewers. `allow_blocked_by` reports the
two facts separately: `missing-reviewer:<name>` and `unattributable-payload (n)`.
The old behaviour never produced a false ALLOW - it failed in the safe
direction - but it named a reviewer that did not exist, which is the kind of
output an operator would chase.
Iron Law: two failing tests first (double entry; anonymous-payload-alone must
forbid ALLOW), then the fix.
Also verified in this pass, by temporarily adding a fake reason to
UNVERIFIED_REASONS: the prose-vocabulary pin does go red when a reason is
declared in the lib but missing from agents/review-coordinator.md. A pin that
cannot fail is not a pin.
Suite 1034 (1032/0/2) -> 1036 (1034/0/2), 0 failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
computeVerdict counted only the findings handed to it (reasoned.kept), so a
finding removed by Pass 2 or Pass 3, and a reviewer whose payload was thrown
away or never arrived, were arithmetically identical to a finding that never
existed. All three pushed the verdict toward ALLOW.
Measured before the fix (probes, 2026-09-01):
- a BLOCKER with a 101-character title -> ALLOW (Pass 2 succinctness)
- a payload with one ad-hoc rule_key is skipped WHOLE at ingest, taking a
valid BLOCKER sibling with it -> ALLOW
- a reviewer that never reported -> ALLOW
Pass 3's own no-citation / unknown-rule_key branches turned out unreachable
through runContract (validateFindings rejects those payloads first), so the
reachable exposure was Pass 2 plus the skipped/absent reviewer.
THE OPEN DESIGN DECISION, and why it went against the order's default.
The order proposed: indeterminate file-existence YES, plain succinctness NO
("a too-long finding is not an uncertain finding"). I kept the first and
overrode the second, on one principle:
A removal is `dropped` only when the test REFUTED the finding as a claim
about this codebase. Every other removal is `unverified`.
Succinctness and actionability read a `.length`. They never examine the claim,
so they cannot establish the finding is unreal - and dropping a BLOCKER for a
101-character title is precisely the fail-open shape being fixed. Three things
settled it:
1. Under the order's default the fix would have been almost inert. Pass 3's
drop branches are unreachable via runContract, so leaving Pass 2 out would
have left the only reachable finding-level exposure open.
2. Cost asymmetry, priced rather than asserted: the verdict is not a gate.
Handover 6 feeds `findings` filtered to BLOCKER+MAJOR into /trekplan
(commands/trekplan.md:218); `verdict` is optional metadata
(docs/HANDOVER-CONTRACTS.md:353). Nothing loops or re-plans on WARN. So a
false `unverified` costs WARN plus a printed reason; a false drop costs a
silent ALLOW over a live BLOCKER.
3. unknown-rule_key joins them for the same reason: an ad-hoc key is a real
defect wearing the wrong label, and v5.1.1 high-effort mode already KEEPS
those, normalised to PLAN_EXECUTE_DRIFT. Refuting them at normal effort
while keeping them at high effort would be incoherent.
no-citation stays a drop: a finding whose file is empty or whose line is
negative names no location, so it makes no checkable claim at all - the one
deterministic refutation, and what the Pass 3 prose already said it was.
Iron Law: tests/lib/coordinator-contract.test.mjs first, red (missing export +
the three measured ALLOWs), then production code. Two existing assertions were
updated AFTER implementation as contract changes, not to make the red pass.
A known-positive control pins that ALLOW is still reachable - without it,
"no ALLOW" is not a fail-closed contract, only a broken one.
lib/review/coordinator-contract.mjs
+ classifySuppression / REFUTING_REASONS / UNVERIFIED_REASONS - one
vocabulary owned by the lib, including the tokens only the LLM
coordinator emits (accuracy:refuted, file-existence:refuted/indeterminate),
so prose and lib cannot drift. Unclassified reasons default to unverified:
the default fails closed.
~ judgeFilter / reasonablenessFilter return {kept, dropped, unverified}
~ computeVerdict(findings, {unverified, missingReviewers}) -> + allow_blocked_by.
Never raises a verdict, only withholds ALLOW. Unverified findings are NOT
counted into a severity tier: their severity was never substantiated, and
counting it would be invention.
~ runContract(payloads, {expectedReviewers}) -> + unverified,
missing_reviewers, allow_blocked_by. `suppressed` stays the union of
dropped + unverified, so existing consumers (gold-eval) keep their meaning.
agents/review-coordinator.md - Pass 2/3 tables gain a fate column, new
"Suppression is two-valued" section, Pass 4 threshold table gains the two
fail-closed rows, Executive Summary must state a withheld ALLOW, Suppressed
Findings tags each line [dropped]/[unverified]. Pass 3's unknown-rule_key
bullet explicitly says high-effort does not reach that branch, so the same
input never has two documented fates.
commands/trekreview.md - Phase 5 "Reviewer accounting": the expected set is
written down before the spawn, a silent reviewer gets one re-ask and then
STOP. That extends the pattern already in the file (schema failure -> 2
bounded re-asks -> "do not feed unvalidated findings to the coordinator") to
the other two ways a reviewer goes missing, rather than softening it to WARN.
The lib's missing_reviewers stays as belt-and-braces for direct callers.
docs/agent-return-channel-defect.md - the "inferred, not observed" caveat on
the unnamed arm above 66 lines is struck: akashic-intelligence S27
(f168630) measured 2/2 unnamed agents returning against a 4370-line plan,
30449 B and 10989 B, both valid JSON. Recorded with akashic's own two
caveats intact - the measurer owns the finding, and byte-identity between
the returned string and the file on disk was not proven. The separate S25
named-arm figures are left standing; these are two measurements, not a
correction of one by the other.
No release, no version bump, no tag, no catalogue ref, no Workflow port.
Suite 1025 (1023/0/2) -> 1034 (1032/0/2), 0 failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scores committed agent-run fixtures against the golden corpus at
(file, rule_key) granularity, building on the deterministic coordinator
contract (4a). Offline: committed reviewer payloads, no live agent spawn,
no LLM, no network (the LLM-in-the-loop grading is the separate 4c tier).
- lib/review/gold-scorer.mjs: scoreFindings (precision/recall/f1 at
(file,rule_key) granularity, line+severity ignored) + scoreVerdict; pure,
with documented vacuous-set conventions.
- tests/fixtures/bakeoff-rich/runs/run-perfect.json: committed run that
reproduces all 5 seeded gold findings through runContract.
- tests/lib/gold-eval.test.mjs: the scoring RUN (precision/recall/f1 = 1.0,
verdict == expected_verdict BLOCK, nothing suppressed/skipped).
- lib/util/test-census.mjs: third census category (goldEval) — a scoring run
is neither behavior coverage nor a doc-pin; honest-count invariant now 3-way.
- docs/eval-corpus/README.md: 4b moved from Future hardening to implemented.
Suite 809 -> 822 (820/0/2). gold-scorer covers TP+FP+FN+degenerate paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJQYC5vpkJWxndS55vQQZ6
Phase 9's dedup hand-off was broken on two layers, both surfaced by the S22
dogfood: (a) plan-critic + scope-guardian (Read/Glob/Grep, no Write) were told
to write /tmp/*-out.json the dedup helper reads — they cannot; (b) even with
Write, their Output format emits markdown, not the helper's
{agent,findings:[{file,line,rule_key,text}]} schema. readJsonOrNull then
swallowed the absent files -> a silent empty merge that discarded every finding.
Fix (operator-chosen A'): keep the reviewers read-only; make the hand-off run.
- plan-review-dedup.mjs gains a --stdin mode reading {plan_critic,scope_guardian};
malformed stdin exits non-zero so a broken hand-off surfaces loudly instead of
collapsing into a silent empty merge. File mode + its tests are untouched.
- plan-critic.md + scope-guardian.md now emit a trailing machine-readable `json`
findings block (the inline hand-off; no Write tool needed).
- trekplan.md + planning-orchestrator.md Phase 9 rewritten in lockstep: extract
both blocks, pipe via heredoc into --stdin. No temp files, portable, no
pathguard dependency.
TDD: malformed-stdin test failed first (CLI ignored stdin -> exit 0 = the bug),
green after impl. New S23 doc-pin asserts both docs use --stdin (not the dead
/tmp paths) and both agents declare the json block. Suite 724 (722/2/0); live
HEAD baseline was 720, not the stale 705 STATE carried forward.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB
Build the prose-vs-Workflow bake-off machinery for /trekreview Phase 5-6 and
run a 1-run/arm smoke to de-risk before the full measurement (operator posture:
build + smoke, then pause for go/no-go on the full >=3-runs/arm run).
New:
- lib/review/fidelity-diff.mjs (+ tests) — the PRIMARY metric: parse two
review.md (or two structured arm outputs) and compare verdict + jaccard over
(file,line,rule_key)-IDs + per-finding severity/rule_key. Reuses jaccard +
frontmatter + NW1 findings-schema + finding-id. fidelityDiffStructured avoids
rendering review.md per run.
- scripts/trekreview-armB.workflow.mjs — Arm B: Phase 5-6 as a Workflow
(parallel([conformance, correctness]) schema-forced -> JS dedup-by-triplet ->
agent(review-coordinator) verdict schema). Path-based input via args (reviewers
carry Read). Inlines dedup + the 12-key rule_key enum (scripts have no imports).
- tests/fixtures/bakeoff/ — committable fixture: real diff of b149538 (NW1) +
brief reconstructed from plan S9. Both arms review the same pinned input.
- docs/T2-bakeoff-results.md — smoke results + verdict + go/no-go recommendation.
Smoke result: SMOKE PASS. Arm B runs the full pipeline (3 agents) with ZERO
classifier interference; fidelity EQUIVALENT to Arm A at the verdict level
(both ALLOW; jaccard 1.0). Caveat: the clean TDD'd fixture yielded ~0 findings,
so finding-SET fidelity was not stressed (only verdict fidelity proven). A
reviewer-level divergence appeared (Arm B raised 1 raw finding, coordinator
filtered it; Arm A raised 0) — to be quantified in the full run on a
richer-finding-surface fixture. NOT the T2 §5 POSITIVE/NEGATIVE verdict.
Suite 647 -> 662 (660 pass / 2 skip / 0 fail; +15 fidelity-diff). claude plugin
validate clean (known root-CLAUDE.md warning only). Plan: docs/W1-narrow-wins-plan.md S10.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB
Codify the /trekreview Phase 5 reviewer JSON contract as a validated schema so
main validates each reviewer's output instead of merely JSON.parse-ing it, and
re-asks on schema failure (not just parse failure). Retires the fragility at
trekreview.md:202-204. No Workflow dependency (S8 tier 1, ships regardless).
New lib/review/findings-schema.mjs (3-layer Content -> Raw-text -> CLI shim,
reusing result.mjs error-shape + rule-catalogue RULE_KEYS/SEVERITY_VALUES):
- validateFindings(payload): hard errors on load-bearing fields the dedup
triplet + verdict depend on — file, rule_key (in catalogue), severity (enum),
line (integer >= 0); accumulates all errors; per-finding location.
- extractFindingsBlock(text): last fenced ```json block (the :202 contract).
- validateReviewerOutput(text): extract + parse + schema, unifying parse and
schema failures under one bounded re-ask path.
Stable codes: FINDINGS_NOT_OBJECT/_NOT_ARRAY/_NO_JSON_BLOCK/_PARSE_ERROR (top),
FINDING_MISSING_FILE/_MISSING_RULE_KEY/_UNKNOWN_RULE_KEY/_BAD_SEVERITY/_BAD_LINE
(per-finding). Descriptive fields + unknown keys tolerated (forward-compat).
Phase 5 prose: replace "parse last json block; on parse error re-emit" with
"validate against findings-schema; on failure re-ask conforming JSON, bounded
N=2; never feed unvalidated findings to the coordinator".
TDD: 27 failing tests first, then minimal code to pass. Suite 606 -> 647
(645 pass / 2 skip / 0 fail). claude plugin validate clean (only the known
root-CLAUDE.md warning). Plan: docs/W1-narrow-wins-plan.md S9.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB