fix(#2440): per-counter progress ratchet — total_plans always takes derived value (#2468)

* fix(#2440): per-counter progress ratchet — total_plans always takes derived value

Two sites fixed (targeted — existing body-only write tests preserved):

Site A — read path: shouldPreserveExistingProgress (state-document.cts:167)
removed total_plans from the all-or-nothing ratchet check. It now joins
total_phases as an always-derived counter. Only completed_phases and
completed_plans keep ratchet behaviour (they are monotonic). This fixes
gsd-tools query state.json reporting stale total_plans when a curated
completed_plans triggers the ratchet.

Site B — write path: applyStatePreservation (state-transition.cts:162)
gained a deriveProgressKeys opt-in flag. When true (passed by
cmdStatePlannedPhase only), total_plans and total_phases take the
derived (post-sync) value instead of the wholesale curated restore.
When false (the default — state.update, state.patch), the existing
#3242 wholesale protection stays fully in force. This fixes the
state planned-phase verb writing a stale total_plans.

The opt-in approach preserves all 8 existing #3242/#1264/#500 body-only
write tests that assert wholesale progress preservation during non-
progress updates.

Tests:
- tests/state.test.cjs: 4 unit tests for shouldPreserveExistingProgress
  (total_plans upward/downward/equality + completed_plans ratchet active).
- tests/state-transition.test.cjs: 2 #2440 regression tests for
  deriveProgressKeys=true (total_plans takes derived; boundary at equality).
  The existing !resync wholesale-restore test stays unchanged (default
  behavior preserved).

References: #2440; #1446 (total_phases read-path fix — same principle);
#3242 Bug A (body-only preservation — protection preserved via the opt-in
gate); ADR-1769 (applyStatePreservation table-driven preservation).

* chore(#2440): backfill pr:2468 in .changeset/mellow-eagles-chatter.md
This commit is contained in:
Tom Boucher
2026-07-20 19:24:21 -04:00
committed by GitHub
parent bb97ffb5aa
commit a54feb4216
8 changed files with 156 additions and 16 deletions

View File

@@ -171,12 +171,17 @@ export function shouldPreserveExistingProgress(existingProgress: unknown, derive
return false;
const existing = existingProgress as ProgressRecord;
const derived = derivedProgress as ProgressRecord;
// total_phases is intentionally excluded from the ratchet: it must always
// take the freshly derived value so it can correct downward (#1446).
// Only completed_phases, total_plans, and completed_plans keep ratchet behaviour.
return (existingProgressExceedsDerived(existing, derived, 'completed_phases') ||
existingProgressExceedsDerived(existing, derived, 'total_plans') ||
existingProgressExceedsDerived(existing, derived, 'completed_plans'));
// total_phases (#1446) and total_plans (#2440) are intentionally excluded
// from the ratchet: both must always take the freshly derived value so they
// can correct in BOTH directions. total_plans legitimately moves up (a new
// phase adds plans) and down (milestone reorganization removes phases).
// Ratcheting it freezes stale values. Only completed_phases and
// completed_plans keep ratchet behaviour — they are monotonic (once a
// phase/plan is complete, it stays complete).
return (
existingProgressExceedsDerived(existing, derived, 'completed_phases') ||
existingProgressExceedsDerived(existing, derived, 'completed_plans')
);
}
export function normalizeProgressNumbers(progress: unknown): unknown {

View File

@@ -147,6 +147,14 @@ export type StatePreservationInput = {
postBodyStoppedAt: string | null;
preBodyPhaseSource: string | null;
postBodyPhaseSource: string | null;
/**
* #2440: when true, total_plans and total_phases take the derived (post-sync)
* value even under !resync, instead of the wholesale curated restore. Used
* by callers (e.g. cmdStatePlannedPhase) where total_plans must correct to
* disk truth after plans are added. Body-only writes (state.update/patch)
* leave this false — the #3242 wholesale protection stays in force.
*/
deriveProgressKeys?: boolean;
};
export type StatePreservationResult = {
@@ -176,7 +184,27 @@ export function applyStatePreservation(input: StatePreservationInput): StatePres
preFm &&
preFm['progress']
) {
postFm['progress'] = preFm['progress'];
// #2440: when the caller opts in (deriveProgressKeys), total_plans and
// total_phases always take the derived (post-sync) value even under !resync.
// This is used by cmdStatePlannedPhase where total_plans must correct upward
// after plans are added. For body-only writes (state.update/patch without
// the flag), the wholesale restore preserves everything as before — the
// #3242 Bug A protection stays fully in force.
if (input.deriveProgressKeys && postFm['progress']) {
const curated = preFm['progress'] as Record<string, unknown> | null;
const derived = (postFm['progress'] ?? {}) as Record<string, unknown>;
const merged: Record<string, unknown> = { ...derived };
if (curated) {
for (const [key, value] of Object.entries(curated)) {
if (key !== 'total_plans' && key !== 'total_phases') {
merged[key] = value;
}
}
}
postFm['progress'] = merged;
} else {
postFm['progress'] = preFm['progress'];
}
mutated = true;
}

View File

@@ -63,6 +63,8 @@ interface StateLockClock {
interface ReadModifyWriteOptions {
resync?: boolean;
/** #2440: when true, total_plans/total_phases take derived values even under !resync. */
deriveProgressKeys?: boolean;
}
interface StateRecordMetricOptions {
@@ -2117,6 +2119,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
const postFm = extractFrontmatter(synced) as Record<string, unknown>;
const preservation = applyStatePreservation({
preFm, postFm, preFmSnapshot, resync,
deriveProgressKeys: options?.deriveProgressKeys === true,
preBodyStatus, postBodyStatus,
preBodyStoppedAt, postBodyStoppedAt,
preBodyPhaseSource, postBodyPhaseSource,
@@ -2477,7 +2480,7 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number, planCou
const result = transitionCore(content, intent, deps);
updated = result.updated;
return result.content;
}, cwd, { resync: false });
}, cwd, { resync: false, deriveProgressKeys: true });
output({ updated, phase: phaseNumber, plan_count: planCount }, raw, updated.length > 0 ? 'true' : 'false');
}