feat(source-control): route /worktree create through a shared worktree-creation helper (#399 Phase A)

[kyle-sexton]

Summary

Phase A of #399: unify worktree creation behind one shared helper so /worktree create no longer lands in Claude Code's in-repo .claude/worktrees/ (the nested placement that triggers the confirmed CLAUDE.md/rules double-load bug, #400). This ships the helper + the skill routing; the native WorktreeCreate hook (Phase B) is intentionally not shipped here — it stays gated on two empirical upstream gates (see Phase B boundary below).

Implements the operator ruling recorded on #398 (2026-07-21): central-external-root placement, with the root stored in a new worktree_root userConfig directory key mirroring the existing babysit_worktree_root shape.

What changed

• plugins/source-control/scripts/worktree-create.sh — the single owner of worktree creation:
  • Computes the external path <root>/<owner>-<repo>-<slug> (owner/repo parsed from the origin remote via a named function that handles GitHub, GitLab subgroups, and Azure DevOps _git/v3 markers; falls back to the repo-dir name when there is no remote).
  • Validates --name up front against the EnterWorktree schema (≤64 chars, each /-segment [A-Za-z0-9._-]) and refuses invalid names (exit 2) rather than letting git worktree add fail opaquely. The branch is used verbatim; only the directory slug transforms (/ → -).
  • Resolves the base ref like Claude Code does: worktree.baseRef fresh (default branch resolved symbolically via origin/HEAD, never a hardcoded origin/main) or head; warns on stderr when it must fall back to local HEAD.
  • Runs git worktree add, then reimplements Claude Code's .worktreeinclude copy — the intersection of .worktreeinclude-matched and gitignored files (that copy is bypassed when a worktree is created with git worktree add directly). NUL-delimited enumeration; copy failures are reported and surfaced (exit 4), not silently counted.
  • Output contract: on success the created worktree path is the sole stdout line (machine-parseable); all diagnostics go to stderr.
• New worktree_root userConfig directory key (plugin.json). Its description encodes refuse-with-guidance — when unset, /worktree create refuses and points at setup; it does not silently fall back to the in-repo default.
• /worktree create patched (SKILL.md + context/create.md): run the helper, then EnterWorktree(path:) into the created worktree. On any non-zero helper exit it STOPs and never falls back to EnterWorktree(name:). Documents the out-of-.claude/worktrees/ approval prompt (not suppressible outside bypassPermissions) and the declined-approval recovery.
• 37-case test suite (worktree-create.test.sh): path computation + slug, refuse-when-unconfigured (empty root and unexpanded ${user_config.*} token), https/scp/Azure/GitLab URL parsing, base-ref fresh/head (incl. uncached-origin/HEAD fallback warning), .worktreeinclude intersection, trailing-valueless-flag guard, unsafe/over-long name refusal, --repo-dir outside a repo, trailing-slash root.

Refuse-behavior evidence

With worktree_root unset (empty or the literal ${user_config.worktree_root} token), the helper exits 3 and prints, to stderr:

worktree-create.sh: worktree root is not configured — refusing to create a worktree.
Set the source-control plugin's `worktree_root` directory key to an external
root (a path OUTSIDE every repository, on the same drive as the repo on Windows),
then retry. Run the worktree setup skill, or configure it via `/plugin`.
Not falling back to the in-repo .claude/worktrees/ default: that nested
placement triggers Claude Code's CLAUDE.md/rules double-load bug (#400).

The skill treats exit 3 as a hard stop — it never falls through to the in-repo path.

Phase B boundary (not in this PR)

The native WorktreeCreate hook routes the same helper, but only once both empirical gates pass:

• Gate 1 — real stdin field set (docs say worktree_name; reality is reportedly .name): upstream anthropics/claude-code #77566.
• Gate 2 — whether a hook-relocated subagent-isolation worktree (outside .claude/worktrees/) prompts/blocks under normal permissions: upstream anthropics/claude-code #78212. Confirmed for the interactive path here (the entry prompts; acceptable because the user is present), but the autonomous path needs #78212 resolved before the hook can ship.

Phase-B precondition surfaced in review: the helper's flag CLI is the stable seam, but the hook adapter will need its own validation of the stdin→flag mapping (base-ref derivation, .name sanitization) before it can be trusted — the helper's guards cover the flags it receives, not the adapter that produces them.

Verification

Local gates all green on the rebased branch: shellcheck, worktree-create.test.sh (37 cases), portability-lint, changelog-parity (--check-bump), validate-plugins, markdownlint, skill-quality (PASS). Signed commits. An independent fresh-context review pass ran before this PR; its CRITICAL (arg-loop hang on a valueless trailing flag) and four IMPORTANT findings are all addressed in the second commit.

Windows-specific path semantics this helper serves are not exercised in CI (no Windows runner) — recorded as a systemic observation on #657.

Closes #399

Related

• feat(source-control): machine-agnostic worktree setup skill (detect -> decide -> install) #398 — worktree-convention epic; its 2026-07-21 operator ruling (central-external-root + the worktree_root userConfig key) is what this PR implements. feat(source-control): machine-agnostic worktree setup skill (detect -> decide -> install) #398 remains open (it owns detect→decide→install and the hook).
• fix(source-control): /worktree creates in-repo .claude/worktrees/ -> hits CC double-load bug; create out-of-tree #400 — the confirmed CLAUDE.md/rules double-load bug (upstream anthropics/claude-code #29599 / #23565). This PR fixes the placement for the interactive path; fix(source-control): /worktree creates in-repo .claude/worktrees/ -> hits CC double-load bug; create out-of-tree #400 stays open and blocked-by this work (it is not closed here).
• chore(source-control): retire stale ghq-layout-sibling-pr-worktrees doc; plugin owns worktree convention #401 — retire the stale dotfiles worktree doc; follows once the plugin fully owns the convention. Not touched here.
• Phase B gates: upstream anthropics/claude-code #77566 (stdin field) and #78212 (out-of-tree permission behavior).

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 4m 39s —— View job

---

PR Review

• Gather context from PR metadata
• Read changed files
• Review worktree-create.sh (main helper script)
• Review worktree-create.test.sh (test suite)
• Review plugin.json, SKILL.md, create.md changes
• Post comprehensive review feedback

---

This is a well-designed, carefully-guarded piece of work. The output contract, refuse-with-guidance pattern, need_value trailing-flag fix, NUL-delimited .worktreeinclude enumeration, and origin/HEAD symbolic resolution are all solid. One implementation question on the .worktreeinclude intersection is worth verifying before merge; the rest are minors.

---

IMPORTANT — .worktreeinclude intersection may copy more than intended

File: worktree-create.sh lines 253–275

git -C "$toplevel" ls-files -o -i -z --exclude-from="$include_file"

The comment says this "yields untracked files matching the include patterns." That is what the intent is, but git ls-files -o -i --exclude-from=FILE may not restrict output to patterns in FILE alone. Git reads the full gitignore chain by default (.gitignore, $GIT_DIR/info/exclude, core.excludesFile); --exclude-from adds patterns, it does not replace the standard sources. Under that semantics, the output would be all gitignored untracked files, and the subsequent check-ignore -q filter would be a no-op (all of them are already gitignored), causing gitignored-but-not-in-.worktreeinclude files to be copied.

In the test fixture this matters for .work/task1/OTHER.md (gitignored via .work/, absent from .worktreeinclude), which assert_file_absent expects to be skipped. If git's behavior is (b) above, the test fails. The fact that the test is reported green suggests git may behave as (a) — restricting to FILE patterns when --no-standard-excludes is implied by the absence of other sources — but the exact semantics depend on git version.

Ask: Please either (a) confirm this with a --no-standard-excludes run to validate the assumption is version-stable, or (b) invert the filter: enumerate .worktreeinclude patterns with git ls-files -o -i -z --exclude-from=.worktreeinclude, then post-filter with git check-ignore -q to keep the gitignore intersection. If the current approach is empirically correct, add a comment citing which behavior of -i --exclude-from you're relying on.

Fix this →

---

Minor — omitted --root exits 3 (refuse) rather than 2 (usage)

File: worktree-create.sh lines 88–125

The code validates --name up front and exits 2 on a missing or invalid name (lines 88–106). But if --root is simply omitted (rather than explicitly set to empty or an unexpanded token), it falls through to the unconfigured-root refuse block (exit 3). The distinction is intentional for the plugin use-case (the skill always passes --root "${user_config.worktree_root}" and a missing-config token is a "refuse" situation), but a user calling the helper directly with a forgotten --root gets a "refuse" message rather than a "usage" message. A short guard after the --name validation:

if [[ -z "$name" ]]; then
  printf '%s: --name is required\n' "$PROG" >&2; exit 2
fi
# Consider: add an explicit "root not provided" check here → exit 2

Not a blocking issue; the guidance message is still actionable. But it breaks the "usage errors → 2, config errors → 3" semantic contract the exit-code table documents.

---

Minor — git config worktree.baseRef path is untested

File: worktree-create.sh lines 212–215

if [[ -z "$base_ref" ]]; then
  base_ref=$(git -C "$toplevel" config worktree.baseRef 2>/dev/null || true)
fi

The test suite covers --base-ref fresh and --base-ref head explicitly, and the uncached-origin/HEAD fallback. But a repo with worktree.baseRef = head set in git config, invoked without a --base-ref flag, is not tested. Since the PR positions git config as the middle tier in the precedence chain, a case like:

git -C "$repo" config worktree.baseRef head
out=$(bash "$HELPER" --name feat/cfg-base --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_file_exists "git config worktree.baseRef = head carries unpushed commit" "$out/UNPUSHED.md"

would close that gap.

---

Minor — Azure SSH URL format (git@ssh.dev.azure.com:v3/…) not tested

File: worktree-create.test.sh lines 156–165

The test covers Azure HTTPS (https://dev.azure.com/myorg/myproject/_git/widget) and GitLab subgroups. The v3 strip in parse_owner_repo also targets Azure SSH SCP syntax (git@ssh.dev.azure.com:v3/myorg/myproject/widget), but that path goes through the *:* branch of parse_owner_repo rather than the *://* branch. A test case:

repo=$(mkrepo --origin "git@ssh.dev.azure.com:v3/myorg/myproject/widget")

would confirm the v3 strip works for SSH and not just HTTPS.

---

Observation — unreachable slug-empty guard

File: worktree-create.sh lines 193–196

After name validation enforces ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$, every segment is guaranteed to contain at least one non-/ alphanumeric/dot/underscore/dash character. Since only / → - in the sanitization step (all other allowed chars pass through), the trimmed slug can never be empty. The guard is harmless defensive code but is dead after the earlier validation. Not worth changing, just noting it so the reader doesn't hunt for a code path that could reach it.

---

Positives

• need_value guard (lines 70–75) is the right fix for the trailing-valueless-flag hang. Calling it as need_value "$@" is intentional and works correctly.
• origin/HEAD symbolic resolution (line 228) rather than origin/main is the correct portability choice.
• Refuse-with-guidance message (lines 114–124) is specific and actionable — naming the config key, the fallback that's being refused, and the bug number.
• NUL-delimited .worktreeinclude enumeration (-z / read -d '') handles paths with spaces and special characters correctly.
• Output contract (stdout = sole path, stderr = all diagnostics) is machine-parseable and the skill correctly captures just stdout.
• parse_owner_repo fallback chain (GitHub, GitLab subgroups, Azure _git, Azure v3, no-remote) is well-structured.
• Test fixture mkrepo correctly seeds origin/HEAD so the "fresh" base-ref path is exercised without a real network fetch.

---

[claude]

Claude finished @kyle-sexton's task in 4m 33s —— View job

---

Security Review

• Gather PR diff and changed files
• Review worktree-create.sh for injection/path traversal/command injection
• Review worktree-create.test.sh for test harness security issues
• Review plugin.json changes
• Review skill/context documentation changes
• Compile and post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md

Overall posture: The script is carefully written. Variables are consistently quoted, the branch name is validated against a strict allowlist before it reaches any git call, NUL-delimited enumeration is used throughout the .worktreeinclude copy, and the -- separator correctly guards git check-ignore against dash-leading filenames. No command injection surface, no CRITICAL or IMPORTANT findings.

Two SUGGESTION-level issues found:

---

SUGGESTION — --root is not validated as an absolute path | confidence: PLAUSIBLE

worktree-create.sh line 203

worktree_path="${root%/}/${dirname}"

$root is accepted as-is from --root. If it is a relative path (e.g. ../../somewhere), the final worktree_path is resolved relative to whatever the shell's $CWD happens to be at the moment the helper runs. When Claude Code invokes the helper, the CWD may not be the repository root — it could be inside a deeply-nested subdirectory, causing the worktree to land far from where the user expects.

The unconfigured-root guard (exit 3 on empty/unexpanded token) prevents the truly-empty case, but a legitimately configured relative path passes through. Since git worktree add will succeed with any writable path, there is no downstream guard.

Suggested fix: add [[ "$root" == /* ]] || { printf '%s: --root must be an absolute path: %s\n' "$PROG" "$root" >&2; exit 2; } immediately after the unconfigured-root check. Absolute paths are the stated contract ("a path OUTSIDE every repository"), and enforcing it here removes the CWD-dependency entirely.

---

SUGGESTION — cp -p in the .worktreeinclude loop follows symlinks to outside-repo targets | confidence: PLAUSIBLE

worktree-create.sh lines 257–268

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). If a repository contains a tracked or untracked symlink whose target is a regular file outside the repository (e.g. secrets.env -> /etc/some-system-config), and that symlink matches a .worktreeinclude pattern and is gitignored, cp -p "$src" reads and copies the contents of the symlink target into the new worktree. No shell injection is possible here — the concern is unintended exfiltration of out-of-repo content during a routine worktree creation.

The risk is bounded to cases the user or a malicious .worktreeinclude/symlink author controls directly, so this is not a remote-exploitation vector. It is worth documenting in the skill or the worktree_root description that .worktreeinclude patterns applied to symlinks will copy the symlink target's contents, not the symlink itself.

---

Notes (informational, not security findings)

• git ls-files output and ../ paths: The rel values piped into the copy loop come from git ls-files -o -i -z --exclude-from, which bounds its traversal to the repository tree. Standard git will not emit ../ paths for untracked files. The NUL-delimiter and read -d '' correctly handle filenames containing spaces or newlines.
• base_commit from symbolic-ref: The ref string from git symbolic-ref refs/remotes/origin/HEAD is always passed as a quoted argument to git worktree add; shell injection is not possible. Pathological ref names (e.g. a ..-containing ref) would cause git itself to error, not to execute attacker-controlled code.
• URL parsing → directory name: Owner and repo strings extracted from the remote URL are used only in filesystem path construction, always quoted. Even a crafted URL (git@host:../../etc/passwd.git) resolves to a directory name of the form etc-passwd-<slug> inside the root, not a traversal above it, because segments are joined with - not /.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7ba0986f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 57s —— View job

---

Security Review

• Read PR diff and changed files
• Audit worktree-create.sh for injection, path traversal, and command execution risks
• Audit worktree-create.test.sh for test harness security issues
• Review plugin.json, SKILL.md, create.md changes
• Compile and post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md

Overall posture: The script is carefully written. Variables are consistently quoted throughout, -- separators guard git calls against dash-leading filenames, NUL-delimited enumeration is used for .worktreeinclude copy, the need_value guard correctly prevents the trailing-flag hang, and URL-derived owner/repo strings are joined with - (not /) so crafted remote URLs cannot produce path traversal. No CRITICAL or IMPORTANT findings. Two SUGGESTION-level issues found — both previously identified in the prior automated review and still unaddressed.

---

SUGGESTION — --root is not validated as an absolute path | confidence: PLAUSIBLE

worktree-create.sh line 205

worktree_path="${root%/}/${dirname}"

$root is accepted from --root without being checked for absolute path form. If it is relative (e.g. ../../outside), the resulting $worktree_path is also relative, but the two consumers that evaluate it disagree on the base:

• [[ -e "$worktree_path" ]] (line 207) resolves relative paths against the shell's CWD at invocation time.
• git -C "$toplevel" worktree add … "$worktree_path" (line 243) resolves relative paths against $toplevel (the repo root).

If these differ — e.g., the helper is called from a subdirectory — the existence check and the actual creation target resolve to different locations. The "target already exists" guard can be bypassed silently, and the worktree can land outside the intended root.

The PR description's guidance says the root "must be a path OUTSIDE every repository", implying an absolute path is intended. The directory type in plugin.json may supply an absolute path via Claude Code's config UI, but the helper enforces no such constraint itself — any caller (the Phase B hook adapter, a direct command-line invocation) can pass a relative path.

Suggested fix: add immediately after the unconfigured-root check (after line 127):

if [[ "$root" != /* ]]; then
  printf '%s: --root must be an absolute path: %s\n' "$PROG" "$root" >&2
  exit 2
fi

---

SUGGESTION — cp -p follows symlinks to out-of-repo targets during .worktreeinclude copy | confidence: PLAUSIBLE

worktree-create.sh lines 262–265

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). If a repository contains a symlink whose target is a regular file outside the repository — e.g. secrets.env -> /etc/some-system-config — and that symlink (a) is gitignored, and (b) matches a .worktreeinclude pattern, then cp -p "$src" "$dest" reads and copies the contents of the symlink target into the new worktree. The symlink itself is replaced by a regular file containing the out-of-repo content.

Exploitation requires local control: the attacker must be able to add such a symlink to the working tree and add a matching .worktreeinclude entry. No remote-execution vector exists. The risk is unintended propagation of out-of-repo content to a new worktree without the user realizing it.

Suggested fix: add a [[ -L "$src" ]] && continue guard before the -f test to skip symlinks entirely, or use cp -P (copy symlinks as symlinks) if preserving them is desirable:

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

Alternatively, add a note to the worktree_root description (or the create.md docs) warning that .worktreeinclude patterns matching symlinks will copy the symlink target's contents, so users know what they are authorizing.

---

Notes (informational, not security findings)

• ..-segment names pass the --name regex but are blocked by git. The name regex (^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$) permits segments composed entirely of dots (..). This is not exploitable for path traversal: the slug step transforms every / to -, so ../.. becomes ..-..-slug — a literal path component inside $root, not an escape. Git's own check-ref-format then rejects .. as an invalid branch name component (exit 4). No path traversal possible; the only harm is a less actionable error message than exit 2 would give.
• URL-derived owner/repo strings are path-traversal-safe. parse_owner_repo joins path segments with - (not /), so even a crafted remote URL (git@host:../../etc/passwd.git) produces a directory name like etc-passwd-<slug> inside $root, never a traversal above it. All downstream uses of owner/repo are quoted.
• git ls-files relative-path output is bounded to the repository tree. rel values piped into the copy loop come from git ls-files -o -i -z, which only emits paths relative to the repository toplevel — ../ traversal in rel is not possible through standard git behavior.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

[claude]

Claude finished @kyle-sexton's task in 4m 4s —— View job

---

PR Review

• Gather context from PR metadata and prior reviews
• Read changed files at current HEAD (b2f5377)
• Diff new commits since prior review (e7ba098 → b2f5377)
• Review worktree-create.sh for open findings
• Review worktree-create.test.sh for open findings
• Review plugin.json, SKILL.md, create.md changes
• Post comprehensive review feedback

---

What the latest commit (b2f5377) does

Purely cosmetic CI-hygiene: strips issue-number references from shell-script code comments (e.g., #399, #400, #531, #77566 + #78212 → prose descriptions) and sets the execute bit (100644 → 100755) on both scripts. No functional changes. Scrubbing only touches the two .sh files — plugin.json still carries #400 in its user-facing description string (intentional: that's consumer-visible guidance, not a code comment).

---

IMPORTANT — worktree.baseRef is a Claude Code setting, not a git config key

File: worktree-create.sh line 215

base_ref=$(git -C "$toplevel" config worktree.baseRef 2>/dev/null || true)

The Codex review correctly identifies this gap (P2): worktree.baseRef is a key in Claude Code's settings.json (settings#worktree-settings), not a standard git-config key. A user who has set worktree.baseRef: head in their Claude Code settings.json will have that preference honored by native EnterWorktree(name:) calls — but this helper reads git config instead, finds nothing, and silently defaults to fresh. Their unpushed commits will not carry in, with no indication why.

There are a few options:

1. Preferred (skill-side fix): The skill already has Claude's context (including worktree.baseRef from settings.json if exposed). Have the skill read the effective setting and pass --base-ref explicitly when calling the helper, rather than relying on the helper to discover it from git config. The helper's --base-ref flag already provides the right seam for this.
2. Document the gap: Add a note in create.md (and the worktree.baseRef usage-message hint) that the git-config path is the mechanism — not settings.json — so users who want head must run git config worktree.baseRef head in the repo.
3. Drop the git-config tier entirely in Phase A and document the precedence as --base-ref (explicit) or fresh (default), removing the misleading middle tier that reads a key users set elsewhere.

The current behavior creates a silent behavioral gap between native EnterWorktree(name:) and the helper path, which is the opposite of what this PR aims to achieve.

Fix suggestion →

---

IMPORTANT (carried from prior review) — .worktreeinclude intersection needs semantic verification

File: worktree-create.sh line 271

git -C "$toplevel" ls-files -o -i -z --exclude-from="$include_file"

git ls-files -o -i enumerates untracked files ignored by any exclude source — standard gitignore (.gitignore, info/exclude, core.excludesFile) PLUS the patterns from --exclude-from. The --exclude-from flag adds to the exclude set, it does not replace the standard sources (that requires --no-standard-excludes). Under this reading, the command yields all gitignored untracked files union any file matching .worktreeinclude patterns, making the subsequent check-ignore -q filter a no-op (everything returned is already gitignored) — and .work/task1/OTHER.md (gitignored but absent from .worktreeinclude) would be copied contrary to the test assertion at line 137.

That the test is reported green suggests one of: (a) git on the test runner happens to restrict to the --exclude-from patterns when no other exclude options are specified, (b) the test fixture setup prevents the standard-gitignore sources from loading, or (c) a version-specific behavior. This is version-sensitive and not portable.

The safe, intent-preserving fix is to add --no-standard-excludes or to restructure: enumerate .worktreeinclude patterns first, then post-filter with check-ignore -q to retain only those that are also gitignored. That second order matches the stated intent ("files that match a .worktreeinclude pattern AND are also gitignored") and is unambiguous across git versions.

This finding was raised in the prior review; it has not been addressed.

---

SUGGESTION (from security review) — --root should be validated as absolute

File: worktree-create.sh line 205

worktree_path="${root%/}/${dirname}"

A relative --root (e.g., ../../elsewhere) is accepted without error and is resolved against the shell's CWD at invocation time — which may not be the repository root when Claude Code calls the helper. The plugin description and refuse-guidance message both say "a path OUTSIDE every repository", implying absolute paths are the contract. Enforcing it explicitly removes the CWD-dependency:

[[ "$root" == /* ]] || { printf '%s: --root must be an absolute path: %s\n' "$PROG" "$root" >&2; exit 2; }

This is SUGGESTION severity because the directory userConfig type likely resolves to an absolute path in practice — but it is not guaranteed for direct invocations.

---

Minor — --root omitted exits 3 (refuse), not 2 (usage)

File: worktree-create.sh lines 110–127

When --root is omitted entirely (flag never passed), $root is "" (default at line 65) and falls into the refuse block (exit 3). The exit-code table documents "2 = usage error / 3 = refuse (root unconfigured)". Omitting a required flag is a usage error, not a configuration-refusal. A direct caller who forgets --root gets a refuse-with-guidance message that points them at plugin setup, when the correct response is "usage: --root is required". A simple guard after the --name check would fix this:

if [[ -z "$root" && "$original_root_flag_not_passed" ]]; then
  printf '%s: --root is required\n' "$PROG" >&2; exit 2
fi

(Implementation: set a flag when --root is seen in the loop, then check it.)

This was raised in the prior review; not addressed.

---

Minor — git config worktree.baseRef middle tier is untested

File: worktree-create.sh lines 214–215

The test suite covers --base-ref fresh, --base-ref head, and the uncached-origin/HEAD fallback. A repo with worktree.baseRef = head set in git config, invoked without --base-ref, is not tested. Noted in prior review; not addressed.

---

Minor — Azure SSH URL not tested

File: worktree-create.test.sh lines 156–165

git@ssh.dev.azure.com:v3/myorg/myproject/widget exercises the SCP path in parse_owner_repo (not the *://* HTTPS path). The v3 strip works through a different branch; no test covers it. Noted in prior review; not addressed.

---

Observation — cp -p follows symlinks (security review, still open)

File: worktree-create.sh lines 262–265

[[ -f "$src" ]] follows symlinks before testing, and cp -p "$src" copies the symlink target's contents. A repo symlink pointing at a file outside the repository (e.g., secrets.env -> /etc/system-config) that matches a .worktreeinclude pattern would have its target contents copied into the new worktree. This is bounded to what the repo author controls, not a remote-exploitation vector. Documenting this caveat in create.md or the .worktreeinclude description is the minimum fix. Not addressed since prior security review.

---

Positives

• Latest commit correctly scrubs internal issue numbers from code comments without touching user-facing strings in plugin.json (where #400 remains as actionable context for consumers).
• Setting the exec bit on both scripts via the mode change is the right fix for portability-lint failures.
• All functional correctness from the prior review's positive notes remains intact: need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited enumeration, output contract, and parse_owner_repo fallback chain.

---

Summary: The two open IMPORTANT findings — the worktree.baseRef git-config/settings.json mismatch (new, from Codex review) and the .worktreeinclude intersection semantics (carried from prior review) — should be resolved before merge. The minors and suggestions can follow or be tracked as issues.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2f5377af1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 4m 9s —— View job

---

PR Review

• Read changed files at HEAD (2ad0030)
• Diff new commits since last review (b2f5377 → 2ad0030)
• Verify status of all open findings from prior reviews
• Check .worktreeinclude intersection semantics (IMPORTANT, carried)
• Check worktree.baseRef git-config vs. settings.json mismatch (IMPORTANT)
• Check eval fixture alignment (Codex P2)
• Review remaining open minors/suggestions
• Post comprehensive review

---

What 2ad0030 does

One file changed: evals.json eval #1 (create-routing-enterworktree-last). The expected_output and expectations array now correctly assert the helper-based routing and EnterWorktree(path: ...) — fixing the Codex P2 finding from the prior review. All other files are unchanged from b2f5377.

---

IMPORTANT (carried) — .worktreeinclude intersection semantics still unresolved

File: worktree-create.sh line 271

git -C "$toplevel" ls-files -o -i -z --exclude-from="$include_file"

This was raised in both prior reviews and is the only IMPORTANT finding not addressed. The question is whether git ls-files -o -i --exclude-from=FILE restricts its output to patterns in FILE alone, or whether it emits all untracked files that match any exclude source (standard gitignore chain + FILE patterns).

If the standard gitignore chain is active (which man git ls-files implies it is unless --no-standard-excludes is passed), then git ls-files -o -i --exclude-from=.worktreeinclude yields all gitignored untracked files (.work/ catches everything under .work/). The subsequent check-ignore -q filter then passes for all of them — it also uses standard gitignore. The net effect is that every gitignored untracked file gets copied, not just those matched by .worktreeinclude. In the test fixture, .work/task1/OTHER.md is gitignored via .work/ but absent from .worktreeinclude — the test asserts it is absent from the worktree, but under this reading it would be present (test would fail).

That the 37 tests are reported green suggests git may be applying mode (a) — restricting to --exclude-from patterns when no other exclude sources are present in the fixture. But this behavior is version-sensitive and not guaranteed by the documentation.

The unresolved ask: either (a) confirm with --no-standard-excludes that the current approach is portable, or (b) invert the filter: enumerate .worktreeinclude patterns first (git ls-files -o -i -z --no-standard-excludes --exclude-from=.worktreeinclude), then post-filter with check-ignore -q to retain only those also gitignored. Option (b) is unambiguous across git versions and matches the stated intent.

The inline comment on line 252 still says "ls-files -o -i --exclude-from yields untracked files matching the include patterns" — that assertion needs proof or the implementation needs the guard.

Fix this →

---

IMPORTANT (carried) — worktree.baseRef settings.json gap not addressed

File: worktree-create.sh line 215 / context/create.md line 29

base_ref=$(git -C "$toplevel" config worktree.baseRef 2>/dev/null || true)

worktree.baseRef is a Claude Code settings.json worktree setting, not a git config key. Users who set worktree.baseRef: "head" in their Claude Code settings.json get that honored by native EnterWorktree(name:) calls — but not by this helper path, which reads git config instead and silently falls through to fresh. Users lose their unpushed commits in the new worktree without any indication why.

create.md line 29 adds to the confusion: "The shared helper resolves the base the same way Claude Code's worktree.baseRef setting does" — immediately qualified by "The helper reads worktree.baseRef from git config". These are different stores. The preamble claim is inaccurate.

The create.md helper invocation also doesn't pass --base-ref:

bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" \
  --name "<validated-name>" --root "${user_config.worktree_root}"

The cleanest options (first is preferred):

1. Add a worktree_base_ref userConfig key (mirroring worktree_root's pattern) and pass ${user_config.worktree_base_ref} to --base-ref in the skill invocation when set. This keeps parity with the babysit_worktree_root / worktree_root shape already established.
2. Drop the git-config tier and document --base-ref (explicit) or fresh (default) as the two-tier precedence. Remove the misleading "same way Claude Code does" claim.
3. Document the gap explicitly in create.md — tell users who want head to run git config worktree.baseRef head in the repo.

This finding was first surfaced by the Codex P2 review and carried through the second automated review. It is still unaddressed.

Fix this →

---

Addressed — Eval fixture (Codex P2) ✓

Eval #1 (create-routing-enterworktree-last) now correctly asserts the helper-based routing and EnterWorktree(path: ...), replacing the old EnterWorktree(name: ...) expectation. The expectations array is specific and testable. This finding is closed.

---

Minor — --root omitted exits 3 (refuse), not 2 (usage) [carried]

File: worktree-create.sh line 115

When --root is never passed, $root is "" (the initializer on line 64) and falls into the unconfigured-root refuse block (exit 3). The exit-code table documents "2 = usage error / 3 = refuse (root unconfigured)". Omitting a required flag is a usage error; a user calling the helper directly who simply forgot --root gets a refuse-with-guidance message pointing them at plugin setup rather than a usage hint. This breaks the semantic contract. A flag to track whether --root was seen in the arg loop and a guard after the --name check would fix it.

---

Minor — --root not validated as absolute path [carried]

File: worktree-create.sh line 205

A relative --root (e.g. ../../elsewhere) is accepted without error. The existence check on line 207 and git worktree add on line 243 resolve relative paths against different bases (shell CWD vs. $toplevel), so they can silently disagree — the "target already exists" guard can be bypassed and the worktree can land outside the intended root. The directory userConfig type likely supplies an absolute path, but the helper enforces no such constraint itself. A one-liner guard after the unconfigured-root check removes the CWD-dependency:

[[ "$root" == /* ]] || { printf '%s: --root must be an absolute path: %s\n' "$PROG" "$root" >&2; exit 2; }

---

Minor — git config worktree.baseRef middle tier untested [carried]

File: worktree-create.test.sh

The test suite exercises --base-ref fresh, --base-ref head, and the uncached-origin/HEAD fallback, but no case invokes the helper WITHOUT --base-ref against a repo that has git config worktree.baseRef head set. A two-line addition:

git -C "$repo" config worktree.baseRef head
out=$(bash "$HELPER" --name feat/cfg-base --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_file_exists "git config worktree.baseRef = head carries unpushed commit" "$out/UNPUSHED.md"

would close the gap for the tier the PR documents as supported.

---

Minor — Azure SSH URL (git@ssh.dev.azure.com:v3/…) not tested [carried]

File: worktree-create.test.sh lines 156–165

The v3 strip in parse_owner_repo exercises a different code path for SSH vs. HTTPS. The Azure HTTPS case is tested; the SSH SCP form git@ssh.dev.azure.com:v3/myorg/myproject/widget is not:

repo=$(mkrepo --origin "git@ssh.dev.azure.com:v3/myorg/myproject/widget")
out=$(bash "$HELPER" --name feat/az-ssh --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_eq "Azure SSH v3 URL -> project as owner" "$root/myproject-widget-feat-az-ssh" "$out"

---

Observation — cp -p follows symlinks to out-of-repo targets [carried]

File: worktree-create.sh lines 262–265

[[ -f "$src" ]] resolves symlinks before testing; cp -p "$src" copies the symlink target's contents. A repo symlink whose target is a file outside the repository (e.g. secrets.env -> /etc/some-system-config) that is gitignored and matches a .worktreeinclude pattern would have its target contents copied silently. Not a remote-exploitation vector — requires local control to set up such a symlink. At minimum, a sentence in create.md (or the .worktreeinclude usage notes) warning that matching symlinks copy their target's contents rather than the symlink itself would set user expectations. A [[ -L "$src" ]] && continue guard before the -f test would close the vector entirely.

---

Positives

• Eval fixture fixed (2ad0030): the new expectations array is specific to the helper/path routing and will catch a regression to the old EnterWorktree(name:) path.
• create.md line 5 correctly names the bug numbers and explains why EnterWorktree(name:) is forbidden — this is the right level of context for the model executing the skill.
• need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited enumeration, output contract, and parse_owner_repo fallback chain are all solid and unchanged.
• worktree_root userConfig key in plugin.json is properly typed as directory and the description encodes the refuse-not-fallback contract.

---

Summary: Two IMPORTANT findings remain open — the .worktreeinclude intersection semantics (needs --no-standard-excludes or an inverted filter to be version-portable) and the worktree.baseRef settings.json gap (users with that preference set get fresh silently; the preamble in create.md line 29 overpromises parity). The eval fixture finding is closed. The minors can follow or be tracked as issues, but the two IMPORTANT items should be addressed before merge.

[claude]

Claude finished @kyle-sexton's task in 2m 56s —— View job

---

Security Review

• Gather PR context and metadata
• Read PR diff and changed files
• Audit worktree-create.sh for injection, path traversal, symlink, and command execution risks
• Audit worktree-create.test.sh for test harness security issues
• Review plugin.json, SKILL.md, context/create.md, evals.json changes
• Compile and post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md, evals.json (latest HEAD: 2ad0030)

Overall posture: The latest commit (2ad0030) touches only evals.json — no new security surface introduced. All functional security analysis targets the helper script added and hardened in earlier commits. Variables are consistently quoted, the -- separator is used correctly against dash-leading filenames, NUL-delimited enumeration handles paths with special characters, name validation is strict, and URL-derived strings are joined with - (not /) preventing traversal above $root. No CRITICAL findings. The three issues below were identified in both prior automated security passes and remain unaddressed in this HEAD.

---

IMPORTANT — .worktreeinclude intersection may propagate unexpected secrets | confidence: PLAUSIBLE

worktree-create.sh line 271

git -C "$toplevel" ls-files -o -i -z --exclude-from="$include_file"

The --exclude-from flag adds patterns to the exclude set — it does not replace the standard gitignore chain (.gitignore, $GIT_DIR/info/exclude, core.excludesFile). With -i, git reports files excluded by any source, including the standard chain. That makes the output: all gitignored untracked files ∪ files matching .worktreeinclude patterns, not the intended files matching .worktreeinclude patterns only. The subsequent check-ignore -q filter is then a no-op (every file in the output is already gitignored), so gitignored-but-not-in-.worktreeinclude files — including .env, .aws/credentials, .npmrc tokens, and other secrets that live in the main checkout but are intentionally absent from .worktreeinclude — can be silently copied into every new worktree.

The worktree is placed at an external root outside the repo, potentially at a path with different access controls, shared with CI runners, or synced to cloud storage. Secret propagation to that path may not be what the user intended and is not visible until the copy has already occurred.

Why the test still passes: the behavior is git-version-sensitive. If the test environment's git implementation restricts -i output to --exclude-from patterns when no other standard excludes apply (a valid but not universally required behavior), the assert_file_absent for .work/task1/OTHER.md holds. On a git version that respects the full standard chain, the same assert would fail — the file would be copied.

Suggested fix: Either add --no-standard-excludes to restrict the output to only .worktreeinclude patterns (then check-ignore -q performs the gitignore intersection as intended), or invert the order: enumerate .worktreeinclude patterns first with --no-standard-excludes, then post-filter with check-ignore -q to keep only the gitignored ones. Adding a --no-standard-excludes test that passes on the same fixture would confirm the fix is version-stable.

Fix this →

---

SUGGESTION — --root accepts relative paths; existence check and git worktree add may resolve against different bases | confidence: PLAUSIBLE

worktree-create.sh lines 205–210

worktree_path="${root%/}/${dirname}"

if [[ -e "$worktree_path" ]]; then
  printf '%s: target path already exists: %s\n' "$PROG" "$worktree_path" >&2
  exit 4
fi

And at line 243:

git -C "$toplevel" worktree add -b "$name" "$worktree_path" "$base_commit"

$root is accepted from --root without a check for absolute-path form. A relative path (e.g. ../../outside-tree) causes two separate resolutions:

• [[ -e "$worktree_path" ]] resolves relative to the shell's CWD at invocation time
• git -C "$toplevel" worktree add … "$worktree_path" resolves relative to $toplevel (the git repository root)

When the helper is invoked from a subdirectory of the repository (CWD ≠ $toplevel), these two resolve to different filesystem paths. The "target already exists" guard checks one location; git creates the worktree at another. A race between two concurrent invocations with a relative --root could bypass the guard entirely.

The directory userConfig type in Claude Code's config UI likely supplies an absolute path in practice — but the helper itself enforces no such constraint. The Phase B hook adapter, direct CLI invocation, and any future consumer can pass a relative path without error.

Suggested fix:

if [[ "$root" != /* ]]; then
  printf '%s: --root must be an absolute path: %s\n' "$PROG" "$root" >&2
  exit 2
fi

Add this immediately after the unconfigured-root check (after line 127). This makes the absolute-path contract explicit and eliminates the CWD-dependency entirely.

Fix this →

---

SUGGESTION — cp -p follows symlinks; out-of-repo symlink targets are copied | confidence: PLAUSIBLE

worktree-create.sh lines 262–265

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). If a repository contains a tracked or untracked symlink (e.g. secrets.env -> /etc/some-system-config) that (a) is gitignored, and (b) matches a .worktreeinclude pattern, then cp -p "$src" "$dest" reads and copies the contents of the symlink target — an out-of-repo file — into the new worktree. The copy is silent; only the byte count in the "copied N .worktreeinclude file(s)" message reveals that something was transferred.

Exploitation requires local control (the attacker controls .worktreeinclude and the symlink), not a remote attack vector. However, a compromised dependency or dotfile that plants such a symlink could cause sensitive files to propagate to the external worktree root, particularly if that root has broader access or is synced.

Suggested fix:

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

Alternatively, document the behavior in create.md (under the .worktreeinclude files note) so users know what they are authorizing.

Fix this →

---

Notes (informational, not security findings)

• URL-derived owner/repo strings are path-traversal-safe. parse_owner_repo joins segments with - (not /), so crafted remote URLs cannot produce directory traversal above $root.
• git ls-files output is bounded to the repository tree. rel values from git ls-files -o -i -z are repository-relative and do not contain ../ components through standard git behavior.
• base_commit from symbolic-ref is injection-safe. The ref string is always passed as a quoted argument to git worktree add; shell injection is not possible. Pathological ref names cause git to error rather than execute attacker-controlled code.
• ..-segment names are not a path-traversal vector. The name regex permits .. segments, but the slug transformation converts / to -, making ../.. → ..-..-slug — a literal component inside $root, not above it. Git then rejects .. as an invalid branch name component (exit 4), producing a less-actionable error than exit 2 would.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

---

Third pass summary: All three findings were raised and confirmed in both prior automated security reviews (e7ba098 and b2f5377). None have been addressed in 2ad0030. The IMPORTANT finding (.worktreeinclude intersection) carries a meaningful secret-propagation risk that scales with any gitignored secrets the user has in their main checkout; it should be resolved before merge.

[kyle-sexton]

Automated babysit-lane classification (safe tier — reply only; not resolving threads). Re: Claude PR review (comment 5038417106, head b2f5377).

#	Finding (severity)	Classification	Evidence	Reacted
1	IMPORTANT — worktree.baseRef is a Claude setting, not a git-config key	VALID (defer — design decision)	Confirmed via the official worktrees doc (settings#worktree-settings): worktree.baseRef lives in settings.json ({"worktree":{"baseRef":"head"}}), not git config; helper reads git config (worktree-create.sh:214-215) and create.md:29 claims false parity. Real gap; three divergent fixes → author's call. Detail on the Codex inline thread at worktree-create.sh:215.	👍
2	IMPORTANT (carried) — .worktreeinclude intersection may over-copy (line 271)	INCORRECT (concrete claim); portability = SUGGESTION at most	Empirically on git 2.54.0.windows.1: git ls-files -o -i --exclude-from=.worktreeinclude returns only the pattern-matched file, not the union of all standard-gitignored files — a gitignored-but-unmatched file (.work/task1/OTHER.md) is never enumerated, so never copied. Test [20] ("gitignored-but-unmatched skipped") passes for exactly this case; the check-ignore -q filter (line 261) does the complementary half. So "OTHER.md gets copied contrary to the test" is false. The version-portability concern is unreproduced — at most a non-blocking SUGGESTION to pin behavior (e.g. --no-standard-excludes), not a live defect.	👍
3	SUGGESTION — --root should be validated as absolute (line 205)	Concern VALID (defer); suggested fix INCORRECT as written	The CWD-vs-$toplevel ambiguity is real (existence check line 207 resolves against shell CWD; git worktree add line 243 against $toplevel). BUT the suggested guard [[ "$root" == /* ]] rejects Windows drive-letter paths (C:/Users/...), which this repo explicitly supports (CLAUDE.md: "on the same drive as the repo on Windows") — it would break the primary platform. Any fix must accept both POSIX /... and Windows <drive>:/.... Deferred for a Windows-safe validation choice.	👍
4	Minor — omitted --root exits 3 (refuse) not 2 (usage)	VALID (defer — cosmetic exit-code)	Accurate: omitted --root leaves $root="" and hits the refuse block (exit 3). Low impact; refuse message still guides. Non-blocking polish.	👍
5	Minor — git config worktree.baseRef middle tier untested	VALID (defer)	Accurate test gap, but the tier's correct design is finding #1; a test should follow whatever resolution #1 takes rather than pin the current source.	👍
6	Minor — Azure SSH (v3/…) URL not tested	VALID (defer — minor test gap)	parse_owner_repo strips a leading v3 (line 163) and test [29] covers the Azure _git shape; the SSH v3/org/project/repo path lacks a dedicated case. Non-blocking.	👍
7	Observation — cp -p follows symlinks to out-of-repo targets (262-265)	VALID (defer — low severity, local-control only)	Accurate: [[ -f "$src" ]] (263) follows symlinks and cp -p (265) copies target contents. Requires local control of both a gitignored symlink and a matching .worktreeinclude entry; no remote vector. Fix is a [[ -L "$src" ]] && continue guard or a docs caveat — author's call.	👍

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ad0030a07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

Automated babysit-lane classification (safe tier — reply only; not resolving threads). Re: Claude security review (comment 5038416880, head b2f5377).

Note: this review's own text states it found no high-severity items at head — the higher-severity words in its body are that posture prose, not live findings. Two genuine suggestion-level items:

#	Finding	Classification	Evidence	Reacted
1	--root not validated as absolute (line 205)	Concern VALID (defer); suggested fix [[ "$root" == /* ]] INCORRECT for Windows	Same as PR-review finding #3: the CWD-vs-$toplevel ambiguity is real, but the /* guard rejects Windows drive-letter paths (C:/Users/...) this repo supports. A Windows-safe validation is needed if enforced.	👍
2	cp -p follows symlinks during .worktreeinclude copy (262-265)	VALID (defer — low severity, local-control only)	Same as PR-review observation #7: no remote vector; fix is a [[ -L "$src" ]] && continue guard or a docs caveat.	👍

[kyle-sexton]

Automated babysit-lane classification (safe tier — reply only; not resolving threads). Re: superseded prior-commit reviews — Claude PR review 5038353117 and Claude security review 5038353867, both reviewed at the earlier commit e7ba098.

Both were re-run at the current head b2f5377 (5038417106 and 5038416880); their genuine findings are the same items, now classified on those head-review comments above.

#	Source marker	Classification	Evidence
1	5038353117 — the .worktreeinclude-intersection item	Superseded → INCORRECT at head	Same finding as head PR-review #2; empirically disproven on git 2.54 (see that classification).
2	5038353867 — the two --root-absolute / cp -p-symlink items	VALID (defer) — duplicates carried to head	Classified unchanged on the head security-review comment above.
3	5038353867 — the two higher-severity words in its posture prose	INCORRECT — not findings	Those words are the "carefully written… Two suggestion-level issues found" overall-posture prose, not severity-labelled findings — no high-severity finding was ever raised.

[kyle-sexton]

Automated babysit-lane classification (safe tier — reply only; not resolving threads). Re: the re-reviews at head 2ad0030 — Claude PR review 5039659838 and security review 5039660202.

Eval fixture (Codex P2): confirmed CLOSED — the PR review marks it "Addressed — Eval fixture ✓" (fixed in 2ad0030).

The rest are the same carried items classified on the b2f5377 head-review comments above; classifications unchanged, with one decisive update on the .worktreeinclude finding:

#	Finding (severity)	Classification	Evidence	Reacted
1	IMPORTANT — .worktreeinclude intersection / secret-propagation (line 271)	INCORRECT — premise and suggested fix both disproven	git ls-files does not load the standard gitignore chain by default; it is opt-in via --exclude-standard, which this code never passes. So the only active exclude source is --exclude-from=.worktreeinclude, and -i matches only those patterns. Verified empirically on git 2.54.0 under BOTH a directory-level (.work/) and a file-level (sub/*.md, .env) gitignore: ls-files -o -i --exclude-from=.worktreeinclude returns ONLY the matched file — sub/OTHER.md and .env are never emitted, so never copied. The check-ignore -q filter is therefore meaningful (it drops .worktreeinclude-matched-but-not-gitignored files), not a no-op. No union, no secret leak. Separately, the suggested --no-standard-excludes is not a valid git flag (error: unknown option 'no-standard-excludes') — applying it would break the script; the real opt-in is --exclude-standard, which must NOT be added here. No code change warranted.	👍
2	IMPORTANT — worktree.baseRef settings.json gap (line 215 / create.md:29)	VALID (defer — design decision, top priority for author)	Confirmed genuine (settings.json key, not git config). The review's option 1 (add a worktree_base_ref userConfig and pass --base-ref) is the cleanest path but is a feature addition (plugin.json userConfig + skill wiring + test), not a mechanical fix — author's call among the three options, and it also corrects the misleading create.md:29 parity claim. Detail on the Codex inline thread at worktree-create.sh:215 and the b2f5377 PR-review classification above.	👍
3	Minors + cp -p (—root exit code, —root absolute, baseRef tier untested, Azure SSH untested, cp -p symlinks)	VALID (defer); suggested --root fix INCORRECT for Windows	Unchanged from the b2f5377 head-review classifications above. Reminder: the [[ "$root" == /* ]] guard both reviews suggest rejects Windows drive-letter paths (C:/...) this repo supports.	👍

Net: one IMPORTANT (worktree.baseRef) is a genuine deferred design decision for the author; the other IMPORTANT (.worktreeinclude) is INCORRECT with its suggested fix being an invalid flag; minors deferred.

[kyle-sexton]

This was generated by AI (decide lane — decision-brief pass; parked PR-thread decisions surfaced via periodic scan).

Two parked design decisions in this PR's Codex review threads have no ratified resolution. Briefing both. Base-ref facts below are verified against the primary source (Claude Code worktrees doc, code.claude.com/docs/en/worktrees, "Choose the base branch"), not the author's own thread replies.

---

Decision 1 — helper reads worktree.baseRef from the wrong store (real bug)

Thread: discussion_r3625261664 (Codex P2 + author defer).

Problem. The helper resolves the base ref via git -C "$toplevel" config worktree.baseRef (worktree-create.sh:215), but worktree.baseRef is a Claude Code settings.json key, not a git-config key — the worktrees doc sets it as {"worktree":{"baseRef":"head"}} under settings, and git config never returns it. A user who set head in settings.json silently gets the fresh path and loses their unpushed/feature-branch commits.

Today. context/create.md:29 makes this worse: it claims parity ("resolves the base the same way Claude Code's worktree.baseRef setting does") and states "The helper reads worktree.baseRef from git config" — a false claim, since the setting lives in settings.json. So the read is dead (virtually no one sets that key in git config) and the parity claim is untrue. The fix must either make the read actually work or drop the false claim; a doc tweak alone that keeps a dead read is a half-measure.

Options.

• (a) — RECOMMENDED — caller resolves the effective setting and passes --base-ref; helper drops the git-config probe. Base-ref belongs to the caller, which knows the effective Claude setting; the helper stays a pure flag-driven seam (matching the PR's own "flag CLI is the stable seam" framing and the Phase B hook-adapter boundary). This is the only option that delivers the promised parity instead of quietly removing it, and it removes both the wrong-store read and the false claim in one move. Cost: the skill/adapter must resolve the merged settings cascade (user/project/local) to read the effective value — non-trivial but bounded, and it is the honest implementation of the stated contract. Fully reversible (additive; can fall back to (c) later).
• (b) — keep git-config as the mechanism, drop only the parity claim in create.md:29. Cheapest (doc-only), removes the false statement, but leaves a vestigial git-config read that no real user populates — dead code presenting as a feature. Reversible but doesn't fix the bug, only stops advertising it.
• (c) — drop the middle tier entirely. Helper is always fresh unless --base-ref is passed explicitly; delete the git-config probe and the parity claim. Simplest and honest, but removes settings-driven head support (a real capability native offers). Reversible-ish (re-adding is (a)).

---

Decision 2 — fresh probes only origin/HEAD (non-origin-default clones) — DEFAULTED-UNRATIFIED

Thread: discussion_r3626146017 (Codex P2 + author defer).

Problem. --base-ref fresh probes only refs/remotes/origin/HEAD (worktree-create.sh:230); a repo cloned with a non-origin default remote (git clone -o upstream) can have upstream/HEAD cached but not origin/HEAD, so fresh falls back to local HEAD and carries unpushed commits into a supposedly fresh worktree. A default (origin-only) was taken but never ratified.

Today. On an origin/HEAD miss the helper falls back to local HEAD with a loud stderr warning + remediation (git remote set-head origin --auto) (worktree-create.sh:234); the comment at 224–229 states the native-parity intent explicitly.

Verified against primary source: native fresh also keys specifically on origin/HEAD and "falls back to your current local HEAD" when it isn't cached (worktrees doc, "Choose the base branch"). So the current origin-only behavior is faithful native parity — the author's framing is correct, not merely asserted. The Codex suggestion to "reuse the repo's remote resolver" is also inaccurate: no such resolver exists in the source-control scripts (checked babysit-readiness-gate.sh, fetch-all-pr-comments.sh, test-helpers.sh) — "broaden" would be build-new, not reuse.

Options.

• (a) — RECOMMENDED — keep the native-parity default (origin/HEAD only); it already warns. Broadening would diverge from the documented "same way Claude Code does" contract (create.md:29) for an edge case (clone -o upstream) that native itself doesn't handle, and the existing loud warning + remediation means the failure is visible, not silent. Fully reversible — keeping the narrow default preserves the option to broaden later; broadening now is harder to walk back once users depend on the wider resolution. Lowest risk, honest to the stated contract.
• (b) — broaden to a resolver (branch-remote → origin → sole-other). Handles non-origin-default clones, but is net-new code (no resolver to reuse), diverges from documented native parity, and adds a resolution surface to test/maintain. Reversible but a genuine behavior/contract change.
• (c) — keep origin-only, add a one-line note in create.md documenting the non-origin-default-clone limitation alongside the runtime warning. Cheapest honesty improvement; pairs with (a).

Related, separate (not owned by this thread — flagging for honesty since Decision 2's recommendation rests on the parity claim): native fresh also fetches to keep origin/HEAD current (24h staleness, 5s cap) before resolving; this helper reads only the cached ref and does not fetch. That is a distinct parity gap, out of scope here — worth a follow-up issue if full fresh parity is the goal.

---

Cross-links

• feat(source-control): unify native-hook + /worktree-skill worktree creation via one shared helper #399 — the "unify creation behind one shared helper / native parity" goal both decisions serve; this PR Closes it.
• feat(source-control): machine-agnostic worktree setup skill (detect -> decide -> install) #398 — worktree-convention epic (2026-07-21 operator ruling this PR implements); owns detect→decide→install and the Phase B hook.
• fix(source-control): /worktree creates in-repo .claude/worktrees/ -> hits CC double-load bug; create out-of-tree #400 — the out-of-tree placement / CLAUDE.md double-load bug; sibling concern (placement, not base-ref), stays open and blocked-by this work.

[claude]

Claude finished @kyle-sexton's task in 2m 37s —— View job

---

PR Review

• Read new commit(s) since last review (2ad0030 → aa0f5e8)
• Verify worktree.baseRef fix (IMPORTANT, addressed in aa0f5e8)
• Re-check all open findings from prior reviews
• Review changed files at HEAD
• Post comprehensive review

---

What aa0f5e8 does

Two files changed:

worktree-create.sh — removes the dead git config worktree.baseRef probe. The 5-line precedence-chain block (explicit --base-ref → git-config → fresh default) collapses to a single line: [[ -z "$base_ref" ]] && base_ref="fresh". The --help text is updated to correctly describe --base-ref as the flag the caller uses to pass the effective Claude settings.json setting, with fresh as the unconditional default when omitted.

context/create.md — replaces the contradictory preamble ("resolves the base the same way Claude Code's worktree.baseRef setting does" / "The helper reads worktree.baseRef from git config") with accurate, actionable instruction: worktree.baseRef is a settings.json key (not git config); the skill must read the effective value (project .claude/settings.json wins over user ~/.claude/settings.json); if it is head, pass --base-ref head; otherwise omit it. The helper invocation note is updated to make --base-ref head conditional on that read.

This is exactly option (a) from the decision brief: the helper stays a pure flag-driven seam; the caller owns the base-ref policy.

---

IMPORTANT (prior) — worktree.baseRef settings.json gap ✓ CLOSED

File: worktree-create.sh line 217 / context/create.md line 29–31

The dead git-config probe is removed and the false parity claim in create.md is corrected. The new "The caller owns this choice" block is specific enough to implement correctly — it names the right config path, the right precedence rule, and the right flag value to pass. This finding is fully resolved.

---

IMPORTANT (prior) — .worktreeinclude intersection semantics ✓ CLASSIFIED INCORRECT

Per the author's babysit-lane classification at 2026-07-21T22:41:14Z: git ls-files -o -i does not load the standard gitignore chain by default (that requires --exclude-standard, which this code never passes). With only --exclude-from=.worktreeinclude active, -i restricts output to patterns in that file — making the subsequent check-ignore -q filter a meaningful gitignore intersection, not a no-op. Empirically confirmed on git 2.54. The suggested --no-standard-excludes is not a valid git flag. No code change warranted; the logic is correct as written.

---

Minor (prior) — git-config worktree.baseRef middle tier is now moot

The middle tier is gone (aa0f5e8). The open item ("add a test for git config worktree.baseRef head") closes automatically — there's no longer a code path to test.

---

Minor (carried) — --root omitted exits 3 (refuse), not 2 (usage)

File: worktree-create.sh line 116

When --root is never passed, $root is "" (the initializer at line 66) and falls into the unconfigured-root refuse block (exit 3). The exit-code table documents "2 = usage error / 3 = refuse (root unconfigured)". A user calling the helper directly who forgot --root gets a refuse-with-guidance message pointing them at plugin setup rather than a usage hint. Non-blocking; the guidance message is still actionable. Deferred.

---

Minor (carried) — --root not validated as absolute path

File: worktree-create.sh line 206

The CWD-vs-$toplevel ambiguity is real: [[ -e "$worktree_path" ]] (line 208) resolves relative to the shell's CWD; git -C "$toplevel" worktree add (line 243) resolves relative to $toplevel. A relative --root can silently bypass the "target already exists" guard. Note that the suggested [[ "$root" == /* ]] guard breaks Windows drive-letter paths (C:/Users/…) — this repo explicitly supports Windows (plugin.json description says "on the same drive as the repo on Windows"). Any enforced guard needs a Windows-safe form (e.g. [[ "$root" == /* || "$root" =~ ^[A-Za-z]:/ ]]). Deferred; directory userConfig type likely supplies absolute paths in practice.

---

Minor (carried) — Azure SSH URL (git@ssh.dev.azure.com:v3/…) not tested

File: worktree-create.test.sh line 157–165

The v3 strip in parse_owner_repo exercises the SCP branch (*:*), separate from the HTTPS *://* branch. The Azure HTTPS case is tested; the SSH SCP form is not. A one-case addition would close it:

repo=$(mkrepo --origin "git@ssh.dev.azure.com:v3/myorg/myproject/widget")
out=$(bash "$HELPER" --name feat/az-ssh --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_eq "Azure SSH v3 URL -> project as owner" "$root/myproject-widget-feat-az-ssh" "$out"

Deferred; non-blocking.

---

Observation (carried) — cp -p follows symlinks to out-of-repo targets

File: worktree-create.sh lines 262–265

[[ -f "$src" ]] follows symlinks before testing; cp -p copies the symlink target's contents. A repo symlink whose target is outside the repository (e.g. secrets.env -> /etc/some-system-config) that is gitignored and matches a .worktreeinclude pattern would have its target contents silently copied. Requires local control — no remote-exploitation vector. A [[ -L "$src" ]] && continue guard before the -f test, or a one-sentence note in create.md, would close this. Deferred.

---

Positives

• worktree.baseRef fix is the right approach. Removing the git-config probe keeps the helper a pure flag-driven seam (matching the "flag CLI is the stable seam" framing); shifting the policy read to the caller is honest about what each layer owns.
• create.md "The caller owns this choice" block is specific, actionable, and names both the right config path and the right precedence rule — a model executing this skill can implement it correctly.
• --help text update is consistent with the implementation change: it no longer implies a git-config middle tier.
• All prior positives carry: need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited enumeration, output contract, parse_owner_repo fallback chain, and eval fixture alignment.

---

Summary: Both IMPORTANT findings from the prior review cycle are now resolved. The worktree.baseRef gap is closed in aa0f5e8; the .worktreeinclude intersection finding is correctly classified as incorrect (the logic was right all along). Remaining open items are all deferred minors or observations — none block merge.

[claude]

Claude finished @kyle-sexton's task in 2m 42s —— View job

---

Security Review

• Get PR diff
• Read changed files at HEAD (aa0f5e8)
• Audit worktree-create.sh for injection, path traversal, symlink, and command execution risks
• Audit worktree-create.test.sh for test harness security issues
• Review plugin.json, SKILL.md, context/create.md, evals.json changes
• Compile and post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md, evals.json (HEAD: aa0f5e8)

Overall posture: The helper is carefully written. Variables are consistently quoted throughout, -- separators guard git calls against dash-leading filenames, NUL-delimited enumeration handles paths with special characters, the need_value guard prevents the trailing-flag hang, name validation is strict, and URL-derived owner/repo strings are joined with - (not /) preventing traversal above $root. The latest commit (aa0f5e8) removes the dead git config worktree.baseRef probe and moves base-ref policy correctly to the caller — closing the IMPORTANT finding from prior reviews. No CRITICAL findings. Two SUGGESTION-level issues remain unaddressed from the prior two security passes.

---

SUGGESTION — --root accepts relative paths; existence check and git worktree add may resolve against different bases | confidence: PLAUSIBLE

worktree-create.sh lines 206–211

worktree_path="${root%/}/${dirname}"

if [[ -e "$worktree_path" ]]; then
  printf '%s: target path already exists: %s\n' "$PROG" "$worktree_path" >&2
  exit 4
fi

And at line 243:

git -C "$toplevel" worktree add -b "$name" "$worktree_path" "$base_commit"

$root is accepted from --root without being checked for absolute-path form. A relative path (e.g. ../../outside) causes two separate resolutions:

• [[ -e "$worktree_path" ]] resolves relative to the shell's CWD at invocation time
• git -C "$toplevel" worktree add … "$worktree_path" resolves relative to $toplevel (the git repository root)

When the helper is called from a subdirectory (CWD ≠ $toplevel), these resolve to different locations — the "target already exists" guard checks one path; git creates the worktree at another. The directory userConfig type likely supplies an absolute path from the UI, but the helper enforces no such constraint for the Phase B hook adapter or direct CLI callers.

Note on the prior suggested fix: the guard [[ "$root" == /* ]] rejects Windows drive-letter paths (C:/Users/…) which this repo explicitly targets. A cross-platform guard would need to accept both POSIX (/…) and Windows ([A-Za-z]:/…) absolute forms.

Fix this →

---

SUGGESTION — cp -p follows symlinks; out-of-repo symlink targets are copied into the new worktree | confidence: PLAUSIBLE

worktree-create.sh lines 262–265

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). If a repository contains a symlink — tracked or untracked — whose target is a regular file outside the repo (e.g. secrets.env -> /etc/some-system-config), and that symlink (a) is gitignored, and (b) matches a .worktreeinclude pattern, then cp -p "$src" reads and copies the contents of the symlink target into the new worktree. No shell injection is possible; the risk is unintended propagation of out-of-repo content to the external worktree root, which may have different access controls or be synced to cloud storage.

Exploitation requires local control: the attacker must control both the symlink in the working tree and a matching .worktreeinclude entry. No remote-exploitation vector exists. A one-line guard eliminates the vector:

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

Alternatively, documenting the behavior in create.md (under the .worktreeinclude files note) would set user expectations without a code change.

Fix this →

---

Resolved — worktree.baseRef settings.json gap ✓

The git config probe (git -C "$toplevel" config worktree.baseRef) has been removed in aa0f5e8. The helper now defaults to fresh when --base-ref is omitted, and create.md lines 28–31 now correctly instruct the skill to read the effective worktree.baseRef from Claude Code's settings.json cascade (project wins over user global) and pass it as --base-ref head when set. The misleading "resolves the base the same way Claude Code does" claim is gone; the current framing is accurate. This IMPORTANT finding from prior reviews is closed.

---

Notes (informational, not security findings)

• .worktreeinclude intersection semantics: git ls-files -o -i --exclude-from=FILE does not load the standard gitignore chain by default (--exclude-standard would be needed for that, and is never passed here). Only .worktreeinclude patterns are active; the subsequent check-ignore -q filter correctly computes the intersection with gitignore. The prior IMPORTANT finding on this point was incorrect. The flag --no-standard-excludes suggested in prior reviews is not a valid git option.
• URL-derived owner/repo strings are path-traversal-safe. parse_owner_repo joins path segments with - (not /), so a crafted remote URL (git@host:../../etc/passwd.git) produces etc-passwd-<slug> inside $root, never a traversal above it.
• git ls-files output is bounded to the repository tree. rel values piped into the copy loop come from git ls-files -o -i -z, which emits only repository-relative paths — ../ traversal is not possible through standard git behavior.
• ..-segment names are blocked by git, not a traversal vector. The name regex permits segments like .. (. is in the charset), but the slug transformation converts / to -, placing the result inside $root as a literal filename component, not a directory traversal. Git's own check-ref-format then rejects .. as an invalid branch component (exit 4); no path traversal is possible.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

---

Summary: aa0f5e8 closes the previous IMPORTANT (worktree.baseRef wrong-store read). Two SUGGESTION-level items carried from prior passes — relative --root accepted without validation, and cp -p following symlinks during the .worktreeinclude copy — remain unaddressed. Neither is a remote-exploitation vector; both are bounded to local-control scenarios.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa0f5e8f6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69e1c53fe1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 4m 31s —— View job

---

PR Review

• Read changed files at current HEAD (1ff6d69)
• Diff new commits since last automated review (ac00bf4 → HEAD)
• Verify status of all open/carried findings
• Review worktree-create.sh and worktree-create.test.sh
• Review plugin.json, SKILL.md, context/create.md, evals.json
• Post comprehensive review

---

What the latest commits do

Three commits since the last full automated review pass (81627ac):

f723710 adds normalize_path — a pure-string lexical collapse of ./../redundant slashes in an absolute path, called on worktree_path after the relative-root anchoring step. This closes a bypass where a root like /tmp/nonexistent/../<repo>/.claude/worktrees would stop the ancestor walk at the nonexistent nonexistent segment, never probe the real <repo> ancestor, and let git worktree add create the directory, resolve .. via the OS, and land the checkout inside the repo. Test case: '..' after-nonexistent in-repo root refuses exit 3.

ac00bf4 adds Windows backslash canonicalization — swaps \ → / in $root before the anchor + normalize + walk pipeline, gated to $OSTYPE == msys || cygwin. Without this, an all-backslash path's ${probe%/*} never changes (pattern splits on /), so the loop breaks on iteration 1 with parent == probe, the containment block is skipped entirely, and git worktree add creates the checkout inside the repo. Uses intermediate variables (bslash/fwd) to make bash parameter substitution work with literal backslashes — the right approach. Windows tests are correctly gated and skip off-Windows (where \ is a legal filename byte).

1ff6d69 merges origin/main. Two new plugin.json userConfig keys land from main (babysit_extra_dependency_manager_logins, branch_issue_pattern); the worktree_root key is intact at line 152. No changes to worktree-create.sh, worktree-create.test.sh, SKILL.md, context/create.md, or evals.json.

---

normalize_path implementation — verified correct

The function handles all four reachable cases:

Input form	root	Behaviour
Absolute POSIX /…	"/"	correct
Windows drive C:/…	"C:/"	correct
Relative (anchored before call)	""	safe — anchoring ensures this only happens for C:foo (drive-relative, documented OOS)
.. clamped at root (/a/../..)	"/"	pops a, no-op second pop → /

The empty-array printf '%s%s' "$root" "${out[*]}" at the POSIX root is correct: "${out[*]}" with IFS='/' and an empty array expands to "", so the result is /. A worktree_path that normalizes to / would then have the ancestor walk probe / (a non-git directory on any real system), produce no location, and fall through to git worktree add which would fail to create //owner-repo-slug with an environment error (exit 4). Acceptable behaviour for a completely degenerate input.

The local IFS='/' scoping before printf is correct — it only affects "${out[*]}" expansion within that function scope.

---

Addressed — prior SUGGESTION: --root relative-path CWD ambiguity ✓

The b9ddad9 + f723710 combo fully closes this: relative roots are anchored to $toplevel before normalize_path and the containment walk, so both the existence check and git worktree add resolve against the same base. The ..-after-nonexistent variant is also closed.

---

Addressed — Windows backslash bypass ✓ (ac00bf4)

The canonicalization is correctly scoped to $root only (not $name or $repo_dir), correctly gated to Windows shells, and correctly applied before anchor + normalize + walk, so all three subsequent containment layers see a forward-slash path. The non-regression test (external backslash root still creates exit 0) confirms the guard doesn't over-reject legitimate external Windows paths.

---

Addressed — merge-from-main is clean ✓

The conflict was in plugin.json and CHANGELOG.md only. The worktree_root key (line 152) is correctly preserved between the pre-existing babysit_worktree_root and worktree_stale_days entries. The two new keys from main (babysit_extra_dependency_manager_logins, branch_issue_pattern) are clean additions with no structural conflict.

---

All IMPORTANT findings from the full review cycle: resolved ✓

Finding	Commit
.worktreeinclude intersection semantics	Correctly classified INCORRECT — git ls-files -o -i without --exclude-standard does not load the standard gitignore chain; the implementation is correct
worktree.baseRef reads from wrong store	aa0f5e8 — dead git-config probe removed; caller owns policy via --base-ref
Local settings.local.json override omitted	7ce20d7 — three-tier precedence documented in create.md
Relative-root CWD-vs-$toplevel ambiguity	b9ddad9 + f723710
In-repo placement bypass (source repo)	69e1c53
In-repo placement bypass (any checkout)	96826cf
Placement in .git dirs / bare clones	81627ac
..-after-nonexistent bypass	f723710
Windows backslash bypass	ac00bf4

---

Deferred items (all carried from prior review cycles, tracked in #657)

Finding	Status
--root omitted exits 3 (refuse), not 2 (usage)	Deferred — guidance still actionable; cosmetic exit-code distinction
Azure SSH git@ssh.dev.azure.com:v3/… URL not tested	Deferred — gap on an already-working code path; no defect
cp -p follows symlinks to out-of-repo targets (lines 322–325)	Deferred (SUGGESTION, local-control only, no remote vector). Fix: [[ -L "$src" ]] && continue before the -f test, or a note in create.md. Tracked in #657
Refuse message references nonexistent "worktree setup skill"	Deferred — /plugin configure is offered alongside and works; tracked in #657
git check-ref-format --branch not run after regex	Deferred — names like feat/foo..bar pass the char-class regex but git rejects them (exit 4, not 2); tracked in #657
Drive-relative C:foo root (no / after :)	Out-of-scope — cygpath CWD dependency makes a clean fix non-trivial; documented OOS in ac00bf4 comment

---

Positives

• normalize_path design is correct: pure-string lexical collapse means it resolves .. even through nonexistent components (where realpath -m would be needed but is GNU-only, and realpath -- / readlink -f require existing prefixes). The no-op guarantee for ordinary paths (no ./..///) is accurate — re-split and re-join with the same separator is idempotent.
• Windows backslash handling is correctly scoped, gated, and sequenced: swap → anchor → normalize → walk is the right order. Using intermediate bslash/fwd variables avoids bash backslash-quoting pitfalls in parameter substitution.
• Test suite is now at 60 pass / 0 fail (per the ac00bf4 commit message). Every containment bypass that was filed and fixed has a matching regression test.
• Merge conflict resolution on plugin.json is clean — the PR's worktree_root entry survived intact.
• All prior positives carry: need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited .worktreeinclude enumeration, output contract, parse_owner_repo fallback chain, eval fixture alignment, and the worktree.baseRef settings.json fix.

---

Summary: The two commits since the last automated review (f723710, ac00bf4) are both correctly implemented and close the last known containment-guard bypass paths. The merge from main is clean. No IMPORTANT findings remain open. The deferred minors and SUGGESTION-level items are all non-blocking and tracked in #657. This PR is ready to merge.

[claude]

Claude finished @kyle-sexton's task in 3m 12s —— View job

---

Security Review

• Get PR diff and identify changed files
• Read worktree-create.sh (primary security surface)
• Read worktree-create.test.sh (test harness)
• Read plugin.json, SKILL.md, context/create.md changes
• Audit for injection, path traversal, symlink, and command execution risks
• Post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md, evals.json at HEAD 1ff6d69

New commits since last security review (7ce20d7): 81627ac (reject .git-dir ancestors), f723710 (normalize .. before the walk), ac00bf4 (canonicalize Windows backslash roots), 1ff6d69 (rebase merge). Full security analysis covers all PR commits.

Overall posture: The helper is carefully written. Variables are consistently double-quoted throughout; -- separators guard git calls against dash-leading filenames; NUL-delimited enumeration handles paths with special characters; name validation is strict; URL-derived path components are joined with - (not /), preventing traversal above $root; the latest commits add correct layered defenses — path normalization, .git-dir detection via --is-inside-git-dir, and Windows backslash canonicalization — all implemented using git plumbing commands rather than string-prefix heuristics. No CRITICAL or IMPORTANT findings. One SUGGESTION-level issue carried from prior passes remains open.

---

Resolved since last review ✓

81627ac — git-directory / bare-clone ancestor check: When --show-toplevel fails (no working tree), the new elif branch calls --is-inside-git-dir. The output comparison [[ "$(…)" == "true" ]] uses a quoted right-hand side in [[…]], so no glob expansion is possible. The $detail assignment and heredoc expansion are safe — bash heredocs do not re-evaluate $() inside expanded variables; the git-dir path is printed literally, never executed. ✓

f723710 — normalize_path before the containment walk: The function uses only bash builtins (IFS, read, array indexing). No external commands, no eval, no command substitution. The root-clamping (.. at root is a no-op) and Windows drive-letter detection (^[A-Za-z]:/) are correct. A re-split of a path with no ./../// segment produces the identical string. ✓

ac00bf4 — backslash canonicalization: root="${root//"$bslash"/"$fwd"}" is pure bash string substitution with no command execution. The $OSTYPE == msys || cygwin gate keeps the transform off POSIX systems where \ is a valid filename byte. ✓

---

SUGGESTION (carried) — cp -p follows symlinks; out-of-repo symlink targets copied into the new worktree | confidence: PLAUSIBLE

worktree-create.sh lines 387–390

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). A repository symlink — tracked or untracked — whose target is a regular file outside the repo (e.g. secrets.env -> /etc/system-config) that (a) is gitignored and (b) matches a .worktreeinclude pattern would have its target's contents silently copied into the new worktree at the external root, which may have different access controls or be synced to cloud storage.

Exploitation requires local control: an attacker must control both the symlink in the working tree and a matching .worktreeinclude entry. No remote-exploitation vector. The risk is unintended propagation of out-of-repo content across the tool's core placement boundary.

This finding has been raised in all prior automated security passes and remains open at HEAD.

Suggested fix (one guard before the -f test eliminates the vector entirely):

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

Alternatively, a one-sentence note in create.md under the .worktreeinclude section would set user expectations without a code change.

Fix this →

---

Notes (informational, not security findings)

• $detail in the refuse heredoc is injection-safe. The detail variable is set via detail=" git dir: $(git -C "$probe" rev-parse --absolute-git-dir 2>/dev/null)" — the $(…) runs at assignment time. The heredoc later expands $detail by simple variable substitution; bash does not re-evaluate $() inside an expanded variable in a heredoc. A git-dir path containing $(something) would be printed literally, never executed.
• --is-inside-git-dir output comparison is exact-match safe. [[ "$(…)" == "true" ]] uses a double-quoted "true" on the right-hand side, which forces string equality (not glob matching). Git outputs only "true", "false", or nothing on error; the last case evaluates "" == "true" → false, so errors fail-closed.
• normalize_path clamping is correct at the filesystem root. A .. attempt at / or C:/ leaves the out array empty and the path stays at the root, with no pop error. A "C:/a/../../b" normalizes to "C:/b" — clamped at the drive root, not an infinite or invalid path.
• URL-derived path components are traversal-safe. parse_owner_repo joins segments with - (not /), so a crafted remote URL (git@host:../../etc/passwd.git) produces etc-passwd-<slug> inside $root — a literal filename component, never a traversal above it. The URL parsing uses only array operations; no eval or external command execution.
• git ls-files output is bounded to the repository tree. rel values from git ls-files -o -i -z are repository-relative; standard git does not emit ../ traversal paths for untracked files. The NUL-delimiter and read -d '' correctly handle filenames containing spaces, newlines, or other special characters.
• .worktreeinclude intersection semantics are correct. git ls-files -o -i without --exclude-standard does NOT load the standard gitignore chain; only --exclude-from=.worktreeinclude patterns are active. The subsequent check-ignore -q filter computes the genuine gitignore intersection. The earlier IMPORTANT finding on this point was incorrect; the suggested --no-standard-excludes flag does not exist in git.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

---

Summary: No CRITICAL or IMPORTANT findings at HEAD 1ff6d69. The three commits added since the prior security pass — .git-dir detection, ..-path normalization, and Windows backslash canonicalization — are all correctly implemented and close real fail-opens in the helper's containment guard. The cp -p symlink issue (SUGGESTION, local-control only, carried across eight prior passes) is the sole remaining open security item.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ff6d696be

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

Re: Anchor colon-bearing relative roots (create.md drive-relative C:foo) — confirmed a genuine but distinct, pre-existing fail-open. Different grammar class from this PR's backslash-separator fix (the \→/ swap is a verified no-op on separatorless C:foo). Tracked as #962 (area:security, with repro + fix direction) and deferred under the #650 model: it fails only on drive-relative-with-no-separator input, which no realistic worktree_root (an absolute directory from a picker or user) produces — not a default-path bypass. Resolving here against #962.

[kyle-sexton]

Re: Quote unset userConfig placeholders safely (create.md:70) — fixed in 9f93c77: single-quoted --root '${user_config.worktree_root}' so an unset key reaches the helper as an inert literal and triggers the documented exit-3 refusal + guidance, instead of a Bash bad substitution on the dotted name. Mirrors the #950 fix at the parse-branch-issue call site.

[claude]

Claude finished @kyle-sexton's task in 1m 48s —— View job

---

PR Review

• Read new commit (9f93c77) diff and changed files
• Verify single-quote fix for unset userConfig placeholder (latest Codex P2)
• Re-check all open findings from prior reviews
• Post comprehensive review

---

What 9f93c77 does

One file changed: context/create.md, one character changed.

Before:

bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" \
  --name "<validated-name>" --root "${user_config.worktree_root}"

After:

bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" \
  --name "<validated-name>" --root '${user_config.worktree_root}'

${user_config.worktree_root} contains a dot, which makes it an invalid bash variable name. Double-quoting caused bash to attempt expansion and fail with bad substitution on the first-run path — before the helper ever ran, meaning the documented exit-3 refusal + guidance was never seen. Single-quoting passes the literal token to the helper, which detects the unexpanded ${user_config prefix via the existing guard at worktree-create.sh:116 and exits 3 with guidance. The # shellcheck disable=SC2016 at line 115 confirms the literal-match is intentional.

---

Codex P2 (create.md:70) — single-quote fix ✓ CLOSED

The fix is correct, minimal, and precisely targeted. The interaction chain is clean:

1. worktree_root unset → Claude Code leaves the literal ${user_config.worktree_root} string
2. Single-quoted in create.md → reaches the helper as-is, no bash expansion
3. Helper line 116: [[ "$root" == *'${user_config'* ]] → matches → exit 3 + guidance
4. Skill line 75: "surface that guidance to the user verbatim — then stop"

The fix mirrors the same pattern already applied at the parse-branch-issue call site per the commit message, so it's consistent with the repo's established approach to this class of placeholder.

---

All IMPORTANT/P2 findings from the full review cycle: resolved ✓

Finding	Commit
.worktreeinclude intersection semantics	CORRECTLY CLASSIFIED INCORRECT — git ls-files -o -i without --exclude-standard does not load the standard gitignore chain; the implementation is correct; the suggested --no-standard-excludes flag does not exist in git
worktree.baseRef reads from wrong store	aa0f5e8 — dead git-config probe removed; caller owns policy via --base-ref
Local settings.local.json override omitted	7ce20d7 — three-tier precedence documented in create.md
Relative-root CWD-vs-$toplevel ambiguity	b9ddad9 + f723710
In-repo placement bypass (source repo)	69e1c53
In-repo placement bypass (any checkout)	96826cf
Placement in .git dirs / bare clones	81627ac
..-after-nonexistent bypass	f723710
Windows backslash separator bypass	ac00bf4
Unset userConfig placeholder bad substitution	9f93c77

---

Deferred items (carried, all non-blocking, all tracked in #657)

Finding	Status
--root omitted exits 3 (refuse) not 2 (usage)	Deferred — guidance is still actionable; cosmetic exit-code distinction
Azure SSH git@ssh.dev.azure.com:v3/… not tested	Deferred — gap on an already-working code path; no defect
cp -p follows symlinks to out-of-repo targets	Deferred (SUGGESTION, local-control only, no remote vector). [[ -L "$src" ]] && continue before line 388 closes it; a note in create.md is the minimum
Refuse message references nonexistent "worktree setup skill"	Deferred — /plugin configure works; tracked in #657
git check-ref-format --branch not run after regex	Deferred — names like feat/foo..bar exit 4 instead of 2; tracked in #657
Drive-relative C:foo root (no / after :)	Documented OOS in ac00bf4; tracked in #962

---

Positives

• Minimal, precisely-targeted fix — one character change, no collateral modifications.
• Symmetry with the prior fix at the parse-branch-issue call site — same class of problem (${user_config.dotted.key} in bash), same solution (single-quote), established pattern in the codebase.
• Guard in the helper validates this end-to-end — the fix is not relying on undocumented Claude Code behavior; the detection at worktree-create.sh:116 explicitly matches *'${user_config'* (and the SC2016 disable confirms the literal match is intentional), so the refuse-with-guidance path is robustly exercised.
• All prior positives carry: need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited .worktreeinclude enumeration, output contract, parse_owner_repo fallback chain, eval fixture alignment, all containment guards, and the worktree.baseRef settings.json fix.

---

Summary: 9f93c77 closes the last open Codex P2 — the bad substitution failure on the first-run path that prevented the documented exit-3 guidance from ever being shown. No new findings. All IMPORTANT and P2 findings from the full review cycle are resolved. Remaining deferred items are SUGGESTION-level or minor test gaps, tracked in #657. This PR is ready to merge.

[claude]

Claude finished @kyle-sexton's task in 3m 46s —— View job

---

Security Review

• Read PR diff at HEAD (9f93c77)
• Read worktree-create.sh (primary security surface, full pass)
• Read worktree-create.test.sh
• Read context/create.md (latest change)
• Review plugin.json, SKILL.md, evals.json
• Audit new commit (9f93c77) — single-quote placeholder fix
• Verify status of all carried findings
• Post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md, evals.json, CHANGELOG.md at HEAD 9f93c77

New since last security review (1ff6d69): 9f93c77 — single-quote the worktree_root placeholder in context/create.md so an unset key reaches the helper as an inert literal instead of causing a Bash bad substitution error.

Overall posture: No CRITICAL or IMPORTANT findings at HEAD. The fix in 9f93c77 is correct and closes the Codex P2 "Quote unset userConfig placeholders safely" finding for create.md. One inconsistency between SKILL.md and create.md introduced by the same PR (not by 9f93c77) is worth a SUGGESTION. The carried cp -p symlink issue remains.

---

Resolved since last review ✓

Codex P2 — unset worktree_root placeholder caused bad substitution (9f93c77): context/create.md:70 changed from "${user_config.worktree_root}" (double-quoted) to '${user_config.worktree_root}' (single-quoted). When worktree_root is unset, Claude Code leaves the literal ${user_config.worktree_root} token in the document. Double-quoting caused Bash to try to expand it and fail with bad substitution before the helper ran; single-quoting passes the literal token to the helper, which catches it via the *'${user_config'* guard (line 116 of worktree-create.sh) and returns the documented exit-3 refusal with guidance. Fix is correct and mirrors the #950 fix at the parse-branch-issue call site. ✓

---

SUGGESTION — SKILL.md headline invariant still double-quotes the worktree_root placeholder | confidence: CONFIRMED

SKILL.md line 69

--root "${user_config.worktree_root}"   ← double-quoted (SKILL.md line 69)
--root '${user_config.worktree_root}'   ← single-quoted (create.md line 70, fixed in 9f93c77)

9f93c77 fixed the authoritative procedure in create.md but left the headline safety invariant in SKILL.md with the double-quoted form. SKILL.md is the first document a model executing the skill reads; if it follows the SKILL.md inline snippet directly without consulting create.md — or uses SKILL.md as a shortcut for fast-pathing — it could construct a bash command with double-quoted ${user_config.worktree_root}, which fails with bad substitution in an unconfigured environment. The user gets a shell error instead of the documented exit-3 guidance.

The SKILL.md line was added by this PR (not pre-existing), so the inconsistency was introduced here. The 9f93c77 commit fixed only create.md.

Suggested fix: single-quote the worktree_root placeholder in SKILL.md:69 to match create.md:70:

-- **Create via the shared helper, not `EnterWorktree(name:)`.** Run `bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" --name "<validated-name>" --root "${user_config.worktree_root}"`; …
+- **Create via the shared helper, not `EnterWorktree(name:)`.** Run `bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" --name "<validated-name>" --root '${user_config.worktree_root}'`; …

Note: inside a backtick-enclosed inline code span in markdown, the single-quote is not a markdown metacharacter — it renders correctly as the shell literal the model needs to follow.

Fix this →

---

SUGGESTION (carried) — cp -p follows symlinks; out-of-repo symlink targets copied into the new worktree | confidence: PLAUSIBLE

worktree-create.sh lines 387–390

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] follows symlinks before testing (POSIX -f follows symlinks). A repository symlink whose target is a regular file outside the repo (e.g. secrets.env -> /etc/system-config) that is both gitignored and matches a .worktreeinclude pattern would have its target's contents silently copied into the new worktree at the external root. Exploitation requires local control (attacker must control both the symlink and a matching .worktreeinclude entry). No remote vector. Carried across nine prior passes; deferred to #657.

Minimum fix (one guard before the -f test):

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

---

Notes (informational, not security findings)

• 9f93c77 scopes the fix correctly. The change touches only create.md:70 (one character: " → '). The helper's *'${user_config'* pattern check (line 116) already matched the unquoted literal; the fix ensures it reaches the helper rather than being stopped by Bash first.
• Path-with-single-quote edge case is not a meaningful injection vector. A worktree_root value containing a literal ' (e.g. /home/user's/wt) would break the single-quoted shell command, but the result is a shell syntax error — not silent command injection. The directory userConfig type in plugin.json further limits realistic values.
• All prior resolved findings remain closed: in-repo/any-checkout containment (96826cf), git-dir/bare-repo rejection (81627ac), ..-path normalization (f723710), Windows backslash canonicalization (ac00bf4), worktree.baseRef wrong-store read (aa0f5e8), local settings override (7ce20d7), Codex P2 unset placeholder (9f93c77).
• .worktreeinclude intersection semantics are correct. git ls-files -o -i without --exclude-standard does not load the standard gitignore chain. Only --exclude-from=.worktreeinclude patterns are active; the subsequent check-ignore -q filter correctly computes the intersection. The earlier IMPORTANT finding on this was incorrect.
• parse_owner_repo is traversal-safe. Segments joined with - (not /), so crafted remote URLs cannot produce directory traversal above $root.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

---

Summary: 9f93c77 correctly fixes the Codex P2 "Quote unset userConfig placeholders safely" finding for create.md. The same fix was not applied to the inline example in SKILL.md:69, which was added earlier in this PR and still double-quotes the placeholder — that is the only new finding here (SUGGESTION). The cp -p symlink issue (SUGGESTION, deferred to #657) remains the sole other open item.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f93c7706e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

Re: Single-quote the worktree_root placeholder (SKILL.md:69) — fixed in a90cc41, the second call site of the same quoting fix (create.md:70 was 9f93c77). Grepped the plugin for all "${user_config.worktree_root}" occurrences; these two were the only ones. Single-quoted so an unset key reaches the helper as an inert literal → documented exit-3 guidance instead of Bash bad substitution.

[claude]

Claude finished @kyle-sexton's task in 2m 22s —— View job

---

PR Review

• Read changed files at HEAD (a90cc41)
• Diff new commit since last review (9f93c77 → a90cc41)
• Verify SKILL.md single-quote fix (last Codex P2)
• Re-check all open findings from prior reviews
• Post comprehensive review

---

What a90cc41 does

One file changed: SKILL.md, one character changed.

Before (9f93c77):

Run `bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" --name "<validated-name>" --root "${user_config.worktree_root}"`

After (a90cc41):

Run `bash "${CLAUDE_PLUGIN_ROOT}/scripts/worktree-create.sh" --name "<validated-name>" --root '${user_config.worktree_root}'`

The double-quote → single-quote change on ${user_config.worktree_root} in SKILL.md:69 closes the last Codex P2 finding: when worktree_root is unset Claude Code leaves the literal ${user_config.worktree_root} token, and Bash aborts with bad substitution on a dotted variable name inside double-quotes before the helper ever runs. Single-quoting passes the literal token through to the helper, which detects the unexpanded ${user_config prefix via the *'${user_config'* guard at line 116 and exits 3 with the documented guidance.

---

Last Codex P2 (SKILL.md:69 single-quote fix) ✓ CLOSED

The fix is correct, minimal, and precisely targeted — a single character change that mirrors the 9f93c77 fix already applied to create.md:70. The two call sites are now consistent:

• SKILL.md:69 — '${user_config.worktree_root}' ✓
• context/create.md:70 — '${user_config.worktree_root}' ✓

The end-to-end chain is solid: unset key → literal token reaches the helper → [[ "$root" == *'${user_config'* ]] (with the SC2016 disable confirming the literal match is intentional) → exit 3 + refuse-with-guidance. Verified by the test at worktree-create.test.sh:74.

---

All IMPORTANT and P2 findings from the full review cycle: resolved ✓

Finding	Resolution
.worktreeinclude intersection semantics	CORRECTLY CLASSIFIED INCORRECT — git ls-files -o -i without --exclude-standard does not load the standard gitignore chain; the implementation is correct; the suggested --no-standard-excludes flag does not exist in git
worktree.baseRef reads from wrong store	aa0f5e8 — dead git-config probe removed; caller owns policy via --base-ref
Local settings.local.json override omitted	7ce20d7 — three-tier precedence documented in create.md
Relative-root CWD-vs-$toplevel ambiguity	b9ddad9 + f723710
In-repo placement bypass (source repo)	69e1c53
In-repo placement bypass (any checkout)	96826cf
Placement in .git dirs / bare clones	81627ac
..-after-nonexistent bypass	f723710
Windows backslash separator bypass	ac00bf4
Unset userConfig placeholder bad substitution (create.md)	9f93c77
Unset userConfig placeholder bad substitution (SKILL.md)	a90cc41 ← this commit

---

Deferred items (all carried, all non-blocking, tracked in #657)

Finding	Status
--root omitted exits 3 (refuse) not 2 (usage)	Deferred — guidance is still actionable; cosmetic exit-code distinction
Azure SSH git@ssh.dev.azure.com:v3/… not tested	Deferred — gap on an already-working code path; no defect
cp -p follows symlinks to out-of-repo targets (worktree-create.sh:387-390)	Deferred (SUGGESTION, local-control only, no remote vector). [[ -L "$src" ]] && continue before the -f test closes it cleanly; so would a note in create.md. Neither is blocking.
Refuse message references nonexistent "worktree setup skill"	Deferred — /plugin configure is offered alongside and works; tracked in #657
git check-ref-format --branch not run after regex	Deferred — names like feat/foo..bar exit 4 instead of 2; tracked in #657
Drive-relative C:foo root (no / after :)	Documented OOS; tracked in #962

---

Positives

• Minimal, precisely-targeted fix — one character change, zero collateral modifications.
• Symmetry with the prior fix. 9f93c77 fixed create.md:70; a90cc41 applies the identical fix to SKILL.md:69 — same class of problem (dotted ${user_config.*} placeholder in bash), same solution (single-quote), consistent pattern.
• The test at worktree-create.test.sh:74 already covers this code path — the unexpanded-token case passes '${user_config.worktree_root}' directly to the helper and asserts exit 3, confirming the guard fires for both the create.md and SKILL.md forms.
• Both call sites are now consistent. A model fast-pathing through the SKILL.md invariant block and a model following the full create.md procedure both produce the same shell command for the unset-key case.
• All prior positives carry: need_value guard, origin/HEAD symbolic resolution, refuse-with-guidance message, NUL-delimited .worktreeinclude enumeration, output contract, parse_owner_repo fallback chain, eval fixture alignment, all containment guards, and the worktree.baseRef settings.json fix.

---

Summary: a90cc41 applies the final single-character fix that closes the last Codex P2 — the SKILL.md headline invariant now single-quotes the worktree_root placeholder, matching create.md:70. No new findings. All IMPORTANT and P2 findings from the full review cycle are resolved. Remaining deferred items are SUGGESTION-level or minor test gaps, tracked in #657. This PR is ready to merge.

[claude]

Claude finished @kyle-sexton's task in 2m 36s —— View job

---

Security Review

• Read PR diff (a90cc41)
• Read changed files at HEAD
• Audit worktree-create.sh for injection, path traversal, symlink, and command execution risks
• Audit worktree-create.test.sh for test harness security issues
• Review plugin.json, SKILL.md, context/create.md, evals.json changes
• Compile and post findings

---

Scope: worktree-create.sh, worktree-create.test.sh, plugin.json, SKILL.md, context/create.md, evals.json, CHANGELOG.md at HEAD a90cc41

New since last security review (9f93c77): a90cc41 — single-quotes the worktree_root placeholder in SKILL.md:69 to match the fix already applied to create.md:70 in 9f93c77. No executable code changed.

Overall posture: No CRITICAL or IMPORTANT findings at HEAD. The a90cc41 fix closes the last identified security-relevant issue introduced in this PR. The cp -p symlink issue (SUGGESTION, local-control only) remains the sole open item and has been deferred across every prior review cycle.

---

Resolved since last review ✓

SKILL.md unset-placeholder bad substitution (a90cc41): SKILL.md:69 changed "${user_config.worktree_root}" (double-quoted) to '${user_config.worktree_root}' (single-quoted). When worktree_root is unset, Claude Code leaves the literal ${user_config.worktree_root} token. The dotted key is an invalid bash variable name; double-quoting caused Bash to abort with bad substitution before worktree-create.sh ran, so the documented exit-3 guidance was never surfaced. Single-quoting passes the literal token to the helper, which catches it via the *'${user_config'* guard at line 116 and exits 3 with guidance. Fix is correct and consistent with create.md:70 (fixed in 9f93c77). Verified: no remaining double-quoted ${user_config.*} placeholders exist in any worktree skill file. ✓

---

SUGGESTION (carried) — cp -p follows symlinks; out-of-repo symlink targets copied into the new worktree | confidence: PLAUSIBLE

worktree-create.sh lines 387–390

src="$toplevel/$rel"
[[ -f "$src" ]] || continue
dest="$worktree_path/$rel"
if mkdir -p "$(dirname "$dest")" && cp -p "$src" "$dest"; then

[[ -f "$src" ]] resolves symlinks before testing (POSIX -f follows symlinks). A repository symlink — tracked or untracked — whose target is a regular file outside the repo (e.g. secrets.env -> /etc/some-system-config) that (a) is gitignored and (b) matches a .worktreeinclude pattern would have its target's contents silently copied into the new worktree at the external root, which may have different access controls or be synced to cloud storage.

Exploitation requires local control of both the symlink in the working tree and a matching .worktreeinclude entry. No remote-exploitation vector. Deferred across all prior automated security passes.

Suggested fix (one guard before the -f test eliminates the vector):

[[ -L "$src" ]] && continue   # skip symlinks — target may be outside the repo
[[ -f "$src" ]] || continue

Or document the behavior in create.md under the .worktreeinclude section so users know that matching symlinks copy their target's contents, not the symlink itself.

Fix this →

---

All IMPORTANT findings from the full review cycle: resolved ✓

Finding	Commit
.worktreeinclude intersection semantics	CORRECTLY CLASSIFIED INCORRECT — git ls-files -o -i without --exclude-standard does NOT load the standard gitignore chain; only --exclude-from patterns are active; check-ignore -q computes the real intersection
worktree.baseRef reads from wrong store	aa0f5e8 — dead git config worktree.baseRef probe removed; caller owns policy via --base-ref
Local settings.local.json override omitted	7ce20d7 — three-tier precedence documented in create.md
Relative-root CWD-vs-$toplevel ambiguity	b9ddad9 + f723710
In-repo placement bypass (source repo)	69e1c53
In-repo placement bypass (any checkout)	96826cf
Placement in .git dirs / bare clones	81627ac
..-after-nonexistent bypass	f723710
Windows backslash separator bypass	ac00bf4
Unset userConfig placeholder bad substitution (create.md)	9f93c77
Unset userConfig placeholder bad substitution (SKILL.md)	a90cc41

---

Notes (informational, not security findings)

• .worktreeinclude intersection semantics are correct. git ls-files -o -i without --exclude-standard does NOT load the standard gitignore chain. Only --exclude-from=.worktreeinclude patterns are active; the check-ignore -q post-filter computes the genuine gitignore intersection. The earlier IMPORTANT finding was incorrect; --no-standard-excludes is not a valid git flag.
• URL-derived path components are traversal-safe. parse_owner_repo joins segments with - (not /), so a crafted remote URL (git@host:../../etc/passwd.git) produces etc-passwd-<slug> inside $root — a literal filename component, never a traversal above it.
• git ls-files output is bounded to the repository tree. rel values from git ls-files -o -i -z are repository-relative; ../ traversal paths are not emitted through standard git behavior.
• .. segments in names are not a traversal vector. The name regex permits . chars, but the slug step converts / → -, making ../.. → ..-..-slug (a literal component inside $root). Git then rejects .. as an invalid branch name component (exit 4).
• IFS='/' read -r -a parts in parse_owner_repo is command-scoped. The IFS='/' prefix applies only to read; it does not permanently modify IFS.
• $(dirname "$dest") cannot be injected. $dest is formed via double-quoted variable expansion; any special characters in $rel are literal string content passed to the external dirname command, not re-evaluated by the shell.
• --is-inside-git-dir output comparison is exact-match safe. [[ "$(…)" == "true" ]] uses a quoted right-hand side in [[…]], so no glob expansion is possible. Git outputs only "true", "false", or nothing on error; the last case evaluates "" == "true" → false.
• No GitHub Actions changes in this PR — no workflow injection surface to review.

---

Summary: No CRITICAL or IMPORTANT findings at HEAD a90cc41. The latest commit closes the last identified call site of the unset-userConfig-placeholder bad substitution issue. The sole remaining open security item is the cp -p symlink traversal (SUGGESTION, local-control only, deferred throughout the review cycle).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a90cc41140

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

Re: Escape worktree_root before invoking the helper (single-quote breaks on a value containing ', e.g. /Users/O'Connor/worktrees) — acknowledged and tracked as #965, deferred (not fixed on this PR).

The interim single-quote fix on this PR is a net improvement: it correctly handles the COMMON unset/first-run path (literal token → helper exit-3 guidance), trading it for a RARE special-char-in-path case. Neither quote style is safe for an arbitrary path value under raw template substitution — the complete fix needs the Claude Code ${user_config} substitution/escaping semantics verified from official docs first (fresh-docs mandate), which is out of scope for this worktree-helper PR.

Deferring per the loop-breaker carve-out: both failure modes fail LOUD (visible Bash bad substitution / syntax error — nothing runs, no silent-wrong path), bounded blast radius, no data-loss/security. #965 carries the repro + fix direction + docs-to-verify + origin. Resolving here against #965.