feat(source-control): prefer melodic.worktreeroot for worktree placement by kyle-sexton · Pull Request #2643 · melodic-software/claude-code-plugins

kyle-sexton · 2026-08-14T23:17:46Z

Closes #2610
Closes #2612
Closes #2611

Summary

Prefer melodic.worktreeroot for worktree placement; block raw in-repo git worktree add; pass plugin option via --fallback-root-file.

Fix

See commits on this branch.

Verification

See CI checks on this PR.

All three are documentation / skill-instruction prose updates — this commit renames the flag the /worktree create skill instructs itself to pass (--root-file → --fallback-root-file), addressing the Codex P1 comment on this PR. It contains no changes to plugins/source-control/scripts/worktree-create.sh or plugins/source-control/hooks/worktree-create-gate.sh — the actual root-resolution, git-config-trust-boundary, and containment-guard logic reviewed previously is unchanged. No shell-escaping behavior changes either (the ${user_config.worktree_root} raw-substitution warnings and Write-tool-temp-file mitigation already documented remain intact, just relabeled to the new flag name).

Conclusion: no security-relevant surface in this push. No findings.

Hygiene exec-bit requires shebang scripts to be mode 100755 in the index. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

claude · 2026-08-15T00:27:46Z

Claude finished @cursor[bot]'s task in 46s —— View job

Code review

Gather context (PR state, diff, changed files)
Run skip-gate checks
Review changed files for correctness / maintainability
Post final findings

Skip gate: no review performed.

Per /review:code-review's skip gate ("PR is closed or not open"), this PR is already merged (state: MERGED) as of this invocation, so there is no open pull request to review or attach findings to. No further action taken.

github-actions · 2026-08-15T00:28:58Z

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

