fix(tab-configs): shell-escape worktree path prefix so spaces in repo names don't break git worktree add / cd (#11144)#11392
Open
david-engelmann wants to merge 1 commit into
Conversation
…acharacters in repo names don't break `git worktree add` / `cd` (warpdotdev#11144) The "+ > New worktree config" sidebar flow generated commands like git worktree add -b turquoise-palo-verde /Users/x/.warp/worktrees/My Project/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 warpdotdev#11144. Two parallel call sites needed the same fix: * `app/src/user_config/mod.rs` — `materialize_default_worktree_config`, reached via the sidebar "+ > New worktree config" flow. Substitutes the repo-derived `worktree_path` into the rendered TOML commands. * `app/src/tab_configs/session_config.rs` — `build_tab_config`, the in-Rust-code worktree config builder used by the session-config modal. Same `format!`-into-command pattern. Both helpers 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 warpdotdev#11132's 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. To keep the change reviewable, 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). ## Tests 11 new unit tests across the two existing sibling `_tests.rs` files, covering both code paths: * spaces in the repo name (the original report) * `$` 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 $ 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 clean $ cargo fmt -p warp -- --check clean ``` Sibling PRs in the same shell-escape family: warpdotdev#11391 (warpdotdev#11132) covers the agent grep / file_glob / is_file_path helpers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
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 Powered by Oz |
Contributor
There was a problem hiding this comment.
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_submitbuilds its TOML viacrate::tab_configs::build_worktree_config_toml(...), whose commands still usegenerated_worktree_path_string(...)directly inapp/src/tab_configs/tab_config.rs. That is the+ > New worktreeflow 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The "+ > New worktree config" sidebar flow generated commands like
with the repo-derived path interpolated without quoting or escaping. The user's shell tokenized the path on the space, so
git worktree addreceived four positional args and printed itsusage: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:
app/src/user_config/mod.rs:250-253materialize_default_worktree_configlet worktree_path = generated_worktree_path_string(...)then string-substituted into the command templateapp/src/tab_configs/session_config.rs:118-122build_tab_config(autogenerated branch)format!("git worktree add -b {autogenerated_branch_name} {worktree_path}")app/src/tab_configs/session_config.rs:125-129build_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)inuser_config/mod.rsbuild_shell_safe_worktree_path(directory, branch_placeholder)intab_configs/session_config.rsBoth 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).
Shell-level before / after
Testing
11 new unit tests across the two existing sibling
_tests.rsfiles cover both code paths:/Users/luizv/Developer/2026-05 Site da Jô)$in the repo name (would otherwise be expanded as a variable)build_tab_config(uses the same helper)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_repositoryhelpers. The two PRs touch disjoint files (tab_configs/+user_config/here vs.ai/blocklist/action_model/execute/there) and ship independently. Both use the samewarp_util::path::ShellFamily::shell_escapeprimitive.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