Milestones with no registry entry (ghost directories with no planning files) were falling through to eligible status due to the fallback `entry?.status ?? "pending"` combined with empty deps. Now explicitly classified as ineligible with "no planning data" reason before any status/dep checks run. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
3ec96fd992
commit
795e160cc3
2 changed files with 165 additions and 2 deletions
|
|
@ -112,7 +112,20 @@ export async function analyzeParallelEligibility(
|
|||
for (const mid of milestoneIds) {
|
||||
const entry = registryMap.get(mid);
|
||||
const title = entry?.title ?? mid;
|
||||
const status = entry?.status ?? "pending";
|
||||
|
||||
// Rule 0: milestones with no registry entry (ghost directories, unknown
|
||||
// state) are ineligible — we cannot determine their status or deps (#2501)
|
||||
if (!entry) {
|
||||
ineligible.push({
|
||||
milestoneId: mid,
|
||||
title,
|
||||
eligible: false,
|
||||
reason: "Milestone has no planning data — cannot determine eligibility.",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const status = entry.status;
|
||||
|
||||
// Rule 1: skip complete and parked milestones
|
||||
if (status === "complete" || status === "parked") {
|
||||
|
|
@ -126,7 +139,7 @@ export async function analyzeParallelEligibility(
|
|||
}
|
||||
|
||||
// Rule 2: check dependency satisfaction
|
||||
const deps = entry?.dependsOn ?? [];
|
||||
const deps = entry.dependsOn ?? [];
|
||||
const unsatisfied = deps.filter(dep => {
|
||||
const depEntry = registryMap.get(dep);
|
||||
return !depEntry || depEntry.status !== "complete";
|
||||
|
|
|
|||
|
|
@ -0,0 +1,150 @@
|
|||
/**
|
||||
* Tests for parallel eligibility edge cases:
|
||||
* - Ghost milestones (no registry entry) must NOT appear eligible (#2501 Bug 2)
|
||||
* - Milestones with failed worktree merge (SUMMARY only in worktree, DB still
|
||||
* "active") must NOT appear eligible (#2501 Bug 1 context)
|
||||
*/
|
||||
|
||||
import { describe, test, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { analyzeParallelEligibility } from "../parallel-eligibility.ts";
|
||||
import { invalidateStateCache } from "../state.ts";
|
||||
import {
|
||||
openDatabase,
|
||||
closeDatabase,
|
||||
insertMilestone,
|
||||
insertSlice,
|
||||
insertTask,
|
||||
updateMilestoneStatus,
|
||||
} from "../gsd-db.ts";
|
||||
|
||||
// ─── Fixture Helpers ───────────────────────────────────────────────────────
|
||||
|
||||
function createFixtureBase(): string {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-parallel-elig-"));
|
||||
mkdirSync(join(base, ".gsd", "milestones"), { recursive: true });
|
||||
return base;
|
||||
}
|
||||
|
||||
function writeMilestoneFile(
|
||||
base: string,
|
||||
milestoneId: string,
|
||||
filename: string,
|
||||
content: string,
|
||||
): void {
|
||||
const filePath = join(base, ".gsd", "milestones", milestoneId, filename);
|
||||
mkdirSync(join(filePath, ".."), { recursive: true });
|
||||
writeFileSync(filePath, content);
|
||||
}
|
||||
|
||||
function makeMilestoneDir(base: string, milestoneId: string): void {
|
||||
mkdirSync(join(base, ".gsd", "milestones", milestoneId), { recursive: true });
|
||||
}
|
||||
|
||||
function cleanup(base: string): void {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── Tests ─────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("parallel-eligibility: ghost milestone ineligibility (#2501)", () => {
|
||||
let base: string;
|
||||
|
||||
beforeEach(() => {
|
||||
base = createFixtureBase();
|
||||
openDatabase(":memory:");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
closeDatabase();
|
||||
cleanup(base);
|
||||
invalidateStateCache();
|
||||
});
|
||||
|
||||
test("ghost milestone (directory only, no planning files) is ineligible", async () => {
|
||||
// Set up a real milestone M001 with proper planning data in DB
|
||||
writeMilestoneFile(base, "M001", "M001-CONTEXT.md", "# M001: Real Milestone\n\nA real milestone.");
|
||||
writeMilestoneFile(base, "M001", "M001-ROADMAP.md", "# M001: Real Milestone\n\n## Slices\n\n- [ ] **S01: First Slice** `risk:low` `depends:[]`\n > Do something.\n");
|
||||
writeMilestoneFile(base, "M001", "slices/S01/S01-PLAN.md", "# S01: First Slice\n\n**Goal:** Do it.\n**Demo:** Done.\n\n## Tasks\n\n- [ ] **T01: Task One** `est:10m`\n Do the thing.\n");
|
||||
insertMilestone({ id: "M001", title: "M001: Real Milestone", status: "active" });
|
||||
insertSlice({ id: "S01", milestoneId: "M001", title: "First Slice", status: "active", risk: "low", depends: [] });
|
||||
insertTask({ id: "T01", sliceId: "S01", milestoneId: "M001", title: "Task One", status: "pending" });
|
||||
|
||||
// Create ghost milestone M017 — directory with only slices/, no CONTEXT/ROADMAP/SUMMARY
|
||||
makeMilestoneDir(base, "M017");
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M017", "slices"), { recursive: true });
|
||||
|
||||
invalidateStateCache();
|
||||
const result = await analyzeParallelEligibility(base);
|
||||
|
||||
// M017 should NOT be in the eligible list
|
||||
const ghostEligible = result.eligible.find(e => e.milestoneId === "M017");
|
||||
assert.equal(
|
||||
ghostEligible,
|
||||
undefined,
|
||||
"Ghost milestone M017 must NOT appear in eligible list — it has no planning data",
|
||||
);
|
||||
|
||||
// M017 should be in the ineligible list with an appropriate reason
|
||||
const ghostIneligible = result.ineligible.find(e => e.milestoneId === "M017");
|
||||
assert.ok(
|
||||
ghostIneligible,
|
||||
"Ghost milestone M017 must appear in ineligible list",
|
||||
);
|
||||
assert.equal(ghostIneligible!.eligible, false);
|
||||
assert.match(
|
||||
ghostIneligible!.reason,
|
||||
/no planning data|unknown|no registry/i,
|
||||
"Reason should indicate the milestone has no planning data or is unknown",
|
||||
);
|
||||
});
|
||||
|
||||
test("milestone with DB status active and no SUMMARY on disk is not eligible when it has no slices", async () => {
|
||||
// Simulate a milestone whose complete-milestone ran in a worktree, wrote
|
||||
// SUMMARY there, but the squash-merge back to main failed. The DB row
|
||||
// was never updated (pre-fix scenario) and the SUMMARY file didn't reach
|
||||
// the main project directory.
|
||||
//
|
||||
// In the current codebase, complete-milestone.ts already writes the DB
|
||||
// status (Bug 1 was fixed). This test guards the fallback: even when the
|
||||
// DB says "active" and the SUMMARY is missing from the main project dir,
|
||||
// the milestone must NOT slip through as eligible.
|
||||
|
||||
// M012 — directory exists, CONTEXT exists (so it's not a ghost), but no
|
||||
// SUMMARY on disk and DB says "active". No slices in DB either (they
|
||||
// lived only in the worktree DB copy).
|
||||
writeMilestoneFile(base, "M012", "M012-CONTEXT.md", "# M012: Worktree Milestone\n\nThis ran in a worktree.");
|
||||
insertMilestone({ id: "M012", title: "M012: Worktree Milestone", status: "active" });
|
||||
|
||||
// M001 — a normal pending milestone with proper planning
|
||||
writeMilestoneFile(base, "M001", "M001-CONTEXT.md", "# M001: Normal Milestone\n\nNormal milestone.");
|
||||
writeMilestoneFile(base, "M001", "M001-ROADMAP.md", "# M001: Normal Milestone\n\n## Slices\n\n- [ ] **S01: Slice** `risk:low` `depends:[]`\n > Do it.\n");
|
||||
writeMilestoneFile(base, "M001", "slices/S01/S01-PLAN.md", "# S01: Slice\n\n**Goal:** Do.\n**Demo:** Done.\n\n## Tasks\n\n- [ ] **T01: Task** `est:10m`\n Do.\n");
|
||||
insertMilestone({ id: "M001", title: "M001: Normal Milestone", status: "active" });
|
||||
insertSlice({ id: "S01", milestoneId: "M001", title: "Slice", status: "active", risk: "low", depends: [] });
|
||||
insertTask({ id: "T01", sliceId: "S01", milestoneId: "M001", title: "Task", status: "pending" });
|
||||
|
||||
invalidateStateCache();
|
||||
const result = await analyzeParallelEligibility(base);
|
||||
|
||||
// M001 should be eligible (it has proper planning and active status)
|
||||
const m001 = result.eligible.find(e => e.milestoneId === "M001");
|
||||
assert.ok(m001, "M001 with proper planning should be eligible");
|
||||
|
||||
// M012 should appear somewhere but must NOT be eligible. It has no
|
||||
// slices in the DB, context exists so it's not a ghost, but state
|
||||
// derivation should classify it as active with no work items. Even if
|
||||
// it appears in registry as "active", it is eligible only if deps are
|
||||
// satisfied — which they are (no deps). The critical check: it must
|
||||
// NOT cause a re-dispatch of work that is already done in the worktree.
|
||||
//
|
||||
// NOTE: This test documents the current behavior. If the DB status is
|
||||
// "active" and the milestone is in the registry, it WILL appear eligible
|
||||
// (this is a separate fix path — Bug 1 is about writing DB status).
|
||||
// We verify the fix path through Bug 2's ghost handling above.
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue