fix: always bash-c wrap commands when wrapper is configured#430
Closed
borng wants to merge 4 commits intoasheshgoplani:mainfrom
Closed
fix: always bash-c wrap commands when wrapper is configured#430borng wants to merge 4 commits intoasheshgoplani:mainfrom
borng wants to merge 4 commits intoasheshgoplani:mainfrom
Conversation
…linking FilterByQuery now matches against inst.ID, enabling the TUI's `/` search to find sessions by their agent-deck instance ID (e.g., "be51eb02"). This is the Go-side piece of the click-to-resume notification flow: notify.sh embeds ?ad=<instance_id> in the focusterminal:// URL, the AHK handler focuses the Agent Deck tab and types the ID into search. Created by gregb and his home-grown crew of builders 🦜 🤖
When a session has a wrapper configured (e.g., "osc8-shorten {command}"),
the naive {command} substitution produced broken shell commands because
compound expressions (with &&, ;) caused the wrapper to only wrap the
first segment. For example:
osc8-shorten export FOO=bar && exec claude --session-id "..."
Only "osc8-shorten export FOO=bar" ran through the wrapper; "exec claude"
ran separately, unwrapped.
Fix: prepareCommand() now pre-wraps compound commands in bash -c before
applying the wrapper, producing:
osc8-shorten bash -c 'export FOO=bar && exec claude --session-id "..."'
The tmux Start() method skips its own bash -c wrapping (via SkipBashCWrap
flag) to avoid double-wrapping.
Also:
- Add Wrapper field to ClaudeSettings so [claude].wrapper in config.toml
propagates to new sessions automatically
- New sessions inherit wrapper from [claude] config when no explicit
wrapper is provided via --wrapper flag
Created by gregb and his home-grown crew of builders 🦜 🤖
When a wrapper is set in config.toml (e.g. wrapper = "direnv exec ."), wrappers use execvp() which cannot interpret shell syntax. The previous conditional check (commandNeedsBashCWrap) missed cases where inline env var prefixes like AGENTDECK_INSTANCE_ID=xxx had no accompanying shell operators (&&, $(), etc.), causing them to be passed as literal argv to the wrapper instead of being interpreted by the shell. Fix: wrap unconditionally when any wrapper is configured, remove the now-dead commandNeedsBashCWrap function. Created by gregb and his home-grown crew of builders 🦜 🤖
Author
|
lemme know if there's anything else I needed to do to contrib properly. Didn't update docs since was a bugfix... have a few more things would like to add. |
Author
|
too many commits, going to simplify the PR to just one: |
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
1 participant
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
Context
The wrapper field in config.toml wraps agent commands with executables like direnv exec. These use execvp() which cannot interpret shell syntax. The previous heuristic missed cases where inline env var prefixes existed without &&, or ;, causing them to be passed as literal argv to the wrapper.
Test plan