fix(state): honor completed owning requirements

This commit is contained in:
Mikael Hugo 2026-05-15 18:24:16 +02:00
parent b08cb13c20
commit a863672463
3 changed files with 143 additions and 21 deletions

View file

@ -483,33 +483,38 @@ export function parseRequirementCounts(content) {
* Parse REQUIREMENTS.md and group entries by their `Primary owning
* milestone`, returning a count of complete vs incomplete per milestone.
*
* Used by deriveStateFromDb's milestone-completion gate so a milestone
* whose contract-level requirements are all complete short-circuits
* out of pre-planning even when slice state would otherwise re-route
* (sf-mp74hftw-zud6ba).
* Purpose: support legacy/unmigrated REQUIREMENTS.md projections when the DB
* requirements table is unavailable, without making markdown the primary state
* source.
*
* Consumer: deriveStateFromDb's milestone-completion gate fallback.
*
* "Complete" matches the existing isClosedStatus rule: `complete`,
* `done`, `skipped`. Status case-insensitive. Returns a Map keyed by
* milestone id (string). Milestones with zero owning requirements
* are not present in the map.
*
* Supported owner fields:
* - Primary owning milestone: M003
* - Primary owning slice: M003/S01
*
* Format expected (one entry):
* ### REQ-01 short title
* ### REQ-01 - short title
* - Class: Some class
* - Status: complete
* - Description:
* - Description: ...
* - Primary owning milestone: M003
*/
export function parseRequirementsByMilestone(content) {
const result = new Map();
if (!content) return result;
// Split on entry headers; each block begins at `### XX-NN — …`
// Split on entry headers; each block begins at `### XX-NN - ...`
const blocks = content.split(/(?=^###\s+[A-Z][\w-]*\d+\s+—)/m);
for (const block of blocks) {
if (!block.startsWith("###")) continue;
const ownerMatch = block.match(
/^-\s+Primary owning milestone:\s+(M\d+)\s*$/im,
);
const ownerMatch =
block.match(/^-\s+Primary owning milestone:\s+(M\d+)\s*$/im) ??
block.match(/^-\s+Primary owning slice:\s+(M\d+)(?:\/S\d+)?\s*$/im);
if (!ownerMatch) continue;
const owner = ownerMatch[1];
const statusMatch = block.match(/^-\s+Status:\s+(\w+)\s*$/im);

View file

@ -21,6 +21,7 @@ import {
} from "./paths.js";
import { loadQueueOrder, sortByQueueOrder } from "./queue-order.js";
import {
getActiveRequirements,
getAllMilestones,
getMilestone,
getMilestoneSlices,
@ -47,6 +48,43 @@ import { logWarning } from "./workflow-logger.js";
// isStatusDone replaced by isClosedStatus from status-guards.ts (single source of truth).
// Alias kept for backward compatibility within this file.
const isStatusDone = isClosedStatus;
function milestoneIdFromRequirementOwner(owner) {
const match = String(owner ?? "").match(/^(M\d+)(?:\/S\d+)?$/);
return match?.[1] ?? null;
}
function addRequirementMilestoneCount(result, owner, status) {
const milestoneId = milestoneIdFromRequirementOwner(owner);
if (!milestoneId) return;
const current = result.get(milestoneId) ?? { complete: 0, incomplete: 0 };
if (isClosedStatus(String(status ?? "").toLowerCase())) current.complete += 1;
else current.incomplete += 1;
result.set(milestoneId, current);
}
/**
* Return requirement completion counts keyed by owning milestone.
*
* Purpose: let the milestone state machine honor the DB-backed requirements
* contract before falling back to slice artifacts or markdown projections.
*
* Consumer: handleAllSlicesDone when deciding whether a milestone with only
* skipped/stale slices should complete or re-enter pre-planning.
*/
async function readRequirementsByMilestone(basePath) {
if (isDbAvailable()) {
const result = new Map();
for (const req of getActiveRequirements()) {
addRequirementMilestoneCount(result, req.primary_owner, req.status);
}
if (result.size > 0) return result;
}
const reqsFile = resolveSfRootFile(basePath, "REQUIREMENTS");
const reqsContent = reqsFile ? await loadFile(reqsFile) : null;
return parseRequirementsByMilestone(reqsContent ?? "");
}
/**
* Derive SF state from the milestones/slices/tasks DB tables.
* Non-planning control files (PARKED, CONTINUE, REPLAN, REPLAN-TRIGGER,
@ -335,23 +373,38 @@ async function handleAllSlicesDone(
// requirements layer doesn't get re-decomposed every time its slice
// state changes shape (e.g. someone skips a stale placeholder).
try {
const reqsFile = resolveSfRootFile(basePath, "REQUIREMENTS");
const reqsContent = reqsFile ? await loadFile(reqsFile) : null;
if (reqsContent) {
const reqsByMs = parseRequirementsByMilestone(reqsContent);
const owning = reqsByMs.get(activeMilestone.id);
if (owning && owning.incomplete === 0 && owning.complete > 0) {
const reqsByMs = await readRequirementsByMilestone(basePath);
const owning = reqsByMs.get(activeMilestone.id);
if (owning && owning.incomplete === 0 && owning.complete > 0) {
// Route to validating-milestone, NOT completing-milestone:
// the PDD purpose gate (ADR-0000) requires all 8 PDD fields
// — Purpose, Consumer, Contract, Failure boundary, Evidence,
// Non-goals, Invariants, Assumptions — to be present and
// satisfied before a milestone closes. Requirements-complete
// only covers the Contract field. The validate-milestone
// unit is the one that checks the full PDD; sending the
// milestone there honors the gate while still keeping it
// out of pre-planning's re-decomposition trap.
const { terminal: validationTerminal } =
await readMilestoneValidationVerdict(
basePath,
activeMilestone.id,
loadFile,
);
if (!validationTerminal) {
return {
activeMilestone,
activeSlice: null,
activeTask: null,
phase: "completing-milestone",
phase: "validating-milestone",
recentDecisions: [],
blockers: [],
nextAction:
`All ${owning.complete} requirement(s) owned by ${activeMilestone.id} ` +
`are marked complete in REQUIREMENTS.md. Write milestone summary ` +
`(or close via \`sf headless complete-milestone ${activeMilestone.id}\`).`,
`All ${owning.complete} requirement(s) owned by ` +
`${activeMilestone.id} are marked complete (Contract gate ` +
`satisfied). Run validate-milestone to check the remaining ` +
`PDD fields — Purpose, Consumer, Failure boundary, Evidence, ` +
`Non-goals, Invariants, Assumptions — before closing.`,
registry,
requirements,
progress: { milestones: milestoneProgress, slices: sliceProgress },
@ -359,7 +412,7 @@ async function handleAllSlicesDone(
}
}
} catch {
// Best-effort — REQUIREMENTS.md parse failure must not break state derivation.
// Best-effort: requirements lookup must not break state derivation.
}
// All-slices-done collapses three quite different states (every
// slice complete, every slice skipped, mix of both) into one

View file

@ -27,6 +27,7 @@ import { afterEach, test } from "vitest";
import {
closeDatabase,
insertMilestone,
insertRequirement,
insertSlice,
isDbAvailable,
openDatabase,
@ -148,3 +149,66 @@ test("multiple_skipped_slices_when_deriving_state_returns_pre_planning", async (
assert.equal(state.phase, "pre-planning");
});
test("all_slices_skipped_when_owned_requirements_complete_returns_completing_milestone", async () => {
const dir = makeProject("M505");
insertSlice({
milestoneId: "M505",
id: "S01",
title: "Migration placeholder",
status: "skipped",
sequence: 1,
});
for (const id of ["R501", "R502"]) {
insertRequirement({
id,
class: "runtime",
status: "complete",
description: `${id} done`,
why: "contract satisfied",
source: "test",
primary_owner: "M505/S01",
supporting_slices: "",
validation: "unit test",
notes: "",
full_content: "",
superseded_by: null,
});
}
const state = await deriveState(dir);
assert.equal(state.activeMilestone?.id, "M505");
assert.equal(state.phase, "completing-milestone");
assert.match(state.nextAction, /All 2 requirement\(s\) owned by M505/);
});
test("all_slices_skipped_when_owned_requirement_incomplete_returns_pre_planning", async () => {
const dir = makeProject("M506");
insertSlice({
milestoneId: "M506",
id: "S01",
title: "Migration placeholder",
status: "skipped",
sequence: 1,
});
insertRequirement({
id: "R503",
class: "runtime",
status: "active",
description: "still open",
why: "contract not satisfied",
source: "test",
primary_owner: "M506/S01",
supporting_slices: "",
validation: "",
notes: "",
full_content: "",
superseded_by: null,
});
const state = await deriveState(dir);
assert.equal(state.activeMilestone?.id, "M506");
assert.equal(state.phase, "pre-planning");
});