fix(source-control): convert the mktemp path to Windows mixed form before it feeds the Write tool

[kyle-sexton]

Closes #3496

Summary

plugins/source-control/skills/worktree/context/create.md step 1 told the model to print a bare mktemp -d path and hand it onward — as the Write tool's file_path (the worktree-root / data-root steps) and substituted into the later Bash block. On Windows, Git Bash's GNU mktemp prints the POSIX literal /tmp/tmp.XXXXXXXXXX (TMPDIR is unset; MSYS mounts /tmp on %TEMP%). The Write tool is native node.exe: Win32 resolves the leading / against the current drive, so the file lands in a phantom C:\tmp\tmp.XXXXXXXXXX while the real directory sits in %TEMP%. Nothing errors — the silent drive-root class docs/conventions/windows-path-emit/README.md owns, previously worked as #2834. Confirmed live on this machine: an empty C:\tmp\tmp.rSFIkHm5DO created at 14:15:56 on 2026-08-30 by exactly this path.

Fix

The snippet now converts before printing, per the windows-path-emit convention:

root_dir="$(mktemp -d)"
case "${OSTYPE:-}" in
msys* | cygwin* | win32) root_dir="$(cygpath -m -l -- "$root_dir")" || exit 2 ;;
esac
printf '%s\n' "$root_dir"

• Rule 3 — cygpath, mixed form. -m (C:/...) is correct for both consumers of the one printed value: the Write tool and the later Git Bash block (--fallback-root-file, rm -rf), so no half-converted split arises (Rule 2). -l additionally expands the 8.3 short name %TEMP% carries on this machine (KYLESE~1), whose ~ misbehaves downstream.
• Rule 4 — fail loud. || exit 2; the unconverted MSYS literal is never printed, because it is precisely what writes to the wrong place. Non-Windows passes through unchanged, gated on $OSTYPE exactly as scripts/emit-windows-path.sh does. The helper itself is marketplace-root tooling not shipped inside the plugin, and create.md is markdown instruction — so its contract is inlined, not called.
• mktemp -d -p "$TEMP" rejected, twice over: mktemp -p is an ACTIVE flagged GNU/BSD-divergence token in scripts/shell-portability-tokens.txt (it would fail the portability gate), and it yields mixed separators (C:\...\Temp/tmp.XXX).
• The "load-bearing details" list now documents the conversion (two entries → three) so the next editor does not revert it as noise. Both original properties are preserved: mktemp -d (directory exists, file inside does not — the Write-overwrite refusal) and the deliberate Write-tool round-trip for the raw ${user_config.worktree_root} substitution.

No sibling doc in the skill repeats the snippet (checked: the only other occurrence is the evals.json grading criterion, which is form-agnostic and stays accurate).

Verification

• Executed the new snippet live in Git Bash on Windows: prints C:/Users/KyleSexton/AppData/Local/Temp/tmp.qiZCfYGBa6 — mixed form, short name expanded. With OSTYPE=linux-gnu the path passes through unchanged, exit 0. With a stubbed failing cygpath, exit 2 and zero stdout.
• scripts/check-shell-portability.sh --paths <file>: clean ("No unexcused GNU-only constructs").
• scripts/check-changed-skills.sh origin/main: CHECK-SKILL worktree PASS — 0 errors.
• npx markdownlint-cli2 <file>: 0 issues.
• scripts/affected-tests.sh --run --explain: the changed file is the excused *.md class (hygiene lane); basename over-selection picked 6 shell suites that merely name create.md. Four completed locally, zero failures — repo-local telemetry sink (2 PASS), claude-ops telemetry sink (PASS=16 FAIL=0), flag-commit-pr-skill-bypass (PASS=35 FAIL=0), check-hook-wiring-liveness (all assertions passed) — plus 71 green assertions from the two pr-linkage suites before the local 10-minute tool cap cut them off (0 failures observed; they still run detached locally). CI re-runs the authoritative suites on this PR.
• Independent fresh-context verifier: 8/8 criteria PASS (mktemp -d property, mixed-form emission, POSIX pass-through, fail-loud, no mktemp -p, doc coherence for both consumers, markdownlint, convention-link resolution). Non-blocking notes only (an empty temp dir is orphaned in %TEMP% if cygpath fails — OS temp cleaning covers it).

Related

• Closes source-control: worktree create.md emits a bare mktemp -d POSIX path that the native Write tool resolves to a phantom drive-root tmp #3496 — the tracked defect this PR fixes.
• Windows verification harnesses: an MSYS /d/ path literal silently redirected two tzdata sabotage cases into a duplicate of case E #2834 — the previously-worked instance of the same drive-root emit class (harness fixtures under a phantom tree).
• docs/conventions/windows-path-emit/README.md — the owning convention (Rules 2–4 applied here).
• scripts/emit-windows-path.sh — the repo helper whose contract (mixed form default, $OSTYPE gate, fail-loud) this inlines.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T01:24:39.306768Z	65195ae	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

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

