From aeac47b95ac0ef59db9c61deec62d504b450b45a Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Tue, 15 Sep 2026 00:41:35 -0500 Subject: [PATCH] fix(#4465): bound /gsd:undo commit selection to the phase directory and HEAD (#4472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#4465): bound /gsd:undo commit selection to the phase directory and HEAD `--phase` documented a primary path reading `.planning/.phase-manifest.json`, but nothing in the repository writes that file, so the documented fallback was the only reachable path: git log --oneline --no-merges --all | grep -E "\(0*${TARGET_PHASE}(-[0-9]+)?\):" | head -50 That selection has no milestone bound and no reachability bound, and it feeds `git revert --no-commit`. On a project that reuses a phase number it stages deletion of a previous milestone's files, under a confirmation gate that displays only `{hash} — {message}` — the one field that carries no milestone discriminator. Port the #3995 anchor already live in code-review.md: resolve the phase's own directory via `find-phase` (which resolves through planningDir, so it is workstream-correct), take PHASE_START as the first commit adding anything under it, and select over `PHASE_START^..HEAD`. Drop `--all`. Fail closed when no anchor resolves rather than widening to a repository-wide search, and report truncation instead of silently capping at 50. Also resolve the dead manifest read rather than leaving documented-but- unreachable behaviour, and hoist dependency_check onto the workstream-resolved planning root — it read a hardcoded `.planning/ROADMAP.md` and `.planning/phases/`, which are the wrong tree under an active workstream. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01B58zMdMYc3n16mfdqMHMqv * fix(#4465): close three defects the pre-create adversarial review found Root-commit off-by-one: `${PHASE_START}..HEAD` EXCLUDES PHASE_START, so when the phase's first commit is the repository root the selection silently dropped it and the undo refused legitimate work. The root branch now selects over `HEAD`. Empty-selection exit status: `grep` exits 1 on no match, and the removed `| head -50` had been masking that rc. Both selection pipelines now end in `|| true` so an empty selection reaches the workflow's own Empty check instead of aborting the block. Truncation stop was documented for MODE=phase only; MODE=plan could still cap silently. Both modes now carry it. Also documents two residuals the review surfaced rather than leaving them implicit: a revision range is ancestry and not chronology, so a pre-phase side branch merged in after PHASE_START stays selectable; and the `--diff-filter=A` anchor does not follow renames, so an archived phase directory under-selects (reverts too little or refuses, never too much). Tests: 12 assertions, negative-controlled. A-H and K-L are RED against the pre-fix workflow; J is RED against the intermediate revision that carried the off-by-one; I is green in both directions by design. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01B58zMdMYc3n16mfdqMHMqv * chore(#4465): backfill the changeset fragment's pr field The fragment could not carry `pr:` before the PR existed; both `scripts/changeset/lint.cjs` and `scripts/lint-docs-required.cjs` require it and reported `missing_pr` until now. Both are green with it filled in. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01B58zMdMYc3n16mfdqMHMqv * chore(#4465): acknowledge the deliberate growth of undo.md The emitted-attribution gate flags undo.md growing 11881 -> 17435 bytes. The growth is the fix: a one-line `git log --all | grep` selection is replaced by an anchored, HEAD-bounded selection for BOTH modes, each with its own fail-closed branch and truncation stop, plus three residuals documented next to the code they qualify rather than left implicit. A workflow document is the executable contract, so the residuals belong in it. Emitted-Drift-Ack-Growth: undo.md — replaces a one-line unbounded commit-subject grep with an anchored HEAD-bounded selection in both --phase and --plan, each with a fail-closed branch and a truncation stop, plus three residuals documented in-workflow (#4465) Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01B58zMdMYc3n16mfdqMHMqv * test(#4465): execute undo.md's selection fences against a git fixture The shipped regression test matched substrings in undo.md's bash fences. A substring cannot tell a live invocation from a dead one, and this is the boundary/reachability defect class this repo's own conventions want driven with limit-1/limit/limit+1 execution proof. So the fences the runtime runs — sliced out of undo.md by content anchor, never by position — are now replayed with `bash -c` inside createTempGitProject fixtures against the real `gsd-tools.cjs`, the same fence-execution shape #2308 and #2352 use. Ten executed cases: the fixture's own negative control (the retired `--all` grep over-selects the archived milestone and a dead branch), `--phase` and `--plan` selecting only the current milestone's HEAD-reachable instance, the single-milestone selection unchanged, limit-1 (a matching pre-phase commit excluded, PHASE_START itself included), the root-commit branch selecting over `HEAD`, fail-closed on an absent phase in both --phase and --plan (empty PHASE_DIR and UNDO_RANGE), the active workstream's phase directory winning over the root's, and dependency_check's `planning inspect --pick generated_from.planning_root --raw` resolving the workstream root, the project root, and the `.planning` fallback. Skipped on win32 with the #2352 precedent's reason: the fences are POSIX bash driven through `bash -c`; the shape half still runs there. Negative-controlled against the pre-fix undo.md (upstream/next): 11 of 12 shape assertions red, and the executed block fails at fence extraction. Shape test L now pins the >50 refusal message in both modes, not only its heading — the cross-AI round review removed the paragraphs under intact headings and L stayed green; it now fails on that mutation. * fix(#4465): refuse an archived-milestone PHASE_DIR in both undo modes `find-phase` searches the live `phases/` directory first, then every `milestones/v-phases/` directory in ascending version order, and its ambiguity check is scoped to one directory: the `matches.length > 1` test sits inside `cmdFindPhase`'s per-`searchDir` loop (`src/phase.cts`). A phase number that is not live therefore resolves silently to the OLDEST archived milestone carrying one, with no warning. Anchoring there is wrong in both directions at once. The oldest commit adding that path is the archival move, so the phase's real work predates the window and falls outside it, while the window runs forward from that archival through every later milestone -- where the subject grep matches THEIR same-numbered phase. Driven on a two-archived-milestone fixture, `--phase 03` selected v2.0's `feat(03-01): add search index` and `docs(03-01): v2.0 phase plan` and excluded v1.0's own `feat(03-01): implement auth endpoint`. On `git revert --no-commit` that is the cross-milestone contamination this PR exists to close, recurring. Both modes now blank PHASE_DIR on an archived resolution and refuse with their own message, so the existing fail-closed rule stays load-bearing even if the refusal's prose is not honored. The "archived or renamed phase directory under-selects" residual claimed this failure could revert "too little or refuse, never too much". That was wrong in the direction that matters; it is rewritten to cover renames only, and the archival case is recorded as refused rather than disclosed. Tests: a two-archived-milestone fixture, a negative control asserting the unguarded window selects v2.0's two commits and none of v1.0's, refusals in both --phase and --plan, and an inertness check on a live resolution. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * docs(#4465): close the three Minor review items on undo.md and its test Purpose line: it still advertised rolling back "using the phase manifest", the exact `.planning/.phase-manifest.json` mechanism this PR removes. Test B already reads the whole file, so scope is not why it missed this -- B greps the hyphenated `phase-manifest` token, the filename, while the purpose line named the same dead mechanism in prose. Test N pins the block itself, which is spelling-independent; widening B's pattern to /manifest/i instead would fire on any future sentence that merely mentions one. Merge-commit anchors: `git log --diff-filter=A -- "${PHASE_DIR}"` does not walk merge diffs by default. A directory added on a side branch is still found -- the side-branch commit that added it is in history -- so the uncovered case is narrower than "introduced via a merge": it is a directory first appearing in the merge RESOLUTION. The information is not absent from history, only unrequested: `git log -m` prints that add once per parent. Recorded as a residual rather than fixed -- the failure is a refusal, and the evil-merge fixture costs more than a safe-direction branch is worth. allow-test-rule marker: suppression is site-scoped, and CONTRIBUTING.md pins the window at MAX_MARKER_LOOKAHEAD_LINES = 8 with only blanks and comments between. The file-header marker sat 43 lines above the first `readFileSync` with requires and a function definition in between, so it was inert for both read sites. Moved to each site. The markers are belt-and-braces today, and NOT because the rule ignores `RegExp.test` -- it handles `regex.test(tracked)` explicitly (no-source-grep.cjs:239, :597-605). Neither read is tracked at all: `looksLikeSourcePath` (:378-390) admits only .cjs/.cts/.js/.mjs/.mts/.ts and UNDO_PATH is a .md, and the second site's reader is `readFileNormalized`, which the rule does not recognise as `readFileSync`. They become load-bearing if either scope widens. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * chore(#4465): record the archived-milestone refusal in the changeset The fragment described the PHASE_START bound and the fail-closed rule but not the archived-resolution refusal added this round, which is a user-visible behaviour change: `--phase N` on a number that is no longer live now stops rather than anchoring on an archived directory. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * docs(#4465): disclose the same-number-and-slug residual in undo.md The round's own cross-model body audit found this stated in the PR description and nowhere in the deployed workflow -- which is the half that survives merge, and the half this PR's whole argument says residuals belong in. The anchor is the CURRENT path and `--diff-filter=A` does not follow renames, so a later milestone that re-creates the same literal directory (`03-auth` again, not merely phase `03` again) makes the oldest add at that path the previous occupant's. The archived-milestone refusal added this round structurally cannot reach it: `find-phase` returns the LIVE directory, so nothing is under `milestones/` to refuse. Driven -- v1.0 and v2.0 both using `.planning/phases/03-auth`, v1.0 archived in between: PHASE_DIR resolves live, the guard correctly does not fire, the anchor is `docs(03-01): v1 plan`, and the selection returns all four v1+v2 phase-03 commits. `code-review.md` carries the same residual on the same anchor, where it is read-only; on `git revert --no-commit` it is not, so the note points at `/gsd:undo --last N`. Documented rather than fixed: following renames across a re-created path needs a phase identity that a directory name does not carry, which is the same wall residual 1 hits. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * fix(#4465): refuse a LIVE phase directory whose name is also archived Self-found this round, from the cross-model audit of the previous commit: that commit disclosed same-number-and-slug reuse as a residual and asserted closing it needed a phase identity a directory name does not carry. The audit refuted the second half by producing a fix, and it is cheap and safe-direction, so the case is now refused rather than documented. `--diff-filter=A` does not follow renames, so a later milestone that re-creates the same literal directory (`03-auth` again, not merely phase `03` again) anchors on the EARLIER occupant's add commit and the window opens there. The archived refusal added earlier in this round structurally cannot reach it: `find-phase` returns the LIVE directory, so nothing is under `milestones/` to refuse. Driven before the guard -- v1.0 and v2.0 both at `.planning/phases/03-auth`, v1.0 archived in between: PHASE_DIR resolves live, the archive guard correctly stays inert, the anchor is `docs(03-01): v1 plan`, and all four v1+v2 phase-03 commits are selected. That is a previous milestone's work staged for `git revert --no-commit`. The guard needs no identity reconstruction: the same basename present under an archived `milestones/v*-phases/` means the path has been used before, so the anchor is untrustworthy and both modes refuse with their own message. It fails toward refusing a legitimate undo of the reusing milestone, which `--last N` covers; the alternative is reverting the earlier one's commits. Tests: a reused-slug fixture, a negative control asserting the unguarded window reaches back into v1.0 (all four commits), refusals in both modes, and an inertness check on a distinct slug. All three new checks red against the pre-fix fence. Also narrows the changeset, which still claimed selection "no longer" reaches a previous milestone -- true of the archived route, not of this one until now -- and corrects the concurrent-workstreams residual, which claimed the window removed the previous-milestone class "entirely" while this case remained open. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * fix(#4465): harden the reused-path refusal against its own false positives The cross-model audit of the previous commit refuted four of its claims. All four were right; this closes them. A REAL BUG: the refusal message interpolated `${PHASE_DIR}` after the fence had already blanked it, so it would have rendered "Phase 03 resolves to , but that directory name is also archived at ...". The live path is now preserved in `PHASE_DIR_LIVE` before blanking, and a test pins that it survives. THREE FALSE-REFUSAL ROUTES, each of which could block a legitimate undo: - `[ -e ]` accepted a regular FILE where an archived phase directory would sit. The evidence the refusal claims is "an earlier milestone used this path", and only a directory is that. Now `[ -d ]`. - The glob `v*-phases` accepted milestone directory names `cmdFindPhase` itself rejects -- its filter is /^v\d+.*-phases$/, so `vnondigit-phases` is not a milestone it would ever resolve. Refusing over one is refusing on evidence the producer discards. Now `v[0-9]*-phases`, in the collision check and in the archived-resolution guard alike. - `${PHASE_DIR%/phases/*}` silently left the path unchanged when it carried no `/phases/` segment, so the scan ran against the wrong root and read as clean. The strip must now have fired. Outside today's producer contract either way -- cmdFindPhase's live output always carries `/phases/` -- so this hardens a claim rather than fixing a reachable defect, and is stated as such. The commit message and the changeset both asserted the guard proves the name is "archived"; before this commit it proved only that some entry existed. Both are narrowed to what the fence now actually establishes, and the changeset headline no longer claims more than the anchor plus the two refusals deliver -- residual 2 (a range is ancestry, not chronology) is untouched by either. Tests: a file-not-directory twin, a malformed `vnondigit-phases` twin, and the preserved live path. All three red against the pre-hardening fence, along with shape test M. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * docs(#4465): disclose what the reused-path scan does not survive A fifth audit pass refuted three claims in the previous commit. Two are wording, one is a real fail-open; none is fixed by more shell, so all three are stated. The wording: `v[0-9]*-phases` was described as MIRRORING cmdFindPhase's /^v\d+.*-phases$/. It is not equivalent -- the glob's `*` matches a newline where the regex's `.` does not, so a directory named `v6-phases` is accepted here and rejected there. It tracks the filter closely enough to reject the malformed siblings that motivated it; it does not mirror it, and the comment no longer says so. The changeset likewise said the twin is "an archived phase directory", where the check establishes only that a directory of that name exists under an archived milestone -- narrowed to that. The fail-open: this collision check is the only fence in undo.md that relies on pathname expansion (every other one uses `case`, which `set -f` does not affect). Under a runtime with globbing disabled the scan is skipped silently and a genuine collision passes; under `shopt -s failglob` a NON-match aborts the fence. Both sit outside the shell state this workflow assumes throughout, and defending only this fence while the rest of the file assumes defaults would be inconsistent -- so it is a documented residual rather than a hardened one. Also disclosed: the check is conservative at two edges. `[ -d ]` follows symlinks, and an empty directory of the right name counts, so either can refuse an undo a stricter ownership test would allow. That direction is the intended one -- refusing too often costs a `--last N`, refusing too rarely reverts another milestone's work. No test is added. Asserting behaviour under `set -f` would pin a shell state the workflow does not otherwise support, and the two conservative edges are the documented intent rather than defects. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016gyGdweAdAG6nFv9Jx32vj * chore(#4465): register the new regression test in the conformance-tier list Round 3's only blocker. The platform-conformance-tier classifier, its generated registry and the test that asserts the registry is fresh all landed together on `next` in `bcd99696d` (#4591 / #4598) on 2026-09-10 08:36 -0400 -- a day after this branch's last push, so the gate did not exist when the branch was last green. This PR adds a unit-suite test file, the classifier's content scan selects it, and a list regenerated on `next` without it is therefore stale the moment the two meet. (The review cites `0b928fe28c` for that landing. That commit is #4592, four hours later the same day, and `git diff-tree` shows it touched only `scripts/ci-test-scope.cjs`, `scripts/gen-platform-conformance-tier.cjs` and those two files' tests -- not the generated registry at all. The date and the diagnosis hold either way.) Regenerated with `node scripts/gen-platform-conformance-tier.cjs --write` on the rebased tree: one line, 267 -> 268 entries. The `--target macos` list is unaffected -- it writes a different file, `macos-conformance-tier.generated.cjs`, and its classifier does not select this test (198, already matching) -- so `lint:generated-sync`'s second conformance-tier link needed nothing. **Two different baselines, stated so the counts are not read as one.** CI's failure on the prior head reads `546 !== 547`, and this commit's diff reads 267 -> 268. Both are correct and they are not the same tree: at the merge commit `54b0b197f` the committed list held 546 entries, so the classifier wanted 547. `4d65c248e` (#4641, "narrow the tier to 28.5%") then landed on `next` at 2026-09-11 17:00 -0400 -- after that CI run started at 13:26Z -- and collapsed the list to 266, with `a2331c01f` taking it to 267. Hence 268 here. The two 547-entry lists are not the same file set: upstream's includes `tests/execute-phase-decimal-arithmetic.test.cjs`. Order matters and is worth stating: the registry is derived from the `tests/` tree, and the base range removed 282 entries from it and added 3 (`git diff --numstat` reports `3 282`; the familiar 279 is the net shrinkage, not the count of entries changed). Regenerating before the rebase would have produced a 547-entry list against a base carrying 267, and a three-way `git merge-file` control over that pair does conflict. Rebase first, regenerate second. This clears both reds on the prior head, not one. `lint-tests` is the one the review named; `test (ubuntu-latest, 24, shard 3/3)` is the same staleness seen through `tests/platform-conformance-tier.test.cjs:278`, which asserts the committed list is fresh. It was the only failure in 1267 tests on that shard, and it completed at 13:38:33Z -- five minutes after the review was submitted at 13:33:54Z, which is why the review recorded the ubuntu matrix as green. Control: removing the added line reds `real tests/ tree classification matches the committed list` (1 of 55, `267 !== 268`); restoring it gives 55/55. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01R4EJcuDjaaF1QeeT8AxYbj * test(#4465): pin what find-phase returns for the workstream archive layout Review round 4 found the archived-phase handling blind to the second archive layout, milestones/ws--/phases/, that `workstream complete` writes. The suite had no fixture for it and its comments described one archive reader where there are two. - Names both readers in the resolver-contract comment: find-phase routes to cmdFindPhase, which admits only /^v\d+.*-phases$/ under milestones/, while the phase locator (listArchiveVersionDirs) also enumerates ws-*. - Adds a ws-* fixture built the way `workstream complete` builds it (the whole workstream dir moved into milestones/ws-feat-/). - Negative control: a re-created workstream with the same phase slug, run without the collision guard, selects the archived generation's commits. - Pins the actual find-phase contract: a phase living only in a ws-* archive resolves to nothing, and nothing is selected. - Pins that an ordinary deleted plan file inside a live phase is not treated as a previous occupant. runFences gains an explicit env override so a test can activate a workstream after the leak scrub. Every test here passes on the pre-fix workflow; the assertions that need the fix land with it in the next commit. * fix(#4465): refuse a re-created phase path by its history, not an archive-layout glob Review round 4: the archived-phase handling encoded the archive layout as a literal `v[0-9]*-phases` glob, while the phase locator owns two layouts. The second, milestones/ws--/phases/, is what `workstream complete` writes. Driven on the real fences: a workstream `feat` completed into ws-feat-/ and then re-created with the same 03-auth slug resolves to the live path, the collision glob finds no v*-phases twin, the anchor opens on the archived generation's first commit, and --phase 03 selects that generation's commits. The collision check now asks git whether this exact path went EMPTY somewhere in HEAD's history and came back: `git log -m --no-renames --diff-filter=D` over PHASE_DIR, then `git ls-tree -d` at each deleting commit to tell a vacated directory from an ordinary deleted plan file. That answers for every way a path can be vacated -- the flat archive, the ws-* archive, and a phase removed and re-added under the same slug, which no layout glob could see -- without a second reader of the layout to keep in sync. The refusal message now names the commit that vacated the path. The archived-resolution refusal names both layouts. find-phase (cmdFindPhase) searches only the flat archives today, so a ws-*-only phase resolves to nothing and fails closed on the not-found rule; the ws-* arm keeps the refusal correct if find-phase is ever taught the locator's second layout, and a stubbed-resolver test pins it for both modes. The retired glob's shell-state residual (pathname expansion under set -f / failglob) is gone with it. Its replacement residual is documented in-workflow: the history check misses a single commit that both moves the directory away and re-creates it, and over-refuses when a side branch emptied it and the merge kept it. * fix(#4465): run undo's path-scoped git calls from the project root find-phase answers against Found by this round's pre-push review. find-phase prints PHASE_DIR relative to the PROJECT ROOT -- gsd-tools resolves the root before it dispatches -- while the workflow's shell stays wherever the user invoked /gsd:undo. A git pathspec is read relative to git's cwd, so from a subdirectory every PHASE_DIR-scoped call looked in the wrong place: - the anchor (`git log --diff-filter=A`) came back empty, so a legitimate --phase or --plan undo was refused. Fail-closed, but a refusal of valid work, and it dates from round 1; - the collision check's `git log --diff-filter=D` came back empty, so a re-created path was never recognised. The anchor failing too is the only reason this did not over-select. Both modes now take PROJECT_ROOT from `planning inspect --pick generated_from.cwd` -- the directory find-phase itself resolved against -- and run the three path-scoped calls as `git -C "${PROJECT_ROOT:-.}"`. The project root, not `git rev-parse --show-toplevel`: a project need not sit at the top of its repository, and a control test pins that choice. Selection, revert and rev-parse calls are SHA-only and unchanged. Tests: every behavioural test ran from the fixture root, where the two readings coincide. New ones run the fences from sub/dir -- normal selection in both modes, refusal of a re-created path for both archive layouts in both modes, and a dropped plan file NOT refused, which is how a mis-rooted ls-tree would fail (it lists nothing, and nothing reads as "vacated"). Shape test O pins every PHASE_DIR-scoped git call to the root. The two tests renamed in the previous commit are relabelled as false-positive controls: they pass with the collision check deleted, and say so. Reversion controls, all driven: the pre-change undo.md fails 7 tests; one mode's ls-tree mis-rooted fails 3 in either mode; PROJECT_ROOT from --show-toplevel fails 2. * fix(#4465): refuse a phase planned in another repository, and never widen a foreign anchor Found by this round's second pre-push review, against the previous commit. Running the anchor from the project root is right while the project root and the caller share a repository. In a `sub_repos` project they do not: .planning/ lives in a parent repository and the code in child ones, and from a child findProjectRoot returns the parent. The anchor then came from the PARENT's history -- a commit the child does not hold. `git rev-parse "${PHASE_START}^"` failed in the child, the root-commit arm read that as "no parent", set UNDO_RANGE=HEAD, and selection ran over the child's whole history. Driven: both child commits selected, including one older than the phase. Before the previous commit that case resolved no anchor and failed closed; the previous commit made it destructive. Two layers, both modes: - a same-repository refusal in the guard fence: the caller's git directory and PROJECT_ROOT's, each physically resolved, must be the same one (per-worktree, so a linked worktree compares correctly). A different one sets PHASE_DIR_FOREIGN, blanks PHASE_DIR, and the workflow stops with a --last N pointer; - in the anchor: a PHASE_START that is not a commit this repository holds is blanked before the root-commit arm. That ambiguity -- "no parent" read as "root commit" when it can also mean "not a commit here" -- dates from round 1; the previous commit made it reachable. Shape test O now also pins that PROJECT_ROOT is assigned only from planning inspect: a rooted call is only as good as the root it is handed, and a later reassignment passed every spelling check (review, claim 8). Shape test P pins both layers in both modes. Correction to the previous commit's message: it said an empty anchor was the only reason a missed collision could not over-select before that commit. The review drove a counterexample -- a tracked `.planning` path coinciding with the caller's subdirectory resolves a wrong, non-empty anchor -- so that sentence overstated it. Reversion controls, driven: the previous commit's undo.md fails 3 (P, the sub_repos refusal, defense in depth); the refusal made inert fails 2 (P and the refusal; the anchor check alone still keeps the range empty); the anchor check made inert fails 2 (P and defense in depth; the refusal alone still refuses). A verbatim negative control shows the pre-hardening root-commit arm widening the parent's anchor to all of HEAD. * fix(#4465): compare the repository, not the worktree, and anchor only inside HEAD's history Found by this round's third pre-push review, against the previous commit. Its repository gate compared per-worktree git directories. gsd-tools maps a linked worktree with no .planning/ of its own to the MAIN worktree (resolveMainWorktreeCwd), so from such a worktree PROJECT_ROOT is the main checkout: one repository, one object database, two per-worktree git dirs -- and the gate refused a legitimate undo. It now compares the COMMON git directory, physically resolved, which is the repository's identity; a sub_repos child is still a different one and is still refused. Driving that case showed the anchor check was too weak for it. The anchor comes from the main worktree's branch history, and nothing guaranteed the linked HEAD contains it: a linked branch that left main before the phase started would get a window bounded by a commit outside its own history. The check is now "PHASE_START is an ancestor of HEAD" (`git merge-base --is-ancestor`), which subsumes the previous "is a commit here" test -- a missing object is not an ancestor either -- and changes nothing in the ordinary case, where the anchor is read from HEAD's own log. Tests: a linked-worktree fixture (the linked checkout has no .planning/, which is what makes gsd-tools map it to main; the test asserts the mapping happened) is allowed in both modes and selects the phase; the same shape branched before the phase resolves no range. Shape test P pins the common-dir comparison, forbids --absolute-git-dir, and pins the ancestry check. Reversion controls, driven: the previous commit's undo.md fails 3 (P and both linked-worktree tests); per-worktree git dirs restored fails the same 3; the ancestry check replaced by the previous cat-file test fails 2 (P and the branched-before test, which then selects a linked commit whose history never held the phase); no anchor check at all fails 3 (P, defense in depth, the branched-before test). --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Tom Boucher --- .changeset/undo-bounded-commit-selection.md | 5 + gsd-core/workflows/undo.md | 334 ++++- .../platform-conformance-tier.generated.cjs | 1 + tests/undo-commit-selection-4465.test.cjs | 1199 +++++++++++++++++ 4 files changed, 1521 insertions(+), 18 deletions(-) create mode 100644 .changeset/undo-bounded-commit-selection.md create mode 100644 tests/undo-commit-selection-4465.test.cjs diff --git a/.changeset/undo-bounded-commit-selection.md b/.changeset/undo-bounded-commit-selection.md new file mode 100644 index 000000000..edd72d530 --- /dev/null +++ b/.changeset/undo-bounded-commit-selection.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4472 +--- +**`/gsd:undo --phase` and `--plan` no longer select commits from an unreachable branch, and refuse rather than anchor on a directory an earlier milestone used** — commit selection now anchors on the target phase's own directory (the `#3995` `PHASE_START` pattern already used by `code-review.md`) and runs over `PHASE_START^..HEAD` instead of a repository-wide `git log --all` commit-subject grep. The dead `.planning/.phase-manifest.json` primary path, which nothing in the repository writes, is removed rather than left as documented-but-unreachable behaviour, and both modes now fail closed when no anchor resolves instead of widening to an unbounded search. Two refusals cover the previous-milestone routes the anchor cannot distinguish on its own: a phase number that resolves to an archived `milestones/v-phases/` directory, and a **live** directory whose name also exists as a directory under an archived milestone (a later milestone reusing both the number and the slug re-creates the same literal path, so the oldest add there is the earlier occupant's). The first: `find-phase`'s ambiguity check does not span search directories, so a number that is no longer live resolves silently to the oldest archived milestone that has one — and anchoring there both misses that phase's real work and drags a *later* milestone's same-numbered phase into the window. `dependency_check` reads the workstream-resolved planning root, so an active workstream's roadmap and phase directories are consulted rather than the root's. (#4465) diff --git a/gsd-core/workflows/undo.md b/gsd-core/workflows/undo.md index dacc830aa..98457605f 100644 --- a/gsd-core/workflows/undo.md +++ b/gsd-core/workflows/undo.md @@ -1,5 +1,5 @@ -Safe git revert workflow. Rolls back GSD phase or plan commits using the phase manifest with dependency checks and a confirmation gate. Uses git revert --no-commit (NEVER git reset) to preserve history. +Safe git revert workflow. Rolls back GSD phase or plan commits selected within the phase directory's own commit window with dependency checks and a confirmation gate. Uses git revert --no-commit (NEVER git reset) to preserve history. @@ -78,30 +78,318 @@ Parse the user's selection into COMMITS list. **MODE=phase:** -Read `.planning/.phase-manifest.json` if it exists. +Resolve the phase's own directory, then anchor the selection window on it. `find-phase` +resolves through `planningDir`, so under an active workstream this is that workstream's +phase directory — not the root's same-numbered one. -If the file exists and `manifest.phases?.[TARGET_PHASE]?.commits` is a non-empty array: - - Use `manifest.phases[TARGET_PHASE].commits` entries as COMMITS (each entry is a commit hash) +```bash +PHASE_DIR=$(gsd_run query find-phase "${TARGET_PHASE}" --raw 2>/dev/null) +# find-phase answers relative to the PROJECT ROOT -- gsd-tools resolves it before dispatch -- +# not to this shell's cwd, so from a subdirectory a bare `git log -- "${PHASE_DIR}"` looks in +# the wrong place and every path-scoped git call below comes back empty. Take the root from the +# same owner and run those calls there. Unresolved, `.` keeps the behaviour at the root. A root +# in a DIFFERENT repository from this shell's is refused below, never used. +PROJECT_ROOT=$(gsd_run query planning inspect --pick generated_from.cwd --raw 2>/dev/null) +``` -If the file does not exist, or `manifest.phases?.[TARGET_PHASE]` is missing: - - Display: "Manifest has no entry for phase ${TARGET_PHASE} (or file missing), falling back to git log search" - - Fallback: run git log and filter for the target phase scope: - ```bash - git log --oneline --no-merges --all | grep -E "\(0*${TARGET_PHASE}(-[0-9]+)?\):" | head -50 - ``` - - Use matching commits as COMMITS +If `PHASE_DIR` is empty, the phase does not exist in the active scope: +``` +Phase ${TARGET_PHASE} not found in the active planning scope. Nothing to revert. +``` +Exit cleanly — do NOT fall back to an unbounded search. + +**Refuse an ARCHIVED resolution — it selects the wrong milestone, not merely too few +commits.** `find-phase` searches the live `phases/` directory first, then every +`milestones/v-phases/` directory in ascending version order, and its ambiguity check +is scoped to a *single* directory — it does not span them (`cmdFindPhase`, `src/phase.cts`: +the `matches.length > 1` test sits inside the per-`searchDir` loop). A phase number that is +not live therefore resolves **silently to the oldest archived milestone that has one**, +with no warning. Anchoring there is wrong in both directions at once: the oldest commit +adding that path is the archival **move**, so the phase's real work predates the window and +falls outside it, while the window runs forward from that archival through every later +milestone — where the subject grep matches *their* same-numbered phase. Driven on a +two-archived-milestone fixture, `--phase 03` selected v2.0's `feat(03-01): add search index` +and excluded v1.0's own `feat(03-01): implement auth endpoint`. Feeding that to +`git revert` is the cross-milestone contamination this workflow exists to close, so it +fails closed. + +**Two archive layouts exist, and `find-phase` searches only one of them.** The phase locator +(`listArchiveVersionDirs`, `src/phase-locator.cts`) enumerates both the flat +`milestones/v-phases//` archive and the workstream archive +`milestones/ws--/phases//` that `workstream complete` writes. `cmdFindPhase` +builds its own search list and admits only the first (`/^v\d+.*-phases$/`), so a phase that lives +*only* in a `ws-*` archive resolves to nothing today and the not-found rule above fails closed. +The refusal below still names both layouts, so it keeps holding if `find-phase` is ever taught the +second one: + +```bash +# Match the ARCHIVE LAYOUTS, never the bare token `milestones`. A bare `*/milestones/*` +# REFUSES A LIVE PHASE whenever a workstream or project is itself named `milestones` +# (driven: GSD_WORKSTREAM=milestones resolves `.planning/workstreams/milestones/phases/03-live`, +# which that pattern classifies as archived). The two shapes are the locator's two: +# `v-phases/` and `ws--/phases/`. Blanking PHASE_DIR is +# deliberate: the fail-closed rule below then also holds, so no path reaches selection even if +# this refusal's prose is not honored. +PHASE_DIR_ARCHIVED="" +case "${PHASE_DIR}" in + */milestones/v[0-9]*-phases/*|milestones/v[0-9]*-phases/*|*/milestones/ws-*/phases/*|milestones/ws-*/phases/*) + PHASE_DIR_ARCHIVED="${PHASE_DIR}"; PHASE_DIR="" ;; +esac +# The phase directory must live in the SAME repository as the commits this workflow reverts. In a +# `sub_repos` project, .planning/ sits in a parent repository and the code in child ones; from a +# child, PROJECT_ROOT is the parent, and an anchor read there is a commit the child has never seen. +# Compare the COMMON git directories -- the object database -- physically resolved; a different one +# refuses. Common, not per-worktree: gsd-tools maps a linked worktree's planning to the MAIN +# worktree, which is the same repository and holds every commit the linked one does. +PHASE_DIR_FOREIGN="" +if [ -n "${PHASE_DIR}" ] && [ -n "${PROJECT_ROOT}" ]; then + _gd_here=$(_d=$(git rev-parse --git-common-dir 2>/dev/null) && [ -n "$_d" ] && cd "$_d" && pwd -P) || _gd_here="" + _gd_root=$(cd "${PROJECT_ROOT}" 2>/dev/null && _d=$(git rev-parse --git-common-dir 2>/dev/null) && [ -n "$_d" ] && cd "$_d" && pwd -P) || _gd_root="" + if [ -z "$_gd_here" ] || [ "$_gd_here" != "$_gd_root" ]; then + PHASE_DIR_FOREIGN="${PROJECT_ROOT}"; PHASE_DIR="" + fi +fi +# A LIVE path can still be a previous occupant's. `--diff-filter=A` does not follow renames, +# so re-creating a literal directory that an earlier milestone or workstream used anchors on +# the EARLIER occupant's add. Nothing is under milestones/ to refuse -- find-phase returned the +# live dir -- so ask the question the anchor depends on instead: did this exact path go EMPTY +# somewhere in HEAD's history and come back? Git history owns that answer for every way a path +# can be vacated -- a flat milestone archive, `workstream complete` moving the workstream into +# milestones/ws--/, a removed phase re-added under the same slug -- where a layout +# glob answers only for the layouts it spells. `--no-renames` so a move-out reads as the deletion +# it is at this path; `-m` so a merge that emptied it is seen too; `ls-tree` at that commit is +# what separates "the directory went away" from an ordinary deleted plan file. +PHASE_DIR_REUSED=""; PHASE_DIR_LIVE="" +if [ -n "${PHASE_DIR}" ]; then + for _c in $(git -C "${PROJECT_ROOT:-.}" log -m --no-renames --diff-filter=D --format=%H -- "${PHASE_DIR}" 2>/dev/null); do + if [ -z "$(git -C "${PROJECT_ROOT:-.}" ls-tree -d "$_c" -- "${PHASE_DIR}" 2>/dev/null)" ]; then + PHASE_DIR_LIVE="${PHASE_DIR}"; PHASE_DIR_REUSED="$_c"; PHASE_DIR=""; break + fi + done +fi +``` + +If `PHASE_DIR_ARCHIVED` is non-empty, stop — this message, not the not-found one: +``` +Phase ${TARGET_PHASE} resolves to an ARCHIVED milestone directory (${PHASE_DIR_ARCHIVED}). +Refusing: the anchor there is the archival commit, so the window would span later +milestones and select their same-numbered phase instead of this one. +Use /gsd:undo --last N and select commits explicitly. +``` +If `PHASE_DIR_FOREIGN` is non-empty, stop with its own message: +``` +Phase ${TARGET_PHASE} is planned in the repository at ${PHASE_DIR_FOREIGN}, not the one this +command is running in. Refusing: that repository's history cannot bound commits in this one. +Use /gsd:undo --last N here and select commits explicitly. +``` +And if `PHASE_DIR_REUSED` is non-empty, stop with its own message: +``` +Phase ${TARGET_PHASE} resolves to ${PHASE_DIR_LIVE}, but that path was emptied by commit +${PHASE_DIR_REUSED} and re-created later. Refusing: the first commit adding this path belongs +to the earlier occupant, so the window would open there and select its commits too. +Use /gsd:undo --last N and select commits explicitly. +``` +Exit cleanly in every case. + +Derive the selection window from `PHASE_DIR` (the `#3995` anchor, shared with +`code-review.md`): the base is the parent of the first commit that added anything under +the phase's own directory, and the tip is `HEAD`. + +```bash +PHASE_START=$(git -C "${PROJECT_ROOT:-.}" log --format="%H" --diff-filter=A -- "${PHASE_DIR}" 2>/dev/null | tail -1) +# Only a commit in HEAD's own history may anchor. A SHA from another repository has no resolvable +# parent here, which the root-commit arm below would read as "root commit" and select all of HEAD; +# one from another worktree's branch that HEAD does not contain bounds nothing on this branch. +if [ -n "$PHASE_START" ] && ! git merge-base --is-ancestor "$PHASE_START" HEAD 2>/dev/null; then PHASE_START=""; fi +UNDO_RANGE="" +if [ -n "$PHASE_START" ]; then + if git rev-parse "${PHASE_START}^" >/dev/null 2>&1; then + UNDO_RANGE="${PHASE_START}^..HEAD" + else + # PHASE_START is the root commit — it has no parent to exclude. `${PHASE_START}..HEAD` + # would drop PHASE_START ITSELF, refusing a legitimate revert of the first commit. + UNDO_RANGE="HEAD" + fi +fi +``` + +**Fail closed when no anchor resolves.** If `UNDO_RANGE` is empty, stop: +``` +Cannot determine a reliable commit window for phase ${TARGET_PHASE} (no commit adds ${PHASE_DIR}). +Re-run with /gsd:undo --last N and select commits explicitly. +``` +Exit cleanly. An unbounded repository-wide search is never the fallback — that is the +defect this anchor replaces. + +Select within the window. **No `--all`:** only commits reachable from `HEAD` may be +reverted, because reverting a commit that is not in the current branch's history stages a +change the branch never received. + +```bash +# `|| true`: grep exits 1 on no match. The former `| head -50` masked that rc; the empty +# case is handled by the Empty check step below, so the pipeline must not abort here. +git log --oneline --no-merges "${UNDO_RANGE}" | grep -E "\(0*${TARGET_PHASE}(-[0-9]+)?\):" || true +``` + +Use matching commits as COMMITS. + +**Report truncation, never truncate silently.** If the selection exceeds 50 commits, show +the count and stop rather than capping — a partial phase revert leaves a worse tree state +than either reverting the phase or not: +``` +Phase ${TARGET_PHASE} selects ${N} commits (>50). Refusing to revert a partial phase. +Use /gsd:undo --plan NN-MM per plan, or /gsd:undo --last N. +``` --- **MODE=plan:** -Run: +Extract the phase number from `TARGET_PLAN` (the `NN` of `NN-MM`) and derive the same +window from that phase's own directory — a plan number is unique within its phase, and a +phase number only within its milestone and workstream. + ```bash -git log --oneline --no-merges --all | grep -E "\(${TARGET_PLAN}\)" | head -50 +PLAN_PHASE="${TARGET_PLAN%%-*}" +PHASE_DIR=$(gsd_run query find-phase "${PLAN_PHASE}" --raw 2>/dev/null) +# Project-root-relative, exactly as in MODE=phase: path-scoped git calls run from the root. +PROJECT_ROOT=$(gsd_run query planning inspect --pick generated_from.cwd --raw 2>/dev/null) +# Same archived-resolution refusal as MODE=phase, and for the same reason — an archived +# anchor selects a LATER milestone's same-numbered phase. Blanking PHASE_DIR keeps the +# fail-closed rule below load-bearing. +PHASE_DIR_ARCHIVED="" +case "${PHASE_DIR}" in + */milestones/v[0-9]*-phases/*|milestones/v[0-9]*-phases/*|*/milestones/ws-*/phases/*|milestones/ws-*/phases/*) + PHASE_DIR_ARCHIVED="${PHASE_DIR}"; PHASE_DIR="" ;; +esac +# Same-repository refusal as MODE=phase: a phase planned in another repository cannot anchor here. +PHASE_DIR_FOREIGN="" +if [ -n "${PHASE_DIR}" ] && [ -n "${PROJECT_ROOT}" ]; then + _gd_here=$(_d=$(git rev-parse --git-common-dir 2>/dev/null) && [ -n "$_d" ] && cd "$_d" && pwd -P) || _gd_here="" + _gd_root=$(cd "${PROJECT_ROOT}" 2>/dev/null && _d=$(git rev-parse --git-common-dir 2>/dev/null) && [ -n "$_d" ] && cd "$_d" && pwd -P) || _gd_root="" + if [ -z "$_gd_here" ] || [ "$_gd_here" != "$_gd_root" ]; then + PHASE_DIR_FOREIGN="${PROJECT_ROOT}"; PHASE_DIR="" + fi +fi +# A LIVE path can still be a previous occupant's -- same question, same answer as MODE=phase: +# a path that went EMPTY in HEAD's history and came back anchors on the earlier occupant's add, +# whatever vacated it. Ask git, not a layout glob. Fail closed; `--last N` is the route. +PHASE_DIR_REUSED=""; PHASE_DIR_LIVE="" +if [ -n "${PHASE_DIR}" ]; then + for _c in $(git -C "${PROJECT_ROOT:-.}" log -m --no-renames --diff-filter=D --format=%H -- "${PHASE_DIR}" 2>/dev/null); do + if [ -z "$(git -C "${PROJECT_ROOT:-.}" ls-tree -d "$_c" -- "${PHASE_DIR}" 2>/dev/null)" ]; then + PHASE_DIR_LIVE="${PHASE_DIR}"; PHASE_DIR_REUSED="$_c"; PHASE_DIR=""; break + fi + done +fi +PHASE_START=$(git -C "${PROJECT_ROOT:-.}" log --format="%H" --diff-filter=A -- "${PHASE_DIR}" 2>/dev/null | tail -1) +# As in MODE=phase: an anchor outside HEAD's own history never reaches the root-commit arm. +if [ -n "$PHASE_START" ] && ! git merge-base --is-ancestor "$PHASE_START" HEAD 2>/dev/null; then PHASE_START=""; fi +UNDO_RANGE="" +if [ -n "$PHASE_START" ]; then + if git rev-parse "${PHASE_START}^" >/dev/null 2>&1; then + UNDO_RANGE="${PHASE_START}^..HEAD" + else + # PHASE_START is the root commit — it has no parent to exclude. `${PHASE_START}..HEAD` + # would drop PHASE_START ITSELF, refusing a legitimate revert of the first commit. + UNDO_RANGE="HEAD" + fi +fi +``` + +Apply the same fail-closed rule as MODE=phase when `PHASE_DIR` or `UNDO_RANGE` is empty — +and the same three refusals, each with its own message, when `PHASE_DIR_ARCHIVED`, +`PHASE_DIR_FOREIGN` or `PHASE_DIR_REUSED` is non-empty — then select within the window: + +```bash +# `|| true` for the same reason as MODE=phase: an empty selection is not an error here. +git log --oneline --no-merges "${UNDO_RANGE}" | grep -E "\(${TARGET_PLAN}\):" || true ``` Use matching commits as COMMITS. +**Report truncation, never truncate silently** — the same rule as MODE=phase. If the +selection exceeds 50 commits, show the count and stop rather than capping: +``` +Plan ${TARGET_PLAN} selects ${N} commits (>50). Refusing to revert a partial plan. +Use /gsd:undo --last N and select commits explicitly. +``` + +--- + +**Known residual — a revision range is ancestry, not chronology.** `PHASE_START^..HEAD` +excludes everything reachable from `PHASE_START^`, which is the right bound for the +ordinary linear case. It is not a *chronological* lower bound: a long-lived side branch +created before the phase, carrying matching scopes, and merged in **after** `PHASE_START` +is reachable from `HEAD` without being an ancestor of `PHASE_START^`, so it stays +selectable. This is strictly narrower than the unbounded search it replaces, not a new +exposure — but it is not zero. + +**Known residual — a RENAMED phase directory under-selects.** The anchor is +`--diff-filter=A` on the phase directory's *current* path and does not follow renames, so +for a phase whose directory has since moved the oldest add at that path is the **move** +commit, and the phase's real work commits — which predate it — fall outside the window. +For a rename *within* the live `phases/` tree the failure is under-selection: the undo +reverts too little or refuses, never too much. + +The **archival** case is not that case, and is no longer a residual — it is refused above. +It was previously documented here as under-selection only, which was wrong in the direction +that matters: the window runs forward from the archival commit, so while the target's own +work falls outside it, a *later* milestone's same-numbered phase falls inside and matches +the subject grep. Driven on a two-archived-milestone fixture it selected the wrong +milestone's commit and none of the right one's. Both modes now refuse an archived +`PHASE_DIR` outright; `/gsd:undo --last N` is the route for a phase that has been archived. + +**Known residual — a phase directory introduced by a merge commit resolves no anchor.** +`git log --diff-filter=A -- "${PHASE_DIR}"` does not walk merge diffs by default. A phase +directory added on a side branch is still found, because the side-branch commit that added +it is itself in history; the uncovered case is a directory that first appears *in the merge +resolution itself*, which a **default** `git log` does not show: it suppresses merge diffs +unless asked (`-m` prints the add once per parent, so the information exists — the anchor +command simply does not request it). `PHASE_START` then resolves +empty and both modes fail closed on a legitimate phase. Safe-direction only — it refuses +rather than mis-selects — and untested: constructing the evil-merge fixture costs more than +the branch is worth while the failure mode is a refusal. `/gsd:undo --last N` is the route +if it is ever hit. + +**A re-created directory — same number AND same slug — is REFUSED, not a residual.** The +anchor is the *current path*, and `--diff-filter=A` does not follow renames, so re-creating +a literal directory an earlier occupant used (`03-auth` again, not merely phase `03` again) +makes the oldest add at that path the **previous occupant's**. The archived refusal above +cannot reach it — `find-phase` returns the **live** directory, so nothing is under +`milestones/` to refuse. Driven before the guard: two milestones both using +`.planning/phases/03-auth` anchored on the v1 plan commit and selected all four v1+v2 phase-03 +commits; a workstream completed into `milestones/ws-feat-/` and then re-created as `feat` +with the same `03-auth` did the same across the two workstream generations. The collision check +closes both without a phase identity a directory name does not carry, and without restating any +archive layout: a path that went empty in `HEAD`'s history and came back has had a previous +occupant, so the anchor is untrustworthy and both modes refuse. `code-review.md` carries the +same weakness on the same anchor, where it is read-only and merely widens a review scope; here +it reverts, which is why this one is a refusal rather than a note. + +**Known residual — the collision check reads history, so it sees only what history shows.** +Two edges, in opposite directions. It **misses** a single commit that both moves the directory +away *and* re-creates it at the same path: the path is never empty in any commit's tree, so the +history carries no vacancy to find, and the anchor opens on the earlier occupant. That takes a +hand-assembled commit — it is not the shape of an archive followed by later planning — and the +over-selection it allows still has to pass `confirm_revert`. It **over-refuses** when a side +branch emptied the directory and the merge kept it: the vacancy is real in that branch's +history, so the path reads as reused. Refusing too often costs a `--last N`; refusing too +rarely reverts another occupant's work, which is why the check is keyed on the vacancy itself +rather than on any narrower proof of ownership. + +**Known residual — concurrent workstreams.** The window above is scoped to the target +phase's own directory, which is workstream-correct, but the commit subjects it filters +are not: the executor's scope contract is `type({phase}-{plan})` with no workstream +token, so two workstreams running the same phase number concurrently emit +indistinguishable subjects and both fall inside each other's window. Narrowing the window plus the two +refusals above removes the unreachable-branch class entirely and every previous-milestone +route this workflow can detect — residual 2 is the one it cannot, since a merged side branch +is genuinely reachable from `HEAD`. This last class +needs a discriminator that does not exist in a commit subject today (`#3995`: *"Message +subjects demonstrably do not carry enough information to identify a phase"*). Until one +exists, `confirm_revert` is the backstop for it. + --- **Empty check:** @@ -118,11 +406,20 @@ Exit cleanly. Skip this step entirely for MODE=last. +Resolve the active scope's planning root first — **both** modes below read from it. Under +an active workstream the roadmap and phase directories describing the target are that +workstream's, not the root's: + +```bash +PLANNING_DIR=$(gsd_run query planning inspect --pick generated_from.planning_root --raw 2>/dev/null) +[ -n "$PLANNING_DIR" ] || PLANNING_DIR=".planning" +``` + --- **MODE=phase:** -Read `.planning/ROADMAP.md` inline. +Read `${PLANNING_DIR}/ROADMAP.md` inline. Search for phases that list a dependency on the target phase. Look for patterns like: - "Depends on: Phase ${TARGET_PHASE}" @@ -130,7 +427,7 @@ Search for phases that list a dependency on the target phase. Look for patterns - "depends_on: [${TARGET_PHASE}]" For each dependent phase N found: -1. Check if `.planning/phases/${N}-*/` directory exists +1. Check if `${PLANNING_DIR}/phases/${N}-*/` directory exists 2. If directory exists, check for any PLAN.md or SUMMARY.md files inside it If any downstream phase has started work, collect warnings: @@ -145,7 +442,8 @@ If any downstream phase has started work, collect warnings: Extract the phase number from TARGET_PLAN (the NN part of NN-MM). Extract the plan number (the MM part). -Look for later plans in the same phase directory (`.planning/phases/${NN}-*/`). For each later plan (plans with number > MM): +Look for later plans in the same phase directory (`${PLANNING_DIR}/phases/${NN}-*/`, the +same workstream-resolved root). For each later plan (plans with number > MM): 1. Read the later plan's PLAN.md 2. Check if its `` sections or `consumes` fields reference outputs from the target plan @@ -300,8 +598,8 @@ Show next steps: - [ ] Arguments parsed correctly for all three modes -- [ ] --phase mode reads .planning/.phase-manifest.json using manifest.phases[TARGET_PHASE].commits -- [ ] --phase mode falls back to git log if manifest entry missing +- [ ] --phase mode anchors selection on the phase's own directory (find-phase -> PHASE_START), never a repository-wide commit-subject grep +- [ ] --phase and --plan modes fail closed when no anchor resolves, never widening to an unbounded search - [ ] Dependency check warns when downstream phases have started (MODE=phase) - [ ] Dependency check warns when later plans reference target plan outputs (MODE=plan) - [ ] Dirty-tree guard aborts if working tree has uncommitted changes diff --git a/scripts/lib/platform-conformance-tier.generated.cjs b/scripts/lib/platform-conformance-tier.generated.cjs index 44b239f5e..02a7e850c 100644 --- a/scripts/lib/platform-conformance-tier.generated.cjs +++ b/scripts/lib/platform-conformance-tier.generated.cjs @@ -255,6 +255,7 @@ module.exports = { "tests/state.test.cjs", "tests/teams-status.test.cjs", "tests/todos-workstream-scope.test.cjs", + "tests/undo-commit-selection-4465.test.cjs", "tests/unreachable-guard-drift.test.cjs", "tests/unreachable-shell-guard.test.cjs", "tests/unusable-input.test.cjs", diff --git a/tests/undo-commit-selection-4465.test.cjs b/tests/undo-commit-selection-4465.test.cjs new file mode 100644 index 000000000..29dcff993 --- /dev/null +++ b/tests/undo-commit-selection-4465.test.cjs @@ -0,0 +1,1199 @@ +// This file reads .md product files whose deployed text IS what the runtime loads, so +// testing text content tests the deployed contract. Suppression is SITE-scoped, not +// file-wide (CONTRIBUTING.md: the marker must sit within MAX_MARKER_LOOKAHEAD_LINES = 8 +// of the line it covers, with nothing but blanks and comments between) — so the +// `allow-test-rule` markers live next to the two read sites below, not up here. +// +// Those markers are belt-and-braces today, and the reason is NOT that the rule ignores +// `RegExp.test` — it handles `regex.test(tracked)` explicitly (no-source-grep.cjs:239, +// :597-605). It is that neither read is tracked in the first place: `looksLikeSourcePath` +// (:378-390) admits only .cjs/.cts/.js/.mjs/.mts/.ts and UNDO_PATH is a .md, and the +// second site's reader is `readFileNormalized`, which the rule does not recognise as +// `readFileSync`. The markers are correct where they now sit, and become load-bearing if +// either scope widens. + +/** + * #4465 — /gsd:undo commit selection must be milestone-bounded and HEAD-reachable. + * + * The defect: `--phase` documented a primary path reading `.planning/.phase-manifest.json`, + * a file nothing in the repository writes, so the documented fallback was the only real + * path — `git log --oneline --no-merges --all | grep -E "\(0*${TARGET_PHASE}...` — with no + * milestone bound and no reachability bound. Feeding that selection to `git revert + * --no-commit` stages deletion of a previous milestone's files. + * + * The fix ports #3995's PHASE_START anchor (already live in code-review.md) to both + * `--phase` and `--plan`, drops `--all`, and fails closed instead of widening. + * + * Two halves. The first pins the SHAPE of undo.md's fences (substring assertions over + * the deployed prose — each half's own read site carries the marker). The second + * EXECUTES those fences against a real git fixture and the real `gsd-tools.cjs`, so + * a fence that matches the expected text but does not do the expected thing still fails. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { scanFencedBlocks } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); + +const UNDO_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'undo.md'); + +/** Return the raw text of every ```bash fenced block in `content`. */ +function extractBashBlocks(content) { + const lines = content.split(/\r?\n/); + const blocks = []; + for (const block of scanFencedBlocks(lines)) { + if (block.closeLineIdx === -1) continue; + if ((block.infoString || '').trim().toLowerCase() !== 'bash') continue; + blocks.push(lines.slice(block.openLineIdx, block.closeLineIdx + 1).join('\n')); + } + return blocks; +} + +describe('#4465: undo commit selection is bounded', () => { + // allow-test-rule: source-text-is-the-product (see #4465) + const content = fs.readFileSync(UNDO_PATH, 'utf-8'); + const bash = extractBashBlocks(content).join('\n'); + + test('A: no commit-selection git log uses --all', () => { + // `--all` searches every ref, so a commit unreachable from HEAD can be selected + // for revert. Every selection block must be HEAD-reachable. + const offenders = extractBashBlocks(content).filter( + (b) => /git log/.test(b) && /--all\b/.test(b), + ); + assert.deepEqual( + offenders, [], + `undo.md must not select commits with 'git log ... --all' (#4465). Offending block(s):\n${offenders.join('\n---\n')}`, + ); + }); + + test('B: the dead .phase-manifest.json path is gone', () => { + // Nothing in the repository writes this file, so the documented primary path was + // permanently unreachable and the unbounded fallback was the only real path. + assert.ok( + !/phase-manifest/.test(content), + 'undo.md must not read or assert .planning/.phase-manifest.json — nothing writes it (#4465)', + ); + }); + + test('C: both modes anchor on the phase directory via PHASE_START', () => { + assert.ok( + /PHASE_START=\$\(git -C "\$\{PROJECT_ROOT:-\.\}" log --format="%H" --diff-filter=A -- "\$\{PHASE_DIR\}"/.test(bash), + 'undo.md must derive PHASE_START from the phase directory (the #3995 anchor), from the project root', + ); + // Two derivations: one for MODE=phase, one for MODE=plan. + const anchors = (bash.match(/--diff-filter=A -- "\$\{PHASE_DIR\}"/g) || []).length; + assert.equal(anchors, 2, 'both --phase and --plan must anchor on PHASE_DIR (#4465)'); + }); + + test('O: every PHASE_DIR-scoped git call runs from the project root find-phase answers against', () => { + // find-phase prints a path relative to the PROJECT ROOT (gsd-tools resolves it before + // dispatch), while the workflow's shell stays wherever the user invoked it. A pathspec is + // read relative to git's cwd, so from a subdirectory a bare `git log -- "${PHASE_DIR}"` + // matches nothing: the anchor comes back empty and a legitimate undo is refused, and the + // collision check comes back empty and never fires (review round 5, self-found). + const roots = (bash.match(/PROJECT_ROOT=\$\(gsd_run query planning inspect --pick generated_from\.cwd --raw/g) || []).length; + assert.equal(roots, 2, 'both modes must take PROJECT_ROOT from the same owner as find-phase (#4465)'); + const code = bash.split('\n').filter((l) => !/^\s*#/.test(l)); + // And from nowhere else: a rooted call is only as good as the root it is handed, so a later + // reassignment would pass every spelling check below (review round 5, claim 8). + const assigns = code.filter((l) => /\bPROJECT_ROOT=/.test(l)); + assert.equal(assigns.length, 2, `PROJECT_ROOT may be assigned only from planning inspect; found:\n${assigns.join('\n')}`); + const scoped = code.filter((l) => /\bgit\b.*-- "\$\{PHASE_DIR\}"/.test(l)); + // Per mode: the collision log, the collision ls-tree, the anchor. + assert.equal(scoped.length, 6, `expected three PHASE_DIR-scoped git calls per mode; found:\n${scoped.join('\n')}`); + // Count every git on the line: one rooted call must not excuse a second, unrooted one. + const unrooted = scoped.filter((l) => (l.match(/\bgit\b/g) || []).length + !== (l.match(/\bgit -C "\$\{PROJECT_ROOT:-\.\}" (log|ls-tree)\b/g) || []).length); + assert.deepEqual(unrooted, [], + 'a PHASE_DIR-scoped git call not run as git -C "${PROJECT_ROOT:-.}" misreads the path from a subdirectory (#4465)'); + }); + + test('P: a phase planned in another repository refuses, and a foreign anchor never widens to HEAD', () => { + // In a sub_repos project the project root is a PARENT repository. An anchor read there is a + // commit the caller's repository does not hold, and the root-commit arm reads "no parent" as + // "root commit" and selects all of HEAD (review round 5, driven). Two layers, both modes: + // the same-repository refusal, and an anchor that must be a commit in this repository. + const code = extractBashBlocks(content) + .map((b) => b.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n')); + // The COMMON git directory is the repository's identity: a linked worktree has its own per-worktree + // git dir but shares the object database, and gsd-tools maps its planning to the MAIN worktree, so a + // per-worktree comparison refuses a legitimate undo there (review round 5, third pass). + const gates = code.filter((b) => /PHASE_DIR_FOREIGN=""/.test(b) + && /_gd_here=\$\(_d=\$\(git rev-parse --git-common-dir 2>\/dev\/null\)/.test(b) + && /_gd_root=\$\(cd "\$\{PROJECT_ROOT\}" 2>\/dev\/null && _d=\$\(git rev-parse --git-common-dir 2>\/dev\/null\)/.test(b) + && /PHASE_DIR_FOREIGN="\$\{PROJECT_ROOT\}"; PHASE_DIR=""/.test(b)); + assert.equal(gates.length, 2, `both modes must refuse a phase directory in another repository; found ${gates.length}`); + assert.ok(!/--absolute-git-dir/.test(code.join('\n')), + 'the repository gate must not compare per-worktree git dirs: it refuses a linked worktree'); + const hardened = (bash.match(/! git merge-base --is-ancestor "\$PHASE_START" HEAD 2>\/dev\/null; then PHASE_START=""; fi/g) || []).length; + assert.equal(hardened, 2, 'both modes must blank a PHASE_START outside HEAD\'s own history before the root-commit arm'); + assert.equal((content.match(/PHASE_DIR_FOREIGN/g) || []).length >= 6, true, + 'the refusal must be documented, not only computed'); + }); + + test('D: PHASE_DIR is resolved through find-phase, so it is workstream-correct', () => { + // find-phase resolves through planningDir, which roots an active workstream at + // .planning/workstreams// — a hardcoded .planning/phases/ would read the + // root's same-numbered phase instead. + const uses = (bash.match(/gsd_run query find-phase/g) || []).length; + assert.equal(uses, 2, 'both modes must resolve PHASE_DIR via find-phase (#4465)'); + }); + + test('E: the selection window is bounded above by HEAD', () => { + assert.ok( + /UNDO_RANGE="\$\{PHASE_START\}\^\.\.HEAD"/.test(bash), + 'the selection range must be bounded at HEAD (#4465)', + ); + }); + + test('J: the root-commit branch does not drop PHASE_START itself', () => { + // `${PHASE_START}..HEAD` EXCLUDES PHASE_START. When PHASE_START is the root commit + // there is no parent to exclude, so that spelling silently drops a legitimate first + // phase commit and the undo refuses work it should do. + assert.ok( + !/UNDO_RANGE="\$\{PHASE_START\}\.\.HEAD"/.test(bash), + 'the root-commit branch must not use ${PHASE_START}..HEAD — it drops the root commit (#4465)', + ); + assert.ok( + /UNDO_RANGE="HEAD"/.test(bash), + 'the root-commit branch must select over HEAD so PHASE_START itself stays in range (#4465)', + ); + }); + + test('K: selection pipelines tolerate an empty match', () => { + // grep exits 1 on no match. The removed `| head -50` used to mask that rc, so the + // pipelines must not now abort before the workflow's own Empty check runs. + const selectionBlocks = extractBashBlocks(content).filter( + (b) => /git log --oneline --no-merges "\$\{UNDO_RANGE\}"/.test(b), + ); + assert.equal(selectionBlocks.length, 2, 'expected one bounded selection pipeline per mode'); + for (const block of selectionBlocks) { + assert.ok( + /\|\| true/.test(block), + `every selection pipeline must tolerate grep's no-match exit (#4465). Block:\n${block}`, + ); + } + }); + + test('L: both modes stop rather than silently capping a >50 selection', () => { + const stops = (content.match(/Report truncation, never truncate silently/g) || []).length; + assert.equal(stops, 2, 'both --phase and --plan must document the >50 stop (#4465)'); + // The heading alone is not the rule: pin the refusal each mode instructs the runtime to + // render, so removing the paragraph under an intact heading still fails here. + const refusals = [...content.matchAll(/selects \$\{N\} commits \(>50\)\. Refusing to revert a partial (phase|plan)\./g)] + .map((m) => m[1]); + assert.deepEqual(refusals, ['phase', 'plan'], + 'both modes must carry the >50 refusal message, phase then plan (#4465)'); + // Executable lines only: the fix's own comment explains what `| head -50` used to mask, + // and a comment naming the removed cap is not the cap. + const code = bash + .split('\n') + .filter((l) => !/^\s*#/.test(l)) + .join('\n'); + assert.ok( + !/\| head -50/.test(code), + 'no selection pipeline may silently cap at 50 (#4465)', + ); + }); + + test('F: selection greps run against the bounded range, not the whole repo', () => { + const selectionBlocks = extractBashBlocks(content).filter((b) => /grep -E/.test(b)); + assert.ok(selectionBlocks.length >= 2, 'expected a selection grep for each of --phase and --plan'); + for (const block of selectionBlocks) { + assert.ok( + /\$\{UNDO_RANGE\}/.test(block), + `every commit-selection grep must run over \${UNDO_RANGE} (#4465). Block:\n${block}`, + ); + } + }); + + test('G: the workflow fails closed rather than widening when no anchor resolves', () => { + assert.ok( + /do NOT fall back to an unbounded search/i.test(content), + 'undo.md must state that an unresolved phase does not widen the search (#4465)', + ); + assert.ok( + /An unbounded repository-wide search is never the fallback/i.test(content), + 'undo.md must state the fail-closed rule for an unresolvable anchor (#4465)', + ); + }); + + test('H: dependency_check reads the workstream-resolved planning root', () => { + assert.ok( + /PLANNING_DIR=\$\(gsd_run query planning inspect --pick generated_from\.planning_root/.test(bash), + 'dependency_check must resolve the planning root rather than hardcoding .planning/ (#4465)', + ); + assert.ok( + !/`\.planning\/ROADMAP\.md`/.test(content), + 'dependency_check must not read a hardcoded .planning/ROADMAP.md — wrong file under a workstream (#4465)', + ); + assert.ok( + !/\.planning\/phases\/\$\{/.test(content), + 'dependency_check must not glob a hardcoded .planning/phases/ — wrong tree under a workstream (#4465)', + ); + }); + + test('I: the revert verb is still git revert --no-commit, never git reset --hard', () => { + // Guard the property the original workflow got right, so this fix cannot regress it. + assert.ok(/git revert --no-commit/.test(bash), 'undo.md must still use git revert --no-commit'); + // Scoped to bash blocks: the success-criteria checklist legitimately contains the + // prose "git reset --hard is NEVER used anywhere in this workflow". + assert.ok( + !/git reset --hard/.test(bash), + 'undo.md must never execute git reset --hard', + ); + }); + + test('M: both modes refuse a PHASE_DIR that resolves under milestones/', () => { + // find-phase falls back to archived milestone dirs, and its ambiguity check does + // not span them; anchoring there selects a LATER milestone's same-numbered phase. + // Two guards, one per mode — a single one would leave the other selecting. + // BOTH archive layouts the phase locator enumerates (listArchiveVersionDirs): the flat + // `v-phases/` and the workstream archive `ws--/phases/`. + const guards = extractBashBlocks(content).filter( + (b) => /PHASE_DIR_ARCHIVED=""/.test(b) + && /\*\/milestones\/v\[0-9\]\*-phases\/\*\|milestones\/v\[0-9\]\*-phases\/\*/.test(b) + && /\*\/milestones\/ws-\*\/phases\/\*\|milestones\/ws-\*\/phases\/\*/.test(b), + ); + assert.equal(guards.length, 2, + `undo.md must refuse BOTH archive layouts in BOTH modes; found ${guards.length} guard(s)`); + // The same fence carries the reused-path collision check, and it asks HISTORY, not a + // layout: a path that went empty in HEAD's history and came back anchors on the earlier + // occupant's add, whatever vacated it (review round 4: a layout glob missed the ws-* + // archive and would miss the next layout too). + for (const g of guards) { + const code = g.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'); + assert.ok(/PHASE_DIR_REUSED=""/.test(code) + && /git -C "\$\{PROJECT_ROOT:-\.\}" log -m --no-renames --diff-filter=D --format=%H -- "\$\{PHASE_DIR\}"/.test(code) + && /git -C "\$\{PROJECT_ROOT:-\.\}" ls-tree -d "\$_c" -- "\$\{PHASE_DIR\}"/.test(code), + `each guard fence must refuse a path that history shows was vacated (#4465):\n${code}`); + // No second reader of the archive layout inside the collision check. + const collision = code.slice(code.indexOf('PHASE_DIR_REUSED=""')); + assert.ok(!/milestones/.test(collision), + `the collision check must not re-derive the archive layout from a glob (#4465):\n${collision}`); + } + // The pattern must key on the ARCHIVE LAYOUT. A bare `*/milestones/*` also matches a + // workstream or project legitimately named `milestones` and refuses a LIVE phase. + // Executable lines only: the guard's own comment quotes the rejected pattern to + // explain why it is rejected, and a whole-block match would fire on that. + for (const g of guards) { + const code = g.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'); + assert.ok(!/\*\/milestones\/\*/.test(code), + `the guard must not match the bare token 'milestones' — it refuses live phases:\n${code}`); + } + for (const g of guards) { + assert.ok(/PHASE_DIR=""/.test(g), + `the archived guard must blank PHASE_DIR so the fail-closed rule still holds:\n${g}`); + } + }); + + test('N: the purpose line no longer advertises the removed phase manifest', () => { + // B already reads the WHOLE file, so scope is not why it missed this: it greps the + // hyphenated `phase-manifest` token — the filename — while the purpose line described + // the same dead mechanism in prose, as "the phase manifest". Pinning the block itself + // is spelling-independent, where widening B's pattern to /manifest/i would fire on any + // future sentence that merely mentions one. + // Sliced, not regex-matched: an unbounded `[\s\S]*?` over readFileSync content is a + // catastrophic-backtracking risk and `local/no-unbounded-quantifier` rejects it. + const open = content.indexOf(''); + const close = content.indexOf('', open + 1); + assert.ok(open !== -1 && close !== -1, 'undo.md must carry a block'); + const purpose = content.slice(open, close); + assert.ok(!/manifest/i.test(purpose), + ` must not describe the removed manifest mechanism; got:\n${purpose}`); + }); +}); + +// ─── Behavioral half (review round 1, #4472) ────────────────────────────────── +// +// The block above pins the SHAPE of undo.md's selection fences. This block +// EXECUTES them: the exact ```bash fences the runtime runs are sliced out of +// undo.md by content anchor, glued behind the inputs the workflow would have +// set, and run with `bash -c` inside a real git fixture against the real +// `gsd-tools.cjs` — the same createTempGitProject + fence-execution shape +// new-milestone-clear-phases.test.cjs (#2308) and +// code-review-pipeline-regression.test.cjs (#2352) use. A substring match cannot +// tell a live invocation from a dead one; a run can. +// +// win32: skipped, as the #2352 fence-execution tests are. The fences are POSIX +// bash and the fixture is driven through `bash -c`; Windows shards exercise the +// shape tests above. + +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { createTempGitProject, cleanup, readFileNormalized } = require('./helpers.cjs'); +const { createFixture, seedPhase, seedWorkstream } = require('./fixtures/index.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +const GSD_TOOLS_BIN = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); +const SKIP_WIN32 = process.platform === 'win32' + ? 'POSIX bash fence execution over a git fixture (see #2352 precedent)' + : false; + +/** Fence BODIES (no ``` lines), from a \r\n-normalized read so bash never sees a CR. */ +function fenceBodies(content) { + const lines = content.split('\n'); + const bodies = []; + for (const block of scanFencedBlocks(lines)) { + if (block.closeLineIdx === -1) continue; + if ((block.infoString || '').trim().toLowerCase() !== 'bash') continue; + bodies.push(lines.slice(block.openLineIdx + 1, block.closeLineIdx).join('\n')); + } + return bodies; +} + +/** The one fence whose body satisfies `pred` — located by content, never by position. */ +function fenceWhere(bodies, label, pred) { + const hits = bodies.filter(pred); + assert.equal(hits.length, 1, `expected exactly one ${label} fence in undo.md, found ${hits.length}`); + return hits[0]; +} + +describe('#4465: undo commit selection — executed against a git fixture', { skip: SKIP_WIN32 }, () => { + // allow-test-rule: source-text-is-the-product (see #4465) + const content = readFileNormalized(UNDO_PATH); + const bodies = fenceBodies(content); + + // gather_commits, MODE=phase: resolve → anchor → select + const phaseResolve = fenceWhere(bodies, 'phase resolve', + (b) => b.includes('PHASE_DIR=$(gsd_run query find-phase "${TARGET_PHASE}"')); + const phaseArchivedGuard = fenceWhere(bodies, 'phase archived guard', + (b) => b.includes('PHASE_DIR_ARCHIVED=""') && !b.includes('PLAN_PHASE')); + // Located loosely on purpose: shape test C pins the anchor's spelling, and a locator that + // pinned it too would turn a spelling change into "no such fence" instead of a C failure. + const phaseAnchor = fenceWhere(bodies, 'phase anchor', + (b) => b.includes('PHASE_START=$(') && !b.includes('PLAN_PHASE')); + const phaseSelect = fenceWhere(bodies, 'phase select', + (b) => b.includes('grep -E "\\(0*${TARGET_PHASE}')); + // gather_commits, MODE=plan: resolve+anchor → select + const planAnchor = fenceWhere(bodies, 'plan resolve+anchor', + (b) => b.includes('PLAN_PHASE="${TARGET_PLAN%%-*}"')); + const planSelect = fenceWhere(bodies, 'plan select', + (b) => b.includes('grep -E "\\(${TARGET_PLAN}\\):"')); + // dependency_check: planning-root resolution + const planningRoot = fenceWhere(bodies, 'planning root', + (b) => b.includes('PLANNING_DIR=$(gsd_run query planning inspect')); + + // The composed script replays the fences in order in ONE shell, seeded with + // what the workflow sets (TARGET_*), plus the real gsd_run over the real + // binary. That is deliberately the workflow's own data flow — `PHASE_DIR`, + // `PHASE_START`, `UNDO_RANGE` are carried from fence to fence by the runtime + // that executes undo.md — and it is what these tests prove: the selection + // logic, not the runtime's variable transport between blocks. + const GSD_RUN = 'gsd_run() { node "$GSD_TOOLS_BIN" "$@"; }'; + + // `runIn` is the directory the shell starts in, when it is not the fixture root: the user + // can invoke /gsd:undo from anywhere inside the project. + function runFences(cwd, seed, fences, tail = '', extraEnv = {}, runIn = cwd) { + const script = [seed, GSD_RUN, ...fences, tail].join('\n'); + const env = { ...process.env, GSD_TOOLS_BIN, HOME: cwd }; + // A developer's active workstream must not leak into the fixture; a test that wants + // one names it through `extraEnv`, applied after the scrub. + delete env.GSD_WORKSTREAM; + delete env.GSD_PROJECT; + Object.assign(env, extraEnv); + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: runIn, env, timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + /** `git log --oneline` output → the subject lines, in order. */ + function subjects(oneline) { + return oneline.split('\n').filter(Boolean).map((l) => l.replace(/^[0-9a-f]+ /, '')); + } + + function commitFile(cwd, rel, body, message) { + fs.mkdirSync(path.dirname(path.join(cwd, rel)), { recursive: true }); + fs.writeFileSync(path.join(cwd, rel), body); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', message], { cwd }); + } + + // The reported repro (#4465): milestone 1 ships phase 03 and is archived; + // milestone 2 reuses the number. A dead branch carries a matching scope too. + function multiMilestoneFixture() { + const cwd = createTempGitProject('gsd-4465-mm-'); + const mainBranch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd }).trim(); + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# auth\n' }); + commitFile(cwd, 'src/auth.js', 'auth\n', 'feat(03-01): implement auth endpoint'); + commitFile(cwd, 'src/ratelimit.js', 'rl\n', 'feat(03-02): add rate limiter'); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'v1.0-phases'), { recursive: true }); + gitOrThrow(['mv', '.planning/phases/03-auth', '.planning/milestones/v1.0-phases/03-auth'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: archive v1.0 milestone files'], { cwd }); + gitOrThrow(['checkout', '-q', '-b', 'abandoned/x'], { cwd }); + commitFile(cwd, 'src/experiment.js', 'x\n', 'feat(03-02): abandoned experiment'); + gitOrThrow(['checkout', '-q', mainBranch], { cwd }); + seedPhase(cwd, '03-beta', { '03-01-PLAN.md': '# beta\n' }); + commitFile(cwd, 'src/beta.js', 'beta\n', 'feat(03-01): add beta feature flag'); + return cwd; + } + + test('fixture reproduces #4465: the retired --all grep selected the archived milestone and a dead branch', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + // Negative control for the fixture itself: the pre-fix selection line, verbatim + // from undo.md@b5b9814f0, over this fixture. If it did NOT over-select here, the + // passing tests below would be vacuous. + const old = runFences(cwd, 'TARGET_PHASE=03', [], + 'git log --oneline --no-merges --all | grep -E "\\(0*${TARGET_PHASE}(-[0-9]+)?\\):" | head -50'); + assert.deepEqual(subjects(old).sort(), [ + 'feat(03-01): add beta feature flag', + 'feat(03-01): implement auth endpoint', + 'feat(03-02): abandoned experiment', + 'feat(03-02): add rate limiter', + ]); + }); + + test('--phase selects only the current milestone\'s HEAD-reachable commits', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect]); + assert.deepEqual(subjects(out), ['feat(03-01): add beta feature flag']); + }); + + test('--plan selects only the current milestone\'s instance of a reused plan id', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PLAN=03-01', [planAnchor, planSelect]); + assert.deepEqual(subjects(out), ['feat(03-01): add beta feature flag']); + }); + + // Round 2: find-phase's ambiguity check is scoped to ONE searchDir (the + // `matches.length > 1` test sits inside cmdFindPhase's per-directory loop), and the + // live `phases/` dir is searched first. So a phase number that is NOT live resolves + // silently to the OLDEST archived milestone carrying one. Two archived milestones is + // the reported scenario; the current milestone has not reached phase 03 yet. + // + // THE RESOLVER CONTRACT, stated precisely because there are TWO readers of the archive + // tree and they disagree (review round 4). `find-phase` routes to cmdFindPhase + // (src/phase.cts), which builds its OWN search list: the live `phases/` dir, then + // `milestones/` entries matching /^v\d+.*-phases$/ — the flat layout ONLY. The phase + // locator (listArchiveVersionDirs, src/phase-locator.cts) additionally enumerates the + // workstream archive `milestones/ws--/phases/`, which `workstream complete` + // writes; find-phase never searches it. The tests below pin what find-phase actually + // returns for BOTH layouts, so a change to either reader shows up here. + function twoArchivedMilestonesFixture() { + const cwd = createTempGitProject('gsd-4465-arch-'); + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# auth\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): v1.0 phase plan'], { cwd }); + commitFile(cwd, 'src/auth.js', 'auth\n', 'feat(03-01): implement auth endpoint'); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'v1.0-phases'), { recursive: true }); + gitOrThrow(['mv', '.planning/phases/03-auth', '.planning/milestones/v1.0-phases/03-auth'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: archive v1.0 milestone files'], { cwd }); + seedPhase(cwd, '03-search', { '03-01-PLAN.md': '# search\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): v2.0 phase plan'], { cwd }); + commitFile(cwd, 'src/search.js', 's\n', 'feat(03-01): add search index'); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'v2.0-phases'), { recursive: true }); + gitOrThrow(['mv', '.planning/phases/03-search', '.planning/milestones/v2.0-phases/03-search'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: archive v2.0 milestone files'], { cwd }); + // v3.0 in progress; phase 03 does not exist live, which is what sends find-phase + // into the archives at all. + seedPhase(cwd, '01-setup', { '01-01-PLAN.md': '# setup\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(01-01): v3.0 phase plan'], { cwd }); + return cwd; + } + + test('negative control: WITHOUT the archived guard, an archived resolution selects the WRONG milestone', (t) => { + const cwd = twoArchivedMilestonesFixture(); + t.after(() => cleanup(cwd)); + // Anchor + select with the guard fence omitted — i.e. this PR's round-1 state. + // find-phase returns v1.0's dir, PHASE_START is the v1.0 ARCHIVAL commit, and the + // window then runs forward into v2.0 and matches its same-numbered phase, while + // v1.0's own work commit sits before the window. Both halves are asserted, because + // "reverts too little" and "reverts someone else's milestone" are different bugs + // and only the second is destructive. + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect], + 'echo "PHASE_DIR=${PHASE_DIR}"'); + assert.ok(out.includes('PHASE_DIR=.planning/milestones/v1.0-phases/03-auth'), + `expected the OLDEST archived dir to win; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/PHASE_DIR=.*\n?/, '')), + ['feat(03-01): add search index', 'docs(03-01): v2.0 phase plan'], + 'the unguarded window must select v2.0\'s phase 03 and none of v1.0\'s'); + }); + + test('--phase REFUSES an archived resolution: no anchor, no range, nothing selected', (t) => { + const cwd = twoArchivedMilestonesFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "ARCHIVED=[%s]\\nPHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR_ARCHIVED" "$PHASE_DIR" "$UNDO_RANGE"'); + assert.ok(out.includes('ARCHIVED=[.planning/milestones/v1.0-phases/03-auth]'), + `the refusal must name the archived directory it declined; got:\n${out}`); + assert.ok(out.includes('PHASE_DIR=[]'), `PHASE_DIR must be blanked; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(ARCHIVED|PHASE_DIR|UNDO_RANGE)=.*\n?/g, '')), [], + 'nothing may be selected once the resolution is refused'); + }); + + test('--plan REFUSES an archived resolution too', (t) => { + const cwd = twoArchivedMilestonesFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PLAN=03-01', [planAnchor, planSelect], + 'printf "ARCHIVED=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR_ARCHIVED" "$UNDO_RANGE"'); + assert.ok(out.includes('ARCHIVED=[.planning/milestones/v1.0-phases/03-auth]'), + `--plan must refuse the same resolution; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(ARCHIVED|UNDO_RANGE)=.*\n?/g, '')), [], + 'nothing may be selected once the resolution is refused'); + }); + + test('the archived guard is inert on a LIVE resolution — it refuses archives, not phases', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'echo "ARCHIVED=[${PHASE_DIR_ARCHIVED}]"'); + assert.ok(out.includes('ARCHIVED=[]'), `a live phase dir must not trip the guard; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/ARCHIVED=.*\n?/, '')), ['feat(03-01): add beta feature flag']); + }); + + // Round 2, self-found: a LIVE path can still be a previous occupant's. A later milestone + // that re-creates the same literal directory (same number AND same slug) anchors on the + // older milestone's add commit. The archive guard cannot see it -- find-phase returns the + // live directory -- so the collision check asks history whether this exact path was ever + // vacated (round 4: it used to look for an archived twin by layout, and missed ws-*). + function reusedSlugFixture() { + const cwd = createTempGitProject('gsd-4465-slug-'); + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# v1\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): v1 plan'], { cwd }); + commitFile(cwd, 'src/a.js', 'a\n', 'feat(03-01): v1 auth work'); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'v1.0-phases'), { recursive: true }); + gitOrThrow(['mv', '.planning/phases/03-auth', '.planning/milestones/v1.0-phases/03-auth'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: archive v1.0'], { cwd }); + // v2.0 re-creates the SAME literal path. + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# v2\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): v2 plan'], { cwd }); + commitFile(cwd, 'src/b.js', 'b\n', 'feat(03-01): v2 auth work'); + return cwd; + } + + test('negative control: WITHOUT the collision guard, a reused slug selects the EARLIER milestone too', (t) => { + const cwd = reusedSlugFixture(); + t.after(() => cleanup(cwd)); + // Resolve + anchor + select with the guard fence omitted. find-phase returns the LIVE + // path, so the archive guard is inert here by construction -- this is the case it misses. + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect], + 'echo "PHASE_DIR=${PHASE_DIR}"'); + assert.ok(out.includes('PHASE_DIR=.planning/phases/03-auth'), + `the LIVE path must win -- this is why the archive guard cannot reach it; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/PHASE_DIR=.*\n?/, '')), [ + 'feat(03-01): v2 auth work', + 'docs(03-01): v2 plan', + 'feat(03-01): v1 auth work', + 'docs(03-01): v1 plan', + ], 'the unguarded window must reach back into v1.0'); + }); + + test('--phase REFUSES a reused directory name: no anchor, no range, nothing selected', (t) => { + const cwd = reusedSlugFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "REUSED=[%s]\\nPHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$PHASE_DIR" "$UNDO_RANGE"'); + assert.ok(out.includes('REUSED=[chore: archive v1.0]'), + `the refusal must name the commit that vacated the path; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(REUSED|PHASE_DIR|UNDO_RANGE)=.*\n?/g, '')), [], + 'nothing may be selected once the reused path is refused'); + }); + + test('--plan REFUSES a reused directory name too', (t) => { + const cwd = reusedSlugFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PLAN=03-01', [planAnchor, planSelect], + 'printf "REUSED=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$UNDO_RANGE"'); + assert.ok(out.includes('REUSED=[chore: archive v1.0]'), + `--plan must refuse the same collision; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + }); + + test('the collision guard is inert when no archived twin exists', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + // 03-beta is live and 03-auth is archived -- different slugs, so no collision. + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'echo "REUSED=[${PHASE_DIR_REUSED}]"'); + assert.ok(out.includes('REUSED=[]'), `a distinct slug must not collide; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/REUSED=.*\n?/, '')), ['feat(03-01): add beta feature flag']); + }); + + test('false-positive control: a same-named entry under milestones/ does not refuse a never-vacated path', (t) => { + // A CONTROL, not a proof the collision check works: with the check deleted this still + // passes. What it catches is the opposite failure -- a check that refuses on a NAME it + // finds under milestones/ rather than on this path's history, which is the layout-keyed + // shape round 4 replaced. Round 2 pinned it for a stray FILE against the old glob. + const cwd = createTempGitProject('gsd-4465-file-twin-'); + t.after(() => cleanup(cwd)); + seedPhase(cwd, '06-live', { '06-01-PLAN.md': '# live\n' }); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'v1.0-phases'), { recursive: true }); + // A FILE where an archived phase directory would sit. + fs.writeFileSync(path.join(cwd, '.planning', 'milestones', 'v1.0-phases', '06-live'), 'not a dir\n'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(06-01): live plan'], { cwd }); + commitFile(cwd, 'src/f.js', 'f\n', 'feat(06-01): live work'); + const out = runFences(cwd, 'TARGET_PHASE=06', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'echo "REUSED=[${PHASE_DIR_REUSED}]"'); + assert.ok(out.includes('REUSED=[]'), `a regular file is not an archived phase dir; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/REUSED=.*\n?/, '')), [ + 'feat(06-01): live work', + 'docs(06-01): live plan', + ], 'the undo must still work'); + }); + + test('false-positive control: a same-named phase dir under a malformed milestone dir does not refuse', (t) => { + // A CONTROL, like the one above: it passes with the collision check deleted, and fails only + // if the check starts refusing on a directory name. `vnondigit-phases` is not a milestone + // either reader of the archive tree admits, and this live path was never vacated. + const cwd = createTempGitProject('gsd-4465-malformed-'); + t.after(() => cleanup(cwd)); + seedPhase(cwd, '05-live', { '05-01-PLAN.md': '# live\n' }); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones', 'vnondigit-phases', '05-live'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'milestones', 'vnondigit-phases', '05-live', 'x.md'), 'x\n'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(05-01): live plan'], { cwd }); + commitFile(cwd, 'src/m.js', 'm\n', 'feat(05-01): live work'); + const out = runFences(cwd, 'TARGET_PHASE=05', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'echo "REUSED=[${PHASE_DIR_REUSED}]"'); + assert.ok(out.includes('REUSED=[]'), + `a live path that was never vacated must not refuse; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/REUSED=.*\n?/, '')), [ + 'feat(05-01): live work', + 'docs(05-01): live plan', + ]); + }); + + test('the reused-path refusal can still name the live path it declined', (t) => { + // The fence blanks PHASE_DIR, so the message must read the preserved copy — otherwise it + // renders "resolves to , but ...". Round-2 audit finding. + const cwd = reusedSlugFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard], + 'printf "LIVE=[%s]\\nPHASE_DIR=[%s]\\n" "$PHASE_DIR_LIVE" "$PHASE_DIR"'); + assert.ok(out.includes('LIVE=[.planning/phases/03-auth]'), + `the declined live path must survive for the message; got:\n${out}`); + assert.ok(out.includes('PHASE_DIR=[]'), `PHASE_DIR must still be blanked; got:\n${out}`); + }); + + test('the guard keys on the archive LAYOUT: a workstream named "milestones" is still live', (t) => { + // The conventional live path above cannot catch this. `milestones` is a legal + // workstream (and project) name, so a bare `*/milestones/*` pattern classifies + // .planning/workstreams/milestones/phases/NN-x as archived and refuses a phase that + // is live and revertible — a fail-closed bug, but a bug. Only the archive LAYOUTS + // (`v-phases/` and `ws--/phases/`) separate the two. + const cwd = createTempGitProject('gsd-4465-wsname-'); + t.after(() => cleanup(cwd)); + const dir = path.join(cwd, '.planning', 'workstreams', 'milestones', 'phases', '03-live'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, '03-01-PLAN.md'), '# live\n'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): workstream phase plan'], { cwd }); + commitFile(cwd, 'src/live.js', 'l\n', 'feat(03-01): workstream work'); + // Activate by env, the same route find-phase honours for an active workstream. + const script = [ + 'TARGET_PHASE=03', GSD_RUN, phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect, + 'echo "ARCHIVED=[${PHASE_DIR_ARCHIVED}]"', + ].join('\n'); + const env = { ...process.env, GSD_TOOLS_BIN, HOME: cwd, GSD_WORKSTREAM: 'milestones' }; + delete env.GSD_PROJECT; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd, env, timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, 'bash '); + assert.ok(r.stdout.includes('ARCHIVED=[]'), + `a workstream NAMED "milestones" is a live scope, not an archive; got:\n${r.stdout}`); + assert.deepEqual(subjects(r.stdout.replace(/ARCHIVED=.*\n?/, '')), [ + 'feat(03-01): workstream work', + 'docs(03-01): workstream phase plan', + ]); + }); + + // Round 4: the WORKSTREAM archive layout. `workstream complete` moves + // `.planning/workstreams//` whole into `.planning/milestones/ws--/`, so an + // archived phase's former path is `.planning/workstreams//phases/`. Re-creating + // the workstream under the same name with the same phase slug re-creates that exact path. + function workstreamArchiveFixture({ recreate }) { + const cwd = createTempGitProject('gsd-4465-ws-'); + commitFile(cwd, '.planning/workstreams/feat/phases/03-auth/03-01-PLAN.md', '# gen 1\n', + 'docs(03-01): feat generation-1 plan'); + commitFile(cwd, 'src/gen1.js', 'g1\n', 'feat(03-01): feat generation-1 work'); + fs.mkdirSync(path.join(cwd, '.planning', 'milestones'), { recursive: true }); + gitOrThrow(['mv', '.planning/workstreams/feat', '.planning/milestones/ws-feat-2026-09-01'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: complete workstream feat'], { cwd }); + if (recreate) { + commitFile(cwd, '.planning/workstreams/feat/phases/03-auth/03-01-PLAN.md', '# gen 2\n', + 'docs(03-01): feat generation-2 plan'); + commitFile(cwd, 'src/gen2.js', 'g2\n', 'feat(03-01): feat generation-2 work'); + } else { + // A later phase 03 commit in the current scope with no live phase dir behind it. + commitFile(cwd, 'src/later.js', 'l\n', 'feat(03-01): later milestone work'); + } + return cwd; + } + const WS_FEAT = { GSD_WORKSTREAM: 'feat' }; + + test('negative control: WITHOUT the collision guard, a re-created workstream selects its archived generation too', (t) => { + const cwd = workstreamArchiveFixture({ recreate: true }); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect], + 'echo "PHASE_DIR=${PHASE_DIR}"', WS_FEAT); + assert.ok(out.includes('PHASE_DIR=.planning/workstreams/feat/phases/03-auth'), + `find-phase must return the LIVE re-created path; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/PHASE_DIR=.*\n?/, '')), [ + 'feat(03-01): feat generation-2 work', + 'docs(03-01): feat generation-2 plan', + 'feat(03-01): feat generation-1 work', + 'docs(03-01): feat generation-1 plan', + ], 'the unguarded window must reach back into the archived workstream generation'); + }); + + test('--phase REFUSES a path re-created after a workstream archive (the ws-* layout)', (t) => { + const cwd = workstreamArchiveFixture({ recreate: true }); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "REUSED=[%s]\\nLIVE=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$PHASE_DIR_LIVE" "$UNDO_RANGE"', + WS_FEAT); + assert.ok(out.includes('REUSED=[chore: complete workstream feat]'), + `the refusal must name the workstream archive that vacated the path; got:\n${out}`); + assert.ok(out.includes('LIVE=[.planning/workstreams/feat/phases/03-auth]'), `got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(REUSED|LIVE|UNDO_RANGE)=.*\n?/g, '')), [], + 'nothing may be selected: selection must not cross into the archived generation'); + }); + + test('--plan REFUSES a path re-created after a workstream archive too', (t) => { + const cwd = workstreamArchiveFixture({ recreate: true }); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PLAN=03-01', [planAnchor, planSelect], + 'printf "REUSED=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$UNDO_RANGE"', + WS_FEAT); + assert.ok(out.includes('REUSED=[chore: complete workstream feat]'), `got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `UNDO_RANGE must stay empty; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(REUSED|UNDO_RANGE)=.*\n?/g, '')), []); + }); + + test('find-phase does not search the ws-* archive: a phase only there resolves to nothing, and nothing is selected', (t) => { + // The actual resolver contract, pinned: cmdFindPhase admits only /^v\d+.*-phases$/ under + // milestones/. So the ws-* archive never reaches the archived refusal through find-phase; + // the not-found rule fails closed instead. If find-phase is ever taught the ws-* layout this + // test goes red, and the stubbed-resolver test below is what keeps the refusal honest then. + const cwd = workstreamArchiveFixture({ recreate: false }); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "PHASE_DIR=[%s]\\nARCHIVED=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR" "$PHASE_DIR_ARCHIVED" "$UNDO_RANGE"'); + assert.ok(out.includes('PHASE_DIR=[]'), `find-phase must not resolve into ws-*; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `no anchor, no range; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(PHASE_DIR|ARCHIVED|UNDO_RANGE)=.*\n?/g, '')), [], + 'the later milestone\'s same-numbered commit must not be selected'); + }); + + test('the archived refusal covers the ws-* layout if a resolver ever returns it (both modes)', (t) => { + // Stubbed resolver: stands in for a find-phase that has been taught the locator's second + // layout. Without the ws-* arm, PHASE_START would be the archival commit and the window + // would run forward from it -- the exact contamination the refusal exists for. + const cwd = workstreamArchiveFixture({ recreate: false }); + t.after(() => cleanup(cwd)); + const stub = 'gsd_run() { echo ".planning/milestones/ws-feat-2026-09-01/phases/03-auth"; }'; + const report = 'printf "ARCHIVED=[%s]\\nPHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR_ARCHIVED" "$PHASE_DIR" "$UNDO_RANGE"'; + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [stub, phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [stub, planAnchor, planSelect]], + ]) { + const out = runFences(cwd, seed, fences, report); + assert.ok(out.includes('ARCHIVED=[.planning/milestones/ws-feat-2026-09-01/phases/03-auth]'), + `${seed}: a ws-* archive path must be refused; got:\n${out}`); + assert.ok(out.includes('PHASE_DIR=[]') && out.includes('UNDO_RANGE=[]'), `${seed}: got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(ARCHIVED|PHASE_DIR|UNDO_RANGE)=.*\n?/g, '')), [], + `${seed}: nothing may be selected once the resolution is refused`); + } + }); + + test('self-found: a phase REMOVED and re-added under the same slug is refused too', (t) => { + // No archive anywhere -- the path was simply deleted and re-created. A layout glob has no + // entry to find; the history check sees the vacancy. Same anchor defect, same refusal. + const cwd = createTempGitProject('gsd-4465-readd-'); + t.after(() => cleanup(cwd)); + commitFile(cwd, '.planning/phases/03-auth/03-01-PLAN.md', '# first\n', 'docs(03-01): first plan'); + commitFile(cwd, 'src/first.js', 'f\n', 'feat(03-01): first attempt'); + gitOrThrow(['rm', '-rq', '.planning/phases/03-auth'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'chore: remove phase 03'], { cwd }); + commitFile(cwd, '.planning/phases/03-auth/03-01-PLAN.md', '# second\n', 'docs(03-01): second plan'); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "REUSED=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$UNDO_RANGE"'); + assert.ok(out.includes('REUSED=[chore: remove phase 03]'), `got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `got:\n${out}`); + }); + + function droppedPlanFixture() { + const cwd = createTempGitProject('gsd-4465-dropplan-'); + commitFile(cwd, '.planning/phases/03-auth/03-01-PLAN.md', '# one\n', 'docs(03-01): plan one'); + commitFile(cwd, '.planning/phases/03-auth/03-02-PLAN.md', '# two\n', 'docs(03-02): plan two'); + gitOrThrow(['rm', '-q', '.planning/phases/03-auth/03-02-PLAN.md'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-02): drop plan two'], { cwd }); + commitFile(cwd, 'src/auth.js', 'a\n', 'feat(03-01): auth work'); + return cwd; + } + const DROPPED_PLAN_PHASE = [ + 'feat(03-01): auth work', + 'docs(03-02): drop plan two', + 'docs(03-02): plan two', + 'docs(03-01): plan one', + ]; + + test('the collision check is not tripped by an ordinary deleted file inside a live phase', (t) => { + // The vacancy test is on the DIRECTORY (`ls-tree -d` at the deleting commit), so dropping + // one plan file from a phase that still exists is not a previous occupant. + const cwd = droppedPlanFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'echo "REUSED=[${PHASE_DIR_REUSED}]"'); + assert.ok(out.includes('REUSED=[]'), `a dropped plan file is not a vacated phase; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/REUSED=.*\n?/, '')), DROPPED_PLAN_PHASE, + 'the whole phase must still be selectable'); + }); + + // Review round 5, self-found: the user can run /gsd:undo from anywhere inside the project. + // find-phase answers relative to the PROJECT ROOT; a pathspec is read relative to git's cwd. + // Every test above runs from the fixture root, where the two coincide and the mismatch is + // invisible. These run the same fences from `sub/dir`. + function fromSubdir(cwd) { + const runIn = path.join(cwd, 'sub', 'dir'); + fs.mkdirSync(runIn, { recursive: true }); + return (seed, fences, tail = '', extraEnv = {}) => + runFences(cwd, seed, fences, tail, extraEnv, runIn); + } + + test('from a subdirectory: --phase and --plan select exactly what they select at the root', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const run = fromSubdir(cwd); + const report = 'printf "PHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR" "$UNDO_RANGE"'; + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [planAnchor, planSelect]], + ]) { + const out = run(seed, fences, report); + // The resolver half: find-phase did find the project from here, so a failure below is + // the git half's, not a lookup that never happened. + assert.ok(out.includes('PHASE_DIR=[.planning/phases/03-beta]'), `${seed}: got:\n${out}`); + assert.ok(!out.includes('UNDO_RANGE=[]'), `${seed}: the anchor must resolve from a subdirectory; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(PHASE_DIR|UNDO_RANGE)=.*\n?/g, '')), + ['feat(03-01): add beta feature flag'], `${seed}: from sub/dir`); + } + }); + + test('from a subdirectory: a re-created path is still refused (both layouts, both modes)', (t) => { + const report = 'printf "REUSED=[%s]\\nUNDO_RANGE=[%s]\\n" "$(git log -1 --format=%s "$PHASE_DIR_REUSED" 2>/dev/null)" "$UNDO_RANGE"'; + for (const [label, build, vacatedBy, env] of [ + ['flat archive', reusedSlugFixture, 'chore: archive v1.0', {}], + ['ws-* archive', () => workstreamArchiveFixture({ recreate: true }), 'chore: complete workstream feat', WS_FEAT], + ]) { + const cwd = build(); + t.after(() => cleanup(cwd)); + const run = fromSubdir(cwd); + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [planAnchor, planSelect]], + ]) { + const out = run(seed, fences, report, env); + assert.ok(out.includes(`REUSED=[${vacatedBy}]`), + `${label}, ${seed}: the collision check must see the vacancy from sub/dir; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `${label}, ${seed}: got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(REUSED|UNDO_RANGE)=.*\n?/g, '')), [], `${label}, ${seed}`); + } + } + }); + + test('from a subdirectory: a dropped plan file still does not refuse (both modes)', (t) => { + // The other direction. An `ls-tree` that misreads the path lists nothing, and "lists nothing" + // is exactly what the check reads as "the directory went away": a mis-rooted ls-tree does not + // miss the collision, it refuses every phase that ever lost a file. + const cwd = droppedPlanFixture(); + t.after(() => cleanup(cwd)); + const run = fromSubdir(cwd); + for (const [seed, fences, expected] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], DROPPED_PLAN_PHASE], + ['TARGET_PLAN=03-02', [planAnchor, planSelect], ['docs(03-02): drop plan two', 'docs(03-02): plan two']], + ]) { + const out = run(seed, fences, 'echo "REUSED=[${PHASE_DIR_REUSED}]"'); + assert.ok(out.includes('REUSED=[]'), `${seed}: a dropped plan file is not a vacated phase; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/REUSED=.*\n?/, '')), expected, `${seed}: from sub/dir`); + } + }); + + // Review round 5, second pass: a `sub_repos` project keeps .planning/ in a PARENT repository and + // the code in child repositories. From a child, findProjectRoot returns the parent, so the project + // root and the repository the commits live in are different repositories. + function subReposFixture(t) { + const parent = createFixture({ prefix: 'gsd-4465-subrepos-', planning: false, git: true, projectDoc: false }); + t.after(() => cleanup(parent)); + fs.writeFileSync(path.join(parent, '.gitignore'), 'child/\n'); + fs.mkdirSync(path.join(parent, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(parent, '.planning', 'config.json'), '{"sub_repos":["child"]}\n'); + commitFile(parent, '.planning/phases/03-auth/03-01-PLAN.md', '# parent plan\n', 'docs(03-01): parent phase plan'); + const child = path.join(parent, 'child'); + fs.mkdirSync(child); + const g = (args) => gitOrThrow(args, { cwd: child }); + g(['init', '-q']); g(['config', 'user.email', 'test@test.com']); g(['config', 'user.name', 'Test']); + g(['config', 'commit.gpgsign', 'false']); + commitFile(child, 'a.js', 'a\n', 'feat(03-01): old child work'); + commitFile(child, 'b.js', 'b\n', 'feat(03-01): current child work'); + // HOME off the parent, so the sub_repos branch of findProjectRoot is the one that resolves. + const home = createFixture({ prefix: 'gsd-4465-home-', planning: false, git: false }); + t.after(() => cleanup(home)); + return { parent, child, env: { HOME: home } }; + } + + test('negative control: the pre-hardening root-commit arm widened a foreign anchor to all of HEAD', (t) => { + // The anchor's root-commit branch as it stood before this round's hardening, verbatim, fed the + // PARENT repository's anchor from inside the child. If it did not widen here, the refusal tests + // below would be vacuous. + const { parent, child, env } = subReposFixture(t); + const foreign = gitOrThrow(['rev-parse', 'HEAD'], { cwd: parent }).trim(); + const out = runFences(parent, `PHASE_START=${foreign}`, [ + 'UNDO_RANGE=""; if [ -n "$PHASE_START" ]; then if git rev-parse "${PHASE_START}^" >/dev/null 2>&1; then UNDO_RANGE="${PHASE_START}^..HEAD"; else UNDO_RANGE="HEAD"; fi; fi', + 'echo "UNDO_RANGE=${UNDO_RANGE}"', + 'git log --oneline --no-merges "${UNDO_RANGE}" | grep -E "\\(0*03(-[0-9]+)?\\):" || true', + ], '', env, child); + assert.ok(out.includes('UNDO_RANGE=HEAD'), `got:\n${out}`); + assert.deepEqual(subjects(out.replace(/UNDO_RANGE=.*\n?/, '')), + ['feat(03-01): current child work', 'feat(03-01): old child work']); + }); + + test('sub_repos: a phase planned in the parent repository is REFUSED from a child (both modes)', (t) => { + const { parent, child, env } = subReposFixture(t); + const report = 'printf "FOREIGN=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR_FOREIGN" "$UNDO_RANGE"'; + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [planAnchor, planSelect]], + ]) { + const out = runFences(parent, seed, fences, report, env, child); + const foreign = /FOREIGN=\[(.*)\]/.exec(out); + assert.ok(foreign && foreign[1] !== '' && fs.realpathSync(foreign[1]) === fs.realpathSync(parent), + `${seed}: must refuse, naming the parent repository; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `${seed}: got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(FOREIGN|UNDO_RANGE)=.*\n?/g, '')), [], `${seed}: nothing may be selected`); + } + }); + + test('defense in depth: without the repository refusal, a foreign anchor still resolves no range', (t) => { + // The repository refusal lives in the guard fence; drop it and the anchor fence's own check -- + // PHASE_START must be a commit THIS repository holds -- still keeps the window from widening. + const { parent, child, env } = subReposFixture(t); + const out = runFences(parent, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect], + 'printf "PHASE_START=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_START" "$UNDO_RANGE"', env, child); + assert.ok(out.includes('PHASE_START=[]') && out.includes('UNDO_RANGE=[]'), `got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(PHASE_START|UNDO_RANGE)=.*\n?/g, '')), []); + }); + + // Review round 5, third pass: a LINKED worktree. gsd-tools maps a linked worktree with no + // .planning/ of its own to the MAIN worktree (resolveMainWorktreeCwd), so PROJECT_ROOT is the main + // checkout while the shell sits in the linked one: two worktrees, one repository. The anchor then + // comes from the main branch's history, which the linked HEAD may or may not contain. + function linkedWorktreeFixture(t, { branchAfterPhase }) { + const main = createTempGitProject('gsd-4465-wt-main-'); + t.after(() => cleanup(main)); + const linked = createFixture({ prefix: 'gsd-4465-wt-linked-', planning: false, git: false }); + t.after(() => cleanup(linked)); + const phase = () => { + seedPhase(main, '03-auth', { '03-01-PLAN.md': '# plan\n' }); + gitOrThrow(['add', '-A'], { cwd: main }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): phase plan'], { cwd: main }); + commitFile(main, 'src/a.js', 'a\n', 'feat(03-01): main work'); + }; + if (branchAfterPhase) phase(); + gitOrThrow(['worktree', 'add', '-q', '-b', 'feature', linked], { cwd: main }); + if (!branchAfterPhase) phase(); + // No .planning/ in the linked checkout: that is what sends gsd-tools to the main worktree. + gitOrThrow(['rm', '-rq', '.planning'], { cwd: linked }); + gitOrThrow(['commit', '-q', '-m', 'chore: executor worktree without planning'], { cwd: linked }); + commitFile(linked, 'src/b.js', 'b\n', 'feat(03-01): linked work'); + return { main, linked }; + } + const LINKED_REPORT = 'printf "ROOT=[%s]\\nFOREIGN=[%s]\\nUNDO_RANGE=[%s]\\n" "$PROJECT_ROOT" "$PHASE_DIR_FOREIGN" "$UNDO_RANGE"'; + const assertMappedToMain = (out, main, seed) => { + // The mapping is what makes this case: were PROJECT_ROOT the linked checkout, nothing would + // compare two worktrees and the test would pass vacuously. + const root = /ROOT=\[(.*)\]/.exec(out); + assert.ok(root && root[1] !== '' && fs.realpathSync(root[1]) === fs.realpathSync(main), + `${seed}: planning must resolve to the MAIN worktree from the linked one; got:\n${out}`); + }; + + test('linked worktree: the main worktree\'s planning is the same repository, and is not refused (both modes)', (t) => { + const { main, linked } = linkedWorktreeFixture(t, { branchAfterPhase: true }); + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [planAnchor, planSelect]], + ]) { + const out = runFences(main, seed, fences, LINKED_REPORT, {}, linked); + assertMappedToMain(out, main, seed); + assert.ok(out.includes('FOREIGN=[]'), `${seed}: a linked worktree is the same repository; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(ROOT|FOREIGN|UNDO_RANGE)=.*\n?/g, '')), + ['feat(03-01): linked work', 'feat(03-01): main work', 'docs(03-01): phase plan'], `${seed}`); + } + }); + + test('linked worktree branched BEFORE the phase: an anchor outside HEAD\'s history resolves no range (both modes)', (t) => { + // Same repository, so the gate passes -- but the phase started on the main branch after this + // branch left it, so the anchor is not in the linked HEAD's history and bounds nothing on it. + // Without the ancestry check the window would be every linked commit since the fork. + const { main, linked } = linkedWorktreeFixture(t, { branchAfterPhase: false }); + for (const [seed, fences] of [ + ['TARGET_PHASE=03', [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect]], + ['TARGET_PLAN=03-01', [planAnchor, planSelect]], + ]) { + const out = runFences(main, seed, fences, LINKED_REPORT, {}, linked); + assertMappedToMain(out, main, seed); + assert.ok(out.includes('FOREIGN=[]') && out.includes('UNDO_RANGE=[]'), `${seed}: got:\n${out}`); + assert.deepEqual(subjects(out.replace(/(ROOT|FOREIGN|UNDO_RANGE)=.*\n?/g, '')), [], `${seed}: nothing may be selected`); + } + }); + + test('control: the root is the PROJECT root, not the repository top level', (t) => { + // A project need not sit at the top of its repository. find-phase answers relative to the + // directory holding .planning/, so `git rev-parse --show-toplevel` would be the wrong -C + // here; generated_from.cwd is the directory find-phase itself resolved against. Run from the + // project root, so this pins the choice of root, not the subdirectory case above. + const repo = createFixture({ prefix: 'gsd-4465-nested-', planning: false, git: true, projectDoc: false }); + t.after(() => cleanup(repo)); + const project = path.join(repo, 'app'); + commitFile(repo, 'app/.planning/phases/03-auth/03-01-PLAN.md', '# nested\n', 'docs(03-01): nested plan'); + commitFile(repo, 'app/src/a.js', 'a\n', 'feat(03-01): nested work'); + const out = runFences(repo, 'TARGET_PHASE=03', + [phaseResolve, phaseArchivedGuard, phaseAnchor, phaseSelect], + 'printf "PROJECT_ROOT=[%s]\\n" "$PROJECT_ROOT"', {}, project); + assert.ok(/PROJECT_ROOT=\[.*[\\/]app\]/.test(out), `expected the nested project's root; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/PROJECT_ROOT=.*\n?/, '')), [ + 'feat(03-01): nested work', + 'docs(03-01): nested plan', + ]); + }); + + test('single-milestone selection is unchanged: every phase commit, none from a later phase', (t) => { + const cwd = createTempGitProject('gsd-4465-single-'); + t.after(() => cleanup(cwd)); + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# auth\n' }); + commitFile(cwd, 'src/a.js', 'a\n', 'feat(03-01): implement auth endpoint'); + commitFile(cwd, 'src/b.js', 'b\n', 'feat(03-02): add rate limiter'); + commitFile(cwd, 'src/c.js', 'c\n', 'fix(03-02): correct limiter window'); + commitFile(cwd, 'src/d.js', 'd\n', 'docs(03): phase summary'); + seedPhase(cwd, '04-search', { '04-01-PLAN.md': '# search\n' }); + commitFile(cwd, 'src/e.js', 'e\n', 'feat(04-01): add search index'); + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect]); + assert.deepEqual(subjects(out), [ + 'docs(03): phase summary', + 'fix(03-02): correct limiter window', + 'feat(03-02): add rate limiter', + 'feat(03-01): implement auth endpoint', + ]); + }); + + test('limit-1: a matching commit one before PHASE_START is excluded; PHASE_START itself is included', (t) => { + const cwd = createTempGitProject('gsd-4465-limit-'); + t.after(() => cleanup(cwd)); + // Matching scope, committed BEFORE the phase directory exists: outside the window. + commitFile(cwd, 'src/pre.js', 'pre\n', 'feat(03-01): stray pre-phase commit'); + // PHASE_START: the commit that adds the phase directory, and it matches the scope. + seedPhase(cwd, '03-auth', { '03-01-PLAN.md': '# auth\n' }); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): add phase plan'], { cwd }); + commitFile(cwd, 'src/a.js', 'a\n', 'feat(03-01): implement auth endpoint'); + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect]); + assert.deepEqual(subjects(out), [ + 'feat(03-01): implement auth endpoint', + 'docs(03-01): add phase plan', + ]); + }); + + test('root commit: a phase whose first commit is the repository root is fully selected', (t) => { + // createTempGitProject seeds an initial commit, so build the root by hand. + const cwd = createFixture({ prefix: 'gsd-4465-root-', planning: true, git: false }); + t.after(() => cleanup(cwd)); + const g = (args) => gitOrThrow(args, { cwd }); + g(['init', '-q']); + g(['config', 'user.email', 'test@test.com']); + g(['config', 'user.name', 'Test']); + g(['config', 'commit.gpgsign', 'false']); + seedPhase(cwd, '01-seed', { '01-01-PLAN.md': '# seed\n' }); + g(['add', '-A']); + g(['commit', '-q', '-m', 'docs(01-01): add root phase plan']); + commitFile(cwd, 'src/a.js', 'a\n', 'feat(01-01): first feature'); + const out = runFences(cwd, 'TARGET_PHASE=01', [phaseResolve, phaseAnchor, phaseSelect], + 'echo "UNDO_RANGE=${UNDO_RANGE}"'); + assert.ok(out.includes('UNDO_RANGE=HEAD'), `root branch must select over HEAD; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/UNDO_RANGE=.*\n?/, '')), [ + 'feat(01-01): first feature', + 'docs(01-01): add root phase plan', + ]); + }); + + test('fail-closed: an unknown phase resolves no anchor and no range — nothing widens', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PHASE=07', [phaseResolve, phaseAnchor], + 'printf "PHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR" "$UNDO_RANGE"'); + assert.ok(out.includes('PHASE_DIR=[]'), `expected an empty PHASE_DIR for an absent phase; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `expected an empty UNDO_RANGE for an absent phase; got:\n${out}`); + }); + + test('fail-closed (--plan): an unknown plan\'s phase resolves no anchor and no range', (t) => { + const cwd = multiMilestoneFixture(); + t.after(() => cleanup(cwd)); + const out = runFences(cwd, 'TARGET_PLAN=07-01', [planAnchor], + 'printf "PHASE_DIR=[%s]\\nUNDO_RANGE=[%s]\\n" "$PHASE_DIR" "$UNDO_RANGE"'); + assert.ok(out.includes('PHASE_DIR=[]'), `expected an empty PHASE_DIR for an absent plan phase; got:\n${out}`); + assert.ok(out.includes('UNDO_RANGE=[]'), `expected an empty UNDO_RANGE for an absent plan phase; got:\n${out}`); + }); + + test('workstream: --phase resolves the ACTIVE workstream\'s phase directory, not the root\'s', (t) => { + const cwd = createTempGitProject('gsd-4465-ws-'); + t.after(() => cleanup(cwd)); + // Root scope: phase 03 exists and has a matching commit. + seedPhase(cwd, '03-root', { '03-01-PLAN.md': '# root\n' }); + commitFile(cwd, 'src/root.js', 'r\n', 'feat(03-01): root-scope work'); + // Workstream scope: its own phase 03, activated by the pointer file. + seedWorkstream(cwd, { name: 'payments', active: true }); + fs.mkdirSync(path.join(cwd, '.planning', 'workstreams', 'payments', 'phases', '03-pay'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'workstreams', 'payments', 'phases', '03-pay', '03-01-PLAN.md'), '# pay\n'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-q', '-m', 'docs(03-01): payments phase plan'], { cwd }); + commitFile(cwd, 'src/pay.js', 'p\n', 'feat(03-01): payments work'); + const out = runFences(cwd, 'TARGET_PHASE=03', [phaseResolve, phaseAnchor, phaseSelect], + 'echo "PHASE_DIR=${PHASE_DIR}"'); + assert.ok(out.includes('PHASE_DIR=.planning/workstreams/payments/phases/03-pay'), + `find-phase must resolve the active workstream's directory; got:\n${out}`); + assert.deepEqual(subjects(out.replace(/PHASE_DIR=.*\n?/, '')), [ + 'feat(03-01): payments work', + 'docs(03-01): payments phase plan', + ]); + }); + + test('dependency_check: PLANNING_DIR resolves the active workstream root, and falls back to .planning without one', (t) => { + const cwd = createTempGitProject('gsd-4465-pd-'); + t.after(() => cleanup(cwd)); + const tail = 'echo "PLANNING_DIR=${PLANNING_DIR}"'; + const flat = runFences(cwd, '', [planningRoot], tail); + assert.ok(/PLANNING_DIR=.*[\\/]\.planning$/m.test(flat), `expected the project's .planning; got:\n${flat}`); + seedWorkstream(cwd, { name: 'payments', active: true }); + const ws = runFences(cwd, '', [planningRoot], tail); + assert.ok(/PLANNING_DIR=.*[\\/]\.planning[\\/]workstreams[\\/]payments$/m.test(ws), + `expected the active workstream's root; got:\n${ws}`); + // And the fallback is real, not the only path: with no .planning at all the + // pick yields '' (planning_root is null) and the fence lands on the literal. + const bare = createFixture({ prefix: 'gsd-4465-noplan-', planning: false, git: true, projectDoc: false }); + t.after(() => cleanup(bare)); + const none = runFences(bare, '', [planningRoot], tail); + assert.ok(none.includes('PLANNING_DIR=.planning'), `expected the literal fallback; got:\n${none}`); + }); +});