* test(#3233): zero plans (0/0) is a no-op; plans-but-none-done still writes 0% cmdStateUpdateProgress mapped 0/0 through clampPercent to 0% and rewrote the shipped Progress record after milestone close. Replace the stale 'handles zero plans gracefully' test (which asserted the buggy percent:0) with a #3233 no-op regression (100% record preserved, updated:false), and add a negative-space guard: plans exist but none done must still write a legitimate 0%. RED — fails on next; fix follows. * fix(#3233): no-op state update-progress when the milestone scan finds zero plans cmdStateUpdateProgress mapped 0/0 through clampPercent to 0% and unconditionally rewrote the body Progress line, so after /gsd-complete-milestone archived the phases (.planning/phases/ empty, scope COMPLETE) a routine update-progress run destroyed the shipped record ([██████████] 100% → [░░░░░░░░░░] 0%). Add an early-return no-op when totalPlans === 0 — mirroring the established scope-withholding no-op (stderr WARNING + {updated:false, reason}) and computeProgressPercent's null-for-empty contract ('nothing to measure' ≠ '0% done'). The legitimate 0% case (plans exist, none summarized) is unaffected: totalPlans > 0 reaches clampPercent(0, N>0) = 0 and writes 0% as before. * test(#3233): unshadow 'Progress field missing' — clear the zero-plans guard The new totalPlans===0 no-op guard fires before the 'Progress field not found' branch, so the existing 'returns error when Progress field missing' test (no phase dirs → 0 plans) was passing for the wrong reason and that branch lost coverage. Give that test a phase dir + PLAN so totalPlans > 0 clears the guard and it reaches the branch it is named for. (Isolated review finding.) * chore(#3233): add changeset fragment * chore(#3233): backfill changeset PR number (#3375) --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/quiet-marble-field.md
Normal file
5
.changeset/quiet-marble-field.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3375
|
||||
---
|
||||
**`gsd-tools query state update-progress` no longer rewrites Progress to 0% after a milestone close** — when the current-milestone phase scan finds zero plans (the post-archive state, where `.planning/phases/` is empty), the command is now a no-op that leaves STATE.md unchanged, instead of mapping 0/0 to 0% and destroying the shipped `[██████████] 100%` record. The legitimate 0% case (plans exist, none summarized) still writes 0%. (#3233)
|
||||
@@ -804,6 +804,27 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void {
|
||||
return;
|
||||
}
|
||||
|
||||
// #3233: zero plans in the current-milestone phases means there is nothing to
|
||||
// measure — most often the milestone was just closed and its phases archived
|
||||
// (.planning/phases/ empty, but scope COMPLETE — "a real empty"). clampPercent
|
||||
// maps 0/0 to 0%, which would clobber the shipped Progress record (e.g.
|
||||
// [██████████] 100% → [░░░░░░░░░░] 0%). No-op instead, mirroring the
|
||||
// scope-withhold above and computeProgressPercent's null-for-empty contract
|
||||
// ("nothing to measure" ≠ "0% done"). The legitimate 0% case (plans exist,
|
||||
// none summarized → clampPercent(0, N>0) = 0) is unaffected: totalPlans > 0.
|
||||
if (totalPlans === 0) {
|
||||
process.stderr.write(
|
||||
`[gsd-tools] WARNING: state update-progress skipped — no plans found in current-milestone phases (0 plans). ` +
|
||||
`STATE.md's Progress field was left unchanged (milestone archived?).\n`
|
||||
);
|
||||
output(
|
||||
{ updated: false, reason: 'no plans found in current-milestone phases — STATE.md left unchanged (milestone archived?)' },
|
||||
raw,
|
||||
'false',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const percent = clampPercent(totalSummaries, totalPlans);
|
||||
const barWidth = 10;
|
||||
const filled = Math.round(percent / 100 * barWidth);
|
||||
|
||||
@@ -1724,17 +1724,49 @@ describe('cmdStateUpdateProgress (state update-progress)', () => {
|
||||
assert.ok(updated.includes('50%'), 'STATE.md Progress should contain 50%');
|
||||
});
|
||||
|
||||
test('handles zero plans gracefully', () => {
|
||||
test('#3233: zero plans (0/0) is a no-op — does not clobber the Progress record', () => {
|
||||
// Post-milestone-close: .planning/phases/ holds no plans (0/0). The buggy path
|
||||
// mapped 0/0 through clampPercent to 0% and rewrote the shipped 100% record.
|
||||
// The fix no-ops when there are zero plans to measure.
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'# Project State\n\n**Progress:** [░░░░░░░░░░] 0%\n'
|
||||
'# Project State\n\n**Progress:** [██████████] 100% of v1.0\n'
|
||||
);
|
||||
const before = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
|
||||
|
||||
const result = runGsdTools('state update-progress', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.percent, 0, 'percent should be 0 when no plans found');
|
||||
assert.strictEqual(output.updated, false, 'zero plans → no-op (updated:false)');
|
||||
assert.ok(
|
||||
/no plans found/i.test(String(output.reason)),
|
||||
`should explain the no-op; got reason: ${output.reason}`
|
||||
);
|
||||
|
||||
const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
|
||||
assert.strictEqual(after, before, 'STATE.md must be unchanged when no plans are found (#3233)');
|
||||
});
|
||||
|
||||
test('#3233 negative-space: plans exist but none done still writes 0%', () => {
|
||||
// The fix no-ops ONLY on totalPlans===0. A milestone with plans but none
|
||||
// summarized must still write a legitimate 0% (not be suppressed).
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'# Project State\n\n**Progress:** [██████████] 100%\n'
|
||||
);
|
||||
const phase01Dir = path.join(tmpDir, '.planning', 'phases', '01');
|
||||
fs.mkdirSync(phase01Dir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan\n');
|
||||
|
||||
const result = runGsdTools('state update-progress', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.updated, true, 'plans exist → write (updated:true)');
|
||||
assert.strictEqual(output.percent, 0, 'none done → 0% (legitimate, not suppressed)');
|
||||
const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
|
||||
assert.ok(after.includes('0%'), 'STATE.md Progress should reflect 0%');
|
||||
});
|
||||
|
||||
test('returns error when Progress field missing', () => {
|
||||
@@ -1742,13 +1774,22 @@ describe('cmdStateUpdateProgress (state update-progress)', () => {
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'# Project State\n\n**Status:** Active\n'
|
||||
);
|
||||
// #3233: give the scan a plan so totalPlans > 0 clears the zero-plans
|
||||
// no-op guard and this test reaches the 'Progress field not found' branch
|
||||
// it is named for (otherwise the guard fires first and the branch is uncovered).
|
||||
const phase01Dir = path.join(tmpDir, '.planning', 'phases', '01');
|
||||
fs.mkdirSync(phase01Dir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan\n');
|
||||
|
||||
const result = runGsdTools('state update-progress', tmpDir);
|
||||
assert.ok(result.success, `Command should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.updated, false, 'updated should be false');
|
||||
assert.ok(output.reason !== undefined, 'should have a reason');
|
||||
assert.ok(
|
||||
/Progress field not found/i.test(String(output.reason)),
|
||||
`should be the 'Progress field not found' reason; got: ${output.reason}`
|
||||
);
|
||||
});
|
||||
|
||||
// ── #2177: frontmatter `progress:` key must not shadow the body Progress: line ──
|
||||
|
||||
Reference in New Issue
Block a user