You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(editor): add opt-in Vim keybindings for writable file editors - #28412
Adds a Vim mode to Orca's built-in file editor. A new Editor Keybindings setting under Settings → General → Editor offers Default (unchanged, persisted default) or Vim. Choosing Vim turns on modal editing in writable file editors and in the writable half of editable diffs, with a status row that shows the current mode (--NORMAL--, --INSERT--, etc.). :w writes the file through the normal save path. Users who never switch see no change.
New persisted setting editorKeybindings ('default' | 'vim', default 'default'); unknown/missing values resolve to Default without rewriting the stored value.
New segmented Editor Keybindings row in Settings → General → Editor, searchable via vim, keybindings, modal, normal mode keywords.
monaco-vim@0.4.4 added as a dependency, loaded lazily via import() so Default sessions never pull it.
New monaco-vim-mode.ts + use-monaco-vim-attachment.tsx seam: attaches initVimMode only to writable, full-height Monaco editors and the modified side of editable DiffViewers; renders a status row below the editor body; registers a single process-global defineEx('write') that routes :w to whichever editor currently owns focus.
A capture-phase Escape bridge on the editor container closes the find widget before Vim can swallow Escape; other Save/Find chords keep their existing capture-phase listeners.
Toggle off removes the status row and disposes the adapter without remounting the editor, so undo history survives.
Why
#26921 asks for Vim editing so users can make quick manual edits without leaving Orca. Orca embeds Monaco, not a VS Code extension host, so VSCodeVim cannot be installed; monaco-vim is the smallest maintained bridge. A home-grown keymap was rejected for the same reason the older, broader PR #9755 was left alone: this keeps scope to Default/Vim plus :w and reuses the existing Save path rather than reimplementing save or tab commands.
N/A — recorded but not attached here: the Electron e2e spec could not run on this host (see Testing). Settings row is a segmented Default/Vim control matching the existing Editor Word Wrap row; the status row is a muted bar under the editor showing --NORMAL--.
Testing
Automated tests added/updated, or explained why not below
I manually tested these changes locally
Ran in the worktree:
pnpm test src/renderer/src/components/editor/monaco-vim-mode.test.ts — real monaco.editor.create + monaco-vim under happy-dom: attaches in --NORMAL--, x deletes, i/Escape round-trips modes, :w reaches the focused editor's save callback, Escape closes an open find widget instead of reaching Vim, dispose removes the adapter.
pnpm test on the new settings/search/shared tests and the existing DiffViewer/DiffSectionBody/EditorContent lifecycle suites — all pass (64 tests).
oxlint --deny-warnings and oxfmt --check clean on changed files.
Not run — this host OOM-kills the full-renderer typecheck and the Electron e2e build (8 GB RAM, saturated by other sessions; both exit 137):
pnpm tc
ORCA_BACKGROUND_LAUNCH=1 pnpm test:e2e -- tests/e2e/vim-keybindings-file-editor.spec.ts (the spec is committed and covers the full acceptance flow; it needs a host that can build the app)
Notes
Cross-platform: no platform-specific code; shortcuts stay on the existing platform-aware handlers. SSH/folder workspaces share the same renderer and save path — no wire or protocol changes.
The only vitest config change is inlining monaco-vim so Vite resolves its extensionless monaco-editor internal imports under the test runner; production bundling resolves them without any alias (verified with a standalone Vite build).
Attach failure is non-fatal: the error is logged, the adapter is disposed, and the editor stays on Default keybindings.
Checklist
This PR is small and focused
I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
Before/after screenshots or videos attached for UI changes, or N/A with reason — could not launch the app on this host; settings row follows the existing segmented-control pattern
Self-reviewed for correctness, security, and performance
Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred) — changed-file lint/format and the touched test suites pass; full typecheck and e2e blocked on host memory as noted above
Adds an editorKeybindings setting (default/vim) under Settings → General →
Editor. When set to vim, writable full-height file editors and the modified
side of editable diffs attach monaco-vim, start in Normal mode, show the
adapter status row, and route :w through the surface's existing save path.
Orca Save/Find stay on their capture-phase shortcuts; Escape closes the find
widget before vim sees it. Read-only, auto-height, combined-diff, terminal,
and rich-markdown surfaces are untouched.
Refs stablyai#26921
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Walkthrough
Adds an opt-in Vim keybinding setting for Monaco editors. Vim attaches to writable, full-height file editors and the modified side of editable diffs. The integration supports Vim mode changes, routes :w through existing save callbacks, and handles Escape when the find widget is open. It also adds preference defaults, settings search, unit and end-to-end tests, and documentation.
Merge Risk: Minimal · up to e1f95
The identified save-routing and type-binding risks are resolved; no actionable merge-blocking issue remains after normal checks.
Pre-merge checks: 3 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Description check
The description has all template sections: ELI5, What Changed, Why, Linked Issue (Fixes #26921), Visual Proof, Testing, Risk, AI Disclosure, Review, Notes, and Checklist. The author gives a clear befo…
Add a Risk section. Name the riskiest part of the change, such as the monaco-vim attachment, the global Escape capture bridge, and the settings-type changes, and state what users would see if it fails. Set the Level to Low, Medium, or High.…
Passed checks: 3 passed
Check name
Status
Explanation
Title check
The title clearly and concisely describes the main change: adding opt-in Vim keybindings for writable file editors.
Linked Issues check
The only directly linked issue, #26921, is closed. Per the assessment rules, it supplies background only. No active directly linked targets remain, so no linked-issue coding requirements apply. The PR…
Out of Scope Changes check
The current whole-PR summary (merge base b0ad924 to head e1f950f) lists no changes to docs/bug-reproductions/delayed-agent-launch.md and no MiniMaxEndpoint relocation or import changes. The earlier ou…
Full details: Linked Issues check
Explanation
The only directly linked issue, #26921, is closed. Per the assessment rules, it supplies background only. No active directly linked targets remain, so no linked-issue coding requirements apply. The PR still implements the historical request for Vim editing: opt-in Vim mode in writable file editors and editable diff sides, but this does not create a requirement. No coding requirement is unmet.
Full details: Out of Scope Changes check
Explanation
The current whole-PR summary (merge base b0ad924 to head e1f950f) lists no changes to docs/bug-reproductions/delayed-agent-launch.md and no MiniMaxEndpoint relocation or import changes. The earlier out-of-scope findings are therefore no longer supported by the current evidence. The PR comments also state that these changes were reverted. The remaining changes connect to the Vim feature: the Vim dependency and lazy loading, the vitest inlining of monaco-vim, the settings type and default, the settings row and search entry, the MonacoEditor and DiffViewer attachment, the monaco-vim-mode adapter and hook, the e2e probe getValue addition, tests, the e2e spec, documentation, and the plan file. The move of editor settings from global-settings-types.ts into the new editor-global-settings.ts type supports placing the new editorKeybindings field. This is a type refactor that connects to the feature. This verdict relies on the AI-generated summary and the PR comments. No repository inspection was available to confirm the reverted files directly.
Full details: Description check
Explanation
The description has all template sections: ELI5, What Changed, Why, Linked Issue (Fixes #26921), Visual Proof, Testing, Risk, AI Disclosure, Review, Notes, and Checklist. The author gives a clear before and after for the user, the mechanism, and the reason for the chosen approach. Gaps: Visual Proof is N/A but the template requires before and after visuals for UI changes, and the author gives a reason (the app could not launch on this host). The Risk section is missing entirely. The template requires a Risk section with a Level and a backout plan. The Agent skill upstream boundary section is also absent. Manual testing is unchecked, and the lint, typecheck, test, and build checklist item is only partly done. Typecheck and the e2e spec were not run.
Resolution
Add a Risk section. Name the riskiest part of the change, such as the monaco-vim attachment, the global Escape capture bridge, and the settings-type changes, and state what users would see if it fails. Set the Level to Low, Medium, or High. Explain how to back it out: a plain revert, or what a revert would not undo, such as persisted editorKeybindings values. Add the Agent skill upstream boundary section and check the applicable box. Complete the manual test or state clearly that it was not done. Add the missing before and after visuals, or keep N/A with the reason. Run pnpm typecheck and the e2e spec on a host with enough memory, or state the remaining gap. Also review the removal of editorAutoSave, editorAutoSaveDelayMs, editorMinimapEnabled, editorFontFamily, and editorWordWrap from GlobalSettings. The description does not explain that change, but the summary shows it. Include it under What Changed or Risk.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
Missing MiniMaxEndpoint import (thread + review): Valid, fixed. GlobalSettings.minimaxEndpoint referenced the type after the re-export removal. Resolution follows upstream's new GlobalSettingsBase in src/shared/global-settings-base.ts — MiniMaxEndpoint stays a local export of global-settings-types.ts, no extra module needed.
Out-of-scope warnings: docs/bug-reproductions/delayed-agent-launch.md formatting reverted to upstream. The MiniMaxEndpoint relocation existed only because upstream's GlobalSettingsBase refactor pushed global-settings-types.ts past the oxlint 300-line cap once editorKeybindings joined; upstream now declares the type locally again, so the extra module and import repoints are removed entirely.
Docstring coverage: Declined — docstrings are not this repo's style; comments stay minimal per project convention.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ELI5
Adds a Vim mode to Orca's built-in file editor. A new Editor Keybindings setting under Settings → General → Editor offers Default (unchanged, persisted default) or Vim. Choosing Vim turns on modal editing in writable file editors and in the writable half of editable diffs, with a status row that shows the current mode (
--NORMAL--,--INSERT--, etc.).:wwrites the file through the normal save path. Users who never switch see no change.What Changed
Diff size: source +418 / −16 (18 files), tests +419 / −0 (5 files).
editorKeybindings('default' | 'vim', default'default'); unknown/missing values resolve to Default without rewriting the stored value.vim,keybindings,modal,normal modekeywords.monaco-vim@0.4.4added as a dependency, loaded lazily viaimport()so Default sessions never pull it.monaco-vim-mode.ts+use-monaco-vim-attachment.tsxseam: attachesinitVimModeonly to writable, full-height Monaco editors and the modified side of editableDiffViewers; renders a status row below the editor body; registers a single process-globaldefineEx('write')that routes:wto whichever editor currently owns focus.Why
#26921 asks for Vim editing so users can make quick manual edits without leaving Orca. Orca embeds Monaco, not a VS Code extension host, so VSCodeVim cannot be installed;
monaco-vimis the smallest maintained bridge. A home-grown keymap was rejected for the same reason the older, broader PR #9755 was left alone: this keeps scope to Default/Vim plus:wand reuses the existing Save path rather than reimplementing save or tab commands.Linked Issue
Fixes #26921
Visual Proof
N/A — recorded but not attached here: the Electron e2e spec could not run on this host (see Testing). Settings row is a segmented Default/Vim control matching the existing Editor Word Wrap row; the status row is a muted bar under the editor showing
--NORMAL--.Testing
Ran in the worktree:
pnpm test src/renderer/src/components/editor/monaco-vim-mode.test.ts— realmonaco.editor.create+monaco-vimunder happy-dom: attaches in--NORMAL--,xdeletes,i/Escape round-trips modes,:wreaches the focused editor's save callback, Escape closes an open find widget instead of reaching Vim, dispose removes the adapter.pnpm teston the new settings/search/shared tests and the existing DiffViewer/DiffSectionBody/EditorContent lifecycle suites — all pass (64 tests).oxlint --deny-warningsandoxfmt --checkclean on changed files.Not run — this host OOM-kills the full-renderer typecheck and the Electron e2e build (8 GB RAM, saturated by other sessions; both exit 137):
pnpm tcORCA_BACKGROUND_LAUNCH=1 pnpm test:e2e -- tests/e2e/vim-keybindings-file-editor.spec.ts(the spec is committed and covers the full acceptance flow; it needs a host that can build the app)Notes
monaco-vimso Vite resolves its extensionlessmonaco-editorinternal imports under the test runner; production bundling resolves them without any alias (verified with a standalone Vite build).Checklist
N/Awith reason — could not launch the app on this host; settings row follows the existing segmented-control patternpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred) — changed-file lint/format and the touched test suites pass; full typecheck and e2e blocked on host memory as noted above