renumber insert corrupts phase directories when the highest phase is a decimal #36

New issue

Closed

opened 2026-07-23 14:19:31 +00:00 by crenshawdev · 0 comments

crenshawdev commented

2026-07-23 14:19:31 +00:00

(Migrated from github.com)

Severity: high · Area: renumber · surfaced in the post-v1.2.0 cross-model review sweep (reproduced with a fixture)

What happens

renumber insert renames the .planning/phases/ directories from the top down to make room for the new phase. It computes "the top" as the highest phase number — but that maximum includes decimal insertion phases like 2.1. When a decimal is the highest phase, the countdown runs on fractional values and never lands on the integer directories it was supposed to move, while it does move the decimal directory, which by design must stay put. The command reports success the whole time, leaving every integer phase's directory pointing at the wrong phase number.

This is the operation cad-phase exists to get right, broken precisely for repos that use decimal insertions — the case that most needs a reliable renumber.

Mechanism (verified against source)

const maxN = Math.max(...phases.map((p) => p.n)) (cadence-core/bin/planning.mjs:586) takes the max over all phases, decimals included.
The insert dir-move loop for (let k = maxN; k >= at; k--) (planning.mjs:604) therefore starts at a fractional ceiling. With phases 1, 2, 2.1 and --at 1: k runs 2.1, 1.1 and stops. existingDir(2.1) matches → phases/2.1 is pushed to move to phases/3.1; existingDir(1.1) does not exist → skipped. The integer directories phases/1 and phases/2 are never visited.
Meanwhile shiftPhaseTokens (called at :616) correctly leaves decimal phases alone in the ROADMAP, so the ROADMAP renumbers to Phase 2 / Phase 3 while the directories still read 1 / 2 — a silent desync.
The remove path (planning.mjs:606) starts at the integer at + 1 and increments, so k stays integer and remove is unaffected.

Reproduction

ROADMAP phases 1, 2, 2.1, then renumber insert --at 1:

ROADMAP → Phase 2, Phase 3 (correct), decimal left as Phase 2.1 (correct).
Directories → phases/1 and phases/2 unchanged (wrong), phases/2.1 renamed to phases/3.1 (wrong — decimals must never move).
Command returns ok:true.

Fix direction

Compute the dir-move ceiling from integer phases only (e.g. Math.max over phases.filter((p) => Number.isInteger(p.n))). Then the insert loop visits only integer directories and leaves decimals untouched, matching the design contract already stated at planning.mjs:590-593 and enforced by shiftPhaseTokens.

**Severity:** high · **Area:** renumber · surfaced in the post-v1.2.0 cross-model review sweep (reproduced with a fixture) ## What happens `renumber insert` renames the `.planning/phases/` directories from the top down to make room for the new phase. It computes "the top" as the highest phase number — but that maximum includes decimal insertion phases like `2.1`. When a decimal is the highest phase, the countdown runs on fractional values and never lands on the integer directories it was supposed to move, while it *does* move the decimal directory, which by design must stay put. The command reports success the whole time, leaving every integer phase's directory pointing at the wrong phase number. This is the operation `cad-phase` exists to get right, broken precisely for repos that use decimal insertions — the case that most needs a reliable renumber. ## Mechanism (verified against source) - `const maxN = Math.max(...phases.map((p) => p.n))` (`cadence-core/bin/planning.mjs:586`) takes the max over **all** phases, decimals included. - The insert dir-move loop `for (let k = maxN; k >= at; k--)` (`planning.mjs:604`) therefore starts at a fractional ceiling. With phases `1, 2, 2.1` and `--at 1`: `k` runs `2.1`, `1.1` and stops. `existingDir(2.1)` matches → `phases/2.1` is pushed to move to `phases/3.1`; `existingDir(1.1)` does not exist → skipped. The integer directories `phases/1` and `phases/2` are **never visited**. - Meanwhile `shiftPhaseTokens` (called at `:616`) correctly leaves decimal phases alone in the ROADMAP, so the ROADMAP renumbers to Phase 2 / Phase 3 while the directories still read 1 / 2 — a silent desync. - The remove path (`planning.mjs:606`) starts at the integer `at + 1` and increments, so `k` stays integer and remove is unaffected. ## Reproduction ROADMAP phases `1, 2, 2.1`, then `renumber insert --at 1`: - ROADMAP → Phase 2, Phase 3 (correct), decimal left as Phase 2.1 (correct). - Directories → `phases/1` and `phases/2` unchanged (wrong), `phases/2.1` renamed to `phases/3.1` (wrong — decimals must never move). - Command returns `ok:true`. ## Fix direction Compute the dir-move ceiling from integer phases only (e.g. `Math.max` over `phases.filter((p) => Number.isInteger(p.n))`). Then the insert loop visits only integer directories and leaves decimals untouched, matching the design contract already stated at `planning.mjs:590-593` and enforced by `shiftPhaseTokens`.