From 23e6d499296b7deebbc1d27f69ca739257ed68c4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 11 Aug 2026 22:59:16 -0400 Subject: [PATCH] fix(#3233): no-op state update-progress when the milestone scan finds zero plans (#3375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .changeset/quiet-marble-field.md | 5 ++++ src/state.cts | 21 ++++++++++++++ tests/state.test.cjs | 49 +++++++++++++++++++++++++++++--- 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 .changeset/quiet-marble-field.md diff --git a/.changeset/quiet-marble-field.md b/.changeset/quiet-marble-field.md new file mode 100644 index 000000000..76d98b5af --- /dev/null +++ b/.changeset/quiet-marble-field.md @@ -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) diff --git a/src/state.cts b/src/state.cts index cb328e144..dd7e6d4db 100644 --- a/src/state.cts +++ b/src/state.cts @@ -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); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index a0f20c34a..aea8966fd 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -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 ──