…2878) ## Summary `D:\d` was recreated for the third time on 2026-08-16 at 16:46:30 EDT, this time by an ordinary agent lane creating a worktree. #2841 shipped the convention, the helper and the detector for this class; none of them is a preventive control, and the detector is structurally incapable of firing before the damage. The trigger turns out **not** to be the path form. The creating lane ran `export MSYS_NO_PATHCONV=1` earlier in the same command string — to work around MSYS mangling an unrelated `<rev>:<path>` argument — and that export silently unconverted a `git worktree add /d/worktrees/...` seven segments later. The same lane had already run the identical path argument successfully. **A guard keyed on the `/[a-z]/` path shape has a false negative on the real incident**, which is the same false-green shape this body of work exists to remove. This PR adds a guard keyed on the environment instead, and fixes a shipped defect in the detector's remediation advice that the same incident exposed. ## Fix **1. New `PreToolUse` guard: `plugins/guardrails/hooks/block-exported-msys-pathconv.sh`** (`Bash|PowerShell`, Windows hosts only, default on, kill switch `block_exported_msys_pathconv_enabled`). Blocks `export MSYS_NO_PATHCONV` / `export MSYS2_ARG_CONV_EXCL` and the `declare -x` / `typeset -x` spellings. Leaves the per-command prefix form and bare assignments alone. A cheap substring pre-filter runs before any parsing, so a command that never names either variable exits immediately. **2. `scripts/check-drive-root-litter.sh` remediation text.** Its footer told the reader to "remove the phantom tree once you have confirmed it holds nothing else." The first real hit on this machine, `D:\d\worktrees\ccp-measure2`, is a **live registered git worktree** (`.git` → `gitdir: D:/repos/.../.git/worktrees/ccp-measure2`, present in `git worktree list`). Deleting it as advised strands the registry entry and leaves `git worktree prune` as the only cleanup — on a repository currently carrying 69 registered worktrees across live lanes. The text now sends the reader to `git worktree remove --force` first. `git grep worktree` over the script previously returned nothing. **3. `docs/conventions/windows-path-emit/README.md`** gains rule 5 (never export a suppressor) and a section on the conditional colon-argument mangling that sends authors reaching for one. Doc surfaces wired: guard table, kill-switch table, generated options table, `docs/CATALOG.md`, telemetry schema + example + producer registry, `skills/setup/SKILL.md`, count-bearing prose, CHANGELOG, version `0.28.30` → `0.29.0`. ### Why not extend `block-windows-drive-tmp.sh` Reuse-or-replace applies, and the answer is an openly-scoped sibling rather than a silent second way. That guard is a **path-shape and write-target** matcher scoped to the literal component `tmp`, with tmp-specific allowances (`/var/tmp`, `%TEMP%`). This one is an **environment-variable** matcher with no path component at all. Neither would fire on the other's cases. Folding two disjoint matchers and two exclusion sets into one hook makes both harder to reason about and risks regressing a guard that works. Both headers now carry a reciprocal cross-reference naming the split. ### Why not `scripts/check-shell-portability.sh` An authoring-time lint class for this idiom is genuinely complementary and worth doing — but a parallel lane owns that dispatcher right now (#2840). Routed as a follow-up in #2870 rather than colliding. ## Verification **Mechanism, by execution.** No artifact was created at any real drive root; `subst` mapped throwaway virtual drives onto scratch subdirectories, all removed with removal verified by listing. `D:\d` was never modified, moved or removed, and `git worktree prune` was never run. The exact incident command shape, PowerShell, throwaway repo on `subst` drive `P:`: ```text git -C P:\repo worktree add --detach /d/worktrees/ccp-reproA ``` created and registered `P:\d\worktrees\ccp-reproA`. Drive-dependence, same argument from two drives: ```text Set-Location P:\ ; git init /d/worktrees/probe -> P:\d\worktrees\probe Set-Location Q:\ ; git init /d/worktrees/probe -> Q:\d\worktrees\probe ``` An independent agent, given no sight of the above reasoning, re-derived the same result from scratch on `X:`/`Y:` and isolated the responsible layer — the result flips with argv conversion alone, so `git.exe` resolves `/y/...` against the current drive and Git Bash normally masks it. **The predicate, four ways in one measurement.** `git rev-parse --sq-quote` prints exactly what `git.exe` received and creates nothing: ```text bash -c 'git rev-parse --sq-quote /d/probe' -> 'D:/probe' converted bash -c 'MSYS_NO_PATHCONV=1; git rev-parse --sq-quote /d/probe' -> 'D:/probe' bare assignment: no effect bash -c 'export MSYS_NO_PATHCONV=1; git rev-parse ... /d/probe' -> '/d/probe' THE DEFECT bash -c 'MSYS_NO_PATHCONV=1 git ... /d/a; git ... /d/b' -> '/d/a' then 'D:/b' prefix scopes it ``` **Measured precision (ADR-0003).** Corpus: 16,919 `tool_input.command` strings from 701 local transcript JSONL files (388 MB), split by tool, plus 710 lines of PowerShell console history. | matcher | corpus | n | fires | rate | | --- | --- | --- | --- | --- | | path shape `/[a-z]/` (**rejected**) | Bash tool | 14,234 | 6,506 | **45.7 %** | | path shape, narrowed to `git worktree add` (**rejected**) | Bash tool | 137 | 111 | **81 %** | | **exported suppressor (shipped)** | Bash tool | 14,234 | **46** | **0.32 %** | | exported suppressor | PowerShell tool + console history | 3,395 | 0 | 0 % | | per-command prefix — the safe idiom, correctly **not** fired on | Bash tool | — | 193 | — | Numbers are not a regex approximation: **every command in the corpus was replayed through the built hook binary**, with `OSTYPE=msys`, as a real PreToolUse payload on stdin. ```text MSYS-mentioning Bash cmds total=204 blocked=46 allowed=158 control sample (no mention) total=400 blocked=0 allowed=400 ``` The 46 span four distinct lanes (`ccp-measure2`, `ccp-silent-revert-calib`, `ccp-silent-revert-fixture`, `ccp-verify-2843`) and two repositories, including the lane family that produced this incident. They are **unseeded** — the reproductions run for this work used the safe prefix form and are among the 193 correctly left alone — so this is ADR-0003's measured-precision path, not its seeded-defect exemption. **Precision claim, stated exactly.** All 46 are genuine instances of the anti-pattern: the exported form always has unbounded blast radius over the rest of the command string, and the remedy is one keystroke. What the measurement **cannot** claim is a per-fire count of "would have produced litter" — that depends on which later command consumed a path, which is not statically decidable. Two honesty notes: the corpus is one machine's, the deployment surface but not a fleet; and it contains this investigation, which is why no count keyed on a bare mention of the variables is used. **Declared coverage gaps**, in the hook header rather than hidden: a suppressor exported by a script the command invokes; `set -a` plus a bare assignment; an expansion-built value; and any spawner outside the two tool surfaces (CI runners, `subprocess`), which no PreToolUse hook can see. ## Test plan New `plugins/guardrails/hooks/block-exported-msys-pathconv.test.sh` — **PASS=50 FAIL=0**. Auto-discovered by `scripts/run-plugin-tests.sh`; no `ci.yml` change, so `check-lane-coverage.sh` is unaffected. Covers: the POSIX host gate; the real #2870 incident shape; `export`, `declare -x`, `typeset -x`, chained-`&&`, both-variables and leading-unrelated-assignment forms; the safe per-command-prefix and bare-assignment forms; six mention-not-setting cases (grep, commit prose, `gh issue create` title, `unset`, a similarly-named variable, `echo`); an ordinary `git worktree add` with an MSYS path, asserting the design decision not to match a path shape; the PowerShell surface; fail-closed over-length and payload cases; and the kill switch. Everything else run locally against the branch: | gate | result | | --- | --- | | `scripts/check-drive-root-litter.test.sh` | PASS 16/16 (2 new assertions on the remediation text) | | `plugins/guardrails/hooks/block-windows-drive-tmp.test.sh` | PASS 93/93 (unchanged behavior after the header edit) | | `require-jq-posture.test.sh` (auto-enrolls the new hook) | PASS 40/40 | | `require-jq-notice-isolation.test.sh` | PASS 2/2, 10 distinct keys | | `scripts/check-silent-skips.sh` | clean | | `scripts/check-hook-exec-form.sh` | clean | | `scripts/check-hook-userconfig-argv.sh` | clean | | `shellcheck -x` (repo `.shellcheckrc`) on all changed shell | rc=0 | | `scripts/check-shell-portability.sh origin/main` | clean, 8 files | | `check-changelog-parity.sh` `--check` / `--check-bump` / `--check-order` | all pass | | `scripts/validate-plugins.sh` | pass | | `sync-plugin-options-docs.py --check` / `generate-catalog.mjs --check` / `generate-cheatsheet.mjs --check` | all in sync | | `markdownlint-cli2` on every changed markdown | 0 errors | Both new files are mode `100755`. ## Related - Closes #2870 - #2841 — landed the convention, helper and detector this completes - #2834 (closed by #2841) — same family, harness-authored producer - #2594 (closed) — produced `block-windows-drive-tmp.sh`, the sibling guard - #2611 (closed by #2643) — produced `worktree-add-containment-gate.sh`, the second guard that did not fire - `docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md` - melodic-software/dotfiles#486 — machine-level rule for the same mechanism ## Review-round matcher changes (added after the five review threads) The three review-round commits (`6f462b3b8`, `a8462fe82`, `acbb881f0`) changed the matcher's firing envelope in both directions, and one widening is easy to miss from the commit messages alone: - **Narrowed** (review thread on quoted prose): without a shell word in the command string, the export keyword must now sit at command position, so commit messages, `echo` arguments, and grep patterns quoting `export MSYS_NO_PATHCONV=1` are allowed. - **Widened** (verification finding): quote normalization now strips quotes from BOTH sides of every token, so a shell name at the very end of a quoted string flips the matcher into loose mode. Concretely, `git commit -m "do not export MSYS_NO_PATHCONV=1 in bash"` now blocks (the trailing `bash"` reads as the word `bash`), while the same sentence not ending in the shell name — `"...in bash, do not export MSYS_NO_PATHCONV=1"` — stays allowed. This is inside the header's declared residual false-positive class, errs fail-closed with an instructive message, and is the accepted cost of closing a true-leak false negative: a fully quoted shell word (`'bash'`, or `"C:\Program Files\Git\bin\bash.exe"` behind PowerShell's `&`) previously evaded the shell-word check entirely. - **Widened** (verification finding, pre-existing gap closed): the assignment side of the token walk gets the same quote normalization, so `bash -c "MSYS_NO_PATHCONV=1 bash -c '...'"` — where the suppressor prefix is the first word of a quoted child command string — now blocks. All declared residuals are documented in the hook header and pinned by tests (127 assertions, up from 66 at review time). --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

