diff --git a/.changeset/967-verify-key-links-author-strict-docs.md b/.changeset/967-verify-key-links-author-strict-docs.md new file mode 100644 index 000000000..990e6f745 --- /dev/null +++ b/.changeset/967-verify-key-links-author-strict-docs.md @@ -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) diff --git a/agents/gsd-plan-checker.md b/agents/gsd-plan-checker.md index f3b6149fa..4f10662cb 100644 --- a/agents/gsd-plan-checker.md +++ b/agents/gsd-plan-checker.md @@ -707,8 +707,8 @@ must_haves: min_lines: 30 key_links: - from: "src/components/LoginForm.tsx" - to: "/api/auth/login" - via: "fetch in onSubmit" + to: "src/app/api/auth/login/route.ts" + via: "fetch in onSubmit → POST /api/auth/login" ``` Aggregate across plans for full picture of what phase delivers. diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index f456260a7..fc4fc7999 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -632,12 +632,12 @@ must_haves: contains: "model Message" key_links: - from: "src/components/Chat.tsx" - to: "/api/chat" - via: "fetch in useEffect" + to: "src/app/api/chat/route.ts" + via: "fetch in useEffect — calls /api/chat endpoint" pattern: "fetch.*api/chat" - from: "src/app/api/chat/route.ts" - to: "prisma.message" - via: "database query" + to: "prisma/schema.prisma" + via: "database query via prisma.message" pattern: "prisma\\.message\\.(find|create)" ``` diff --git a/agents/gsd-verifier.md b/agents/gsd-verifier.md index 2b2641b4e..3bb38db9d 100644 --- a/agents/gsd-verifier.md +++ b/agents/gsd-verifier.md @@ -136,9 +136,9 @@ must_haves: - path: "src/components/Chat.tsx" provides: "Message list rendering" key_links: - - from: "Chat.tsx" - to: "api/chat" - via: "fetch in useEffect" + - from: "src/components/Chat.tsx" + to: "src/app/api/chat/route.ts" + via: "fetch in useEffect — calls /api/chat endpoint" ``` **Step 2c: Merge must-haves** diff --git a/docs/reference/plan-md.md b/docs/reference/plan-md.md index 1878dcedc..e2de09fbc 100644 --- a/docs/reference/plan-md.md +++ b/docs/reference/plan-md.md @@ -53,8 +53,8 @@ must_haves: exports: ["PostCard"] key_links: - from: "src/components/PostFeed.tsx" - to: "/api/feed" - via: "fetch in useEffect" + to: "src/app/api/feed/route.ts" + via: "fetch in useEffect — calls /api/feed endpoint" pattern: "fetch.*api/feed" --- ``` @@ -92,9 +92,9 @@ must_haves: | `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. | | `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[].to` | string | Target file, endpoint, or module. | -| `key_links[].via` | string | Description of how they connect (e.g. `fetch in useEffect`, `Prisma query`, `import`). | +| `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 (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, 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. | --- diff --git a/gsd-core/templates/phase-prompt.md b/gsd-core/templates/phase-prompt.md index 256a99fcd..bdf9efd6b 100644 --- a/gsd-core/templates/phase-prompt.md +++ b/gsd-core/templates/phase-prompt.md @@ -568,12 +568,12 @@ must_haves: contains: "model Message" key_links: - from: "src/components/Chat.tsx" - to: "/api/chat" - via: "fetch in useEffect" + to: "src/app/api/chat/route.ts" + via: "fetch in useEffect — calls /api/chat endpoint" pattern: "fetch.*api/chat" - from: "src/app/api/chat/route.ts" - to: "prisma.message" - via: "database query" + to: "prisma/schema.prisma" + via: "database query via prisma.message" pattern: "prisma\\.message\\.(find|create)" ``` @@ -589,9 +589,9 @@ must_haves: | `artifacts[].exports` | Optional. Expected exports to verify. | | `artifacts[].contains` | Optional. Pattern that must exist in file. | | `key_links` | Critical connections between artifacts. | -| `key_links[].from` | Source artifact. | -| `key_links[].to` | Target artifact or endpoint. | -| `key_links[].via` | How they connect (description). | +| `key_links[].from` | Source file (relative path from project root). Describe components or symbols in `via:`. | +| `key_links[].to` | Target file (relative path from project root). Describe endpoints, APIs, or modules in `via:`. | +| `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. | **Why this matters:** diff --git a/scripts/lint-regression-test-names.allowlist.json b/scripts/lint-regression-test-names.allowlist.json index ec95d452c..941ff4681 100644 --- a/scripts/lint-regression-test-names.allowlist.json +++ b/scripts/lint-regression-test-names.allowlist.json @@ -260,6 +260,7 @@ "bug-947-hermes-gsd-prefix.test.cjs", "bug-948-state-noop-write-guard.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-978-milestone-complete-force.test.cjs" ] diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index ea2ebaa20..66ef758c8 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -116,6 +116,7 @@ "bug-2994-verify-reapply-patches-installed-path.test.cjs", "bug-3381-verify-work-workstream.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-mvp-uat.test.cjs", "verify-npm-publish.test.cjs", diff --git a/src/verify.cts b/src/verify.cts index b68a1b814..96ab08575 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -453,7 +453,7 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi const sourceContent = safeReadFile(path.join(cwd, (link['from'] as string) || '')); 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']) { try { const regex = new RegExp(link['pattern'] as string); diff --git a/tests/bug-967-verify-key-links-strict-paths.test.cjs b/tests/bug-967-verify-key-links-strict-paths.test.cjs new file mode 100644 index 000000000..4abc4de89 --- /dev/null +++ b/tests/bug-967-verify-key-links-strict-paths.test.cjs @@ -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}`), + '---', + '', + '', + '', + ' Task 1: Do thing', + ' src/a.js', + ' Do it', + ' echo ok', + ' Done', + '', + '', + ].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: 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: 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.', + ); + }); +});