From b26dca40ecb974ab92bcc223944f2c17fa02261e Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Thu, 30 Apr 2026 13:34:24 +0200 Subject: [PATCH] fix: Stop milestone completion git archaeology --- .../sf/prompts/complete-milestone.md | 4 ++-- .../extensions/sf/session-forensics.ts | 7 ++++++ .../sf/tests/complete-milestone.test.ts | 24 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/resources/extensions/sf/prompts/complete-milestone.md b/src/resources/extensions/sf/prompts/complete-milestone.md index 736c35b0b..dc1f7ab2b 100644 --- a/src/resources/extensions/sf/prompts/complete-milestone.md +++ b/src/resources/extensions/sf/prompts/complete-milestone.md @@ -18,7 +18,7 @@ Then: 1. Use the **Milestone Summary** output template from the inlined context above 2. Inspect the inlined or linked **MILESTONE VALIDATION** verdict. If the verdict is not `pass` (including `needs-attention` or `needs-remediation`), record it as a **verification failure** and follow the failure path. Do NOT call `sf_complete_milestone` for a non-pass validation verdict. 3. {{skillActivation}} -4. **Verify code changes exist.** Run `git diff --stat HEAD $(git merge-base HEAD main) -- ':!.sf/'` (or the equivalent for the integration branch). If no non-`.sf/` files appear in the diff, the milestone produced only planning artifacts and no actual code. Record this as a **verification failure**. +4. **Verify implementation evidence exists.** Use the inlined validation verdict and slice summaries as the primary proof that implementation happened. If a slice summary lists non-`.sf/` key files, accepted verification commands, or committed work, that satisfies code-change verification for already-integrated milestone work. Run only `git status --short` to check for unresolved local changes. Do **not** inspect git history, compute merge bases, or run branch-diff archaeology unless the validation verdict and slice summaries are missing or contradictory. If no slice summary or validation evidence names implementation files, record this as a **verification failure**. 5. 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. Record any criterion that was NOT met as a **verification failure**. 6. Verify the milestone's **definition of done** — all slices are `[x]`, all slice summaries exist, and any cross-slice integration points work correctly. Record any unmet items as a **verification failure**. 7. If the roadmap includes a **Horizontal Checklist**, verify each item was addressed during the milestone. Note unchecked items in the milestone summary. @@ -50,7 +50,7 @@ If work falls into the second bucket, do not fail the milestone just because it **Success path** (all verifications passed — continue with steps 10–14): 10. For each requirement whose status changed in step 9, call `sf_requirement_update` with the requirement ID and updated `status` and `validation` fields — the tool regenerates `.sf/REQUIREMENTS.md` automatically. Do this BEFORE completing the milestone so requirement updates are persisted. -11. **Persist completion through `sf_complete_milestone`.** Call it with the parameters below. The tool updates the milestone status in the DB, renders `{{milestoneSummaryPath}}`, and validates all slices are complete before proceeding. +11. **Persist completion through `sf_complete_milestone`.** Call it with the parameters below as soon as steps 2, 4, 5, 6, and 10 are satisfied. Do not keep reading historical commits or re-running broad test suites after the requirements are updated. The tool updates the milestone status in the DB, renders `{{milestoneSummaryPath}}`, and validates all slices are complete before proceeding. **Required parameters:** - `milestoneId` (string) — Milestone ID (e.g. M001) diff --git a/src/resources/extensions/sf/session-forensics.ts b/src/resources/extensions/sf/session-forensics.ts index 732dbd28c..601c2cc29 100644 --- a/src/resources/extensions/sf/session-forensics.ts +++ b/src/resources/extensions/sf/session-forensics.ts @@ -449,6 +449,13 @@ function formatRecoveryPrompt( "4. Run the narrowest verification command that proves the resumed work.", "5. Complete the unit, or explicitly report the blocker with the exact remaining file/command.", ); + if (unitType === "complete-milestone") { + sections.push( + "", + "### Complete-Milestone Recovery Rule", + "For milestone closeout, do not inspect git history or compute merge-base diffs on resume. Use the existing validation verdict, slice summaries, and already-run requirement updates. If validation passed and all slices are complete, call `sf_complete_milestone` with a concise summary instead of continuing branch archaeology.", + ); + } return sections.join("\n"); } diff --git a/src/resources/extensions/sf/tests/complete-milestone.test.ts b/src/resources/extensions/sf/tests/complete-milestone.test.ts index 7c6cded9e..232b16dbc 100644 --- a/src/resources/extensions/sf/tests/complete-milestone.test.ts +++ b/src/resources/extensions/sf/tests/complete-milestone.test.ts @@ -205,6 +205,30 @@ describe("complete-milestone", () => { ); }); + test("prompt avoids branch-diff archaeology for already-integrated milestone work", () => { + const prompt = loadPromptFromWorktree("complete-milestone", { + workingDirectory: "/tmp/test-project", + milestoneId: "M001", + milestoneTitle: "Integrated Work Test", + roadmapPath: ".sf/milestones/M001/M001-ROADMAP.md", + inlinedContext: "context", + }); + + assert.ok( + prompt.includes("Use the inlined validation verdict and slice summaries"), + "prompt should use inlined validation and slice summaries as implementation proof", + ); + assert.ok( + /Do\s+\*\*not\*\*\s+inspect git history/.test(prompt) || + prompt.includes("Do not inspect git history"), + "prompt should explicitly forbid git-history archaeology", + ); + assert.ok( + !prompt.includes("git diff --stat HEAD $(git merge-base HEAD main)"), + "prompt must not use a merge-base diff that is empty on integration branch", + ); + }); + test("prompt treats non-pass validation verdict as a completion blocker", () => { const prompt = loadPromptFromWorktree("complete-milestone", { workingDirectory: "/tmp/test-project",