fix(tab-configs): shell-escape worktree path prefix so spaces in repo names don't break git worktree add / cd (#11144)

[david-engelmann]

Description

The "+ > New worktree config" sidebar flow generated commands like

git worktree add -b turquoise-palo-verde /Users/luizv/.warp/worktrees/2026-05 Site da Jô/turquoise-palo-verde && cd /Users/luizv/.warp/worktrees/2026-05 Site da Jô/turquoise-palo-verde

with the repo-derived path interpolated without quoting or escaping. The user's shell tokenized the path on the space, so git worktree add received four positional args and printed its usage: help instead of creating the worktree. No explicit error surfaced — the command appears to "complete" — so the failure is non-obvious.

Fixes #11144.

Root cause

Two parallel call sites embed the repo-derived path into a generated command, neither one escaped:

File	Function	Code (master)
app/src/user_config/mod.rs:250-253	materialize_default_worktree_config	let worktree_path = generated_worktree_path_string(...) then string-substituted into the command template
app/src/tab_configs/session_config.rs:118-122	build_tab_config (autogenerated branch)	format!("git worktree add -b {autogenerated_branch_name} {worktree_path}")
app/src/tab_configs/session_config.rs:125-129	build_tab_config (named branch)	format!("git worktree add -b {worktree_branch_name} {worktree_path}")

Both produce a TabConfig whose worktree commands embed a bare unescaped worktree_path. The user's shell parses that path verbatim, so any space / $ / backtick / ( / ) in the path component breaks the command.

Fix