kyle-sexton enabled auto-merge (squash) August 14, 2026 23:17

chatgpt-codex-connector Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread plugins/source-control/scripts/worktree-create.sh

auto-merge was automatically disabled August 14, 2026 23:23
Pull Request is not mergeable

kyle-sexton enabled auto-merge (squash) August 14, 2026 23:25

cursor Bot force-pushed the cursor/feat-source-control-worktree-convention-f8b0 branch 8 times, most recently from aa8ae81 to c62f487 Compare August 15, 2026 00:09

fix(source-control): mark worktree convention tests executable

f113275

Hygiene exec-bit requires shebang scripts to be mode 100755 in the index. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

cursor Bot force-pushed the cursor/feat-source-control-worktree-convention-f8b0 branch from c62f487 to f113275 Compare August 15, 2026 00:11

kyle-sexton merged commit d55ffbf into main Aug 15, 2026
39 checks passed

kyle-sexton deleted the cursor/feat-source-control-worktree-convention-f8b0 branch August 15, 2026 00:19

Navigation Menu

Uh oh!

feat(source-control): prefer melodic.worktreeroot for worktree placement - #2643

feat(source-control): prefer melodic.worktreeroot for worktree placement#2643
kyle-sexton merged 1 commit into
mainfrom
cursor/feat-source-control-worktree-convention-f8b0

kyle-sexton commented Aug 14, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

claude Bot commented Aug 15, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Verification

Related

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review in progress

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review

Uh oh!

Uh oh!

claude Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kyle-sexton commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 14, 2026 •

edited

Loading

github-actions Bot commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 15, 2026 •

edited

Loading