From 53d7350e0de27860d7290856d72054f0d6bfbeff Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 21 Mar 2026 23:05:56 -0400 Subject: [PATCH] fix: warn when milestone merge contains only metadata and no code (#1906) (#1927) mergeMilestoneToMain now detects when the squash-merge commit contains only .gsd/ metadata files and no actual code changes. The worktree resolver surfaces a clear warning so users know the milestone summary may describe planned work that was never implemented. The complete-milestone prompt now requires the LLM to verify code changes exist on the branch before declaring verification passed. Fixes #1906 Co-authored-by: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/auto-worktree.ts | 25 +++++- .../extensions/gsd/auto/loop-deps.ts | 2 +- .../gsd/prompts/complete-milestone.md | 19 ++--- .../extensions/gsd/tests/auto-loop.test.ts | 2 +- .../auto-worktree-milestone-merge.test.ts | 48 +++++++++++ .../gsd/tests/worktree-resolver.test.ts | 81 ++++++++++++++++++- .../extensions/gsd/worktree-resolver.ts | 39 ++++++--- 7 files changed, 191 insertions(+), 25 deletions(-) diff --git a/src/resources/extensions/gsd/auto-worktree.ts b/src/resources/extensions/gsd/auto-worktree.ts index 0d928fb12..1ee7a4817 100644 --- a/src/resources/extensions/gsd/auto-worktree.ts +++ b/src/resources/extensions/gsd/auto-worktree.ts @@ -970,7 +970,7 @@ export function mergeMilestoneToMain( originalBasePath_: string, milestoneId: string, roadmapContent: string, -): { commitMessage: string; pushed: boolean; prCreated: boolean } { +): { commitMessage: string; pushed: boolean; prCreated: boolean; codeFilesChanged: boolean } { const worktreeCwd = process.cwd(); const milestoneBranch = autoWorktreeBranch(milestoneId); @@ -1187,6 +1187,27 @@ export function mergeMilestoneToMain( } } + // 8c. Detect whether any non-.gsd/ code files were actually merged (#1906). + // When a milestone only produced .gsd/ metadata (summaries, roadmaps) but no + // real code, the user sees "milestone complete" but nothing changed in their + // codebase. Surface this so the caller can warn the user. + let codeFilesChanged = false; + if (!nothingToCommit) { + try { + const mergedFiles = nativeDiffNumstat( + originalBasePath_, + "HEAD~1", + "HEAD", + ); + codeFilesChanged = mergedFiles.some( + (entry) => !entry.path.startsWith(".gsd/"), + ); + } catch { + // If HEAD~1 doesn't exist (first commit), assume code was changed + codeFilesChanged = true; + } + } + // 9. Auto-push if enabled let pushed = false; if (prefs.auto_push === true && !nothingToCommit) { @@ -1282,5 +1303,5 @@ export function mergeMilestoneToMain( originalBase = null; nudgeGitBranchCache(previousCwd); - return { commitMessage, pushed, prCreated }; + return { commitMessage, pushed, prCreated, codeFilesChanged }; } diff --git a/src/resources/extensions/gsd/auto/loop-deps.ts b/src/resources/extensions/gsd/auto/loop-deps.ts index a7ac10bb1..126ed680d 100644 --- a/src/resources/extensions/gsd/auto/loop-deps.ts +++ b/src/resources/extensions/gsd/auto/loop-deps.ts @@ -103,7 +103,7 @@ export interface LoopDeps { basePath: string, milestoneId: string, roadmapContent: string, - ) => { pushed: boolean }; + ) => { pushed: boolean; codeFilesChanged: boolean }; teardownAutoWorktree: (basePath: string, milestoneId: string) => void; createAutoWorktree: (basePath: string, milestoneId: string) => string; captureIntegrationBranch: ( diff --git a/src/resources/extensions/gsd/prompts/complete-milestone.md b/src/resources/extensions/gsd/prompts/complete-milestone.md index 611b372ea..23fc9cfa1 100644 --- a/src/resources/extensions/gsd/prompts/complete-milestone.md +++ b/src/resources/extensions/gsd/prompts/complete-milestone.md @@ -17,16 +17,17 @@ All relevant context has been preloaded below — the roadmap, all slice summari Then: 1. Use the **Milestone Summary** output template from the inlined context above 2. {{skillActivation}} -3. Verify each **success criterion** from the milestone definition in `{{roadmapPath}}`. For each criterion, confirm it was met with specific evidence from slice summaries, test results, or observable behavior. List any criterion that was NOT met. -4. Verify the milestone's **definition of done** — all slices are `[x]`, all slice summaries exist, and any cross-slice integration points work correctly. -5. Validate **requirement status transitions**. For each requirement that changed status during this milestone, confirm the transition is supported by evidence. Requirements can move between Active, Validated, Deferred, Blocked, or Out of Scope — but only with proof. -6. Write `{{milestoneSummaryPath}}` using the milestone-summary template. Fill all frontmatter fields and narrative sections. The `requirement_outcomes` field must list every requirement that changed status with `from_status`, `to_status`, and `proof`. -7. Update `.gsd/REQUIREMENTS.md` if any requirement status transitions were validated in step 5. -8. Update `.gsd/PROJECT.md` to reflect milestone completion and current project state. -9. Review all slice summaries for cross-cutting lessons, patterns, or gotchas that emerged during this milestone. Append any non-obvious, reusable insights to `.gsd/KNOWLEDGE.md`. -10. Do not commit manually — the system auto-commits your changes after this unit completes. +3. **Verify code changes exist.** Run `git diff --stat HEAD $(git merge-base HEAD main) -- ':!.gsd/'` (or the equivalent for the integration branch). If no non-`.gsd/` files appear in the diff, the milestone produced only planning artifacts and no actual code. In that case, do NOT mark the milestone as passing verification — document the gap clearly in the summary and state that implementation is missing. +4. Verify each **success criterion** from the milestone definition in `{{roadmapPath}}`. For each criterion, confirm it was met with specific evidence from slice summaries, test results, or observable behavior. List any criterion that was NOT met. +5. Verify the milestone's **definition of done** — all slices are `[x]`, all slice summaries exist, and any cross-slice integration points work correctly. +6. Validate **requirement status transitions**. For each requirement that changed status during this milestone, confirm the transition is supported by evidence. Requirements can move between Active, Validated, Deferred, Blocked, or Out of Scope — but only with proof. +7. Write `{{milestoneSummaryPath}}` using the milestone-summary template. Fill all frontmatter fields and narrative sections. The `requirement_outcomes` field must list every requirement that changed status with `from_status`, `to_status`, and `proof`. +8. Update `.gsd/REQUIREMENTS.md` if any requirement status transitions were validated in step 5. +9. Update `.gsd/PROJECT.md` to reflect milestone completion and current project state. +10. Review all slice summaries for cross-cutting lessons, patterns, or gotchas that emerged during this milestone. Append any non-obvious, reusable insights to `.gsd/KNOWLEDGE.md`. +11. Do not commit manually — the system auto-commits your changes after this unit completes. -**Important:** Do NOT skip the success criteria and definition of done verification (steps 3-4). The milestone summary must reflect actual verified outcomes, not assumed success. If any criterion was not met, document it clearly in the summary and do not mark the milestone as passing verification. +**Important:** Do NOT skip the code change verification, success criteria, or definition of done verification (steps 3-5). The milestone summary must reflect actual verified outcomes, not assumed success. If any criterion was not met or no code changes exist, document it clearly in the summary and do not mark the milestone as passing verification. **File system safety:** When scanning milestone directories for evidence, use `ls` or `find` to list directory contents first — never pass a directory path (e.g. `tasks/`, `slices/`) directly to the `read` tool. The `read` tool only accepts file paths, not directories. diff --git a/src/resources/extensions/gsd/tests/auto-loop.test.ts b/src/resources/extensions/gsd/tests/auto-loop.test.ts index 9cc2877e5..2b09de863 100644 --- a/src/resources/extensions/gsd/tests/auto-loop.test.ts +++ b/src/resources/extensions/gsd/tests/auto-loop.test.ts @@ -337,7 +337,7 @@ function makeMockDeps( pruneQueueOrder: () => {}, isInAutoWorktree: () => false, shouldUseWorktreeIsolation: () => false, - mergeMilestoneToMain: () => ({ pushed: false }), + mergeMilestoneToMain: () => ({ pushed: false, codeFilesChanged: true }), teardownAutoWorktree: () => {}, createAutoWorktree: () => "/tmp/wt", captureIntegrationBranch: () => {}, diff --git a/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts b/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts index 0bbf0c39d..a2bb897f6 100644 --- a/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts +++ b/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts @@ -723,6 +723,54 @@ async function main(): Promise { ); } + // ─── Test 18: #1906 — codeFilesChanged false when only .gsd/ metadata merged ── + console.log("\n=== #1906: codeFilesChanged=false when only .gsd/ metadata merged ==="); + { + const repo = freshRepo(); + const wtPath = createAutoWorktree(repo, "M180"); + + // Only add .gsd/ metadata files — no actual code + mkdirSync(join(wtPath, ".gsd", "milestones", "M180"), { recursive: true }); + writeFileSync( + join(wtPath, ".gsd", "milestones", "M180", "SUMMARY.md"), + "# M180 Summary\n\nThis milestone was planned but not implemented.\n", + ); + run("git add .", wtPath); + run('git commit -m "chore: add milestone summary"', wtPath); + + const roadmap = makeRoadmap("M180", "Metadata-only milestone", []); + + const result = mergeMilestoneToMain(repo, "M180", roadmap); + assertEq( + result.codeFilesChanged, + false, + "#1906: codeFilesChanged must be false when only .gsd/ files were merged", + ); + } + + // ─── Test 19: #1906 — codeFilesChanged true when real code is merged ── + console.log("\n=== #1906: codeFilesChanged=true when real code is merged ==="); + { + const repo = freshRepo(); + const wtPath = createAutoWorktree(repo, "M190"); + + addSliceToMilestone(repo, wtPath, "M190", "S01", "Real code", [ + { file: "real-code.ts", content: "export const real = true;\n", message: "add real code" }, + ]); + + const roadmap = makeRoadmap("M190", "Code milestone", [ + { id: "S01", title: "Real code" }, + ]); + + const result = mergeMilestoneToMain(repo, "M190", roadmap); + assertEq( + result.codeFilesChanged, + true, + "#1906: codeFilesChanged must be true when real code files were merged", + ); + assertTrue(existsSync(join(repo, "real-code.ts")), "real-code.ts merged to main"); + } + } finally { process.chdir(savedCwd); for (const d of tempDirs) { diff --git a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts index beff8be62..2c4330dfe 100644 --- a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts +++ b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts @@ -52,7 +52,7 @@ function makeDeps( fn: "mergeMilestoneToMain", args: [basePath, milestoneId, roadmapContent], }); - return { pushed: false }; + return { pushed: false, codeFilesChanged: true }; }, syncWorktreeStateBack: ( mainBasePath: string, @@ -424,7 +424,7 @@ test("mergeAndExit in worktree mode shows pushed status", () => { const deps = makeDeps({ isInAutoWorktree: () => true, getIsolationMode: () => "worktree", - mergeMilestoneToMain: () => ({ pushed: true }), + mergeMilestoneToMain: () => ({ pushed: true, codeFilesChanged: true }), }); const ctx = makeNotifyCtx(); const resolver = new WorktreeResolver(s, deps); @@ -659,6 +659,81 @@ test("mergeAndExit in none mode is a no-op", () => { assert.equal(ctx.messages.length, 0); }); +// ─── #1906 — metadata-only merge warning ──────────────────────────────────── + +test("mergeAndExit warns when merge contains no code changes (#1906)", () => { + const s = makeSession({ + basePath: "/project/.gsd/worktrees/M001", + originalBasePath: "/project", + }); + const deps = makeDeps({ + isInAutoWorktree: () => true, + getIsolationMode: () => "worktree", + mergeMilestoneToMain: () => ({ pushed: false, codeFilesChanged: false }), + }); + const ctx = makeNotifyCtx(); + const resolver = new WorktreeResolver(s, deps); + + resolver.mergeAndExit("M001", ctx); + + assert.ok( + ctx.messages.some((m) => m.msg.includes("NO code changes") && m.level === "warning"), + "must emit warning when only .gsd/ metadata was merged", + ); + assert.ok( + !ctx.messages.some((m) => m.msg.includes("merged to main") && m.level === "info"), + "must NOT emit success-style info notification for metadata-only merge", + ); +}); + +test("mergeAndExit emits info when merge contains code changes (#1906)", () => { + const s = makeSession({ + basePath: "/project/.gsd/worktrees/M001", + originalBasePath: "/project", + }); + const deps = makeDeps({ + isInAutoWorktree: () => true, + getIsolationMode: () => "worktree", + mergeMilestoneToMain: () => ({ pushed: false, codeFilesChanged: true }), + }); + const ctx = makeNotifyCtx(); + const resolver = new WorktreeResolver(s, deps); + + resolver.mergeAndExit("M001", ctx); + + assert.ok( + ctx.messages.some((m) => m.msg.includes("merged to main") && m.level === "info"), + "must emit info notification when code files were merged", + ); + assert.ok( + !ctx.messages.some((m) => m.msg.includes("NO code changes")), + "must NOT emit metadata-only warning when code files were merged", + ); +}); + +test("mergeAndExit branch mode warns when merge contains no code changes (#1906)", () => { + const s = makeSession({ + basePath: "/project", + originalBasePath: "/project", + }); + const deps = makeDeps({ + isInAutoWorktree: () => false, + getIsolationMode: () => "branch", + getCurrentBranch: () => "milestone/M001", + autoWorktreeBranch: () => "milestone/M001", + mergeMilestoneToMain: () => ({ pushed: false, codeFilesChanged: false }), + }); + const ctx = makeNotifyCtx(); + const resolver = new WorktreeResolver(s, deps); + + resolver.mergeAndExit("M001", ctx); + + assert.ok( + ctx.messages.some((m) => m.msg.includes("NO code changes") && m.level === "warning"), + "branch mode must emit warning when only .gsd/ metadata was merged", + ); +}); + // ─── mergeAndEnterNext Tests ───────────────────────────────────────────────── test("mergeAndEnterNext calls mergeAndExit then enterMilestone", () => { @@ -677,7 +752,7 @@ test("mergeAndEnterNext calls mergeAndExit then enterMilestone", () => { _roadmap: string, ) => { callOrder.push(`merge:${milestoneId}`); - return { pushed: false }; + return { pushed: false, codeFilesChanged: true }; }, getAutoWorktreePath: () => null, createAutoWorktree: (basePath: string, milestoneId: string) => { diff --git a/src/resources/extensions/gsd/worktree-resolver.ts b/src/resources/extensions/gsd/worktree-resolver.ts index c8ca8c409..4a7723eee 100644 --- a/src/resources/extensions/gsd/worktree-resolver.ts +++ b/src/resources/extensions/gsd/worktree-resolver.ts @@ -28,7 +28,7 @@ export interface WorktreeResolverDeps { basePath: string, milestoneId: string, roadmapContent: string, - ) => { pushed: boolean }; + ) => { pushed: boolean; codeFilesChanged: boolean }; syncWorktreeStateBack: ( mainBasePath: string, worktreePath: string, @@ -371,10 +371,23 @@ export class WorktreeResolver { milestoneId, roadmapContent, ); - ctx.notify( - `Milestone ${milestoneId} merged to main.${mergeResult.pushed ? " Pushed to remote." : ""}`, - "info", - ); + if (mergeResult.codeFilesChanged) { + ctx.notify( + `Milestone ${milestoneId} merged to main.${mergeResult.pushed ? " Pushed to remote." : ""}`, + "info", + ); + } else { + // (#1906) Milestone produced only .gsd/ metadata — no actual code was + // merged. This typically means the LLM wrote planning artifacts + // (summaries, roadmaps) but never implemented the code. Surface this + // clearly so the user knows the milestone is not truly complete. + ctx.notify( + `WARNING: Milestone ${milestoneId} merged to main but contained NO code changes — only .gsd/ metadata files. ` + + `The milestone summary may describe planned work that was never implemented. ` + + `Review the milestone output and re-run if code is missing.`, + "warning", + ); + } } else { // No roadmap at either location — teardown but PRESERVE the branch so // commits are not orphaned. The user can merge manually later (#1573). @@ -478,10 +491,18 @@ export class WorktreeResolver { // Rebuild GitService after merge (branch HEAD changed) this.rebuildGitService(); - ctx.notify( - `Milestone ${milestoneId} merged (branch mode).${mergeResult.pushed ? " Pushed to remote." : ""}`, - "info", - ); + if (mergeResult.codeFilesChanged) { + ctx.notify( + `Milestone ${milestoneId} merged (branch mode).${mergeResult.pushed ? " Pushed to remote." : ""}`, + "info", + ); + } else { + ctx.notify( + `WARNING: Milestone ${milestoneId} merged (branch mode) but contained NO code changes — only .gsd/ metadata. ` + + `Review the milestone output and re-run if code is missing.`, + "warning", + ); + } debugLog("WorktreeResolver", { action: "mergeAndExit", milestoneId,