* 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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<X.Y>-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) <noreply@anthropic.com> 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 <purpose> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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<newline>-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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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-<name>-<date>/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-<date>/). - 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-<name>-<date>/phases/, is what `workstream complete` writes. Driven on the real fences: a workstream `feat` completed into ws-feat-<date>/ 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) <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
0xdhx
5
.changeset/undo-bounded-commit-selection.md
Normal file
5
.changeset/undo-bounded-commit-selection.md
Normal file
@@ -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<X.Y>-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)
|
||||
@@ -1,5 +1,5 @@
|
||||
<purpose>
|
||||
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.
|
||||
</purpose>
|
||||
|
||||
<required_reading>
|
||||
@@ -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<X.Y>-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<X.Y>-phases/<phase>/` archive and the workstream archive
|
||||
`milestones/ws-<name>-<date>/phases/<phase>/` 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<X.Y>-phases/<phase>` and `ws-<name>-<date>/phases/<phase>`. 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-<name>-<date>/, 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-<date>/` 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 `<files>` sections or `consumes` fields reference outputs from the target plan
|
||||
|
||||
@@ -300,8 +598,8 @@ Show next steps:
|
||||
|
||||
<success_criteria>
|
||||
- [ ] 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
|
||||
|
||||
@@ -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",
|
||||
|
||||
1199
tests/undo-commit-selection-4465.test.cjs
Normal file
1199
tests/undo-commit-selection-4465.test.cjs
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user