* fix(#1202): make verify key-links wave-aware for planned future files Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: backfill changeset PR number (#1219) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/calm-seals-glide.md
Normal file
5
.changeset/calm-seals-glide.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1219
|
||||
---
|
||||
**verify key-links no longer fails on planned future files** — a from: link whose file is declared in a current/upcoming wave plan’s files_modified is now reported pending instead of a hard missing-file failure. (#1202)
|
||||
@@ -21,6 +21,8 @@ import frontmatterMod = require('./frontmatter.cjs');
|
||||
import stateMod = require('./state.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- model-profiles.cjs is an export= CommonJS module
|
||||
import modelProfilesMod = require('./model-profiles.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module
|
||||
import planScanMod = require('./plan-scan.cjs');
|
||||
import { execGit, platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs';
|
||||
import { PACKAGE_NAME } from './package-identity.cjs';
|
||||
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
|
||||
@@ -525,6 +527,36 @@ function cmdVerifyArtifacts(cwd: string, planFilePath: string, raw: boolean): vo
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a Set of file paths (relative to cwd) that are promised by plans in
|
||||
* the same phase directory at a wave number >= minWave.
|
||||
*
|
||||
* Used by cmdVerifyKeyLinks to avoid hard-failing a missing `from:` file that
|
||||
* is a planned future artifact (fix #1202).
|
||||
*/
|
||||
function collectPromisedFilesAtOrAfterWave(phaseDir: string, minWave: number): Set<string> {
|
||||
const promised = new Set<string>();
|
||||
const { planFiles } = planScanMod.scanPhasePlans(phaseDir);
|
||||
for (const planFile of planFiles) {
|
||||
const planFullPath = path.join(phaseDir, planFile);
|
||||
const planContent = safeReadFile(planFullPath);
|
||||
if (!planContent) continue;
|
||||
const fm = extractFrontmatter(planContent);
|
||||
const waveRaw = fm['wave'];
|
||||
const wave = typeof waveRaw === 'string' ? parseInt(waveRaw, 10) : (typeof waveRaw === 'number' ? waveRaw : NaN);
|
||||
if (isNaN(wave) || wave < minWave) continue;
|
||||
const filesModified = fm['files_modified'];
|
||||
if (!filesModified) continue;
|
||||
const files: unknown[] = Array.isArray(filesModified)
|
||||
? filesModified
|
||||
: (typeof filesModified === 'string' ? [filesModified] : []);
|
||||
for (const f of files) {
|
||||
if (typeof f === 'string' && f.trim()) promised.add(f.trim());
|
||||
}
|
||||
}
|
||||
return promised;
|
||||
}
|
||||
|
||||
function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): void {
|
||||
if (!planFilePath) {
|
||||
error('plan file path required');
|
||||
@@ -542,7 +574,27 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
|
||||
return;
|
||||
}
|
||||
|
||||
// Derive the current plan's wave number and phase directory for wave-aware
|
||||
// missing-file handling (fix #1202).
|
||||
const currentFm = extractFrontmatter(content);
|
||||
const currentWaveRaw = currentFm['wave'];
|
||||
const currentWave = typeof currentWaveRaw === 'string'
|
||||
? parseInt(currentWaveRaw, 10)
|
||||
: (typeof currentWaveRaw === 'number' ? currentWaveRaw : 1);
|
||||
const phaseDir = path.dirname(fullPath);
|
||||
|
||||
// Collect files promised by plans at wave >= currentWave (lazy: computed once
|
||||
// the first time a missing source is encountered).
|
||||
let promisedFiles: Set<string> | null = null;
|
||||
function getPromisedFiles(): Set<string> {
|
||||
if (promisedFiles === null) {
|
||||
promisedFiles = collectPromisedFilesAtOrAfterWave(phaseDir, isNaN(currentWave) ? 1 : currentWave);
|
||||
}
|
||||
return promisedFiles;
|
||||
}
|
||||
|
||||
const results: Record<string, unknown>[] = [];
|
||||
let pendingCount = 0;
|
||||
for (const link of keyLinks) {
|
||||
if (typeof link === 'string') continue;
|
||||
const check: Record<string, unknown> = {
|
||||
@@ -553,9 +605,19 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
|
||||
detail: '',
|
||||
};
|
||||
|
||||
const sourceContent = safeReadFile(path.join(cwd, (link['from'] as string) || ''));
|
||||
const fromPath = (link['from'] as string) || '';
|
||||
const sourceContent = safeReadFile(path.join(cwd, fromPath));
|
||||
if (!sourceContent) {
|
||||
check['detail'] = 'Source file not found (from: must be a relative file path; describe components/endpoints in via:)';
|
||||
// Check if the missing file is promised by a plan at the same or later wave.
|
||||
const promised = getPromisedFiles();
|
||||
const isPromised = fromPath.trim() !== '' && promised.has(fromPath.trim());
|
||||
if (isPromised) {
|
||||
check['pending'] = true;
|
||||
check['detail'] = 'Source file not yet created — declared in files_modified of a same-or-later-wave plan';
|
||||
pendingCount++;
|
||||
} else {
|
||||
check['detail'] = 'Source file not found (from: must be a relative file path; describe components/endpoints in via:)';
|
||||
}
|
||||
} else if (link['pattern']) {
|
||||
try {
|
||||
const regex = new RegExp(link['pattern'] as string);
|
||||
@@ -587,15 +649,20 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
|
||||
}
|
||||
|
||||
const verified = results.filter((r) => r['verified']).length;
|
||||
// A pending link (from: file promised by a same-or-later-wave plan) is not a
|
||||
// hard failure — it should not count against the all_verified gate (#1202).
|
||||
const hardFailed = results.filter((r) => !r['verified'] && !r['pending']).length;
|
||||
const allVerified = hardFailed === 0;
|
||||
output(
|
||||
{
|
||||
all_verified: verified === results.length,
|
||||
all_verified: allVerified,
|
||||
verified,
|
||||
pending: pendingCount,
|
||||
total: results.length,
|
||||
links: results,
|
||||
},
|
||||
raw,
|
||||
verified === results.length ? 'valid' : 'invalid',
|
||||
allVerified ? 'valid' : 'invalid',
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -19,16 +19,21 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
|
||||
// ─── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
function writePlanWithKeyLinks(tmpDir, keyLinksYaml) {
|
||||
function writePlanWithKeyLinks(tmpDir, keyLinksYaml, opts) {
|
||||
// parseMustHavesBlock expects 4-space indent for block name, 6-space for items
|
||||
const wave = (opts && opts.wave != null) ? opts.wave : 1;
|
||||
const filesModified = (opts && opts.filesModified) ? opts.filesModified : ['src/a.js'];
|
||||
const filesModifiedYaml = filesModified.length === 1
|
||||
? `[${filesModified[0]}]`
|
||||
: `[${filesModified.join(', ')}]`;
|
||||
const content = [
|
||||
'---',
|
||||
'phase: 01-test',
|
||||
'plan: 01',
|
||||
'type: execute',
|
||||
'wave: 1',
|
||||
`wave: ${wave}`,
|
||||
'depends_on: []',
|
||||
'files_modified: [src/a.js]',
|
||||
`files_modified: ${filesModifiedYaml}`,
|
||||
'autonomous: true',
|
||||
'must_haves:',
|
||||
' key_links:',
|
||||
@@ -50,6 +55,39 @@ function writePlanWithKeyLinks(tmpDir, keyLinksYaml) {
|
||||
fs.writeFileSync(planPath, content);
|
||||
}
|
||||
|
||||
/**
|
||||
* Write an additional plan file in the same phase directory with specific
|
||||
* wave + files_modified (no key_links, just declaring future artifacts).
|
||||
*/
|
||||
function writeCompanionPlan(tmpDir, planFileName, wave, filesModified) {
|
||||
const filesModifiedYaml = `[${filesModified.join(', ')}]`;
|
||||
const content = [
|
||||
'---',
|
||||
'phase: 01-test',
|
||||
'plan: 02',
|
||||
'type: execute',
|
||||
`wave: ${wave}`,
|
||||
'depends_on: []',
|
||||
`files_modified: ${filesModifiedYaml}`,
|
||||
'autonomous: true',
|
||||
'must_haves:',
|
||||
' key_links: []',
|
||||
'---',
|
||||
'',
|
||||
'<tasks>',
|
||||
'<task type="auto">',
|
||||
' <name>Task 2: Create file</name>',
|
||||
' <files>src/b.js</files>',
|
||||
' <action>Create it</action>',
|
||||
' <verify><automated>echo ok</automated></verify>',
|
||||
' <done>Done</done>',
|
||||
'</task>',
|
||||
'</tasks>',
|
||||
].join('\n');
|
||||
const planPath = path.join(tmpDir, '.planning', 'phases', '01-test', planFileName);
|
||||
fs.writeFileSync(planPath, content);
|
||||
}
|
||||
|
||||
describe('bug-967 verify key-links strict file-path contract', () => {
|
||||
let tmpDir;
|
||||
|
||||
@@ -122,10 +160,123 @@ describe('bug-967 verify key-links strict file-path contract', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// ── 3. Doc-contract guard: reference example must use a file path for to: ──
|
||||
// ── 3. Regression #1202: missing from: file promised by a same-wave plan → pending:true ──
|
||||
//
|
||||
// A from: file absent on disk but listed in files_modified of another plan at
|
||||
// the same wave must be reported pending:true (not verified:false) and must NOT
|
||||
// count against the all_verified gate.
|
||||
//
|
||||
// This test MUST FAIL before the fix is applied (the gate hard-fails today).
|
||||
test('pending:true and all_verified:true when from: file is promised by a same-wave plan', () => {
|
||||
// Plan under test is wave 2; it references src/future-artifact.js which does not
|
||||
// exist on disk yet.
|
||||
writePlanWithKeyLinks(tmpDir, [
|
||||
'- from: "src/future-artifact.js"',
|
||||
' to: "src/consumer.js"',
|
||||
' via: "requires future-artifact"',
|
||||
' pattern: "future-artifact"',
|
||||
], { wave: 2, filesModified: ['src/consumer.js'] });
|
||||
|
||||
// A companion plan also at wave 2 declares src/future-artifact.js in files_modified
|
||||
writeCompanionPlan(tmpDir, '01-02-PLAN.md', 2, ['src/future-artifact.js']);
|
||||
|
||||
// Do NOT create src/future-artifact.js on disk — it is a planned future file
|
||||
|
||||
const result = runGsdTools(
|
||||
'verify key-links .planning/phases/01-test/01-01-PLAN.md',
|
||||
tmpDir,
|
||||
);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const out = JSON.parse(result.output);
|
||||
assert.strictEqual(
|
||||
out.links[0].pending,
|
||||
true,
|
||||
`Expected pending:true for a from: file promised by a same-wave plan. Got: ${JSON.stringify(out.links[0])}`,
|
||||
);
|
||||
assert.strictEqual(
|
||||
out.all_verified,
|
||||
true,
|
||||
`Expected all_verified:true (pending links should not fail the gate). Got: ${JSON.stringify(out)}`,
|
||||
);
|
||||
assert.strictEqual(
|
||||
out.links[0].verified,
|
||||
false,
|
||||
`Expected verified:false (file is not yet verified — just pending). Got: ${JSON.stringify(out.links[0])}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ── 4. Regression #1202: missing from: file promised by a LATER-wave plan → pending:true ──
|
||||
test('pending:true and all_verified:true when from: file is promised by a later-wave plan', () => {
|
||||
// Plan under test is wave 1; companion plan is wave 3 (later wave promises the file)
|
||||
writePlanWithKeyLinks(tmpDir, [
|
||||
'- from: "src/later-artifact.js"',
|
||||
' to: "src/consumer.js"',
|
||||
' via: "later wave dependency"',
|
||||
], { wave: 1, filesModified: ['src/consumer.js'] });
|
||||
|
||||
writeCompanionPlan(tmpDir, '01-02-PLAN.md', 3, ['src/later-artifact.js']);
|
||||
|
||||
const result = runGsdTools(
|
||||
'verify key-links .planning/phases/01-test/01-01-PLAN.md',
|
||||
tmpDir,
|
||||
);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const out = JSON.parse(result.output);
|
||||
assert.strictEqual(
|
||||
out.links[0].pending,
|
||||
true,
|
||||
`Expected pending:true for from: file promised by a later-wave plan. Got: ${JSON.stringify(out.links[0])}`,
|
||||
);
|
||||
assert.strictEqual(
|
||||
out.all_verified,
|
||||
true,
|
||||
`Expected all_verified:true (pending links not counted against gate). Got: ${JSON.stringify(out)}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ── 5. Regression #1202: missing from: file NOT promised by any plan → hard failure ──
|
||||
//
|
||||
// Absence of from: file with no plan promising it must remain a genuine verified:false failure.
|
||||
test('verified:false and all_verified:false when from: file is absent and not promised by any plan', () => {
|
||||
writePlanWithKeyLinks(tmpDir, [
|
||||
'- from: "src/truly-missing.js"',
|
||||
' to: "src/consumer.js"',
|
||||
' via: "no plan promises this"',
|
||||
], { wave: 1, filesModified: ['src/consumer.js'] });
|
||||
|
||||
// No companion plan that promises src/truly-missing.js
|
||||
|
||||
const result = runGsdTools(
|
||||
'verify key-links .planning/phases/01-test/01-01-PLAN.md',
|
||||
tmpDir,
|
||||
);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const out = JSON.parse(result.output);
|
||||
assert.strictEqual(
|
||||
out.links[0].verified,
|
||||
false,
|
||||
`Expected verified:false for absent+unpromised from: file. Got: ${JSON.stringify(out.links[0])}`,
|
||||
);
|
||||
assert.strictEqual(
|
||||
out.all_verified,
|
||||
false,
|
||||
`Expected all_verified:false (hard failure). Got: ${JSON.stringify(out)}`,
|
||||
);
|
||||
// pending must not be true
|
||||
assert.notStrictEqual(
|
||||
out.links[0].pending,
|
||||
true,
|
||||
`Expected pending not to be true for an absent+unpromised file. Got: ${JSON.stringify(out.links[0])}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ── 6. Doc-contract guard: reference example must use a file path for to: ──
|
||||
//
|
||||
// The old reference example had to: "/api/feed" (an HTTP endpoint).
|
||||
// After the fix, to: must be a relative file path like "app/api/feed/route.ts".
|
||||
// After fix #967, to: must be a relative file path like "app/api/feed/route.ts".
|
||||
// This test reads the canonical docs file and asserts the example is consistent
|
||||
// with the strict-path contract.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user