Merge pull request #3921 from jeremymcs/fix/3915-manifest-skipped-slices

fix(gsd): avoid false manifest and skipped-slice warnings
This commit is contained in:
Jeremy McSpadden 2026-04-10 07:02:29 -05:00 committed by GitHub
commit 2ee5cc6c98
4 changed files with 65 additions and 18 deletions

View file

@ -8,6 +8,7 @@ import { resolveMilestoneFile, resolveMilestonePath, resolveSliceFile, resolveSl
import { deriveState, isMilestoneComplete } from "./state.js";
import { invalidateAllCaches } from "./cache.js";
import { loadEffectiveGSDPreferences, type GSDPreferences } from "./preferences.js";
import { isClosedStatus } from "./status-guards.js";
import type { DoctorIssue, DoctorIssueCode, DoctorReport } from "./doctor-types.js";
import { GLOBAL_STATE_CODES } from "./doctor-types.js";
@ -474,15 +475,16 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
if (!roadmapContent) continue;
// Normalize slices: prefer DB, fall back to parser
type NormSlice = RoadmapSliceEntry & { pending?: boolean };
type NormSlice = RoadmapSliceEntry & { pending?: boolean; skipped?: boolean };
let slices: NormSlice[];
if (isDbAvailable()) {
const dbSlices = getMilestoneSlices(milestoneId);
slices = dbSlices.map(s => ({
id: s.id,
title: s.title,
done: s.status === "complete",
done: isClosedStatus(s.status),
pending: s.status === "pending",
skipped: s.status === "skipped",
risk: (s.risk || "medium") as RoadmapSliceEntry["risk"],
depends: s.depends,
demo: s.demo,
@ -578,8 +580,9 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
const slicePath = resolveSlicePath(basePath, milestoneId, slice.id);
if (!slicePath) {
// Pending slices haven't been planned yet — directories are created
// lazily by ensurePreconditions() at dispatch time. Skip them.
if (slice.pending) continue;
// lazily by ensurePreconditions() at dispatch time. Skipped slices are
// intentionally allowed to remain summary-less and directory-less.
if (slice.pending || slice.skipped) continue;
const expectedPath = relSlicePath(basePath, milestoneId, slice.id);
issues.push({
severity: slice.done ? "warning" : "error",
@ -603,7 +606,8 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
const tasksDir = resolveTasksDir(basePath, milestoneId, slice.id);
if (!tasksDir) {
// Pending slices haven't been planned yet — tasks/ is created on demand.
if (slice.pending) continue;
// Skipped slices may legitimately never create tasks/.
if (slice.pending || slice.skipped) continue;
issues.push({
severity: slice.done ? "warning" : "error",
code: "missing_tasks_dir",

View file

@ -215,17 +215,9 @@ export function checkAutoStartAfterDiscuss(): boolean {
// Gate 4: Discussion manifest process verification (multi-milestone only)
// The LLM writes DISCUSSION-MANIFEST.json after each Phase 3 gate decision.
// If the project is multi-milestone, the manifest is required. When it is
// missing, fail closed instead of assuming the discussion finished.
// When it exists, validate it before auto-starting. Project history alone is
// not a reliable signal for the current discussion mode.
const manifestPath = join(gsdRoot(basePath), "DISCUSSION-MANIFEST.json");
const requiresManifest = projectIds.length > 1 || findMilestoneIds(basePath).length > 1;
if (requiresManifest && !existsSync(manifestPath)) {
ctx.ui.notify(
"Multi-milestone discussion manifest is missing. Auto-start will remain paused until the manifest is written.",
"warning",
);
return false;
}
if (existsSync(manifestPath)) {
try {
const manifest = JSON.parse(readFileSync(manifestPath, "utf-8"));

View file

@ -100,7 +100,7 @@ describe("#2985 Bug 4 — getDiscussionMilestoneId must be keyed by basePath", (
});
});
test("checkAutoStartAfterDiscuss fails closed when a multi-milestone manifest is missing", () => {
test("checkAutoStartAfterDiscuss ignores missing manifest for single-milestone discuss on established project", () => {
const base = mkdtempSync(join(tmpdir(), "gsd-auto-start-manifest-"));
try {
const gsdDir = join(base, ".gsd");
@ -123,7 +123,7 @@ test("checkAutoStartAfterDiscuss fails closed when a multi-milestone manifest is
});
const started = checkAutoStartAfterDiscuss();
assert.equal(started, false, "auto-start should fail closed without the manifest");
assert.equal(started, true, "project history alone should not require a manifest");
} finally {
clearPendingAutoStart();
rmSync(base, { recursive: true, force: true });

View file

@ -15,7 +15,7 @@ import { tmpdir } from "node:os";
import test from "node:test";
import assert from "node:assert/strict";
import { runGSDDoctor } from "../../doctor.ts";
import { closeDatabase } from "../../gsd-db.ts";
import { closeDatabase, insertMilestone, insertSlice, openDatabase } from "../../gsd-db.ts";
function makeTmp(name: string): string {
const dir = join(tmpdir(), `doctor-fixlevel-${name}-${Date.now()}-${Math.random().toString(36).slice(2)}`);
@ -177,6 +177,57 @@ test("legacy roadmap fallback: future slices are treated as pending, active slic
);
});
test("db skipped slices do not report missing directories", async (t) => {
const tmp = makeTmp("skipped-slice-dir");
t.after(() => {
try { closeDatabase(); } catch { /* noop */ }
rmSync(tmp, { recursive: true, force: true });
});
const gsd = join(tmp, ".gsd");
const m = join(gsd, "milestones", "M001");
mkdirSync(m, { recursive: true });
writeFileSync(join(m, "M001-ROADMAP.md"), `# M001: Test
## Slices
- [ ] **S05: Skipped Slice** \`risk:low\` \`depends:[]\`
> Intentionally skipped
`);
openDatabase(join(gsd, "gsd.db"));
insertMilestone({ id: "M001", title: "Test", status: "active" });
insertSlice({ id: "S05", milestoneId: "M001", title: "Skipped Slice", status: "skipped", sequence: 5 });
const report = await runGSDDoctor(tmp, { scope: "M001" });
const missingDirIssues = report.issues.filter(
i =>
(i.code === "missing_slice_dir" || i.code === "missing_tasks_dir") &&
i.unitId === "M001/S05",
);
assert.deepStrictEqual(
missingDirIssues,
[],
"skipped slices should not require slice or tasks directories",
);
});
test("doctor source treats skipped DB slices as closed and directory-optional", () => {
const doctorSource = readFileSync(join(process.cwd(), "src/resources/extensions/gsd/doctor.ts"), "utf8");
assert.match(
doctorSource,
/done:\s*isClosedStatus\(s\.status\)/,
"doctor should normalize skipped DB slices through isClosedStatus()",
);
assert.match(
doctorSource,
/if \(slice\.pending \|\| slice\.skipped\) continue;/,
"doctor should skip missing-directory checks for skipped slices",
);
});
test("fixLevel:all — delimiter_in_title still fixable", async (t) => {
const tmp = makeTmp("delimiter-fix");
t.after(() => rmSync(tmp, { recursive: true, force: true }));