From 46ba9ed4648b569ed1fb7f860bc10b5ace106cf4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 21 Jul 2026 08:12:52 -0400 Subject: [PATCH] fix(#2444): branch plan-structure validation on task type=checkpoint:* (#2473) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2444): failing-first regression for checkpoint:* plan-structure validation Add acceptance-criteria tests covering the three canonical checkpoint task types (human-verify, decision, human-action) plus an unknown-subtype forward-compat case. Each canonical type must pass verify plan-structure when it carries its type-specific required fields (per gsd-core/references/checkpoints.md), and must be flagged when those fields are missing. Non-checkpoint tasks keep the existing /// requirements unchanged (AC3 regression guards). The existing 'errors when checkpoint task but autonomous is true' fixture is updated to use the canonical checkpoint:human-verify triple (//) so it does not collide with the new per-type validator; the assertion (autonomous is not false) is unchanged. * fix(#2444): branch plan-structure validation on task type=checkpoint:* cmdVerifyPlanStructure unconditionally required /// on every task, so every checkpoint:* task — which uses the checkpoint convention's type-specific fields instead — was reported as a structural error. Checkpoint-heavy phases produced walls of false findings. The fix introduces two pure helpers in verify.cts: - extractPlanTaskInfos(content): single ReDoS-safe pass over ... blocks that captures BOTH the opening-tag attribute string (so the type= selector is not lost, as it is with extractTaggedBlocks) and the body, returning a typed PlanTaskInfo. - validatePlanTaskStructure(task): branches on the task's type. checkpoint:human-verify requires // (the canonical triple). checkpoint:decision requires //. checkpoint:human-action requires // /. Unknown checkpoint:* subtypes require only the universal (forward-compat). All other types keep the historical /// requirements unchanged. Canonical reference: gsd-core/references/checkpoints.md. Per-type field sets validated against the documented templates in agents/gsd-planner.md and gsd-core/templates/phase-prompt.md. * fix(#2444): re-resolve body-parser to 2.3.0 in lockfile (GHSA-v422-hmwv-36x6) GHSA-v422-hmwv-36x6 (body-parser DoS via invalid limit value, low severity, published 2026-07-20T23:23:26Z) made tests/npm-integrity-gate.test.cjs (#3588: root workspace production tree has no advisories) fail any subsequent npm audit --omit=dev. The advisory affects body-parser >=2.0.0 <2.3.0 pulled transitively via @anthropic-ai/claude-agent-sdk -> @modelcontextprotocol/sdk -> express -> body-parser@2.2.2. express@5.2.1 already declares body-parser as ^2.2.1, so 2.3.0 is a valid re-resolution within express's own compatibility range — no override needed. Regenerated the lockfile via 'npm audit fix --omit=dev' which re-resolves transitive deps within their declared ranges; package.json is unchanged. Verified: npm audit --omit=dev reports 0/0/0/0/0 advisories; body-parser now reads as 2.3.0 in 'npm ls body-parser --omit=dev'. * test(#2444): close review gap-closure tests + harden type-attr charset Orthogonal review (code-review + security-review subagents) returned APPROVE on Standards and Spec. Per the playbook's zero-tolerance policy, address every Low finding: Spec gap-closures: - AC3 verbatim: add explicit and regression tests for non-checkpoint tasks (pre-existing tests only covered and ). - AC2: add checkpoint:decision missing , checkpoint:human-action missing , checkpoint:human-action missing cases (the implementation enforces all of these; only one missing-field case per type was previously tested). - Remove the duplicate 'returns error for nonexistent file' test that leaked into the new describe block from the insertion edit. Security hardening (Low-sev, defense-in-depth): - Tighten the task type= attribute extractor in src/verify.cts from [^"'>\s]+ to [\w:-]+ so a hostile type= attribute cannot carry markup fragments (e.g. type=evil ( ) &) in the surfaced type field. * docs(changeset): add Fixed fragments for #2444 PR Two fragments: - sturdy-jays-tumble.md: the verify plan-structure checkpoint fix - witty-badgers-hum.md: the body-parser 2.3.0 re-resolution PR number backfilled to 0 placeholder per CLAUDE.md 'PR Number Handling'; will backfill to the real PR number immediately after gh pr create returns. * docs(changeset): backfill PR number to 2473 Per CLAUDE.md 'PR Number Handling': backfill the placeholder pr:0 with the real PR number returned by gh pr create. --- .changeset/sturdy-jays-tumble.md | 5 + .changeset/witty-badgers-hum.md | 5 + package-lock.json | 59 +++-- src/verify.cts | 184 +++++++++++++-- tests/verify.test.cjs | 387 ++++++++++++++++++++++++++++++- 5 files changed, 606 insertions(+), 34 deletions(-) create mode 100644 .changeset/sturdy-jays-tumble.md create mode 100644 .changeset/witty-badgers-hum.md diff --git a/.changeset/sturdy-jays-tumble.md b/.changeset/sturdy-jays-tumble.md new file mode 100644 index 000000000..68b57ca47 --- /dev/null +++ b/.changeset/sturdy-jays-tumble.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2473 +--- +**`verify plan-structure` no longer false-flags checkpoint tasks for missing ``/``/``** — every `` was reported as a structural error because the verifier unconditionally required the auto-task fields. It now branches on the task's `type` attribute: `checkpoint:human-verify` requires its canonical triple (``/``/``), `checkpoint:decision` requires ``/``/``, `checkpoint:human-action` requires ``/``/``/`` (per `gsd-core/references/checkpoints.md`), and unknown `checkpoint:*` subtypes require only the universal ``. Non-checkpoint tasks keep the historical ``/``/``/`` requirements unchanged. diff --git a/.changeset/witty-badgers-hum.md b/.changeset/witty-badgers-hum.md new file mode 100644 index 000000000..7b8d9f05d --- /dev/null +++ b/.changeset/witty-badgers-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2473 +--- +**Dependency tree no longer carries a known body-parser advisory** — GHSA-v422-hmwv-36x6 (low-severity DoS via invalid `limit` value, published 2026-07-20) in `body-parser@2.2.2` was pulled transitively via `@anthropic-ai/claude-agent-sdk` → `@modelcontextprotocol/sdk` → `express` and surfaced by `npm audit --omit=dev`. Re-resolved `body-parser` to 2.3.0 in `package-lock.json` within `express`'s already-declared `^2.2.1` range; no `overrides` block needed, `package.json` is unchanged. diff --git a/package-lock.json b/package-lock.json index 2020a3ef2..98e5252cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,7 @@ "bin": { "gsd_run": "gsd-core/bin/gsd_run", "gsd-core": "bin/install.js", + "gsd-mcp-server": "bin/gsd-mcp-server.js", "gsd-tools": "gsd-core/bin/gsd-tools.cjs" }, "devDependencies": { @@ -2160,20 +2161,20 @@ } }, "node_modules/body-parser": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", - "integrity": "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.3.0.tgz", + "integrity": "sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==", "license": "MIT", "dependencies": { "bytes": "^3.1.2", - "content-type": "^1.0.5", + "content-type": "^2.0.0", "debug": "^4.4.3", - "http-errors": "^2.0.0", - "iconv-lite": "^0.7.0", + "http-errors": "^2.0.1", + "iconv-lite": "^0.7.2", "on-finished": "^2.4.1", - "qs": "^6.14.1", - "raw-body": "^3.0.1", - "type-is": "^2.0.1" + "qs": "^6.15.2", + "raw-body": "^3.0.2", + "type-is": "^2.1.0" }, "engines": { "node": ">=18" @@ -2183,6 +2184,19 @@ "url": "https://opencollective.com/express" } }, + "node_modules/body-parser/node_modules/content-type": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.0.0.tgz", + "integrity": "sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/brace-expansion": { "version": "5.0.6", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", @@ -4981,17 +4995,34 @@ } }, "node_modules/type-is": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.0.1.tgz", - "integrity": "sha512-OZs6gsjF4vMp32qrCbiVSkrFmXtG/AZhY3t0iAMrMBiAZyV9oALtXO8hsrHbMXF9x6L3grlFuwW2oAz7cav+Gw==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.1.0.tgz", + "integrity": "sha512-faYHw0anBbc/kWF3zFTEnxSFOAGUX9GFbOBthvDdLsIlEoWOFOtS0zgCiQYwIskL9iGXZL3kAXD8OoZ4GmMATA==", "license": "MIT", "dependencies": { - "content-type": "^1.0.5", + "content-type": "^2.0.0", "media-typer": "^1.1.0", "mime-types": "^3.0.0" }, "engines": { - "node": ">= 0.6" + "node": ">= 18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/type-is/node_modules/content-type": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.0.0.tgz", + "integrity": "sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" } }, "node_modules/typed-inject": { diff --git a/src/verify.cts b/src/verify.cts index f346f385d..664313c90 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -557,6 +557,162 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[] return { warnings, valid: true as const }; } +// ─── Plan-task structure validation (#2444) ────────────────────────────────── + +/** + * Per-task structural information extracted from a PLAN.md `` block. + * Captures the task's `type` attribute (so `checkpoint:*` tasks validate + * against their type-specific canonical field set per + * `gsd-core/references/checkpoints.md`) plus presence flags for every tag the + * validator cares about. Pure data — no I/O. + */ +interface PlanTaskInfo { + name: string; + /** Lowercased `type` attribute value, or '' when the opening tag has no type. */ + type: string; + hasName: boolean; + // auto-task fields + hasFiles: boolean; + hasAction: boolean; + hasVerify: boolean; + hasDone: boolean; + // checkpoint:human-verify fields + hasWhatBuilt: boolean; + hasHowToVerify: boolean; + // checkpoint:decision fields + hasDecision: boolean; + hasOptions: boolean; + // checkpoint:human-action fields + hasInstructions: boolean; + hasVerification: boolean; + // cross-checkpoint common + hasResumeSignal: boolean; +} + +/** + * Single pass over `…` blocks. The body pattern is + * ReDoS-safe stop-at-next-open (mirrors `taggedBlockPattern` in + * markdown-sectionizer.cts): bounded attributes (`[^>]{0,1000}`) and a body + * boundary that terminates at the NEXT `]` opening, so a document + * full of unclosed `` openings scans linearly. Captures both the + * attribute string (group 1, so the `type=` selector is not lost the way it is + * with `extractTaggedBlocks`) and the body (group 2). + */ +const PLAN_TASK_BLOCK_RE = /]{0,1000})?>((?:(?!])[\s\S])*?)<\/task>/g; + +/** + * Extract one `PlanTaskInfo` per `…` block in `content`. + * + * Why a dedicated regex instead of `extractTaggedBlocks('task', true)`: + * `extractTaggedBlocks` discards the opening tag, so the task's `type=` + * attribute (which selects the validation branch) is lost. This helper + * captures both the attribute string and the body in one pass, then reuses + * `extractTaggedBlocks` on the body for sub-element extraction. + */ +function extractPlanTaskInfos(content: string): PlanTaskInfo[] { + const infos: PlanTaskInfo[] = []; + if (typeof content !== 'string' || content.length === 0) return infos; + + PLAN_TASK_BLOCK_RE.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = PLAN_TASK_BLOCK_RE.exec(content)) !== null) { + const attrs = match[1] ?? ''; + const body = match[2] ?? ''; + + const typeMatch = attrs.match(/\btype\s*=\s*["']?([\w:-]+)/i); + const type = typeMatch ? typeMatch[1].toLowerCase() : ''; + + const nameArr = extractTaggedBlocks(body, 'name'); + const hasName = nameArr.length > 0; + const name = hasName ? nameArr[0].trim() : ''; + + infos.push({ + name, + type, + hasName, + hasFiles: //.test(body), + hasAction: //.test(body), + hasVerify: //.test(body), + hasDone: //.test(body), + hasWhatBuilt: //.test(body), + hasHowToVerify: //.test(body), + hasDecision: //.test(body), + hasOptions: //.test(body), + hasInstructions: //.test(body), + hasVerification: //.test(body), + hasResumeSignal: //.test(body), + }); + + // Guard against zero-length matches looping forever. + if (match.index === PLAN_TASK_BLOCK_RE.lastIndex) { + PLAN_TASK_BLOCK_RE.lastIndex++; + } + } + return infos; +} + +function isCheckpointType(type: string): boolean { + return type.startsWith('checkpoint:'); +} + +/** + * Validate one plan task's structure against its type-specific canonical field + * set (per `gsd-core/references/checkpoints.md`): + * - `checkpoint:human-verify` requires `` / `` / + * `` (the "checkpoint triple"). + * - `checkpoint:decision` requires `` / `` / + * ``. + * - `checkpoint:human-action` requires `` / `` / + * `` / ``. + * - Unknown `checkpoint:*` subtypes require only the universal + * `` (forward-compat — newer checkpoint types registered + * in the reference don't need a verifier change to pass structure + * validation). + * - All other types (`auto`, `tracer`, `manual`, bare ``, …) keep the + * historical `` / `` / `` / `` requirements. + */ +function validatePlanTaskStructure(task: PlanTaskInfo): { errors: string[]; warnings: string[] } { + const errors: string[] = []; + const warnings: string[] = []; + const taskName = task.hasName ? task.name : 'unnamed'; + + if (!task.hasName) { + errors.push('Task missing element'); + } + + if (isCheckpointType(task.type)) { + if (!task.hasResumeSignal) { + errors.push(`Task '${taskName}' missing `); + } + switch (task.type) { + case 'checkpoint:human-verify': + if (!task.hasWhatBuilt) errors.push(`Task '${taskName}' missing `); + if (!task.hasHowToVerify) errors.push(`Task '${taskName}' missing `); + break; + case 'checkpoint:decision': + if (!task.hasDecision) errors.push(`Task '${taskName}' missing `); + if (!task.hasOptions) errors.push(`Task '${taskName}' missing `); + break; + case 'checkpoint:human-action': + if (!task.hasAction) errors.push(`Task '${taskName}' missing `); + if (!task.hasInstructions) errors.push(`Task '${taskName}' missing `); + if (!task.hasVerification) errors.push(`Task '${taskName}' missing `); + break; + default: + // Unknown checkpoint:* subtype: is the only universal + // requirement (forward-compat). + break; + } + } else { + if (!task.hasAction) errors.push(`Task '${taskName}' missing `); + if (!task.hasVerify) warnings.push(`Task '${taskName}' missing `); + if (!task.hasDone) warnings.push(`Task '${taskName}' missing `); + if (!task.hasFiles) warnings.push(`Task '${taskName}' missing `); + } + + return { errors, warnings }; +} + function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): void { if (!filePath) { error('file path required'); @@ -577,22 +733,20 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo if (fm[field] === undefined) errors.push(`Missing required frontmatter field: ${field}`); } + const extractedTasks = extractPlanTaskInfos(content); const tasks: Record[] = []; - for (const taskContent of extractTaggedBlocks(content, 'task', true)) { - const nameArr = extractTaggedBlocks(taskContent, 'name'); - const taskName = nameArr.length ? nameArr[0].trim() : 'unnamed'; - const hasFiles = //.test(taskContent); - const hasAction = //.test(taskContent); - const hasVerify = //.test(taskContent); - const hasDone = //.test(taskContent); - - if (nameArr.length === 0) errors.push('Task missing element'); - if (!hasAction) errors.push(`Task '${taskName}' missing `); - if (!hasVerify) warnings.push(`Task '${taskName}' missing `); - if (!hasDone) warnings.push(`Task '${taskName}' missing `); - if (!hasFiles) warnings.push(`Task '${taskName}' missing `); - - tasks.push({ name: taskName, hasFiles, hasAction, hasVerify, hasDone }); + for (const task of extractedTasks) { + const verdict = validatePlanTaskStructure(task); + errors.push(...verdict.errors); + warnings.push(...verdict.warnings); + tasks.push({ + name: task.hasName ? task.name : 'unnamed', + type: task.type, + hasFiles: task.hasFiles, + hasAction: task.hasAction, + hasVerify: task.hasVerify, + hasDone: task.hasDone, + }); } if (tasks.length === 0) warnings.push('No elements found'); diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index 712245fee..be6a974d1 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -263,12 +263,11 @@ describe('verify plan-structure command', () => { ' echo ok', ' Done', '', - '', + '', ' Task 2: Verify UI', - ' some/file.ts', - ' Check the UI', - ' Visit the app', - ' UI verified', + ' UI at localhost:3000', + ' Visit the app', + ' Type "approved"', '', '', ].join('\n'); @@ -299,6 +298,384 @@ describe('verify plan-structure command', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// verify plan-structure — checkpoint task types (#2444) +// A checkpoint:* task uses type-specific required fields (per +// gsd-core/references/checkpoints.md), NOT the auto-task //. +// ───────────────────────────────────────────────────────────────────────────── + +describe('verify plan-structure — checkpoint task types (#2444)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-test'), { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // Helper: wrap a task body in a complete valid PLAN.md scaffold. + function planWithTask(taskBody, { autonomous = 'false' } = {}) { + return [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [some/file.ts]', + `autonomous: ${autonomous}`, + 'must_haves:', + ' truths:', + ' - "something"', + '---', + '', + '', + taskBody, + '', + ].join('\n'); + } + + function runVerify(planContent) { + const planPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-PLAN.md'); + fs.writeFileSync(planPath, planContent); + const result = runGsdTools('verify plan-structure .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + return JSON.parse(result.output); + } + + // ── AC1: canonical checkpoint tasks pass with zero findings ──────────────── + + test('checkpoint:human-verify with canonical triple passes (AC1)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: verify UI', + ' Dashboard at localhost:3000', + ' Visit /dashboard, check layout', + ' Type "approved" or describe issues', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.errors, [], `expected no errors; got: ${JSON.stringify(output.errors)}`); + assert.strictEqual(output.task_count, 1, 'should count the checkpoint task'); + }); + + test('checkpoint:decision with canonical fields passes (AC1)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: pick auth provider', + ' Select authentication provider', + ' Need user authentication.', + ' ', + ' ', + ' ', + ' ', + ' Select: supabase or clerk', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.errors, [], `expected no errors; got: ${JSON.stringify(output.errors)}`); + }); + + test('checkpoint:human-action with canonical fields passes (AC1)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: complete email verification', + ' Click the verification link in your inbox', + ' I created the account; check your email.', + ' API key works via curl', + ' Type "done" when email verified', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.errors, [], `expected no errors; got: ${JSON.stringify(output.errors)}`); + }); + + test('unknown checkpoint:* subtype passes with just (forward-compat)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: future', + ' Type "ok"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.errors, [], `expected no errors; got: ${JSON.stringify(output.errors)}`); + }); + + test('mixed plan: auto task + checkpoint:human-verify task passes (AC1 realistic)', () => { + const output = runVerify(planWithTask([ + '', + ' Task 1: build dashboard', + ' src/dashboard.ts', + ' Scaffold the dashboard', + ' npm test', + ' Dashboard renders', + '', + '', + ' Checkpoint: visual review', + ' Dashboard at localhost:3000', + ' Visit /dashboard, check responsive layout', + ' Type "approved"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.errors, [], `expected no errors; got: ${JSON.stringify(output.errors)}`); + assert.strictEqual(output.task_count, 2, 'should count both tasks'); + }); + + // ── AC2: checkpoint tasks missing required fields are still flagged ──────── + + test('checkpoint:human-verify missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: verify UI', + ' UI at localhost:3000', + ' Type "approved"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:human-verify missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: verify UI', + ' Visit /dashboard', + ' Type "approved"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:decision missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: pick', + ' Select provider', + ' Select: a or b', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:human-action missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: act', + ' Do the thing', + ' curl returns 200', + ' Type "done"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:decision missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: pick', + ' ', + ' ', + ' ', + ' Select: a', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:human-action missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: act', + ' Do it.', + ' curl returns 200', + ' Type "done"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint:human-action missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: act', + ' Do it', + ' Do it.', + ' Type "done"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('any checkpoint:* missing is flagged (AC2)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: verify UI', + ' UI', + ' Visit', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('checkpoint task without a type attribute still gets non-checkpoint rules (regression guard)', () => { + // A bare (no type=) is NOT treated as a checkpoint; current rules apply. + const output = runVerify(planWithTask([ + '', + ' Bare task', + ' x.ts', + ' echo ok', + ' ok', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid (missing )'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + // ── AC3: non-checkpoint tasks missing fields are still flagged (no regression) ── + + test('non-checkpoint task missing is still flagged (AC3)', () => { + const output = runVerify(planWithTask([ + '', + ' Task 1: no action', + ' echo ok', + ' Done', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error: ${JSON.stringify(output.errors)}` + ); + }); + + test('non-checkpoint task missing still warns (AC3)', () => { + const output = runVerify(planWithTask([ + '', + ' Task 1: no verify', + ' x.ts', + ' Do it', + ' Done', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.ok( + output.warnings.some(w => w.includes('missing ')), + `Expected "missing " warning: ${JSON.stringify(output.warnings)}` + ); + }); + + test('non-checkpoint task missing still warns (AC3)', () => { + const output = runVerify(planWithTask([ + '', + ' Task 1: no done', + ' x.ts', + ' Do it', + ' echo ok', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.ok( + output.warnings.some(w => w.includes('missing ')), + `Expected "missing " warning: ${JSON.stringify(output.warnings)}` + ); + }); + + test('non-checkpoint task missing still warns (AC3)', () => { + const output = runVerify(planWithTask([ + '', + ' Task 1: no files', + ' Do it', + ' echo ok', + ' Done', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.ok( + output.warnings.some(w => w.includes('missing ')), + `Expected "missing " warning: ${JSON.stringify(output.warnings)}` + ); + }); + + // ── Security: type-attribute charset is bounded (no markup injection) ────── + + test('task type attribute with hostile markup fragment is not surfaced unsanitized', () => { + // Per CONTRIBUTING.md §"Security and prompt-injection surfaces": a hostile + // PLAN.md cannot inject unclosed-tag fragments into the verifier's typed + // JSON output via the type= attribute. The charset [a-zA-Z0-9_:-] rejects + // '<', '>', '(', '&', etc., so a payload like type=evil', + ' Hostile', + ' do', + ' echo ok', + ' ok', + '', + ].join('\n'), { autonomous: 'true' })); + + const hostile = output.tasks.find(t => t.name === 'Hostile'); + assert.ok(hostile, `Expected to find Hostile task in output.tasks: ${JSON.stringify(output.tasks)}`); + assert.ok( + !/[<>()&]/.test(hostile.type), + `Expected type to contain no markup chars; got: ${JSON.stringify(hostile.type)}` + ); + assert.strictEqual(hostile.type, 'evil', `Expected capture to stop at '<'; got: ${JSON.stringify(hostile.type)}`); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // verify phase-completeness command // ─────────────────────────────────────────────────────────────────────────────