Shell widget handoff: hand ctrl-r and ctrl-t to fzf/atuin (CORE-3807) - #15513
Open
warp-agent-staging[bot] wants to merge 76 commits into
Open
Shell widget handoff: hand ctrl-r and ctrl-t to fzf/atuin (CORE-3807)#15513warp-agent-staging[bot] wants to merge 76 commits into
warp-agent-staging[bot] wants to merge 76 commits into
Conversation
At an idle prompt, ctrl-r hands off to the shell's own external ctrl-r history widget (fzf or atuin) instead of opening Warp's command search, when bootstrap detects the shell has rebound ^R away from its default reverse-history-search widget. - zsh bootstrap generically detects a rebound ^R widget (not an fzf/atuin allowlist) and reports it via a new external_ctrl_r_history shell_plugins tag. A bootstrap-installed helper re-runs the detected tool's own picker command and reports the selection via a new ExternalCtrlRSelection DCS hook. - The client runs that helper through the normal command-execution path when the tag is present at an idle prompt, reusing the existing long-running-command machinery to hide the input editor and forward keystrokes to the picker's PTY-driven UI. - The selected command (or the prior buffer, on cancel) is restored into the input editor without executing, reusing the same buffer-restore path already used for prompt-chip commands like cd. - Gated behind a new FzfCtrlRHandoff feature flag (in DOGFOOD_FLAGS).
Contributor
Author
|
This PR was generated with Warp. Comment    |
…ff token, redact selection text, hide synthetic helper block - zsh_body.sh: narrow ctrl-r widget detection to exactly the tools warp_run_external_ctrl_r_widget can invoke (fzf/atuin), so an unsupported rebound widget falls through to Warp's normal command search instead of being tagged as invocable. Echo the handoff token back in the ExternalCtrlRSelection hook. - input.rs: require the ExternalCtrlRSelection hook's session_id and token to match an in-flight handoff before applying it, so an unsolicited or stale write to the pty can't alter the input editor. Hide the synthetic helper command's block once it completes so it doesn't clutter scrollback. - blocks.rs: add BlockList::hide_block, which hides a block and refreshes the block-heights sumtree so a completed (non-active) block's height update takes effect immediately. - event.rs / dcs_hooks.rs: redact the selected command text from Debug output in two places (Event::ExternalCtrlRSelection and ExternalCtrlRSelectionValue) so it can't leak into debug logs.
…ell_plugins reporting - Detect ctrl-r rebound to fzf's __fzf_history__ via bind -X and tag external_ctrl_r_history in shell_plugins, matching the pattern already used for zsh. atuin is intentionally not supported for bash: its integration binds ctrl-r indirectly through intermediate key sequences and depends on the readline widget-chain machinery, so it can't be invoked standalone the way __fzf_history__ can. - Add warp_run_external_ctrl_r_widget for bash, mirroring the zsh helper. - Fix a pre-existing bug where shell_plugins was computed but never actually included in bash's primary (non-MSYS2) Bootstrapped JSON payload, and where the bash array was collapsed to its first element instead of being joined into the newline-separated list the client expects.
update_blocks_and_sumtree doesn't send a wakeup event itself (its other callers are driven by a GPUI context that already notifies the right view), so the synthetic ctrl-r helper block stayed visible after completion despite its height being zeroed in the sumtree. Add the same explicit send_wakeup_event() call unhide_block uses. Verified with computer use: the helper block no longer appears in scrollback after either cancelling or selecting a history entry.
zsh: atuin's own zsh integration swaps stdout/stderr through fd 3 before invoking 'atuin search -i', because atuin writes its TUI to stdout and its cursor-position query has nothing to answer it under plain command substitution (stdout is a pipe), causing it to bail during startup with a blank list. The bare invocation previously used here hit exactly that failure. Also strip the '__atuin_accept__:' prefix atuin uses to signal that enter_accept fired, since we only ever want the selection, never to run it. bash: apply the same fd-3 fix, invoking 'atuin search' directly rather than atuin's own key-binding machinery. Detection needed a different approach than fzf's: atuin's bash integration binds ctrl-r through an intermediate key sequence and a widget-index dispatcher rather than a direct 'bind -x' on ctrl-r itself, so it doesn't show up in the 'bind -X' scan used for fzf. Detect it instead via atuin's own signal for whether it bound ctrl-r () plus confirming its integration is loaded. This doesn't affect invocation, which bypasses atuin's key-binding machinery entirely either way.
… (CORE-3807) Adds to the fish bootstrap the pieces fish needs for the ctrl-r handoff: - warp_external_ctrl_r_widget: generic detection of a rebound ^R via `bind \cr`, treating any non---preset binding as a user/plugin rebinding. - shell_plugins in the Bootstrapped hook, newline-separated like bash/zsh, carrying the external_ctrl_r_history tag. The client already parses this field for every shell, so no client change is needed. - warp_run_external_ctrl_r_widget: runs fzf's or atuin's picker as a plain foreground command and reports the selection over the ExternalCtrlRSelection DCS hook, never touching fish's `commandline` line buffer.
Third finding: detection matched any ctrl-r binding whose widget/command name merely contained "fzf" or "atuin" (zsh/bash/fish). An RC can legitimately bind ctrl-r to an unrelated fzf/atuin-flavored widget that isn't the history search we know how to invoke, which would hijack that binding and lose Warp's own command search too. Replace substring matching with an exact allowlist of each integration's canonical widget/function name in all three shells' detection and invocation dispatch. Fourth finding: bash's atuin detection relied on $__atuin_bind_ctrl_r, a flag set once during atuin's own init that a later `bind` in the RC can leave stale. Read the live `bind -X` binding for ctrl-r instead, matching only the exact __fzf_history__/__atuin_history function names. Newer atuin bash versions bind ctrl-r through an indirect widget-index dispatcher that isn't reliably distinguishable from an arbitrary user macro via bind -X alone; per the reviewer's explicit guidance we decline the handoff in that case (biasing to false negatives) rather than risk hijacking a rebound key. Also propagate the ctrl-r helper's history-exclusion (already present for zsh) to bash (HISTIGNORE) and fish (a composing fish_should_add_to_history wrapper), so the synthetic handoff invocation doesn't pollute the very history list this feature searches on the next ctrl-r.
The fish_should_add_to_history wrapper had two bugs, both stemming from treating "no backup exists" as a state to handle at call time instead of establishing the invariant at install time: 1. When no user-defined fish_should_add_to_history existed (the common case), warp_original_fish_should_add_to_history was never created, so the fallback branch's `functions -q warp_original...; and warp_original...` evaluated to false for every command. Fish treats a nonzero return as reject, so this silently disabled fish history recording entirely. 2. Re-sourcing this bootstrap script in the same fish process (shell reload, or a nested fish subshell) backed up whatever fish_should_add_to_history currently was -- on a second run, that's already our own wrapper, not the user's original -- into warp_original_fish_should_add_to_history, making every history check call itself and hit fish's call stack limit. Fix: always establish warp_original_fish_should_add_to_history exactly once, before installing the wrapper, as either the user's real original function or an explicit accept-everything default. The wrapper then unconditionally delegates to it, with no call-time branching on whether a backup exists. Added regression tests (bootstrap_tests.rs) that extract the real installer snippet from fish.sh and run it against a live fish process, sourced twice (simulating re-sourcing), both with and without a pre-existing user hook. Each test asserts a positive control (an ordinary command is still accepted) alongside the negative one (the ctrl-r helper invocation is rejected), so a test can't pass merely because everything is being rejected.
…ings Third ordering the reviewer found: source #1 with no user hook installs the accept-everything default backup; a user or plugin then defines fish_should_add_to_history; source #2 saw a backup already existed and left it alone, silently discarding the intervening hook. Install-time rule that now covers all three orders (no hook, hook before, hook between): on every sourcing, if the current fish_should_add_to_history is not recognizably our own wrapper (identified by the warp_run_external_ctrl_r_widget sentinel in its body), capture it as the backup -- replacing whatever backup existed, including an earlier accept-everything default -- so the backup always reflects the latest real hook. Only when the current function already is our wrapper (i.e. an unmodified re-source) is the existing backup left alone, which is what keeps the recursion path from the previous fix closed. functions -c refuses to overwrite an existing destination, so the previous backup is erased first. Added a third regression test for this exact ordering (hook installed between two sourcings), alongside the existing no-hook and hook-before-first- source cases, each with the same positive-control assertion.
The HISTIGNORE/zshaddhistory/fish_should_add_to_history exclusions added previously only stop the shell's own history from recording the warp_run_external_ctrl_r_widget invocation. atuin maintains a separate SQLite history database and records every command through its own preexec/precmd hooks, independent of those shell-level mechanisms, so the synthetic invocation was still landing in atuin's own history -- the exact history list this feature searches on the next ctrl-r, and the one place this pollution hurts most. Fix by deleting the entry atuin's own hooks record for this invocation, using 'atuin search --delete' with the handoff token to match it exactly. Also sweep up any invocations from before this fix (or from a session that exited before its own cleanup ran) once per bootstrap, backgrounded since it's pure hygiene and not required for the handoff to work. fzf's ctrl-r widgets (fzf-history-widget / __fzf_history__) read the shell's own history builtin directly and keep no separate store, so they don't have this problem.
4 tasks
HISTIGNORE (bash), the zshaddhistory hook (zsh), and the fish_should_add_to_history wrapper (fish) only ever stopped the *shell's* history file from recording the warp_run_external_ctrl_r_widget invocation. atuin records commands through its own preexec hook straight into its own sqlite database, independent of and untouched by any of that, so for atuin users the pollution didn't get fixed -- it moved out of scrollback and into the very history list this feature exists to search. Fix by prefixing the invocation with a leading space when executing it, honoring the "ignorespace" convention atuin implements itself in its own binary (per its docs on excluding commands), independent of the shell's own history settings. Verified empirically: a leading-space-prefixed command is excluded from atuin's search results while an unprefixed one is recorded normally. fzf has no equivalent private store to worry about here -- its history widgets (fzf-history-widget / __fzf_history__ / the fish pipeline) all read directly from the shell's own history (via fc -rl / builtin history), which the existing exclusions already cover. Updated fish's fish_should_add_to_history sentinel match to be unanchored so the new leading space doesn't defeat it, and added a regression test asserting the exact leading-space invocation shape is still rejected. Existing atuin database entries recorded before this fix are not retroactively cleaned up; users who want to remove them can delete matching entries via atuin's own history search/delete tooling.
…doff' into factory/core-3807-fzf-ctrl-r-handoff
…ntion A parallel fix landed independently on this branch that deletes the warp_run_external_ctrl_r_widget entry from atuin's own history database after each invocation (per-token exact delete) and sweeps any leftover entries once per bootstrap (catching pollution from before either fix existed). That is complementary to, not redundant with, the leading-space prevention: the sweep handles retroactive cleanup of already-recorded entries that the leading space can't touch, while the per-invocation delete becomes a safety net that should normally find nothing to delete now that the invocation is never recorded in the first place. Updated the per-invocation delete's comment in all three shells to describe that relationship instead of restating (now inaccurate) that atuin always records the invocation.
…ted history Both the per-invocation delete (matched on the handoff token) and the bootstrap-time sweep (matched on the bare function name) assumed the query passed to `atuin search --delete` behaved as an exact match. It does not: atuin's search defaults to fuzzy matching and tokenizes a multi-word query into independent terms, so the query can match -- and delete -- history entries the user typed themselves, not just the synthetic invocation. Because atuin deletions are sync records that propagate to a user's other machines, this could destroy real command history with no prompt and no way back. The leading-space prevention already merged onto this branch means atuin's own "ignorespace" convention keeps the invocation out of its history database in the first place, so the deletes were only ever meant to backstop that mechanism for entries recorded before it existed. That backstop isn't worth the risk: a few residual rows in a dogfood user's local atuin database is a knowable, harmless leftover; deleting an unrelated command from someone's synced history is not recoverable. Comments at each site now explain why we deliberately don't attempt a delete.
2 tasks
Newer atuin (>= 18.10) binds ctrl-r through an indirect key-sequence/ widget-index dispatcher that bind -X can't identify directly, so the existing exact-allowlist live-binding match never fires for it and bash+atuin users fell through to Warp's own command search. Add a fallback for bash only: when the live-binding match doesn't resolve ctrl-r, trust atuin's own $__atuin_bind_ctrl_r init-time flag together with __atuin_history actually being defined as sufficient evidence atuin owns ctrl-r. Both are set unconditionally by `atuin init bash`, so this is real evidence, not a name guess. Accepted trade-off: a ctrl-r rebind that happens after atuin's init runs in the same session won't be detected. zsh and fish are unaffected; atuin binds a named widget directly in both already.
Mechanical rename ahead of adding ctrl-t file search support: the flag now gates the whole shell-widget-handoff mechanism (ctrl-r today, ctrl-t next), not just the ctrl-r case it originally shipped with.
Adds the DCS hook (ExternalCtrlTSelection), Input-side state (PendingCtrlTHandoff, trigger/set functions), the input-restore splice branch in handle_block_completed_event, and the TerminalView entry point (maybe_trigger_external_ctrl_t_file_search) plus its shell plugin tag/helper-command constants -- all as a parallel structure alongside the existing, verified ctrl-r implementation rather than a shared abstraction. Not yet wired: the actual ctrl-t keybinding, and the three shells' bootstrap-side detection/helper scripts. Rust side builds clean.
Adds WorkspaceAction::TriggerExternalCtrlTFileSearch, its Workspace handler (mirrors show_command_search but with no Warp-native fallback UI, since ctrl-t currently does nothing in Warp's input box), and the default ctrl-t key binding in the Input context. Rust side of the ctrl-t feature is now fully wired end to end; only the shell-side detection/helper scripts remain.
Mirrors the ctrl-r external-history detection/helper structure already present in each shell, using an independent shell_plugins tag (external_ctrl_t_file) and a parallel warp_run_external_ctrl_t_widget() helper: - bash: detects fzf's ctrl-t binding via bind -X, invokes __fzf_select__ directly. - zsh: detects via bindkey -M main '^T', invokes __fzf_select (single underscore) directly. - fish: adds warp_external_ctrl_t_widget (mirrors the ctrl-r detector), and since fish has no picker function separable from its fzf-file-widget, invokes fzf directly against a find-style command honoring $FZF_CTRL_T_COMMAND/$FZF_CTRL_T_OPTS. This deliberately skips fish's own commandline-token parsing (dir/query/prefix), landing a plain selection at the cursor like bash/zsh -- a documented simplification. All three scripts extend their respective history-exclusion mechanisms (HISTIGNORE, _warp_zshaddhistory, fish_should_add_to_history) to keep the new helper invocation out of the user's history.
…n exists before tagging Both zsh and fish's fzf-file-widget zle/bind name has stayed stable across fzf releases, but the private picker function each shell's warp_run_external_ctrl_t_widget calls has not: - zsh: fzf < 0.48 (still the version several distros package, e.g. Ubuntu's 0.44.1) exposes the picker as __fsel; current fzf renamed it to __fzf_select. Detection matched the widget name in both cases but invocation only ever called __fzf_select, so on older fzf the function didn't exist and ctrl-t was silently swallowed with no picker shown. - fish: same widget name in both, but older fzf's fish integration has no __fzf_defaults function at all (it builds FZF_DEFAULT_OPTS inline instead); our invocation depends on it, hitting the same swallow. Fix: detection now checks that the function(s) invocation actually depends on are defined before tagging/intercepting. - zsh falls back to __fsel when __fzf_select isn't defined, so ctrl-t keeps working on older fzf rather than merely declining. - fish declines (no tag, no interception) when __fzf_defaults or __fzfcmd is missing, since fish never exposed a picker function separable from its own commandline-token parsing in either fzf generation, making a from-scratch reimplementation more surface area than the payoff justified. Verified against both the real Ubuntu-packaged fzf 0.44.1 shell scripts and a freshly generated fzf 0.74.3 --zsh/--fish integration: zsh tags and hands off successfully on both; fish tags on the new one and correctly declines (no interception) on the old one.
clippy::disallowed_types rejects std::process::Command; every other call site in app/src uses command::blocking::Command. Only surfaced now because presubmit's --all-targets --tests is the first invocation on this branch to compile the test target.
hide_block() only zeroes a block's scrollback height; last_completed_command_text() walked blocks without consulting it, so the shell-widget handoff's helper invocation became the vertical tab's label once its block completed. Affected ctrl-r since it shipped, not just the new ctrl-t path. Falls through to the existing "New session" fallback when the hidden helper was the only completed block.
bash 5.3 prints bind -X as `"\C-r" "widget"` (space) instead of `"\C-r": "widget"` (colon). The extractor required a colon, so plugin tags were empty on macOS brew bash 5.3 even though fzf bindings installed. Accept either separator, parse the live pipeline in unit tests instead of copying it, and panic if bind -X lists a probe the extractor misses.
Windows CI's `bash` is the WSL launcher: it exists, so NotFound never fires, then exits 1. Probe with a sentinel before running scripts so that stub skips, while a real bash that fails a later script still panics. The sed-pattern assertion always runs; only the live bash extraction is skipped.
The requester asked that this not ship enabled on all default builds. Remove it from app/Cargo.toml default and DOGFOOD_FLAGS; add it to PREVIEW_FLAGS (which still enables dogfood). Leave the enabled_features() cfg bridge so `--features shell_widget_handoff` remains a force-enable hatch.
macOS fish CI failed test_fzf_ctrl_r_selects_history_unexecuted with an empty editor after a successful handoff. fzf 0.74.3's widget ends on commandline -f repaint; a too-soon commandline read comes back empty, and Warp treats empty as cancel. Retry the read once, and clear the buffer first so 0.74's (commandline) --query does not filter to the helper invocation.
Temporary CI bisect. Keep the fish ctrl-r settle changes. Restore shell_widget_handoff to Cargo default and ShellWidgetHandoff to DOGFOOD_FLAGS (remove from PREVIEW_FLAGS). Do not ship.
The integration binary's ChannelState had no additional_features, so ShellWidgetHandoff was only on at init_feature_flags when it sat in Cargo default. Wire DEBUG/DOGFOOD/PREVIEW like dev.rs so tests exercise the shipped configuration. Drop the speculative fish commandline retry. Keep the Preview demotion; do not restore the flag to default.
This reverts commit a3b4e44.
shell_widget_handoff tests already enable FeatureFlag::ShellWidgetHandoff per test. ChannelState should stay the bare Integration channel, matching fde719e.
Keep the settle/unit test required with Preview gating. A parallel commit on the shared branch had reverted a3b4e44; put it back.
Returns the branch to the intended state after a divergent local branch was merged over the remote, which deleted the integration channel wiring and reinstated the fish commandline retry. Keeps: - shell_widget_handoff out of app/Cargo.toml `default` - FeatureFlag::ShellWidgetHandoff in PREVIEW_FLAGS only - both integration binaries wiring DEBUG/DOGFOOD/PREVIEW flag sets, so integration tests exercise the shipped channel configuration instead of relying on the Cargo default feature Drops the fish `commandline` pre-clear and empty-read retry. A controlled bisect showed both present in a failing tree and in a passing tree, so neither is necessary or sufficient for the macOS fish failure; the two reads are back-to-back with nothing to order them against fzf's repaint.
fc4a368 reintroduced DEBUG/DOGFOOD/PREVIEW on both integration binaries and deleted the fzf-history-widget empty-read retry. Put both back: ChannelState stays the bare Integration channel; fish retries an empty commandline read after accept, with the matching bootstrap test.
Wiring the whole DEBUG/DOGFOOD/PREVIEW sets fixed the macOS fish ctrl-r failure but also enabled NativeShellCompletions for every integration test, breaking ui_tests::test_alias_expansion_has_limit on Linux. Add just the one flag instead. The integration channel still enables it before feature initialization, which is what the handoff needs, without changing behavior for unrelated tests.
The helper was executed as CommandExecutionSource::User with should_add_command_to_history hardcoded true, so Warp recorded warp_run_external_ctrl_r/t_widget. Shell histfile exclusions already worked. Thread false through those two triggers, matching EnvVarCollection. Keep User so the pty path is unchanged.
Drop the live fzf handoff integration tests, Warp-history unit tests, CI fzf install, and integration-channel flag wiring. Keep Preview demotion, the bash 5.3 bind -X fix, and omitting helper invocations from Warp history.
Drop the PR-only warp_terminal bootstrap tests. Match warp_hex_decode_string to master. Model ctrl-r and ctrl-t as one pending handoff with a small apply-kind enum so they stay mutually exclusive.
Both payloads were identical (buffer, token, session_id); PendingShellWidgetHandoff already holds the ctrl-r vs ctrl-t apply distinction. Emit and parse a single ExternalShellWidgetSelection hook and apply it through one Input setter.
One pending handoff plus session_id and block_id already identify the in-flight widget. Drop the UUID token from pending state, the protocol payload, helper argv, and Bash/Fish/Zsh JSON.
acarl005
approved these changes
Sep 1, 2026
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
2 participants
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
Renames this feature from the ctrl-r-specific "fzf ctrl-r handoff" to shell widget handoff (
FeatureFlag::ShellWidgetHandoff, cargo featureshell_widget_handoff), and extends it to fzf's ctrl-t file search.When a shell has a supported widget bound to a key, Warp hands the keypress to that widget's own picker instead of handling the key itself, and puts the result into the input editor without executing anything:
fzf-file-widgetis invoked directly and already performs its own token-aware replacement, the reported result replaces the buffer wholesale (see Known limitations).Detection is per widget and per shell, and the two are independent — a user can have either, both, or neither handed off. Supported on bash, zsh and fish.
Detection also verifies that the function the invocation will actually call is present before claiming a binding. Without that, a version mismatch would let Warp intercept a key and then have nothing to run, silently swallowing the keystroke.
Known limitations
fzf-file-widgetis invoked directly (seeded with the real draft line and cursor via a hex-encoded{cursor}:{draft}argument to the helper invocation), so it performs its own token-aware replacement itself, the same way it would if the user pressed ctrl-t with no interception at all. Warp therefore lands its reported result as the finished buffer wholesale rather than reconstructing an insertion to splice, which is why fish's landing differs from bash/zsh's cursor-splice (both of which report a plain path with no knowledge of the draft). Not a simplification like the previous fish behavior was — this is fish's own real widget output.bind -Xat all -- it was added in bash 4.3 (NEWS-4.3 item q) -- so detection, which depends on it entirely, sees nothing and neither binding is detected there. On bash < 4 fzf also installs ctrl-r/ctrl-t as readline macros, whichbind -Xcould not report even where it exists. Pre-existing and shared by both bindings; deliberately out of scope.bind -Xlayout. bash 5.3 prints"\C-r" "widget"(space, no colon) instead of"\C-r": "widget". Detection accepts either separator. macOS brew bash is 5.3; Linux CI bash is still 5.2. The unit-test guard now panics ifbind -Xlists a probe the extractor misses, instead of skip-passing.fzf-file-widget) is stable across fzf releases, but the function it calls is not:__fselbefore 0.48,__fzf_selectafter.bind -x(bash >= 4);__fzf_select__has always shipped in the same integration script as the binding.fzf-file-widgetdirectly, so the only requirement is that function's presence -- no longer the__fzf_defaults/__fzfcmdhelpers the old reimplementation depended on. This widens coverage to older fzf releases some distributions still package (0.44.1, for instance), which the previous gate excluded.Rollout
ShellWidgetHandoff is Preview, not default. Removed from
app/Cargo.tomldefaultand fromDOGFOOD_FLAGS; added toPREVIEW_FLAGS(dogfood still gets it automatically). Theenabled_features()#[cfg(feature = "shell_widget_handoff")]bridge is left in place so--features shell_widget_handoffremains a force-enable hatch.The Cargo
default+ bridge is GUI-only; the per-channel arrays drive both GUI and TUI. Before this move, GUI stable compiled the flag on viadefaultwhile TUI only saw it in dogfood. After, both surfaces follow PREVIEW_FLAGS on the same channels.Linked Issue
CORE-3807
Testing
macOS latest-bash CI failed because brew bash 5.3's
bind -Xdropped the colon between keyseq and command;bash_body.shnow accepts colon or space. Confirmed on a macOS runner: fzf bindings install and PATH is fine; the empty plugin map was the extractor.test_bash_bind_x_extraction_accepts_colon_and_space_formatscovers both layouts without needingbind -X.Live fzf integration tests, CI fzf install, and integration-channel flag wiring were reverted at requester request. Remaining automated coverage is unit tests: bash 5.3
bind -Xextraction inbootstrap_tests.rs, plus existing handoff lifecycle tests. Helper invocations are omitted from Warp history viashould_add_command_to_history: false(no new history-assertion tests)../script/formatandcargo clippy --workspace --all-targets --tests -- -D warningsare clean.Verified live in the running client, in tabs Warp itself bootstrapped — startup shell selected in Settings, then a new tab, with a command run to completion first so the handoff fired at a real idle prompt. Nested shells were explicitly avoided: a shell started inside a tab is the foreground process, so keystrokes reach it directly and exercise fzf's native binding rather than this feature.
echo XXXwith the cursor afterechobecomesecho alpha_test_file.txt XXX— spliced at the cursor, trailing text preserved, nothing executed.__fselfallback.Shell-side detection and invocation were additionally exercised standalone in real zsh, bash and fish processes against both fzf versions.
Regression tests added for ctrl-t: bash detection declining when the picker function is absent and tagging when it is present (extracted from
bash_body.shviainclude_str!so it can't drift from what ships); the handoff lifecycle, mirroring ctrl-r's cases; the cursor splice mid-line, at the end of the buffer, into an empty buffer, with a multi-byte character before the cursor, and on cancel; and the pty fallback, dispatched through the realWorkspaceAction::TriggerExternalCtrlTFileSearchso deleting the fallback fails the test.Three defects were found by review and fixed here: the ctrl-t binding didn't exclude
LongRunningCommand, so the key was captured and dropped while a program was in the foreground; with no handoff available the key was silently consumed rather than forwarded; and bash tagged the binding without checking the picker function existed. The binding is also now gated on the feature flag at registration, so a flag-off user sees no change to ctrl-t at all.Full presubmit run:
./script/format, all three clippy invocations andcheck_no_inline_test_modulesare clean. Full-workspace nextest reports 42 failures, all environmental in this sandbox — headless X11, network-dependent and font-rendering tests — plustest_histignorespace_support_in_zsh, which was traced to a Rust code path (update_command_history/Shell::should_add_command_to_history) that this branch never touches; its harness fakes the bootstrap lifecycle, so the modified shell hook is never involved.fish native-widget follow-up
Verified live in the running client, fish set as the login shell so a fresh tab boots directly into it (no nested shell). All four scenarios exercised in the same idle fish tab:
Empty line: ctrl-t opens the picker, Escape cancels leaving the line empty, cursor at column 0.
Token-prefix (
vim src/): the picker scopes to exactly the 3 files undersrc/, confirmingfzf-file-widget's own token parsing is in effect (not a $PWD-wide search). Selecting one landsvim src/main.rswholesale.Cancel preserves the line:
echo START MIDDLEwith the cursor beforeMIDDLE, ctrl-t pre-fills the query withMIDDLE(token-aware), Escape restores the byte-identical text.This pass only checked the restored text was byte-identical; it didn't check where the cursor ended up, and that's exactly where a real defect was hiding. A later live recheck (typing a marker character immediately after cancel, instead of trusting the rendered caret) showed the cursor landing at the end of the line rather than back at
MIDDLE's original position. This is a regression introduced by this PR, not pre-existing: before switching fish toCtrlTApplyMode::Replaceplus the realfzf-file-widget, cancel reported an empty selection like bash/zsh still do.fzf-file-widgethas no way to signal cancellation distinctly from a selection -- on Escape it leaves the commandline exactly as seeded -- so a cancelled search was reporting a "selection" identical to the pre-handoff draft, whichInput::handle_block_completed_eventlanded on the(Replace, Some(_))no-op-on-cursor arm instead of the cancel arm. Fixed inwarp_run_external_ctrl_t_widget(fish-only; bash/zsh's plain__fzf_select__/__fselpipelines never seed anything for cancellation to echo back, so they were never affected) by collapsing an unchanged result to empty, reusing the existing cancel convention. See the Testing section below for the fix's own verification.Multi-byte draft (
echo 'héllo wörld '): the first pass (typed via a dead-key/compose workaround for the accented characters) surfaced a fish "quotes are not balanced" error from__fzf_parse_commandlinealongside the picker. Retested with the exact bytes set viaxclipand pasted (ruling out a text-entry artifact from the workaround) and with an isolated PTY-level reproduction seeded from a draft file computed the same way the Rust code computes it (byte-to-char cursor conversion viastring-offset): both reproduce cleanly with no error, the picker opens normally, and cancel restores the line byte-identical with the accented characters intact.Scrollback was also checked after all four scenarios: no stray
warp_run_external_ctrl_t_widgethelper blocks left behind.fish ctrl-t cancel cursor-restore fix
Root-caused, fixed, and re-verified after the above follow-up surfaced the defect (see that section for the original observation).
ctrl_t_handoff_cancel_restores_cursor_captured_by_a_real_triggerdrivesInput::trigger_external_ctrl_t_file_searchdirectly, rather than hand-constructingPendingCtrlTHandoffwith a chosencursor_offset, so the offset under test is the one actually captured live. This is necessary because the pre-existingctrl_t_handoff_cancel_restores_cursor_to_original_offset_mid_linetest never exercised that capture, and passed throughout even though it isn't exercising the code path where this bug lived.trigger_external_ctrl_t_file_search'scursor_offsetcapture, hardcoded toByteOffset::from(0): the new real-trigger test failed (0vs11) while the old hand-constructed test still passed — the exact coverage gap this test closes. Also caught and fixed a vacuous-test bug in the new test itself: this harness never advances the block list without a real pty, so without forcingdeferred_remote_operations.latest_block_idstale (as the older helper already does), the restore branch never ran and the assertion passed regardless of the mutation.warp_ctrl_t_widget_result's comparison, inverted (=to!=): both new fish-side tests failed with real value mismatches (result=[echo START MIDDLE]andresult=[]), not just an extraction fixture breaking.functions -q warp_ctrl_t_widget_resultthat the fresh session loaded the updated helper, then repeated the marker-character cancel test:echo START MIDDLE, cursor beforeMIDDLE, ctrl-t, Escape, typeQ→ resultecho START QMIDDLE, cursor correctly restored.fzf-file-widgetalways appends a trailing space on completion, so the result differs by that space; and re-selecting the same file with the cursor already past a completed token (trailing space present, no active token) — confirmed live that this appends a duplicate (vim src/main.rs src/main.rs) rather than reproducing the original line. Both shapes always change the line, so this is documented in the code comment as unreachable in practice rather than resolved with a second signal.fish ctrl-r fzf-invocation fix
While re-verifying ctrl-t after a later merge with master (see below), ctrl-r was also live-checked as due diligence and found broken on fish: the history picker rendered as a garbled, overlapping overlay, and selecting a highlighted entry inserted unrelated text instead.
Root cause:
warp_run_external_ctrl_r_widget's fzf case hand-builtFZF_DEFAULT_OPTSvia__fzf_defaults(an fzf shell-integration helper) and passed--wrap-sign,--highlight-line,--accept-nth, and--with-shell. Against a real fzf 0.44.1 install (still commonly packaged, e.g. Debian)__fzf_defaultsdoesn't exist in that version's integration at all ("Unknown command: __fzf_defaults"), and none of those four flags are in that fzf's own--helpeither. Fish continues past the failedsetby default, so fzf still launched, just without the options that command was meant to produce, while the piped-in history remained formatted assuming they were active — fzf rendered its raw, index-prefixed input directly (the garbling) and searched/selected over the whole line, index prefix included, rather than just the command text (the wrong-entry selection).This is this PR's own code — the fish ctrl-r handoff doesn't exist on master — so despite surfacing during the merge below, it isn't a merge regression: ctrl-r's cancel path and ctrl-t's full selection path, which share the same relocated dispatch code the merge touched, both worked correctly throughout.
--wrap-signwas present from this PR's very first ctrl-r commit, which predates the fzf 0.44.1 testing claims above — meaning those claims never actually exercised fish's ctrl-r on that fzf version, despite what they said.Fix: call the user's own bound
fzf-history-widgetdirectly instead of reimplementing its fzf invocation, mirroringwarp_run_external_ctrl_t_widget'sfzf-file-widgetcall. It has no version-dependent flags of its own (it ships with the fzf release it targets), replaces the whole commandline on selection and leaves it untouched on cancel — exactly ctrl-r's semantics — so readingcommandline()back and clearing it affords the same execution-queuing safety the ctrl-t fix already established.Verified in isolation before touching the live app: confirmed
__fzf_defaultsis genuinely undefined against the installed fzf; confirmedfzf-history-widgetcalled directly correctly replacescommandlinewith the selected entry; and confirmed the newwarp_run_external_ctrl_r_widget(instrumented to dump its captured result to a file, to rule out a redirection artifact that produced a false empty reading on an earlier attempt) captures the selected entry on selection and empty on cancel.Added
test_fish_ctrl_r_widget_reports_fzf_history_widget_selectionand..._reports_empty_buffer_when_widget_leaves_commandline_untouched, which stubfzf-history-widgetand the interactive-onlycommandlinebuiltin to exercise the delegation and result-capture wiring headlessly, without needing any specific fzf version's flags to exist. Mutation-checked: hardcoding the captured result to empty makes the selection test fail with the exact wrong value while the cancel test still passes; confirmed, then reverted.Live-reverified on a fully fresh app relaunch and new fish tab: ctrl-r selection and cancel both behave correctly, and the picker renders as a clean fzf list rather than the previously-observed garbled overlay.
Reconciling with a master crate-extraction refactor
Between this PR's last sync and this pass, master extracted parts of the
terminalmodule into a newwarp_terminalcrate. A literalgit rebasehit the same modify/delete conflict on every one of this branch's commits, so this used a singlegit merge origin/masterinstead.The one git-flagged conflict was directly in the ctrl-r/ctrl-t handoff path:
app/src/terminal/model/ansi/mod.rs(deleted in master, relocated tocrates/warp_terminal/src/model/ansi/mod.rs) still carried this branch's dispatch of theExternalCtrlRSelection/ExternalCtrlTSelectionDCS hooks to the handler. Resolved by reapplying both dispatch arms at the new location. Two more spots auto-merged without conflict markers but didn't compile or work:terminal_model.rs's handler methods were pointed at a helper method that now expected a differentEventtype (fixed to match every sibling handler in the sameimpl), and the movedbootstrap_tests.rs'sinclude_str!asset paths needed an extra../since the assets themselves didn't move.Added
parse_dcs_external_ctrl_r_selection,parse_dcs_external_ctrl_t_selection, and a negative session-id-gate test, which drive real hex-encoded DCS payloads through the relocated dispatch and assert the handler receives the exact decoded value — proving the reapplied wiring actually fires, not just compiles. Deleting either dispatch arm, or swapping which handler each calls, is now a compile error (exhaustive matching over the hook enum, plus the two hooks having distinct value types), a stronger guarantee than a runtime test; a mutation that does compile (exempting one hook from the session-id gate) was confirmed to make the new negative test fail, then reverted.Multi-line selection/draft handling fix
While answering a review question about whether a multi-line ctrl-r selection could be mishandled the same way the ctrl-t draft-decode defect (fixed above) was, confirmed via direct fish testing that it could, and found the same class of bug independently on the ctrl-t selection path too.
Root cause:
set result (commandline)(ctrl-r) and(warp_ctrl_t_widget_result "$original_line" (commandline))(ctrl-t) both capturecommandline's output via an unquoted command substitution, which fish splits into a list by newline. A multi-line buffer therefore corrupted differently on each path:$resultbecame a multi-element list;warp_escape_json "$result"quotes it back down to a single argument, and fish joins a quoted list with a space, not a newline -- so a multi-line history entry's selection reported its lines space-joined instead of newline-separated.(commandline)passed directly aswarp_ctrl_t_widget_result's second argument expanded to multiple arguments, silently truncating$argv[2]to the result's first line -- for both a real multi-line selection (truncated) and an unchanged multi-line draft on cancel (falsely reported as changed, since the truncated first line differs from the full original).Fix: pipe both
commandlinecalls throughstring collect, matching the fix already applied to the draft-decode path.Added
test_fish_ctrl_r_widget_reports_multiline_selection_with_embedded_newline(using the realwarp_escape_json, since the defect is specifically in escaping a space-joined value),test_fish_ctrl_t_widget_reports_full_multiline_change_without_truncation, andtest_fish_ctrl_t_widget_reports_empty_when_multiline_draft_is_left_unchanged(both exercising the fullwarp_run_external_ctrl_t_widgetrunner against a real draft file with a statefulcommandlinestub). Mutation-checked: reverting eitherstring collectreproduces the described corruption exactly (space-joined for ctrl-r; truncated to the first line for both ctrl-t cases).Also confirmed the Rust side has no analogous limitation:
PendingCtrlRHandoff::restore_textandPendingCtrlTHandoff::insertionare plainStrings piped straight intoeditor.set_buffer_text/select_and_replace, with no line-based splitting anywhere in that path.Ctrl-t draft handoff via a hex-encoded argument
The draft line and cursor fish's ctrl-t helper seeds
fzf-file-widgetwith are now passed directly as a single{char_cursor}:{hex_draft}argument to the helper invocation, instead of through a temp file. Hex keeps the draft a single token -- no whitespace, no newlines, no quoting -- so multiline and trailing-newline drafts round-trip without depending on shell-parsing fragility. Cursor and draft are combined into one argument, not two, because the invocation is typed into the terminal as literal text for the shell to parse; a separate, empty hex field (ctrl-t on a blank line) would vanish under the shell's own word-splitting. Fish decodes with a newwarp_hex_decode_string(the decode counterpart to the existingwarp_hex_encode_string), piped throughstring collect --no-trim-newlinesto preserve a trailing newline in the draft.Live verification also surfaced an unrelated, pre-existing cancel-cursor-restore bug: the cancel-detection comparison read
commandlineas a nested command substitution passed directly into the comparison call, immediately afterfzf-file-widget's own repaint on cancel, which can race that repaint and read a stale value on a real terminal. Fixed by capturing the readback into a local variable first, confirmed live (three consecutive repetitions) to reliably resolve it.fish ctrl-r empty-editor race (fzf 0.74.3)
macOS fish CI failed
test_fzf_ctrl_r_selects_history_unexecutedthree times: the handoff fired, but the editor stayed empty. fzf 0.74.3'sfzf-history-widgetends oncommandline -f repaint; a too-sooncommandlineread comes back empty, and Warp treats empty as cancel (restores the pre-ctrl-r draft, usually empty).Fix in
warp_run_external_ctrl_r_widget: clear the commandline before the widget so 0.74's(commandline)--querydoes not filter to the helper invocation, then retry the read once if the first is empty.test_fish_ctrl_r_widget_retries_empty_commandline_read_after_acceptstubs the first read empty and the second as the selection. Mutation-checked: removing the retry reportsbuffer: "". Preview demotion is unchanged.Computer use screenshots
Exact-tip verification on
4c89de0f9f005d0bb6cdef13c8231e4324897e1e: ctrl-r and ctrl-t through real fzf widgets in Bash, Zsh, and Fish, plus Replace and Splice cancellation restoration.