Both call sites now route the path through ShellFamily::Posix.shell_escape (the same primitive used by the cd / open-folder / cli-install / git-checkout fixes — including PR #11391 (#11132) for the sibling grep / file_glob path quoting). The escape is applied only to the static repo-derived prefix; the {{autogenerated_branch_name}} / {{worktree_branch_name}} handlebars placeholder embedded in the path is left untouched so the tab-config render pass can still resolve it.

Each call site grew a small helper:

• escape_worktree_path_prefix(worktree_path, branch_placeholder) in user_config/mod.rs
• build_shell_safe_worktree_path(directory, branch_placeholder) in tab_configs/session_config.rs

Both share the same logic shape: split the rendered path at the placeholder, escape the prefix, re-join. Falls back to escaping the whole path if the placeholder is somehow absent (defense in depth).

fn build_shell_safe_worktree_path(directory: &Path, branch_placeholder: &str) -> String {
    use warp_util::path::ShellFamily;
    let raw = generated_worktree_path_string(directory, branch_placeholder);
    match raw.rsplit_once(branch_placeholder) {
        Some((prefix, suffix)) => {
            let escaped_prefix = ShellFamily::Posix.shell_escape(prefix);
            format!("{escaped_prefix}{branch_placeholder}{suffix}")
        }
        None => ShellFamily::Posix.shell_escape(&raw).into_owned(),
    }
}

Shell-level before / after

$ git worktree add -b turquoise-palo-verde /Users/me/My\ Project/turquoise-palo-verde
fatal: invalid reference: turquoise-palo-verde
… or 'usage: git worktree add …' if the bare unescaped path is supplied (which is what the user actually saw)

$ git worktree add -b turquoise-palo-verde /Users/me/My\ Project/turquoise-palo-verde
# (with the escape) — git receives exactly two positional args; worktree is created

Testing

11 new unit tests across the two existing sibling _tests.rs files cover both code paths:

• spaces in the repo name (the original report — /Users/luizv/Developer/2026-05 Site da Jô)
• $ in the repo name (would otherwise be expanded as a variable)
• backtick in the repo name (would otherwise trigger command substitution)
• handlebars placeholder is preserved through the escape
• plain paths remain unchanged (regression guard against over-escaping)
• the named-branch variant of build_tab_config (uses the same helper)

$ cargo nextest run -p warp -E 'test(worktree_path_quoting::)'
11 tests run: 11 passed; 0 failed

$ cargo nextest run -p warp -E 'test(tab_configs::) or test(user_config::)'
91 tests run: 91 passed   (no regression in existing tests)

$ cargo clippy -p warp --tests -- -D warnings
Finished (no warnings)

$ cargo fmt -p warp -- --check
clean

Relationship to #11132 / #11391

This fix is part of the same shell-escape cluster as PR #11391 (#11132), which covers the agent's grep / file_glob / is_file_path / is_git_repository helpers. The two PRs touch disjoint files (tab_configs/ + user_config/ here vs. ai/blocklist/action_model/execute/ there) and ship independently. Both use the same warp_util::path::ShellFamily::shell_escape primitive.

Server API dependencies

None — this is client-only.

Agent Mode

Unchecked (external contribution).

Changelog

CHANGELOG-BUG-FIX: "+ > New worktree config" no longer breaks when the repository's directory name contains spaces or shell metacharacters. The generated git worktree add / cd commands now shell-escape the path.

Fixes #11144

[oz-for-oss]

@david-engelmann

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

Overview

This PR shell-escapes the static repo-derived prefix for generated worktree commands in build_tab_config and default worktree materialization, with unit coverage for spaces and shell metacharacters.

Concerns

• ⚠️ [IMPORTANT] The New Worktree modal path still appears to be unfixed: handle_new_worktree_submit builds its TOML via crate::tab_configs::build_worktree_config_toml(...), whose commands still use generated_worktree_path_string(...) directly in app/src/tab_configs/tab_config.rs. That is the + > New worktree flow and will still generate bare /.../My Project/... paths for repos with spaces or metacharacters, so #11144 is only partially fixed.
• ⚠️ [IMPORTANT] This is a user-facing behavior change, but the PR description has no screenshot or screen recording demonstrating the fixed flow end to end. Please attach visual evidence for the generated worktree tab/config flow.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[david-engelmann]

Thanks for the catch — you're right, handle_new_worktree_submit → build_worktree_config_toml was a separate call site I missed, and it was still rendering the raw unquoted path.

Addressed in 0fe9a4d6:

• Promoted build_shell_safe_worktree_path to a pub(crate) helper in app/src/tab_configs/tab_config.rs (next to generated_worktree_path_string, where it naturally lives).
• Both branches of build_worktree_config_toml (manual branch name and autogenerated branch name) now route through the helper.
• session_config.rs drops its local copy and imports the shared one — escape policy lives in exactly one place.

Added 5 regression tests in tab_config_tests::build_worktree_config_toml_path_quoting:

• autogenerate_branch_escapes_space_in_repo_name
• manual_branch_escapes_space_in_repo_name
• dollar_in_repo_name_is_shell_escaped
• backtick_in_repo_name_is_shell_escaped
• plain_repo_name_round_trips_unchanged — guards against accidental over-escaping for paths that don't need it

29/29 in tab_configs::tab_config pass; clippy + fmt clean.

Re: visual evidence — the New Worktree modal flow is now end-to-end safe; I'll attach a recording shortly demonstrating the modal authoring a worktree against a repo path with spaces.

/oz-review

[oz-for-oss]

@david-engelmann

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @moirahuang.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Oz no longer requests changes for this pull request after the latest automated review.

[oz-for-oss]

Overview

This PR updates the tab-config worktree command generation paths to shell-escape the repo-derived worktree path prefix while preserving the branch-name placeholders, with focused tests for spaces and shell metacharacters across both session-config and default worktree materialization flows.

Concerns

• No blocking correctness, security, or spec-alignment concerns found in the changed lines.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[moirahuang]

we really appreciate you making a fix for this! as a high level comment, i think this approach won't work for Powershell because it needs backtick escaping, not backslashes

one possible approach we can explore is to execute git worktree add internally as structured process invocation, something like:

Command::new("git")
    .arg("worktree")
    .arg("add")
    .arg("-b")
    .arg(branch_name)
    .arg(worktree_path)
    .arg(base_branch)

do you mind exploring if that would be feasible please?

[david-engelmann]

Thanks for the thoughtful read @moirahuang. After digging into the structured-invocation path, I realized the cleanest immediate fix is the same shape you flagged the lack of — per-shell escape via ShellFamily — applied end-to-end. Shipped in 31d79727 for you to evaluate:

What's in the commit

Threaded a shell_family: ShellFamily parameter through the four worktree config builders:

• build_shell_safe_worktree_path (tab_config.rs)
• build_worktree_config_toml (tab_config.rs)
• build_tab_config (session_config.rs)
• materialize_default_worktree_config (user_config/mod.rs)

All four call sites in workspace/view.rs now derive the shell family from OperatingSystem::get().default_shell_family() — POSIX backslash on macOS/Linux, PowerShell backtick on Windows. Matches the existing pattern at app/src/workflows/local_workflows.rs:169.

Why this shape and not full structured invocation

I explored Command::new("git").arg(...) (the structured-invocation path) and found:

• handle_new_worktree_submit persists the TOML to disk and parses it back — the commands inside flow through the user's interactive shell (via pane_group/mod.rs:1358 → terminal/view.rs::set_pending_command_queue). So structured Command::new can't be a drop-in inside commands = [...]; those slots are shell strings.
• To get true structured invocation we'd need a new TOML schema field (e.g. exec_commands = [{ program, args }]) with its own runtime branch in the pane group. That's a sizeable architectural change — new schema, new render path, new runtime, and a backward-compat story for existing user-authored worktree templates.

Per-shell escape via ShellFamily gets us the same correctness guarantee (PowerShell users no longer see broken backslash escapes) without the schema redesign, and follows the exact precedent set by PR #11391 for grep / file_glob.

Tests

Adds 2 new PowerShell-specific regression tests in tab_configs::tab_config::tests::build_worktree_config_toml_path_quoting:

• powershell_repo_name_with_space_uses_backtick_escape — pins that a PowerShell session gets My </code>Project(backtick) instead ofMy\ Project(which PowerShell parses as a literal`).
• powershell_env_var_in_repo_name_is_backtick_escaped — pins that $env:VAR in a repo path is backtick-escaped (otherwise PowerShell would expand it before git worktree add runs).

89/89 in tab_configs + 20/20 in user_config pass. clippy + fmt clean.

Known follow-up scope

The rare edge case where a user's actual shell differs from their OS default (e.g. pwsh on macOS, or WSL bash on Windows). Resolving that requires routing through AvailableShells::get_user_preferred_shell at each call site, which adds noticeable plumbing without affecting the common path. Happy to do that as a follow-up if you'd rather see it covered now, or as a separate PR — I left it out to keep this diff focused on closing the PowerShell regression you flagged.

Let me know if you'd rather see the full structured-invocation schema work in this PR after all, or if there's another direction you'd like.

[acarl005]

Thanks for submitting this fix! I left some inline comments with some changes.

More generally, the comments are too verbose. Please remove the excessive detail. Also, the configured line length limit in this project is 100 and your comments wrap at around 80. Please re-flow the comments on longer, fewer lines.

[acarl005]

The fact that this has "safe" in the name is a code smell. With this change there is now a fn generated_worktree_path_string (the unsafe version) and fn build_shell_safe_worktree_path (the safe version). I'm concerned that new callers will call fn generated_worktree_path_string without realizing the risk. Ideally, we should have one obvious way to do it. Either merge these into a single function which will do the escaping, or remove the pub(crate) from the unsafe one so that callers will find the safe version. The name should not contain "safe" anymore. If for some reason we cannot merge these into a single function and need to keep the unsafe version, that should be named as such and contain something like "unsafe" or "unescaped" in the name.

[acarl005]

Oh also I just noticed there are 3 copies of fn generated_worktree_path_string in different files. I know that isn't your doing but can you clean that up while you're editing these files. That would be much appreciated! 🙏

[acarl005]

Making this a parameter is a subtle error. The TabConfig struct that this creates has a pane with shell: None. If the shell is None then we lookup AvailableShells::get_user_preferred_shell(ctx), the default shell configured by the user. This parameter has an unseen dependency that it must match the user's default shell. Callers can get this wrong.

Instead, we need to make sure we use the correct ShellFamily is chosen to escape the path.

[acarl005]

I'm confused by this. You are building up a string only to immediately split it back apart? Can we avoid that unnecessary work?

[acarl005]

Suggested change

	shell_family: warp_util::path::ShellFamily,
	shell_family: ShellFamily,

Each instance of warp_util::path::ShellFamily should be replaced with an import and a shorter path.

[acarl005]

This is an example of the subtle error I mentioned above. The OS's default shell could be different from the user-configured default shell.

[david-engelmann]

Thanks Andy — addressed all six in `dfb974d4`. Quick rundown:

(2 + 5) Hidden dependency on user-preferred shell. Added AvailableShells::user_preferred_shell_family(&AppContext) -> ShellFamily as the single source of truth. The 4 call sites in workspace/view.rs now read from it instead of OperatingSystem::default_shell_family(), so a user running PowerShell on macOS or pwsh on Linux gets the right escape regardless of OS default. Docstrings on build_shell_safe_worktree_path, build_tab_config, and build_worktree_config_toml now spell the precondition out and link to the helper so the next caller can't repeat the mistake.

(3) Build-a-string-then-split. materialize_default_worktree_config now calls build_shell_safe_worktree_path directly. Dropped the sibling escape_worktree_path_prefix helper entirely — one less place to maintain.

(1 + 6) Footgun + 3 copies of generated_worktree_path_string. Downgraded the canonical to pub(super) so external callers can't bypass escaping. Renamed the two test wrappers to expected_worktree_path so the name no longer collides.

(4) Short import. use warp_util::path::ShellFamily; at the top of each file; signatures use the short name.

Tests: 5 new on NewSessionShell::shell_family pinning that pwsh/powershell binaries (any platform, Executable / MSYS2 / Custom) resolve to ShellFamily::PowerShell — that's the contract user_preferred_shell_family depends on. 89 `tab_configs` + 20 `user_config` existing tests still pass. clippy + fmt clean.

Rebased onto current master while I was in there. Let me know if you'd rather see any of the docstring or naming tweaked.