fix(source-control): copy settings.local.json into new worktrees

[kyle-sexton]

Summary

• Carry .claude/settings.local.json from the git common dir so env overrides such as DISK_HYGIENE_GUARD_WATCHDOG_SECONDS load in worktree sessions

Closes #2119

Test plan

• bash plugins/source-control/scripts/worktree-create.test.sh

Related

N/A

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

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

---

Security Review Complete

• Fetch PR diff
• Read changed files (worktree-create.sh, worktree-create.test.sh)
• Analyze security implications of copying settings.local.json into worktrees
• Check for path traversal / injection / unsafe file handling
• Post findings

No CRITICAL or IMPORTANT security issues found. The change adds ~19 lines to worktree-create.sh that copy .claude/settings.local.json from the git common dir into a freshly created worktree; the corresponding test just asserts the file lands there. All paths involved ($toplevel, $worktree_path, $common_dir) are properly quoted, there's no eval/unquoted interpolation, and nothing here touches GitHub Actions workflows, permissions, or CI trust boundaries (this PR only touches a local shell script + its test).

One low-severity note for awareness:

• SUGGESTION / Confidence: Low — cp -p "$settings_src" "$settings_dest" follows symlinks by default (no -P/-d). If $settings_src (resolved at L712-L716) were ever a symlink, its target's contents would be dereferenced and copied into the new worktree's .claude/settings.local.json rather than the symlink itself. Exploiting this requires the attacker to already control the contents of the local .git metadata directory, which implies a much broader local compromise than this script alone — so this is theoretical, not a practical vulnerability in the current threat model. It's also consistent with the pre-existing .worktreeinclude copy logic just above it (same cp -p pattern, not part of this diff). Flagging only because this new code path now applies that pattern to a file (settings.local.json) that's more likely to carry secrets/credentials than arbitrary .worktreeinclude entries. No action required unless you want defense-in-depth (e.g. cp -p --no-dereference / reject symlink sources).

I did not find any injection, authorization, or trust-boundary issues introduced by this diff.

