Fix bash bootstrap dropping shell_plugins from the Bootstrapped payload - #15518
Merged
Conversation
bash computed escaped_shell_plugins from the shell_plugins array (used today to tag Starship), but the primary (non-MSYS2) Bootstrapped JSON payload never actually included that field, so the client never learned about any bash shell_plugins tags. Separately, the value being escaped, "$shell_plugins" (a bash array used in scalar context), only expands to the array's first element, so even the MSYS2 kv-pairs path (which did send the field) would drop everything but one tag whenever more than one was set. Fix both: join the array into a newline-separated list (matching zsh's shell_plugins reporting) before escaping, and add the missing key to the primary JSON payload. Also switch the MSYS2 kv-pair to the escaped variant, matching how other multi-value fields (aliases, builtins, etc.) are sent there. Found while building CORE-3807 (prototype ctrl-r external history widget handoff), which depends on shell_plugins actually reaching the client, but this is an independent, pre-existing bug: Starship detection has silently never reached the client for bash users.
The line it describes says what it does.
acarl005
marked this pull request as ready for review
August 26, 2026 03:46
Contributor
Author
|
This PR was generated with Warp. Comment    |
Contributor
|
/oz-review |
Contributor
|
I'm re-reviewing this pull request in response to a review 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 |
![warp-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR fixes bash bootstrap serialization so shell_plugins is newline-joined, escaped consistently, and included in the primary Bootstrapped JSON payload. No approved spec context was provided, and the security pass did not identify any findings.
Concerns
- The bash shell plugin bootstrap regression is only covered by
bash -n; there is no automated behavior test that would fail ifshell_pluginsis dropped from the Bootstrapped payload again.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| local escaped_shell_path="$(warp_escape_json "$BASH")" | ||
| local escaped_cdpath="$(warp_escape_json "$CDPATH")" | ||
| local escaped_json="{\"hook\": \"Bootstrapped\", \"value\": {\"histfile\": \"$escaped_histfile\", \"session_id\": $WARP_SESSION_ID, \"shell\": \"bash\", \"home_dir\": \"$HOME\", \"user\":\"$_user\", \"host\":\"$_hostname\", \"path\": \"$escaped_path\", \"cdpath\": \"$escaped_cdpath\", \"editor\": \"$escaped_editor\", \"env_var_names\": \"$escaped_env_var_names\", \"abbreviations\": \"$escaped_abbrs\", \"aliases\": \"$escaped_aliases\", \"function_names\": \"$escaped_function_names\", \"builtins\": \"$escaped_builtins\", \"keywords\": \"$escaped_keywords\", \"shell_version\": \"$BASH_VERSION\", \"shell_options\": \"$escaped_shell_options\", \"rcfiles_start_time\": \"$rcfiles_start_time\", \"rcfiles_end_time\": \"$rcfiles_end_time\", \"vi_mode_enabled\": \"$vi_mode_enabled\", \"os_category\": \"$os_category\", \"linux_distribution\": \"$linux_distribution\", \"wsl_name\": \"$WSL_DISTRO_NAME\", \"shell_path\": \"$escaped_shell_path\"}}" | ||
| local escaped_json="{\"hook\": \"Bootstrapped\", \"value\": {\"histfile\": \"$escaped_histfile\", \"session_id\": $WARP_SESSION_ID, \"shell\": \"bash\", \"home_dir\": \"$HOME\", \"user\":\"$_user\", \"host\":\"$_hostname\", \"path\": \"$escaped_path\", \"cdpath\": \"$escaped_cdpath\", \"editor\": \"$escaped_editor\", \"env_var_names\": \"$escaped_env_var_names\", \"abbreviations\": \"$escaped_abbrs\", \"aliases\": \"$escaped_aliases\", \"function_names\": \"$escaped_function_names\", \"builtins\": \"$escaped_builtins\", \"keywords\": \"$escaped_keywords\", \"shell_version\": \"$BASH_VERSION\", \"shell_options\": \"$escaped_shell_options\", \"rcfiles_start_time\": \"$rcfiles_start_time\", \"rcfiles_end_time\": \"$rcfiles_end_time\", \"shell_plugins\": \"$escaped_shell_plugins\", \"vi_mode_enabled\": \"$vi_mode_enabled\", \"os_category\": \"$os_category\", \"linux_distribution\": \"$linux_distribution\", \"wsl_name\": \"$WSL_DISTRO_NAME\", \"shell_path\": \"$escaped_shell_path\"}}" |
Contributor
There was a problem hiding this comment.
STARSHIP_SHELL=bash and asserts the bootstrapped session includes the starship shell plugin so this payload field cannot be dropped again.
acarl005
enabled auto-merge (squash)
August 26, 2026 16:34
acarl005
approved these changes
Aug 26, 2026
iamwavecut
pushed a commit
to iamwavecut/warp
that referenced
this pull request
Aug 27, 2026
…ad (warpdotdev#15518) ## Description `bash_body.sh` computes `escaped_shell_plugins` from the `shell_plugins` array (currently used to tag Starship), but the primary (non-MSYS2) `Bootstrapped` JSON payload never actually includes that field — so the client never learns about any bash `shell_plugins` tags. Separately, the value being escaped, `"$shell_plugins"` (a bash array referenced in scalar context), only expands to the array's first element, so even the MSYS2 kv-pairs path (which did send the field) would silently drop everything but one tag whenever more than one was set. This is a pre-existing, independent bug — not something introduced by this PR. It means Starship detection for bash has silently never reached the client (`session.shell().plugins()` is always empty for bash sessions, regardless of what the bootstrap script actually detects). ## Fix - Join the `shell_plugins` array into a newline-separated list (matching zsh's own `shell_plugins` reporting, which uses `print -l --`) before escaping. - Add the missing `shell_plugins` key to the primary (non-MSYS2) `Bootstrapped` JSON payload. - Switch the MSYS2 kv-pair line to `warp_send_hook_kv_pair_escaped`, matching how other multi-value fields (`aliases`, `builtins`, `function_names`, etc.) are already sent on that path. ## Context Found while building [CORE-3807](https://linear.app/warpdotdev/issue/CORE-3807/prototype-hand-ctrl-r-to-fzfatuin-when-shell-history-widgets-are) (prototype ctrl-r external-history-widget handoff, see warpdotdev#15513), which depends on `shell_plugins` actually reaching the client for its own new `external_ctrl_r_history` tag. That PR currently duplicates this same fix on its own branch; once this lands, I'll rebase that branch on top of this fix (or drop the duplicate diff) so the change isn't reviewed twice. ## Testing - `bash -n app/assets/bundled/bootstrap/bash_body.sh` (syntax check). - Config/data-only bootstrap-script change — no Rust code affected, so no `cargo` tests apply. Verified by inspection that the resulting JSON payload now includes `shell_plugins` with all tags newline-joined, matching zsh's existing wire format (`parse_shell_options_list` on the Rust side splits on whitespace, so either newline- or space-separated tags parse identically). --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Chiiz0
pushed a commit
to Infinimesh-ai/InfiniShell-Desktop
that referenced
this pull request
Aug 29, 2026
…ad (warpdotdev#15518) ## Description `bash_body.sh` computes `escaped_shell_plugins` from the `shell_plugins` array (currently used to tag Starship), but the primary (non-MSYS2) `Bootstrapped` JSON payload never actually includes that field — so the client never learns about any bash `shell_plugins` tags. Separately, the value being escaped, `"$shell_plugins"` (a bash array referenced in scalar context), only expands to the array's first element, so even the MSYS2 kv-pairs path (which did send the field) would silently drop everything but one tag whenever more than one was set. This is a pre-existing, independent bug — not something introduced by this PR. It means Starship detection for bash has silently never reached the client (`session.shell().plugins()` is always empty for bash sessions, regardless of what the bootstrap script actually detects). ## Fix - Join the `shell_plugins` array into a newline-separated list (matching zsh's own `shell_plugins` reporting, which uses `print -l --`) before escaping. - Add the missing `shell_plugins` key to the primary (non-MSYS2) `Bootstrapped` JSON payload. - Switch the MSYS2 kv-pair line to `warp_send_hook_kv_pair_escaped`, matching how other multi-value fields (`aliases`, `builtins`, `function_names`, etc.) are already sent on that path. ## Context Found while building [CORE-3807](https://linear.app/warpdotdev/issue/CORE-3807/prototype-hand-ctrl-r-to-fzfatuin-when-shell-history-widgets-are) (prototype ctrl-r external-history-widget handoff, see warpdotdev#15513), which depends on `shell_plugins` actually reaching the client for its own new `external_ctrl_r_history` tag. That PR currently duplicates this same fix on its own branch; once this lands, I'll rebase that branch on top of this fix (or drop the duplicate diff) so the change isn't reviewed twice. ## Testing - `bash -n app/assets/bundled/bootstrap/bash_body.sh` (syntax check). - Config/data-only bootstrap-script change — no Rust code affected, so no `cargo` tests apply. Verified by inspection that the resulting JSON payload now includes `shell_plugins` with all tags newline-joined, matching zsh's existing wire format (`parse_shell_options_list` on the Rust side splits on whitespace, so either newline- or space-separated tags parse identically). --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com>
TranscriptionFactory
pushed a commit
to TranscriptionFactory/warp
that referenced
this pull request
Aug 30, 2026
…ad (warpdotdev#15518) `bash_body.sh` computes `escaped_shell_plugins` from the `shell_plugins` array (currently used to tag Starship), but the primary (non-MSYS2) `Bootstrapped` JSON payload never actually includes that field — so the client never learns about any bash `shell_plugins` tags. Separately, the value being escaped, `"$shell_plugins"` (a bash array referenced in scalar context), only expands to the array's first element, so even the MSYS2 kv-pairs path (which did send the field) would silently drop everything but one tag whenever more than one was set. This is a pre-existing, independent bug — not something introduced by this PR. It means Starship detection for bash has silently never reached the client (`session.shell().plugins()` is always empty for bash sessions, regardless of what the bootstrap script actually detects). - Join the `shell_plugins` array into a newline-separated list (matching zsh's own `shell_plugins` reporting, which uses `print -l --`) before escaping. - Add the missing `shell_plugins` key to the primary (non-MSYS2) `Bootstrapped` JSON payload. - Switch the MSYS2 kv-pair line to `warp_send_hook_kv_pair_escaped`, matching how other multi-value fields (`aliases`, `builtins`, `function_names`, etc.) are already sent on that path. Found while building [CORE-3807](https://linear.app/warpdotdev/issue/CORE-3807/prototype-hand-ctrl-r-to-fzfatuin-when-shell-history-widgets-are) (prototype ctrl-r external-history-widget handoff, see warpdotdev#15513), which depends on `shell_plugins` actually reaching the client for its own new `external_ctrl_r_history` tag. That PR currently duplicates this same fix on its own branch; once this lands, I'll rebase that branch on top of this fix (or drop the duplicate diff) so the change isn't reviewed twice. - `bash -n app/assets/bundled/bootstrap/bash_body.sh` (syntax check). - Config/data-only bootstrap-script change — no Rust code affected, so no `cargo` tests apply. Verified by inspection that the resulting JSON payload now includes `shell_plugins` with all tags newline-joined, matching zsh's existing wire format (`parse_shell_options_list` on the Rust side splits on whitespace, so either newline- or space-separated tags parse identically). --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com> (cherry picked from commit 607be8c)
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.
Description
bash_body.shcomputesescaped_shell_pluginsfrom theshell_pluginsarray (currently used to tag Starship), but the primary (non-MSYS2)BootstrappedJSON payload never actually includes that field — so the client never learns about any bashshell_pluginstags. Separately, the value being escaped,"$shell_plugins"(a bash array referenced in scalar context), only expands to the array's first element, so even the MSYS2 kv-pairs path (which did send the field) would silently drop everything but one tag whenever more than one was set.This is a pre-existing, independent bug — not something introduced by this PR. It means Starship detection for bash has silently never reached the client (
session.shell().plugins()is always empty for bash sessions, regardless of what the bootstrap script actually detects).Fix
shell_pluginsarray into a newline-separated list (matching zsh's ownshell_pluginsreporting, which usesprint -l --) before escaping.shell_pluginskey to the primary (non-MSYS2)BootstrappedJSON payload.warp_send_hook_kv_pair_escaped, matching how other multi-value fields (aliases,builtins,function_names, etc.) are already sent on that path.Context
Found while building CORE-3807 (prototype ctrl-r external-history-widget handoff, see #15513), which depends on
shell_pluginsactually reaching the client for its own newexternal_ctrl_r_historytag. That PR currently duplicates this same fix on its own branch; once this lands, I'll rebase that branch on top of this fix (or drop the duplicate diff) so the change isn't reviewed twice.Testing
bash -n app/assets/bundled/bootstrap/bash_body.sh(syntax check).cargotests apply. Verified by inspection that the resulting JSON payload now includesshell_pluginswith all tags newline-joined, matching zsh's existing wire format (parse_shell_options_liston the Rust side splits on whitespace, so either newline- or space-separated tags parse identically).