* 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 <action>/<verify>/<done>/<files> 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 (<what-built>/<how-to-verify>/<resume-signal>) 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 <action>/<verify>/<done>/ <files> 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 <task ...>...</task> 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 <what-built>/<how-to-verify>/ <resume-signal> (the canonical triple). checkpoint:decision requires <decision>/<options>/<resume-signal>. checkpoint:human-action requires <action>/<instructions>/ <verification>/<resume-signal>. Unknown checkpoint:* subtypes require only the universal <resume-signal> (forward-compat). All other types keep the historical <action>/<verify>/<done>/<files> 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 <done> and <files> regression tests for non-checkpoint tasks (pre-existing tests only covered <action> and <verify>). - AC2: add checkpoint:decision missing <decision>, checkpoint:human-action missing <action>, checkpoint:human-action missing <verification> 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<fragment) into the verifier's typed JSON output. All legitimate type values (auto, tracer, manual, checkpoint:human-verify, checkpoint:decision, checkpoint:human-action, checkpoint:tdd-review) match the tighter charset. - Add adversarial regression test asserting type=evil<fragment surfaces as 'evil' (capture stops at '<'), with no markup chars (< > ( ) &) 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.
This commit is contained in:
Tom Boucher
184
src/verify.cts
184
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 `<tasks>` 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 `<task ...>…</task>` 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 `<task[\s>]` opening, so a document
|
||||
* full of unclosed `<task>` 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 = /<task(\s[^>]{0,1000})?>((?:(?!<task[\s>])[\s\S])*?)<\/task>/g;
|
||||
|
||||
/**
|
||||
* Extract one `PlanTaskInfo` per `<task …>…</task>` 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: /<files>/.test(body),
|
||||
hasAction: /<action>/.test(body),
|
||||
hasVerify: /<verify>/.test(body),
|
||||
hasDone: /<done>/.test(body),
|
||||
hasWhatBuilt: /<what-built>/.test(body),
|
||||
hasHowToVerify: /<how-to-verify>/.test(body),
|
||||
hasDecision: /<decision>/.test(body),
|
||||
hasOptions: /<options>/.test(body),
|
||||
hasInstructions: /<instructions>/.test(body),
|
||||
hasVerification: /<verification>/.test(body),
|
||||
hasResumeSignal: /<resume-signal>/.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 `<what-built>` / `<how-to-verify>` /
|
||||
* `<resume-signal>` (the "checkpoint triple").
|
||||
* - `checkpoint:decision` requires `<decision>` / `<options>` /
|
||||
* `<resume-signal>`.
|
||||
* - `checkpoint:human-action` requires `<action>` / `<instructions>` /
|
||||
* `<verification>` / `<resume-signal>`.
|
||||
* - Unknown `checkpoint:*` subtypes require only the universal
|
||||
* `<resume-signal>` (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 `<task>`, …) keep the
|
||||
* historical `<action>` / `<verify>` / `<done>` / `<files>` 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 <name> element');
|
||||
}
|
||||
|
||||
if (isCheckpointType(task.type)) {
|
||||
if (!task.hasResumeSignal) {
|
||||
errors.push(`Task '${taskName}' missing <resume-signal>`);
|
||||
}
|
||||
switch (task.type) {
|
||||
case 'checkpoint:human-verify':
|
||||
if (!task.hasWhatBuilt) errors.push(`Task '${taskName}' missing <what-built>`);
|
||||
if (!task.hasHowToVerify) errors.push(`Task '${taskName}' missing <how-to-verify>`);
|
||||
break;
|
||||
case 'checkpoint:decision':
|
||||
if (!task.hasDecision) errors.push(`Task '${taskName}' missing <decision>`);
|
||||
if (!task.hasOptions) errors.push(`Task '${taskName}' missing <options>`);
|
||||
break;
|
||||
case 'checkpoint:human-action':
|
||||
if (!task.hasAction) errors.push(`Task '${taskName}' missing <action>`);
|
||||
if (!task.hasInstructions) errors.push(`Task '${taskName}' missing <instructions>`);
|
||||
if (!task.hasVerification) errors.push(`Task '${taskName}' missing <verification>`);
|
||||
break;
|
||||
default:
|
||||
// Unknown checkpoint:* subtype: <resume-signal> is the only universal
|
||||
// requirement (forward-compat).
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
if (!task.hasAction) errors.push(`Task '${taskName}' missing <action>`);
|
||||
if (!task.hasVerify) warnings.push(`Task '${taskName}' missing <verify>`);
|
||||
if (!task.hasDone) warnings.push(`Task '${taskName}' missing <done>`);
|
||||
if (!task.hasFiles) warnings.push(`Task '${taskName}' missing <files>`);
|
||||
}
|
||||
|
||||
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<string, unknown>[] = [];
|
||||
for (const taskContent of extractTaggedBlocks(content, 'task', true)) {
|
||||
const nameArr = extractTaggedBlocks(taskContent, 'name');
|
||||
const taskName = nameArr.length ? nameArr[0].trim() : 'unnamed';
|
||||
const hasFiles = /<files>/.test(taskContent);
|
||||
const hasAction = /<action>/.test(taskContent);
|
||||
const hasVerify = /<verify>/.test(taskContent);
|
||||
const hasDone = /<done>/.test(taskContent);
|
||||
|
||||
if (nameArr.length === 0) errors.push('Task missing <name> element');
|
||||
if (!hasAction) errors.push(`Task '${taskName}' missing <action>`);
|
||||
if (!hasVerify) warnings.push(`Task '${taskName}' missing <verify>`);
|
||||
if (!hasDone) warnings.push(`Task '${taskName}' missing <done>`);
|
||||
if (!hasFiles) warnings.push(`Task '${taskName}' missing <files>`);
|
||||
|
||||
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 <task> elements found');
|
||||
|
||||
Reference in New Issue
Block a user