feat(CC-014): repo-wide git worktree tooling for parallel development#358
Merged
Conversation
Was scoped only to --parallel PR gate reviewer isolation. Requirement now covers a general worktree create/list/clean utility for parallel multi-ticket development, with gate reviewer isolation as one downstream use case rather than the whole scope.
…ees skill Adds a repo-wide git worktree registry so multiple branches/tickets can be worked on in parallel without interfering with each other's uncommitted changes. Wraps `git worktree add/remove/prune` with an out-of-repo JSONL manifest (state store, keyed by main-repo identity). Identity seam: manifest partitioning uses a new _sw_main_repo_root / _sw_worktree_project_key pair in state-paths.sh, resolved via `git rev-parse --git-common-dir` instead of `--show-toplevel` — this makes `pmctl worktree list/gc` return the same result whether invoked from the primary checkout or from inside a linked worktree it created. Verified manually before building the rest of the feature on top of it. Existing _sw_project_key/dispatch/gate partitioning is untouched. `--parallel` PR gate reviewer isolation is intentionally out of scope here (tracked as a separate follow-up) — this PR only adds the general-purpose worktree utility and its skill doc.
…self-cwd bug Addresses NO-GO findings from pr-gate (qa-tester/risk-reviewer block, critic block-soft): - gc --merged evaluated "merged" against the invoking worktree's own HEAD, so running it from inside a linked worktree treated that worktree's branch as trivially "merged into itself" and removed it. Now evaluated against the primary checkout's HEAD via a new _pmctl_worktree_main_root helper, regardless of which worktree --cd points at. - gc unconditionally force-removed merged/aged worktrees, discarding uncommitted changes with no confirmation (remove requires --force for the same case). gc now attempts a plain removal first and skips + reports dirty matches unless the caller passes the new `gc --force` flag. Orphaned/untracked entries (nothing live to lose) are unaffected. - Manifest reg_dir is now resolved once up front via `sw_project_worktree_dir <work_dir>` (an explicit-arg variant added to state-paths.sh) instead of re-deriving it through `cd "$work_dir"` after a destructive operation may have deleted that very directory. - scripts/test-pmctl-worktree.sh: inline shellcheck disable comments (SC1091/SC2154) matching the repo's existing test-pmctl-*.sh convention, raw `shellcheck` is clean without relying solely on lint.yml ignore_names. - Added regression coverage for the self-removal and dirty-merged-skip cases, plus behavior/Steps docstrings on every case per QA convention.
…coverage Addresses second-round pr-gate findings (qa-tester block, critic/risk- reviewer medium): - create no longer silently succeeds when the manifest append fails after git worktree add already ran. gc cannot discover a worktree that never got a manifest entry, so this is not gc-recoverable — return nonzero and point at manual `git worktree remove` recovery instead of a misleading "run gc to reconcile" message. - Added direct coverage for gc --max-age-days actually removing an aged entry (previous test only proved a fresh entry survives) and for --max-age-days rejecting a non-integer value.
…tching, stale-slug guard
Addresses third-round pr-gate findings (security-reviewer block, hard
gate; architecture-reviewer block-soft; qa-tester block; corroborated
independently by critic and risk-reviewer):
- pmctl_worktree_ensure_root_safe() routes every worktree-registry write
(manifest mkdir, mktemp/rewrite, and the git checkout create/remove/gc
perform under it) through the same _sw_ensure_store_root_safe check
every other state-store writer already goes through, so a symlinked or
non-owned PM_DISPATCH_STATE_ROOT is rejected instead of silently
accepted.
- gc --merged now does exact string matching against `git branch --merged`
output (stripping git's fixed 2-char marker column) instead of
interpolating the branch name into a `grep -E` pattern -- a branch
containing regex metacharacters (e.g. "a.b") could previously
false-positive match an unrelated already-merged branch ("axb") and get
wrongly removed.
- create now also checks the manifest itself for an existing entry with
the target slug, not just live filesystem/git state -- a checkout
deleted and `git worktree prune`d outside pmctl (bypassing
remove/gc) left a stale manifest row that the old checks couldn't see,
letting the same slug get registered twice.
- Added direct regression coverage for all three: symlinked state-root
rejection, stale-manifest duplicate-slug recreation, and a regex-
metacharacter branch name that must not false-positive match.
Addresses fourth-round pr-gate finding (risk-reviewer block; corroborated
by critic block-soft and qa-tester block): --cd on create/list/remove/gc
accepted a missing value and fell back to the invoking pmctl's own repo
(work_dir="${args[i+1]:-}" -> empty -> "${work_dir:-$repo_root}"), so a
malformed `pmctl worktree remove <x> --cd` (or gc) with the value omitted
could silently operate on the pm-dispatch checkout instead of failing --
risky for subcommands that delete worktrees.
--cd now requires a non-empty operand at all four call sites, matching
the existing --from/--name validation style, and exits 2 before touching
git or the manifest. Added a missing-operand regression test per
subcommand.
…--max-age-days
Addresses fifth-round pr-gate finding (qa-tester block; critic advise).
The `date -jf` fallback used for gc --max-age-days on platforms without
GNU `date -d` stripped the timezone suffix with `${created_ts%%[+-]*}`
(greedy, longest-suffix match) instead of `${created_ts%[+-]*}` (shortest
match). For an ISO timestamp like "2026-07-02T13:27:18+09:00" the greedy
form strips from the FIRST "-" in the string (inside the date portion
itself), leaving only "2026" -- so the fallback path silently computed a
wrong epoch and aged worktrees were never recognized as aged on that
path.
Added a regression test that installs a fake `date` on PATH which
rejects `-d` (forcing the fallback branch to actually run) and re-derives
the epoch through the real system date for `-jf` calls -- this proves
both that the fallback branch executes and that it parses the timestamp
correctly, on Linux CI where the primary `date -d` path would otherwise
always be taken. Verified the test fails against the pre-fix `%%` form
before restoring the fix.
…/create race Addresses sixth-round pr-gate findings (risk-reviewer block; critic and architecture-reviewer block-soft; qa-tester block on missing concurrency coverage) -- all four independently converged on the same root cause. remove/gc read the manifest unlocked to DECIDE what to remove, then wrote that decision's full-content snapshot back under a lock that only protected the final `mv`. A `create` that appended between the read and the write was silently overwritten out of existence: the checkout stayed on disk but `pmctl worktree list/gc` could no longer see it. Replaced the "read snapshot -> compute full replacement -> lock only the write" pattern (pmctl_worktree_manifest_rewrite, now removed) with pmctl_worktree_manifest_remove_slugs(reg_dir, slug...): a single locked critical section that re-reads the manifest, filters out only the given slugs, and writes back -- all inside one lock acquisition. remove commits one slug; gc accumulates removed_slugs during its (still unlocked) decision pass and commits them all in one call at the end. Either way, any entry a concurrent create appended after the decision-time read is never in the removal set, so it survives the commit regardless of when it landed relative to that read. remove also now fails (nonzero exit) if manifest cleanup fails after the git worktree is gone, instead of reporting success with a stale registry (critic/risk-reviewer medium finding, folded in with the same commit since it's the same code path). Added a deterministic regression test that calls the manifest primitives directly to force the exact interleaving (decision read -> concurrent append -> removal commit) without relying on OS-level thread timing, and asserts the concurrently-appended entry survives.
… checks Addresses seventh-round pr-gate finding (security-reviewer block, hard gate, reproduced with a `[`-containing state root; risk-reviewer block; qa-tester block; critic block-soft) -- the same regex-vs-data bug class already fixed once for branch names in gc --merged, missed here because it also existed independently in two other call sites that interpolate a filesystem path (not a branch name) into `grep -q "^worktree $path\$"`. PM_DISPATCH_STATE_ROOT is arbitrary external data, not something this tool controls the syntax of -- a state root containing `[` or other regex metacharacters corrupted the pattern, so `gc`'s "is this path still tracked by git" check could false-negative a live, dirty worktree as "git no longer tracks this" and force-remove it with no --force confirmation, discarding uncommitted changes. The same interpolation existed in `create`'s duplicate-path check (non-destructive there, but same bug class). Both call sites now use `grep -Fxq` (fixed-string, whole-line match) against `git worktree list --porcelain` output instead of a regex. Added a regression test that reproduces the exact scenario security- reviewer used (PM_DISPATCH_STATE_ROOT containing `[`, a dirty live worktree, plain `gc`) and asserts the worktree survives. Verified the test fails against the pre-fix regex form before restoring the fix.
Marks CC-014 terminal (✅ closed 2026-07-02 / ✅ done) with pr:#358 and an Outcome summary, per pr-gate GO on this branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pmctl worktree create/list/remove/gc— a general-purpose git worktree registry for parallel multi-ticket development (not just PR gate reviewer isolation, which is scoped out to a separate follow-up)git rev-parse --git-common-dir) instead of the caller's own toplevel, so the registry resolves to the same partition whether invoked from the primary checkout or from inside a linked worktreecommands/using-git-worktrees.mdskill doc for using the toolDesign notes
_sw_project_key/ dispatch / gate partitioning is untouched — this adds a new resolver (_sw_worktree_project_key) used only by the worktree feature--parallelPR gate reviewer isolation intentionally out of scope (tracked separately)Test plan
bash scripts/test-pmctl-worktree.sh— 22/22 passingbash scripts/test-commands.sh— 221/221 passing (includes new using-git-worktrees.md contract assertions)bash scripts/test-state-paths.sh,test-pmctl-artifacts.sh,test-state-store.sh— no regressions from state-paths.sh changesshellcheckclean on all touched fileslint-scripts.sh,lint-frontmatter.sh,pmctl backlog lintcleanpmctl gate run --executor codex