Add change-keybinding bundled skill for Agent Mode#9415
Conversation
Teaches Warp's built-in AI agent how to handle requests like "change ctrl+space to ctrl+s" by editing ~/.warp*/keybindings.yaml, covering the YAML format, keystroke encoding rules, multi-keystroke chords, the `none` removal sentinel, and the app-restart requirement. Co-Authored-By: Warp <agent@warp.dev>
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment You can view the conversation on Warp. I reviewed this pull request and requested human review from: @alokedesai. I left feedback as a comment so a maintainer can approve. Comment Powered by Oz |
![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 a bundled Agent Mode skill that explains how to edit Warp keybindings. The skill is auto-discovered by the existing bundled-skill loader, but some of the guidance does not match the app's keybinding implementation.
Concerns
- The skill points agents at
~/.warp*/keybindings.yaml, butkeyboard::keybinding_file_path()loads fromwarp_core::paths::config_local_dir(), which is platform-specific and not~/.warp*on Linux or Windows. - The instructions for directing users to the Keybindings UI cite an incorrect default shortcut/action pairing.
Verdict
Found: 0 critical, 2 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
| ~/.warp/keybindings.yaml | ||
| ``` | ||
|
|
||
| Non-stable Warp builds use a channel-specific variant (`~/.warp-dev/`, `~/.warp-preview/`, `~/.warp-oss/`, etc.). To find the right one, list available Warp data directories: |
There was a problem hiding this comment.
keybinding_file_path() loads from warp_core::paths::config_local_dir(), which is ~/.config/warp-oss / ~/.config/warp-terminal on Linux and AppData on Windows, so agents following this will edit a file Warp never reads off macOS.
There was a problem hiding this comment.
Fixed in 41ab2bc. Wired a {{keybindings_file_path}} handlebars variable into build_bundled_skill_context (app/src/ai/skills/skill_manager.rs) backed by keyboard::keybinding_file_path(), which already uses warp_core::paths::config_local_dir() and is therefore platform-correct on Linux (XDG config dir) and Windows (AppData) as well as macOS. The skill now embeds the resolved path directly instead of telling agents to guess ~/.warp*/.
Also dropped the incidental #[cfg(not(test))] gate on keybinding_file_path() so it's reachable from the test build of build_bundled_skill_context (matches the existing user_preferences_toml_file_path pattern, which has no such gate).
| 1. **By current key combo** ("change ctrl+space to ctrl+s"): | ||
| - Read `keybindings.yaml` and look for an entry whose value contains the current trigger (e.g. `grep ctrl-space ~/.warp*/keybindings.yaml`). | ||
| - If found, that's the action — rewrite its value to the new trigger. | ||
| - If not found, the binding is a default and you cannot introspect it from disk. **Do not invent an action name from generic intuition.** Confirm with the user before writing — either via `ask_user_question` (you may propose a candidate as the recommended option only if you have a concrete documented source for it, e.g. an explicit example in this skill or in Warp's published docs), or by directing them to Settings → Keybindings (default `cmd-k`, action `workspace:toggle_keybindings_page`) to copy the canonical `namespace:action_name`. |
There was a problem hiding this comment.
workspace:toggle_keybindings_page is registered with cmdorctrl-/, while the keybindings settings/editor actions are separate (workspace:show_settings_keyboard_shortcuts_page / workspace:show_keybinding_settings, with cmd-ctrl-k on macOS for the latter). Update this so users are sent to the actual Keybindings UI.
There was a problem hiding this comment.
Fixed in 41ab2bc. The skill now points users at the actual keybindings editor — workspace:show_keybinding_settings (default cmd-ctrl-k on macOS via with_mac_key_binding in app/src/workspace/mod.rs:876; Settings → Keyboard Shortcuts menu on Linux/Windows since it has no default cross-platform binding). Removed the incorrect cmd-k / workspace:toggle_keybindings_page reference in both the "By current key combo" fallback and the "By description" branch.
Confirmed against source while fixing: workspace:toggle_keybindings_page is bound to cmdorctrl-/ (CustomAction::ToggleKeybindingsPage in app/src/util/bindings.rs:371) and opens the shortcuts cheatsheet, not the editor — so directing users there for action-name lookup was wrong on both counts.
- Wire {{keybindings_file_path}} handlebars variable into the bundled
skill context so the skill points agents at the actual path Warp
reads (warp_core::paths::config_local_dir() — channel- and
platform-specific). The previous ~/.warp*/keybindings.yaml guidance
was correct only on macOS; Linux uses XDG config dirs and Windows
uses AppData.
- Drop the now-incidental #[cfg(not(test))] gate from
keybinding_file_path() so the path can be read from
build_bundled_skill_context() (mirrors user_preferences_toml_file_path).
- Replace the bogus cmd-k / workspace:toggle_keybindings_page
reference (the latter is actually bound to cmdorctrl-/ and opens the
shortcuts cheatsheet, not the editor) with the real keybindings
editor: workspace:show_keybinding_settings (cmd-ctrl-k on macOS;
Settings → Keyboard Shortcuts on Linux/Windows).
Co-Authored-By: Warp <agent@warp.dev>
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a bundled change-keybinding skill and wires {{keybindings_file_path}} into bundled skill template context so the skill can point agents at the correct per-channel keybindings file.
Concerns
- The skill currently includes an unquoted shell
grepexample using the templated keybindings path. That path can contain spaces or shell metacharacters depending on the user's platform/config directory, so the skill should avoid training agents to interpolate it into a shell command unquoted.
Security
- Unquoted interpolation of
{{keybindings_file_path}}into a shell command can lead to command argument splitting and, for hostile path contents, shell interpretation.
Verdict
Found: 0 critical, 1 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
| Defaults are compiled into Warp and are **not** discoverable from the keybindings file on disk. Only previously-customized bindings appear there. Pick the right strategy based on how the user described the change: | ||
|
|
||
| 1. **By current key combo** ("change ctrl+space to ctrl+s"): | ||
| - Read the keybindings file and look for an entry whose value contains the current trigger (e.g. `grep ctrl-space {{keybindings_file_path}}`). |
There was a problem hiding this comment.
{{keybindings_file_path}} into a shell command unquoted; config paths can contain spaces or shell metacharacters, so prefer a filesystem read/search tool or quote the path explicitly.
There was a problem hiding this comment.
Fixed in 514f49d. The skill now tells the agent to read the keybindings file with its filesystem read tool and scan for the trigger inline, with an explicit warning that the templated path can contain spaces or shell metacharacters and a single-quoted fallback (grep ctrl-space '{{keybindings_file_path}}') for the rare case it must shell out. No other shell interpolations of the path remain in the skill.
Prefer filesystem read tool over shelling out, and quote the templated path if a shell is used (path can contain spaces or shell metacharacters depending on platform/config dir).
|
/oz-review Addressed the unquoted-shell-interpolation concern in 514f49d — the skill now recommends reading the keybindings file with the agent's filesystem tool, and the single shell example uses a single-quoted path. Reply on the thread above. |
There was a problem hiding this comment.
Overview
This PR adds a bundled Agent Mode skill for editing Warp keybindings and exposes {{keybindings_file_path}} in bundled skill template context.
Concerns
- The skill includes registered-action examples using
editor:delete_all_*, but the corresponding editable bindings are registered aseditor_view:delete_all_*; following those examples would write entries that do not affect any shortcut. - The skill still includes a shell fallback that interpolates the templated keybindings path. Even with single quotes, paths containing quotes can break shell quoting; the skill should keep agents on filesystem read tools rather than teaching a shell command for this user-local path.
Verdict
Found: 0 critical, 3 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
| "editor:delete_all_left": cmd-shift-A | ||
| "editor:delete_all_right": cmd-shift-D escape |
There was a problem hiding this comment.
| "editor:delete_all_left": cmd-shift-A | |
| "editor:delete_all_right": cmd-shift-D escape | |
| "editor_view:delete_all_left": cmd-shift-A | |
| "editor_view:delete_all_right": cmd-shift-D escape |
There was a problem hiding this comment.
Fixed in 0effc19 — applied the suggestion verbatim (editor:delete_all_{left,right} → editor_view:delete_all_{left,right}).
| Defaults are compiled into Warp and are **not** discoverable from the keybindings file on disk. Only previously-customized bindings appear there. Pick the right strategy based on how the user described the change: | ||
|
|
||
| 1. **By current key combo** ("change ctrl+space to ctrl+s"): | ||
| - Read the keybindings file with your filesystem read tool and scan for an entry whose value matches the current trigger (e.g. a line like `"some:action": ctrl-space`). Prefer this over shelling out — the templated path can contain spaces or shell metacharacters depending on the user's platform/config dir, so a raw `grep ctrl-space {{keybindings_file_path}}` is unsafe. If you must use a shell, single-quote the path: `grep ctrl-space '{{keybindings_file_path}}'`. |
There was a problem hiding this comment.
', so the skill can teach an unsafe command.
| - Read the keybindings file with your filesystem read tool and scan for an entry whose value matches the current trigger (e.g. a line like `"some:action": ctrl-space`). Prefer this over shelling out — the templated path can contain spaces or shell metacharacters depending on the user's platform/config dir, so a raw `grep ctrl-space {{keybindings_file_path}}` is unsafe. If you must use a shell, single-quote the path: `grep ctrl-space '{{keybindings_file_path}}'`. | |
| - Read the keybindings file with your filesystem read tool and scan for an entry whose value matches the current trigger (e.g. a line like `"some:action": ctrl-space`). Prefer this over shelling out — the templated path can contain spaces, quotes, or shell metacharacters depending on the user's platform/config dir. |
There was a problem hiding this comment.
Fixed in 0effc19. Dropped the shell fallback entirely; the skill now tells agents to read the keybindings file with their filesystem read tool only, with an explicit note that paths can contain spaces, quotes, or other shell metacharacters and that quoting around them is fragile.
| Two-keystroke chord: | ||
|
|
||
| ```yaml | ||
| "editor:delete_all_right": cmd-shift-D escape |
There was a problem hiding this comment.
editor: namespace; the registered action is editor_view:delete_all_right.
| "editor:delete_all_right": cmd-shift-D escape | |
| "editor_view:delete_all_right": cmd-shift-D escape |
There was a problem hiding this comment.
Fixed in 0effc19 (editor:delete_all_right → editor_view:delete_all_right in the chord example).
- Rename editor:delete_all_{left,right} → editor_view:delete_all_*
(the actual registered action namespace; previous examples would
have produced inert custom entries).
- Remove the single-quoted shell fallback for {{keybindings_file_path}}.
Single quotes still break for paths containing apostrophes; keep
agents on the filesystem read tool only.
|
/oz-review Addressed all three concerns from the previous review in 0effc19:
Note: the same wrong-namespace examples exist in the doc comment on |
There was a problem hiding this comment.
Overview
This PR adds a new bundled change-keybinding skill and wires {{keybindings_file_path}} into bundled-skill template context so Agent Mode can point users at the correct channel-specific keybindings file. The Rust test coverage was updated for the new context key.
Concerns
- No blocking correctness or security concerns found in the changed lines. The documented keybinding file path, YAML shape, removal sentinel, and keystroke parsing guidance align with the surrounding keybinding loader and parser behavior.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
captainsafia
added
the
external-contributor
Warp Dev Github Integration
alokedesai
left a comment
alokedesai
left a comment
There was a problem hiding this comment.
Thank you so much for contributing! Left a few comments but I think this is good to go after that :D
| - **Letter casing**: applies only to single-letter keys. Without `shift`, the letter is lowercase (`ctrl-s`). With `shift`, the letter is **uppercase** (`shift-A`, never `shift-a`). Mixing them is invalid. | ||
| - **Special keys**: `space`, `enter`, `escape`, `tab`, `backspace`, `delete`, `insert`, `up`, `down`, `left`, `right`, `home`, `end`, `pageup`, `pagedown`, `f1`–`f20`, `numpadenter`. Always lowercase, even with `shift` (`ctrl-shift-space`, `shift-tab` — never `ctrl-shift-SPACE`). Use the literal word `space` — not `" "`. | ||
| - **Punctuation** is the bare character: `cmd-=`, `cmd-,`, `cmdorctrl-/`. | ||
| - **Multi-keystroke chord**: separate keystrokes with a single space inside the value, e.g. `cmd-shift-D escape`. |
There was a problem hiding this comment.
I would not suggest allowing multi keystroke chords currently. They are partially supported at best there are a ton of issues with them (which is why we don't allow them within the keybindings UI right now)
There was a problem hiding this comment.
Done — dropped the chord example in the File format block, the encoding-rule bullet, and the standalone "Two-keystroke chord" example in fa7dd14.
| 1. **By current key combo** ("change ctrl+space to ctrl+s"): | ||
| - Read the keybindings file with your filesystem read tool and scan for an entry whose value matches the current trigger (e.g. a line like `"some:action": ctrl-space`). Do not shell out for this — the templated path is user-local and can contain spaces, quotes, or other shell metacharacters depending on the user's platform/config dir, and quoting around it is fragile. |
There was a problem hiding this comment.
For now, thoughts on removing this? I think it's actually too vague to be that useful because we don't yet have documentaiton of every single keybinding name that the agent can go consult
There was a problem hiding this comment.
- Remove multi-keystroke chord references (per-review: chords are partially supported and intentionally not exposed in the keybindings UI, so the skill shouldn't teach the agent to produce them). - Drop the "by current key combo" strategy under Identifying the action and roll its trigger into the description-based fallback that points the user at the keybindings editor (no action-name catalog yet, so the on-disk grep is too unreliable to be worth the turn).
alokedesai
left a comment
alokedesai
left a comment
There was a problem hiding this comment.
Thank you for contributing!
|
/oz-review |
## Description Adds a new bundled skill at \`resources/bundled/skills/change-keybinding/\` that teaches Warp's Agent Mode how to remap keyboard shortcuts when the user asks (e.g. _"Please change ctrl+space mapping to ctrl+s"_). The skill walks the agent through: - Discovering the right \`~/.warp*/keybindings.yaml\` file (channel-aware via \`ls -d ~/.warp*/\`). - The YAML format, including the **mandatory quoting** of action names that contain \`:\`. - Keystroke encoding: modifier order, the **uppercase-after-shift** rule, special key names, multi-keystroke chords (space-separated), and the \`none\` removal sentinel. - The discoverability gap — defaults are compiled into Rust and not introspectable from disk, so the agent must \`ask_user_question\` rather than guess when it can't find the binding in the user's yaml. - The fact that \`keybindings.yaml\` is read only at app launch (via \`load_custom_keybindings\` in \`app/src/lib.rs\`), so the user must restart Warp. No Rust changes are needed: \`SkillManager::load_bundled_skills\` (\`app/src/ai/skills/skill_manager.rs:421\`) auto-discovers any directory under \`resources/bundled/skills/\` with a \`SKILL.md\`, and the default \`BundledSkillActivation::Always\` is appropriate since this skill doesn't depend on a bundled resource file. ## Testing Tested manually in a local \`./script/run\` build of the OSS channel against \`~/.warp-oss/keybindings.yaml\`: - _"Please change ctrl+space mapping to ctrl+s"_ → agent grepped the yaml, didn't find a match (default), and used \`ask_user_question\` to disambiguate the action instead of guessing. ✓ - Direct rebinds by action name produced correctly-quoted entries with the right keystroke normalization (including \`shift-A\` casing). ✓ - Multi-keystroke chords (\`cmd-shift-D escape\`) produced as expected. ✓ - Removal via \`none\` sentinel applied correctly. ✓ - After restarting Warp, the new bindings took effect, confirming the format the skill produces is one Warp's parser actually accepts. No automated test added — the skill is a markdown content file with no executable surface beyond what the existing \`read_bundled_skills\` parser already covers, and behavior is LLM-driven. ## Server API dependencies N/A — no server changes. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode ## Changelog Entries for Stable CHANGELOG-IMPROVEMENT: Agent Mode can now help you remap Warp keyboard shortcuts — ask it to change a shortcut and it will edit your keybindings.yaml for you. Co-Authored-By: Warp <agent@warp.dev> --------- Co-authored-by: Warp <agent@warp.dev>
## Description Adds a new bundled skill at \`resources/bundled/skills/change-keybinding/\` that teaches Warp's Agent Mode how to remap keyboard shortcuts when the user asks (e.g. _"Please change ctrl+space mapping to ctrl+s"_). The skill walks the agent through: - Discovering the right \`~/.warp*/keybindings.yaml\` file (channel-aware via \`ls -d ~/.warp*/\`). - The YAML format, including the **mandatory quoting** of action names that contain \`:\`. - Keystroke encoding: modifier order, the **uppercase-after-shift** rule, special key names, multi-keystroke chords (space-separated), and the \`none\` removal sentinel. - The discoverability gap — defaults are compiled into Rust and not introspectable from disk, so the agent must \`ask_user_question\` rather than guess when it can't find the binding in the user's yaml. - The fact that \`keybindings.yaml\` is read only at app launch (via \`load_custom_keybindings\` in \`app/src/lib.rs\`), so the user must restart Warp. No Rust changes are needed: \`SkillManager::load_bundled_skills\` (\`app/src/ai/skills/skill_manager.rs:421\`) auto-discovers any directory under \`resources/bundled/skills/\` with a \`SKILL.md\`, and the default \`BundledSkillActivation::Always\` is appropriate since this skill doesn't depend on a bundled resource file. ## Testing Tested manually in a local \`./script/run\` build of the OSS channel against \`~/.warp-oss/keybindings.yaml\`: - _"Please change ctrl+space mapping to ctrl+s"_ → agent grepped the yaml, didn't find a match (default), and used \`ask_user_question\` to disambiguate the action instead of guessing. ✓ - Direct rebinds by action name produced correctly-quoted entries with the right keystroke normalization (including \`shift-A\` casing). ✓ - Multi-keystroke chords (\`cmd-shift-D escape\`) produced as expected. ✓ - Removal via \`none\` sentinel applied correctly. ✓ - After restarting Warp, the new bindings took effect, confirming the format the skill produces is one Warp's parser actually accepts. No automated test added — the skill is a markdown content file with no executable surface beyond what the existing \`read_bundled_skills\` parser already covers, and behavior is LLM-driven. ## Server API dependencies N/A — no server changes. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode ## Changelog Entries for Stable CHANGELOG-IMPROVEMENT: Agent Mode can now help you remap Warp keyboard shortcuts — ask it to change a shortcut and it will edit your keybindings.yaml for you. Co-Authored-By: Warp <agent@warp.dev> --------- Co-authored-by: Warp <agent@warp.dev>
3 participants
Description
Adds a new bundled skill at `resources/bundled/skills/change-keybinding/` that teaches Warp's Agent Mode how to remap keyboard shortcuts when the user asks (e.g. "Please change ctrl+space mapping to ctrl+s").
The skill walks the agent through:
No Rust changes are needed: `SkillManager::load_bundled_skills` (`app/src/ai/skills/skill_manager.rs:421`) auto-discovers any directory under `resources/bundled/skills/` with a `SKILL.md`, and the default `BundledSkillActivation::Always` is appropriate since this skill doesn't depend on a bundled resource file.
Testing
Tested manually in a local `./script/run` build of the OSS channel against `~/.warp-oss/keybindings.yaml`:
No automated test added — the skill is a markdown content file with no executable surface beyond what the existing `read_bundled_skills` parser already covers, and behavior is LLM-driven.
Server API dependencies
N/A — no server changes.
Agent Mode
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Agent Mode can now help you remap Warp keyboard shortcuts — ask it to change a shortcut and it will edit your keybindings.yaml for you.
Co-Authored-By: Warp agent@warp.dev