---

Code review complete

• Gather context (PR diff, changed file, /review:code-review criteria)
• Review plugins/source-control/skills/worktree/context/create.md for correctness and maintainability
• Post findings

No blocking findings. This is a small (+7/-2), well-scoped fix and it holds up under review:

• The case "${OSTYPE:-}" in msys* | cygwin* | win32) gate matches scripts/emit-windows-path.sh's host gate exactly (scripts/emit-windows-path.sh#L96-L97).
• cygpath -m -l combining mixed-form with 8.3 long-name expansion isn't invented for this PR — it's the same flag combination already established in lib/hook-utils.sh's hook::expand_8dot3 (cygpath -l -m, lib/hook-utils.sh#L438-L439) and the paired-path check (cygpath -lm, lib/hook-utils.sh#L650-L651). -l requires -w/-m per cygpath's own contract, so the pairing is correct usage, not a novel risk.
• || exit 2 fails loud on a cygpath failure (including "not found") before anything is printed, matching Rule 4 of the convention doc it cites.
• The relative link to docs/conventions/windows-path-emit/README.md resolves correctly from the file's location, and Rules 3–4 as quoted in the new bullet accurately reflect that doc's text.
• Reusing the single converted mixed-form value for both the Write tool and the later Bash block (rm -rf, the helper invocation) is a deliberate call against Rule 2's "convert at the boundary, not the source" — but it's the right call here: Git Bash/MSYS accepts C:/... mixed-form paths transparently in [[ -f ]]/cat/rm, so there's no half-converted split, only one crossing point for both consumers.
• The eval file's expectation (plugins/source-control/skills/worktree/evals/evals.json) is confirmed form-agnostic — it only checks for a mktemp -d directory and the Write-tool handoff pattern, not path spelling — so it doesn't need updating alongside this fix, as the PR description claims.

Author-claimed verification (live Git Bash run, check-shell-portability.sh, check-changed-skills.sh, markdownlint-cli2, affected-tests.sh) is noted but not independently re-run in this review pass.

[claude]

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

---

Security review — /review:security-review

• Gather context (PR diff, changed file)
• Apply skip-gate check
• Post final result

Result: no security-relevant surface — skip gate applies.

The change is confined to plugins/source-control/skills/worktree/context/create.md, converting a mktemp -d path to Windows mixed form before it's used as a Write tool file_path. Reviewed for the things this lane hunts (logic/trust-boundary flaws, injection, credential handling, dangerous Actions patterns):

• root_dir originates from mktemp -d output — not user- or attacker-influenced input, so there's no injection surface to exploit via the cygpath call.
• cygpath -m -l -- "$root_dir" quotes the variable and uses -- to block option injection even in a hypothetical adversarial-path scenario.
• || exit 2 is a fail-loud path (no silent fallback to an unconverted/unsafe value) — a defensive improvement, not a new gap.
• No GitHub Actions workflow, permissions, secrets handling, or dependency pins are touched — this is a markdown instruction file for an agent skill, not executable CI config.
• The pre-existing shell-injection note in this same file about ${user_config.worktree_root} (handled via the Write tool rather than shell interpolation) is unchanged by this diff and out of scope for this review.

No security findings to report for this PR.

[github-actions]

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

[github-actions]

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65195aee49

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

---

Security review — /review:security-review

• Gather context (PR diff, changed files, prior review state)
• Apply skip gate
• Post final result

Result: skip gate applies — no new security-relevant surface since the last completed review.

The last security review on this PR ran at head 65195aee and covered the only security-relevant change in this PR: plugins/source-control/skills/worktree/context/create.md (converting a mktemp -d path to Windows mixed form before it feeds the Write tool). That review found no security findings: root_dir originates from mktemp -d, not attacker-controlled input; cygpath -m -l -- "$root_dir" is properly quoted with -- to block option injection; || exit 2 fails loud instead of silently falling back to an unsafe path; no GitHub Actions workflow, permissions, or secrets handling is touched.

The commits added since that review (65195ae → c9230f4) only touch:

• plugins/source-control/.claude-plugin/plugin.json — version bump 0.55.31 → 0.55.32
• plugins/source-control/CHANGELOG.md — matching changelog entry

Both are metadata/documentation with no logic, trust-boundary, injection, credential, or Actions-workflow surface. This addresses the Codex P1 comment from the prior commit (publish the fix under a new plugin version), and introduces nothing new to review.

No security findings to report for this PR.

[claude]

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

---

Code review complete

• Gather context (PR diff, changed file, /review:code-review criteria)
• Review changed files for correctness and maintainability
• Post findings

