From 7cfce2b996b5e94c67d1e9804045535f63413fcd Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Fri, 19 Jun 2026 21:59:31 +0200 Subject: [PATCH] =?UTF-8?q?fix(voyage):=20S26=20=E2=80=94=20parse=20plan?= =?UTF-8?q?=5Fversion=20prose=20form=20(S22=20defect=20#2/#3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plan template (templates/plan-template.md:21) emits plan_version ONLY in the prose blockquote metadata line: > Generated by trekplan v{version} on {YYYY-MM-DD} — `plan_version: 1.7` It carries no frontmatter plan_version (the frontmatter block is trekreview-only and holds source_findings). But PLAN_VERSION_REGEX in lib/parsers/plan-schema.mjs was `^`-anchored (/m), matching ONLY line-start (frontmatter). The backtick- wrapped prose form never matched → extractPlanVersion returned null → every real generated plan got a spurious PLAN_NO_VERSION warning from plan-validator. Not caught earlier because no test ran the parser against the actual template; the synthetic fixtures all carry frontmatter plan_version and parse fine. Internal source contradiction surfaced: the parser comment documents intent "frontmatter OR doc body", planning-orchestrator.md:228 + the template place it in the prose line, while HANDOVER-CONTRACTS.md calls it a frontmatter field. Operator-chosen fix (parser, not template): relax the regex to `/(?:^|`)plan_version:.../m` so it matches line-start (frontmatter) OR the backtick-prefixed prose form. Honors the parser's own documented contract, fixes all existing + future plans, changes no plan output, touches one code file — vs the template-fix which would alter every plan's shape, contradict planning-orchestrator.md, and force a 3-file doc reconciliation (scope creep). TDD (red first): added prose-form + canonical-template regression pins in plan-schema.test.mjs and a no-PLAN_NO_VERSION pin in plan-validator.test.mjs; all 3 red on the `^`-anchored regex, green after the relax. The template pin ties the parser directly to the real generated artifact. S22 defect closed in docs/S22-happy-path-dogfood.md (§Pipeline defects + log). Tests: 728 (726 pass / 2 skip / 0 fail; live `node --test`, = 725 baseline +3). plugin validate passes (1 accepted CLAUDE.md-at-root warning). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB --- docs/S22-happy-path-dogfood.md | 4 ++-- lib/parsers/plan-schema.mjs | 5 ++++- tests/lib/plan-schema.test.mjs | 17 +++++++++++++++++ tests/validators/plan-validator.test.mjs | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/docs/S22-happy-path-dogfood.md b/docs/S22-happy-path-dogfood.md index 8b9f5d4..cd76179 100644 --- a/docs/S22-happy-path-dogfood.md +++ b/docs/S22-happy-path-dogfood.md @@ -104,7 +104,7 @@ Execute succeeded (15/15, suite green, CLI works). **One finding only execute co ### Pipeline defects the dogfood surfaced (the bonus the S14 audit could not get — it never ran the pipeline) 1. **`/trekplan` Phase 9 is broken as documented.** It instructs plan-critic + scope-guardian to "Write structured JSON output to `/tmp/…out.json`", then runs `plan-review-dedup.mjs` on those files. **Both agents' frontmatter grants only `Read/Glob/Grep` — no `Write`/`Bash`** — so the files are never created and the dedup step cannot run. Both agents fell back to returning JSON inline. **Severity: MAJOR** (a documented, wired step that cannot execute). Fix options: grant the reviewers `Write`, or have the orchestrator persist the returned JSON before calling the dedup helper. 2. **Oracle-into-swarm contamination** (see caveat) — a real trap for dogfooding planning tools on their own repo. -3. **`plan_version` not parsed.** The plan template emits `plan_version: 1.7` as prose in the "Generated by" line; `plan-validator` then warns `PLAN_NO_VERSION`. Minor template/validator mismatch — the validator looks for a frontmatter/parseable field the template doesn't emit. +3. **`plan_version` not parsed.** ~~The plan template emits `plan_version: 1.7` as prose in the "Generated by" line; `plan-validator` then warns `PLAN_NO_VERSION`. Minor template/validator mismatch — the validator looks for a frontmatter/parseable field the template doesn't emit.~~ **RESOLVED (S26).** `PLAN_VERSION_REGEX` (`lib/parsers/plan-schema.mjs`) was `^`-anchored and only matched frontmatter; relaxed to `/(?:^|`)plan_version:.../m` so it honors the parser's documented "frontmatter or doc body" contract and parses the backtick-wrapped prose form the template emits. Regression-pinned by running `extractPlanVersion` against the actual `templates/plan-template.md`. 4. **Version skew.** The *installed* plugin (skill the operator invokes) is cached at **v5.1.1**; the repo under development is **v5.5.0**. The dogfood ran the v5.1.1 command text against v5.5.0 `lib/`. Harmless here (the phases are stable across the bump) but worth noting: operators dogfooding the installed plugin are not testing the dev tree. ### Honest limitations @@ -120,7 +120,7 @@ The happy path **works** and produces high-quality, executable plans — and the | Claim | How verified | |-------|--------------| | Brief validates clean | `node lib/validators/brief-validator.mjs --json` → `{valid:true,errors:[],warnings:[]}` | -| Plan passes schema | `node lib/validators/plan-validator.mjs --strict --json` → `valid:true, 4 steps, 0 errors` (1 soft `PLAN_NO_VERSION` warning = defect #3) | +| Plan passes schema | `node lib/validators/plan-validator.mjs --strict --json` → `valid:true, 4 steps, 0 errors` (1 soft `PLAN_NO_VERSION` warning = defect #3, **resolved in S26**) | | 15 new tests pass; suite green | `node --test` in worktree → new file 15/15; full suite 720 (718/2/0) = 705 baseline +15 | | CLI works on live dir | `node lib/validators/project-doctor.mjs .claude/projects/2026-06-19-voyage-doctor` → `PASS`, exit 0; `--json` → valid JSON | | Execute isolated; main untouched | `git worktree remove` → `git status` clean, HEAD `aeee4c6`, `lib/validators/project-doctor.mjs` absent from main | diff --git a/lib/parsers/plan-schema.mjs b/lib/parsers/plan-schema.mjs index a6812a1..cf73cb9 100644 --- a/lib/parsers/plan-schema.mjs +++ b/lib/parsers/plan-schema.mjs @@ -13,7 +13,10 @@ export const STEP_HEADING_REGEX = /^### Step (\d+):\s+(.+?)\s*$/m; export const STEP_HEADING_GLOBAL = /^### Step (\d+):\s+(.+?)\s*$/gm; export const FORBIDDEN_HEADING_REGEX = /^(?:##|###) (?:Fase|Phase|Stage|Steg) \d+/m; export const FORBIDDEN_HEADING_GLOBAL = /^(?:##|###) (?:Fase|Phase|Stage|Steg) \d+/gm; -export const PLAN_VERSION_REGEX = /^plan_version:\s*['"]?([\d.]+)['"]?/m; +// Matches plan_version in either location the schema allows: at line start +// (frontmatter) or backtick-wrapped inside the prose "Generated by" metadata +// line the plan template emits (`> Generated by ... — `plan_version: 1.7``). +export const PLAN_VERSION_REGEX = /(?:^|`)plan_version:\s*['"]?([\d.]+)['"]?/m; /** * Find all step heading positions in plan text. diff --git a/tests/lib/plan-schema.test.mjs b/tests/lib/plan-schema.test.mjs index 6a14f25..6220448 100644 --- a/tests/lib/plan-schema.test.mjs +++ b/tests/lib/plan-schema.test.mjs @@ -1,5 +1,6 @@ import { test } from 'node:test'; import { strict as assert } from 'node:assert'; +import { readFileSync } from 'node:fs'; import { findSteps, findForbiddenHeadings, @@ -135,3 +136,19 @@ test('extractPlanVersion — from frontmatter', () => { test('extractPlanVersion — null when absent', () => { assert.equal(extractPlanVersion('foo: bar\n'), null); }); + +test('extractPlanVersion — from prose "Generated by" metadata line (defect S26)', () => { + // The plan template emits plan_version only in the prose blockquote line + // (`> Generated by ... — `plan_version: 1.7``), not as frontmatter. + // extractPlanVersion must honor its documented "frontmatter or doc body" + // contract and parse the backtick-wrapped prose form. + const prose = '> Generated by trekplan v4.1.0 on 2026-05-10 — `plan_version: 1.7`\n'; + assert.equal(extractPlanVersion(prose), '1.7'); +}); + +test('extractPlanVersion — recognizes the canonical plan template (regression pin S26)', () => { + // The real generated artifact: no test exercised the parser against the + // actual template, which is why defect S26 went unnoticed. + const tpl = readFileSync(new URL('../../templates/plan-template.md', import.meta.url), 'utf-8'); + assert.equal(extractPlanVersion(tpl), '1.7'); +}); diff --git a/tests/validators/plan-validator.test.mjs b/tests/validators/plan-validator.test.mjs index a5569a6..a4e505d 100644 --- a/tests/validators/plan-validator.test.mjs +++ b/tests/validators/plan-validator.test.mjs @@ -97,3 +97,17 @@ test('validatePlan — older version triggers warning', () => { const r = validatePlanContent(old, { strict: true }); assert.ok(r.warnings.find(w => w.code === 'PLAN_VERSION_MISMATCH')); }); + +test('validatePlan — prose plan_version line parses, no PLAN_NO_VERSION (defect S26)', () => { + // A plan shaped like the real template: plan_version lives only in the + // prose "Generated by" blockquote, not frontmatter. Must still parse. + const prosePlan = VALID_PLAN + .replace(/---\nplan_version: "1\.7"\n---\n\n/, '') + .replace( + '# Plan', + '# Plan\n\n> Generated by trekplan v4.1.0 on 2026-05-10 — `plan_version: 1.7`', + ); + const r = validatePlanContent(prosePlan, { strict: true }); + assert.equal(r.parsed.planVersion, '1.7'); + assert.equal(r.warnings.find(w => w.code === 'PLAN_NO_VERSION'), undefined); +});