golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#2868): resume a phase stranded between its last plan and verification (#3041)

Browse Source 
* fix(#2868): resume a phase stranded between its last plan and verification

discover_and_group_plans exited unconditionally once every plan was filtered
out, conflating "no plan work left" with "phase fully done". Those differ once a
run can be interrupted between the final wave's SUMMARY and the verify step --
most often by a checkpoint plan that is retired but still writes a SUMMARY. The
result was a phase that looked healthy from every index yet had no
VERIFICATION.md, and whose recommended recovery command provably no-opped,
because the only step that produces the artifact sits ten steps past that exit.

The exit is now conditional. When the verification report is genuinely missing
and no filter is active, the run reports the situation by name and continues at
the tail gates instead of stopping.

Two guards keep the normal paths untouched:

- A filtered run (--gaps-only, or an explicit wave) finding nothing left in its
  own slice says nothing about whether the phase as a whole is done, so it exits
  exactly as before. Without this, --wave 1 on a finished first wave would jump
  to verification with later waves still outstanding.
- A phase that already has its report exits as before too.

The recovered path deliberately keeps the code-review and regression gates. The
manual workaround this replaces skipped both, and that gap is the reason a real
route exists rather than telling users to spawn the verifier by hand.

Also acknowledges the emitted growth of the workflow file. As with #2830 it is
appended to the fragment that already owns that path, since the linter hard-fails
when two acknowledgment sources name the same one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#2868): never treat a blocked-and-incomplete phase as finished

Three findings from adversarial review, all fixed.

BLOCKER -- the trigger conflated two different zero-runnable states. This step
now has two skip rules: has_summary (the #2868 target) and, from #2830, a skip
for plans whose blocked_by is non-empty. "All filtered" was therefore reachable
with plans that never ran: plan A halts and is summarized, plan B is blocked by
A and has no summary. The resume path fired, announced "All N plans are
summarized" -- false -- skipped the wave steps so B was never dispatched, and
jumped to the gates. B was silently abandoned, which is the same class of
disappearance #2830 exists to prevent, reintroduced one layer up.

The decision is now an explicit ordered three-way: a filtered run exits
unchanged; any blocked-plan skip reports the phase as stuck on a halt and exits,
routing to resolving the halt rather than to verification; only an
all-summarized, unfiltered phase with a missing report resumes.

MAJOR -- RESUME_TAIL_ONLY was set and never read anywhere in the workflow or its
step fragments. Dead state implying enforcement that did not exist. Removed; the
imperative at the decision point is what actually carries the control flow, so it
now says so plainly.

MAJOR -- the resume path skipped aggregate_results, which is the only step that
runs the secure-phase threats-open gate. A phase with open threats would have
advanced with no warning where a normal run always shows one. The path now
enters at aggregate_results, verified to read only on-disk phase artifacts and
independent queries, so it tolerates having executed no plans this run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#2868): backfill changeset pr number

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-08-04 08:38:31 -04:00
committed by GitHub
parent ef823ca9d9
commit b146b82e3c
5 changed files with 243 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

122
tests/execute-phase-wave.test.cjs
View File

@@ -105,6 +105,128 @@ describe('execute-phase workflow: wave filtering', () => {
});
});
// #2868: a phase whose plans are ALL summarized but which never reached
// verify_phase_goal (most commonly a retired checkpoint plan that still wrote a
// SUMMARY) must resume at the phase gates instead of exiting unconditionally —
// the prior behavior made `code_review_gate`, `regression_gate`, and
// `verify_phase_goal` (the only producer of *-VERIFICATION.md) unreachable.
describe('execute-phase workflow: #2868 stranded-phase resume on discover_and_group_plans', () => {
test('W1: all-filtered outcome is no longer an unconditional exit; it consults verification status', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
assert.ok(
!content.includes('If all filtered: "No matching incomplete plans" → exit.'),
'the old unconditional all-filtered exit line must be gone (#2868)'
);
assert.ok(
content.includes('VERIFY_STATUS'),
'discover_and_group_plans should consult VERIFY_STATUS before exiting on all-filtered'
);
assert.ok(
content.includes('verification status'),
'discover_and_group_plans should call the verification status query'
);
});
test('W2: the resume path names both code_review_gate and regression_gate', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
const discoverIdx = content.indexOf('<step name="discover_and_group_plans">');
const discoverEnd = content.indexOf('</step>', discoverIdx) + '</step>'.length;
assert.ok(discoverIdx >= 0, 'discover_and_group_plans step should exist');
const discoverSection = content.substring(discoverIdx, discoverEnd);
assert.ok(
discoverSection.includes('code_review_gate'),
'discover_and_group_plans should name code_review_gate as the resume target'
);
assert.ok(
discoverSection.includes('regression_gate'),
'discover_and_group_plans should name regression_gate so a future rename breaks this test ' +
'instead of silently orphaning the resume path'
);
});
test('W3: the resume path is gated off when a filter is active (--gaps-only or WAVE_FILTER)', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
const discoverIdx = content.indexOf('<step name="discover_and_group_plans">');
const discoverEnd = content.indexOf('</step>', discoverIdx) + '</step>'.length;
assert.ok(discoverIdx >= 0, 'discover_and_group_plans step should exist');
const discoverSection = content.substring(discoverIdx, discoverEnd);
const filterIdx = discoverSection.indexOf('A filter is active');
assert.ok(filterIdx >= 0, 'discover_and_group_plans should describe a filter-active branch');
// Both flags must be mentioned near the filter-active branch, not merely
// anywhere in the step (e.g. in the pre-existing filtering prose above).
const filterClause = discoverSection.substring(filterIdx, filterIdx + 200);
assert.ok(
filterClause.includes('--gaps-only'),
'filter-active branch should mention --gaps-only'
);
assert.ok(
filterClause.includes('WAVE_FILTER'),
'filter-active branch should mention WAVE_FILTER'
);
});
test('W4: the resume decision is gated on the absence of blocked_by-skipped plans', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
const discoverIdx = content.indexOf('<step name="discover_and_group_plans">');
const discoverEnd = content.indexOf('</step>', discoverIdx) + '</step>'.length;
assert.ok(discoverIdx >= 0, 'discover_and_group_plans step should exist');
const discoverSection = content.substring(discoverIdx, discoverEnd);
// Scope to the resume-decision text specifically (from the "If all filtered" marker
// onward), not the pre-existing #2830 filtering prose above it that already mentions
// blocked_by unconditionally — otherwise this assertion would be vacuous.
const decisionIdx = discoverSection.indexOf('If all filtered');
assert.ok(decisionIdx >= 0, 'discover_and_group_plans should have an all-filtered decision block');
const decisionText = discoverSection.substring(decisionIdx);
assert.ok(
decisionText.includes('blocked_by'),
'the resume-decision text must reference blocked_by so an all-blocked phase is never ' +
'reported as finished (#2868 finding 1)'
);
assert.ok(
/stuck/i.test(decisionText),
'the resume-decision text must call out the blocked-and-incomplete case as stuck, ' +
'distinct from genuinely finished'
);
});
test('W5: the resume path enters at aggregate_results, not code_review_gate', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
const discoverIdx = content.indexOf('<step name="discover_and_group_plans">');
const discoverEnd = content.indexOf('</step>', discoverIdx) + '</step>'.length;
assert.ok(discoverIdx >= 0, 'discover_and_group_plans step should exist');
const discoverSection = content.substring(discoverIdx, discoverEnd);
const continueMatch = discoverSection.match(/continue (?:directly )?at\s+`([a-zA-Z_]+)`/);
assert.ok(continueMatch, 'resume decision should state which step it continues at');
assert.strictEqual(
continueMatch[1],
'aggregate_results',
'the resume path must enter at aggregate_results (the only step running the ' +
'SECURITY_FILE / secure-phase threats-open gate), not code_review_gate — skipping ' +
'aggregate_results silently drops the only security gate (#2868 finding 3)'
);
assert.notStrictEqual(
continueMatch[1],
'code_review_gate',
'resume entry point must not be code_review_gate'
);
});
test('W6: RESUME_TAIL_ONLY (dead, write-only state) must not appear anywhere in the workflow', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
assert.ok(
!content.includes('RESUME_TAIL_ONLY'),
'RESUME_TAIL_ONLY was set but never read anywhere in the workflow or its steps files ' +
'(#2868 finding 2) — remove it; the imperative instruction at the decision point is ' +
'what actually carries control flow'
);
});
});
describe('execute-phase docs: user-facing wave flag', () => {
test('COMMANDS.md documents --wave usage', () => {
const content = fs.readFileSync(COMMANDS_DOC_PATH, 'utf-8');