fix(state): requirements-complete short-circuits the planning ladder
Symptom: dr-repo M003 had all 8 owning requirements (UNI-01..05,
PIL-01..03) marked Status: complete in .sf/REQUIREMENTS.md, but
the milestone row was still active because its only slice was a
post-migration skipped placeholder. After the previous fix routed
all-skipped milestones to pre-planning, SF ran roadmap-meeting +
plan-milestone and wrote 3 new slices on a milestone whose
contract-level work was already done — burned ~4 LLM turns on
plausibly-adjacent but unwanted re-decomposition.
Root cause: deriveStateFromDb's milestone-completion gate consults
only slice statuses (and indirectly the milestone row's own status
field). It never reads REQUIREMENTS.md to check whether the
contract is already satisfied. The slice-based view collapsed the
real signal.
Fix:
- New parseRequirementsByMilestone(content) helper in files.js:
parses REQUIREMENTS.md, groups entries by their `Primary owning
milestone` field, returns Map<id, {complete, incomplete}>.
- handleAllSlicesDone now reads REQUIREMENTS.md before its
slice-based real-work check. If a milestone has at least one
owning requirement and zero of them are incomplete, route to
completing-milestone with nextAction naming the requirement count
(so the operator can see *why* the milestone is being closed
without manually opening REQUIREMENTS.md).
- Best-effort: REQUIREMENTS.md parse failure falls through to the
existing slice-based rule. Missing file likewise — no regression
for projects that don't keep a requirements file.
Resolves sf-mp74hftw-zud6ba filed via the headless feedback CLI.
End-to-end verified by re-running sf headless query on dr-repo
M003: now reports phase=completing-milestone with the right
requirement-count message.
Tests: 5 new cases — all complete + slice skipped → completing,
some active → pre-planning, zero owning requirements falls through,
missing file falls through, all complete + real slice work still
completes. Existing 4 all-skipped-replan cases still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6a2c61d5ee
commit
b08cb13c20
3 changed files with 285 additions and 1 deletions
|
|
@ -478,6 +478,51 @@ export function parseRequirementCounts(content) {
|
|||
counts.active + counts.validated + counts.deferred + counts.outOfScope;
|
||||
return counts;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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).
|
||||
*
|
||||
* "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.
|
||||
*
|
||||
* Format expected (one entry):
|
||||
* ### REQ-01 — short title
|
||||
* - Class: Some class
|
||||
* - Status: complete
|
||||
* - 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 — …`
|
||||
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,
|
||||
);
|
||||
if (!ownerMatch) continue;
|
||||
const owner = ownerMatch[1];
|
||||
const statusMatch = block.match(/^-\s+Status:\s+(\w+)\s*$/im);
|
||||
const status = statusMatch ? statusMatch[1].toLowerCase() : "active";
|
||||
const closed =
|
||||
status === "complete" || status === "done" || status === "skipped";
|
||||
const current = result.get(owner) ?? { complete: 0, incomplete: 0 };
|
||||
if (closed) current.complete += 1;
|
||||
else current.incomplete += 1;
|
||||
result.set(owner, current);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
// ─── Deferred Requirement Parser ──────────────────────────────────────────
|
||||
/**
|
||||
* Parse requirement entries under the "## Deferred" section of REQUIREMENTS.md.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,12 @@
|
|||
|
||||
import { existsSync, readdirSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { loadFile, parseRequirementCounts, parseSummary } from "./files.js";
|
||||
import {
|
||||
loadFile,
|
||||
parseRequirementCounts,
|
||||
parseRequirementsByMilestone,
|
||||
parseSummary,
|
||||
} from "./files.js";
|
||||
import { findMilestoneIds } from "./milestone-ids.js";
|
||||
import {
|
||||
resolveMilestoneFile,
|
||||
|
|
@ -321,6 +326,41 @@ async function handleAllSlicesDone(
|
|||
sliceProgress,
|
||||
activeMilestoneSlices,
|
||||
) {
|
||||
// Requirements-aware completion gate (sf-mp74hftw-zud6ba).
|
||||
// Before the slice-based "real work" check, consult REQUIREMENTS.md:
|
||||
// if every owning requirement for this milestone is closed
|
||||
// (complete/done/skipped), the contract is satisfied regardless of
|
||||
// slice state. Route to completing-milestone (write SUMMARY) instead
|
||||
// of pre-planning, so a milestone whose work was tracked at the
|
||||
// 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) {
|
||||
return {
|
||||
activeMilestone,
|
||||
activeSlice: null,
|
||||
activeTask: null,
|
||||
phase: "completing-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}\`).`,
|
||||
registry,
|
||||
requirements,
|
||||
progress: { milestones: milestoneProgress, slices: sliceProgress },
|
||||
};
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Best-effort — REQUIREMENTS.md parse failure must not break state derivation.
|
||||
}
|
||||
// All-slices-done collapses three quite different states (every
|
||||
// slice complete, every slice skipped, mix of both) into one
|
||||
// "ready to validate" signal. That's wrong when zero slices carry
|
||||
|
|
|
|||
|
|
@ -0,0 +1,199 @@
|
|||
/**
|
||||
* Requirements-aware milestone completion (sf-mp74hftw-zud6ba).
|
||||
*
|
||||
* When every owning requirement for the active milestone is closed,
|
||||
* deriveState should route to completing-milestone instead of
|
||||
* pre-planning, regardless of slice state. This prevents:
|
||||
*
|
||||
* - re-decomposition of milestones whose work was tracked at the
|
||||
* requirements layer rather than via slices
|
||||
* - the autonomous loop burning LLM turns planning a milestone that
|
||||
* REQUIREMENTS.md says is already done
|
||||
*
|
||||
* Verified end-to-end against dr-repo M003 (8 owning requirements
|
||||
* all complete; previously re-planned, now completes).
|
||||
*/
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } 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, reqsContent) {
|
||||
const dir = mkdtempSync(join(tmpdir(), "sf-state-reqs-"));
|
||||
tmpDirs.push(dir);
|
||||
mkdirSync(join(dir, ".sf", "milestones", milestoneId, "slices", "S01"), {
|
||||
recursive: true,
|
||||
});
|
||||
writeFileSync(join(dir, ".sf", "REQUIREMENTS.md"), reqsContent);
|
||||
openDatabase(join(dir, ".sf", "sf.db"));
|
||||
insertMilestone({
|
||||
id: milestoneId,
|
||||
title: "Requirements-aware milestone",
|
||||
status: "active",
|
||||
});
|
||||
return dir;
|
||||
}
|
||||
|
||||
const REQS_ALL_COMPLETE = `# Requirements
|
||||
|
||||
## Validated
|
||||
|
||||
### REQ-01 — first req
|
||||
- Class: Test
|
||||
- Status: complete
|
||||
- Description: x
|
||||
- Primary owning milestone: M050
|
||||
|
||||
### REQ-02 — second req
|
||||
- Class: Test
|
||||
- Status: complete
|
||||
- Description: y
|
||||
- Primary owning milestone: M050
|
||||
`;
|
||||
|
||||
const REQS_MIXED = `# Requirements
|
||||
|
||||
## Validated
|
||||
|
||||
### REQ-01 — first req
|
||||
- Class: Test
|
||||
- Status: complete
|
||||
- Description: x
|
||||
- Primary owning milestone: M051
|
||||
|
||||
## Active
|
||||
|
||||
### REQ-02 — second req
|
||||
- Class: Test
|
||||
- Status: active
|
||||
- Description: y
|
||||
- Primary owning milestone: M051
|
||||
`;
|
||||
|
||||
test("requirements all complete + slice skipped → completing-milestone (not pre-planning)", async () => {
|
||||
const dir = makeProject("M050", REQS_ALL_COMPLETE);
|
||||
insertSlice({
|
||||
milestoneId: "M050",
|
||||
id: "S01",
|
||||
title: "Migration placeholder",
|
||||
status: "skipped",
|
||||
sequence: 1,
|
||||
});
|
||||
|
||||
const state = await deriveState(dir);
|
||||
|
||||
assert.equal(state.activeMilestone?.id, "M050");
|
||||
assert.equal(state.phase, "completing-milestone");
|
||||
assert.match(
|
||||
state.nextAction,
|
||||
/All 2 requirement\(s\) owned by M050 are marked complete/,
|
||||
);
|
||||
});
|
||||
|
||||
test("some requirements still active → pre-planning rule still fires", async () => {
|
||||
const dir = makeProject("M051", REQS_MIXED);
|
||||
insertSlice({
|
||||
milestoneId: "M051",
|
||||
id: "S01",
|
||||
title: "Migration placeholder",
|
||||
status: "skipped",
|
||||
sequence: 1,
|
||||
});
|
||||
|
||||
const state = await deriveState(dir);
|
||||
|
||||
assert.equal(state.phase, "pre-planning");
|
||||
});
|
||||
|
||||
test("zero owning requirements falls through to slice-based check", async () => {
|
||||
const reqsNoOwning = `# Requirements
|
||||
## Validated
|
||||
### REQ-99 — unrelated
|
||||
- Class: Other
|
||||
- Status: complete
|
||||
- Description: not owned by M052
|
||||
- Primary owning milestone: M999
|
||||
`;
|
||||
const dir = makeProject("M052", reqsNoOwning);
|
||||
insertSlice({
|
||||
milestoneId: "M052",
|
||||
id: "S01",
|
||||
title: "Placeholder",
|
||||
status: "skipped",
|
||||
sequence: 1,
|
||||
});
|
||||
|
||||
const state = await deriveState(dir);
|
||||
|
||||
// No owning requirements → can't use requirements gate → slice-based
|
||||
// check fires, routes to pre-planning per the existing skipped rule.
|
||||
assert.equal(state.phase, "pre-planning");
|
||||
});
|
||||
|
||||
test("missing REQUIREMENTS.md doesn't break state derivation", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "sf-state-reqs-"));
|
||||
tmpDirs.push(dir);
|
||||
mkdirSync(join(dir, ".sf", "milestones", "M053", "slices", "S01"), {
|
||||
recursive: true,
|
||||
});
|
||||
openDatabase(join(dir, ".sf", "sf.db"));
|
||||
insertMilestone({
|
||||
id: "M053",
|
||||
title: "No requirements file",
|
||||
status: "active",
|
||||
});
|
||||
insertSlice({
|
||||
milestoneId: "M053",
|
||||
id: "S01",
|
||||
title: "Placeholder",
|
||||
status: "skipped",
|
||||
sequence: 1,
|
||||
});
|
||||
|
||||
const state = await deriveState(dir);
|
||||
|
||||
// No REQUIREMENTS.md → fallback to slice-based rule.
|
||||
assert.equal(state.phase, "pre-planning");
|
||||
});
|
||||
|
||||
test("requirements complete + real slice work → still completing", async () => {
|
||||
// Even when a real slice exists, all-reqs-complete short-circuits
|
||||
// (this path doesn't get to the slice-real-work check).
|
||||
const dir = makeProject("M054", REQS_ALL_COMPLETE.replace(/M050/g, "M054"));
|
||||
insertSlice({
|
||||
milestoneId: "M054",
|
||||
id: "S01",
|
||||
title: "Real",
|
||||
status: "complete",
|
||||
sequence: 1,
|
||||
});
|
||||
|
||||
const state = await deriveState(dir);
|
||||
|
||||
assert.equal(state.phase, "completing-milestone");
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue