fix(#967): make verify key-links docs author-strict (from:/to: are file paths; symbols go in via:) (#990)
* fix(#967): make verify key-links docs author-strict (from:/to: are file paths; symbols go in via:) Closes #967 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(#967): backfill changeset pr number (990) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/967-verify-key-links-author-strict-docs.md
Normal file
5
.changeset/967-verify-key-links-author-strict-docs.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
type: Fixed
|
||||||
|
pr: 990
|
||||||
|
---
|
||||||
|
**`verify key-links` docs now correctly state `from:`/`to:` are relative file paths** — the reference implied component/endpoint values the verifier never supported, so locator-style links failed with a misleading 'Source file not found' and the author's `pattern:` was never evaluated. (#967)
|
||||||
@@ -707,8 +707,8 @@ must_haves:
|
|||||||
min_lines: 30
|
min_lines: 30
|
||||||
key_links:
|
key_links:
|
||||||
- from: "src/components/LoginForm.tsx"
|
- from: "src/components/LoginForm.tsx"
|
||||||
to: "/api/auth/login"
|
to: "src/app/api/auth/login/route.ts"
|
||||||
via: "fetch in onSubmit"
|
via: "fetch in onSubmit → POST /api/auth/login"
|
||||||
```
|
```
|
||||||
|
|
||||||
Aggregate across plans for full picture of what phase delivers.
|
Aggregate across plans for full picture of what phase delivers.
|
||||||
|
|||||||
@@ -632,12 +632,12 @@ must_haves:
|
|||||||
contains: "model Message"
|
contains: "model Message"
|
||||||
key_links:
|
key_links:
|
||||||
- from: "src/components/Chat.tsx"
|
- from: "src/components/Chat.tsx"
|
||||||
to: "/api/chat"
|
to: "src/app/api/chat/route.ts"
|
||||||
via: "fetch in useEffect"
|
via: "fetch in useEffect — calls /api/chat endpoint"
|
||||||
pattern: "fetch.*api/chat"
|
pattern: "fetch.*api/chat"
|
||||||
- from: "src/app/api/chat/route.ts"
|
- from: "src/app/api/chat/route.ts"
|
||||||
to: "prisma.message"
|
to: "prisma/schema.prisma"
|
||||||
via: "database query"
|
via: "database query via prisma.message"
|
||||||
pattern: "prisma\\.message\\.(find|create)"
|
pattern: "prisma\\.message\\.(find|create)"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|||||||
@@ -136,9 +136,9 @@ must_haves:
|
|||||||
- path: "src/components/Chat.tsx"
|
- path: "src/components/Chat.tsx"
|
||||||
provides: "Message list rendering"
|
provides: "Message list rendering"
|
||||||
key_links:
|
key_links:
|
||||||
- from: "Chat.tsx"
|
- from: "src/components/Chat.tsx"
|
||||||
to: "api/chat"
|
to: "src/app/api/chat/route.ts"
|
||||||
via: "fetch in useEffect"
|
via: "fetch in useEffect — calls /api/chat endpoint"
|
||||||
```
|
```
|
||||||
|
|
||||||
**Step 2c: Merge must-haves**
|
**Step 2c: Merge must-haves**
|
||||||
|
|||||||
@@ -53,8 +53,8 @@ must_haves:
|
|||||||
exports: ["PostCard"]
|
exports: ["PostCard"]
|
||||||
key_links:
|
key_links:
|
||||||
- from: "src/components/PostFeed.tsx"
|
- from: "src/components/PostFeed.tsx"
|
||||||
to: "/api/feed"
|
to: "src/app/api/feed/route.ts"
|
||||||
via: "fetch in useEffect"
|
via: "fetch in useEffect — calls /api/feed endpoint"
|
||||||
pattern: "fetch.*api/feed"
|
pattern: "fetch.*api/feed"
|
||||||
---
|
---
|
||||||
```
|
```
|
||||||
@@ -92,9 +92,9 @@ must_haves:
|
|||||||
| `artifacts[].exports` | array of strings (optional) | Expected named exports to verify. |
|
| `artifacts[].exports` | array of strings (optional) | Expected named exports to verify. |
|
||||||
| `artifacts[].contains` | string (optional) | Regex or literal pattern that must appear in the file. |
|
| `artifacts[].contains` | string (optional) | Regex or literal pattern that must appear in the file. |
|
||||||
| `key_links` | array of objects | Critical connections between artifacts — the wiring that makes the system work end-to-end. |
|
| `key_links` | array of objects | Critical connections between artifacts — the wiring that makes the system work end-to-end. |
|
||||||
| `key_links[].from` | string | Source file or component. |
|
| `key_links[].from` | string | Source file (relative path from project root). Must be a literal file path — describe components or symbols in `via:`. |
|
||||||
| `key_links[].to` | string | Target file, endpoint, or module. |
|
| `key_links[].to` | string | Target file (relative path from project root). Must be a literal file path — describe endpoints, modules, or APIs in `via:`. |
|
||||||
| `key_links[].via` | string | Description of how they connect (e.g. `fetch in useEffect`, `Prisma query`, `import`). |
|
| `key_links[].via` | string | Description of how they connect, including any endpoint, component, or symbol name (e.g. `fetch in useEffect — calls /api/feed`, `Prisma query via prisma.message`, `import`). |
|
||||||
| `key_links[].pattern` | string (optional) | Regex to verify the connection exists in source. |
|
| `key_links[].pattern` | string (optional) | Regex to verify the connection exists in source. |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -568,12 +568,12 @@ must_haves:
|
|||||||
contains: "model Message"
|
contains: "model Message"
|
||||||
key_links:
|
key_links:
|
||||||
- from: "src/components/Chat.tsx"
|
- from: "src/components/Chat.tsx"
|
||||||
to: "/api/chat"
|
to: "src/app/api/chat/route.ts"
|
||||||
via: "fetch in useEffect"
|
via: "fetch in useEffect — calls /api/chat endpoint"
|
||||||
pattern: "fetch.*api/chat"
|
pattern: "fetch.*api/chat"
|
||||||
- from: "src/app/api/chat/route.ts"
|
- from: "src/app/api/chat/route.ts"
|
||||||
to: "prisma.message"
|
to: "prisma/schema.prisma"
|
||||||
via: "database query"
|
via: "database query via prisma.message"
|
||||||
pattern: "prisma\\.message\\.(find|create)"
|
pattern: "prisma\\.message\\.(find|create)"
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -589,9 +589,9 @@ must_haves:
|
|||||||
| `artifacts[].exports` | Optional. Expected exports to verify. |
|
| `artifacts[].exports` | Optional. Expected exports to verify. |
|
||||||
| `artifacts[].contains` | Optional. Pattern that must exist in file. |
|
| `artifacts[].contains` | Optional. Pattern that must exist in file. |
|
||||||
| `key_links` | Critical connections between artifacts. |
|
| `key_links` | Critical connections between artifacts. |
|
||||||
| `key_links[].from` | Source artifact. |
|
| `key_links[].from` | Source file (relative path from project root). Describe components or symbols in `via:`. |
|
||||||
| `key_links[].to` | Target artifact or endpoint. |
|
| `key_links[].to` | Target file (relative path from project root). Describe endpoints, APIs, or modules in `via:`. |
|
||||||
| `key_links[].via` | How they connect (description). |
|
| `key_links[].via` | How they connect, including any endpoint or symbol name (e.g. `fetch in useEffect — calls /api/chat`, `Prisma query via prisma.message`). |
|
||||||
| `key_links[].pattern` | Optional. Regex to verify connection exists. |
|
| `key_links[].pattern` | Optional. Regex to verify connection exists. |
|
||||||
|
|
||||||
**Why this matters:**
|
**Why this matters:**
|
||||||
|
|||||||
@@ -260,6 +260,7 @@
|
|||||||
"bug-947-hermes-gsd-prefix.test.cjs",
|
"bug-947-hermes-gsd-prefix.test.cjs",
|
||||||
"bug-948-state-noop-write-guard.test.cjs",
|
"bug-948-state-noop-write-guard.test.cjs",
|
||||||
"bug-950-quick-summary-status-complete.test.cjs",
|
"bug-950-quick-summary-status-complete.test.cjs",
|
||||||
|
"bug-967-verify-key-links-strict-paths.test.cjs",
|
||||||
"bug-974-graphify-budget-missing-value.test.cjs",
|
"bug-974-graphify-budget-missing-value.test.cjs",
|
||||||
"bug-978-milestone-complete-force.test.cjs"
|
"bug-978-milestone-complete-force.test.cjs"
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -116,6 +116,7 @@
|
|||||||
"bug-2994-verify-reapply-patches-installed-path.test.cjs",
|
"bug-2994-verify-reapply-patches-installed-path.test.cjs",
|
||||||
"bug-3381-verify-work-workstream.test.cjs",
|
"bug-3381-verify-work-workstream.test.cjs",
|
||||||
"bug-3657-verify-reapply-patches-pristine-drift.test.cjs",
|
"bug-3657-verify-reapply-patches-pristine-drift.test.cjs",
|
||||||
|
"bug-967-verify-key-links-strict-paths.test.cjs",
|
||||||
"verify-health.test.cjs",
|
"verify-health.test.cjs",
|
||||||
"verify-mvp-uat.test.cjs",
|
"verify-mvp-uat.test.cjs",
|
||||||
"verify-npm-publish.test.cjs",
|
"verify-npm-publish.test.cjs",
|
||||||
|
|||||||
@@ -453,7 +453,7 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi
|
|||||||
|
|
||||||
const sourceContent = safeReadFile(path.join(cwd, (link['from'] as string) || ''));
|
const sourceContent = safeReadFile(path.join(cwd, (link['from'] as string) || ''));
|
||||||
if (!sourceContent) {
|
if (!sourceContent) {
|
||||||
check['detail'] = 'Source file not found';
|
check['detail'] = 'Source file not found (from: must be a relative file path; describe components/endpoints in via:)';
|
||||||
} else if (link['pattern']) {
|
} else if (link['pattern']) {
|
||||||
try {
|
try {
|
||||||
const regex = new RegExp(link['pattern'] as string);
|
const regex = new RegExp(link['pattern'] as string);
|
||||||
|
|||||||
164
tests/bug-967-verify-key-links-strict-paths.test.cjs
Normal file
164
tests/bug-967-verify-key-links-strict-paths.test.cjs
Normal file
@@ -0,0 +1,164 @@
|
|||||||
|
/**
|
||||||
|
* Regression test for bug #967: verify key-links reads from:/to: as literal
|
||||||
|
* relative file paths; the reference docs wrongly implied component/endpoint
|
||||||
|
* values were valid. Fix direction: author-strict — docs corrected to match code.
|
||||||
|
*
|
||||||
|
* Contract pinned here:
|
||||||
|
* 1. from: must be a relative file path; pattern: is evaluated against its content.
|
||||||
|
* 2. from: pointing to a non-existent file → verified:false, detail "Source file not found".
|
||||||
|
* 3. docs/reference/plan-md.md reference example uses a file path for to: (NOT /api/feed).
|
||||||
|
*/
|
||||||
|
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('fs');
|
||||||
|
const path = require('path');
|
||||||
|
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||||
|
|
||||||
|
// ─── helpers ──────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
function writePlanWithKeyLinks(tmpDir, keyLinksYaml) {
|
||||||
|
// parseMustHavesBlock expects 4-space indent for block name, 6-space for items
|
||||||
|
const content = [
|
||||||
|
'---',
|
||||||
|
'phase: 01-test',
|
||||||
|
'plan: 01',
|
||||||
|
'type: execute',
|
||||||
|
'wave: 1',
|
||||||
|
'depends_on: []',
|
||||||
|
'files_modified: [src/a.js]',
|
||||||
|
'autonomous: true',
|
||||||
|
'must_haves:',
|
||||||
|
' key_links:',
|
||||||
|
...keyLinksYaml.map(line => ` ${line}`),
|
||||||
|
'---',
|
||||||
|
'',
|
||||||
|
'<tasks>',
|
||||||
|
'<task type="auto">',
|
||||||
|
' <name>Task 1: Do thing</name>',
|
||||||
|
' <files>src/a.js</files>',
|
||||||
|
' <action>Do it</action>',
|
||||||
|
' <verify><automated>echo ok</automated></verify>',
|
||||||
|
' <done>Done</done>',
|
||||||
|
'</task>',
|
||||||
|
'</tasks>',
|
||||||
|
].join('\n');
|
||||||
|
const planPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-PLAN.md');
|
||||||
|
fs.mkdirSync(path.dirname(planPath), { recursive: true });
|
||||||
|
fs.writeFileSync(planPath, content);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('bug-967 verify key-links strict file-path contract', () => {
|
||||||
|
let tmpDir;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
tmpDir = createTempProject();
|
||||||
|
fs.mkdirSync(path.join(tmpDir, 'src'), { recursive: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup(tmpDir);
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── 1. Happy path: from: is a real file path and pattern: matches ──────────
|
||||||
|
test('verified:true when from: is a relative file path and pattern: matches', () => {
|
||||||
|
writePlanWithKeyLinks(tmpDir, [
|
||||||
|
'- from: "src/component.js"',
|
||||||
|
' to: "src/api/feed.js"',
|
||||||
|
' via: "fetch in useEffect"',
|
||||||
|
' pattern: "fetch.*api/feed"',
|
||||||
|
]);
|
||||||
|
// Create the source file containing the pattern
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(tmpDir, 'src', 'component.js'),
|
||||||
|
"fetch('/api/feed').then(r => r.json());\n",
|
||||||
|
);
|
||||||
|
// Create the target file too (not strictly needed for this path, but realistic)
|
||||||
|
fs.mkdirSync(path.join(tmpDir, 'src', 'api'), { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(tmpDir, 'src', 'api', 'feed.js'), 'module.exports = {};\n');
|
||||||
|
|
||||||
|
const result = runGsdTools(
|
||||||
|
'verify key-links .planning/phases/01-test/01-01-PLAN.md',
|
||||||
|
tmpDir,
|
||||||
|
);
|
||||||
|
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||||
|
|
||||||
|
const output = JSON.parse(result.output);
|
||||||
|
assert.strictEqual(
|
||||||
|
output.all_verified,
|
||||||
|
true,
|
||||||
|
`Expected all_verified:true (file-path from: + matching pattern:). Got: ${JSON.stringify(output)}`,
|
||||||
|
);
|
||||||
|
assert.strictEqual(output.links[0].verified, true);
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── 2. Contract: missing source file → verified:false, explicit detail ─────
|
||||||
|
test('verified:false with "Source file not found" detail when from: file does not exist', () => {
|
||||||
|
writePlanWithKeyLinks(tmpDir, [
|
||||||
|
'- from: "src/missing-file.js"',
|
||||||
|
' to: "src/api/feed.js"',
|
||||||
|
' via: "fetch in useEffect"',
|
||||||
|
' pattern: "fetch.*api/feed"',
|
||||||
|
]);
|
||||||
|
// Deliberately do NOT create src/missing-file.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 output = JSON.parse(result.output);
|
||||||
|
assert.strictEqual(
|
||||||
|
output.links[0].verified,
|
||||||
|
false,
|
||||||
|
`Expected verified:false for absent source file. Got: ${JSON.stringify(output.links[0])}`,
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
output.links[0].detail.includes('Source file not found'),
|
||||||
|
`Expected detail to include "Source file not found". Got: "${output.links[0].detail}"`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── 3. 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".
|
||||||
|
// This test reads the canonical docs file and asserts the example is consistent
|
||||||
|
// with the strict-path contract.
|
||||||
|
//
|
||||||
|
// allow-test-rule: <runtime-contract-is-the-product> the plan-md.md reference
|
||||||
|
// example IS the documented authoring surface for key_links; asserting it uses
|
||||||
|
// a file path (not an endpoint) directly tests the documented contract.
|
||||||
|
test('docs/reference/plan-md.md key_links example uses a relative file path for to:, not an HTTP endpoint', () => {
|
||||||
|
// Locate plan-md.md relative to this test file's repo root
|
||||||
|
const docPath = path.join(__dirname, '..', 'docs', 'reference', 'plan-md.md');
|
||||||
|
assert.ok(fs.existsSync(docPath), `plan-md.md not found at ${docPath}`);
|
||||||
|
const content = fs.readFileSync(docPath, 'utf-8'); // allow-test-rule: <runtime-contract-is-the-product> the plan-md.md reference example IS the documented authoring surface for key_links; asserting it uses a file path (not an endpoint) directly tests the documented contract.
|
||||||
|
|
||||||
|
// Find the key_links block in the annotated example (the first YAML frontmatter fence)
|
||||||
|
// The bad old value was: to: "/api/feed"
|
||||||
|
assert.ok(
|
||||||
|
!content.includes('to: "/api/feed"'),
|
||||||
|
'docs/reference/plan-md.md still contains the endpoint-style to: "/api/feed" — ' +
|
||||||
|
'the reference example must use a relative file path (e.g. "app/api/feed/route.ts") ' +
|
||||||
|
'to match the strict file-path contract.',
|
||||||
|
);
|
||||||
|
|
||||||
|
// Also assert the corrected example actually uses a path-like value
|
||||||
|
// (must contain at least one '/' and not start with 'http')
|
||||||
|
const toMatch = content.match(/key_links:[\s\S]*?to:\s*"([^"]+)"/);
|
||||||
|
assert.ok(
|
||||||
|
toMatch,
|
||||||
|
'Could not find a to: field in the key_links example in plan-md.md',
|
||||||
|
);
|
||||||
|
const toValue = toMatch[1];
|
||||||
|
assert.ok(
|
||||||
|
!toValue.startsWith('/api') && !toValue.startsWith('http'),
|
||||||
|
`to: value in the docs example looks like an HTTP endpoint: "${toValue}". ` +
|
||||||
|
'It must be a relative file path.',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user