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

fix(#2969): ratchet completed_plans up for gap-closure plans under deriveProgressKeys (#3091)

Browse Source 
* test(#2969): completed_plans must ratchet up for gap-closure plans

Failing-first regression: when deriveProgressKeys=true (cmdStatePlannedPhase's
opt-in), applyStatePreservation restores completed_plans to its pre-growth
curated value, so gap-closure plans that complete never increment it —
STATE.md shows completed_plans < total_plans forever even though every PLAN
has a SUMMARY. Three cases: the ratchet-up (derived 54 > curated 50), the
ratchet-down protection (derived 47 < curated 50 keeps curated), and the
body-only write protection (no deriveProgressKeys → wholesale restore).

The existing #2440 test covers the case where derived < curated (ratchet
holds); this adds the missing case where derived > curated (ratchet must
release upward).

* fix(#2969): ratchet completed_plans/plases up under deriveProgressKeys

applyStatePreservation's deriveProgressKeys path (cmdStatePlannedPhase's
opt-in) let total_plans/total_phases take the derived value but restored
completed_plans/completed_phases to their pre-growth curated value — so
gap-closure plans that completed after the plan count grew never incremented
them, leaving STATE.md at completed_plans < total_plans forever (every PLAN
had a SUMMARY).

Extend the deriveProgressKeys exclusion to also let completed_plans and
completed_phases take the derived value, but ratcheted UP only (never derive
downward past curated) — preserving the #3242 curated-progress protection
for cases unrelated to plan-count growth (e.g. a deleted SUMMARY). percent
takes the derived value (the resync already recomputed it from disk counts).

Scoped to deriveProgressKeys (plan-phase only); body-only writes
(state.update/patch without the flag) keep the full #3242 wholesale restore.

* fix(#2969): also take derived percent under deriveProgressKeys

Isolated-review blocker: percent fell into the else branch and was
overwritten with the stale curated value, contradicting the inline comment
and leaving STATE.md incoherent (e.g. completed_plans:54/total_plans:54 at
percent:93). Skip percent in the ratchet loop so the derived (resync-
recomputed) value survives.

* chore(#2969): add changeset fragment

* chore(#2969): backfill changeset PR number 3091

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-05 16:52:37 -04:00
committed by GitHub
parent 53ea8e0664
commit b0f1722662
3 changed files with 78 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/lucky-tunas-leap.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3091
---
**`progress.completed_plans` no longer stays pinned after a gap-closure cycle** — when plan-phase re-planned a phase and added gap-closure plans, `total_plans` corrected upward but `completed_plans` was restored to its pre-growth value, so STATE.md showed `completed_plans < total_plans` permanently even after every plan (including the gap-closure ones) was summarized. `completed_plans` and `completed_phases` now ratchet up to the disk-derived count under the plan-phase progress opt-in (never deriving downward, preserving the curated-progress ratchet for unrelated edits). (#2969)

19
src/state-transition.cts
View File

@@ -195,8 +195,25 @@ export function applyStatePreservation(input: StatePreservationInput): StatePres
const derived = (postFm['progress'] ?? {}) as Record<string, unknown>;
const merged: Record<string, unknown> = { ...derived };
if (curated) {
// #2440: total_plans and total_phases always take the derived value.
// #2969: completed_plans and completed_phases take the derived value
// when it is GREATER than the curated value (gap-closure plans that
// completed after the plan count grew) — ratcheting UP only, never
// deriving downward (preserves the #3242 curated-progress protection
// for cases unrelated to plan-count growth, e.g. a deleted SUMMARY).
// percent also takes the derived value — the resync recomputed it from
// disk counts, and a stale curated percent would be incoherent against
// the ratcheted-up completed counts (e.g. 54/54 at 93%).
const ratchetUpKeys = new Set(['completed_plans', 'completed_phases']);
for (const [key, value] of Object.entries(curated)) {
if (key !== 'total_plans' && key !== 'total_phases') {
if (key === 'total_plans' || key === 'total_phases' || key === 'percent') continue;
if (ratchetUpKeys.has(key)) {
const derivedNum = typeof derived[key] === 'number' ? derived[key] : -Infinity;
const curatedNum = typeof value === 'number' ? value : -Infinity;
// Take the derived value only when it ratchets up; else keep curated.
if (derivedNum > curatedNum) continue;
merged[key] = value;
} else {
merged[key] = value;
}
}

55
tests/state-transition.test.cjs
View File

@@ -1340,6 +1340,61 @@ describe('ADR-1769 #1796: applyStatePreservation — table-driven post-sync cons
'total_plans equality → derived value (identity)');
});
test('#2969: deriveProgressKeys=true — completed_plans ratchets UP when disk count exceeds curated (gap-closure plans completed)', () => {
// Gap-closure scenario: a phase had 50 plans all summarized (completed_plans: 50),
// then 4 gap-closure plans were added (total_plans -> 54) and all 4 got SUMMARYs.
// Disk scan now counts 54 summaries. The curated completed_plans (50) must
// ratchet UP to the derived value (54), not stay pinned at 50 — otherwise
// completed_plans < total_plans forever even though every plan is summarized.
const curated = { progress: { total_plans: 54, completed_plans: 50, total_phases: 2, completed_phases: 1, percent: 93 } };
const r = applyStatePreservation({
preFm: curated,
preFmSnapshot: curated,
postFm: { progress: { total_plans: 54, completed_plans: 54, total_phases: 2, completed_phases: 1, percent: 100 } },
resync: false,
deriveProgressKeys: true,
...untouched,
});
assert.equal(r.postFm.progress.total_plans, 54, 'total_plans takes derived value');
assert.equal(r.postFm.progress.completed_plans, 54,
'completed_plans must ratchet UP to derived (54 > curated 50 — gap-closure plans completed) (#2969)');
assert.equal(r.postFm.progress.percent, 100,
'percent must reflect the true completion fraction (54/54) (#2969)');
});
test('#2969 ratchet-down protection: deriveProgressKeys=true keeps curated when disk count < curated', () => {
// The ratchet must only go UP. If the disk count is somehow LOWER than
// curated (e.g. a SUMMARY was deleted), keep the curated value — do not
// derive downward. (#3242 curated-progress protection, scoped to deriveProgressKeys.)
const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
const r = applyStatePreservation({
preFm: curated,
preFmSnapshot: curated,
postFm: { progress: { total_plans: 54, completed_plans: 47, percent: 87 } },
resync: false,
deriveProgressKeys: true,
...untouched,
});
assert.equal(r.postFm.progress.completed_plans, 50,
'completed_plans must NOT derive downward (47 < curated 50) — ratchet-up only (#2969/#3242)');
});
test('#2969 body-only write protection: deriveProgressKeys absent keeps wholesale restore', () => {
// state.update/patch (no deriveProgressKeys flag) must keep the full #3242
// wholesale curated restore — completed_plans never moves for a body-only edit.
const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
const r = applyStatePreservation({
preFm: curated,
preFmSnapshot: curated,
postFm: { progress: { total_plans: 54, completed_plans: 54, percent: 100 } },
resync: false,
// deriveProgressKeys NOT set — body-only write path
...untouched,
});
assert.equal(r.postFm.progress.completed_plans, 50,
'body-only write must keep curated completed_plans (no deriveProgressKeys) (#2969/#3242)');
});
test('progress: NOT restored when transition re-derives from disk (resync=true) — sync/advancePlan/completePhase path', () => {
const recomputed = { progress: { total_phases: 5, completed_phases: 1, percent: 20 } };
const r = applyStatePreservation({