fix: milestone-prune stops answering ok:true over a half-finished close #169
No reviewers
Labels
No labels
already-shipped
bug
documentation
duplicate
enhancement
external-review
good first issue
help wanted
in progress
invalid
needs-decision
proposal
question
security
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: crenshawdev/cadence-archived#169
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/v3.3.1-milestone-prune"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
warningswhile the loop continued. The twoatomicWritecalls below then ran unconditionally and the envelope answeredok: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 relayswarnings[]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.
missingcounts as cleared (it already was, which keeps a re-run idempotent);faileddoes not. A partial application returnsok:false, reason:"partial-prune"withfailednaming the phases that did not clear, and those phases keep their roadmap line and their## Activerows, so the tree and the documents still agree. Re-running picks up only what is left.workflows/milestone.mdgained 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, andrenameSyncthen followed it, depositing phase directories outside the planning root. The source comment stated this as an accepted residual.Now: the path is classified with
lstatbefore anything moves, so a symlink failsisDirectory()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, sincerenameSynconto an empty directory silently succeeds and would destroy that evidence.Verification
node --test cadence-core/bin/milestone-prune.test.mjsnode --test cadence-core/bin/*.test.mjsnode cadence-core/bin/self-verify.mjsproblems: []npx tsc -p tsconfig.ci.json --noEmitFour new seam tests, each shown red against the pre-fix seam by reverting
planning.mjsto HEAD and re-running. Forcing the failure needs nochmodand no root check: an occupied destination makesrenameSyncthrow deterministically on any platform this ships to.One thing to look at
weight-budgets.jsonforworkflows/milestone.mdgoes 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.