Note: I was unable to execute bash plugins/source-control/scripts/worktree-create.test.sh in this sandbox (Bash tool required interactive approval I don't have here), so I reviewed the test and script statically rather than confirming the test run — this doesn't affect the security findings above, which are based on reading the diff.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a614f8df89

ℹ️ 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 30s —— View job

---

Code review: PR #2453

• Gather context (diff, CLAUDE.md, REVIEW.md)
• Understand the request
• Review changed files for correctness, error handling, test coverage, maintainability
• Post findings

Summary

This PR adds logic to worktree-create.sh to carry .claude/settings.local.json into new worktrees, plus a regression test. I found one confirmed correctness bug and one confirmed test-coverage gap, both posted as inline comments on the relevant lines:

🔴 worktree-create.sh:712-716 — the copy source path is wrong. git rev-parse --git-common-dir returns the .git metadata directory, not the working-tree root that actually contains .claude/. I confirmed this directly in this repo (git rev-parse --git-common-dir → .git; git rev-parse --show-toplevel → the repo root). As written, settings_src resolves to <repo>/.git/.claude/settings.local.json, which will essentially never exist — the real file lives at <repo>/.claude/settings.local.json, a sibling of .git. The feature silently no-ops in every real invocation. Fix is to use dirname "$common_dir" as the source root.

🟡 worktree-create.test.sh:442-450 — the new test can't catch the bug above. The test commits .claude/settings.local.json to git history (git add + git commit), even though that file is gitignored in this project (.gitignore:13) and is untracked in real usage — that's precisely why it needs an explicit copy step. Because it's committed in the test, git worktree add checks it out automatically via normal git behavior, independent of the new copy code. The assertion passes regardless of whether the copy logic works, which is how the path bug above shipped undetected. The .worktreeinclude test case immediately above (lines 422–433) shows the correct pattern — fixture files are created untracked, after the commit, so the assertions genuinely exercise the copy code.

Note: I verified the git rev-parse behavior myself by running it in this checkout. I did not independently run worktree-create.test.sh (the author-claimed test plan in the PR description is unverified by me).

No other issues found — the rest of the diff (mkdir/cp error handling, log messages, doc comment) follows the existing .worktreeinclude copy pattern in the same file and is otherwise sound. Per this repo's REVIEW.md lane split, security-relevant findings (none applicable here) are left to the dedicated security lane.

Branch: cursor/worktree-settings-carryin-63e1

[github-actions]

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

[claude]

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

---

Security Review Complete

• Fetch PR diff (git diff origin/main...HEAD)
• Read changed files (worktree-create.sh, worktree-create.test.sh)
• Analyze security implications of copying settings.local.json into worktrees
• Check for path traversal / injection / unsafe file handling / trust-boundary issues
• Post findings

Scope: only the two files changed in this PR (plugins/source-control/scripts/worktree-create.sh +19, plugins/source-control/scripts/worktree-create.test.sh +10). No GitHub Actions workflow files are touched by this diff, so the pull_request_target/workflow_run/permissions/pinning categories don't apply here.

No CRITICAL or IMPORTANT security issues found.

The new block resolves common_dir via git -C "$toplevel" rev-parse --git-common-dir (relativized against $toplevel if not absolute), builds a fixed-suffix settings_src="$common_dir/.claude/settings.local.json", and — if that path exists — cp -ps it to $worktree_path/.claude/settings.local.json. Every variable expansion in the new code ($toplevel, $common_dir, $settings_src, $settings_dest, $worktree_path) is double-quoted; there's no eval, no unquoted word-splitting, and no attacker-influenced input feeding path construction (the only inputs are the git-computed common dir and a hardcoded relative suffix). This mirrors the pre-existing .worktreeinclude copy loop directly above it, which already has NUL-safe handling for the untrusted case (arbitrary filenames from ls-files) — the new code doesn't need that since the filename is hardcoded.

One low-severity note, same as flagged in the earlier security pass on this PR and still applicable to the current diff:

• SUGGESTION / Confidence: Low — cp -p "$settings_src" "$settings_dest" follows symlinks by default. If $settings_src were ever a symlink, its target's contents (not the symlink itself) would be dereferenced and written into the new worktree's .claude/settings.local.json. Exploiting this would require an attacker to already control contents under the local git common dir, which implies a broader local compromise than this script alone — theoretical, not a practical vulnerability in the current threat model. Worth a second look only because settings.local.json is more likely than an arbitrary .worktreeinclude entry to carry secrets/credentials. No action required unless defense-in-depth is desired (cp -p --no-dereference / reject symlink sources).

I did not find any injection, authorization, or trust-boundary issues introduced by this diff. (Note: a separate, non-security correctness bug in this same block — common_dir vs. the checkout root — has already been flagged by the code-review lane and Codex on this PR; that's out of scope for this security-only pass.)

[claude]

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

---

Code review: PR #2453

• Gather context (gh pr diff, CLAUDE.md, REVIEW.md)
• Check whether prior review findings (settings_src path bug, test coverage gap) were addressed
• Review changed files for correctness, error handling, test coverage, maintainability
• Post findings

Summary

I reviewed the current state of this PR (commit 8120587) against the code-review lane's scope in REVIEW.md (correctness, design, error handling, tests, maintainability — security is explicitly left to the dedicated claude-security-review lane, which exists in this repo).

The diff content at this commit is unchanged from what earlier review passes on this PR already examined, and both previously-flagged issues are still present:

🔴 worktree-create.sh:726 — settings_src is computed from the .git metadata dir, not the checkout root, so the copy silently no-ops. I independently confirmed git rev-parse --git-common-dir returns .git (not the toplevel) in this checkout, and that .claude/settings.local.json is gitignored (.gitignore:13), sitting as a sibling of .git rather than inside it. As written, settings_src resolves to <repo>/.git/.claude/settings.local.json, which never exists — full details and a suggested fix (dirname "$common_dir") posted inline.

🟡 worktree-create.test.sh:455 — the new test can't catch the bug above. It git adds and commits .claude/settings.local.json before creating the worktree, so git worktree add checks it out via ordinary git tracking, independent of the new copy logic. The assertion passes regardless of whether the copy code works. The .worktreeinclude case just above it (lines 427-445) shows the correct untracked-fixture pattern — details and a corrected test snippet posted inline.

Both were raised by two earlier review passes on this PR (Codex and a prior Claude pass) and do not appear to have been addressed yet in the current commit — I'm re-confirming them here since this is the code-review lane's pass over the current PR state, not assuming the earlier reports still stand without re-checking.

No other issues found. The rest of the diff (mkdir/cp error handling pattern, log messages, doc comment) follows the existing .worktreeinclude copy block in the same file and is otherwise sound, aside from one minor, non-blocking observation: unlike the .worktreeinclude block (which exit 4s on a copy failure), the settings-copy block only logs a warning and continues — this looks like an intentional "best-effort" choice for a non-critical convenience file rather than an oversight, so I'm not flagging it as a finding.

Per this repo's REVIEW.md lane split, security-relevant findings (none applicable here) are left to the dedicated security lane.

I did not independently run worktree-create.test.sh — the author-claimed test plan in the PR description is unverified by me; my findings above are based on static reading of the diff plus commands I ran myself (git rev-parse --git-common-dir / --show-toplevel, .gitignore contents).

Branch: cursor/worktree-settings-carryin-63e1

[claude]

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

---

Security Review Complete

• Fetch PR diff (git diff origin/main...HEAD)
• Read changed files (worktree-create.sh, worktree-create.test.sh)
• Check for changes since last security review pass
• Analyze for injection / path traversal / trust-boundary / secret-handling issues
• Post findings

Scope: only the two files changed in this PR (plugins/source-control/scripts/worktree-create.sh +19, plugins/source-control/scripts/worktree-create.test.sh +10). No GitHub Actions workflow files are touched, so the pull_request_target/workflow_run/permissions/pinning categories don't apply.

No changes since the last security pass. I diffed the current HEAD (13e9eb3) against origin/main and confirmed the file content is byte-identical to what was already reviewed at commit 8120587 (same file SHAs: worktree-create.sh → d00f507, worktree-create.test.sh → 7cdb6a5). The findings below are therefore unchanged from the prior security pass.

No CRITICAL or IMPORTANT security issues found.

The new block resolves common_dir via git -C "$toplevel" rev-parse --git-common-dir (relativized against $toplevel if not absolute), builds a fixed-suffix settings_src="$common_dir/.claude/settings.local.json", and — if that path exists — cp -ps it to $worktree_path/.claude/settings.local.json. Every variable expansion in the new code ($toplevel, $common_dir, $settings_src, $settings_dest, $worktree_path) is double-quoted; there's no eval, no unquoted word-splitting, and no attacker-influenced input feeding path construction (the only inputs are the git-computed common dir and a hardcoded relative suffix). This mirrors the pre-existing .worktreeinclude copy loop directly above it.

One low-severity note, carried forward from both prior passes:

• SUGGESTION / Confidence: Low — cp -p "$settings_src" "$settings_dest" follows symlinks by default. If $settings_src were ever a symlink, its target's contents (not the symlink itself) would be dereferenced and written into the new worktree's .claude/settings.local.json. Exploiting this would require an attacker to already control contents under the local git common dir, which implies a broader local compromise than this script alone — theoretical, not a practical vulnerability in the current threat model. Worth a second look only because settings.local.json is more likely than an arbitrary .worktreeinclude entry to carry secrets/credentials. No action required unless defense-in-depth is desired (cp -p --no-dereference / reject symlink sources).

I did not find any injection, authorization, or trust-boundary issues introduced by this diff. (Note: a separate, non-security correctness bug in this same block — settings_src resolving under .git instead of the checkout root — has already been flagged repeatedly by the code-review lane and Codex on this PR; that's out of scope for this security-only pass.)

Branch: cursor/worktree-settings-carryin-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 16s —— View job

---

Security Review Complete

• Gather PR diff (gh pr diff 2453)
• Check skip gate — file content changed since the last security pass, so re-review
• Read changed files at current HEAD (00e14ae)
• Analyze for injection / path traversal / trust-boundary / secret-handling issues
• Post findings

Scope: only the two shell files with logic changes (plugins/source-control/scripts/worktree-create.sh +20, plugins/source-control/scripts/worktree-create.test.sh +8). plugin.json/CHANGELOG.md are metadata-only. No GitHub Actions workflow files are touched, so the pull_request_target/workflow_run/permissions/pinning categories don't apply.

No CRITICAL or IMPORTANT security issues found.

The previously-flagged correctness bug (settings_src resolving under .git instead of the checkout root) is fixed at this head: worktree-create.sh:722-727 now takes main_worktree="$(dirname "$common_dir")" before building settings_src, correctly landing on the working-tree root that actually contains .claude/. This is a correctness fix, not a security-relevant change in itself, but it does mean the copy logic now actually executes on real inputs, so I re-verified the security properties of the executing path:

• All variable expansions ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest, $worktree_path) are double-quoted; no eval, no unquoted word-splitting.
• settings_src's filename suffix (.claude/settings.local.json) is a hardcoded literal, not attacker-influenced — the only variable input is the git-computed common dir, so there's no path-traversal or injection surface here.
• main_worktree is derived the same way from a linked worktree as from the main checkout (--git-common-dir always points at the main repo's .git), so the fix behaves consistently in both invocation shapes.

One low-severity note, unchanged from earlier passes on this PR and still theoretical: cp -p "$settings_src" "$settings_dest" follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself. Exploiting this requires the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone — no action required.

I did not find any injection, authorization, or trust-boundary issues introduced by this diff. The test-file change (worktree-create.test.sh) is a plain fixture/assertion addition with no security surface.

[github-actions]

Last security-reviewed head: 8c07b877eb525046090f3582f39e032263b3f88c. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @cursor[bot]'s task in 1m 28s —— View job

---

Code review: PR #2453

• Gather context (gh pr diff, CLAUDE.md, REVIEW.md)
• Check whether prior review findings (settings_src path bug, test coverage gap) were addressed at HEAD 00e14ae
• Review changed files for correctness, error handling, test coverage, maintainability
• Post findings

Summary

Both issues raised by earlier passes on this PR (Codex and prior Claude code-review passes) are fixed as of this commit (00e14ae):

✅ worktree-create.sh:717-720 — settings_src path bug resolved. The block now takes dirname "$common_dir" (relativizing common_dir against $toplevel first if --git-common-dir returned a relative path) to get main_worktree, then builds settings_src="$main_worktree/.claude/settings.local.json". I confirmed independently that dirname of a .git common dir path correctly yields the checkout root that's a sibling of .git, matching the suggested fix from the earlier reviews.

✅ worktree-create.test.sh:449-455 — test-coverage gap resolved. The new case now creates .claude/settings.local.json as an untracked file (mkdir -p + printf after mkrepo's initial commit, never git added), matching the correct pattern used by the .worktreeinclude case just above it. This means the assertion now genuinely depends on the copy logic in worktree-create.sh running — it would fail if that block were deleted or the path were wrong, unlike the previous version which committed the fixture and let ordinary git checkout satisfy the assertion regardless of the copy code.

I was unable to execute bash plugins/source-control/scripts/worktree-create.test.sh myself in this sandbox (interactive approval unavailable), so I did not independently confirm a green test run — the author-claimed test plan in the PR description is unverified by me. My conclusions above are based on static reading of the diff, tracing the common_dir/dirname logic, and confirming .claude/settings.local.json is repo-gitignored (consistent with the test intentionally leaving the fixture untracked).

No other issues found. Version bump (0.53.11 → 0.53.12) and CHANGELOG.md entry accurately describe the fix. Per this repo's REVIEW.md lane split, security-relevant findings are left to the dedicated security lane (which has already passed on this diff in a separate review pass, noting only a low-severity, out-of-scope cp -p symlink-following observation).
· Branch: cursor/worktree-settings-carryin-63e1

[claude]

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

---

Security Review Complete

• Gather context (skill instructions, prior review history on this PR)
• Check skip gate — this exact diff already has an applicable security review
• Fetch PR diff (git diff origin/main...HEAD)
• Confirm no plugins/source-control/ changes since the last-reviewed head
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is 2beea994600418e7d552c390ec31cef51657764e, a merge commit ("Merge branch 'main' into cursor/worktree-settings-carryin-63e1"). I diffed it against the last security-reviewed head (00e14aee95d71833358144e8e32df52f76c78acc, per the prior security pass's tracking comment) and confirmed:

git diff 00e14aee..2beea99 -- plugins/source-control/

produces no output — none of the files this PR actually touches (worktree-create.sh, worktree-create.test.sh, plugin.json, CHANGELOG.md) changed. The merge only pulled in unrelated commits from main (autonomy, bash-format, education, guardrails, powershell-format, rate-limit-guard, session-flow, songwriting, work-items plugins, and a top-level verification script) — none of which are part of this PR's diff (git diff origin/main...HEAD confirms the PR diff is still exactly the same 4 files, byte-identical to what was reviewed at 00e14ae).

Per the security-review skill's guidance, re-posting the same findings would add noise rather than signal, so per the skip gate I'm not re-reviewing. The prior conclusions still stand:

No CRITICAL or IMPORTANT security issues. The carry-in logic uses only double-quoted, non-attacker-influenced path construction ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from all prior passes: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this would require the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.
· Branch: cursor/worktree-settings-carryin-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 20s —— View job

---

Security Review Complete

• Load /review:security-review skill instructions
• Check skip gate — file content unchanged since last security pass
• Fetch PR diff (git diff origin/main...HEAD) and confirm scope
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is 6772168 (67721687ff49c9c3a27716236e47a1bdfb350ab5). Diffing the actual PR content (git diff origin/main...HEAD) against the last fully-reviewed state (commit 00e14ae, re-confirmed unchanged at merge commit 2beea99) shows:

• plugins/source-control/scripts/worktree-create.sh — byte-identical to the previously reviewed version. The carry-in logic (common_dir="$(git -C "$toplevel" rev-parse --git-common-dir)" → main_worktree="$(dirname "$common_dir")" → settings_src="$main_worktree/.claude/settings.local.json" → guarded cp -p) is unchanged.
• plugins/source-control/scripts/worktree-create.test.sh — byte-identical to the previously reviewed version (untracked fixture pattern).
• plugins/source-control/.claude-plugin/plugin.json and CHANGELOG.md — only a version bump (0.53.14 → 0.53.15) and changelog entry, picked up from rebasing onto a newer origin/main. No security surface.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing. Prior conclusions on this diff still stand:

No CRITICAL or IMPORTANT security issues. All path construction in the carry-in block uses double-quoted, non-attacker-influenced variables ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from every prior pass: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this would require the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.

No GitHub Actions workflow files are touched by this PR, so the pull_request_target/workflow_run/permissions/pinning categories don't apply.

Branch: cursor/worktree-settings-carryin-63e1

[claude]

Claude finished @cursor[bot]'s task in 2m 7s —— View job

---

Code review: PR #2453

• Gather context (gh pr diff, CLAUDE.md, REVIEW.md)
• Check whether prior review findings (settings_src path bug, test-coverage gap) are still addressed at current HEAD (6772168)
• Review changed files for correctness, error handling, test coverage, maintainability
• Post findings

Summary

Reviewed the current head (6772168, PR diff identical in content to what earlier passes already confirmed fixed at 00e14ae/2beea99 — the branch has since been rebased to two commits, but the resulting diff against origin/main is the same). Both previously-flagged issues remain fixed:

✅ worktree-create.sh:717-735 — settings_src path bug. The block resolves common_dir via git -C "$toplevel" rev-parse --git-common-dir (relativized against $toplevel when relative), takes main_worktree="$(dirname "$common_dir")", and builds settings_src="$main_worktree/.claude/settings.local.json". That correctly lands on the checkout root that's a sibling of .git, not inside it — the fix earlier reviews asked for.

✅ worktree-create.test.sh:447-453 — test-coverage gap. The fixture is created untracked (mkdir -p + printf, never git added) after mkrepo's initial commit, matching the correct pattern used by the .worktreeinclude case just above it (lines 427-445). This means the assertion now genuinely depends on the copy block in worktree-create.sh running, rather than being satisfied by ordinary git checkout of a tracked file.

No other issues found:

• All variable expansions in the new block are double-quoted; no injection/path-traversal surface (also confirmed clean by the dedicated security lane on this PR).
• mkdir -p "$(dirname "$settings_dest")" && cp -p ... follows the existing .worktreeinclude copy pattern in the same file; failure only logs a warning rather than exiting like the .worktreeinclude block does — this reads as an intentional best-effort choice for a non-critical convenience file, not an oversight, so not flagged.
• plugin.json version bump (0.53.14 → 0.53.15) and the CHANGELOG.md entry accurately describe the fix.

I was unable to execute bash plugins/source-control/scripts/worktree-create.test.sh myself in this sandbox (the Bash tool required interactive approval unavailable here), so I did not independently confirm a green test run — the test plan in the PR description is author-claimed and unverified by me. My conclusions above are from static reading of the diff and tracing the common_dir/dirname logic by hand.

Per this repo's REVIEW.md lane split, security-relevant findings are left to the dedicated security lane, which has already reviewed this same diff content and found no critical/important issues (one low-severity, still-theoretical cp -p symlink-following note, unchanged from earlier passes).

Branch: cursor/worktree-settings-carryin-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 6s —— View job

---

Security Review Complete

• Load /review:security-review skill instructions
• Fetch PR diff (gh pr diff 2453, base origin/main)
• Check skip gate — diff content vs. last-reviewed head
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is d1021ae (d1021aee5a66cab6dd5234b4c4e0cd2373c95b58). I compared the actual PR diff (gh pr diff 2453, base origin/main) against the last fully-reviewed state (previously reviewed at commits 00e14ae / 2beea99 / 6772168):

• plugins/source-control/scripts/worktree-create.sh — byte-identical to the previously reviewed carry-in block. common_dir="$(git -C "$toplevel" rev-parse --git-common-dir)" → relativized against $toplevel if needed → main_worktree="$(dirname "$common_dir")" → settings_src="$main_worktree/.claude/settings.local.json" → guarded cp -p. Unchanged.
• plugins/source-control/scripts/worktree-create.test.sh — byte-identical to the previously reviewed untracked-fixture test case.
• plugins/source-control/.claude-plugin/plugin.json — version bump only (0.53.15 → 0.53.16).
• plugins/source-control/CHANGELOG.md — changelog entry only, no executable content.

The branch also picked up an unrelated, already-merged-to-main commit (8b6c53c, sudo clustered-short-option peeling in hook-utils.sh) via rebase/merge — that file is not part of this PR's diff against origin/main (confirmed via gh pr diff 2453), so it's out of scope here and was already reviewed/merged separately.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing the logic. Prior conclusions on this diff still stand:

No CRITICAL or IMPORTANT security issues. All path construction in the carry-in block uses double-quoted, non-attacker-influenced variables ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from every prior pass: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this requires the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.

No GitHub Actions workflow files are touched by this PR, so the pull_request_target / workflow_run / permissions / pinning categories don't apply.

Branch: cursor/worktree-settings-carryin-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 18s —— View job

---

Code review: PR #2453

• Gather context (git diff, CLAUDE.md, REVIEW.md, prior review history on this PR)
• Check skip gate — this head already has a successful code-review pass that still applies
• Confirm no content changes since the last-reviewed state
• Post findings

Skip gate hit (condition 4): this head already has a successful review from this lane that still applies.

Current HEAD is d1021ae ("fix(source-control): resolve settings.local carry-in path and test fixture"). I diffed the PR content (git diff f58cfe5^..HEAD, equivalent to origin/main...HEAD) and confirmed the two files with actual logic changes are byte-identical to what this lane already reviewed and confirmed fixed at commit 00e14ae (and re-confirmed unchanged at 2beea99 and 67721687):

• worktree-create.sh:717-736 — settings_src correctly resolves via main_worktree="$(dirname "$common_dir")" before building the source path, fixing the original .git-metadata-dir bug (flagged by Codex and two earlier Claude passes).
• worktree-create.test.sh:447-453 — the fixture is created untracked (mkdir -p + printf, never git added), matching the correct pattern from the .worktreeinclude case above it, so the assertion genuinely exercises the copy logic.

plugin.json (0.53.15 → 0.53.16) and CHANGELOG.md only carry a version bump and changelog entry accurately describing the fix — no logic surface.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing line-by-line. No new findings. Both issues originally raised on this PR (path bug, test-coverage gap) remain fixed at this head.

I was unable to execute bash plugins/source-control/scripts/worktree-create.test.sh myself in this sandbox (interactive Bash approval unavailable), so I did not independently confirm a green test run — the test plan in the PR description is author-claimed and unverified by me. My conclusion above is based on static diffing of file content against the previously-reviewed-and-approved state.

Per this repo's REVIEW.md lane split, security-relevant findings are left to the dedicated security lane, which has already reviewed this same diff content and found no critical/important issues (one low-severity, still-theoretical cp -p symlink-following note, unchanged across all passes).

Branch: cursor/worktree-settings-carryin-63e1

[claude]

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

---

Security Review Complete

• Load /review:security-review skill instructions
• Fetch PR diff (git diff origin/main...HEAD) and confirm scope
• Check skip gate — diff content vs. last-reviewed head
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is c0daa67 (c0daa67ea98a10e8cf9222f0763f44eb6c765ecb), a merge commit ("merge: forward main after #2465 hook-utils sync"). I confirmed the actual PR diff is unchanged:

• git diff origin/main...HEAD --stat shows exactly the same 3 files this PR has always touched: plugins/source-control/CHANGELOG.md, plugins/source-control/scripts/worktree-create.sh, plugins/source-control/scripts/worktree-create.test.sh (35 insertions, 0 deletions — same as prior passes).
• git diff d1021ae..HEAD -- plugins/source-control/scripts/worktree-create.sh plugins/source-control/scripts/worktree-create.test.sh (the last security-reviewed head) produces no output — these two files with actual logic are byte-identical to what was already reviewed.
• The merge did bring in plugins/source-control/hooks/hook-utils.sh (the emit_telemetry temp-file fix, hook::emit_telemetry drops the envelope once data exceeds the Windows command-line limit #1595) and its CHANGELOG.md entry, but that commit (fa762fb) already landed on origin/main independently — it does not appear in this PR's diff against origin/main and is out of scope here.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing the logic. Prior conclusions on this diff still stand:

No CRITICAL or IMPORTANT security issues. The carry-in block in worktree-create.sh:717-736 uses only double-quoted, non-attacker-influenced path construction ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from every prior pass: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this requires the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.

No GitHub Actions workflow files are touched by this PR, so the pull_request_target / workflow_run / permissions / pinning categories don't apply.

Branch: cursor/worktree-settings-carryin-63e1

[claude]

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

---

Security Review Complete

• Gather context (PR diff, prior review history on this PR)
• Invoke /review:security-review skill instructions
• Check skip gate — diff content vs. last-reviewed head
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is 63f7aed (63f7aedc9c05d49bf141a9db745fa3092ff898d7). Diffing against the last fully-reviewed head (c0daa67, reviewed here) shows only:

• plugins/source-control/.claude-plugin/plugin.json — version bump only (0.53.16 → 0.53.17).
• plugins/source-control/CHANGELOG.md — changelog entry only, no executable content.

The two files with actual logic — worktree-create.sh and worktree-create.test.sh — are byte-identical to what was already reviewed and cleared at c0daa67 / d1021ae / 00e14ae.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing the logic. Prior conclusions on this diff still stand:

No CRITICAL or IMPORTANT security issues. The settings.local.json carry-in block in worktree-create.sh:717-736 uses only double-quoted, non-attacker-influenced path construction ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from every prior pass: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this requires the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.

No GitHub Actions workflow files are touched by this PR, so the pull_request_target / workflow_run / permissions / pinning categories don't apply.

Branch: cursor/worktree-settings-carryin-63e1

[claude]

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

---

Security Review Complete

• Load /review:security-review skill instructions
• Fetch PR diff (git diff origin/main...HEAD) and confirm scope
• Check skip gate — diff content vs. last-reviewed head
• Post findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

Current HEAD is 8c07b87 (8c07b877eb525046090f3582f39e032263b3f88c, "chore: retrigger CI"). Diffing against the last fully-reviewed head (63f7aed, reviewed here):

• git diff origin/main...HEAD --stat shows the same 4 files this PR has always touched (38 insertions, 1 deletion).
• plugins/source-control/scripts/worktree-create.sh and plugins/source-control/scripts/worktree-create.test.sh — the two files with actual logic — are byte-identical to what was already reviewed and cleared (git diff 63f7aed..HEAD -- <these two files> produces no output).
• plugins/source-control/.claude-plugin/plugin.json — version bump only (0.53.17 → 0.53.18).
• plugins/source-control/CHANGELOG.md — new changelog entry for this fix plus reordering of an unrelated, already-released 0.53.17 entry. No executable content.

Per the skill's skip gate, re-posting identical findings would add noise rather than signal, so I'm not re-reviewing the logic. Prior conclusions on this diff still stand:

No CRITICAL or IMPORTANT security issues. The settings.local.json carry-in block in worktree-create.sh:717-736 uses only double-quoted, non-attacker-influenced path construction ($toplevel, $common_dir, $main_worktree, $settings_src, $settings_dest) with a hardcoded filename suffix — no injection or path-traversal surface. One low-severity, still-theoretical note carried forward from every prior pass: cp -p follows symlinks by default, so a symlinked settings_src would have its target's contents copied rather than the link itself — exploiting this requires the attacker to already control content under the local git common dir, which implies a broader local compromise than this script alone. No action required.

No GitHub Actions workflow files are touched by this PR, so the pull_request_target / workflow_run / permissions / pinning categories don't apply.

Branch: cursor/worktree-settings-carryin-63e1