fix: align planner plan contract with phase index (#3436)
* test: add planner/query contract regressions (#3430) * fix: align planner plan contract with phase index (#3430) * docs: add changeset for #3430 * fix: keep planner contract docs within size budget (#3430) * docs: set changeset pr for #3430
This commit is contained in:
5
.changeset/silly-jaguars-sing.md
Normal file
5
.changeset/silly-jaguars-sing.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3436
|
||||
---
|
||||
**`/gsd-plan-phase` now documents phase-plan-index-compatible plan conventions** — planner guidance uses canonical `depends_on` and SUMMARY forms, and native phase indexing warns when it ignores noncanonical plan filenames.
|
||||
@@ -426,7 +426,7 @@ phase: XX-name
|
||||
plan: NN
|
||||
type: execute
|
||||
wave: N # Execution wave (1, 2, 3...)
|
||||
depends_on: [] # Plan IDs this plan requires
|
||||
depends_on: [] # Use `01-01`/`01-01-auth-hardening`
|
||||
files_modified: [] # Files this plan touches
|
||||
autonomous: true # false if plan has checkpoints
|
||||
requirements: [] # REQUIRED — Requirement IDs from ROADMAP this plan addresses. MUST NOT be empty.
|
||||
@@ -496,7 +496,7 @@ Output: [Artifacts created]
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/XX-name/{phase}-{plan}-SUMMARY.md`
|
||||
Create `.planning/phases/XX-name/{padded_phase}-{plan}-SUMMARY.md` when done
|
||||
</output>
|
||||
```
|
||||
|
||||
|
||||
@@ -505,4 +505,27 @@ describe('phasePlanIndex', () => {
|
||||
expect(planA!.depends_on).toEqual([]);
|
||||
expect(planB!.depends_on).toEqual(['15-01']);
|
||||
});
|
||||
|
||||
it('#3430: native phase-plan-index warns about noncanonical plan-shaped files it cannot index', async () => {
|
||||
const phase16 = join(tmpDir, '.planning', 'phases', '16-warning');
|
||||
await mkdir(phase16, { recursive: true });
|
||||
await writeFile(join(phase16, '16-PLAN-01-eval-harness.md'), [
|
||||
'---',
|
||||
'phase: 16-warning',
|
||||
'plan: 01',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'<objective>Noncanonical plan filename.</objective>',
|
||||
].join('\n'));
|
||||
|
||||
const result = await phasePlanIndex(['16'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
|
||||
expect(data.plans).toEqual([]);
|
||||
expect(data.warnings).toEqual([
|
||||
'Ignored noncanonical plan files: 16-PLAN-01-eval-harness.md',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -278,6 +278,11 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
const phaseFiles = await readdir(phaseDir);
|
||||
const planFiles = phaseFiles.filter(f => f.endsWith('-PLAN.md') || f === 'PLAN.md').sort();
|
||||
const summaryFiles = phaseFiles.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md');
|
||||
const nonCanonicalPlanFiles = phaseFiles.filter((f) => (
|
||||
f.toLowerCase().endsWith('.md')
|
||||
&& /(^|-)plan(-|\.)/i.test(f)
|
||||
&& !(f.endsWith('-PLAN.md') || f === 'PLAN.md')
|
||||
)).sort();
|
||||
|
||||
// Build set of plan IDs with summaries — match the planId derivation logic
|
||||
const completedPlanIds = new Set(
|
||||
@@ -433,6 +438,10 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
let hasCheckpoints = false;
|
||||
const warnings: string[] = [];
|
||||
|
||||
if (nonCanonicalPlanFiles.length > 0) {
|
||||
warnings.push(`Ignored noncanonical plan files: ${nonCanonicalPlanFiles.join(', ')}`);
|
||||
}
|
||||
|
||||
for (const raw of rawPlans) {
|
||||
if (!raw.autonomous) {
|
||||
hasCheckpoints = true;
|
||||
|
||||
44
tests/bug-3430-planner-phase-contract.test.cjs
Normal file
44
tests/bug-3430-planner-phase-contract.test.cjs
Normal file
@@ -0,0 +1,44 @@
|
||||
// allow-test-rule: source-text-is-the-product
|
||||
// Planner markdown is the deployed planning contract; these checks lock the
|
||||
// exact canonical forms that downstream phase-plan-index accepts.
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const PLANNER_PATH = path.join(__dirname, '..', 'agents', 'gsd-planner.md');
|
||||
|
||||
function readPlanner() {
|
||||
return fs.readFileSync(PLANNER_PATH, 'utf8');
|
||||
}
|
||||
|
||||
test('#3430: planner SUMMARY instruction uses canonical padded phase/plan form', () => {
|
||||
const content = readPlanner();
|
||||
assert.match(
|
||||
content,
|
||||
/Create `\.planning\/phases\/XX-name\/\{padded_phase\}-\{plan\}-SUMMARY\.md` when done/,
|
||||
'planner must instruct executors to write SUMMARY files in canonical padded-phase form'
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
content,
|
||||
/After completion, create `\.planning\/phases\/XX-name\/\{phase\}-\{plan\}-SUMMARY\.md`/,
|
||||
'planner must not instruct the broken {phase}-{plan}-SUMMARY.md form'
|
||||
);
|
||||
});
|
||||
|
||||
test('#3430: planner depends_on docs show canonical in-phase plan ids', () => {
|
||||
const content = readPlanner();
|
||||
assert.match(
|
||||
content,
|
||||
/depends_on:[^\n]*Use `01-01`\/`01-01-auth-hardening`/,
|
||||
'planner must document canonical depends_on examples that phase-plan-index resolves'
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
content,
|
||||
/depends_on:[^\n]*01-trust\/01/,
|
||||
'planner must not document phase-slug/plan-number depends_on examples as canonical'
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user