From ff2d08d453d5f855b7041a9c5c1d5d8ca01c602f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 00:14:07 -0400 Subject: [PATCH] fix(#3193): tolerate attributes on plan-task child tags (#3433) * fix(#3193): tolerate attributes on plan-task child tags * chore(#3193): add changeset * chore(#3193): set changeset pr to 3433 --------- Co-authored-by: sim --- .changeset/zesty-tigers-forage.md | 5 + src/verify.cts | 27 +-- tests/verify.test.cjs | 274 ++++++++++++++++++++++++++++++ 3 files changed, 295 insertions(+), 11 deletions(-) create mode 100644 .changeset/zesty-tigers-forage.md diff --git a/.changeset/zesty-tigers-forage.md b/.changeset/zesty-tigers-forage.md new file mode 100644 index 000000000..db5f68f1d --- /dev/null +++ b/.changeset/zesty-tigers-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3433 +--- +verify plan-structure now recognizes task child elements that carry attributes on their opening tag (e.g. ), so plans annotating verify mode (auto vs human) or other child-tag attributes no longer produce false "missing " / "missing " / etc. warnings. Bare tags continue to validate exactly as before. diff --git a/src/verify.cts b/src/verify.cts index 8fa988a15..5a40278f0 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -749,17 +749,22 @@ function extractPlanTaskInfos(content: string): PlanTaskInfo[] { 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), + // #3193: child-tag presence uses /]/ (attribute-tolerant) so an + // opener like still counts as present, matching how + // the parent is read by PLAN_TASK_BLOCK_RE. The [\s>] + // terminator (not \b) keeps from satisfying and + // prevents a hyphenated sibling like from masking . + 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. diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index 0073f3444..c40e6071b 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -741,6 +741,280 @@ describe('verify plan-structure — checkpoint task types (#2444)', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// verify plan-structure — attributed child tags (#3193) +// A task's child elements (files/action/verify/done + every checkpoint-specific +// field) must be recognized as present even when the opening tag carries an +// attribute (e.g. …), consistent with how the +// parent is already read. The presence regexes were literal +// (//.test(body)) and were defeated by any attribute. +// ───────────────────────────────────────────────────────────────────────────── + +describe('verify plan-structure — attributed child tags (#3193)', () => { + 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. Mirrors the + // #2444 suite's planWithTask so each test reads as a one-task plan. + 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: attributed child tags pass with zero findings ────────────────────── + + test('auto task with every required field attributed passes (AC1)', () => { + // Every required auto-task child carries a `mode="auto"` attribute on its + // opening tag — the exact shape the issue reports as false-flagged. + const output = runVerify(planWithTask([ + '', + ' Task 1: attributed', + ' some/file.ts', + ' Do the thing', + ' echo ok', + ' Thing is done', + '', + ].join('\n'), { autonomous: 'true' })); + + 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.deepStrictEqual(output.warnings, [], `expected no warnings; got: ${JSON.stringify(output.warnings)}`); + }); + + test('checkpoint:human-verify with attributed triple passes (AC1)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: verify UI', + ' Dashboard at localhost:3000', + ' Visit /dashboard, check 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)}`); + }); + + test('checkpoint:decision with attributed fields passes (AC1)', () => { + const output = runVerify(planWithTask([ + '', + ' Checkpoint: pick auth provider', + ' Select authentication provider', + ' ', + ' ', + ' ', + ' Select: supabase', + '', + ].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 attributed 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"', + '', + ].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: attributed auto task + attributed checkpoint task passes (AC1 realistic)', () => { + // Mirrors the issue's "two plans in one project" shape: every required + // field across BOTH task types carries an attribute. + 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', + ' 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: genuinely absent child tag is still flagged (no false negatives) ─── + + test('auto task with attributed siblings but verify omitted still warns (AC2)', () => { + // files/action/done are attributed; verify is entirely absent (not bare, + // not attributed). The fix must not invent presence from nothing. + const output = runVerify(planWithTask([ + '', + ' Task 1: no verify', + ' some/file.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('checkpoint:human-verify with attributed siblings but how-to-verify omitted 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-action with attributed siblings but instructions omitted 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 task with attributed siblings but resume-signal omitted 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)}` + ); + }); + + // ── AC3: bare + attributed tags mix cleanly (no regression on bare form) ──── + + test('mixed bare and attributed tags within one task pass (AC3)', () => { + // is bare; // are attributed. Proves the + // attribute-tolerant regex did not stop matching the bare opener. + const output = runVerify(planWithTask([ + '', + ' Task 1: mixed', + ' some/file.ts', + ' Do the thing', + ' echo ok', + ' Done', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.strictEqual(output.valid, true, `expected valid; errors: ${JSON.stringify(output.errors)}`); + assert.deepStrictEqual(output.warnings, [], `expected no warnings; got: ${JSON.stringify(output.warnings)}`); + }); + + // ── AC4: boundary — a hyphenated sibling tag must not satisfy a shorter tag ─ + + test(' present, absent does not satisfy (AC4 boundary)', () => { + // The fix uses /]/ so that ``) does NOT count as `` presence. A bare + // hyphenated opener must not mask a genuinely-missing shorter tag. + const output = runVerify(planWithTask([ + '', + ' Task 1: hyphen sibling', + ' some/file.ts', + ' Do it', + ' not a real verify tag', + ' Done', + '', + ].join('\n'), { autonomous: 'true' })); + + assert.ok( + output.warnings.some(w => w.includes('missing ')), + `Expected "missing " warning despite : ${JSON.stringify(output.warnings)}` + ); + }); + + test(' does not satisfy the checkpoint:human-action requirement (AC4 boundary)', () => { + // The [\s>] terminator after the tag name must keep and + // distinct: a opener is NOT a + // opener, so a human-action task with only must still be flagged + // for the missing . + const output = runVerify(planWithTask([ + '', + ' Checkpoint: act', + ' Do it', + ' Do it.', + ' echo ok', + ' Type "done"', + '', + ].join('\n'))); + + assert.strictEqual(output.valid, false, 'should be invalid'); + assert.ok( + output.errors.some(e => e.includes('missing ')), + `Expected "missing " error (not satisfied by ): ${JSON.stringify(output.errors)}` + ); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // verify phase-completeness command // ─────────────────────────────────────────────────────────────────────────────