No blocking findings.

Reviewed the current HEAD (c9230f4), which includes both commits: the create.md conversion fix and the follow-up version/changelog bump.

• The new snippet (create.md L87-93) matches the established cygpath-conversion precedent exactly: the case "${OSTYPE:-}" in msys* | cygwin* | win32) gate is byte-identical to scripts/emit-windows-path.sh#L96-L97, and cygpath -m -l combining mixed-form with 8.3 expansion mirrors lib/hook-utils.sh's hook::expand_8dot3 / paired-path-strip call sites — not novel usage.
• root_dir="$(cygpath ...)" || exit 2 correctly propagates cygpath's exit status through the assignment (bash sets $? from the command substitution), so a missing/failing cygpath fails loud with nothing printed to stdout, per Rule 4 of the windows-path-emit convention it cites.
• Verified the relative link ../../../../../docs/conventions/windows-path-emit/README.md resolves correctly from create.md's location (5 levels up from skills/worktree/context/ reaches repo root).
• Verified evals/evals.json is indeed form-agnostic (checks for the mktemp -d + Write-tool handoff pattern, not path spelling), confirming the PR's claim that it didn't need a matching update.
• The Codex P1 finding on this PR ("publish under a new plugin version") is already resolved in the current HEAD — plugin.json is bumped to 0.55.32 with a matching CHANGELOG.md entry.

One non-blocking observation: the inlined snippet doesn't carry the reference helper's [[ -n "$converted" ]] empty-output guard (scripts/emit-windows-path.sh#L112) — if cygpath ever exited 0 with empty output, root_dir would go empty and the later rm -rf "<root_dir>" would become a silent no-op instead of erroring. mktemp -d always hands cygpath a real, just-created directory, so this is very unlikely to fire in practice; not blocking.

Author-claimed verification (live Git Bash run, check-shell-portability.sh, check-changed-skills.sh, markdownlint-cli2, affected-tests.sh) is noted but not independently re-run in this pass — this lane's tool grant doesn't include arbitrary script execution.

[claude]

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

---

/review:code-review — CI code-review lane

• Gather context (PR diff, changed files, prior review state, /review:code-review criteria)
• Apply skip gate
• Post result

Result: skip gate applies — no new review surface since the last completed review from this lane.

This PR was already reviewed by this lane at head c9230f4 (see the comment posted 2026-08-31T05:07:35Z), which covered both the create.md mktemp/cygpath conversion fix and the version-bump/changelog commit that addressed the Codex P1 finding, and found no blocking issues.

The only commit added since then (9f4a700) does not touch create.md at all — it only renumbers the version:

• plugin.json: 0.55.32 → 0.55.34
• CHANGELOG.md: the ## [0.55.32] entry becomes ## [0.55.34], body text unchanged

This is a mechanical rebase renumbering: origin/main published 0.55.32 and 0.55.33 while this branch was in flight, so the original 0.55.32 bump would have collided (PUBLISHED VERSION REUSE). Confirmed origin/main's plugin.json is currently at 0.55.33, so 0.55.34 is next-available and non-colliding — the bump is correct.

Since the content already reviewed is unchanged and the delta is a trivial, non-colliding version renumber with no logic to evaluate, there is nothing new to flag. Standing non-blocking note from the prior review still applies for context: the inlined snippet omits the reference helper's empty-output guard ([[ -n "$converted" ]]) after cygpath, so a cygpath that exited 0 with empty output would make the later rm -rf "$root_dir" a silent no-op — extremely unlikely since mktemp -d always hands cygpath a real, just-created directory. Not blocking.

No new findings to report for this PR.

[claude]

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

---

Security review — /review:security-review

• Gather context (PR diff, changed files, prior review state)
• Apply skip gate
• Post final result

Result: skip gate applies — no new security-relevant surface since the last completed review.

The last security review on this PR ran at head c9230f4 and covered the security-relevant change: plugins/source-control/skills/worktree/context/create.md (converting a mktemp -d path to Windows mixed form before it feeds the Write tool). That review found no security findings: root_dir originates from mktemp -d, not attacker-controlled input; cygpath -m -l -- "$root_dir" is quoted with -- to block option injection; || exit 2 fails loud instead of silently falling back to an unsafe value; no GitHub Actions workflow, permissions, or secrets handling is touched.

The commits added since that review (c9230f4 → 9f4a700) only touch:

• plugins/source-control/.claude-plugin/plugin.json — version bump 0.55.33 → 0.55.34
• plugins/source-control/CHANGELOG.md — matching changelog entry

Both are metadata/documentation with no logic, trust-boundary, injection, credential, or Actions-workflow surface (this is the marketplace release commit publishing the fix under a new plugin version — the version bump requested by the earlier Codex P1 finding). Nothing new to review.

No security findings to report for this PR.