fix: shell precedence, missing escaping, and license badge drift#1584
Open
worktrunk-bot wants to merge 2 commits intomainfrom
Open
fix: shell precedence, missing escaping, and license badge drift#1584worktrunk-bot wants to merge 2 commits intomainfrom
worktrunk-bot wants to merge 2 commits intomainfrom
Conversation
…r hint, license badge Three fixes found during nightly code quality survey: 1. build_remove_command (process.rs): The `|| true` for fsmonitor stop had incorrect shell operator precedence — it could swallow failures from the entire left chain. Wrapped in brace group to scope correctly. 2. WorktreePathOccupied hint (error.rs): Branch name and path in the suggested `cd ... && git switch ...` command were not shell-escaped, unlike other error hints that use shell_escape::escape(). 3. README license badge: Said "MIT" but Cargo.toml declares "MIT OR Apache-2.0". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
worktrunk-bot
commented
Mar 17, 2026
Collaborator
Author
worktrunk-bot
left a comment
There was a problem hiding this comment.
Shell precedence fix and license badge are correct. One issue with the escaping fix:
format_path_for_display converts absolute paths to tilde-relative (/home/user/repo → ~/repo). shell_escape::escape treats ~ as unsafe (not in [A-Za-z0-9@%+=:,./-]), so ~/repo becomes '~/repo' — which prevents tilde expansion and breaks the cd command.
Fix: escape the raw path instead of path_display for the command, while keeping path_display for the descriptive text.
…hint format_path_for_display converts absolute paths to tilde-relative form (~/repo), but shell_escape quotes the ~ character, breaking tilde expansion in the suggested cd command. Use the raw path instead. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary
Three fixes found during nightly code quality survey:
build_remove_command(process.rs): The|| truefor fsmonitor stop had incorrect precedence — it could swallow failures from the entiresleep 1 && git ...chain. Wrapped in{ ...; }brace group to scope correctly.WorktreePathOccupiedhint (error.rs): Branch name and path in the suggestedcd ... && git switch ...command were not shell-escaped, unlike other error hints that useshell_escape::escape().Cargo.tomldeclares "MIT OR Apache-2.0".Also opened issues for items that need more design consideration:
wt step pushmissing--no-ffflagget_config()swallows all git errorsget_*naming convention violationsTest plan
test_build_remove_command,snapshot_worktree_path_occupied)cargo fmtandcargo clippypass🤖 Generated with Claude Code