diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 1333939252a..bb7f03ef094 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 4.2m + + downloads: 4.3m @@ -15,7 +15,7 @@ downloads downloads - 4.2m - 4.2m + 4.3m + 4.3m diff --git a/docs/editor-find-layout-aware-shortcut.md b/docs/editor-find-layout-aware-shortcut.md new file mode 100644 index 00000000000..9f75390c176 --- /dev/null +++ b/docs/editor-find-layout-aware-shortcut.md @@ -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. diff --git a/docs/new-worktree-sidebar-reveal.md b/docs/new-worktree-sidebar-reveal.md new file mode 100644 index 00000000000..75abb7bd647 --- /dev/null +++ b/docs/new-worktree-sidebar-reveal.md @@ -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. diff --git a/package.json b/package.json index ad112158be9..757b3d7e22c 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts new file mode 100644 index 00000000000..5d5cf163746 --- /dev/null +++ b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts @@ -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).stdout = new EventEmitter() + ;( + (process as unknown as Record).stdout as EventEmitter & { + setEncoding: () => void + } + ).setEncoding = vi.fn() + ;(process as unknown as Record).stderr = new EventEmitter() + ;(process as unknown as Record).kill = vi.fn() + ;(process as unknown as Record).exitCode = null + ;(process as unknown as Record).signalCode = null + return process +} + +async function writeRel(root: string, relPath: string): Promise { + 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() + }) +}) diff --git a/src/main/ipc/filesystem-list-files-git-fallback.ts b/src/main/ipc/filesystem-list-files-git-fallback.ts index 3b77ac28131..8b5bb526f8e 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback.ts @@ -3,9 +3,10 @@ import { gitSpawn } from '../git/runner' import { buildGitLsFilesArgsForQuickOpen } from '../../shared/quick-open-filter' import { createQuickOpenReaddirBudget, - expandQuickOpenGitFilesWithNestedRepos, + expandQuickOpenGitFileListing, listQuickOpenFilesWithReaddir } from '../../shared/quick-open-readdir-walk' +import { fileListingCancellationError } from '../../shared/file-listing-cancellation' /** * Fallback file lister using git ls-files. Used when rg is not available. @@ -16,9 +17,13 @@ import { */ async function isInsideGitWorkTree( rootPath: string, - localGitOptions: { wslDistro?: string } + localGitOptions: { wslDistro?: string }, + signal?: AbortSignal ): Promise { - return new Promise((resolve) => { + if (signal?.aborted) { + throw fileListingCancellationError(signal) + } + return new Promise((resolve, reject) => { const child = gitSpawn(['rev-parse', '--is-inside-work-tree'], { cwd: rootPath, ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), @@ -26,22 +31,37 @@ async function isInsideGitWorkTree( }) let done = false let timer: ReturnType + const cleanup = (): void => { + clearTimeout(timer) + child.off('error', handleError) + child.off('close', handleClose) + signal?.removeEventListener('abort', handleAbort) + } const finish = (isGitRepo: boolean): void => { if (done) { return } done = true - clearTimeout(timer) - child.off('error', handleError) - child.off('close', handleClose) + cleanup() resolve(isGitRepo) } + const cancel = (): void => { + if (done) { + return + } + done = true + child.kill() + cleanup() + reject(fileListingCancellationError(signal)) + } const handleError = (): void => finish(false) const handleClose = (code: number | null, signal: NodeJS.Signals | null): void => finish(code === 0 && signal === null) + const handleAbort = (): void => cancel() child.once('error', handleError) child.once('close', handleClose) + signal?.addEventListener('abort', handleAbort, { once: true }) timer = setTimeout(() => { child.kill() finish(false) @@ -52,16 +72,23 @@ async function isInsideGitWorkTree( export async function listFilesWithGit( rootPath: string, excludePathPrefixes: readonly string[], - localGitOptions: { wslDistro?: string } + localGitOptions: { wslDistro?: string }, + signal?: AbortSignal ): Promise { - if (!(await isInsideGitWorkTree(rootPath, localGitOptions))) { + const isGitWorkTree = await isInsideGitWorkTree(rootPath, localGitOptions, signal) + if (signal?.aborted) { + throw fileListingCancellationError(signal) + } + if (!isGitWorkTree) { return listQuickOpenFilesWithReaddir(rootPath, { excludePathPrefixes, - budget: createQuickOpenReaddirBudget() + budget: createQuickOpenReaddirBudget(), + signal }) } const gitPaths = new Set() + const directoryPaths = new Set() const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes) const children: { child: ChildProcess @@ -78,7 +105,11 @@ export async function listFilesWithGit( if (!path) { return } - gitPaths.add(path) + if (path.endsWith('/')) { + directoryPaths.add(path) + } else { + gitPaths.add(path) + } } // Why: git ls-files outputs paths relative to cwd, so we set cwd to @@ -168,7 +199,7 @@ export async function listFilesWithGit( }) } - const killSurvivors = (): void => { + const killSurvivors = (reason = 'git ls-files canceled after sibling failure'): void => { // Why: Promise.all rejects on the first failed pass; cancel the sibling so // a stuck git process cannot keep scanning after Quick Open has failed. for (const entry of children) { @@ -178,20 +209,32 @@ export async function listFilesWithGit( if (entry.child.exitCode === null && entry.child.signalCode === null) { entry.child.kill() } - entry.reject(new Error('git ls-files canceled after sibling failure')) + entry.reject(new Error(reason)) } } + const onAbort = (): void => killSurvivors('git ls-files cancelled') + signal?.addEventListener('abort', onAbort, { once: true }) try { await Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]) } catch (err) { killSurvivors() + if (signal?.aborted) { + throw fileListingCancellationError(signal) + } throw err + } finally { + signal?.removeEventListener('abort', onAbort) } - return expandQuickOpenGitFilesWithNestedRepos({ + const files = await expandQuickOpenGitFileListing({ rootPath, gitPaths, - excludePathPrefixes + directoryPaths, + excludePathPrefixes, + signal }) + // Why: directory placeholders are expanded after Git exits; restore Git's + // path order so empty queries and fuzzy-score ties remain stable. + return files.sort() } diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index 638775a4811..a3a556b2340 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -365,6 +365,9 @@ describe('filesystem-list-files', () => { expect(gitCalls.length).toBe(2) expect(gitCalls[0][1]).toContain('ls-files') expect(gitCalls[0][1]).toContain('-s') + expect(gitCalls[0][1]).toContain('--directory') + expect(gitCalls[1][1]).toContain('--directory') + expect(gitCalls[1][1]).toContain('--no-empty-directory') // Should include valid files and filter node_modules expect(result).toContain('src/index.ts') diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 36376227305..9dcd2c45038 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -19,7 +19,8 @@ import { listFilesWithGit } from './filesystem-list-files-git-fallback' export async function listQuickOpenFiles( rootPath: string, store: Store, - excludePaths?: string[] + excludePaths?: string[], + signal?: AbortSignal ): Promise { const authorizedRootPath = await resolveAuthorizedPath(rootPath, store) const localGitOptions = getLocalGitOptionsForRegisteredWorktree( @@ -40,7 +41,7 @@ export async function listQuickOpenFiles( // can run. const rgAvailable = await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro) if (!rgAvailable) { - return listFilesWithGit(authorizedRootPath, excludePathPrefixes, localGitOptions) + return listFilesWithGit(authorizedRootPath, excludePathPrefixes, localGitOptions, signal) } const files = new Set() diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 7e7006a375e..100245dca8b 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -1063,7 +1063,7 @@ export function registerFilesystemHandlers( signal: controller?.signal }) } - return await listQuickOpenFiles(args.rootPath, store, args.excludePaths) + return await listQuickOpenFiles(args.rootPath, store, args.excludePaths, controller?.signal) } finally { if (args.requestToken) { listFilesCancellations.delete(args.requestToken) @@ -1073,8 +1073,7 @@ export function registerFilesystemHandlers( ) ipcMain.handle('fs:cancelListFiles', (_event, args: { requestToken: string }): void => { - // Why: best-effort — the entry is gone once the listing settles, and - // local scans are fast enough to simply let them finish. + // Why: best-effort — the entry is gone once the listing settles. listFilesCancellations.get(args.requestToken)?.abort() }) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index e0a2bbf5db1..43ef357eed6 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -477,7 +477,11 @@ async function refreshRemoteTrackingBaseForWorktreeCreate( return getOrStartSshWorktreeCreateFetch( getSshWorktreeCreateBaseFetchKey(repo, base), getSshWorktreeCreateRemoteQueueKey(repo, base.remote), - () => provider.fetchRemoteTrackingRef(repo.path, base.remote, base.branch, base.ref) + () => + // Why: the exact-base refresh gates create; unrelated repo housekeeping must not extend it. + provider.fetchRemoteTrackingRef(repo.path, base.remote, base.branch, base.ref, { + skipAutoMaintenance: true + }) ) } diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 43c94325789..7b4f699cd62 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -3733,7 +3733,8 @@ describe('registerWorktreeHandlers', () => { '/remote/repo', 'origin', 'master', - 'refs/remotes/origin/master' + 'refs/remotes/origin/master', + { skipAutoMaintenance: true } ) }) @@ -3800,7 +3801,8 @@ describe('registerWorktreeHandlers', () => { '/remote/repo', 'origin', 'main', - 'refs/remotes/origin/main' + 'refs/remotes/origin/main', + { skipAutoMaintenance: true } ) expect(provider.addWorktree).toHaveBeenCalledWith( '/remote/repo', @@ -4032,7 +4034,8 @@ describe('registerWorktreeHandlers', () => { '/remote/repo', 'origin', 'main', - 'refs/remotes/origin/main' + 'refs/remotes/origin/main', + { skipAutoMaintenance: true } ) expect(provider.addWorktree).toHaveBeenCalledTimes(2) }) diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index acfaefaed12..0974367ef81 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -850,14 +850,16 @@ describe('SshGitProvider', () => { '/home/user/repo', 'origin', 'main', - 'refs/remotes/origin/main' + 'refs/remotes/origin/main', + { skipAutoMaintenance: true } ) expect(mux.request).toHaveBeenCalledWith('git.fetchRemoteTrackingRef', { worktreePath: '/home/user/repo', remote: 'origin', branch: 'main', - ref: 'refs/remotes/origin/main' + ref: 'refs/remotes/origin/main', + skipAutoMaintenance: true }) }) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 5ba6a29e380..0cde62d534f 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -553,14 +553,16 @@ export class SshGitProvider implements IGitProvider { worktreePath: string, remote: string, branch: string, - ref: string + ref: string, + options?: { skipAutoMaintenance?: boolean } ): Promise { await this.runWithDiffDedupeClear(async () => { await this.mux.request('git.fetchRemoteTrackingRef', { worktreePath, remote, branch, - ref + ref, + ...(options?.skipAutoMaintenance ? { skipAutoMaintenance: true } : {}) }) }) } diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index fce2a5e6d59..14c18115636 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -22,10 +22,19 @@ vi.mock('../git/runner', async (importOriginal) => { // normally — none of them trigger IO until a runtime method is called. import { OrcaRuntimeService } from './orca-runtime' +function isFetchArgs(argv: unknown): argv is string[] { + if (!Array.isArray(argv)) { + return false + } + let commandIndex = 0 + while (argv[commandIndex] === '-c' && typeof argv[commandIndex + 1] === 'string') { + commandIndex += 2 + } + return argv[commandIndex] === 'fetch' +} + function fetchCallCount(): number { - return gitExecFileAsyncMock.mock.calls.filter( - ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' - ).length + return gitExecFileAsyncMock.mock.calls.filter(([argv]) => isFetchArgs(argv)).length } function exactBaseRefreshOptions(cwd: string): { @@ -36,6 +45,21 @@ function exactBaseRefreshOptions(cwd: string): { return { cwd, timeout: 60_000, useConfiguredSshCommandForNetwork: true } } +function exactBaseRefreshArgs(branch = 'main'): string[] { + return [ + '-c', + 'maintenance.auto=false', + '-c', + 'maintenance.commit-graph.auto=0', + '-c', + 'gc.auto=0', + 'fetch', + '--no-tags', + 'origin', + `+refs/heads/${branch}:refs/remotes/origin/${branch}` + ] +} + // Why (STA-1292): the broad create-path fetch must carry a timeout so a Windows // credential-manager GUI hang can't wedge worktree creation forever. function fullRemoteFetchOptions(cwd: string): { cwd: string; timeout: number } { @@ -188,11 +212,23 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { }) expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], + exactBaseRefreshArgs(), exactBaseRefreshOptions('/repo/f') ) }) + it('keeps automatic maintenance enabled for ordinary full remote fetches', async () => { + mockFetchResults([{ stdout: '', stderr: '' }]) + const runtime = new OrcaRuntimeService(null) + + await runtime.getOrStartRemoteFetch('/repo/full-maintenance', 'origin') + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['fetch', 'origin'], + fullRemoteFetchOptions('/repo/full-maintenance') + ) + }) + it('shares an in-flight remote-tracking base refresh and reuses exact-base freshness', async () => { let resolveFetch!: () => void const pending = new Promise<{ stdout: string; stderr: string }>((resolve) => { @@ -297,14 +333,9 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { { ok: true }, { ok: true } ]) - const fetchCalls = gitExecFileAsyncMock.mock.calls.filter( - ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' - ) + const fetchCalls = gitExecFileAsyncMock.mock.calls.filter(([argv]) => isFetchArgs(argv)) expect(fetchCalls).toEqual([ - [ - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - exactBaseRefreshOptions('/repo/h') - ], + [exactBaseRefreshArgs(), exactBaseRefreshOptions('/repo/h')], [['fetch', 'origin'], fullRemoteFetchOptions('/repo/h')] ]) }) @@ -343,15 +374,10 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { { ok: true }, { ok: true } ]) - const fetchCalls = gitExecFileAsyncMock.mock.calls.filter( - ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' - ) + const fetchCalls = gitExecFileAsyncMock.mock.calls.filter(([argv]) => isFetchArgs(argv)) expect(fetchCalls).toEqual([ [['fetch', 'origin'], fullRemoteFetchOptions('/repo/i')], - [ - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - exactBaseRefreshOptions('/repo/i') - ] + [exactBaseRefreshArgs(), exactBaseRefreshOptions('/repo/i')] ]) }) @@ -389,15 +415,10 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { { ok: false, errorKind: 'git_error' }, { ok: true } ]) - const fetchCalls = gitExecFileAsyncMock.mock.calls.filter( - ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' - ) + const fetchCalls = gitExecFileAsyncMock.mock.calls.filter(([argv]) => isFetchArgs(argv)) expect(fetchCalls).toEqual([ [['fetch', 'origin'], fullRemoteFetchOptions('/repo/i-fail')], - [ - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - exactBaseRefreshOptions('/repo/i-fail') - ] + [exactBaseRefreshArgs(), exactBaseRefreshOptions('/repo/i-fail')] ]) }) }) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index ef14013273d..b5fb46e500c 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -2312,7 +2312,7 @@ describe('OrcaRuntimeService', () => { if (args[0] === 'rev-parse' && args[1] === '--verify') { return { stdout: 'base-sha\n', stderr: '' } } - if (args[0] === 'fetch') { + if (args.includes('fetch')) { return refresh.promise } return { stdout: '', stderr: '' } @@ -2325,7 +2325,18 @@ describe('OrcaRuntimeService', () => { await vi.waitFor(() => { expect(gitSpy).toHaveBeenCalledWith( - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], + [ + '-c', + 'maintenance.auto=false', + '-c', + 'maintenance.commit-graph.auto=0', + '-c', + 'gc.auto=0', + 'fetch', + '--no-tags', + 'origin', + '+refs/heads/main:refs/remotes/origin/main' + ], { cwd: TEST_REPO_PATH, useConfiguredSshCommandForNetwork: true, @@ -2443,7 +2454,7 @@ describe('OrcaRuntimeService', () => { if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/main^{commit}')) { return { stdout: 'base-sha\n', stderr: '' } } - if (args[0] === 'fetch') { + if (args.includes('fetch')) { throw new Error('network unavailable') } return { stdout: '', stderr: '' } @@ -2505,7 +2516,7 @@ describe('OrcaRuntimeService', () => { if (args[0] === 'rev-parse' && args.includes('refs/heads/develop^{commit}')) { return { stdout: 'develop-sha\n', stderr: '' } } - if (args[0] === 'fetch') { + if (args.includes('fetch')) { throw new Error('network unavailable') } return { stdout: '', stderr: '' } @@ -2559,7 +2570,7 @@ describe('OrcaRuntimeService', () => { if (args[0] === 'rev-parse' && args.includes('refs/heads/team/feature^{commit}')) { return { stdout: 'team-feature-sha\n', stderr: '' } } - if (args[0] === 'fetch') { + if (args.includes('fetch')) { throw new Error('network unavailable') } return { stdout: '', stderr: '' } @@ -2580,7 +2591,18 @@ describe('OrcaRuntimeService', () => { false ) expect(gitSpy).not.toHaveBeenCalledWith( - ['fetch', '--no-tags', 'team', '+refs/heads/feature:refs/remotes/team/feature'], + [ + '-c', + 'maintenance.auto=false', + '-c', + 'maintenance.commit-graph.auto=0', + '-c', + 'gc.auto=0', + 'fetch', + '--no-tags', + 'team', + '+refs/heads/feature:refs/remotes/team/feature' + ], expect.any(Object) ) } finally { @@ -2604,7 +2626,7 @@ describe('OrcaRuntimeService', () => { if (args[0] === 'rev-parse' && args[1] === '--verify') { throw new Error('missing ref') } - if (args[0] === 'fetch') { + if (args.includes('fetch')) { throw new Error('network unavailable') } return { stdout: '', stderr: '' } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index a16fcca9f85..f420ebd0031 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -49,6 +49,7 @@ import { getClonePathComparisonKey } from '../git/repo-clone-path' import { getGitCloneFailureMessage } from '../../shared/git-clone-failure-message' +import { GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS } from '../../shared/git-fetch-auto-maintenance' import { createHash, randomUUID } from 'node:crypto' import { homedir } from 'node:os' import { isAbsolute, join, resolve } from 'node:path' @@ -14620,8 +14621,15 @@ export class OrcaRuntimeService { if (this.getFreshFetchCompletedAt(key) !== null) { return { ok: true } } + // Why: this exact refresh gates worktree create; ordinary fetches still own maintenance. return gitExecFileAsync( - ['fetch', '--no-tags', base.remote, `+refs/heads/${base.branch}:${base.ref}`], + [ + ...GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS, + 'fetch', + '--no-tags', + base.remote, + `+refs/heads/${base.branch}:${base.ref}` + ], { cwd: repoPath, ...gitOptions, diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 9e69339bfcd..6cceb5de7a7 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -10,7 +10,7 @@ import { spawn } from 'node:child_process' import { fileListingCancellationError } from '../shared/file-listing-cancellation' import type { SearchOptions, SearchResult } from './fs-handler-utils' import { buildGitLsFilesArgsForQuickOpen } from '../shared/quick-open-filter' -import { expandQuickOpenGitFilesWithNestedRepos } from '../shared/quick-open-readdir-walk' +import { expandQuickOpenGitFileListing } from '../shared/quick-open-readdir-walk' import { buildGitGrepArgs, buildSubmatchRegex, @@ -40,6 +40,7 @@ export function listFilesWithGit( return Promise.reject(fileListingCancellationError(signal)) } const gitPaths = new Set() + const directoryPaths = new Set() const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes) const children: { child: ReturnType @@ -56,7 +57,11 @@ export function listFilesWithGit( if (!path) { return } - gitPaths.add(path) + if (path.endsWith('/')) { + directoryPaths.add(path) + } else { + gitPaths.add(path) + } } const child = spawn('git', ['ls-files', ...args], { @@ -172,14 +177,18 @@ export function listFilesWithGit( signal?.addEventListener('abort', onAbort, { once: true }) return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]) - .then(() => - expandQuickOpenGitFilesWithNestedRepos({ + .then(async () => { + const files = await expandQuickOpenGitFileListing({ rootPath, gitPaths, + directoryPaths, excludePathPrefixes, signal }) - ) + // Why: directory placeholders are expanded after Git exits; restore + // Git's path order for empty queries and fuzzy-score ties over SSH. + return files.sort() + }) .catch((err) => { killSurvivors('git ls-files canceled after sibling failure') if (signal?.aborted) { diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 753ce02fddc..5a1804437bc 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -98,6 +98,8 @@ describe('relay quick open ignored file listing', () => { }) it('git fallback ignored pass includes ignored non-env files', async () => { + const root = await makeTempRoot() + await writeRel(root, 'dist/generated.js') const primaryProc = createMockProcess() const ignoredProc = createMockProcess() let callIndex = 0 @@ -107,7 +109,7 @@ describe('relay quick open ignored file listing', () => { return callIndex === 1 ? primaryProc : ignoredProc }) - const promise = listFilesWithGit('/remote/root', ['packages/other']) + const promise = listFilesWithGit(root, ['packages/other']) setTimeout(() => { ;(primaryProc.stdout as unknown as EventEmitter).emit( @@ -120,26 +122,28 @@ describe('relay quick open ignored file listing', () => { ) primaryProc.emit('close', 0, null) - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/\0') ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\0') ignoredProc.emit('close', 0, null) }, 10) - await expect(promise).resolves.toEqual(['src/index.ts', 'tab\tfile.txt', 'dist/generated.js']) + await expect(promise).resolves.toEqual(['dist/generated.js', 'src/index.ts', 'tab\tfile.txt']) const ignoredArgs = spawnMock.mock.calls[1][1] as string[] - expect(ignoredArgs).toEqual([ + expect(ignoredArgs.slice(0, 6)).toEqual([ 'ls-files', '-z', '-s', '--others', '--ignored', - '--exclude-standard', - '--', - '.', - ':(exclude,glob)packages/other', - ':(exclude,glob)packages/other/**' + '--exclude-standard' ]) + expect(ignoredArgs).toContain('--') + expect(ignoredArgs).toContain('.') + expect(ignoredArgs).toContain('--directory') + expect(ignoredArgs).toContain('--no-empty-directory') + expect(ignoredArgs).toContain(':(exclude,glob)packages/other') + expect(ignoredArgs).toContain(':(exclude,glob)packages/other/**') }) it('git fallback fills nested git repos returned as root-relative placeholders', async () => { diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index 07b84db43f9..ba8f631d815 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -1222,7 +1222,8 @@ describe('GitHandler', () => { worktreePath: tmpDir, remote: 'origin', branch: 'main', - ref: 'refs/remotes/origin/main' + ref: 'refs/remotes/origin/main', + skipAutoMaintenance: true }) const second = dispatcher.callRequest('git.diff', { @@ -1238,7 +1239,18 @@ describe('GitHandler', () => { expect(gitBufferSpy).toHaveBeenCalledTimes(2) expect(gitSpy).toHaveBeenCalledWith( - ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], + [ + '-c', + 'maintenance.auto=false', + '-c', + 'maintenance.commit-graph.auto=0', + '-c', + 'gc.auto=0', + 'fetch', + '--no-tags', + 'origin', + '+refs/heads/main:refs/remotes/origin/main' + ], tmpDir ) }) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index a5ae6d3cb7a..c878dfd563e 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -60,6 +60,7 @@ import { import { getGitCloneFailureMessage } from '../shared/git-clone-failure-message' import { syncForkDefaultBranch, validateGitForkSyncExpectedUpstream } from '../shared/git-fork-sync' import { InFlightPromiseDedupe, stableInFlightKey } from '../shared/in-flight-promise-dedupe' +import { GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS } from '../shared/git-fetch-auto-maintenance' const execFileAsync = promisify(execFile) const MAX_GIT_BUFFER = 10 * 1024 * 1024 @@ -844,10 +845,14 @@ export class GitHandler { const remote = params.remote const branch = params.branch const ref = params.ref + const skipAutoMaintenance = params.skipAutoMaintenance try { if (typeof remote !== 'string' || typeof branch !== 'string' || typeof ref !== 'string') { throw new Error('Invalid remote-tracking fetch request.') } + if (skipAutoMaintenance !== undefined && typeof skipAutoMaintenance !== 'boolean') { + throw new Error('Invalid remote-tracking fetch maintenance option.') + } if (remote.startsWith('-') || branch.startsWith('-')) { throw new Error('Remote-tracking fetch inputs must not start with "-".') } @@ -866,7 +871,16 @@ export class GitHandler { } await this.git(['check-ref-format', `refs/heads/${branch}`], worktreePath) await this.git(['check-ref-format', ref], worktreePath) - await this.git(['fetch', '--no-tags', remote, `+refs/heads/${branch}:${ref}`], worktreePath) + await this.git( + [ + ...(skipAutoMaintenance ? GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS : []), + 'fetch', + '--no-tags', + remote, + `+refs/heads/${branch}:${ref}` + ], + worktreePath + ) } catch (error) { // Why: create-worktree needs a write-capable fetch, but generic git.exec // intentionally rejects fetch. This narrow RPC keeps the relay allowlist diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 1cfe10cf9b7..0b18f5f3f09 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -1409,7 +1409,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { skipRestoreFocusRef.current = true // Why: selecting a project or repo group is a sidebar navigation action; // it should reveal the grouping row without activating an arbitrary workspace. - revealSidebarRow(result.rowKey, { highlight: true }) + revealSidebarRow(result.rowKey, { behavior: 'smooth', highlight: true }) recordFeatureInteraction('cmd-j') closeModal() setSelectedItemId('') diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index a206fa8c8f9..8f9eee91a8e 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -18,7 +18,8 @@ import { createProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks import { joinPath } from '@/lib/path' import { detectLanguage } from '@/lib/language-detect' import { setWithLRU } from '@/lib/scroll-cache' -import { getConnectionId, getConnectionIdForFile } from '@/lib/connection-context' +import { getConnectionIdForFile } from '@/lib/connection-context' +import { getCombinedDiffSectionConnectionId } from './combined-diff-section-connection' import { findWorktreeById } from '@/store/slices/worktree-helpers' import { writeRuntimeFile } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' @@ -622,7 +623,11 @@ export default function CombinedDiffViewer({ let result: GitDiffResult let error: string | undefined try { - const connectionId = getConnectionId(file.worktreeId) ?? undefined + const connectionId = getCombinedDiffSectionConnectionId( + file.worktreeId, + file.filePath, + entry.path + ) const state = useAppStore.getState() const fileSettings = settingsForRuntimeOwner(state.settings, file.runtimeEnvironmentId) if ((isBranchMode || (isAllMode && !('area' in entry))) && branchCompare) { diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index 71fe21cf983..ffb1aa36b9b 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -27,7 +27,7 @@ import { DiffSectionHeader } from './DiffSectionHeader' import type { DiffSection } from './diff-section-types' import type { DiffComment } from '../../../../shared/types' import { isDiffComment } from '@/lib/diff-comment-compat' -import { installEditorSaveShortcut } from './editor-shortcuts' +import { installEditorSaveShortcut, installMonacoEditorFindShortcut } from './editor-shortcuts' import { DiffSectionBody } from './DiffSectionBody' import { useDiffSectionLayoutMetrics } from './useDiffSectionLayoutMetrics' import { disposeUnattachedMonacoModelPaths } from './diff-monaco-model-disposal' @@ -343,9 +343,12 @@ export function DiffSectionItem({ } modifiedEditorsRef.current.set(index, modified) + const original = editor.getOriginalEditor() const cleanupSaveShortcut = installEditorSaveShortcut(modified.getContainerDomNode(), () => handleSectionSaveRef.current(index) ) + const cleanupOriginalFindShortcut = installMonacoEditorFindShortcut(original) + const cleanupModifiedFindShortcut = installMonacoEditorFindShortcut(modified) const modelContentSub = modified.onDidChangeModelContent(() => { const current = modified.getValue() setSections((prev) => { @@ -380,9 +383,11 @@ export function DiffSectionItem({ }) }) modified.onDidDispose(() => { - // Why: editable diff sections own both the save shortcut and model-change - // subscription for this Monaco editor instance. + // Why: editable diff sections own both panes' shortcut bridges and the + // model subscription for the lifetime of this Monaco diff instance. cleanupSaveShortcut() + cleanupOriginalFindShortcut() + cleanupModifiedFindShortcut() modelContentSub.dispose() }) } diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index 7f58688754f..9aae498c1c1 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -16,7 +16,7 @@ import { import { applyDiffEditorLineNumberOptions } from './diff-editor-line-number-options' import type { DiffComment } from '../../../../shared/types' import { isDiffComment } from '@/lib/diff-comment-compat' -import { installEditorSaveShortcut } from './editor-shortcuts' +import { installEditorSaveShortcut, installMonacoEditorFindShortcut } from './editor-shortcuts' import { diffEditorScrollbarOptions } from './diff-editor-scrollbar-options' import { LargeDiffFallback } from './LargeDiffFallback' import { getLargeDiffRenderLimit } from './large-diff-render-limit' @@ -336,15 +336,19 @@ export default function DiffViewer({ onSaveRef.current?.(modifiedEditor.getValue()) } ) + const cleanupOriginalFindShortcut = installMonacoEditorFindShortcut(originalEditor) + const cleanupModifiedFindShortcut = installMonacoEditorFindShortcut(modifiedEditor) // Track changes const modelContentSub = modifiedEditor.onDidChangeModelContent(() => { onContentChangeRef.current?.(modifiedEditor.getValue()) }) modifiedEditor.onDidDispose(() => { - // Why: editable diff views own both the save shortcut and - // model-change subscription for this Monaco editor instance. + // Why: editable diff views own both panes' shortcut bridges and the + // model subscription for the lifetime of this Monaco diff instance. cleanupSaveShortcut() + cleanupOriginalFindShortcut() + cleanupModifiedFindShortcut() modelContentSub.dispose() }) diff --git a/src/renderer/src/components/editor/IpynbViewer.tsx b/src/renderer/src/components/editor/IpynbViewer.tsx index d813e3ff90b..fc19ca04175 100644 --- a/src/renderer/src/components/editor/IpynbViewer.tsx +++ b/src/renderer/src/components/editor/IpynbViewer.tsx @@ -51,7 +51,11 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip import { ShortcutKeyCombo } from '@/components/ShortcutKeyCombo' import { useShortcutKeyDetails, type ShortcutKeyComboDetails } from '@/hooks/useShortcutLabel' import { registerPendingEditorFlush } from './editor-pending-flush' -import { editorShortcutMatches, installEditorSaveShortcut } from './editor-shortcuts' +import { + editorShortcutMatches, + installEditorSaveShortcut, + installMonacoEditorFindShortcut +} from './editor-shortcuts' import { getIpynbCodeCellEditorHeight, getIpynbCodeCellPreviewLines } from './ipynb-code-cell-lines' import MonacoCodeExcerpt from './MonacoCodeExcerpt' import { @@ -349,13 +353,15 @@ function CodeCell({ void onSaveRequestRef.current() } ) + const cleanupFindShortcut = installMonacoEditorFindShortcut(editorInstance) const blurSub = editorInstance.onDidBlurEditorWidget(() => { onDeactivateRef.current() }) editorInstance.onDidDispose(() => { - // Why: the inline source editor owns both the save shortcut and blur - // subscription for this Monaco editor instance. + // Why: the inline source editor owns its shortcut bridges and blur + // subscription for the lifetime of this Monaco editor instance. cleanupSaveShortcut() + cleanupFindShortcut() blurSub.dispose() }) editorInstance.addCommand(monacoInstance.KeyCode.Escape, () => { diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index bf901883143..d99af690af5 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -44,7 +44,7 @@ import { getDiffCommentPopoverTop } from '../diff-comments/diff-comment-popover-position' import { isLinuxUserAgent } from '../terminal-pane/pane-helpers' -import { installEditorSaveShortcut } from './editor-shortcuts' +import { installEditorSaveShortcut, installMonacoEditorFindShortcut } from './editor-shortcuts' import { Plus } from 'lucide-react' import { getMonacoMarkdownSelectionAnnotationTarget, @@ -390,13 +390,12 @@ export default function MonacoEditor({ return model.getValueInRange(selection) }) - const cleanupSaveShortcut = installEditorSaveShortcut( - editorInstance.getContainerDomNode(), - () => { - const value = editorInstance.getValue() - propsRef.current.onSave(value) - } - ) + const editorDomNode = editorInstance.getContainerDomNode() + const cleanupSaveShortcut = installEditorSaveShortcut(editorDomNode, () => { + const value = editorInstance.getValue() + propsRef.current.onSave(value) + }) + const cleanupFindShortcut = installMonacoEditorFindShortcut(editorInstance) const searchInFilesAction = editorInstance.addAction({ id: 'orca.searchInFiles', label: translate('auto.components.editor.MonacoEditor.fd68ae03b3', 'Search in Files'), @@ -442,7 +441,6 @@ export default function MonacoEditor({ } }) } - const editorDomNode = editorInstance.getContainerDomNode() editorDomNode.addEventListener('paste', onLargeTextPaste, { capture: true }) // Track cursor line for "copy path to line" feature @@ -496,6 +494,7 @@ export default function MonacoEditor({ scrollStateSub.dispose() gutterMouseDownSub.dispose() cleanupSaveShortcut() + cleanupFindShortcut() editorDomNode.removeEventListener('paste', onLargeTextPaste, { capture: true }) searchInFilesAction.dispose() autoHeightSub?.dispose() diff --git a/src/renderer/src/components/editor/combined-diff-section-connection.test.ts b/src/renderer/src/components/editor/combined-diff-section-connection.test.ts new file mode 100644 index 00000000000..894f6df7830 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff-section-connection.test.ts @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types' +import { useAppStore } from '@/store' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' +import { getCombinedDiffSectionConnectionId } from './combined-diff-section-connection' + +const initialState = useAppStore.getInitialState() + +function makeRepo(overrides: Partial & { id: string }): Repo { + return { + path: '/home/neil/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + ...overrides + } +} + +function makeFolderWorkspace(folderPath: string): FolderWorkspace { + return { + id: 'folder-workspace-1', + projectGroupId: 'group-1', + name: 'Platform workspace', + folderPath, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 0, + createdAt: 1, + updatedAt: 1 + } +} + +function makeGroup(parentPath: string): ProjectGroup { + return { + id: 'group-1', + name: 'Platform', + parentPath, + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } +} + +describe('getCombinedDiffSectionConnectionId', () => { + afterEach(() => { + useAppStore.setState(initialState, true) + }) + + it('routes a section to its SSH child repo in a mixed folder workspace (#6688)', () => { + const workspaceKey = folderWorkspaceKey('folder-workspace-1') + useAppStore.setState({ + folderWorkspaces: [makeFolderWorkspace('/home/neil/platform')], + projectGroups: [makeGroup('/home/neil/platform')], + repos: [ + makeRepo({ id: 'repo-local', path: '/home/neil/platform/web', projectGroupId: 'group-1' }), + makeRepo({ + id: 'repo-ssh', + path: '/home/neil/platform/api', + projectGroupId: 'group-1', + connectionId: 'ssh-1' + }) + ], + worktreesByRepo: {} + }) + + // Section paths are repo-tree-relative; joined onto the workspace root they + // must resolve to the owning child repo, not the ambiguous workspace. + expect( + getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'api/src/index.ts') + ).toBe('ssh-1') + // Local child repo -> no connection (local read). + expect( + getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'web/src/index.ts') + ).toBeUndefined() + // A path owned by no single child repo stays ambiguous. + expect( + getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'README.md') + ).toBeUndefined() + }) + + it('composes Windows section paths with the workspace root separator', () => { + const workspaceKey = folderWorkspaceKey('folder-workspace-1') + // A mixed local+SSH workspace makes the whole-workspace owner ambiguous, so + // resolution must fall through to composing the section path — the behavior + // this test guards. A single-SSH workspace would resolve before joinPath. + useAppStore.setState({ + folderWorkspaces: [makeFolderWorkspace('C:\\Users\\neil\\platform')], + projectGroups: [makeGroup('C:\\Users\\neil\\platform')], + repos: [ + makeRepo({ + id: 'repo-local', + path: 'C:\\Users\\neil\\platform\\web', + projectGroupId: 'group-1' + }), + makeRepo({ + id: 'repo-ssh', + path: 'C:\\Users\\neil\\platform\\api', + projectGroupId: 'group-1', + connectionId: 'ssh-1' + }) + ], + worktreesByRepo: {} + }) + + expect( + getCombinedDiffSectionConnectionId( + workspaceKey, + 'C:\\Users\\neil\\platform', + 'api/src/index.ts' + ) + ).toBe('ssh-1') + expect( + getCombinedDiffSectionConnectionId( + workspaceKey, + 'C:\\Users\\neil\\platform', + 'web/src/index.ts' + ) + ).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/editor/combined-diff-section-connection.ts b/src/renderer/src/components/editor/combined-diff-section-connection.ts new file mode 100644 index 00000000000..b330ba4c13a --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff-section-connection.ts @@ -0,0 +1,17 @@ +import { getConnectionIdForFile } from '@/lib/connection-context' +import { joinPath } from '@/lib/path' + +/** + * Resolve the SSH connectionId that owns a combined-diff section's file. + * + * Why: folder workspaces mix local and SSH child repos, so the workspace + * worktree is an ambiguous owner; the section must resolve its host by the + * concrete child-repo path or a remote read is misrouted locally (#6688). + */ +export function getCombinedDiffSectionConnectionId( + worktreeId: string | null, + worktreeRootPath: string, + sectionPath: string +): string | undefined { + return getConnectionIdForFile(worktreeId, joinPath(worktreeRootPath, sectionPath)) ?? undefined +} diff --git a/src/renderer/src/components/editor/editor-shortcuts.test.ts b/src/renderer/src/components/editor/editor-shortcuts.test.ts new file mode 100644 index 00000000000..daacea7f647 --- /dev/null +++ b/src/renderer/src/components/editor/editor-shortcuts.test.ts @@ -0,0 +1,198 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const shortcutState = vi.hoisted(() => ({ + keybindings: {} as Record, + platform: 'darwin' as NodeJS.Platform +})) + +vi.mock('@/lib/shortcut-platform', () => ({ + getShortcutPlatform: () => shortcutState.platform +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ keybindings: shortcutState.keybindings }) + } +})) + +import { installEditorFindShortcut, installMonacoEditorFindShortcut } from './editor-shortcuts' + +type ShortcutFixture = { + container: HTMLDivElement + dispose: () => void + input: HTMLTextAreaElement + onDownstreamKeyDown: ReturnType + onFind: ReturnType +} + +function createShortcutFixture(): ShortcutFixture { + const container = document.createElement('div') + const input = document.createElement('textarea') + const onDownstreamKeyDown = vi.fn() + const onFind = vi.fn() + + container.appendChild(input) + document.body.appendChild(container) + input.addEventListener('keydown', onDownstreamKeyDown) + + return { + container, + dispose: installEditorFindShortcut(container, onFind), + input, + onDownstreamKeyDown, + onFind + } +} + +function dispatchKeyDown(target: HTMLElement, init: KeyboardEventInit): KeyboardEvent { + const event = new KeyboardEvent('keydown', { + ...init, + bubbles: true, + cancelable: true + }) + target.dispatchEvent(event) + return event +} + +beforeEach(() => { + shortcutState.keybindings = {} + shortcutState.platform = 'darwin' +}) + +afterEach(() => { + document.body.replaceChildren() +}) + +describe('installEditorFindShortcut', () => { + it.each([ + { label: 'macOS', platform: 'darwin' as const, modifier: { metaKey: true } }, + { label: 'Linux', platform: 'linux' as const, modifier: { ctrlKey: true } }, + { label: 'Windows', platform: 'win32' as const, modifier: { ctrlKey: true } } + ])('matches logical f on physical KeyU for $label', ({ platform, modifier }) => { + shortcutState.platform = platform + const fixture = createShortcutFixture() + + const event = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyU', + ...modifier + }) + + expect(event.defaultPrevented).toBe(true) + expect(fixture.onFind).toHaveBeenCalledTimes(1) + expect(fixture.onDownstreamKeyDown).not.toHaveBeenCalled() + fixture.dispose() + }) + + it('consumes the QWERTY shortcut before Monaco can handle it again', () => { + const fixture = createShortcutFixture() + + const event = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyF', + metaKey: true + }) + + expect(event.defaultPrevented).toBe(true) + expect(fixture.onFind).toHaveBeenCalledTimes(1) + expect(fixture.onDownstreamKeyDown).not.toHaveBeenCalled() + fixture.dispose() + }) + + it('consumes matched repeats without invoking find again', () => { + const fixture = createShortcutFixture() + + const initialEvent = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyF', + metaKey: true + }) + const repeatEvent = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyF', + metaKey: true, + repeat: true + }) + + expect(initialEvent.defaultPrevented).toBe(true) + expect(repeatEvent.defaultPrevented).toBe(true) + expect(fixture.onFind).toHaveBeenCalledTimes(1) + expect(fixture.onDownstreamKeyDown).not.toHaveBeenCalled() + fixture.dispose() + }) + + it.each([ + { label: 'ordinary f typing', init: { key: 'f', code: 'KeyU' } }, + { + label: 'an unrelated shortcut', + init: { key: 'g', code: 'KeyG', metaKey: true } + } + ])('leaves $label untouched', ({ init }) => { + const fixture = createShortcutFixture() + + const event = dispatchKeyDown(fixture.input, init) + + expect(event.defaultPrevented).toBe(false) + expect(fixture.onFind).not.toHaveBeenCalled() + expect(fixture.onDownstreamKeyDown).toHaveBeenCalledTimes(1) + fixture.dispose() + }) + + it('honors a custom editor.find binding', () => { + shortcutState.keybindings = { 'editor.find': ['Mod+G'] } + const fixture = createShortcutFixture() + + const defaultEvent = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyF', + metaKey: true + }) + const customEvent = dispatchKeyDown(fixture.input, { + key: 'g', + code: 'KeyU', + metaKey: true + }) + + expect(defaultEvent.defaultPrevented).toBe(false) + expect(customEvent.defaultPrevented).toBe(true) + expect(fixture.onFind).toHaveBeenCalledTimes(1) + expect(fixture.onDownstreamKeyDown).toHaveBeenCalledTimes(1) + fixture.dispose() + }) + + it('removes the listener when disposed', () => { + const fixture = createShortcutFixture() + fixture.dispose() + + const event = dispatchKeyDown(fixture.input, { + key: 'f', + code: 'KeyU', + metaKey: true + }) + + expect(event.defaultPrevented).toBe(false) + expect(fixture.onFind).not.toHaveBeenCalled() + expect(fixture.onDownstreamKeyDown).toHaveBeenCalledTimes(1) + }) + + it('runs Monaco existing find action through the shared bridge', () => { + const container = document.createElement('div') + const input = document.createElement('textarea') + const run = vi.fn() + const getAction = vi.fn((_id: string) => ({ run })) + container.appendChild(input) + document.body.appendChild(container) + const dispose = installMonacoEditorFindShortcut({ + getAction, + getContainerDomNode: () => container + }) + + dispatchKeyDown(input, { key: 'f', code: 'KeyU', metaKey: true }) + + expect(getAction).toHaveBeenCalledWith('actions.find') + expect(run).toHaveBeenCalledTimes(1) + dispose() + }) +}) diff --git a/src/renderer/src/components/editor/editor-shortcuts.ts b/src/renderer/src/components/editor/editor-shortcuts.ts index 66f30c493b2..86a7e721226 100644 --- a/src/renderer/src/components/editor/editor-shortcuts.ts +++ b/src/renderer/src/components/editor/editor-shortcuts.ts @@ -28,3 +28,31 @@ export function installEditorSaveShortcut(target: HTMLElement, onSave: () => voi target.addEventListener('keydown', handleKeyDown, true) return () => target.removeEventListener('keydown', handleKeyDown, true) } + +export function installEditorFindShortcut(target: HTMLElement, onFind: () => void): () => void { + const handleKeyDown = (event: KeyboardEvent): void => { + if (!editorShortcutMatches('editor.find', event)) { + return + } + event.preventDefault() + event.stopPropagation() + // Why: matched repeats must stay consumed so Monaco cannot reopen or reset find. + if (!event.repeat) { + onFind() + } + } + + target.addEventListener('keydown', handleKeyDown, true) + return () => target.removeEventListener('keydown', handleKeyDown, true) +} + +type MonacoFindShortcutEditor = { + getAction: (id: string) => { run: () => void | Promise } | null + getContainerDomNode: () => HTMLElement +} + +export function installMonacoEditorFindShortcut(editor: MonacoFindShortcutEditor): () => void { + return installEditorFindShortcut(editor.getContainerDomNode(), () => { + void editor.getAction('actions.find')?.run() + }) +} diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx index 15cd29adb44..90807e66cc7 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx @@ -10,6 +10,7 @@ import type { DiffContent, FileContent } from './editor-panel-content-types' const mocks = vi.hoisted(() => ({ readRuntimeFileContent: vi.fn(), getRuntimeGitDiff: vi.fn(), + getRuntimeGitBranchDiff: vi.fn(), getConnectionId: vi.fn(), getConnectionIdForFile: vi.fn(), isWorktreeConnectionResolved: vi.fn(() => true), @@ -28,7 +29,7 @@ vi.mock('@/runtime/runtime-file-client', () => ({ })) vi.mock('@/runtime/runtime-git-client', () => ({ - getRuntimeGitBranchDiff: vi.fn(), + getRuntimeGitBranchDiff: mocks.getRuntimeGitBranchDiff, getRuntimeGitCommitDiff: vi.fn(), getRuntimeGitDiff: mocks.getRuntimeGitDiff, getRuntimeGitScope: vi.fn(() => null) @@ -131,6 +132,7 @@ describe('useEditorPanelContentState', () => { latestDiffContents = {} mocks.readRuntimeFileContent.mockReset() mocks.getRuntimeGitDiff.mockReset() + mocks.getRuntimeGitBranchDiff.mockReset() mocks.getConnectionId.mockReset() mocks.getConnectionId.mockReturnValue(undefined) mocks.getConnectionIdForFile.mockReset() @@ -188,6 +190,60 @@ describe('useEditorPanelContentState', () => { ) }) + it('loads folder workspace branch diffs through the path-specific SSH connection', async () => { + const activeFile = createOpenFile({ + id: 'branch-diff', + filePath: '/home/neil/platform/api/src/file.ts', + relativePath: 'api/src/file.ts', + worktreeId: 'folder:folder-workspace-1', + mode: 'diff', + diffSource: 'branch', + branchCompare: { + baseRef: 'main', + compareRef: 'feature', + compareVersion: 'feature', + baseOid: 'base', + headOid: 'head', + mergeBase: 'merge-base' + } + }) + mocks.getConnectionIdForFile.mockReturnValue('ssh-1') + mocks.getRuntimeGitBranchDiff.mockResolvedValue({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'remote branch diff', + originalIsBinary: false, + modifiedIsBinary: false + }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + + await act(async () => { + root?.render() + }) + + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('remote branch diff') + ) + expect(mocks.getConnectionIdForFile).toHaveBeenCalledWith( + 'folder:folder-workspace-1', + '/home/neil/platform/api/src/file.ts' + ) + expect(mocks.getRuntimeGitBranchDiff).toHaveBeenCalledWith( + expect.objectContaining({ + worktreeId: 'folder:folder-workspace-1', + worktreePath: '/home/neil/platform', + connectionId: 'ssh-1' + }), + expect.objectContaining({ + compare: expect.objectContaining({ mergeBase: 'merge-base' }), + filePath: 'api/src/file.ts' + }) + ) + }) + it('does not read locally while a remote host worktree owner is still hydrating (#6648)', async () => { const activeFile = createOpenFile({ filePath: '/home/user/project/src/index.ts', diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index ae05de95475..f4f668a5a4c 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -3,11 +3,7 @@ make the hook coordination harder to audit. */ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { OpenFile } from '@/store/slices/editor' -import { - getConnectionId, - getConnectionIdForFile, - isWorktreeConnectionResolved -} from '@/lib/connection-context' +import { getConnectionIdForFile, isWorktreeConnectionResolved } from '@/lib/connection-context' import { joinPath } from '@/lib/path' import { useAppStore } from '@/store' import { getDiskBaselineSignature } from './diff-content-signature' @@ -224,7 +220,7 @@ export function useEditorPanelContentState({ ? file.branchCompare : null const commitCompare = file.commitCompare?.commitOid ? file.commitCompare : null - const connectionId = getConnectionId(file.worktreeId) ?? undefined + const connectionId = getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined const activeSettings = useAppStore.getState().settings const fileSettings = settingsForRuntimeOwner(activeSettings, file.runtimeEnvironmentId) const gitScope = getRuntimeGitScope(fileSettings, connectionId) diff --git a/src/renderer/src/components/settings/VoiceSpeechModelSection.test.tsx b/src/renderer/src/components/settings/VoiceSpeechModelSection.test.tsx index b6e061994ae..4825397fbaf 100644 --- a/src/renderer/src/components/settings/VoiceSpeechModelSection.test.tsx +++ b/src/renderer/src/components/settings/VoiceSpeechModelSection.test.tsx @@ -8,6 +8,7 @@ import type { SpeechModelManifest, SpeechModelState } from '../../../../shared/s import { getDefaultVoiceSettings } from '../../../../shared/constants' const toastErrorMock = vi.hoisted(() => vi.fn()) +const menuDismissMock = vi.hoisted(() => vi.fn()) vi.mock('sonner', () => ({ toast: { @@ -32,7 +33,7 @@ vi.mock('../ui/dropdown-menu', () => ({ }: { children: ReactNode disabled?: boolean - onSelect?: () => void + onSelect?: (event: Event) => void className?: string }) => (
({ role="option" onClick={() => { if (!disabled) { - onSelect?.() + const selectEvent = new Event('select', { cancelable: true }) + onSelect?.(selectEvent) + if (!selectEvent.defaultPrevented) { + menuDismissMock() + } } }} > @@ -73,6 +78,7 @@ const secondLocalModel: SpeechModelManifest = { function renderSection(args: { deleteModel: (modelId: string) => Promise + downloadModel?: (modelId: string) => Promise catalog?: SpeechModelManifest[] modelStates?: SpeechModelState[] refreshModelStates?: () => void @@ -81,7 +87,7 @@ function renderSection(args: { api: { speech: { deleteModel: vi.fn(args.deleteModel), - downloadModel: vi.fn() + downloadModel: vi.fn(args.downloadModel ?? (() => Promise.resolve())) } } }) @@ -111,6 +117,7 @@ function renderSection(args: { describe('VoiceSpeechModelSection', () => { beforeEach(() => { toastErrorMock.mockReset() + menuDismissMock.mockReset() }) afterEach(() => { @@ -218,4 +225,21 @@ describe('VoiceSpeechModelSection', () => { expect(refreshModelStates).not.toHaveBeenCalled() root.unmount() }) + + it('keeps the model menu open when starting a local model download', async () => { + const { container, root } = renderSection({ + deleteModel: () => Promise.resolve(), + modelStates: [{ id: localModel.id, status: 'not-downloaded' }] + }) + const modelOption = container.querySelector('[role="option"]') + + await act(async () => { + modelOption!.dispatchEvent(new MouseEvent('click', { bubbles: true })) + await Promise.resolve() + }) + + expect(window.api.speech.downloadModel).toHaveBeenCalledWith(localModel.id) + expect(menuDismissMock).not.toHaveBeenCalled() + root.unmount() + }) }) diff --git a/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx b/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx index 0380bbc0ce8..ea96c68fd39 100644 --- a/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx +++ b/src/renderer/src/components/settings/VoiceSpeechModelSection.tsx @@ -82,12 +82,14 @@ export function VoiceSpeechModelSection({ { + onSelect={(event) => { if (isReady) { onUpdateVoiceSettings({ sttModel: manifest.id }) } else if (isCloud) { onOpenOpenAiDialog(manifest.id) } else if (!isDownloading) { + // Why: download progress appears in this menu, so starting one should not dismiss it. + event.preventDefault() void window.api.speech .downloadModel(manifest.id) .catch(() => diff --git a/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx b/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx index 33de6b6eb1d..5ec3354ec22 100644 --- a/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx +++ b/src/renderer/src/components/sidebar/NonGitFolderDialog.tsx @@ -82,7 +82,10 @@ const NonGitFolderDialog = React.memo(function NonGitFolderDialog() { onboarding, hadProjectBeforeAdd ) - activateAndRevealWorktree(folderWorktree.id, startup ? { startup } : undefined) + activateAndRevealWorktree(folderWorktree.id, { + sidebarRevealBehavior: 'auto', + ...(startup ? { startup } : {}) + }) } } catch (err) { // This code path calls addRemote directly (not through the store), diff --git a/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx b/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx index 66409526770..f3580472439 100644 --- a/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx +++ b/src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx @@ -145,7 +145,9 @@ describe('ProjectAddedDialog', () => { expect(markup).toBe('') expect(mocks.state.fetchWorktrees).toHaveBeenCalledWith('repo-1') - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('repo-1::folder') + expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('repo-1::folder', { + sidebarRevealBehavior: 'auto' + }) expect(mocks.state.closeModal).toHaveBeenCalledTimes(1) expect(mocks.finishProjectAddWithDefaultCheckout).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx b/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx index 34f53ffd6ac..6532a39cae8 100644 --- a/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx +++ b/src/renderer/src/components/sidebar/ProjectAddedDialog.tsx @@ -81,7 +81,7 @@ export default function ProjectAddedDialog(): null { } const folderWorktree = useAppStore.getState().worktreesByRepo[repoId]?.[0] if (folderWorktree) { - activateAndRevealWorktree(folderWorktree.id) + activateAndRevealWorktree(folderWorktree.id, { sidebarRevealBehavior: 'auto' }) } closeModal() })() diff --git a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx index 033cb286dfa..7bfda38d231 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx @@ -36,7 +36,7 @@ function revealCompactAgentCard(agentListRoot: HTMLElement | null): void { if (!(sidebarElement instanceof HTMLElement) || !worktreeOptionElement) { return } - revealElementInScrollContainer(sidebarElement, worktreeOptionElement) + revealElementInScrollContainer(sidebarElement, worktreeOptionElement, 'auto') } type Props = { @@ -238,7 +238,7 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ // Why: defer the reveal scroll out of the expand commit. Running it inline // forces a synchronous sidebar layout that blocks the animation's opening // frames (a visible jump); next-frame keeps the open smooth and the - // instant reveal still lands before the height transition finishes. + // ScrollBehavior 'auto' still lands before the height transition finishes. const handle = requestAnimationFrame(() => { revealCompactAgentCard(compactAgentListRootRef.current) }) diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 84f5ab77838..e9403b41b11 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -470,15 +470,31 @@ function markSidebarWorktreeActiveImmediately(worktreeId: string, primaryRowKey? } } +function revealMountedWorktreeElement( + container: HTMLElement, + worktreeId: string, + behavior: ScrollBehavior, + optionId?: string +): HTMLElement | null { + const element = optionId + ? document.getElementById(optionId) + : getMountedWorktreeOptions(worktreeId, container)[0] + if (!element || !container.contains(element)) { + return null + } + return revealElementInScrollContainer(container, element, behavior) ? element : null +} + function revealMountedSidebarRowElement( container: HTMLElement, - rowKey: string + rowKey: string, + behavior: ScrollBehavior ): HTMLElement | null { const element = document.getElementById(getWorktreeOptionId(rowKey)) if (!element || !container.contains(element)) { return null } - return revealElementInScrollContainer(container, element) ? element : null + return revealElementInScrollContainer(container, element, behavior) ? element : null } function getRenderRowSidebarKey(row: RenderRow): string | null { @@ -2119,15 +2135,14 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp clearPendingRevealWorktreeId() } } - const optionId = getRenderRowOptionId(targetRow, pendingRevealWorktree.worktreeId) - const option = optionId - ? document.getElementById(optionId) - : getMountedWorktreeOptions(pendingRevealWorktree.worktreeId, container)[0] - const mountedOption = container && option && container.contains(option) ? option : null - const revealedOption = - container && mountedOption && revealElementInScrollContainer(container, mountedOption) - ? mountedOption - : null + const revealedOption = container + ? revealMountedWorktreeElement( + container, + pendingRevealWorktree.worktreeId, + pendingRevealWorktree.behavior, + getRenderRowOptionId(targetRow, pendingRevealWorktree.worktreeId) + ) + : null if (revealedOption) { if (pendingRevealWorktree.highlight) { const revealedRowKey = @@ -2283,7 +2298,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp const container = scrollRef.current const revealedElement = container - ? revealMountedSidebarRowElement(container, pendingRevealSidebarRow.rowKey) + ? revealMountedSidebarRowElement( + container, + pendingRevealSidebarRow.rowKey, + pendingRevealSidebarRow.behavior + ) : null if (revealedElement) { if (pendingRevealSidebarRow.highlight) { @@ -6641,6 +6660,7 @@ const WorktreeList = React.memo(function WorktreeList({ { target: { type: 'sidebar-row' } } > revealSidebarRow(detail.target.rowKey, { + behavior: 'smooth', highlight: sidebarDetail.highlight !== false }) return @@ -6662,6 +6682,7 @@ const WorktreeList = React.memo(function WorktreeList({ clearFilters() } revealWorktreeInSidebar(currentSidebarWorktreeId, { + behavior: 'smooth', highlight: true, beginRename: (detail as { beginRename?: boolean } | undefined)?.beginRename === true }) diff --git a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts index 83caaaa6d15..51a5ad755b5 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts @@ -213,7 +213,9 @@ describe('useCreateRepo default-checkout handoff', () => { kind: 'git' }) expect(mocks.fetchWorktrees).toHaveBeenCalledWith(repo.id) - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith(worktree.id) + expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith(worktree.id, { + sidebarRevealBehavior: 'auto' + }) expect(mocks.markOnboardingProjectAdded).toHaveBeenCalledWith('addedFolder') expect(closeModal).toHaveBeenCalled() expect(mocks.onGitRepoReady).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/sidebar/useCreateRepo.ts b/src/renderer/src/components/sidebar/useCreateRepo.ts index eb42f347e6f..abf35db7e66 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.ts @@ -190,7 +190,7 @@ export function useCreateRepo( } const folderWorktree = useAppStore.getState().worktreesByRepo[repo.id]?.[0] if (folderWorktree) { - activateAndRevealWorktree(folderWorktree.id) + activateAndRevealWorktree(folderWorktree.id, { sidebarRevealBehavior: 'auto' }) } await markOnboardingProjectAdded('addedFolder') closeModal() diff --git a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts index aa71d479445..924a4edc88a 100644 --- a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts @@ -6,7 +6,6 @@ import { WORKTREE_SIDEBAR_REVEAL_TOP_INSET, shouldAdjustWorktreeSidebarMeasuredRowScroll } from './WorktreeList' -import { revealElementInScrollContainer } from './worktree-sidebar-reveal' import { extractWorktreeVirtualRowIndexes, estimateRenderRowSize, @@ -44,11 +43,8 @@ const makeImportedCardRow = (): Extract ({ scrollTop, clientHeight, scrollHeight }) as HTMLElement +const makeScrollContainer = (scrollTop: number, clientHeight: number): HTMLElement => + ({ scrollTop, clientHeight }) as HTMLElement describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => { it('counts record keys once per object reference', () => { @@ -125,52 +121,64 @@ describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => { }) describe('getScrollTopToRevealBounds', () => { - it('centers a fully visible target', () => { + it('treats the sticky header as occluding the viewport top', () => { const container = makeScrollContainer(100, 400) expect( getScrollTopToRevealBounds( container, { - start: 300, - end: 400 + start: 100, + end: 216 + }, + GROUP_HEADER_ROW_HEIGHT + ) + ).toBe(72) + }) + + it('includes extra reveal clearance for the highlight ring', () => { + const container = makeScrollContainer(100, 400) + + expect( + getScrollTopToRevealBounds( + container, + { + start: 100, + end: 216 }, WORKTREE_SIDEBAR_REVEAL_TOP_INSET ) - ).toBe(133) + ).toBe(66) + }) + + it('does not scroll when the bounds are below the sticky header', () => { + const container = makeScrollContainer(100, 400) + + expect( + getScrollTopToRevealBounds( + container, + { + start: 128, + end: 244 + }, + GROUP_HEADER_ROW_HEIGHT + ) + ).toBeNull() }) - it('centers within the area below the sticky header', () => { + it('keeps the viewport bottom independent of the sticky header inset', () => { const container = makeScrollContainer(100, 400) expect( getScrollTopToRevealBounds( container, { - start: 300, - end: 400 + start: 430, + end: 520 }, GROUP_HEADER_ROW_HEIGHT ) - ).toBe(136) - }) - - it('clamps a boundary target to the list edge instead of padding the list', () => { - // Regression (#8019 follow-up): centering first/last rows via temporary - // paddingStart/paddingEnd left a permanent phantom gap in the sidebar once - // the reveal finished, since nothing cleared the padding afterwards. - const container = { - scrollTop: 100, - clientHeight: 200, - contains: () => true, - getBoundingClientRect: () => ({ top: 0, bottom: 200 }) - } as unknown as HTMLElement - const firstRow = { - getBoundingClientRect: () => ({ top: -40, bottom: 20 }) - } as unknown as Element - - expect(revealElementInScrollContainer(container, firstRow)).toBe(true) - expect(container.scrollTop).toBe(0) + ).toBe(120) }) }) diff --git a/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts b/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts index e2ec2cfaffd..933261f20a4 100644 --- a/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts +++ b/src/renderer/src/components/sidebar/worktree-scroll-to-current-button.test.ts @@ -8,17 +8,15 @@ describe('getScrollTopToRevealBounds', () => { clientHeight }) as HTMLElement - it('centers a mounted current workspace card that starts above the viewport', () => { - // Raw result may be negative; revealElementInScrollContainer clamps it to - // the list edge so boundary reveals never pad the list. - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(-10) + it('scrolls upward to reveal a mounted current workspace card above the viewport', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(60) }) - it('centers a mounted current workspace card that starts below the viewport', () => { - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(195) + it('scrolls downward to reveal a mounted current workspace card below the viewport', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(140) }) - it('recenters a card that is already fully visible', () => { - expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 125, end: 260 })).toBe(92.5) + it('does not scroll when the current workspace card is already fully visible', () => { + expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 125, end: 260 })).toBeNull() }) }) diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts index 56586682a50..38b158532af 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-reveal.ts @@ -22,16 +22,24 @@ export function getScrollTopToRevealBounds( container: HTMLElement, bounds: SidebarRevealBounds, topInset = 0 -): number { +): number | null { const viewportTopInset = Math.max(0, Math.min(container.clientHeight, topInset)) - // Why: the sticky header reduces the usable viewport, so center within the - // visible area below it instead of behind it. - const targetCenter = bounds.start + (bounds.end - bounds.start) / 2 - const viewportCenterOffset = (viewportTopInset + container.clientHeight) / 2 - return targetCenter - viewportCenterOffset + const viewportTop = container.scrollTop + viewportTopInset + const viewportBottom = container.scrollTop + container.clientHeight + if (bounds.start < viewportTop) { + return bounds.start - viewportTopInset + } + if (bounds.end > viewportBottom) { + return bounds.end - container.clientHeight + } + return null } -export function revealElementInScrollContainer(container: HTMLElement, element: Element): boolean { +export function revealElementInScrollContainer( + container: HTMLElement, + element: Element, + behavior: ScrollBehavior +): boolean { if (!container.contains(element)) { return false } @@ -40,9 +48,17 @@ export function revealElementInScrollContainer(container: HTMLElement, element: getElementScrollBounds(container, element), WORKTREE_SIDEBAR_REVEAL_TOP_INSET ) - // Why: sidebar reveal is a focus handoff, so reposition immediately instead - // of making the user track an animated list. Boundary rows clamp to the list - // edge — padding the list so they can center leaves a phantom gap behind. - container.scrollTop = Math.max(0, nextScrollTop) + if (nextScrollTop === null) { + return true + } + // Why: honor the user's reduced-motion preference by jumping instantly instead of + // animating a smooth scroll (also makes the reveal deterministic in headless + // environments that never tick the smooth-scroll animation). + const prefersReducedMotion = + typeof window !== 'undefined' && + window.matchMedia?.('(prefers-reduced-motion: reduce)').matches === true + const resolvedBehavior: ScrollBehavior = + behavior === 'smooth' && prefersReducedMotion ? 'auto' : behavior + container.scrollTo({ top: Math.max(0, nextScrollTop), behavior: resolvedBehavior }) return true } diff --git a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts index 35686a4abee..8cd870aec13 100644 --- a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts @@ -153,7 +153,9 @@ describe('forkAgentSessionFromPane', () => { launchSource: 'terminal_context_menu' }) ) - expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork') + expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork', { + sidebarRevealBehavior: 'auto' + }) expect(mockToast.success).toHaveBeenCalledWith( 'Top-level session fork opened in a new workspace' ) @@ -344,7 +346,9 @@ describe('forkAgentSessionFromPane', () => { undefined ) expect(mockLaunchAgentInNewTab).not.toHaveBeenCalled() - expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork') + expect(mockActivateAndRevealWorktree).toHaveBeenCalledWith('wt-fork', { + sidebarRevealBehavior: 'auto' + }) expect(mockWriteClipboardText).toHaveBeenCalledWith( expect.stringContaining('Assistant: here is the current plan') ) diff --git a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts index 7f2d62b4782..6b36a638749 100644 --- a/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts +++ b/src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts @@ -266,7 +266,7 @@ export async function startAgentSessionFork(fork: PreparedAgentSessionFork): Pro const forkWorktreeId = created.worktree.id if (!fork.agent) { - activateAndRevealWorktree(forkWorktreeId) + activateAndRevealWorktree(forkWorktreeId, { sidebarRevealBehavior: 'auto' }) return copyAgentSessionForkContext(fork) } await preflightForkAgentTrust({ @@ -287,7 +287,7 @@ export async function startAgentSessionFork(fork: PreparedAgentSessionFork): Pro launchSource: 'terminal_context_menu', ...(launchPlatform ? { launchPlatform } : {}) }) - activateAndRevealWorktree(forkWorktreeId) + activateAndRevealWorktree(forkWorktreeId, { sidebarRevealBehavior: 'auto' }) if (!result) { return copyAgentSessionForkContext(fork) diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 4f4b6ea8b10..d1316b98678 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -3687,6 +3687,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS startupPlan.launchToken = createBrowserUuid() } const activation = activateAndRevealWorktree(worktree.id, { + sidebarRevealBehavior: 'auto', setup: result.setup, defaultTabs: result.defaultTabs, issueCommand, diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index f8d46e38b8c..bef55602893 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -4025,6 +4025,7 @@ describe('useIpcEvents CLI-created worktree activation', () => { expect(activateAndRevealWorktree).toHaveBeenCalledTimes(1) expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-new', { setup, + sidebarRevealBehavior: 'auto', notifyHostRuntime: false }) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 80e5c2e30c0..fab0f38e013 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -931,10 +931,12 @@ export function useIpcEvents(): void { // stream and is allowed through this helper separately. return } + const existedBeforeFetch = Boolean(useAppStore.getState().getKnownWorktreeById(worktreeId)) // Why: fetch worktrees first so the activation helper can resolve // the CLI-created worktree via findWorktreeById — it arrived from // the main process and is not yet in the renderer state. await useAppStore.getState().fetchWorktrees(repoId) + const existsAfterFetch = Boolean(useAppStore.getState().getKnownWorktreeById(worktreeId)) // Why: route through activateAndRevealWorktree so CLI-created // worktrees share the canonical activation path with UI-created // ones. This records the visit in the back/forward history stack @@ -944,6 +946,7 @@ export function useIpcEvents(): void { ...(setup ? { setup } : {}), ...(startup ? { startup } : {}), ...(defaultTabs ? { defaultTabs } : {}), + ...(!existedBeforeFetch && existsAfterFetch ? { sidebarRevealBehavior: 'auto' } : {}), // Why: this activation already came from the host runtime event stream. // Echoing it back as worktree.activate can create a selection loop. notifyHostRuntime: false diff --git a/src/renderer/src/lib/connection-context.test.ts b/src/renderer/src/lib/connection-context.test.ts index bef67faf05f..48d27ecf1e4 100644 --- a/src/renderer/src/lib/connection-context.test.ts +++ b/src/renderer/src/lib/connection-context.test.ts @@ -256,6 +256,46 @@ describe('getConnectionId', () => { expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform/README.md')).toBeUndefined() }) + it('resolves folder workspace combined diff sections by child repo path', () => { + const workspaceKey = folderWorkspaceKey('folder-workspace-1') + useAppStore.setState({ + folderWorkspaces: [makeFolderWorkspace()], + projectGroups: [ + { + id: 'group-1', + name: 'Platform', + parentPath: '/home/neil/platform', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ], + repos: [ + makeRepo({ + id: 'repo-local', + path: '/home/neil/platform/web', + projectGroupId: 'group-1' + }), + makeRepo({ + id: 'repo-ssh', + path: '/home/neil/platform/api', + projectGroupId: 'group-1', + connectionId: 'ssh-1' + }) + ], + worktreesByRepo: {} + }) + + expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform')).toBeUndefined() + expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform/api/src/index.ts')).toBe( + 'ssh-1' + ) + }) + it('keeps explicit folder workspace provenance isolated from unrelated same-path SSH repos', () => { useAppStore.setState({ folderWorkspaces: [ diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index 7d88276cc51..61f49274edb 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -216,6 +216,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom !isTuiAgentEnabled(agentOverride, latestStore.settings?.disabledTuiAgents) ) { activateAndRevealWorktree(worktreeId, { + sidebarRevealBehavior: 'auto', setup: result.setup }) toast.error(unavailableAgentErrorMessage()) @@ -281,6 +282,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom })) const activation = activateAndRevealWorktree(worktreeId, { + sidebarRevealBehavior: 'auto', setup: result.setup, defaultTabs: result.defaultTabs, ...buildDirectWorkItemStartupOpts(effectiveAgent, startupPlan, launchSource) diff --git a/src/renderer/src/lib/worktree-activation-created-agent.test.ts b/src/renderer/src/lib/worktree-activation-created-agent.test.ts index 184eb7dc7c8..174c1fab32d 100644 --- a/src/renderer/src/lib/worktree-activation-created-agent.test.ts +++ b/src/renderer/src/lib/worktree-activation-created-agent.test.ts @@ -299,10 +299,10 @@ describe('activateAndRevealWorktree created agent reopen', () => { revealWorktreeInSidebar }) - const result = activateAndRevealWorktree(worktree.id) + const result = activateAndRevealWorktree(worktree.id, { sidebarRevealBehavior: 'auto' }) expect(result).toEqual({ primaryTabId: expect.any(String) }) - expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id) + expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id, { behavior: 'auto' }) }) it('asks the host runtime to activate the worktree in the paired web client', async () => { diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index 4f735de5c17..1d94786a345 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -25,6 +25,7 @@ import { CLIENT_PLATFORM } from './new-workspace' import { tuiAgentToAgentKind } from './telemetry' import { agentKindToTuiAgent } from '../../../shared/agent-kind' import { useAppStore } from '@/store' +import type { PendingSidebarWorktreeReveal } from '@/store/slices/ui' import { tabHasLivePty } from '@/lib/tab-has-live-pty' import { activateWebRuntimeSessionWorktree, @@ -182,6 +183,7 @@ function ensureFolderWorkspaceInitialTerminal( export function activateAndRevealFolderWorkspace( folderWorkspaceId: string, opts?: { + sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior'] startup?: WorktreeStartupPayload runtimeEnvironmentId?: string | null } @@ -225,7 +227,11 @@ export function activateAndRevealFolderWorkspace( resumeSleepingAgentSessionsForWorktree(workspaceKey) const primaryTabId = ensureFolderWorkspaceInitialTerminal(folderWorkspace, opts?.startup) - state.revealWorktreeInSidebar(workspaceKey) + if (opts?.sidebarRevealBehavior) { + state.revealWorktreeInSidebar(workspaceKey, { behavior: opts.sidebarRevealBehavior }) + } else { + state.revealWorktreeInSidebar(workspaceKey) + } return { primaryTabId } } @@ -283,6 +289,7 @@ export function activateAndRevealWorktree( setup?: WorktreeSetupLaunch defaultTabs?: WorktreeDefaultTabsLaunch issueCommand?: IssueCommandLaunch + sidebarRevealBehavior?: PendingSidebarWorktreeReveal['behavior'] notifyHostRuntime?: boolean revealInSidebar?: boolean } @@ -382,7 +389,11 @@ export function activateAndRevealWorktree( // 6. Reveal in sidebar if (opts?.revealInSidebar !== false) { - state.revealWorktreeInSidebar(worktreeId) + if (opts?.sidebarRevealBehavior) { + state.revealWorktreeInSidebar(worktreeId, { behavior: opts.sidebarRevealBehavior }) + } else { + state.revealWorktreeInSidebar(worktreeId) + } } if (opts?.notifyHostRuntime !== false) { diff --git a/src/renderer/src/lib/worktree-creation-flow.ts b/src/renderer/src/lib/worktree-creation-flow.ts index 3e37be6d186..9af1dc61ebe 100644 --- a/src/renderer/src/lib/worktree-creation-flow.ts +++ b/src/renderer/src/lib/worktree-creation-flow.ts @@ -222,6 +222,7 @@ async function executeWorktreeCreation( let primaryTabId: string | null if (stillActive) { activation = activateAndRevealWorktree(worktree.id, { + sidebarRevealBehavior: 'auto', ...(result.setup ? { setup: result.setup } : {}), ...(result.defaultTabs ? { defaultTabs: result.defaultTabs } : {}), ...(startupOpt ? { startup: startupOpt } : {}), diff --git a/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts b/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts index 71e071f91f4..a18a2733dcc 100644 --- a/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts +++ b/src/renderer/src/store/slices/repos-onboarding-folder-startup.test.ts @@ -56,6 +56,7 @@ describe('repo slice skipped-onboarding folder startup', () => { 1, 'folder-1::/folder', { + sidebarRevealBehavior: 'auto', startup: { command: "codex '--dangerously-bypass-approvals-and-sandbox'", env: {}, @@ -76,7 +77,7 @@ describe('repo slice skipped-onboarding folder startup', () => { expect(worktreeActivation.activateAndRevealWorktree).toHaveBeenNthCalledWith( 2, 'folder-2::/folder', - undefined + { sidebarRevealBehavior: 'auto' } ) }) }) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 4f535acbce6..6fc31768e28 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -2586,7 +2586,10 @@ export const createRepoSlice: StateCreator = (set, onboarding, hadProjectBeforeAdd ) - activateAndRevealWorktree(folderWorktree.id, startup ? { startup } : undefined) + activateAndRevealWorktree(folderWorktree.id, { + sidebarRevealBehavior: 'auto', + ...(startup ? { startup } : {}) + }) } return repo } catch (err) { diff --git a/src/renderer/src/store/slices/ui.test.ts b/src/renderer/src/store/slices/ui.test.ts index 6c47388db03..eb1732f1a28 100644 --- a/src/renderer/src/store/slices/ui.test.ts +++ b/src/renderer/src/store/slices/ui.test.ts @@ -286,6 +286,7 @@ describe('createUISlice agent send target mode', () => { }) expect(store.getState().pendingRevealWorktree).toMatchObject({ worktreeId, + behavior: 'auto', highlight: true }) }) @@ -1097,12 +1098,14 @@ describe('createUISlice hydratePersistedUI', () => { const store = createUIStore() store.getState().revealWorktreeInSidebar('repo1::/feature', { + behavior: 'smooth', highlight: true, beginRename: true }) expect(store.getState().pendingRevealWorktree).toEqual({ worktreeId: 'repo1::/feature', + behavior: 'smooth', highlight: true, beginRename: true }) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 00f40ee2f3d..321f9ce09a3 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -113,12 +113,14 @@ import { translate } from '@/i18n/i18n' export type PendingSidebarWorktreeReveal = { worktreeId: string + behavior: 'auto' | 'smooth' highlight?: boolean beginRename?: boolean } export type PendingSidebarRowReveal = { rowKey: string + behavior: 'auto' | 'smooth' highlight?: boolean } @@ -896,6 +898,7 @@ export type UISlice = { revealWorktreeInSidebar: ( worktreeId: string, options?: { + behavior?: PendingSidebarWorktreeReveal['behavior'] highlight?: boolean beginRename?: boolean } @@ -903,6 +906,7 @@ export type UISlice = { revealSidebarRow: ( rowKey: string, options?: { + behavior?: PendingSidebarRowReveal['behavior'] highlight?: boolean } ) => void @@ -986,7 +990,7 @@ export const createUISlice: StateCreator = (set, get) targets.some((target) => target.status === 'eligible') && (previousMode?.id !== args.id || previousMode.worktreeId !== args.worktreeId) ) { - get().revealWorktreeInSidebar(args.worktreeId, { highlight: true }) + get().revealWorktreeInSidebar(args.worktreeId, { behavior: 'auto', highlight: true }) } }, closeAgentSendPopoverTargetMode: (id, instanceId) => @@ -2202,6 +2206,7 @@ export const createUISlice: StateCreator = (set, get) set({ pendingRevealWorktree: { worktreeId, + behavior: options?.behavior ?? 'smooth', ...(options?.highlight ? { highlight: true } : {}), ...(options?.beginRename ? { beginRename: true } : {}) } @@ -2210,6 +2215,7 @@ export const createUISlice: StateCreator = (set, get) set({ pendingRevealSidebarRow: { rowKey, + behavior: options?.behavior ?? 'smooth', ...(options?.highlight === false ? {} : { highlight: true }) } }), diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 8e0cb6e3a30..8929f57594c 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -6412,7 +6412,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([wt.id], true) expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) - expect(reveal).toHaveBeenCalledWith(wt.id, { highlight: true }) + expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) }) it('reveals on unpin so the viewport follows the row back to its status group', () => { @@ -6427,7 +6427,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([wt.id], false) expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(false) - expect(reveal).toHaveBeenCalledWith(wt.id, { highlight: true }) + expect(reveal).toHaveBeenCalledWith(wt.id, { behavior: 'smooth', highlight: true }) }) it('skips a no-op toggle without requesting a reveal', () => { @@ -6492,7 +6492,7 @@ describe('setWorktreesPinnedAndReveal', () => { store.getState().setWorktreesPinnedAndReveal([alreadyPinned.id, first.id, second.id], true) expect(reveal).toHaveBeenCalledTimes(1) - expect(reveal).toHaveBeenCalledWith(first.id, { highlight: true }) + expect(reveal).toHaveBeenCalledWith(first.id, { behavior: 'smooth', highlight: true }) // Every targeted row is pinned, not just the revealed one, and the // already-pinned row is left untouched. expect(store.getState().worktreesByRepo.repo1[0].isPinned).toBe(true) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 9c350fd7652..d6fb11a315b 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -3959,7 +3959,7 @@ export const createWorktreeSlice: StateCreator // persistence is async), so the reveal below resolves against a render // where the shortcut row already exists. void get().updateWorktreesMeta(updates) - get().revealWorktreeInSidebar(revealWorktreeId, { highlight: true }) + get().revealWorktreeInSidebar(revealWorktreeId, { behavior: 'smooth', highlight: true }) }, markWorktreeUnread: (worktreeId) => { diff --git a/src/shared/git-fetch-auto-maintenance.ts b/src/shared/git-fetch-auto-maintenance.ts new file mode 100644 index 00000000000..3723298ab37 --- /dev/null +++ b/src/shared/git-fetch-auto-maintenance.ts @@ -0,0 +1,10 @@ +// Why: Git 2.29 can auto-run commit-graph work before maintenance.auto became a gate. +// The other keys cover modern maintenance and legacy auto-gc without changing user config. +export const GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS = [ + '-c', + 'maintenance.auto=false', + '-c', + 'maintenance.commit-graph.auto=0', + '-c', + 'gc.auto=0' +] as const diff --git a/src/shared/quick-open-directory-validation.ts b/src/shared/quick-open-directory-validation.ts new file mode 100644 index 00000000000..501791bb050 --- /dev/null +++ b/src/shared/quick-open-directory-validation.ts @@ -0,0 +1,7 @@ +import type { Stats } from 'node:fs' + +export function isQuickOpenReadableDirectory(stat: Stats, allowSymlinkedRoot = false): boolean { + // Why: an explicitly selected workspace root may be a symlink, while nested + // traversal must never follow a symlink outside that authorized root. + return stat.isDirectory() || Boolean(allowSymlinkedRoot && stat.isSymbolicLink()) +} diff --git a/src/shared/quick-open-expansion-paths.test.ts b/src/shared/quick-open-expansion-paths.test.ts new file mode 100644 index 00000000000..e5e91e9b40c --- /dev/null +++ b/src/shared/quick-open-expansion-paths.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' +import { collapseQuickOpenExpansionPaths } from './quick-open-expansion-paths' + +describe('collapseQuickOpenExpansionPaths', () => { + it('collapses descendants without confusing shared-prefix siblings', () => { + const paths = new Map([ + ['foo/bar/deep', false], + ['foo-sibling', false], + ['foo', false], + ['foo/bar', true] + ]) + + expect(collapseQuickOpenExpansionPaths(paths)).toEqual([ + ['foo', true], + ['foo-sibling', false] + ]) + }) + + it('keeps many sibling placeholders distinct', () => { + const paths = new Map( + Array.from({ length: 10_001 }, (_, index) => [`generated-${index}`, true] as const) + ) + + expect(collapseQuickOpenExpansionPaths(paths)).toHaveLength(paths.size) + }) +}) diff --git a/src/shared/quick-open-expansion-paths.ts b/src/shared/quick-open-expansion-paths.ts new file mode 100644 index 00000000000..43f094edd3f --- /dev/null +++ b/src/shared/quick-open-expansion-paths.ts @@ -0,0 +1,37 @@ +/** + * Remove descendant placeholders already covered by an ancestor. Sorting puts + * ancestors first; prefix lookups avoid quadratic scans across sibling paths. + */ +export function collapseQuickOpenExpansionPaths( + expansionPaths: ReadonlyMap +): [string, boolean][] { + const sortedPaths = Array.from(expansionPaths).sort(([left], [right]) => + left < right ? -1 : left > right ? 1 : 0 + ) + const collapsedPaths = new Map() + + for (const [relPath, includeSymlinks] of sortedPaths) { + let ancestorPath: string | undefined + let slashIndex = relPath.indexOf('/') + while (slashIndex !== -1) { + const candidate = relPath.substring(0, slashIndex) + if (collapsedPaths.has(candidate)) { + ancestorPath = candidate + break + } + slashIndex = relPath.indexOf('/', slashIndex + 1) + } + + if (ancestorPath) { + // Why: primary and ignored passes can overlap; the ancestor walk covers + // the descendant, but must preserve either pass's symlink-leaf contract. + if (includeSymlinks) { + collapsedPaths.set(ancestorPath, true) + } + continue + } + collapsedPaths.set(relPath, includeSymlinks) + } + + return Array.from(collapsedPaths) +} diff --git a/src/shared/quick-open-filter.test.ts b/src/shared/quick-open-filter.test.ts index 2b079f6b7d4..1d96bc1744f 100644 --- a/src/shared/quick-open-filter.test.ts +++ b/src/shared/quick-open-filter.test.ts @@ -85,6 +85,9 @@ describe('buildExcludePathPrefixes', () => { expect(buildExcludePathPrefixes('C:\\repo', ['C:\\repo\\packages\\app'])).toEqual([ 'packages/app' ]) + expect( + buildExcludePathPrefixes('//Server/Share/Repo', ['//server/share/repo/packages/app']) + ).toEqual(['packages/app']) }) it('strips trailing slashes', () => { @@ -239,16 +242,40 @@ describe('normalizeQuickOpenRgLine', () => { describe('buildGitLsFilesArgsForQuickOpen', () => { it('primary pass is --cached --others --exclude-standard', () => { const { primary } = buildGitLsFilesArgsForQuickOpen() - expect(primary).toEqual(['-z', '-s', '--cached', '--others', '--exclude-standard']) + expect(primary).toEqual([ + '-z', + '-s', + '--cached', + '--others', + '--exclude-standard', + '--directory', + '--no-empty-directory' + ]) }) it('ignored pass surfaces ignored files without .env* pathspec whitelist', () => { const { ignoredPass } = buildGitLsFilesArgsForQuickOpen() - expect(ignoredPass).toEqual(['-z', '-s', '--others', '--ignored', '--exclude-standard']) + expect(ignoredPass).toEqual([ + '-z', + '-s', + '--others', + '--ignored', + '--exclude-standard', + '--directory', + '--no-empty-directory' + ]) expect(ignoredPass).not.toContain('.env*') expect(ignoredPass).not.toContain(':(glob)**/.env*') }) + it('collapses untracked directories in both passes without generated pathspec churn', () => { + const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen() + expect(primary).toContain('--directory') + expect(ignoredPass).toContain('--directory') + expect(ignoredPass).toContain('--no-empty-directory') + expect([...primary, ...ignoredPass]).not.toContain(':(exclude,glob)**/node_modules/**') + }) + it('exclude prefixes prepend positive "." pathspec', () => { const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(['packages/app']) const dashDashIdx = primary.indexOf('--') diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index 980c43351a2..899d13aeaa3 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -104,8 +104,8 @@ function pathFlavor(rootPath: string): typeof posix | typeof win32 { if (/^[a-zA-Z]:[\\/]/.test(rootPath)) { return win32 } - // UNC \\server\share - if (rootPath.startsWith('\\\\')) { + // UNC \\server\share or //server/share + if (rootPath.startsWith('\\\\') || rootPath.startsWith('//')) { return win32 } return posix @@ -376,16 +376,28 @@ export function buildGitLsFilesArgsForQuickOpen( excludeSpecs.push(`:(exclude,glob)${escapeGlobPath(prefix)}/**`) } const trailingPathspecs = excludeSpecs.length > 0 ? ['--', '.', ...excludeSpecs] : [] + // Why: collapse untracked trees before Git traverses them; callers expand + // only allowed directory placeholders with the shared bounded walker. + const directoryCollapseArgs = ['--directory', '--no-empty-directory'] // Why: NUL preserves real Git paths; stage mode identifies gitlinks without // lstat probes for ordinary tracked files. - const primary = ['-z', '-s', '--cached', '--others', '--exclude-standard', ...trailingPathspecs] + const primary = [ + '-z', + '-s', + '--cached', + '--others', + '--exclude-standard', + ...directoryCollapseArgs, + ...trailingPathspecs + ] const ignoredPass = [ '-z', '-s', '--others', '--ignored', '--exclude-standard', + ...directoryCollapseArgs, ...trailingPathspecs ] return { primary, ignoredPass } diff --git a/src/shared/quick-open-git-directory-collapse.test.ts b/src/shared/quick-open-git-directory-collapse.test.ts new file mode 100644 index 00000000000..8e197a6d607 --- /dev/null +++ b/src/shared/quick-open-git-directory-collapse.test.ts @@ -0,0 +1,58 @@ +import { execFile } 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 { promisify } from 'node:util' +import { afterEach, describe, expect, it } from 'vitest' +import { buildGitLsFilesArgsForQuickOpen } from './quick-open-filter' + +const execFileAsync = promisify(execFile) +const tempDirs: string[] = [] + +async function writeRel(root: string, relPath: string): Promise { + const absPath = join(root, ...relPath.split('/')) + await mkdir(dirname(absPath), { recursive: true }) + await writeFile(absPath, 'x') +} + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))) +}) + +describe('Quick Open git directory collapse', () => { + it('collapses unignored and ignored trees while preserving individual ignored files', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-quick-open-git-collapse-')) + tempDirs.push(root) + await execFileAsync('git', ['init', '--quiet'], { cwd: root }) + await writeFile( + join(root, '.gitignore'), + ['.cache/', '.local/share/', 'dist/', '*.log'].join('\n') + ) + await Promise.all([ + ...Array.from({ length: 200 }, (_, index) => + writeRel(root, `node_modules/pkg/file-${index}.js`) + ), + writeRel(root, '.cache/state.json'), + writeRel(root, '.local/share/state.json'), + writeRel(root, 'dist/generated.js'), + writeRel(root, 'debug.log') + ]) + + const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen() + const [primaryResult, ignoredResult] = await Promise.all( + [primary, ignoredPass].map((args) => + execFileAsync('git', ['-c', 'core.excludesFile=', 'ls-files', ...args], { + cwd: root, + encoding: 'buffer' + }) + ) + ) + const primaryPaths = primaryResult.stdout.toString().split('\0').filter(Boolean) + const ignoredPaths = ignoredResult.stdout.toString().split('\0').filter(Boolean) + + expect(primaryPaths).toEqual(['.gitignore', 'node_modules/']) + expect(primaryPaths.some((path) => path.startsWith('node_modules/pkg/'))).toBe(false) + expect(ignoredPaths).toEqual(['.cache/', '.local/', 'debug.log', 'dist/']) + expect(ignoredPaths.some((path) => path.startsWith('.local/share/'))).toBe(false) + }) +}) diff --git a/src/shared/quick-open-readdir-walk.test.ts b/src/shared/quick-open-readdir-walk.test.ts index cb73572bc34..2331bc3b1c3 100644 --- a/src/shared/quick-open-readdir-walk.test.ts +++ b/src/shared/quick-open-readdir-walk.test.ts @@ -1,28 +1,32 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -const { lstatMock } = vi.hoisted(() => ({ - lstatMock: vi.fn() +const { lstatMock, readdirMock } = vi.hoisted(() => ({ + lstatMock: vi.fn(), + readdirMock: vi.fn() })) vi.mock('fs/promises', async () => { const actual = await vi.importActual('fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() lstatMock.mockImplementation(actual.lstat) + readdirMock.mockImplementation(actual.readdir) return { ...actual, - lstat: lstatMock + lstat: lstatMock, + readdir: readdirMock } }) -import { mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises' +import { mkdtemp, mkdir, rename, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { classifyQuickOpenGitEntry, createQuickOpenReaddirBudget, - expandQuickOpenGitFilesWithNestedRepos, + expandQuickOpenGitFileListing, isQuickOpenReaddirBudgetError, listQuickOpenFilesWithReaddir, - parseQuickOpenGitLsFilesEntry + parseQuickOpenGitLsFilesEntry, + QUICK_OPEN_READDIR_MAX_FILES } from './quick-open-readdir-walk' import { isFileListingCancellation } from './file-listing-cancellation' @@ -94,7 +98,7 @@ describe('quick-open readdir walk', () => { it('keeps ordinary git entries without lstat calls', async () => { await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: '/unused/root', gitPaths: [ staged('100644', 'README.md'), @@ -152,7 +156,7 @@ describe('quick-open readdir walk', () => { await writeRel(root, 'packages/lib/src/lib.ts') await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: root, gitPaths: [ staged('100644', 'README.md'), @@ -180,7 +184,7 @@ describe('quick-open readdir walk', () => { await writeRel(root, 'packages/lib/c.ts') await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: root, gitPaths: [staged('160000', 'packages/app'), staged('160000', 'packages/lib')], budget: createQuickOpenReaddirBudget({ maxFiles: 2 }) @@ -199,7 +203,7 @@ describe('quick-open readdir walk', () => { } await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: root, gitPaths: [staged('160000', 'packages/app')], excludePathPrefixes: ['packages/app/excluded'], @@ -208,6 +212,209 @@ describe('quick-open readdir walk', () => { ).resolves.toEqual(['packages/app/keep.ts']) }) + it('expands allowed ignored directories without walking blocked or excluded directories', async () => { + const root = await makeTempRoot() + await writeRel(root, 'dist/generated.js') + await writeRel(root, 'node_modules/pkg/index.js') + await writeRel(root, '.cache/state.json') + await writeRel(root, '.local/share/state.json') + await writeRel(root, '.local/config.toml') + await writeRel(root, 'excluded/other.js') + + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: [ + 'dist/', + '.local/', + 'node_modules/', + '.cache/', + '.local/share/', + 'excluded/' + ], + excludePathPrefixes: ['excluded'], + budget: createQuickOpenReaddirBudget({ maxFiles: 2 }) + }) + ).resolves.toEqual(['.local/config.toml', 'dist/generated.js']) + + const walkedPaths = readdirMock.mock.calls.map(([path]) => path) + expect(walkedPaths).toContain(join(root, 'dist')) + expect(walkedPaths).toContain(join(root, '.local')) + expect(walkedPaths).not.toContain(join(root, '.local', 'share')) + }) + + it('batches many allowed directory placeholders with bounded concurrency', async () => { + const root = await makeTempRoot() + const directoryPaths = Array.from({ length: 40 }, (_, index) => `generated-${index}/`) + await Promise.all( + directoryPaths.map((directoryPath, index) => + writeRel(root, `${directoryPath}file-${index}.ts`) + ) + ) + + const actual = await vi.importActual('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + let activeReads = 0 + let maxActiveReads = 0 + readdirMock.mockImplementation(async (...args: Parameters) => { + activeReads++ + maxActiveReads = Math.max(maxActiveReads, activeReads) + await new Promise((resolve) => setTimeout(resolve, 5)) + try { + return await actual.readdir(...args) + } finally { + activeReads-- + } + }) + + try { + const files = await expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths + }) + expect(files).toHaveLength(directoryPaths.length) + expect(maxActiveReads).toBeGreaterThan(1) + expect(maxActiveReads).toBeLessThanOrEqual(32) + } finally { + readdirMock.mockImplementation(actual.readdir) + } + }) + + it('preserves symlink leaves from collapsed Git directories without following them', async () => { + const root = await makeTempRoot() + await mkdirRel(root, 'scratch') + await writeRel(root, 'target/file.ts') + + try { + await symlink(join(root, 'target', 'file.ts'), join(root, 'scratch', 'link.ts')) + await symlink(join(root, 'target'), join(root, 'scratch', 'linked-dir'), 'dir') + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EPERM') { + return + } + throw err + } + + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['scratch/'] + }) + ).resolves.toEqual(['scratch/link.ts', 'scratch/linked-dir']) + }) + + it('does not follow a collapsed directory replaced by a symlink', async () => { + const root = await makeTempRoot() + const outsideRoot = await makeTempRoot() + await writeRel(outsideRoot, 'secret.ts') + + try { + await symlink(outsideRoot, join(root, 'dist'), 'dir') + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EPERM') { + return + } + throw err + } + + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['dist/'] + }) + ).resolves.toEqual([]) + }) + + it('discards entries when a collapsed directory changes during readdir', async () => { + const root = await makeTempRoot() + const outsideRoot = await makeTempRoot() + await mkdirRel(root, 'dist') + await writeRel(outsideRoot, 'secret.ts') + const actual = await vi.importActual('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + const distPath = join(root, 'dist') + let swapped = false + readdirMock.mockImplementation(async (...args: Parameters) => { + if (!swapped && args[0] === distPath) { + swapped = true + await rename(distPath, join(root, 'old-dist')) + await symlink(outsideRoot, distPath, 'dir') + } + return actual.readdir(...args) + }) + + try { + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['dist/'] + }) + ).resolves.toEqual([]) + } finally { + readdirMock.mockImplementation(actual.readdir) + } + }) + + it('walks overlapping primary and ignored placeholders only once', async () => { + const root = await makeTempRoot() + await writeRel(root, 'foo/a.ts') + await writeRel(root, 'foo/bar/b.ts') + + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['foo/', 'foo/bar/'], + budget: createQuickOpenReaddirBudget({ maxFiles: 2 }) + }) + ).resolves.toEqual(['foo/a.ts', 'foo/bar/b.ts']) + + expect( + readdirMock.mock.calls.filter(([path]) => path === join(root, 'foo', 'bar')) + ).toHaveLength(1) + }) + + it('rejects instead of returning a partial ignored-directory expansion', async () => { + const root = await makeTempRoot() + await writeRel(root, 'dist/a.js') + await writeRel(root, 'dist/b.js') + + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['dist/'], + budget: createQuickOpenReaddirBudget({ maxFiles: 1 }) + }) + ).rejects.toThrow('File listing exceeded') + }) + + it('keeps the default safety cap for a very large collapsed directory', async () => { + const root = await makeTempRoot() + await mkdirRel(root, 'dist') + readdirMock.mockResolvedValueOnce( + Array.from({ length: QUICK_OPEN_READDIR_MAX_FILES + 1 }, (_, index) => ({ + name: `file-${index}.ts`, + isDirectory: () => false, + isFile: () => true, + isSymbolicLink: () => false + })) + ) + + // Why: directory collapse prevents generated trees from flooding the relay; + // the Git fallback must reject rather than silently return a partial list. + await expect( + expandQuickOpenGitFileListing({ + rootPath: root, + gitPaths: [], + directoryPaths: ['dist/'] + }) + ).rejects.toThrow('File listing exceeded 10000 files') + }) + it('identifies budget errors so callers can translate only those to install-rg guidance', () => { expect(isQuickOpenReaddirBudgetError(new Error('File listing timed out'))).toBe(true) expect(isQuickOpenReaddirBudgetError(new Error('File listing exceeded 10000 files'))).toBe(true) @@ -248,13 +455,31 @@ describe('quick-open readdir walk', () => { expect(files).not.toContain('linked-dir/file.ts') }) + it('walks an explicitly selected symlinked workspace root', async () => { + const targetRoot = await makeTempRoot() + const linkContainer = await makeTempRoot() + await writeRel(targetRoot, 'src/index.ts') + const linkedRoot = join(linkContainer, 'linked-workspace') + + try { + await symlink(targetRoot, linkedRoot, 'dir') + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EPERM') { + return + } + throw err + } + + await expect(listQuickOpenFilesWithReaddir(linkedRoot)).resolves.toEqual(['src/index.ts']) + }) + it('fills nested repo paths containing spaces and glob metacharacters', async () => { const root = await makeTempRoot() await makeNestedRepo(root, 'packages/app [one] space') await writeRel(root, 'packages/app [one] space/src/main.ts') await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: root, gitPaths: ['packages/app [one] space/'] }) @@ -276,7 +501,7 @@ describe('quick-open readdir walk', () => { await rejection.catch((err) => expect(isQuickOpenReaddirBudgetError(err)).toBe(false)) }) - it('stops nested-repo expansion when the signal aborts (#7721)', async () => { + it('stops ignored-directory expansion when the signal aborts (#7721)', async () => { const root = await makeTempRoot() await writeRel(root, 'src/kept.ts') @@ -284,11 +509,27 @@ describe('quick-open readdir walk', () => { controller.abort() await expect( - expandQuickOpenGitFilesWithNestedRepos({ + expandQuickOpenGitFileListing({ rootPath: root, - gitPaths: ['src/kept.ts'], + gitPaths: [], + directoryPaths: ['src/'], signal: controller.signal }) ).rejects.toSatisfy(isFileListingCancellation) }) + + it('rejects when cancellation lands during an empty readdir batch', async () => { + const root = await makeTempRoot() + const controller = new AbortController() + const actual = await vi.importActual('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + readdirMock.mockImplementationOnce(async (...args: Parameters) => { + const entries = await actual.readdir(...args) + controller.abort() + return entries + }) + + await expect( + listQuickOpenFilesWithReaddir(root, { signal: controller.signal }) + ).rejects.toSatisfy(isFileListingCancellation) + }) }) diff --git a/src/shared/quick-open-readdir-walk.ts b/src/shared/quick-open-readdir-walk.ts index 90ce3ea9fdf..ada9442cd23 100644 --- a/src/shared/quick-open-readdir-walk.ts +++ b/src/shared/quick-open-readdir-walk.ts @@ -1,6 +1,8 @@ import { lstat, readdir } from 'node:fs/promises' import { join, relative } from 'node:path' import { throwIfFileListingCancelled } from './file-listing-cancellation' +import { isQuickOpenReadableDirectory } from './quick-open-directory-validation' +import { collapseQuickOpenExpansionPaths } from './quick-open-expansion-paths' import { HIDDEN_DIR_BLOCKLIST, shouldExcludeQuickOpenRelPath, @@ -9,6 +11,7 @@ import { export const QUICK_OPEN_READDIR_MAX_FILES = 10_000 export const QUICK_OPEN_READDIR_TIMEOUT_MS = 10_000 +const QUICK_OPEN_READDIR_CONCURRENCY = 32 export type QuickOpenReaddirBudget = { remainingFiles: number @@ -96,13 +99,13 @@ function normalizeGitEntry(entry: string): string { } // Translate workspace-root-relative exclude prefixes into prefixes relative to -// a nested repo at `nestedRelPath`, so the nested walk can prune them during -// traversal. Prefixes outside the nested repo are dropped (they cannot match). -function rebaseExcludePrefixesForNestedRepo( +// one expanded subtree, so its walk prunes them during traversal. Prefixes +// outside that subtree are dropped because they cannot match. +function rebaseExcludePrefixesForSubtree( excludePathPrefixes: readonly string[], - nestedRelPath: string + subtreeRelPath: string ): string[] { - const base = `${nestedRelPath}/` + const base = `${subtreeRelPath}/` const rebased: string[] = [] for (const prefix of excludePathPrefixes) { if (prefix.startsWith(base)) { @@ -157,59 +160,132 @@ export async function listQuickOpenFilesWithReaddir( rootPath: string, opts: { excludePathPrefixes?: readonly string[] + workspaceRelPathPrefix?: string budget?: QuickOpenReaddirBudget signal?: AbortSignal } = {} ): Promise { - const files: string[] = [] - const budget = opts.budget ?? createQuickOpenReaddirBudget() - const excludePathPrefixes = opts.excludePathPrefixes ?? [] - - async function walk(dir: string): Promise { - // Why: an abandoned scan (workspace switch) must stop consuming IO and - // event-loop time on the single-threaded relay, not just run to budget. - throwIfFileListingCancelled(opts.signal) - assertWithinDeadline(budget) + return listQuickOpenFilesFromRoots( + [ + { + rootPath, + excludePathPrefixes: opts.excludePathPrefixes ?? [], + workspaceRelPathPrefix: opts.workspaceRelPathPrefix, + allowRootSymlink: true + } + ], + opts.budget ?? createQuickOpenReaddirBudget(), + opts.signal + ) +} - let entries - try { - entries = await readdir(dir, { withFileTypes: true }) - } catch { - // Why: permission denied on an individual subtree is common for broad - // roots. Skipping that subtree preserves the existing relay fallback. - return - } +type QuickOpenReaddirRoot = { + rootPath: string + excludePathPrefixes: readonly string[] + workspaceRelPathPrefix?: string + outputPathPrefix?: string + includeSymlinks?: boolean + allowRootSymlink?: boolean +} - for (const entry of entries) { - throwIfFileListingCancelled(opts.signal) +async function listQuickOpenFilesFromRoots( + roots: readonly QuickOpenReaddirRoot[], + budget: QuickOpenReaddirBudget, + signal?: AbortSignal +): Promise { + const files: string[] = [] + let pendingDirectories = roots.map((root) => ({ + root, + absPath: root.rootPath, + isRoot: true + })) + + while (pendingDirectories.length > 0) { + const nextDirectories: typeof pendingDirectories = [] + for ( + let offset = 0; + offset < pendingDirectories.length; + offset += QUICK_OPEN_READDIR_CONCURRENCY + ) { + // Why: batch only the filesystem calls. Result processing stays serial, + // so the shared cap remains exact while shallow placeholder-heavy repos + // do not pay one relay event-loop turn per directory. + throwIfFileListingCancelled(signal) + assertWithinDeadline(budget) + const batch = pendingDirectories.slice(offset, offset + QUICK_OPEN_READDIR_CONCURRENCY) + const entryGroups = await Promise.all( + batch.map(async (pending) => { + try { + // Why: Git's placeholder may have been replaced with a symlink + // before expansion. Never let readdir follow it outside the root. + const stat = await lstat(pending.absPath) + const allowSymlinkedRoot = pending.isRoot && pending.root.allowRootSymlink + if (!isQuickOpenReadableDirectory(stat, allowSymlinkedRoot)) { + return { pending, entries: [] } + } + const entries = await readdir(pending.absPath, { withFileTypes: true }) + // Why: close the ordinary check/use race. If the directory became + // a symlink while readdir was pending, discard everything read. + const statAfterRead = await lstat(pending.absPath) + if (!isQuickOpenReadableDirectory(statAfterRead, allowSymlinkedRoot)) { + return { pending, entries: [] } + } + return { pending, entries } + } catch { + // Why: permission denied on one subtree is common for broad roots. + return { pending, entries: [] } + } + }) + ) + // Why: an empty directory has no per-entry checkpoint below. Cancellation + // or timeout that lands during readdir must still reject, never resolve []. + throwIfFileListingCancelled(signal) assertWithinDeadline(budget) - const name = entry.name - const absPath = join(dir, name) - const relPath = toRelPath(rootPath, absPath) - if (shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes)) { - continue - } - if (entry.isDirectory()) { - if (shouldDescend(name)) { - await walk(absPath) + for (const { pending, entries } of entryGroups) { + for (const entry of entries) { + throwIfFileListingCancelled(signal) + assertWithinDeadline(budget) + + const name = entry.name + const absPath = join(pending.absPath, name) + const relPath = toRelPath(pending.root.rootPath, absPath) + const workspaceRelPath = pending.root.workspaceRelPathPrefix + ? `${pending.root.workspaceRelPathPrefix}/${relPath}` + : relPath + if (shouldExcludeQuickOpenRelPath(relPath, pending.root.excludePathPrefixes)) { + continue + } + if (entry.isDirectory()) { + if (shouldDescend(name) && shouldIncludeQuickOpenPath(workspaceRelPath)) { + nextDirectories.push({ root: pending.root, absPath, isRoot: false }) + } + continue + } + if ( + (entry.isFile() || (pending.root.includeSymlinks && entry.isSymbolicLink())) && + shouldIncludeQuickOpenPath(workspaceRelPath) + ) { + consumeFileBudget(budget) + files.push( + pending.root.outputPathPrefix + ? `${pending.root.outputPathPrefix}/${relPath}` + : relPath + ) + } } - continue - } - if (entry.isFile()) { - consumeFileBudget(budget) - files.push(relPath) } } + pendingDirectories = nextDirectories } - await walk(rootPath) return files } -export async function expandQuickOpenGitFilesWithNestedRepos(opts: { +export async function expandQuickOpenGitFileListing(opts: { rootPath: string gitPaths: Iterable + directoryPaths?: Iterable excludePathPrefixes?: readonly string[] budget?: QuickOpenReaddirBudget signal?: AbortSignal @@ -217,6 +293,7 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: { const files = new Set() const excludePathPrefixes = opts.excludePathPrefixes ?? [] const budget = opts.budget ?? createQuickOpenReaddirBudget() + const expansionPaths = new Map() const addFinalPath = (relPath: string): void => { if (!relPath) { @@ -243,17 +320,46 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: { continue } - const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), { - // Why: exclude prefixes are workspace-root-relative; rebase them onto the - // nested repo so the walk prunes excluded subtrees during traversal - // instead of burning the shared budget and filtering them at the end. - excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath), - budget, - signal: opts.signal - }) - for (const nestedFile of nestedFiles) { - addFinalPath(`${relPath}/${nestedFile}`) + expansionPaths.set(relPath, expansionPaths.get(relPath) ?? false) + } + + for (const rawPath of opts.directoryPaths ?? []) { + throwIfFileListingCancelled(opts.signal) + assertWithinDeadline(budget) + + const relPath = normalizeGitEntry(rawPath) + // Why: Git intentionally leaves collapsed directories unexpanded; reject + // blocked and nested-worktree placeholders before any filesystem IO. + if ( + !relPath || + shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes) || + !shouldIncludeQuickOpenPath(relPath) + ) { + continue } + + // Why: before directory collapse, Git returned untracked symlink entries + // without following them. Preserve those paths when expanding placeholders. + expansionPaths.set(relPath, true) + } + + const expandedFiles = await listQuickOpenFilesFromRoots( + collapseQuickOpenExpansionPaths(expansionPaths).map(([relPath, includeSymlinks]) => ({ + rootPath: joinRootRel(opts.rootPath, relPath), + // Why: exclude prefixes are workspace-root-relative; rebase them onto + // each expanded subtree so blocked work is pruned before consuming cap. + excludePathPrefixes: rebaseExcludePrefixesForSubtree(excludePathPrefixes, relPath), + // Why: Git can collapse `.local/share/` to `.local/`; keep workspace + // context so the walker still prunes the multi-segment blocklist. + workspaceRelPathPrefix: relPath, + outputPathPrefix: relPath, + includeSymlinks + })), + budget, + opts.signal + ) + for (const expandedFile of expandedFiles) { + addFinalPath(expandedFile) } return Array.from(files) diff --git a/tests/e2e/worktree-scroll-to-current.spec.ts b/tests/e2e/worktree-scroll-to-current.spec.ts index a3bef0d095b..19c51005cfe 100644 --- a/tests/e2e/worktree-scroll-to-current.spec.ts +++ b/tests/e2e/worktree-scroll-to-current.spec.ts @@ -23,9 +23,11 @@ async function prepareSidebarForScrollTest(page: Page): Promise { test.describe('Reveal active workspace button', () => { test.beforeEach(async ({ orcaPage }) => { - // Sidebar reveal now always jumps instantly (worktree-sidebar-reveal.ts - // assigns scrollTop directly), so no reduced-motion emulation is needed - // for the geometry assertions to be deterministic. + // Why: headless Electron under xvfb never ticks a smooth-scroll animation, + // so the reveal's `scrollTo({ behavior: 'smooth' })` would never reach its + // target. Reduced-motion makes the reveal jump instantly (see + // worktree-sidebar-reveal.ts) so the geometry assertions are deterministic. + await orcaPage.emulateMedia({ reducedMotion: 'reduce' }) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) }) diff --git a/tests/e2e/worktree-switch-responsiveness.spec.ts b/tests/e2e/worktree-switch-responsiveness.spec.ts index 11b4dea8c97..45a6e6e6b2f 100644 --- a/tests/e2e/worktree-switch-responsiveness.spec.ts +++ b/tests/e2e/worktree-switch-responsiveness.spec.ts @@ -33,8 +33,8 @@ async function prepareSidebarForSwitchTest(page: Page): Promise<[string, string] if ((state.tabsByWorktree[second.id] ?? []).length === 0) { state.createTab(second.id, undefined, undefined, { pendingActivationSpawn: true }) } - state.revealWorktreeInSidebar(first.id) - state.revealWorktreeInSidebar(second.id) + state.revealWorktreeInSidebar(first.id, { behavior: 'auto' }) + state.revealWorktreeInSidebar(second.id, { behavior: 'auto' }) state.setActiveWorktree(first.id) return [first.id, second.id] })