Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/assets/readme-downloads.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
92 changes: 92 additions & 0 deletions docs/editor-find-layout-aware-shortcut.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Layout-aware find in editable Monaco editors

## Problem

Issue [#7953](https://github.com/stablyai/orca/issues/7953) reports that `Cmd+F` can type `f` into a TypeScript file instead of opening find on macOS.

Orca's editable Monaco surfaces currently install its layout-aware save shortcut through `editor-shortcuts.ts` (`src/renderer/src/components/editor/editor-shortcuts.ts:18`), but leave find entirely to Monaco's internal keycode dispatch. Orca already declares `editor.find` as `Mod+F` (`src/shared/keybindings.ts:784`) and its matcher resolves logical keys before physical-code fallback (`src/shared/keybindings.ts:2041`).

A real Electron repro sends a macOS event with logical key `f`, physical code `KeyU`, and virtual key code `U`, as produced by a non-QWERTY layout. Monaco leaves its find widget closed. The equivalent QWERTY `KeyF` event opens it.

## Root cause

Monaco's built-in find keybinding follows the physical/virtual keycode delivered by Chromium. Orca's shortcut system is layout-aware, but its editable Monaco integrations do not use it for find. On layouts where the key that produces `f` is not physical `KeyF`, Monaco misses the chord and Native Edit Context remains in editing mode.

## Non-goals

- Reimplementing Monaco's find widget, match navigation, or search state.
- Changing find behavior in markdown preview, rich markdown, PDF, browser, terminal, or file search.
- Changing find behavior in read-only diff surfaces.
- Reworking all Monaco keybindings or making Monaco defaults fully obey shortcut unbinding in this patch.
- Adding telemetry for a local keyboard action.

## Design

1. Add a focused editor find installer beside the existing save installer in `editor-shortcuts.ts`. It matches `editor.find` through `editorShortcutMatches`, consumes every matched event before it reaches Native Edit Context or Monaco, and invokes a supplied callback only for the initial, non-repeat keydown. Repeat events must still be prevented and propagation-stopped so Monaco's QWERTY binding cannot reopen/reset the widget.
2. Install that handler on every editable Monaco container in the source editor, editable diff views, and notebook code cells. Run that editor's existing `actions.find` action and dispose the bridge from its existing teardown callback.
3. Add unit coverage at the DOM-listener seam for logical `f` with a non-`KeyF` physical code, QWERTY/default behavior, repeat suppression, unrelated typing, and cleanup.
4. Preserve an Electron regression loop that drives both QWERTY and layout-aware raw key events against a real `.ts` editor and verifies the existing Monaco find widget becomes visible without dirtying the file.

## Data flow

- macOS/Linux/Windows keydown reaches the focused editable Monaco container.
- `editorShortcutMatches('editor.find', event)` resolves the active platform, user bindings, modifiers, and logical key.
- On match, Orca consumes the DOM event and calls Monaco's existing `actions.find` action.
- Monaco owns the visible find widget and focus exactly as before.

## Edge cases

- Auto-repeat must be consumed without invoking find again; returning early before prevention would let Monaco handle a repeated QWERTY `KeyF` event.
- Ordinary unmodified `f` typing and unrelated shortcuts must continue to Monaco unchanged.
- A removed/disposed source editor, diff pane, or notebook cell must not retain the listener.
- QWERTY `Cmd/Ctrl+F` must still open the same Monaco widget once, not twice.
- User-configured bindings accepted by Orca's `editor.find` matcher should open find; Monaco's own default bindings remain outside the scope of this patch.
- The behavior is renderer-local and does not read files or execute commands, so local, SSH, and Remote Orca files share the same path.

## Test plan

- Unit: `editor-shortcuts.test.ts` dispatches keyboard events through a real element and parameterizes the shared bridge across macOS (`metaKey`) and Linux/Windows (`ctrlKey`) using logical `f` with physical `KeyU`. It also asserts QWERTY/default handling, matched-repeat prevention without a second callback, unrelated typing, and disposal behavior used by every editable Monaco integration.
- Electron: open a disposable `.ts` file and drive `Cmd+F`/`Ctrl+F` using the platform modifier; assert `.find-widget.visible`, focused find input, unchanged source text, and clean editor state.
- Electron layout regression: on macOS, dispatch logical `f` with non-QWERTY physical/virtual key identity; assert the same find state and unchanged source text.
- Adjacent smoke: dismiss find, type a normal character, and verify it edits the file rather than reopening find.
- Static: run focused Vitest, `pnpm typecheck`, and `pnpm lint`.

## UI quality bar

No new UI. The existing Monaco find widget must appear in its current position and styling, focus its input, and leave the editor content unchanged. No overlap, clipping, duplicate widget, or focus flicker is acceptable.

## Review screenshots

1. QWERTY/default shortcut with the existing Monaco find widget visible in a `.ts` editor.
2. Non-QWERTY logical-`f` regression path with the same find widget visible and source text unchanged.
3. Adjacent ordinary typing state after find is dismissed, showing the source editor still accepts text normally.

## Rollout

1. Add failing unit coverage for the layout-aware find installer contract.
2. Implement the installer in `editor-shortcuts.ts`.
3. Wire it to each editable Monaco surface's existing find action and lifecycle cleanup.
4. Run focused tests, typecheck, lint, and the Electron QWERTY/layout/typing scenarios.

## Lightweight Eng Review

- Scope: Kept to one shared shortcut installer and the existing mount/teardown seams for source editors, editable diff panes, and notebook code cells; no new find implementation or global shortcut interception.
- Architecture/data flow: The renderer-local Monaco container owns the keydown. Orca's canonical matcher resolves the logical key, while Monaco continues to own widget state and rendering. No main/preload/IPC, persistence, network, SSH, or provider boundary changes.
- Failure modes covered:
- Non-QWERTY logical key differs from physical/virtual keycode.
- QWERTY double handling.
- Auto-repeat escaping to Monaco's native QWERTY handler.
- Listener surviving editor disposal.
- Ordinary typing being consumed.
- Custom `editor.find` chord accepted by Orca but not Monaco.
- Test coverage required:
- DOM-listener unit tests in `src/renderer/src/components/editor/editor-shortcuts.test.ts`, parameterized for Darwin/Meta, Linux/Ctrl, and Windows/Ctrl.
- Electron-visible QWERTY and layout-aware `.ts` scenarios.
- Adjacent ordinary typing smoke test.
- Performance/blast radius: One capture listener per mounted editable Monaco editor, doing a constant-time keybinding comparison only for events within that editor. Multiple mounted diff sections do not fan out because each listener is scoped to its own container. Listeners are removed with Monaco disposal; no polling, scans, IPC, storage, or render-loop work.
- UI quality bar: Existing Monaco find widget only; verify focus, unchanged source text, no duplicate opening, and unchanged styling against `docs/STYLEGUIDE.md`.
- Required review screenshots:
1. Default QWERTY find-open state.
2. Non-QWERTY logical-key find-open state.
3. Find-dismissed ordinary typing state.
- Residual risks: Monaco's internal default bindings remain active when a user explicitly rebinds `editor.find`; fully suppressing/remapping Monaco's native keybinding table is a separate, larger change.
79 changes: 79 additions & 0 deletions docs/new-worktree-sidebar-reveal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# New Worktree Sidebar Reveal

## Problem

Issue https://github.com/stablyai/orca-internal/issues/350 asks that newly created worktrees jump into view in the left sidebar list with no scroll animation.

Current behavior:

- `activateAndRevealWorktree(...)` always calls `state.revealWorktreeInSidebar(worktreeId)` with no options.
- `revealWorktreeInSidebar` defaults `behavior` to `'smooth'` (`ui.ts`).
- `WorktreeList` forwards that behavior to `virtualizer.scrollToIndex(..., { behavior })`.

Result: off-screen targets animate by default, including freshly created worktrees.

## Root Cause

`activateAndRevealWorktree` conflates two intents:

1. activate existing worktree navigation (smooth reveal is fine);
2. activate a just-created worktree (must jump immediately).

Created-worktree callers cannot currently express reveal intent, so they inherit `'smooth'`.

## Scope and Non-goals

- Add an opt-in reveal behavior at activation call sites.
- Apply `'auto'` only where the worktree is newly created/added in that flow.
- Preserve existing behavior for normal worktree navigation (clicks, keyboard, history nav, palette selection of existing worktrees, port/status-driven activation) unless a caller opts in.
- Do not change direct raw reveal paths in IPC handlers that intentionally call `store.revealWorktreeInSidebar(...)` outside `activateAndRevealWorktree` (terminal/editor/mobile focus paths remain smooth).
- Do not change sorting/grouping/filter UI, list virtualization strategy, or sidebar styling.

## Design

1. Extend `activateAndRevealWorktree` options:
- `sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior']`.
2. In `activateAndRevealWorktree`, call:
- `state.revealWorktreeInSidebar(worktreeId, { behavior: opts.sidebarRevealBehavior })` when provided;
- otherwise keep `state.revealWorktreeInSidebar(worktreeId)` so default behavior stays unchanged.
3. Pass `sidebarRevealBehavior: 'auto'` only from created/added-worktree flows:
- `useComposerState` full-create path;
- `useComposerState` quick-create path;
- `useIpcEvents` `onActivateWorktree` only when the event corresponds to a newly created worktree;
- `launch-work-item-direct`;
- folder add/create flows that activate a newly-added synthetic folder worktree (`AddRepoCreateStep` folder branch, `NonGitFolderDialog`, and `repos` slice `addNonGitFolder` path).
4. Keep existing navigation activations smooth, including:
- `AddRepoDialog` / `ProjectAddedDialog` “open primary worktree” actions (these can target pre-existing worktrees, not guaranteed newly created);
- all existing `activateAndRevealWorktree(...)` callers that do not opt in.
5. Keep `WorktreeList` reveal effect unchanged; it already honors `pendingRevealWorktree.behavior`.

## Correctness Notes and Edge Cases

- Repo-filter clearing remains unchanged: `activateAndRevealWorktree` only clears `filterRepoIds` when target repo is excluded.
- Other visibility constraints are not auto-cleared. If the target exists but is hidden by other sidebar state, `resolvePendingSidebarReveal(...)` keeps the reveal pending.
- The reveal effect uncollapses lineage/group containers before scroll; behavior changes only animation mode, not visibility resolution.
- Pending reveal is a single store slot (`pendingRevealWorktree`). Concurrent reveal requests are last-writer-wins; this change should not alter that behavior.
- If activation cannot resolve the worktree (`getKnownWorktreeById` miss), behavior remains unchanged (`false`, no reveal queued).
- `ui:activateWorktree` is an overloaded IPC used by both creation and non-creation activation paths. The renderer must choose `'auto'` only for create cases (for example, worktree absent before fetch and present after fetch), and keep default smooth reveal for existing-worktree activations.
- Multi-window consistency remains per renderer window store; each window applies its own reveal behavior locally.
- This change is renderer-only; it does not add main-process coordination and does not make reveal ordering transactional across concurrent async creators.

## Tests

Add/adjust focused tests in `worktree-activation` coverage:

- explicit `sidebarRevealBehavior: 'auto'` is forwarded to `revealWorktreeInSidebar(worktreeId, { behavior: 'auto' })`;
- no option still calls `revealWorktreeInSidebar(worktreeId)` (store default remains smooth).

Add call-site regression tests (recommended, small):

- one composer create path passes `'auto'`;
- one non-created navigation path stays default (no behavior option).

## Rollout

1. Add `sidebarRevealBehavior` option in `activateAndRevealWorktree`.
2. Update created-worktree callers to pass `'auto'`.
3. Add tests above.
4. Run targeted Vitest tests, then `pnpm typecheck` and `pnpm lint`.
5. Validate in Electron: with sidebar overflow, create a worktree and verify the list jumps to it without smooth animation.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "orca",
"version": "1.4.132",
"version": "1.4.133",
"description": "Next-gen IDE for parallel agentic development",
"homepage": "https://github.com/stablyai/orca",
"author": "stablyai",
Expand Down
103 changes: 103 additions & 0 deletions src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { EventEmitter } from 'node:events'
import type { ChildProcess } from 'node:child_process'
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'

const { gitSpawnMock } = vi.hoisted(() => ({
gitSpawnMock: vi.fn()
}))

vi.mock('../git/runner', () => ({
gitSpawn: gitSpawnMock
}))

import { listFilesWithGit } from './filesystem-list-files-git-fallback'
import { isFileListingCancellation } from '../../shared/file-listing-cancellation'

const tempDirs: string[] = []
const SHA1 = '0123456789abcdef0123456789abcdef01234567'

function createMockProcess(): ChildProcess {
const process = new EventEmitter() as unknown as ChildProcess
;(process as unknown as Record<string, unknown>).stdout = new EventEmitter()
;(
(process as unknown as Record<string, unknown>).stdout as EventEmitter & {
setEncoding: () => void
}
).setEncoding = vi.fn()
;(process as unknown as Record<string, unknown>).stderr = new EventEmitter()
;(process as unknown as Record<string, unknown>).kill = vi.fn()
;(process as unknown as Record<string, unknown>).exitCode = null
;(process as unknown as Record<string, unknown>).signalCode = null
return process
}

async function writeRel(root: string, relPath: string): Promise<void> {
const absPath = join(root, ...relPath.split('/'))
await mkdir(dirname(absPath), { recursive: true })
await writeFile(absPath, 'x')
}

afterEach(async () => {
vi.clearAllMocks()
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })))
})

