milestone-prune returns ok:true after a directory operation fails #128

New issue

Closed

opened 2026-08-15 14:33:33 +00:00 by john · 0 comments

john commented

2026-08-15 14:33:33 +00:00

Owner

Source: deep dive F5; strict re-review C-03 (independent convergence)
Verdict: AGREE - high

Claim as reviewed

Milestone pruning deletes or moves phase directories before writing the updated roadmap and requirements. Failed directory operations become warnings, but the documents are still pruned for every completed phase, and the envelope still reports ok:true, action:"pruned".

At HEAD `f354864` (v3.3.0)

cadence-core/bin/planning.mjs:3579-3609. The per-phase directory op sits in a try/catch whose catch pushes a string into warnings and lets the loop continue. atomicWrite(roadmapFile, pruned.text) then runs unconditionally, followed by ok({action:'pruned', ...}).

The comment at :3579-3580 - "Directories third, writes last: a rename that throws leaves both docs untouched on disk rather than half a close" - is contradicted by the code: the throw never escapes the loop, so the writes always land.

cadence-core/workflows/milestone.md:103 says "Relay warnings[]" and enumerates only the two benign classes (a missing detail section, an unreadable REQUIREMENTS.md). It does not halt, so the close proceeds to commit a roadmap that disagrees with the tree.

cadence-core/bin/milestone-prune.test.mjs has no forced directory-failure case; its only warning test is the missing-REQUIREMENTS one at :279.

Note

Blast radius is bounded: with planning.commit_docs at its default true the phase dirs are in git history, so --mode delete loses no work. The damage is a ROADMAP/REQUIREMENTS pair that disagrees with .planning/phases/ at the moment a milestone closes.

Fix shape: stage both document outputs, apply directory operations first, write documents only for phases whose directory op succeeded, and return ok:false on partial application. See the journal proposal issue.

**Source:** deep dive F5; strict re-review C-03 (independent convergence) **Verdict:** AGREE - high ### Claim as reviewed Milestone pruning deletes or moves phase directories before writing the updated roadmap and requirements. Failed directory operations become warnings, but the documents are still pruned for every completed phase, and the envelope still reports `ok:true, action:"pruned"`. ### At HEAD f354864 (v3.3.0) `cadence-core/bin/planning.mjs:3579-3609`. The per-phase directory op sits in a `try/catch` whose `catch` pushes a string into `warnings` and lets the loop continue. `atomicWrite(roadmapFile, pruned.text)` then runs unconditionally, followed by `ok({action:'pruned', ...})`. The comment at `:3579-3580` - "Directories third, writes last: a rename that throws leaves both docs untouched on disk rather than half a close" - is contradicted by the code: the throw never escapes the loop, so the writes always land. `cadence-core/workflows/milestone.md:103` says "Relay `warnings[]`" and enumerates only the two benign classes (a missing detail section, an unreadable REQUIREMENTS.md). It does not halt, so the close proceeds to commit a roadmap that disagrees with the tree. `cadence-core/bin/milestone-prune.test.mjs` has no forced directory-failure case; its only warning test is the missing-REQUIREMENTS one at `:279`. ### Note Blast radius is bounded: with `planning.commit_docs` at its default `true` the phase dirs are in git history, so `--mode delete` loses no work. The damage is a ROADMAP/REQUIREMENTS pair that disagrees with `.planning/phases/` at the moment a milestone closes. Fix shape: stage both document outputs, apply directory operations first, write documents only for phases whose directory op succeeded, and return `ok:false` on partial application. See the journal proposal issue.