* fix(#4678): check :LINE citations instead of dropping or misreporting them cmdVerifyReferences had two opposite failures on citations carrying a trailing line suffix. The backtick extractor anchored the closing backtick right after the extension, so `src/foo.ts:42` matched neither regex and was silently dropped -- an all-line-numbered document reported {valid:true, found:0, missing:[], total:0}, indistinguishable from one with no citations. The @-extractor treated ':' as a legal path character, so @src/foo.ts:42 was probed with the suffix glued on and a resolvable file was reported missing. Strip the :N / :N-M suffix (stripLineSuffix) before existsSync in both loops -- for filesystem resolution only; found/missing keep reporting the original citation text -- and let the backtick regex accept the optional suffix so those citations are counted at all. URL skip, template-placeholder skip, dedup, ~/ expansion and the output contract are unchanged. Whether a line number past EOF counts as missing is an open design question and stays out of scope. * chore(#4678): backfill changeset pr field with PR number The fragment shipped as the documented pr: 0 placeholder; the merge gate requires pr > 0 before a fragment can land, so backfill 4719 now that the PR number exists. --------- Co-authored-by: TwistedRiCen <16397953+TwistedRiCen@users.noreply.github.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
5
.changeset/sturdy-ravens-hum.md
Normal file
5
.changeset/sturdy-ravens-hum.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4719
|
||||
---
|
||||
**`verify references` no longer mishandles `:LINE` citations** — backtick citations with a line suffix (e.g. `src/foo.ts:42`) are now checked instead of silently dropped, and @-citations with a line suffix resolve against the underlying file instead of being reported missing (#4678).
|
||||
@@ -1312,6 +1312,12 @@ function cmdVerifyPhaseCompleteness(cwd: string, phase: string, raw: boolean): v
|
||||
);
|
||||
}
|
||||
|
||||
// #4678: citations may carry a trailing line suffix (":42", ":1-20") that
|
||||
// describes a location inside the file, not part of the path itself.
|
||||
function stripLineSuffix(ref: string): string {
|
||||
return ref.replace(/:\d+(?:-\d+)?$/, '');
|
||||
}
|
||||
|
||||
function cmdVerifyReferences(cwd: string, filePath: string, raw: boolean): void {
|
||||
if (!filePath) {
|
||||
error('file path required');
|
||||
@@ -1329,9 +1335,10 @@ function cmdVerifyReferences(cwd: string, filePath: string, raw: boolean): void
|
||||
const atRefs = content.match(/@([^\s\n,)]+\/[^\s\n,)]+)/g) || [];
|
||||
for (const ref of atRefs) {
|
||||
const cleanRef = ref.slice(1);
|
||||
const resolved = cleanRef.startsWith('~/')
|
||||
? path.join(process.env['HOME'] || '', cleanRef.slice(2))
|
||||
: path.join(cwd, cleanRef);
|
||||
const fsRef = stripLineSuffix(cleanRef);
|
||||
const resolved = fsRef.startsWith('~/')
|
||||
? path.join(process.env['HOME'] || '', fsRef.slice(2))
|
||||
: path.join(cwd, fsRef);
|
||||
if (fs.existsSync(resolved)) {
|
||||
found.push(cleanRef);
|
||||
} else {
|
||||
@@ -1339,12 +1346,12 @@ function cmdVerifyReferences(cwd: string, filePath: string, raw: boolean): void
|
||||
}
|
||||
}
|
||||
|
||||
const backtickRefs = content.match(/`([^`]+\/[^`]+\.[a-zA-Z]{1,10})`/g) || [];
|
||||
const backtickRefs = content.match(/`([^`]+\/[^`]+\.[a-zA-Z]{1,10}(?::\d+(?:-\d+)?)?)`/g) || [];
|
||||
for (const ref of backtickRefs) {
|
||||
const cleanRef = ref.slice(1, -1);
|
||||
if (cleanRef.startsWith('http') || cleanRef.includes('${') || cleanRef.includes('{{')) continue;
|
||||
if (found.includes(cleanRef) || missing.includes(cleanRef)) continue;
|
||||
const resolved = path.join(cwd, cleanRef);
|
||||
const resolved = path.join(cwd, stripLineSuffix(cleanRef));
|
||||
if (fs.existsSync(resolved)) {
|
||||
found.push(cleanRef);
|
||||
} else {
|
||||
|
||||
@@ -1397,6 +1397,58 @@ describe('verify references command', () => {
|
||||
assert.strictEqual(output.total, 0, `Expected total 0 (template skipped): ${JSON.stringify(output)}`);
|
||||
});
|
||||
|
||||
test('#4678: line-numbered citations are checked, not dropped or misreported', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'src', 'app.js'), 'console.log("app");\n');
|
||||
fs.writeFileSync(path.join(tmpDir, 'src', 'utils', 'helper.js'), 'module.exports = {};\n');
|
||||
const filePath = path.join(tmpDir, '.planning', 'phases', '01-test', 'doc.md');
|
||||
fs.writeFileSync(filePath, [
|
||||
'- `src/gone.ts:99`',
|
||||
'- `src/utils/helper.js:7`',
|
||||
'- @src/app.js:42',
|
||||
'- @src/gone.ts:1',
|
||||
'- `src/utils/helper.js`',
|
||||
'- `src/gone.ts`',
|
||||
'- @src/app.js',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = runGsdTools('verify references .planning/phases/01-test/doc.md', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
// Every citation must land in exactly one bucket: the three gone.ts citations
|
||||
// (with and without the line suffix, in both citation styles) are missing;
|
||||
// the rest resolve.
|
||||
assert.strictEqual(output.total, 7, `Expected total 7: ${JSON.stringify(output)}`);
|
||||
assert.strictEqual(output.found, 4, `Expected found 4: ${JSON.stringify(output)}`);
|
||||
assert.ok(
|
||||
output.missing.includes('src/gone.ts:99'),
|
||||
`Expected missing to keep the original citation text "src/gone.ts:99": ${JSON.stringify(output.missing)}`
|
||||
);
|
||||
assert.ok(
|
||||
output.missing.includes('src/gone.ts:1'),
|
||||
`Expected missing to keep the original citation text "src/gone.ts:1": ${JSON.stringify(output.missing)}`
|
||||
);
|
||||
assert.ok(
|
||||
output.missing.includes('src/gone.ts'),
|
||||
`Expected missing to include "src/gone.ts": ${JSON.stringify(output.missing)}`
|
||||
);
|
||||
assert.strictEqual(output.valid, false, 'should be invalid');
|
||||
});
|
||||
|
||||
test('#4678: an all-missing line-numbered document is not reported valid', () => {
|
||||
const filePath = path.join(tmpDir, '.planning', 'phases', '01-test', 'doc.md');
|
||||
fs.writeFileSync(filePath, ['- `src/gone.ts:99`', '- `src/also-gone.ts:1-20`', ''].join('\n'));
|
||||
|
||||
const result = runGsdTools('verify references .planning/phases/01-test/doc.md', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.total, 2, `Expected total 2: ${JSON.stringify(output)}`);
|
||||
assert.strictEqual(output.missing.length, 2, `Expected both citations missing: ${JSON.stringify(output)}`);
|
||||
assert.strictEqual(output.valid, false, `Must not report valid: ${JSON.stringify(output)}`);
|
||||
});
|
||||
|
||||
test('returns error for nonexistent file', () => {
|
||||
const result = runGsdTools('verify references .planning/phases/01-test/nonexistent.md', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
Reference in New Issue
Block a user