describe('main Quick Open git directory expansion', () => {
it('expands placeholders emitted by both directory-collapsing passes', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-main-git-ignored-dir-'))
tempDirs.push(root)
await writeRel(root, 'dist/generated.js')
await writeRel(root, 'scratch/notes.txt')

const revParse = createMockProcess()
const primary = createMockProcess()
const ignored = createMockProcess()
gitSpawnMock
.mockReturnValueOnce(revParse)
.mockReturnValueOnce(primary)
.mockReturnValueOnce(ignored)

const promise = listFilesWithGit(root, [], {})
revParse.emit('close', 0, null)
await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3))
;(primary.stdout as unknown as EventEmitter).emit(
'data',
`100644 ${SHA1} 0\tsrc/index.ts\0scratch/\0`
)
primary.emit('close', 0, null)
;(ignored.stdout as unknown as EventEmitter).emit('data', 'dist/\0')
ignored.emit('close', 0, null)

await expect(promise).resolves.toEqual([
'dist/generated.js',
'scratch/notes.txt',
'src/index.ts'
])
expect(gitSpawnMock.mock.calls[2][0]).toContain('--directory')
})

it('cancels both local Git passes when Quick Open abandons the request', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-main-git-cancel-'))
tempDirs.push(root)
const revParse = createMockProcess()
const primary = createMockProcess()
const ignored = createMockProcess()
gitSpawnMock
.mockReturnValueOnce(revParse)
.mockReturnValueOnce(primary)
.mockReturnValueOnce(ignored)

const controller = new AbortController()
const promise = listFilesWithGit(root, [], {}, controller.signal)
revParse.emit('close', 0, null)
await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3))
controller.abort()

await expect(promise).rejects.toSatisfy(isFileListingCancellation)
expect(primary.kill).toHaveBeenCalled()
expect(ignored.kill).toHaveBeenCalled()
})
})
Loading