fix: milestone-prune stops answering ok:true over a half-finished close #169

Merged
john merged 1 commit from fix/v3.3.1-milestone-prune into main 2026-08-15 15:26:10 +00:00
Owner

Closes #128. Closes #138.

Both are cmdMilestonePrune, so this is one surgery on one function.

#128 - a half-finished close reported as a finished one

The directory pass ran after the document transforms, and its per-phase failures were caught into warnings while the loop continued. The two atomicWrite calls below then ran unconditionally and the envelope answered ok:true, action:"pruned". So a phase that could not be moved or deleted still lost its ROADMAP line and had its requirement rows archived, and /cad-milestone - which relays warnings[] but halts on none of them - committed that disagreement.

The comment above the loop promised the opposite: "a rename that throws leaves both docs untouched on disk rather than half a close". It never did, because the throw never left the loop.

Now: directories run first, and only the phases they actually cleared reach the transforms. missing counts as cleared (it already was, which keeps a re-run idempotent); failed does not. A partial application returns ok:false, reason:"partial-prune" with failed naming the phases that did not clear, and those phases keep their roadmap line and their ## Active rows, so the tree and the documents still agree. Re-running picks up only what is left.

workflows/milestone.md gained the halt arm for both new refusal reasons.

#138 - archive containment was lexical

resolve() is string arithmetic, so a pre-existing _archive-<label> that is itself a symlink out of the tree resolved lexically inside it. mkdirSync(recursive) succeeds silently against an existing link, and renameSync then followed it, depositing phase directories outside the planning root. The source comment stated this as an accepted residual.

Now: the path is classified with lstat before anything moves, so a symlink fails isDirectory() whatever it points at - which is also the right answer for a regular file squatting the name. A per-phase destination left behind by an interrupted close is refused rather than clobbered, since renameSync onto an empty directory silently succeeds and would destroy that evidence.

Verification

Check Result
node --test cadence-core/bin/milestone-prune.test.mjs 21 pass
node --test cadence-core/bin/*.test.mjs 1799 pass, 0 fail
node cadence-core/bin/self-verify.mjs problems: []
npx tsc -p tsconfig.ci.json --noEmit clean

Four new seam tests, each shown red against the pre-fix seam by reverting planning.mjs to HEAD and re-running. Forcing the failure needs no chmod and no root check: an occupied destination makes renameSync throw deterministically on any platform this ships to.

One thing to look at

weight-budgets.json for workflows/milestone.md goes 10205 -> 10662 (+457B), to document the two new refusal reasons. The prose was tightened once already, which cut the overrun from 763B. Growth is what that gate exists to catch, so this is the line to argue with if you want the bytes paid for elsewhere in that file instead.

Closes #128. Closes #138. Both are `cmdMilestonePrune`, so this is one surgery on one function. ## #128 - a half-finished close reported as a finished one The directory pass ran *after* the document transforms, and its per-phase failures were caught into `warnings` while the loop continued. The two `atomicWrite` calls below then ran unconditionally and the envelope answered `ok:true, action:"pruned"`. So a phase that could not be moved or deleted still lost its ROADMAP line and had its requirement rows archived, and `/cad-milestone` - which relays `warnings[]` but halts on none of them - committed that disagreement. The comment above the loop promised the opposite: *"a rename that throws leaves both docs untouched on disk rather than half a close"*. It never did, because the throw never left the loop. **Now:** directories run first, and only the phases they actually cleared reach the transforms. `missing` counts as cleared (it already was, which keeps a re-run idempotent); `failed` does not. A partial application returns `ok:false, reason:"partial-prune"` with `failed` naming the phases that did not clear, and those phases keep their roadmap line and their `## Active` rows, so the tree and the documents still agree. Re-running picks up only what is left. `workflows/milestone.md` gained the halt arm for both new refusal reasons. ## #138 - archive containment was lexical `resolve()` is string arithmetic, so a pre-existing `_archive-<label>` that is itself a symlink out of the tree resolved lexically inside it. `mkdirSync(recursive)` succeeds silently against an existing link, and `renameSync` then followed it, depositing phase directories outside the planning root. The source comment stated this as an accepted residual. **Now:** the path is classified with `lstat` before anything moves, so a symlink fails `isDirectory()` whatever it points at - which is also the right answer for a regular file squatting the name. A per-phase destination left behind by an interrupted close is refused rather than clobbered, since `renameSync` onto an empty directory silently succeeds and would destroy that evidence. ## Verification | Check | Result | |---|---| | `node --test cadence-core/bin/milestone-prune.test.mjs` | 21 pass | | `node --test cadence-core/bin/*.test.mjs` | 1799 pass, 0 fail | | `node cadence-core/bin/self-verify.mjs` | `problems: []` | | `npx tsc -p tsconfig.ci.json --noEmit` | clean | Four new seam tests, **each shown red against the pre-fix seam** by reverting `planning.mjs` to HEAD and re-running. Forcing the failure needs no `chmod` and no root check: an occupied destination makes `renameSync` throw deterministically on any platform this ships to. ## One thing to look at `weight-budgets.json` for `workflows/milestone.md` goes 10205 -> 10662 (+457B), to document the two new refusal reasons. The prose was tightened once already, which cut the overrun from 763B. Growth is what that gate exists to catch, so this is the line to argue with if you want the bytes paid for elsewhere in that file instead.
fix: milestone-prune stops answering ok:true over a half-finished close
All checks were successful
test / node-test (git, 22) (pull_request) Successful in 20s
test / node-test (git, 24) (pull_request) Successful in 23s
test / node-test (other, 22) (pull_request) Successful in 17s
test / node-test (other, 24) (pull_request) Successful in 17s
test / node-test (planning, 22) (pull_request) Successful in 1m11s
test / node-test (planning, 24) (pull_request) Successful in 1m8s
test / node-test (prose, 22) (pull_request) Successful in 25s
test / node-test (prose, 24) (pull_request) Successful in 26s
test / node-test (review, 22) (pull_request) Successful in 17s
test / node-test (review, 24) (pull_request) Successful in 14s
test / node-test (routing, 22) (pull_request) Successful in 30s
test / node-test (routing, 24) (pull_request) Successful in 31s
test / self-verify (pull_request) Successful in 17s
test / typecheck (pull_request) Successful in 19s
ccf03e00cc
The directory pass ran after the document transforms and its failures were
caught into warnings, so a phase that could not be moved or deleted still lost
its roadmap line and had its requirement rows archived, while the envelope
answered ok:true action:"pruned". The comment above it promised the opposite,
that a rename which throws leaves both documents untouched. It never did: the
throw never left the loop.

Directories now run first and only the phases they actually cleared reach the
transforms, so the tree and the documents still agree. A partial application
returns ok:false reason:"partial-prune" naming the phases that did not clear,
and /cad-milestone halts on it rather than committing the disagreement. A
re-run picks up only what is left.

The archive root is also classified with lstat before anything moves. The
containment check was lexical, so a pre-existing _archive-<label> symlink
resolved inside the planning root, mkdirSync succeeded against it and
renameSync followed it out of the tree. A per-phase destination left behind by
an interrupted close is now refused rather than clobbered.

Four new seam tests, each shown red against the pre-fix seam.

Closes #128, #138
john merged commit 561a12648c into main 2026-08-15 15:26:10 +00:00
john deleted branch fix/v3.3.1-milestone-prune 2026-08-15 15:26:10 +00:00
Sign in to join this conversation.
No description provided.