Support bindkeys #537#9847
Conversation
Captures desired behavior for honoring user-defined zsh, bash, and fish keybindings in Warp's input editor: live-shell discovery, best-effort widget coverage, precedence (user-customized Warp > shell bindkeys > Warp defaults), per-tab independence, and graceful fallback when widgets are unsupported. Open questions are surfaced inline for the tech spec to resolve. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
Plans the implementation grounded in current Warp subsystems: bootstrap-side binding query via a new DProtoHook::ShellBindings DCS variant (zsh/bash/fish), per-tab ShellBindings storage on the Shell struct, fine-grained widget-to-InputAction expansion, a third shell_bindings tier in Keymap with precedence user-Warp > shell > defaults, plus settings toggle, feature flag for staged rollout, and telemetry. Resolves the open questions carried from product.md. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude, @th1nkful on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this spec-only 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 |
|
@cla-bot check |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
The cla-bot has been summoned, and re-checked this pull request! |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for honoring zsh/bash/fish shell keybindings in Warp's command input editor. The specs cover the broad user-facing behavior, rollout, settings, debug UI, parser work, and validation strategy.
Concerns
- String bindings with embedded newlines are specified in a way that does not match shell input-queue behavior.
- The bootstrap/DCS design leaves a new keymap-control trust boundary undefined.
- The dynamic re-query mechanism is under-specified and relies on an env-var trigger the app cannot set inside an already-running shell.
- Fish support does not account for Warp's existing fish bootstrap code that rewrites key bindings during precmd.
- Vi-mode tracking via prompt hooks only will miss mode transitions caused by keystrokes handled entirely inside Warp's editor.
Security
- The new ShellBindings DCS payload can change local key handling, so the spec needs source/session validation and rejection limits before implementation.
- Telemetry for unsupported widget names must account for user-defined widget names, which can be arbitrary/private rather than well-known shell vocabulary.
Verdict
Found: 0 critical, 6 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
Six review comments, all important: 1. product.md warpdotdev#9: bindkey -s injects into the shell input queue, so a newline in the bound string accepts the line — clarified to match shell semantics rather than describing literal text insertion. 2. product.md warpdotdev#11 + tech.md telemetry: user-defined widget names can be arbitrary or private. Telemetry now only sends widget names from a documented shell-vocabulary allowlist; user-defined names redact to the bucket "user-defined". Allowlist co-located with the parser as single source of truth. 3. tech.md: ShellBindings DCS is a privileged terminal-control payload. Added a trust boundary — per-tab WARP_BOOTSTRAP_NONCE embedded in every payload, size caps (256 KiB total, 4 KiB per entry), strict schema validation, no partial application. Prevents arbitrary process output from spoofing bindings. 4. tech.md re-query: original flow was not implementable (the app can't flip an env var inside a running shell). Reworked so the bootstrap caches its own __warp_bindings_hash and decides shell-side when to re-emit; the app is consume-only. Preserves PRODUCT warpdotdev#26/warpdotdev#27. 5. tech.md fish: current bootstrap resets fish_key_bindings and installs four Warp binds on top, which would clobber user vi mode. New plan stops the reset, lets the user's scheme run, then reinstalls only Warp's four reserved binds explicitly per mode — resolves the original input-reporting conflict without losing user bindings. 6. tech.md vi mode: precmd-only mode tracking misses transitions that fire inside the editor (Esc, gg, etc). Added an in-app vi mode state machine driven by dispatched widgets, with shell-reported mode treated as initial state and resync. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
/oz-review |
|
I'm re-reviewing this spec-only 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 |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for honoring zsh, bash/readline, and fish shell key bindings in Warp's command input editor. The specs cover the intended lifecycle, precedence model, parser/matcher architecture, settings, telemetry, and validation plan.
Concerns
- The DCS nonce security model relies on a value placed in the shell environment, which child processes can read, so hostile terminal output could still forge privileged binding updates.
- The tech spec does not preserve the product requirement for string macro bindings to re-enter the key-resolution stream; it describes direct literal buffer insertion instead.
- The setting toggle behavior is incomplete: the product requires toggling back on to re-query each tab, but the tech plan only disables the matcher tier and explicitly avoids app-triggered re-query.
- The zsh query plan omits user-defined keymaps even though the product says they are in scope.
- The fish bootstrap plan intentionally overwrites user bindings on Warp-reserved keys without reflecting that exception in the product precedence model.
Security
- The privileged
ShellBindingsDCS channel needs a trust-boundary design that cannot be spoofed by process output from the PTY. An exported environment nonce is not sufficient.
Verdict
Found: 0 critical, 5 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
Five round-2 review comments, all important: 7. tech.md DCS nonce: env-var-only is insufficient because same-uid processes can read /proc/<pid>/environ. Bootstrap now copies the nonce into a non-exported shell-local variable as its first action, then unsets the env var so descendants cannot read it. Threat model documented explicitly: defends against innocent DCS-in-output and later-spawned descendants, not against same-uid attackers who can already inject keystrokes via TIOCSTI / modify rc files. 8. tech.md string macro contradicts PRODUCT warpdotdev#9: literal buffer writes skip key resolution. Reworked widget enum to distinguish SelfInsert (single-char), Macro(String) (re-injected through the input pipeline so newlines submit and control chars trigger actions), Action, and Unsupported. Added a per-macro length bound to prevent bind-cycle loops. 9. tech.md toggle re-query: original PRODUCT warpdotdev#24 said toggling on re-queries each tab; tech said re-queries are shell-side only. The technical reality is shell-side; PRODUCT warpdotdev#24 is updated to match — toggle on resumes from cached table, drift picked up on next precmd. Pressing Enter on an empty line is the user-visible escape hatch. 10. tech.md zsh keymaps: PRODUCT warpdotdev#2 includes user-defined zsh keymaps but tech only enumerated a fixed list. Bootstrap now uses `bindkey -l` to discover all keymaps including any user-defined ones, then dumps each. 11. tech.md fish reserved keys: \cP, \ep, \ew, \ei conflict with PRODUCT warpdotdev#14's clean precedence rules. Documented as an explicit product-level exception in PRODUCT warpdotdev#14 (fish-only, structural because fish integration uses bind rather than precmd hooks); user bindings on these keys are tagged reserved-by-warp in the debug view. Lifting the exception is a tracked follow-up. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
/oz-review |
|
I'm re-reviewing this spec-only 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 |
There was a problem hiding this comment.
Overview
This spec PR defines product and technical plans for honoring zsh, bash/readline, and fish shell bindkeys in Warp's input editor. The scope is generally aligned with issue #537 and includes lifecycle, precedence, rollout, telemetry, testing, and security considerations.
Concerns
- Dynamic rebind timing is internally inconsistent: the product spec promises the next keystroke after declaration completion, while the tech design and performance invariant allow a late async re-query.
- The reserved-key model omits existing zsh/bash bootstrap bindings that are currently installed and reset by Warp, so the proposed fish-only exception does not match today's architecture.
- The DCS spoofing mitigation overstates nonce secrecy for fish because fish user config can run before the
--init-commandbootstrap can unset an exported nonce. - The keymap integration sketch puts terminal-specific binding state into
warpui_corewithout specifying how crate boundaries and action ownership are preserved. - The setting-toggle validation contradicts the proposed no-active-requery behavior.
Verdict
Found: 0 critical, 5 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
Five round-3 review comments, all important: 1. PRODUCT warpdotdev#4 (async timing): the "next keystroke after declaration" guarantee conflicted with warpdotdev#26's non-blocking guarantee. Reworded so the invariant is "the first keystroke after Warp has parsed the next ShellBindings payload from that prompt", with the small async window made explicit. Declarations never block typing. 2. PRODUCT warpdotdev#14 (zsh/bash reserved keys): claim that only fish has reserved keys was wrong — current zsh and bash bootstraps install binds for ^P/\C-p, \ei, \ep, \ew. Listed the reserved set per shell explicitly (zsh: 2, bash: 4, fish: 4); user bindings on those keys import as "reserved-by-warp" and don't fire. 3. tech.md fish nonce: fish runs config.fish before --init-command, so an env-var nonce is readable to user code. Added a fish-specific path: nonce written to a 0600 tempfile under the runtime dir, path passed via --init-command, bootstrap reads then rms the file. WARP_BOOTSTRAP_NONCE env var not used for fish at all. 4. tech.md warpui_core layering: ShellTabBinding/ShellBinding can't live in the UI core. Reworked: warpui_core gets a generic set_contextual(scope_key, bindings) API only; the terminal layer (new keymap_bridge.rs) translates ShellBinding -> Binding with a ContextPredicate::TabIs predicate and BindingOrigin::Shell tag. Resolution order is enforced by predicate evaluation + origin tag, not by a new tier-typed Vec. 5. tech.md toggle test: now asserts cached-table reuse on toggle-on plus next-precmd refresh, matching the relaxed PRODUCT warpdotdev#24 from round 2. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
/oz-review |
There was a problem hiding this comment.
Overview
This spec proposes importing zsh/bash/fish keybinding tables into Warp's shell input editor, with product invariants for discovery, precedence, modes, settings, and telemetry plus a technical plan using bootstrap-emitted DCS payloads and a new shell-binding keymap tier.
Concerns
- The matcher integration relies on a dynamic per-tab context predicate that the current keymap context model cannot represent as written, so multi-tab isolation and shell-tier precedence are underspecified.
- The multi-key fallback behavior promised by the product spec is not implementable by simply reusing the current matcher prefix logic because the matcher drops pending buffered keys on mismatch instead of replaying them.
- The shell-side requery plan does not require preserving last command status or other shell state around prompt-hook binding dumps, which conflicts with the product invariant that the query has no visible shell-state side effects.
Security
- Diagnostics for unsupported bindings are allowed to log raw widget names and key sequences even though the spec recognizes user-defined widget names can be private; diagnostics need the same redaction boundary as telemetry, or an explicit local-only/non-uploaded guarantee.
Verdict
Found: 0 critical, 4 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
Four round-4 review comments, all important: 1. PRODUCT warpdotdev#11 diagnostics: applied the same allowlist/bucket redaction to diagnostics as telemetry, since "local-only" is not guaranteed forever. User-defined widget names log as "user-defined"; bound key sequences are not included in diagnostics at all. 2. tech.md precmd state preservation: spelled out the discipline needed to keep PRODUCT warpdotdev#27. Hash function saves $?/$status first thing, never touches set -o / setopt / shopt / KEYMAP / fish_key_bindings, uses local/__warp_-prefixed temporaries, avoids subshells where it can capture $? before/after where it can't, uses \bindkey / builtin bind to bypass user aliases, and does not modify any DEBUG trap. An integration test asserts $?/options/KEYMAP/non-__warp_ vars all unchanged across precmd. 3. tech.md ContextPredicate: TabIs(u64) wasn't expressible because the existing predicate is &'static str only (warpui_core/src/keymap/context.rs:10-17). Reworked: tab scoping moves to a storage tier, not into ContextPredicate. New API: ScopeKey { category, id } + set_contextual / clear_contextual / set_active_scopes. Matcher iterates only over active scopes; bindings still use the existing predicate for in-scope conditions. 4. tech.md matcher prefix replay: the current matcher returns None and clears pending state on mismatch (matcher.rs:258+), so buffered prefix keys are dropped — that contradicts PRODUCT warpdotdev#8 (replay on abandoned prefix). Spec'd an explicit API change: MatchOutcome::AbandonedPrefix(replay, current); legacy callers wrap via a match_or_replay() helper that flattens to today's behavior. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
@oss-maintainers — bringing this in for a maintainer pass per CONTRIBUTING.md. Have used the three /oz-review rounds and would appreciate a human review on the spec. Recap of the rounds: The spec is now grounded in real file refs (crates/warpui_core/src/keymap/context.rs:10-17, matcher.rs:258+, app/src/terminal/local_tty/shell.rs:632, app/assets/bundled/bootstrap/{zsh_body,bash_body,fish}.sh). Open questions (rollout default-on vs flag, AI prompt opt-in, user-defined widget forwarding) are explicit and tagged for maintainer input. |
|
I'm re-reviewing this spec-only 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 |
There was a problem hiding this comment.
Overview
This spec proposes honoring zsh/bash/fish shell bindkeys in Warp's input editor through bootstrap-emitted binding tables, per-tab state, keymap integration, settings, telemetry, and staged rollout.
Concerns
- The product spec contradicts itself on binding precedence, which could lead implementers to check shell bindings before explicit Warp user overrides.
- Multi-key sequence behavior is underspecified for prefix timeouts and ambiguous bindings, creating a risk of swallowed or delayed input.
- The tech spec's vi-mode model omits states that the product spec and transition table require.
- The proposed keymap origin layering is internally inconsistent about whether
warpui_coreowns shell-origin metadata.
Security
- The privileged DCS payload validation policy conflicts between dropping oversized entries and rejecting malformed payloads, and it does not pin down where size limits are enforced before allocation/parsing.
Verdict
Found: 0 critical, 5 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
There was a problem hiding this comment.
Overview
This spec proposes honoring zsh/bash/fish shell bindkeys in Warp's input editor through bootstrap-emitted binding tables, per-tab state, keymap integration, settings, telemetry, and staged rollout.
Concerns
- The product spec contradicts itself on binding precedence, which could lead implementers to check shell bindings before explicit Warp user overrides.
- Multi-key sequence behavior is underspecified for prefix timeouts and ambiguous bindings, creating a risk of swallowed or delayed input.
- The tech spec's vi-mode model omits states that the product spec and transition table require.
- The proposed keymap origin layering is internally inconsistent about whether
warpui_coreowns shell-origin metadata.
Security
- The privileged DCS payload validation policy conflicts between dropping oversized entries and rejecting malformed payloads, and it does not pin down where size limits are enforced before allocation/parsing.
Verdict
Found: 0 critical, 5 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
Five round-5 review comments, all important. (Oz double-posted the same review under two ids — addressing the unique set.) 1. PRODUCT warpdotdev#7 contradicted warpdotdev#14 by saying shell bindings match first. Rewrote warpdotdev#7 to walk the warpdotdev#14 ladder explicitly: reserved keys → user Warp overrides → user shell bindings → Warp defaults → default character insertion. Shell bindings are consulted only after the two higher tiers don't match. 2. PRODUCT warpdotdev#8 multi-key sequences: added timeout and ambiguity rules. 500 ms ambiguity timeout when a sequence is both a complete binding and a prefix (Esc/^[ being the canonical case). Pure-prefix accumulation has no timeout (matches readline/ZLE). Abandonment replays buffered keys + the just-received key in arrival order; no keystroke is silently dropped. Focus loss abandons. 3. tech.md DCS validation policy was self-contradictory ("drop oversized entries before parsing" vs "strict validation rejects the whole payload"). Unified to a single three-phase rule: pre-decode byte cap (256 KiB) before serde_json runs; schema decode; post-decode bounds (max 4 KiB per entry, 16 keymaps, 8192 bindings total). Every failure rejects the whole payload — no partial application. 4. tech.md KeymapMode missed ViReplace and ViOperatorPending (zsh viopp), which the dispatch transitions later in the spec depend on. Added explicit variants for both, with comments on what each maps to per shell. Other(String) is documented for user-defined zsh keymaps from `bindkey -N`. 5. tech.md BindingOrigin layering: matcher couldn't enforce precedence if BindingOrigin lived only in the terminal layer. Moved BindingOrigin into warpui_core as a generic enum { Fixed, Editable, Contextual } — shell-free; future callers (plugin bindings, etc.) can reuse the Contextual tier without further core changes. Renamed BindingOrigin::Shell -> ::Contextual throughout. Shell-specific types (ShellBinding, ShellWidget, keymap_bridge) stay in the terminal layer; they translate into core Binding { origin: Contextual } instances before set_contextual. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
There was a problem hiding this comment.
Thanks for working on this. It's one of the top requested Warp features. It's also very complex, so this won't be easy to align on.
I haven't looked at the tech spec yet since I think there are blocking issues with the product spec. I want to emphasize that the most important thing is to make sure we solve common user pain points. What most users actually want when this issue comes up is:
- Integration with popular libraries, mostly
fzf, but alsoatuin, copilot CLI, and others. - Their custom line editor bindings/widgets to work.
My understanding of this product spec is that neither can actually be supported. I left comments and questions inline about those.
I do like this idea of the keybinding precedence chain. I also like the idea of introspecting the shell for its keymaps. I'm just not sure about only supporting a small list of known widgets.
| previous keymap (consistent with the non-blocking guarantee in #26); | ||
| declarations never block typing. | ||
|
|
||
| 5. Each tab tracks its own bindings independently. Changing bindings in one |
There was a problem hiding this comment.
| 5. Each tab tracks its own bindings independently. Changing bindings in one | |
| 5. Each session/pane tracks its own bindings independently. Changing bindings in one |
A Warp "tab" can contain multiple sessions or panes. It would be more exact to say this is per-pane or per-session state.
| silently stealing the keystroke. | ||
| - Keymap modes: emacs vs vi (insert/command/visual) for the shells that | ||
| expose them. Mode switches initiated by the user (e.g. `bindkey -v`, | ||
| `set -o vi`, vi-mode plugins, fish bind modes) take effect without restart. |
There was a problem hiding this comment.
I would put this as very low priority. Mutating your keybindings mid-session seems like a very rare case. It's fine if that works without much additional complexity, but optimizing for this case isn't worth a lot of effort IMO. We should focus on what more users are requesting, e.g. fzf.
Furthermore, Warp has its own vim keybinding implementation (which IMO is quite good) so I don't find this specific example to be very compeling.
| `zle -N my-widget; bindkey '^X' my-widget`), v1 treats these as | ||
| unsupported. A future iteration could forward the keystroke to the | ||
| shell to let it execute the widget. Confirm v1 = unsupported is | ||
| acceptable. |
There was a problem hiding this comment.
How would fzf-history-widget be treated? As a user-defined widget? fzf is the primary reason people request bindkey so I don't think it makes sense to proceed without supporting fzf.
| `set -o vi`, vi-mode plugins, fish bind modes) take effect without restart. | ||
| - Conflict policy with Warp's own keybindings (see Behavior #14). | ||
|
|
||
| Out of scope for this spec: |
There was a problem hiding this comment.
What about PowerShell? It's not blocking if we mark that out-of-scope, but we should explicitly mention it.
| from whichever key the user has bound them to. Behavior of the | ||
| completion UI itself is unchanged by this spec. | ||
|
|
||
| 11. Widgets that have no Warp equivalent (examples: `quoted-insert` in some |
There was a problem hiding this comment.
The stated goal is to honor "user-defined keybindings". However, I'm not convinced this proposal actually solves that pain point. Let's take a simple example of a user-defined keybinding (kill-line but it copies to the system clipboard):
function kill-line-to-clipboard() {
# Kill the line (zsh built-in)
zle kill-line
# Copy the killed text to clipboard
echo -n "$CUTBUFFER" | pbcopy
}
zle -N kill-line-to-clipboard
bindkey '^k' kill-line-to-clipboardIn #10 you have proposed essentially an "allowlist" of known line editor functionality that Warp is able to reproduce. This custom widget kill-line-to-clipboard won't be in that list. Here in #11 you state that this is not something Warp can introspect or fulfill. If Warp can't perform this, how are we solving the original pain point?
|
|
||
| ### AI / agent prompt input | ||
|
|
||
| 22. The AI prompt input editor does not honor shell bindkeys by default — |
There was a problem hiding this comment.
How would this work with our "natural language auto-detection" feature? In agent conversations, we run a custom model on the input on every keypress to classify whether the current buffer is a shell command or natural language. Sometimes this flickers between the 2 as the user is typing. If a shell command is misclassified as natural language it would disable the shell bindkeys which would be frustrating.
Maintainer feedback: the spec deferred user-defined shell-function
widgets ("v1 = unsupported"), which means atuin and fzf — by far the
most common reason users care about bindkeys today — would not work
in v1. Reworked to make pass-through dispatch a first-class v1
capability.
product.md changes:
- New Behavior subsection "Motivating cases" naming atuin, fzf,
zsh-vi-mode, and edit-command-line as the driving examples and
setting the lens for the rest of the spec.
- PRODUCT warpdotdev#10 reorganized into three explicit categories: A built-in
widgets (translated to InputAction); B string macros (per warpdotdev#9 input-
pipeline re-injection); C external shell-function widgets (atuin,
fzf, plugins, edit-command-line). The user does not have to think
about which is which but the spec is precise about each.
- PRODUCT warpdotdev#11 narrowed to Category A only ("widgets Warp can't cheaply
replicate"). Removed the "v1 = unsupported" open question.
- New PRODUCT warpdotdev#11.5 — external widget pass-through user experience.
Buffer is pre-populated into the shell's $BUFFER / $READLINE_LINE /
commandline; widget runs natively (alt-screen if it wants); on exit
the new buffer syncs back into Warp's editor. Cancel preserves the
pre-invocation buffer. Widget-initiated accept-line submits the
command as a normal Warp block. Failure modes covered.
- PRODUCT warpdotdev#25 debug-view statuses expanded: honored-builtin,
honored-macro, honored-passthrough, shadowed-by-warp-user,
reserved-by-warp, unsupported (Category A only).
tech.md changes:
- §3 (Widget mapping): added External(widget_name) variant. Detection
rules per shell — anything not in the documented built-in widget
list is External; bash bind -X output is always External.
- New §6 "External widget pass-through dispatch". Composes existing
primitives — Message::Input for the trigger write, InputBuffer DCS
+ send_input_buffer_to_terminal_editor for the buffer round-trip,
enter_alt_screen / exit_alt_screen for TUI handling. What's new:
one app→shell DCS direction (warp_invoke_widget command line that
Warp suppresses from scrollback) and one shell-side helper
(warp_invoke_widget) per shell that sets the buffer, calls the
widget, and reports back. New WidgetInvocationDone DCS hook so
Warp knows pass-through is over. New PassthroughPending tab state.
Cancel/timeout/accept-line semantics specified. 60s timeout
fallback. Trust boundary preserved (outbound side owned by Warp;
inbound carries the same nonce as every other DCS).
- New integration tests: atuin pass-through happy path per shell,
fzf pass-through per shell, cancel path, accept-line path, timeout
path, External widget detection unit tests.
- Updated open question warpdotdev#11 to be Category A only.
The grounding came from a focused codebase explore — Warp already
has all the primitives we need (alt-screen yield, InputBuffer hook
and editor sync, suppressed-block command convention). The new
mechanism reuses them.
https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
Two over-claims in the previous commit, found before the next review catches them: 1. WARP_SUPPRESS_BLOCK env-var convention does not exist. Replaced with a real proposal: model the warp_invoke_widget block via the existing InteractionMode hide-by-default pattern in app/src/terminal/model/block/interaction_mode.rs (113-228, 325-345), the same plumbing that hides AI-requested command blocks today. v1 adds an InteractionMode::WidgetInvocation variant; the block stays in the model but is not rendered. 2. script/readline-invoke.sh referenced as a "TODO: implement" placeholder is fictional. Reworked the bash story: the helper handles bind -x-bound keys (atuin and fzf are both bind -x, so the motivating cases work) by fetching the body via bind -X and eval'ing it after pre-populating READLINE_LINE / READLINE_POINT. Built-in-readline-function bindings fall back to Category A translation; a v2 keystroke-injection mechanism is the open question, scoped out of v1. Risks section updated to flag both as honest gaps. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
Maintainer feedback: spec must also make custom line editor bindings/widgets work — not just single-keystroke TUI tools but plugins that hook every keystroke (zsh-autosuggestions, syntax highlighting, fish abbreviations, zsh-vi-mode visual indicators). Pass-through (PRODUCT warpdotdev#11.5) covers the TUI half but not the continuous half. product.md changes: - Motivating cases reorganized into two groups: Group 1 single- keystroke TUI widgets (atuin/fzf/edit-command-line, handled by pass-through), Group 2 continuous inline-rendering plugins (zsh-autosuggestions, zsh-/fast-syntax-highlighting, fish abbreviations, zsh-vi-mode). Both are primary v1. - New PRODUCT warpdotdev#11.6 specifying the user-visible behavior for Group 2: inline suggestions appear as you type, suggestion acceptance works on whatever key the plugin binds to it, syntax highlighting renders, fish abbreviations expand on space/enter, vi-mode cursor shapes match the plugin's intent. No double- render, block UI preserved. Detection driven by the same bindkey -L / bind -p / bind query — installed plugin widgets are the signal. tech.md changes: - New §7 "Continuous inline-plugin rendering" with three candidate mechanisms — Candidate A per-keystroke ZLE round- trip (uniform with §6, latency-bound), Candidate B live shell-line zone (shell renders the prompt input area, Warp lifts the output into block flow), Candidate C plugin-aware query API (Warp queries plugin functions per buffer change). Implementation prototypes A first; falls back to B then C based on real measurement. Behavioral invariants from warpdotdev#11.6 are the contract regardless of mechanism. Explicitly NOT "PS1 mode for everything" or "reimplement plugins natively". - New integration tests: zsh-autosuggestions inline rendering + acceptance, zsh-syntax-highlighting tokens, fish abbreviation expansion, zsh-vi-mode cursor shape per mode, p95 keystroke latency budget on the slowest realistic plugin stack (drives the §7 mechanism choice), inline-plugin failure mode (malformed ANSI degrades to plain text). - Manual test now includes the full plugin stack (atuin + fzf + autosuggestions + syntax-highlighting + powerlevel10k) per shell. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
/oz-review |
Maintainer raised: agent conversation input runs a per-keystroke shell-vs-natural-language classifier that can flicker mid-typing. Naive gating of bindkey honoring on classifier output produces three failure modes — flickering inline plugins, missed atuin/fzf key presses when classifier says NL, plugin output appearing in the middle of NL sentences. Spec was silent on this. product.md adds PRODUCT warpdotdev#22.5 specifying the contract for the agent input editor (when warpdotdev#22 is opted in): - (a) Explicit bound keystrokes (Category C: atuin Ctrl-R, fzf Ctrl-T) are classifier-independent. Pressing the key is the intent signal; classifier label is bypassed. Stray-press cost is low (dismiss with Esc); missed-press cost is high. - (b) Inline-plugin rendering is hysteretic — only renders when classifier has held a stable label for N=3-5 consecutive characters, with ~80 ms debounce. No per-keystroke flicker. - (c) State transitions clear plugin output in a single frame. No fade, no partial paint. - (d) Explicit user lock to "shell mode" or "NL mode" for the current buffer when the classifier is consistently wrong. Resets at next agent turn. - (e) Developer-mode indicator surfaces the raw + effective classifier state for debugging user reports. tech.md adds a ClassifierGate wrapper that owns hysteresis, debounce, and lock state: - Category C bypass lives in the matcher *before* the gate is consulted, so explicit bound keys dispatch without classifier state ever being read for that keystroke. - Inline-plugin renderer reads EffectiveMode only; transitions schedule a single-frame clear/repaint. - New command `agent-input.lock-mode` toggles Shell/NL/Auto, surfacing a chip in the input editor. - Telemetry: `agent_input.classifier.flip` and `agent_input.bind_dispatched_in_nl` — the latter measures how often the bypass saved a legitimate binding press from being suppressed; growth is the signal that the classifier needs retraining or hysteresis needs tuning. - Architectural rationale: gate is a wrapper, not a flag in the matcher/renderer, so the same code paths work in the classifier-free shell-command input and the classifier-aware agent input. Validation additions: - ClassifierGate unit test: synthetic stream of raw labels asserts hysteresis suppresses single-keystroke flicker and fires on sustained transition. - Integration test: Ctrl-R bound to atuin, classifier rigged to NL — atuin opens regardless. Covers warpdotdev#22.5(a). - Integration test: NL transition clears dimmed text in a single frame. Covers warpdotdev#22.5(c). - Integration test: lock-mode command forces Shell or NL, resets at next agent turn. Covers warpdotdev#22.5(d). Follow-ups callout: when warpdotdev#22 ships, warpdotdev#22.5's ClassifierGate ships with it — they are inseparable. https://claude.ai/code/session_01AbtuqGqnwn4X9yo2jdQAs5
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
/oz-review |
|
This PR is not linked to an issue that is marked with Issue-state enforcement details:
Readiness check:
To continue, link this PR to a same-repo issue such as Powered by Oz |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-spec.
Issue-state enforcement details:
-
Associated same-repo issues checked: none
-
Required readiness label:
ready-to-spec
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-spec.
Powered by Oz
|
/oz-review |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-spec.
Issue-state enforcement details:
-
Associated same-repo issues checked: #537
-
Required readiness label:
ready-to-spec
Readiness check:
- #537: missing
ready-to-spec; readiness labels present: none
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-spec.
Powered by Oz
|
/oz-review |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-spec.
Issue-state enforcement details:
-
Associated same-repo issues checked: #537
-
Required readiness label:
ready-to-spec
Readiness check:
- #537: missing
ready-to-spec; readiness labels present: none
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-spec.
Powered by Oz
3 participants
Description
Spec PR for #537 — honor user-defined shell bindkeys (zsh bindkey, bash readline bind / ~/.inputrc, fish bind) in Warp’s input editor, instead of silently ignoring them.
Adds two short documents under specs/GH537/:
• product.md — 22 numbered behavior invariants covering discovery, dynamic rebind, the widget set we honor, vi/emacs mode handling, multi-tab independence, conflict precedence (user-Warp Warp customizations > shell bindkeys > Warp defaults), surface boundaries (shell input editor only — not the AI prompt or palettes), settings/feature-flag, and performance.
• tech.md — implementation plan grounded in current code: bootstrap-side query into a new DProtoHook::ShellBindings DCS variant (gated by a per-tab WARP_BOOTSTRAP_NONCE plus size caps and strict schema validation so arbitrary process output can’t spoof bindings), per-tab ShellBindings storage on Shell, expanded InputAction with fine-grained ZLE/readline verbs, a third shell_bindings tier in Keymap, in-app vi-mode state machine driven by dispatched widgets (with shell-reported mode as initial state and resync), settings toggle + FeatureFlag::HonorShellBindkeys for staged rollout, and telemetry with widget-name redaction (allowlist of well-known shell vocabulary; user-defined names bucket to user-defined).
Open questions deliberately surfaced for maintainer input rather than guessed: forwarding user-defined widget bodies back to the shell (deferred), AI prompt opt-in, and default-on vs feature-flagged rollout.
Linked Issue
Closes #537
ready-to-specorready-to-implement.Screenshots / Videos
Testing
Agent Mode