diff --git a/src/resources/extensions/sf/state-db.js b/src/resources/extensions/sf/state-db.js index a40b9cecc..9eaff79e1 100644 --- a/src/resources/extensions/sf/state-db.js +++ b/src/resources/extensions/sf/state-db.js @@ -6,7 +6,15 @@ import { existsSync, readdirSync } from "node:fs"; import { join } from "node:path"; import { loadFile, parseRequirementCounts, parseSummary } from "./files.js"; import { findMilestoneIds } from "./milestone-ids.js"; -import { resolveMilestoneFile, resolveSfRootFile, resolveSliceFile, resolveSlicePath, resolveTaskFile, resolveTasksDir } from "./paths.js"; +import { + resolveMilestoneFile, + resolveSfRootFile, + resolveSliceFile, + resolveSlicePath, + resolveTaskFile, + resolveTasksDir, +} from "./paths.js"; +import { loadQueueOrder, sortByQueueOrder } from "./queue-order.js"; import { getAllMilestones, getMilestone, @@ -18,9 +26,6 @@ import { getSliceTasks, isDbAvailable, } from "./sf-db.js"; -import { isClosedStatus, isDeferredStatus } from "./status-guards.js"; -import { loadQueueOrder, sortByQueueOrder } from "./queue-order.js"; -import { logWarning } from "./workflow-logger.js"; import { canonicalMilestonePrefix, extractContextTitle, @@ -30,6 +35,8 @@ import { readMilestoneValidationVerdict, stripMilestonePrefix, } from "./state-shared.js"; +import { isClosedStatus, isDeferredStatus } from "./status-guards.js"; +import { logWarning } from "./workflow-logger.js"; // ─── DB-backed State Derivation ──────────────────────────────────────────── // isStatusDone replaced by isClosedStatus from status-guards.ts (single source of truth). @@ -328,6 +335,18 @@ async function handleAllSlicesDone( ) : true; // fall back to old behaviour if caller didn't pass slices if (!hasRealWork) { + // Route into the pre-planning ladder. The dispatcher decides: + // + // no CONTEXT file -> discuss-milestone + // has CONTEXT, no RESEARCH -> research-milestone + // has both -> plan-milestone (which decomposes into real + // slices alongside the existing skipped ones) + // + // The milestone's vision/PDD is already in the DB (set when + // the milestone was created via `sf new-milestone`), so the + // planner has the purpose it needs. No operator action + // required - autonomous mode will pick it up on the next + // dispatch tick. return { activeMilestone, activeSlice: null, @@ -336,11 +355,13 @@ async function handleAllSlicesDone( recentDecisions: [], blockers: [], nextAction: - `No slice on milestone ${activeMilestone.id} carries real work ` + - `(every slice is skipped or none were ever planned). Run ` + - `reassess-roadmap or close the milestone explicitly via ` + - `\`sf headless complete-milestone ${activeMilestone.id}\` if the ` + - `work is intentionally deferred.`, + `Milestone ${activeMilestone.id} has no slice carrying real ` + + `work (every slice is skipped). Route into the pre-planning ` + + `ladder so the dispatcher picks discuss/research/plan based ` + + `on which artifacts are missing. Use ` + + `\`sf headless complete-milestone ${activeMilestone.id}\` ` + + `only if you want to intentionally defer the work without ` + + `planning it.`, registry, requirements, progress: { milestones: milestoneProgress, slices: sliceProgress }, diff --git a/src/resources/extensions/sf/tests/state-all-skipped-replan.test.mjs b/src/resources/extensions/sf/tests/state-all-skipped-replan.test.mjs new file mode 100644 index 000000000..5adf40da6 --- /dev/null +++ b/src/resources/extensions/sf/tests/state-all-skipped-replan.test.mjs @@ -0,0 +1,150 @@ +/** + * deriveState: when every slice on the active milestone was *skipped* + * (not completed), route into the pre-planning ladder instead of the + * validating-milestone phase. + * + * Background: the original handleAllSlicesDone branch treated + * isStatusDone uniformly - "complete", "done", and "skipped" all + * counted as "the milestone has finished its work". That collapsed + * the migration-placeholder pattern (skip the stub slice -> milestone + * advances to validation) with the genuinely-done case. The fix + * differentiates them: zero slices in {complete, done} -> pre-planning + * so the dispatcher's discuss -> research -> plan ladder takes over. + * + * This pins: + * - all-skipped milestone routes to pre-planning, not validating + * - at least one complete/done slice keeps the validating-milestone + * behaviour (no regression for the happy path) + * - the nextAction names the dispatcher path so an operator + * reading the snapshot knows what'll happen autonomously + */ + +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, test } from "vitest"; +import { + closeDatabase, + insertMilestone, + insertSlice, + isDbAvailable, + openDatabase, +} from "../sf-db.js"; +import { deriveState, invalidateStateCache } from "../state.js"; + +const tmpDirs = []; + +afterEach(() => { + closeDatabase(); + if (!isDbAvailable()) { + openDatabase(":memory:"); + closeDatabase(); + } + invalidateStateCache(); + while (tmpDirs.length > 0) { + const dir = tmpDirs.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } +}); + +function makeProject(milestoneId) { + const dir = mkdtempSync(join(tmpdir(), "sf-state-allskipped-")); + tmpDirs.push(dir); + mkdirSync(join(dir, ".sf", "milestones", milestoneId, "slices", "S01"), { + recursive: true, + }); + openDatabase(join(dir, ".sf", "sf.db")); + insertMilestone({ + id: milestoneId, + title: "All-skipped milestone", + status: "active", + }); + return dir; +} + +test("all_slices_skipped_when_deriving_state_returns_pre_planning", async () => { + const dir = makeProject("M501"); + insertSlice({ + milestoneId: "M501", + id: "S01", + title: "Migration placeholder", + status: "skipped", + sequence: 1, + }); + + const state = await deriveState(dir); + + assert.equal(state.activeMilestone?.id, "M501"); + assert.equal(state.phase, "pre-planning"); + // activeSlice is intentionally null - pre-planning needs the + // dispatcher to pick the next unit type from artifacts. + assert.equal(state.activeSlice, null); + // nextAction must surface the autonomous path so operators know + // SF will plan on its own. + assert.match( + state.nextAction, + /pre-planning ladder|discuss\/research\/plan|every slice is skipped/i, + ); +}); + +test("complete_slice_when_deriving_state_returns_validating_milestone", async () => { + const dir = makeProject("M502"); + insertSlice({ + milestoneId: "M502", + id: "S01", + title: "Real work", + status: "complete", + sequence: 1, + }); + insertSlice({ + milestoneId: "M502", + id: "S02", + title: "Migration placeholder", + status: "skipped", + sequence: 2, + }); + + const state = await deriveState(dir); + + assert.equal(state.activeMilestone?.id, "M502"); + assert.equal(state.phase, "validating-milestone"); +}); + +test("done_slice_when_deriving_state_returns_validating_milestone", async () => { + const dir = makeProject("M503"); + insertSlice({ + milestoneId: "M503", + id: "S01", + title: "Legacy-style done slice", + status: "done", + sequence: 1, + }); + + const state = await deriveState(dir); + + assert.equal(state.activeMilestone?.id, "M503"); + assert.equal(state.phase, "validating-milestone"); +}); + +test("multiple_skipped_slices_when_deriving_state_returns_pre_planning", async () => { + const dir = makeProject("M504"); + insertSlice({ + milestoneId: "M504", + id: "S01", + title: "First placeholder", + status: "skipped", + sequence: 1, + }); + insertSlice({ + milestoneId: "M504", + id: "S02", + title: "Second placeholder", + status: "skipped", + sequence: 2, + }); + + const state = await deriveState(dir); + + assert.equal(state.phase, "pre-planning"); +});