fix: address review feedback — indentation, resume state, and regression tests
- Fix auto.ts indentation: properly indent inner if-else chain inside summarizing guard - Clear unitDispatchCount on resume path (paused → active) to prevent stale counts - Add parseSummary regression tests for #91: bare scalar "none" coerced to string[] - Add parseSummary test for missing frontmatter fields yielding empty arrays - Verify .slice().join() works on coerced arrays (the original crash pattern) Test results: 273 passed, 0 failed (24 new assertions) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Lex Christopherson
parent
254c3c8931
commit
3230cd65e9
2 changed files with 189 additions and 94 deletions
|
|
@ -269,6 +269,7 @@ export async function startAuto(
|
|||
stepMode = requestedStepMode;
|
||||
cmdCtx = ctx;
|
||||
basePath = base;
|
||||
unitDispatchCount.clear();
|
||||
// Re-initialize metrics in case ledger was lost during pause
|
||||
if (!getLedger()) initMetrics(base);
|
||||
ctx.ui.setStatus("gsd-auto", stepMode ? "next" : "auto");
|
||||
|
|
@ -1005,105 +1006,105 @@ async function dispatchNextUnit(
|
|||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildCompleteSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
} else {
|
||||
// ── Adaptive Replanning: check if last completed slice needs reassessment ──
|
||||
// Computed here (after summarizing guard) so complete-slice always runs first.
|
||||
const needsReassess = await checkNeedsReassessment(basePath, mid, state);
|
||||
if (needsRunUat) {
|
||||
const { sliceId, uatType } = needsRunUat;
|
||||
unitType = "run-uat";
|
||||
unitId = `${mid}/${sliceId}`;
|
||||
const uatFile = resolveSliceFile(basePath, mid, sliceId, "UAT")!;
|
||||
const uatContent = await loadFile(uatFile);
|
||||
prompt = await buildRunUatPrompt(
|
||||
mid, sliceId, relSliceFile(basePath, mid, sliceId, "UAT"), uatContent ?? "", basePath,
|
||||
);
|
||||
// For non-artifact-driven UAT types, pause after the prompt is dispatched.
|
||||
// The agent receives the prompt, writes S0x-UAT-RESULT.md surfacing the UAT,
|
||||
// then auto-mode pauses for human execution. On resume, result file exists → skip.
|
||||
if (uatType !== "artifact-driven") {
|
||||
pauseAfterUatDispatch = true;
|
||||
}
|
||||
} else if (needsReassess) {
|
||||
unitType = "reassess-roadmap";
|
||||
unitId = `${mid}/${needsReassess.sliceId}`;
|
||||
prompt = await buildReassessRoadmapPrompt(mid, midTitle!, needsReassess.sliceId, basePath);
|
||||
} else if (state.phase === "pre-planning") {
|
||||
// Need roadmap — check if context exists
|
||||
const contextFile = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const hasContext = !!(contextFile && await loadFile(contextFile));
|
||||
// ── Adaptive Replanning: check if last completed slice needs reassessment ──
|
||||
// Computed here (after summarizing guard) so complete-slice always runs first.
|
||||
const needsReassess = await checkNeedsReassessment(basePath, mid, state);
|
||||
if (needsRunUat) {
|
||||
const { sliceId, uatType } = needsRunUat;
|
||||
unitType = "run-uat";
|
||||
unitId = `${mid}/${sliceId}`;
|
||||
const uatFile = resolveSliceFile(basePath, mid, sliceId, "UAT")!;
|
||||
const uatContent = await loadFile(uatFile);
|
||||
prompt = await buildRunUatPrompt(
|
||||
mid, sliceId, relSliceFile(basePath, mid, sliceId, "UAT"), uatContent ?? "", basePath,
|
||||
);
|
||||
// For non-artifact-driven UAT types, pause after the prompt is dispatched.
|
||||
// The agent receives the prompt, writes S0x-UAT-RESULT.md surfacing the UAT,
|
||||
// then auto-mode pauses for human execution. On resume, result file exists → skip.
|
||||
if (uatType !== "artifact-driven") {
|
||||
pauseAfterUatDispatch = true;
|
||||
}
|
||||
} else if (needsReassess) {
|
||||
unitType = "reassess-roadmap";
|
||||
unitId = `${mid}/${needsReassess.sliceId}`;
|
||||
prompt = await buildReassessRoadmapPrompt(mid, midTitle!, needsReassess.sliceId, basePath);
|
||||
} else if (state.phase === "pre-planning") {
|
||||
// Need roadmap — check if context exists
|
||||
const contextFile = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const hasContext = !!(contextFile && await loadFile(contextFile));
|
||||
|
||||
if (!hasContext) {
|
||||
if (!hasContext) {
|
||||
await stopAuto(ctx, pi);
|
||||
ctx.ui.notify("No context or roadmap yet. Run /gsd to discuss first.", "warning");
|
||||
return;
|
||||
}
|
||||
|
||||
// Research before roadmap if no research exists
|
||||
const researchFile = resolveMilestoneFile(basePath, mid, "RESEARCH");
|
||||
const hasResearch = !!(researchFile && await loadFile(researchFile));
|
||||
|
||||
if (!hasResearch) {
|
||||
unitType = "research-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildResearchMilestonePrompt(mid, midTitle!, basePath);
|
||||
} else {
|
||||
unitType = "plan-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildPlanMilestonePrompt(mid, midTitle!, basePath);
|
||||
}
|
||||
|
||||
} else if (state.phase === "planning") {
|
||||
// Slice needs planning — but research first if no research exists
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
const researchFile = resolveSliceFile(basePath, mid, sid, "RESEARCH");
|
||||
const hasResearch = !!(researchFile && await loadFile(researchFile));
|
||||
|
||||
if (!hasResearch) {
|
||||
unitType = "research-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildResearchSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
} else {
|
||||
unitType = "plan-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildPlanSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
}
|
||||
|
||||
} else if (state.phase === "replanning-slice") {
|
||||
// Blocker discovered — replan the slice before continuing
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
unitType = "replan-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildReplanSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
|
||||
} else if (state.phase === "executing" && state.activeTask) {
|
||||
// Execute next task
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
const tid = state.activeTask.id;
|
||||
const tTitle = state.activeTask.title;
|
||||
unitType = "execute-task";
|
||||
unitId = `${mid}/${sid}/${tid}`;
|
||||
prompt = await buildExecuteTaskPrompt(mid, sid, sTitle, tid, tTitle, basePath);
|
||||
|
||||
} else if (state.phase === "completing-milestone") {
|
||||
// All slices done — complete the milestone
|
||||
unitType = "complete-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildCompleteMilestonePrompt(mid, midTitle!, basePath);
|
||||
|
||||
} else {
|
||||
if (currentUnit) {
|
||||
const modelId = ctx.model?.id ?? "unknown";
|
||||
snapshotUnitMetrics(ctx, currentUnit.type, currentUnit.id, currentUnit.startedAt, modelId);
|
||||
saveActivityLog(ctx, basePath, currentUnit.type, currentUnit.id);
|
||||
}
|
||||
await stopAuto(ctx, pi);
|
||||
ctx.ui.notify("No context or roadmap yet. Run /gsd to discuss first.", "warning");
|
||||
ctx.ui.notify(`Unexpected phase: ${state.phase}. Stopping auto-mode.`, "warning");
|
||||
return;
|
||||
}
|
||||
|
||||
// Research before roadmap if no research exists
|
||||
const researchFile = resolveMilestoneFile(basePath, mid, "RESEARCH");
|
||||
const hasResearch = !!(researchFile && await loadFile(researchFile));
|
||||
|
||||
if (!hasResearch) {
|
||||
unitType = "research-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildResearchMilestonePrompt(mid, midTitle!, basePath);
|
||||
} else {
|
||||
unitType = "plan-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildPlanMilestonePrompt(mid, midTitle!, basePath);
|
||||
}
|
||||
|
||||
} else if (state.phase === "planning") {
|
||||
// Slice needs planning — but research first if no research exists
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
const researchFile = resolveSliceFile(basePath, mid, sid, "RESEARCH");
|
||||
const hasResearch = !!(researchFile && await loadFile(researchFile));
|
||||
|
||||
if (!hasResearch) {
|
||||
unitType = "research-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildResearchSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
} else {
|
||||
unitType = "plan-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildPlanSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
}
|
||||
|
||||
} else if (state.phase === "replanning-slice") {
|
||||
// Blocker discovered — replan the slice before continuing
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
unitType = "replan-slice";
|
||||
unitId = `${mid}/${sid}`;
|
||||
prompt = await buildReplanSlicePrompt(mid, midTitle!, sid, sTitle, basePath);
|
||||
|
||||
} else if (state.phase === "executing" && state.activeTask) {
|
||||
// Execute next task
|
||||
const sid = state.activeSlice!.id;
|
||||
const sTitle = state.activeSlice!.title;
|
||||
const tid = state.activeTask.id;
|
||||
const tTitle = state.activeTask.title;
|
||||
unitType = "execute-task";
|
||||
unitId = `${mid}/${sid}/${tid}`;
|
||||
prompt = await buildExecuteTaskPrompt(mid, sid, sTitle, tid, tTitle, basePath);
|
||||
|
||||
} else if (state.phase === "completing-milestone") {
|
||||
// All slices done — complete the milestone
|
||||
unitType = "complete-milestone";
|
||||
unitId = mid;
|
||||
prompt = await buildCompleteMilestonePrompt(mid, midTitle!, basePath);
|
||||
|
||||
} else {
|
||||
if (currentUnit) {
|
||||
const modelId = ctx.model?.id ?? "unknown";
|
||||
snapshotUnitMetrics(ctx, currentUnit.type, currentUnit.id, currentUnit.startedAt, modelId);
|
||||
saveActivityLog(ctx, basePath, currentUnit.type, currentUnit.id);
|
||||
}
|
||||
await stopAuto(ctx, pi);
|
||||
ctx.ui.notify(`Unexpected phase: ${state.phase}. Stopping auto-mode.`, "warning");
|
||||
return;
|
||||
}
|
||||
} // close outer else block (summarizing guard)
|
||||
|
||||
await emitObservabilityWarnings(ctx, unitType, unitId);
|
||||
|
||||
|
|
|
|||
|
|
@ -1248,6 +1248,100 @@ console.log('\n=== parseRequirementCounts: total is sum of all section counts ==
|
|||
assertEq(counts.total, counts.active + counts.validated + counts.deferred + counts.outOfScope, 'total is exact sum');
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// parseSummary: bare scalar frontmatter fields (regression test for #91)
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log('\n=== parseSummary: bare scalar "none" coerced to string array (#91) ===');
|
||||
{
|
||||
const content = `---
|
||||
id: T04
|
||||
parent: S03
|
||||
milestone: M001
|
||||
provides:
|
||||
- iOS rules
|
||||
key_files:
|
||||
- .claude/rules/swift-style.md
|
||||
key_decisions: none
|
||||
patterns_established: none
|
||||
drill_down_paths: none
|
||||
observability_surfaces: none — static reference files
|
||||
affects: single-value
|
||||
---
|
||||
|
||||
# T04: iOS Rules
|
||||
|
||||
**Created iOS-specific rules.**
|
||||
|
||||
## What Happened
|
||||
|
||||
Added rules.
|
||||
|
||||
## Deviations
|
||||
|
||||
None.
|
||||
`;
|
||||
|
||||
const s = parseSummary(content);
|
||||
|
||||
// Array fields should remain arrays
|
||||
assertEq(s.frontmatter.provides.length, 1, '#91: provides array preserved');
|
||||
assertEq(s.frontmatter.provides[0], 'iOS rules', '#91: provides value');
|
||||
assertEq(s.frontmatter.key_files.length, 1, '#91: key_files array preserved');
|
||||
|
||||
// Bare scalar "none" must be coerced to ["none"], not crash
|
||||
assertEq(Array.isArray(s.frontmatter.key_decisions), true, '#91: key_decisions is array');
|
||||
assertEq(s.frontmatter.key_decisions.length, 1, '#91: key_decisions has 1 element');
|
||||
assertEq(s.frontmatter.key_decisions[0], 'none', '#91: key_decisions[0] is "none"');
|
||||
|
||||
assertEq(Array.isArray(s.frontmatter.patterns_established), true, '#91: patterns_established is array');
|
||||
assertEq(s.frontmatter.patterns_established.length, 1, '#91: patterns_established coerced');
|
||||
|
||||
assertEq(Array.isArray(s.frontmatter.drill_down_paths), true, '#91: drill_down_paths is array');
|
||||
assertEq(s.frontmatter.drill_down_paths.length, 1, '#91: drill_down_paths coerced');
|
||||
|
||||
// Scalar with spaces: "none — static reference files"
|
||||
assertEq(Array.isArray(s.frontmatter.observability_surfaces), true, '#91: observability_surfaces is array');
|
||||
assertEq(s.frontmatter.observability_surfaces.length, 1, '#91: observability_surfaces coerced');
|
||||
assertEq(s.frontmatter.observability_surfaces[0], 'none — static reference files', '#91: full scalar preserved');
|
||||
|
||||
// Single value (not "none") also coerced
|
||||
assertEq(Array.isArray(s.frontmatter.affects), true, '#91: affects is array');
|
||||
assertEq(s.frontmatter.affects.length, 1, '#91: affects single value coerced');
|
||||
assertEq(s.frontmatter.affects[0], 'single-value', '#91: affects value');
|
||||
|
||||
// .slice().join() must not crash (the original bug)
|
||||
const decisions = s.frontmatter.key_decisions.slice(0, 2).join('; ');
|
||||
assertEq(decisions, 'none', '#91: .slice().join() works on coerced array');
|
||||
}
|
||||
|
||||
console.log('\n=== parseSummary: missing/empty frontmatter fields yield empty arrays ===');
|
||||
{
|
||||
const content = `---
|
||||
id: T05
|
||||
parent: S04
|
||||
milestone: M001
|
||||
---
|
||||
|
||||
# T05: Minimal Summary
|
||||
|
||||
**Minimal.**
|
||||
|
||||
## What Happened
|
||||
|
||||
Nothing.
|
||||
`;
|
||||
|
||||
const s = parseSummary(content);
|
||||
assertEq(s.frontmatter.provides.length, 0, 'missing provides = empty array');
|
||||
assertEq(s.frontmatter.key_decisions.length, 0, 'missing key_decisions = empty array');
|
||||
assertEq(s.frontmatter.affects.length, 0, 'missing affects = empty array');
|
||||
assertEq(s.frontmatter.key_files.length, 0, 'missing key_files = empty array');
|
||||
assertEq(s.frontmatter.patterns_established.length, 0, 'missing patterns_established = empty array');
|
||||
assertEq(s.frontmatter.drill_down_paths.length, 0, 'missing drill_down_paths = empty array');
|
||||
assertEq(s.frontmatter.observability_surfaces.length, 0, 'missing observability_surfaces = empty array');
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// Results
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue