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 |
2 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
ready-to-specorready-to-implement.Screenshots / Videos
Testing
Agent Mode