From 50a70b35bd473be66604d73075403be2707e00f2 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Sat, 18 Apr 2026 13:44:44 +0200 Subject: [PATCH] fix(sf): auto-mode stuck loop on research dispatch (#4414) Cherry-pick of gsd-build/gsd-2@80ae39ccd adapted for sf/ paths: - auto-artifact-paths: Add PARALLEL-BLOCKER sentinel path for parallel-research unitId - auto-dispatch: Skip re-dispatching parallel-research if PARALLEL-BLOCKER exists - auto-recovery: Add parallel-research verification; clear path/parse caches after writeBlockerPlaceholder - doctor: Downgrade active_requirement_missing_owner to warning (noisy during normal planning) Co-Authored-By: Claude Sonnet 4.6 --- .../extensions/sf/auto-artifact-paths.ts | 13 ++++++ src/resources/extensions/sf/auto-dispatch.ts | 6 +++ src/resources/extensions/sf/auto-recovery.ts | 46 +++++++++++++++++++ src/resources/extensions/sf/doctor.ts | 7 ++- 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/sf/auto-artifact-paths.ts b/src/resources/extensions/sf/auto-artifact-paths.ts index 0d96bc968..dbfc3dc72 100644 --- a/src/resources/extensions/sf/auto-artifact-paths.ts +++ b/src/resources/extensions/sf/auto-artifact-paths.ts @@ -43,6 +43,16 @@ export function resolveExpectedArtifactPath( return dir ? join(dir, buildMilestoneFileName(mid, "ROADMAP")) : null; } case "research-slice": { + // #4414: Sentinel unitId "{mid}/parallel-research" fans out across + // multiple slices. Resolve to a milestone-level placeholder path so + // blocker escalation has somewhere to write. Verification for this + // sentinel is handled directly in verifyExpectedArtifact. + if (sid === "parallel-research") { + const mdir = resolveMilestonePath(base, mid); + return mdir + ? join(mdir, buildMilestoneFileName(mid, "PARALLEL-BLOCKER")) + : null; + } const dir = resolveSlicePath(base, mid, sid!); return dir ? join(dir, buildSliceFileName(sid!, "RESEARCH")) : null; } @@ -109,6 +119,9 @@ export function diagnoseExpectedArtifact( case "plan-milestone": return `${relMilestoneFile(base, mid, "ROADMAP")} (milestone roadmap)`; case "research-slice": + if (sid === "parallel-research") { + return `${relMilestoneFile(base, mid, "PARALLEL-BLOCKER")} (parallel slice research sentinel)`; + } return `${relSliceFile(base, mid, sid!, "RESEARCH")} (slice research)`; case "plan-slice": return `${relSliceFile(base, mid, sid!, "PLAN")} (slice plan)`; diff --git a/src/resources/extensions/sf/auto-dispatch.ts b/src/resources/extensions/sf/auto-dispatch.ts index 075aaa25b..2d12a8bc1 100644 --- a/src/resources/extensions/sf/auto-dispatch.ts +++ b/src/resources/extensions/sf/auto-dispatch.ts @@ -420,6 +420,12 @@ export const DISPATCH_RULES: DispatchRule[] = [ // Only dispatch parallel if 2+ slices are ready if (researchReadySlices.length < 2) return null; + // #4414: If a previous parallel-research attempt escalated to a blocker + // placeholder, skip this rule and fall through to per-slice research + // (or other rules) rather than re-dispatching the same failing unit. + const parallelBlocker = resolveMilestoneFile(basePath, mid, "PARALLEL-BLOCKER"); + if (parallelBlocker) return null; + return { action: "dispatch", unitType: "research-slice", diff --git a/src/resources/extensions/sf/auto-recovery.ts b/src/resources/extensions/sf/auto-recovery.ts index d2d5b7ecb..4f83659b3 100644 --- a/src/resources/extensions/sf/auto-recovery.ts +++ b/src/resources/extensions/sf/auto-recovery.ts @@ -261,6 +261,45 @@ export function verifyExpectedArtifact( return true; } + // #4414: research-slice parallel-research sentinel. The unitId + // `{mid}/parallel-research` is not a real slice — it triggers a single agent + // that fans out research across multiple slices. Verify success by checking + // that every slice which was "research-ready" in the roadmap now has a + // RESEARCH file. Without this, resolveExpectedArtifactPath returns null and + // the retry/escalation machinery silently re-dispatches forever. + // + // NOTE: this predicate mirrors the dispatch rule at + // auto-dispatch.ts parallel-research-slices — keep the two in sync. + if (unitType === "research-slice" && unitId.endsWith("/parallel-research")) { + const { milestone: mid } = parseUnitId(unitId); + if (!mid) return false; + const roadmapFile = resolveMilestoneFile(base, mid, "ROADMAP"); + if (!roadmapFile || !existsSync(roadmapFile)) { + logWarning("recovery", `verify-fail ${unitType} ${unitId}: roadmap missing`); + return false; + } + try { + const roadmap = parseLegacyRoadmap(readFileSync(roadmapFile, "utf-8")); + const milestoneResearchFile = resolveMilestoneFile(base, mid, "RESEARCH"); + for (const slice of roadmap.slices) { + if (slice.done) continue; + if (milestoneResearchFile && slice.id === "S01") continue; + const depsComplete = (slice.depends ?? []).every((depId) => + !!resolveSliceFile(base, mid, depId, "SUMMARY"), + ); + if (!depsComplete) continue; + if (!resolveSliceFile(base, mid, slice.id, "RESEARCH")) { + logWarning("recovery", `verify-fail ${unitType} ${unitId}: slice ${slice.id} missing RESEARCH`); + return false; + } + } + return true; + } catch (err) { + logWarning("recovery", `parallel-research verification failed: ${err instanceof Error ? err.message : String(err)}`); + return false; + } + } + const absPath = resolveExpectedArtifactPath(unitType, unitId, base); // For unit types with no verifiable artifact (null path), the parent directory // is missing on disk — treat as stale completion state so the key gets evicted (#313). @@ -435,6 +474,13 @@ export function writeBlockerPlaceholder( ].join("\n"); writeFileSync(absPath, content, "utf-8"); + // #4414: Clear caches so subsequent dispatch guards (e.g. + // resolveMilestoneFile) see the placeholder file. Without this, the + // cached directory listing is stale and the dispatch rule re-fires, + // producing an infinite loop despite the placeholder being on disk. + clearPathCache(); + clearParseCache(); + // Mark the task/slice as complete in the DB so verifyExpectedArtifact passes. // Without this, the DB status stays "pending" and the dispatch loop // re-derives the same unit indefinitely (#2531, #2653). diff --git a/src/resources/extensions/sf/doctor.ts b/src/resources/extensions/sf/doctor.ts index 32c610cd0..a35d8e72d 100644 --- a/src/resources/extensions/sf/doctor.ts +++ b/src/resources/extensions/sf/doctor.ts @@ -172,8 +172,13 @@ function auditRequirements(content: string | null): DoctorIssue[] { const notes = block.match(/^-\s+Notes:\s+(.+)$/m)?.[1]?.trim().toLowerCase() ?? ""; if (status === "active" && (!owner || owner === "none" || owner === "none yet")) { + // #4414: Downgrade to warning. A newly-created requirement has + // primary_owner='' by default until the planning agent wires it to + // a slice via sf_requirement_update. Flagging as error during normal + // planning is noisy — the real failure is when it persists past + // milestone completion, which is covered by other audits. issues.push({ - severity: "error", + severity: "warning", code: "active_requirement_missing_owner", scope: "project", unitId: requirementId,