Milestones were being marked complete with only .gsd/ plan files and zero implementation code. Add hasImplementationArtifacts() that checks git diff against the main branch to verify non-.gsd/ files exist. Applied in both verifyExpectedArtifact (post-unit gate) and the completing-milestone dispatch rule (pre-dispatch guard). Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c68c6331ad
commit
049d432c3c
3 changed files with 230 additions and 0 deletions
|
|
@ -25,6 +25,7 @@ import {
|
|||
} from "./paths.js";
|
||||
import { existsSync, mkdirSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { hasImplementationArtifacts } from "./auto-recovery.js";
|
||||
import {
|
||||
buildResearchMilestonePrompt,
|
||||
buildPlanMilestonePrompt,
|
||||
|
|
@ -543,6 +544,17 @@ const DISPATCH_RULES: DispatchRule[] = [
|
|||
}
|
||||
}
|
||||
|
||||
// Safety guard (#1703): verify the milestone produced implementation
|
||||
// artifacts (non-.gsd/ files). A milestone with only plan files and
|
||||
// zero implementation code should not be marked complete.
|
||||
if (!hasImplementationArtifacts(basePath)) {
|
||||
return {
|
||||
action: "stop",
|
||||
reason: `Cannot complete milestone ${mid}: no implementation files found outside .gsd/. The milestone has only plan files — actual code changes are required.`,
|
||||
level: "error",
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
action: "dispatch",
|
||||
unitType: "complete-milestone",
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ import {
|
|||
writeFileSync,
|
||||
unlinkSync,
|
||||
} from "node:fs";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
// ─── Artifact Resolution & Verification ───────────────────────────────────────
|
||||
|
|
@ -119,6 +120,112 @@ export function resolveExpectedArtifactPath(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a milestone produced implementation artifacts (non-`.gsd/` files)
|
||||
* in the git history. Uses `git log --name-only` to inspect all commits on the
|
||||
* current branch that touch files outside `.gsd/`.
|
||||
*
|
||||
* Returns true if at least one non-`.gsd/` file was committed, false otherwise.
|
||||
* Non-fatal: returns true on git errors to avoid blocking the pipeline when
|
||||
* running outside a git repo (e.g., tests).
|
||||
*/
|
||||
export function hasImplementationArtifacts(basePath: string): boolean {
|
||||
try {
|
||||
// Verify we're in a git repo — fail open if not
|
||||
try {
|
||||
execFileSync("git", ["rev-parse", "--is-inside-work-tree"], {
|
||||
cwd: basePath,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
encoding: "utf-8",
|
||||
});
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Strategy: check `git diff --name-only` against the merge-base with the
|
||||
// main branch. This captures ALL files changed during the milestone's
|
||||
// lifetime. If no merge-base exists (e.g., single-branch workflow), fall
|
||||
// back to checking the last N commits.
|
||||
const mainBranch = detectMainBranch(basePath);
|
||||
const changedFiles = getChangedFilesSinceBranch(basePath, mainBranch);
|
||||
|
||||
// No files changed at all — fail open (could be detached HEAD, single-
|
||||
// commit repo, or other edge case where git diff returns nothing).
|
||||
if (changedFiles.length === 0) return true;
|
||||
|
||||
// Filter out .gsd/ files — only implementation files count.
|
||||
// If every changed file is under .gsd/, the milestone produced no
|
||||
// implementation code (#1703).
|
||||
const implFiles = changedFiles.filter(f => !f.startsWith(".gsd/") && !f.startsWith(".gsd\\"));
|
||||
return implFiles.length > 0;
|
||||
} catch {
|
||||
// Non-fatal — if git operations fail, don't block the pipeline
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect the main/master branch name.
|
||||
*/
|
||||
function detectMainBranch(basePath: string): string {
|
||||
try {
|
||||
const result = execFileSync("git", ["rev-parse", "--verify", "main"], {
|
||||
cwd: basePath,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
encoding: "utf-8",
|
||||
});
|
||||
if (result.trim()) return "main";
|
||||
} catch {
|
||||
// main doesn't exist
|
||||
}
|
||||
try {
|
||||
const result = execFileSync("git", ["rev-parse", "--verify", "master"], {
|
||||
cwd: basePath,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
encoding: "utf-8",
|
||||
});
|
||||
if (result.trim()) return "master";
|
||||
} catch {
|
||||
// master doesn't exist either
|
||||
}
|
||||
return "main"; // default fallback
|
||||
}
|
||||
|
||||
/**
|
||||
* Get files changed since the branch diverged from the target branch.
|
||||
* Falls back to checking HEAD~20 if merge-base detection fails.
|
||||
*/
|
||||
function getChangedFilesSinceBranch(basePath: string, targetBranch: string): string[] {
|
||||
try {
|
||||
// Try merge-base approach first
|
||||
const mergeBase = execFileSync(
|
||||
"git", ["merge-base", targetBranch, "HEAD"],
|
||||
{ cwd: basePath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
|
||||
).trim();
|
||||
|
||||
if (mergeBase) {
|
||||
const result = execFileSync(
|
||||
"git", ["diff", "--name-only", mergeBase, "HEAD"],
|
||||
{ cwd: basePath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
|
||||
).trim();
|
||||
return result ? result.split("\n").filter(Boolean) : [];
|
||||
}
|
||||
} catch {
|
||||
// merge-base failed — fall back
|
||||
}
|
||||
|
||||
// Fallback: check last 20 commits
|
||||
try {
|
||||
const result = execFileSync(
|
||||
"git", ["log", "--name-only", "--pretty=format:", "-20", "HEAD"],
|
||||
{ cwd: basePath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
|
||||
).trim();
|
||||
return result ? [...new Set(result.split("\n").filter(Boolean))] : [];
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether the expected artifact(s) for a unit exist on disk.
|
||||
* Returns true if all required artifacts exist, or if the unit type has no
|
||||
|
|
@ -287,6 +394,13 @@ export function verifyExpectedArtifact(
|
|||
}
|
||||
}
|
||||
|
||||
// complete-milestone must have produced implementation artifacts (#1703).
|
||||
// A milestone with only .gsd/ plan files and zero implementation code is
|
||||
// not genuinely complete — the LLM wrote plan files but skipped actual work.
|
||||
if (unitType === "complete-milestone") {
|
||||
if (!hasImplementationArtifacts(base)) return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
diagnoseExpectedArtifact,
|
||||
buildLoopRemediationSteps,
|
||||
selfHealRuntimeRecords,
|
||||
hasImplementationArtifacts,
|
||||
} from "../auto-recovery.ts";
|
||||
import { parseRoadmap, clearParseCache } from "../files.ts";
|
||||
import { invalidateAllCaches } from "../cache.ts";
|
||||
|
|
@ -484,3 +485,106 @@ test("#793: invalidateAllCaches clears all caches so deriveState sees fresh disk
|
|||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
// ─── hasImplementationArtifacts (#1703) ───────────────────────────────────
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
|
||||
function makeGitBase(): string {
|
||||
const base = join(tmpdir(), `gsd-test-git-${randomUUID()}`);
|
||||
mkdirSync(base, { recursive: true });
|
||||
execFileSync("git", ["init", "--initial-branch=main"], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["config", "user.email", "test@test.com"], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["config", "user.name", "Test"], { cwd: base, stdio: "ignore" });
|
||||
// Create initial commit so HEAD exists
|
||||
writeFileSync(join(base, ".gitkeep"), "");
|
||||
execFileSync("git", ["add", "."], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "initial"], { cwd: base, stdio: "ignore" });
|
||||
return base;
|
||||
}
|
||||
|
||||
test("hasImplementationArtifacts returns false when only .gsd/ files committed (#1703)", () => {
|
||||
const base = makeGitBase();
|
||||
try {
|
||||
// Create a feature branch and commit only .gsd/ files
|
||||
execFileSync("git", ["checkout", "-b", "feat/test-milestone"], { cwd: base, stdio: "ignore" });
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
writeFileSync(join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "# Roadmap");
|
||||
writeFileSync(join(base, ".gsd", "milestones", "M001", "M001-SUMMARY.md"), "# Summary");
|
||||
execFileSync("git", ["add", "."], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "chore: add plan files"], { cwd: base, stdio: "ignore" });
|
||||
|
||||
const result = hasImplementationArtifacts(base);
|
||||
assert.equal(result, false, "should return false when only .gsd/ files were committed");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("hasImplementationArtifacts returns true when implementation files committed (#1703)", () => {
|
||||
const base = makeGitBase();
|
||||
try {
|
||||
// Create a feature branch with both .gsd/ and implementation files
|
||||
execFileSync("git", ["checkout", "-b", "feat/test-impl"], { cwd: base, stdio: "ignore" });
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
writeFileSync(join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "# Roadmap");
|
||||
mkdirSync(join(base, "src"), { recursive: true });
|
||||
writeFileSync(join(base, "src", "feature.ts"), "export function feature() {}");
|
||||
execFileSync("git", ["add", "."], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "feat: add feature"], { cwd: base, stdio: "ignore" });
|
||||
|
||||
const result = hasImplementationArtifacts(base);
|
||||
assert.equal(result, true, "should return true when implementation files are present");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("hasImplementationArtifacts returns true on non-git directory (fail-open)", () => {
|
||||
const base = join(tmpdir(), `gsd-test-nogit-${randomUUID()}`);
|
||||
mkdirSync(base, { recursive: true });
|
||||
try {
|
||||
const result = hasImplementationArtifacts(base);
|
||||
assert.equal(result, true, "should return true (fail-open) in non-git directory");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
// ─── verifyExpectedArtifact: complete-milestone requires impl artifacts (#1703) ──
|
||||
|
||||
test("verifyExpectedArtifact complete-milestone fails with only .gsd/ files (#1703)", () => {
|
||||
const base = makeGitBase();
|
||||
try {
|
||||
// Create feature branch with only .gsd/ files
|
||||
execFileSync("git", ["checkout", "-b", "feat/ms-only-gsd"], { cwd: base, stdio: "ignore" });
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
writeFileSync(join(base, ".gsd", "milestones", "M001", "M001-SUMMARY.md"), "# Milestone Summary\nDone.");
|
||||
execFileSync("git", ["add", "."], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "chore: milestone plan files"], { cwd: base, stdio: "ignore" });
|
||||
|
||||
const result = verifyExpectedArtifact("complete-milestone", "M001", base);
|
||||
assert.equal(result, false, "complete-milestone should fail verification when only .gsd/ files present");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("verifyExpectedArtifact complete-milestone passes with impl files (#1703)", () => {
|
||||
const base = makeGitBase();
|
||||
try {
|
||||
// Create feature branch with implementation files AND milestone summary
|
||||
execFileSync("git", ["checkout", "-b", "feat/ms-with-impl"], { cwd: base, stdio: "ignore" });
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
writeFileSync(join(base, ".gsd", "milestones", "M001", "M001-SUMMARY.md"), "# Milestone Summary\nDone.");
|
||||
mkdirSync(join(base, "src"), { recursive: true });
|
||||
writeFileSync(join(base, "src", "app.ts"), "console.log('hello');");
|
||||
execFileSync("git", ["add", "."], { cwd: base, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "feat: implementation"], { cwd: base, stdio: "ignore" });
|
||||
|
||||
const result = verifyExpectedArtifact("complete-milestone", "M001", base);
|
||||
assert.equal(result, true, "complete-milestone should pass verification with implementation files");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue