From 82023dbe1b1a76af4b6b4a03ee8d40bcfa194d0a Mon Sep 17 00:00:00 2001 From: Federico Pedraza <77793780+federicoPedraza@users.noreply.github.com> Date: Mon, 4 May 2026 18:19:20 -0300 Subject: [PATCH 01/46] feat(worktrees): add experimental symlinks for shared paths on worktree creation (#1029) Co-authored-by: Orca Co-authored-by: federico-ntb Co-authored-by: Jinwoo-H --- .../runtime-home-service.test.ts | 1 + src/main/codex-accounts/service.test.ts | 1 + src/main/ipc/repos.ts | 13 + src/main/ipc/worktree-remote.ts | 16 ++ src/main/ipc/worktree-symlinks.test.ts | 227 ++++++++++++++++++ src/main/ipc/worktree-symlinks.ts | 108 +++++++++ src/main/ipc/worktrees.ts | 15 ++ .../components/settings/ExperimentalPane.tsx | 41 ++++ .../components/settings/RepositoryPane.tsx | 23 ++ .../settings/WorktreeSymlinksSection.tsx | 201 ++++++++++++++++ .../settings/experimental-search.ts | 17 ++ src/renderer/src/store/slices/repos.ts | 1 + src/shared/constants.ts | 3 +- src/shared/types.ts | 12 + 14 files changed, 678 insertions(+), 1 deletion(-) create mode 100644 src/main/ipc/worktree-symlinks.test.ts create mode 100644 src/main/ipc/worktree-symlinks.ts create mode 100644 src/renderer/src/components/settings/WorktreeSymlinksSection.tsx diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index 65d304f71b6..8399055b7a3 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -96,6 +96,7 @@ function createSettings(overrides: Partial = {}): GlobalSettings experimentalAgentDashboard: false, experimentalMobile: false, experimentalSidekick: false, + experimentalWorktreeSymlinks: false, terminalWindowsShell: 'powershell.exe', terminalWindowsPowerShellImplementation: 'powershell.exe', enableGitHubAttribution: true, diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index cff5f956d75..51de5333e88 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -90,6 +90,7 @@ function createSettings(overrides: Partial = {}): GlobalSettings experimentalAgentDashboard: false, experimentalMobile: false, experimentalSidekick: false, + experimentalWorktreeSymlinks: false, terminalWindowsShell: 'powershell.exe', terminalWindowsPowerShellImplementation: 'powershell.exe', enableGitHubAttribution: true, diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 252ee4748ac..bf18216a863 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -406,6 +406,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v | 'hookSettings' | 'worktreeBaseRef' | 'kind' + | 'symlinkPaths' | 'issueSourcePreference' > > @@ -427,6 +428,18 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v ) { delete updates.issueSourcePreference } + // Why: `symlinkPaths` is consumed by `createWorktreeSymlinks` which + // calls `.trim()` on each entry. A renderer bug or preload-version skew + // that persists a non-`string[]` value (e.g. `[42, null]`, a bare + // string) would throw inside the worktree-create path with no UI + // signal. Strip invalid shapes at the boundary the same way + // `issueSourcePreference` is validated above. + if ('symlinkPaths' in updates && updates.symlinkPaths !== undefined) { + const v = updates.symlinkPaths as unknown + if (!Array.isArray(v) || !v.every((e) => typeof e === 'string')) { + delete updates.symlinkPaths + } + } const updated = store.updateRepo(args.repoId, updates) if (updated) { notifyReposChanged(mainWindow) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 49385ef2f37..cb37a482ae4 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -37,6 +37,7 @@ import { areWorktreePathsEqual } from './worktree-logic' import { invalidateAuthorizedRootsCache } from './filesystem-auth' +import { createWorktreeSymlinks } from './worktree-symlinks' import { normalizeSparseDirectories } from './sparse-checkout-directories' export function notifyWorktreesChanged(mainWindow: BrowserWindow, repoId: string): void { @@ -212,6 +213,12 @@ export async function createRemoteWorktree( const meta = store.setWorktreeMeta(worktreeId, metaUpdates) const worktree = mergeWorktree(repo.id, created, meta) + // Why: `experimentalWorktreeSymlinks` is intentionally not wired up for + // remote (SSH) worktrees. Creating symlinks on the remote host would + // require a new relay method and authorization surface; the feature is + // local-only until that protocol work is in scope. Remote repos with + // `symlinkPaths` configured have them silently ignored here. + notifyWorktreesChanged(mainWindow, repo.id) return { worktree } } @@ -452,6 +459,15 @@ export async function createLocalWorktree( // adds 100ms+ to every create. invalidateAuthorizedRootsCache() + // Why: create user-configured symlinks from the primary checkout into the + // new worktree before any setup script runs, so scripts that reuse shared + // state (e.g. `node_modules`, `.env`) see the links already in place. + // Gated on the experimental flag so disabling the feature globally skips + // the work even when a repo still has paths configured. + if (settings.experimentalWorktreeSymlinks && repo.symlinkPaths && repo.symlinkPaths.length > 0) { + await createWorktreeSymlinks(repo.path, created.path, repo.symlinkPaths) + } + // Why: the worktree's own `orca.yaml` (at the tip of the base branch) is // authoritative for what runs post-creation. The repo-level trust already // granted by the user in the pre-create flow covers execution of that diff --git a/src/main/ipc/worktree-symlinks.test.ts b/src/main/ipc/worktree-symlinks.test.ts new file mode 100644 index 00000000000..155655c159a --- /dev/null +++ b/src/main/ipc/worktree-symlinks.test.ts @@ -0,0 +1,227 @@ +import { + mkdtempSync, + mkdirSync, + writeFileSync, + statSync, + lstatSync, + readlinkSync, + rmSync, + symlinkSync, + existsSync +} from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createWorktreeSymlinks, removeWorktreeSymlinks } from './worktree-symlinks' + +describe('createWorktreeSymlinks', () => { + let root: string + let primary: string + let worktree: string + let warn: ReturnType + let error: ReturnType + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-symlinks-')) + primary = join(root, 'primary') + worktree = join(root, 'worktree') + mkdirSync(primary, { recursive: true }) + mkdirSync(worktree, { recursive: true }) + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + error = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + warn.mockRestore() + error.mockRestore() + rmSync(root, { recursive: true, force: true }) + }) + + it('symlinks a file from primary into the worktree at the same relative path', async () => { + writeFileSync(join(primary, '.env'), 'SECRET=1\n') + await createWorktreeSymlinks(primary, worktree, ['.env']) + + const linkStat = lstatSync(join(worktree, '.env')) + expect(linkStat.isSymbolicLink()).toBe(true) + expect(readlinkSync(join(worktree, '.env'))).toBe(join(primary, '.env')) + // Following the link yields the primary's contents. + expect(statSync(join(worktree, '.env')).isFile()).toBe(true) + }) + + it('symlinks a directory from primary into the worktree', async () => { + mkdirSync(join(primary, 'node_modules')) + writeFileSync(join(primary, 'node_modules', 'marker'), 'installed') + await createWorktreeSymlinks(primary, worktree, ['node_modules']) + + expect(lstatSync(join(worktree, 'node_modules')).isSymbolicLink()).toBe(true) + expect(statSync(join(worktree, 'node_modules', 'marker')).isFile()).toBe(true) + }) + + it('creates parent directories lazily for nested paths', async () => { + mkdirSync(join(primary, 'apps', 'web'), { recursive: true }) + writeFileSync(join(primary, 'apps', 'web', '.env'), 'X=1\n') + await createWorktreeSymlinks(primary, worktree, ['apps/web/.env']) + + expect(lstatSync(join(worktree, 'apps', 'web', '.env')).isSymbolicLink()).toBe(true) + }) + + it('skips entries whose source is missing in the primary checkout', async () => { + await createWorktreeSymlinks(primary, worktree, ['node_modules']) + // No link created, no throw. + expect(() => lstatSync(join(worktree, 'node_modules'))).toThrow() + expect(error).not.toHaveBeenCalled() + }) + + it('preserves a pre-existing target in the worktree (no clobber)', async () => { + writeFileSync(join(primary, '.env'), 'FROM_PRIMARY=1\n') + writeFileSync(join(worktree, '.env'), 'FROM_WORKTREE=1\n') + + await createWorktreeSymlinks(primary, worktree, ['.env']) + + // The pre-existing regular file stays; no symlink was created. + expect(lstatSync(join(worktree, '.env')).isSymbolicLink()).toBe(false) + expect(statSync(join(worktree, '.env')).isFile()).toBe(true) + }) + + it('does not escape the primary checkout via a leading-slash path', async () => { + // Why: the helper strips leading slashes (so `/etc/passwd` becomes the + // relative `etc/passwd`). No file is created outside the worktree, and the + // resolved source — which falls inside `primary/etc/passwd` — is missing, + // so the entry is silently skipped rather than linking to `/etc/passwd`. + await createWorktreeSymlinks(primary, worktree, ['/etc/passwd']) + + expect(() => lstatSync(join(worktree, 'etc', 'passwd'))).toThrow() + expect(error).not.toHaveBeenCalled() + }) + + it('rejects parent-directory traversal', async () => { + await createWorktreeSymlinks(primary, worktree, ['../secrets']) + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('[worktree-symlinks] Skipping unsafe path "../secrets"') + ) + }) + + it('rejects nested traversal via ..', async () => { + await createWorktreeSymlinks(primary, worktree, ['safe/../../escape']) + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('[worktree-symlinks] Skipping unsafe path "safe/../../escape"') + ) + }) + + it('rejects traversal using backslash separators (Windows form)', async () => { + // Why: users configuring paths on Windows (or pasting a mixed-separator + // value) could bypass a POSIX-only split. The guard normalizes across + // `/` and `\` so `..\escape` and `foo\..\..\escape` both get rejected. + await createWorktreeSymlinks(primary, worktree, ['..\\escape']) + await createWorktreeSymlinks(primary, worktree, ['foo\\..\\..\\escape']) + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('[worktree-symlinks] Skipping unsafe path "..\\escape"') + ) + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('[worktree-symlinks] Skipping unsafe path "foo\\..\\..\\escape"') + ) + }) + + it('strips a leading slash then treats the remainder as a relative path', async () => { + writeFileSync(join(primary, '.env'), 'X=1\n') + await createWorktreeSymlinks(primary, worktree, ['/.env']) + + // Leading slash is stripped in the helper; the remaining `.env` is a valid relative path. + expect(lstatSync(join(worktree, '.env')).isSymbolicLink()).toBe(true) + expect(warn).not.toHaveBeenCalled() + }) + + it('skips empty and whitespace-only entries', async () => { + // The helper logs an "unsafe path" warn for these; nothing gets linked. + await createWorktreeSymlinks(primary, worktree, ['', ' ']) + expect(error).not.toHaveBeenCalled() + }) + + it('continues processing later entries after one fails', async () => { + writeFileSync(join(primary, '.env'), 'X=1\n') + writeFileSync(join(primary, 'config.json'), '{}') + + await createWorktreeSymlinks(primary, worktree, [ + '../escape', // rejected + 'missing-source', // no source, skipped + '.env', // succeeds + 'config.json' // succeeds + ]) + + expect(lstatSync(join(worktree, '.env')).isSymbolicLink()).toBe(true) + expect(lstatSync(join(worktree, 'config.json')).isSymbolicLink()).toBe(true) + }) + + it('is a no-op for an empty paths list', async () => { + await createWorktreeSymlinks(primary, worktree, []) + expect(warn).not.toHaveBeenCalled() + expect(error).not.toHaveBeenCalled() + }) +}) + +describe('removeWorktreeSymlinks', () => { + let root: string + let primary: string + let worktree: string + let error: ReturnType + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-unlink-')) + primary = join(root, 'primary') + worktree = join(root, 'worktree') + mkdirSync(primary, { recursive: true }) + mkdirSync(worktree, { recursive: true }) + error = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + error.mockRestore() + rmSync(root, { recursive: true, force: true }) + }) + + it('unlinks configured symlinks from the worktree', async () => { + writeFileSync(join(primary, '.env'), 'SECRET=1\n') + mkdirSync(join(primary, 'node_modules')) + symlinkSync(join(primary, '.env'), join(worktree, '.env'), 'file') + symlinkSync(join(primary, 'node_modules'), join(worktree, 'node_modules'), 'dir') + + await removeWorktreeSymlinks(worktree, ['.env', 'node_modules']) + + expect(existsSync(join(worktree, '.env'))).toBe(false) + expect(existsSync(join(worktree, 'node_modules'))).toBe(false) + // Source is untouched. + expect(statSync(join(primary, '.env')).isFile()).toBe(true) + expect(statSync(join(primary, 'node_modules')).isDirectory()).toBe(true) + }) + + it('leaves a regular file at the configured path alone', async () => { + // Why: a user who created a real file at `.env` (instead of symlinking) + // must not lose it just because `.env` is in the configured list. + writeFileSync(join(worktree, '.env'), 'USER_WROTE_THIS=1\n') + + await removeWorktreeSymlinks(worktree, ['.env']) + + expect(lstatSync(join(worktree, '.env')).isSymbolicLink()).toBe(false) + expect(statSync(join(worktree, '.env')).isFile()).toBe(true) + }) + + it('ignores missing entries', async () => { + await removeWorktreeSymlinks(worktree, ['.env', 'node_modules']) + expect(error).not.toHaveBeenCalled() + }) + + it('rejects unsafe paths without touching the filesystem', async () => { + // Parent-dir traversal is silently skipped; no unlink attempted. + writeFileSync(join(root, 'outside-file'), 'DO_NOT_DELETE') + await removeWorktreeSymlinks(worktree, ['../outside-file']) + expect(existsSync(join(root, 'outside-file'))).toBe(true) + }) + + it('is a no-op for an empty paths list', async () => { + await removeWorktreeSymlinks(worktree, []) + expect(error).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/worktree-symlinks.ts b/src/main/ipc/worktree-symlinks.ts new file mode 100644 index 00000000000..67f4bc2c2f2 --- /dev/null +++ b/src/main/ipc/worktree-symlinks.ts @@ -0,0 +1,108 @@ +import { symlink, mkdir, stat, lstat, unlink } from 'fs/promises' +import { dirname, isAbsolute, resolve } from 'path' + +/** Create filesystem symlinks from the primary checkout into a freshly-created + * worktree for each configured path. Failures on individual paths are logged + * and skipped so a missing/stale entry never blocks worktree creation. + * + * Each entry is interpreted relative to `primaryPath` and placed at the same + * relative location inside `worktreePath`. Nested paths (e.g. + * `apps/web/.env`) are supported — parent directories are created lazily. */ +export async function createWorktreeSymlinks( + primaryPath: string, + worktreePath: string, + paths: readonly string[] +): Promise { + for (const rawPath of paths) { + // Why: strip leading separators (both `/` and `\`) before the guard so + // Windows-style input like `\foo` is normalized the same way POSIX `/foo` + // is, and the traversal check below sees the already-relative form. + const rel = rawPath.trim().replace(/^[\\/]+/, '') + // Why: split on both separators so a Windows-authored `..\escape` is + // rejected the same way POSIX `../escape` is. `path.isAbsolute` catches + // drive-letter absolutes (`C:\...`); the split catches relative + // backslash traversal that `.split('/')` would otherwise miss. + if (!rel || isAbsolute(rel) || rel.split(/[\\/]/).includes('..')) { + // Users can only configure paths relative to the repo root; absolute + // paths and `..` traversal are not supported. + console.warn(`[worktree-symlinks] Skipping unsafe path "${rawPath}"`) + continue + } + + const source = resolve(primaryPath, rel) + const target = resolve(worktreePath, rel) + + let sourceIsDirectory = false + try { + const s = await stat(source) + sourceIsDirectory = s.isDirectory() + } catch { + // Source doesn't exist in primary checkout — nothing to link to. This is + // a common case for fresh clones where `node_modules` hasn't been + // installed yet; silently skip rather than leaving a dangling symlink. + continue + } + + try { + // Why: if a file/dir already exists at the target location (e.g. + // git-tracked sibling with the same name), leave it alone rather than + // clobber something the user didn't mean to replace. + await stat(target) + continue + } catch { + // Target does not exist — proceed with symlink creation. + } + + try { + await mkdir(dirname(target), { recursive: true }) + // Why: Windows requires an explicit `type` ('dir' vs 'file' vs + // 'junction') for `fs.symlink`. On POSIX the argument is ignored, so + // passing it unconditionally is safe and removes a Windows-only + // failure mode when Node can't auto-detect from the source. + await symlink(source, target, sourceIsDirectory ? 'dir' : 'file') + } catch (error) { + console.error( + `[worktree-symlinks] Failed to symlink "${rel}" (${source} -> ${target}):`, + error + ) + } + } +} + +/** Remove previously-created symlinks from a worktree before deletion. + * + * Why: `git worktree remove` refuses to delete a worktree that has modified + * or untracked files. A symlink pointing at the primary's `node_modules` + * looks "untracked" to git, so users would hit "It has changed files. Use + * Force Delete" on every deletion once they've configured this feature. + * Unlink the known symlinks up front so the non-force path keeps working. + * + * Safety: only removes entries that are actually symbolic links. A regular + * file or directory at the same path is left alone — we never want to clobber + * something the user created that happens to share a name with a configured + * entry. Missing entries (ENOENT) are silently ignored. */ +export async function removeWorktreeSymlinks( + worktreePath: string, + paths: readonly string[] +): Promise { + for (const rawPath of paths) { + const rel = rawPath.trim().replace(/^[\\/]+/, '') + if (!rel || isAbsolute(rel) || rel.split(/[\\/]/).includes('..')) { + continue + } + const target = resolve(worktreePath, rel) + try { + const s = await lstat(target) + if (!s.isSymbolicLink()) { + continue + } + } catch { + continue + } + try { + await unlink(target) + } catch (error) { + console.error(`[worktree-symlinks] Failed to unlink "${rel}" (${target}):`, error) + } + } +} diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 1cf22e79ef7..e16dd825211 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -37,6 +37,7 @@ import { rebuildAuthorizedRootsCache, ensureAuthorizedRootsCache } from './files import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { killAllProcessesForWorktree } from '../runtime/worktree-teardown' import { getLocalPtyProvider } from './pty' +import { removeWorktreeSymlinks } from './worktree-symlinks' export function registerWorktreeHandlers( mainWindow: BrowserWindow, @@ -312,6 +313,20 @@ export function registerWorktreeHandlers( } } + // Why: `git worktree remove` (non-force) refuses to delete a worktree + // that has untracked files, and a symlink pointing into the primary + // checkout looks untracked to git. Unlink the user-configured symlinks + // first so the normal delete path keeps working — otherwise every + // deletion would require the Force Delete toast once the feature is on. + const settings = store.getSettings() + if ( + settings.experimentalWorktreeSymlinks && + repo.symlinkPaths && + repo.symlinkPaths.length > 0 + ) { + await removeWorktreeSymlinks(worktreePath, repo.symlinkPaths) + } + try { await removeWorktree(repo.path, worktreePath, args.force ?? false) } catch (error) { diff --git a/src/renderer/src/components/settings/ExperimentalPane.tsx b/src/renderer/src/components/settings/ExperimentalPane.tsx index 8046ab172db..fb5ffc08c28 100644 --- a/src/renderer/src/components/settings/ExperimentalPane.tsx +++ b/src/renderer/src/components/settings/ExperimentalPane.tsx @@ -73,6 +73,9 @@ export function ExperimentalPane({ const showOrchestration = matchesSettingsSearch(searchQuery, [ EXPERIMENTAL_PANE_SEARCH_ENTRIES[3] ]) + const showWorktreeSymlinks = matchesSettingsSearch(searchQuery, [ + EXPERIMENTAL_PANE_SEARCH_ENTRIES[3] + ]) const [orchestrationEnabled, setOrchestrationEnabled] = useState(() => { return localStorage.getItem('orca.orchestration.enabled') === '1' @@ -387,6 +390,44 @@ export function ExperimentalPane({ ) : null} + {showWorktreeSymlinks ? ( + +
+
+ +

+ Allows for automatic symlinks of certain folders or files that must be connected to + created worktrees. +

+
+ +
+ + ) : null} + {hiddenExperimentalUnlocked ? : null} ) diff --git a/src/renderer/src/components/settings/RepositoryPane.tsx b/src/renderer/src/components/settings/RepositoryPane.tsx index 8d266a9fef2..4a48cc8798a 100644 --- a/src/renderer/src/components/settings/RepositoryPane.tsx +++ b/src/renderer/src/components/settings/RepositoryPane.tsx @@ -10,6 +10,7 @@ import { Trash2 } from 'lucide-react' import { DEFAULT_REPO_HOOK_SETTINGS } from './SettingsConstants' import { BaseRefPicker } from './BaseRefPicker' import { RepositoryHooksSection } from './RepositoryHooksSection' +import { WorktreeSymlinksSection } from './WorktreeSymlinksSection' import { SparsePresetSettingsSection } from './SparsePresetSettingsSection' import { SearchableSetting } from './SearchableSetting' import { matchesSettingsSearch, type SettingsSearchEntry } from './settings-search' @@ -68,6 +69,21 @@ export function getRepositoryPaneSearchEntries(repo: Repo): SettingsSearchEntry[ ...(isFolder ? [] : [ + { + title: 'Worktree Symlinks', + description: + 'Paths to symlink from the primary checkout into newly created worktrees.', + keywords: [ + repo.displayName, + 'symlink', + 'symlinks', + 'worktree', + 'link', + 'shared', + 'env', + 'node_modules' + ] + }, { title: 'orca.yaml hooks', description: 'Shared setup and archive hook commands for this repository.', @@ -117,6 +133,7 @@ export function RepositoryPane({ }: RepositoryPaneProps): React.JSX.Element { const isFolder = isFolderRepo(repo) const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const symlinksEnabled = useAppStore((state) => state.settings?.experimentalWorktreeSymlinks) const [confirmingRemove, setConfirmingRemove] = useState(null) const [copiedTemplate, setCopiedTemplate] = useState(false) @@ -190,6 +207,7 @@ export function RepositoryPane({ 'Custom GitHub Issue Command' ].includes(entry.title) ) + const symlinkEntries = allEntries.filter((entry) => entry.title === 'Worktree Symlinks') const visibleSections = [ matchesSettingsSearch(searchQuery, identityEntries) ? ( @@ -287,6 +305,11 @@ export function RepositoryPane({ ) : null} ) : null, + !isFolder && + symlinksEnabled && + matchesSettingsSearch(searchQuery, symlinkEntries) ? ( + + ) : null, !isFolder && matchesSettingsSearch(searchQuery, sparsePresetEntries) ? ( ) : null, diff --git a/src/renderer/src/components/settings/WorktreeSymlinksSection.tsx b/src/renderer/src/components/settings/WorktreeSymlinksSection.tsx new file mode 100644 index 00000000000..59a7fcc620e --- /dev/null +++ b/src/renderer/src/components/settings/WorktreeSymlinksSection.tsx @@ -0,0 +1,201 @@ +import { useEffect, useMemo, useState } from 'react' +import { File, Folder, Link2, Plus, X } from 'lucide-react' +import type { Repo } from '../../../../shared/types' +import { Button } from '../ui/button' +import { Command, CommandEmpty, CommandInput, CommandItem, CommandList } from '../ui/command' +import { Popover, PopoverContent, PopoverTrigger } from '../ui/popover' +import { cn } from '@/lib/utils' +import { SearchableSetting } from './SearchableSetting' + +type WorktreeSymlinksSectionProps = { + repo: Repo + updateRepo: (repoId: string, updates: Partial) => void +} + +type DirEntry = { name: string; isDirectory: boolean } + +const MAX_SUGGESTIONS = 50 + +export function WorktreeSymlinksSection({ + repo, + updateRepo +}: WorktreeSymlinksSectionProps): React.JSX.Element { + const [open, setOpen] = useState(false) + const [query, setQuery] = useState('') + const [entries, setEntries] = useState([]) + + const paths = repo.symlinkPaths ?? [] + const queryTrimmed = query.trim().replace(/^\/+/, '') + + useEffect(() => { + let cancelled = false + void window.api.fs + .readDir({ dirPath: repo.path, connectionId: repo.connectionId ?? undefined }) + .then((list) => { + if (cancelled) { + return + } + setEntries(list.map((entry) => ({ name: entry.name, isDirectory: entry.isDirectory }))) + }) + .catch(() => { + // Non-fatal: without entries the combobox still works as a free-text + // input — the user can type any path and commit it. + }) + return () => { + cancelled = true + } + }, [repo.path, repo.connectionId]) + + const filtered = useMemo(() => { + const q = queryTrimmed.toLowerCase() + const base = q ? entries.filter((e) => e.name.toLowerCase().includes(q)) : entries + return base.slice(0, MAX_SUGGESTIONS) + }, [queryTrimmed, entries]) + + const hasExactMatch = filtered.some((e) => e.name === queryTrimmed) + const showLiteralItem = queryTrimmed.length > 0 && !hasExactMatch && !paths.includes(queryTrimmed) + + const commit = (rawName: string): void => { + const trimmed = rawName.trim().replace(/^\/+/, '') + if (!trimmed || paths.includes(trimmed)) { + setQuery('') + return + } + updateRepo(repo.id, { symlinkPaths: [...paths, trimmed] }) + setQuery('') + setOpen(false) + } + + const handleRemove = (path: string): void => { + updateRepo(repo.id, { symlinkPaths: paths.filter((p) => p !== path) }) + } + + return ( + +
+
+

Worktree Symlinks

+

+ When a new worktree is created, each path listed here will be symlinked from the primary + checkout. +

+
+ + + + + + + + + No matches. Keep typing to add a custom path. + {showLiteralItem ? ( + commit(queryTrimmed)} + className="items-center gap-2 px-3 py-2" + > + + + Add{' '} + + {queryTrimmed} + + + + ) : null} + {filtered.map((entry) => { + const alreadyAdded = paths.includes(entry.name) + return ( + commit(entry.name)} + className={cn('items-center gap-2 px-3 py-2', alreadyAdded && 'opacity-50')} + > + {entry.isDirectory ? ( + + ) : ( + + )} + {entry.name} + {alreadyAdded ? ( + + added + + ) : null} + + ) + })} + + + + +
+ + {paths.length === 0 ? ( +
+ No symlink paths configured for this repository. +
+ ) : ( +
+
+
+ +
+
+
+

Linked paths

+ + {paths.length === 1 ? '1 path' : `${paths.length} paths`} + +
+
+ {paths.map((path) => ( + + {path} + + + ))} +
+
+
+
+ )} + + ) +} diff --git a/src/renderer/src/components/settings/experimental-search.ts b/src/renderer/src/components/settings/experimental-search.ts index 9066f38794d..70382a48a7e 100644 --- a/src/renderer/src/components/settings/experimental-search.ts +++ b/src/renderer/src/components/settings/experimental-search.ts @@ -69,5 +69,22 @@ export const EXPERIMENTAL_PANE_SEARCH_ENTRIES: SettingsSearchEntry[] = [ 'worker', 'coordinator' ] + }, + { + title: 'Symlinks on worktrees', + description: + 'Automatically symlink configured files or folders into newly created worktrees so shared state (envs, caches, installs) stays connected.', + keywords: [ + 'experimental', + 'worktree', + 'worktrees', + 'symlink', + 'symlinks', + 'link', + 'links', + 'shared', + 'env', + 'node_modules' + ] } ] diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index f203a36b6c4..dceeb8c43c9 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -23,6 +23,7 @@ export type RepoSlice = { | 'hookSettings' | 'worktreeBaseRef' | 'kind' + | 'symlinkPaths' | 'issueSourcePreference' > > diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 81b2b4d473f..79321ead5c1 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -202,7 +202,8 @@ export function getDefaultSettings(homedir: string): GlobalSettings { experimentalMobile: false, // Why: off by default — opt-in cosmetic joke feature. Leaving the default // false keeps the overlay unmounted for users who never enable it. - experimentalSidekick: false + experimentalSidekick: false, + experimentalWorktreeSymlinks: false } } diff --git a/src/shared/types.ts b/src/shared/types.ts index a10f9a333d6..25b13e3a554 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -37,6 +37,12 @@ export type Repo = { * identically to `'auto'`; writers leave it undefined on creation so * existing persisted records stay forward-compatible. */ issueSourcePreference?: IssueSourcePreference + /** Paths (relative to the primary checkout) that should be symlinked into + * newly created worktrees of this repo. Consumed only when the global + * `experimentalWorktreeSymlinks` flag is on — the per-repo list is the + * "what to link", the global flag is the "whether to link at all" switch. + * Undefined/empty means no symlinks are created for this repo. */ + symlinkPaths?: string[] } export type SetupRunPolicy = 'ask' | 'run-by-default' | 'skip-by-default' @@ -1158,6 +1164,12 @@ export type GlobalSettings = { * off never mount the overlay. Toggling takes effect immediately in the * current session (no relaunch) because it is purely renderer-side. */ experimentalSidekick: boolean + /** Experimental: when creating a worktree, automatically symlink a + * user-configured set of files/folders from the primary checkout (e.g. + * `.env`, `node_modules`) into the new worktree. Opt-in while the + * configuration surface and edge cases (conflicts with existing paths, + * cleanup on worktree delete) are still being worked out. */ + experimentalWorktreeSymlinks: boolean /** Anonymous product-telemetry state. Optional because the one-shot * migration in `Store.load()` is what populates it on first boot of the * telemetry release; before migration runs, the field is absent. After From e2d800ade816f7a62a92115383c175a0cca0432f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 4 May 2026 14:53:02 -0700 Subject: [PATCH 02/46] feat(ssh): gate auto-connect on passphrase prompt (#1409) Adds ssh:needsPassphrasePrompt IPC so tab focus / Cmd+J auto-connect waits for user-driven connect when a passphrase dialog would otherwise pop unprompted. No-passphrase targets continue auto-connecting. Co-authored-by: Orca --- src/main/ipc/ssh.ts | 16 +++++ src/main/ssh/ssh-connection.ts | 14 +++- src/preload/api-types.ts | 1 + src/preload/index.ts | 3 + .../terminal-pane/pty-connection.ts | 68 +++++++++++++++++++ 5 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index 91bba2f0487..edb556697b1 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -162,6 +162,7 @@ export function registerSshHandlers( 'ssh:connect', 'ssh:disconnect', 'ssh:getState', + 'ssh:needsPassphrasePrompt', 'ssh:testConnection', 'ssh:addPortForward', 'ssh:updatePortForward', @@ -424,6 +425,21 @@ export function registerSshHandlers( return connectionManager!.getState(args.targetId) }) + // Why: callers that want to auto-connect (Cmd+J jump, terminal reattach) need + // to know whether doing so will pop a passphrase/password dialog. Auto-firing + // the connect is fine when no prompt is needed, but surprising otherwise — + // the user expects to enter the credential before the app starts connecting. + // Returns true if the target's last successful connect required a credential + // AND the live SshConnection (if any) does not already have one cached. + ipcMain.handle('ssh:needsPassphrasePrompt', (_event, args: { targetId: string }) => { + const target = sshStore!.getTarget(args.targetId) + if (!target?.lastRequiredPassphrase) { + return false + } + const conn = connectionManager!.getConnection(args.targetId) + return !conn?.hasCachedCredential() + }) + ipcMain.handle('ssh:testConnection', async (_event, args: { targetId: string }) => { const target = sshStore!.getTarget(args.targetId) if (!target) { diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index 6c986fbcb48..fc19c622b1c 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -55,6 +55,16 @@ export class SshConnection { return { ...this.target } } + // Why: exposes whether a passphrase/password is already cached in-memory for + // this connection. Used by ssh:needsPassphrasePrompt so callers can decide + // whether a manual-reconnect will prompt or go through silently. Without this, + // lastRequiredPassphrase stays true across the session even after the user + // has entered the credential once, causing redundant "enter passphrase" + // prompts on disconnect→reconnect cycles within a single app session. + hasCachedCredential(): boolean { + return this.cachedPassphrase != null || this.cachedPassword != null + } + async exec(cmd: string): Promise { if (!this.client) { throw new Error('Not connected') @@ -110,7 +120,9 @@ export class SshConnection { this.proxyProcess = null const connectGeneration = ++this.connectGeneration - const resolved = await resolveWithSshG(this.target.configHost || this.target.label).catch(() => null) + const resolved = await resolveWithSshG(this.target.configHost || this.target.label).catch( + () => null + ) const config = buildConnectConfig(this.target, resolved) // Why: ssh2 doesn't support ProxyCommand/ProxyJump natively. Spawn the diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index f687cb8d36f..349d49f0f35 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -969,6 +969,7 @@ export type PreloadApi = { connect: (args: { targetId: string }) => Promise disconnect: (args: { targetId: string }) => Promise getState: (args: { targetId: string }) => Promise + needsPassphrasePrompt: (args: { targetId: string }) => Promise testConnection: (args: { targetId: string }) => Promise<{ success: boolean; error?: string; state?: SshConnectionState }> diff --git a/src/preload/index.ts b/src/preload/index.ts index 0b4cac85b22..6df80cad45c 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1763,6 +1763,9 @@ const api = { getState: (args: { targetId: string }): Promise => ipcRenderer.invoke('ssh:getState', args), + needsPassphrasePrompt: (args: { targetId: string }): Promise => + ipcRenderer.invoke('ssh:needsPassphrasePrompt', args), + testConnection: (args: { targetId: string }): Promise<{ success: boolean; error?: string; state?: SshConnectionState }> => diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index ce27e9a7b00..75d494f39b4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -683,6 +683,74 @@ export function connectPanePty( ) if (pendingSessionId || isDeferredTarget) { void (async () => { + // Why: if the target requires a passphrase/password and no credential + // is cached yet, auto-firing ssh.connect would surprise the user — + // a prompt pops unprompted just because they focused a tab / jumped + // via Cmd+J. Wait for the user to initiate the connect (via + // SshDisconnectedDialog → passphrase dialog) before proceeding with + // the PTY reattach. No-passphrase targets (ssh-agent, unencrypted + // key, cached creds) return false here and continue auto-connecting + // as before. + let needsPrompt = false + try { + needsPrompt = await window.api.ssh.needsPassphrasePrompt({ + targetId: connectionId + }) + } catch (err) { + console.warn('[pty-connection] needsPassphrasePrompt probe failed:', err) + // Why: if the probe fails, fall through to the existing auto-connect + // behavior rather than stranding the tab — a stuck tab is worse + // than a surprising prompt. + } + if (disposed) { + return + } + if (needsPrompt) { + const alreadyConnected = + useAppStore.getState().sshConnectionStates.get(connectionId)?.status === 'connected' + if (!alreadyConnected) { + // Wait for the user-driven connect (SshDisconnectedDialog → + // passphrase dialog → ssh.connect) to complete, then continue. + // Why: resolve on terminal-failure statuses too ('auth-failed', + // 'error', 'reconnection-failed') so this promise can't hang + // forever if the user cancels or the connect fails — + // waitForSshConnection below has its own error path that will + // surface the failure via reportError. + await new Promise((resolve) => { + const isTerminalStatus = (status: string | undefined): boolean => + status === 'connected' || + status === 'auth-failed' || + status === 'error' || + status === 'reconnection-failed' + const unsub = useAppStore.subscribe((state) => { + if (disposed) { + unsub() + resolve() + return + } + if (isTerminalStatus(state.sshConnectionStates.get(connectionId)?.status)) { + unsub() + resolve() + } + }) + // Why: re-read state immediately after subscribing to close the + // race where status transitioned between the alreadyConnected + // check above and the subscribe registration — otherwise we'd + // wait forever for a state change that already happened. + const currentStatus = useAppStore + .getState() + .sshConnectionStates.get(connectionId)?.status + if (isTerminalStatus(currentStatus)) { + unsub() + resolve() + } + }) + if (disposed) { + return + } + } + } + // Why: ensure the SSH connection is established before attempting // PTY reattach. Multiple panes/tabs may need the same connection, // so we wait for it rather than returning early when in-flight. From 56686ea03525296b67cde453b3ba810f06d644b5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 4 May 2026 14:58:09 -0700 Subject: [PATCH 03/46] fix(worktrees): preserve SSH focus recency and route agent clicks through activation helper (#1410) - Scope pruneLastVisitedTimestamps per-repo so not-yet-hydrated SSH repos retain persisted Cmd+J recency instead of being wiped at startup. - Parse lastVisitedAtByWorktreeId leniently: drop only bad entries rather than failing the whole workspace session on one corrupted timestamp. - Route sidebar agent-tab clicks through activateAndRevealWorktree so cross-repo activation and nav history are not silently skipped. Co-authored-by: Orca --- .../components/sidebar/WorktreeCardAgents.tsx | 26 +++++++---------- .../src/store/slices/worktrees.test.ts | 18 ++++++++++-- src/renderer/src/store/slices/worktrees.ts | 28 ++++++++++++++++--- src/shared/workspace-session-schema.test.ts | 21 ++++++++++++++ src/shared/workspace-session-schema.ts | 26 +++++++++++++++-- 5 files changed, 94 insertions(+), 25 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx index c1dbfb69166..6f731c56bc7 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useMemo } from 'react' import { ChevronDown } from 'lucide-react' import { useAppStore } from '@/store' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' import DashboardAgentRow from '@/components/dashboard/DashboardAgentRow' import { useNow } from '@/components/dashboard/useNow' import { useWorktreeAgentRows } from './useWorktreeAgentRows' @@ -50,11 +51,8 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ }: BodyProps) { const dropAgentStatus = useAppStore((s) => s.dropAgentStatus) const dismissRetainedAgent = useAppStore((s) => s.dismissRetainedAgent) - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const setActiveTab = useAppStore((s) => s.setActiveTab) - const setActiveView = useAppStore((s) => s.setActiveView) const acknowledgeAgents = useAppStore((s) => s.acknowledgeAgents) - const markWorktreeVisited = useAppStore((s) => s.markWorktreeVisited) // Why: per-worktree collapse is session-only UI state. Single-primitive // subscription so the card only re-renders when THIS worktree's collapsed @@ -89,24 +87,20 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ const handleActivateAgentTab = useCallback( (tabId: string, paneKey: string) => { acknowledgeAgents([paneKey]) - setActiveWorktree(worktreeId) - // Why: sidebar agent-tab click is a user-initiated switch; stamp focus - // recency for Cmd+J. See docs/cmd-j-empty-query-ordering.md. - markWorktreeVisited(worktreeId) - setActiveView('terminal') + // Why: route through activateAndRevealWorktree so cross-repo clicks also + // set activeRepoId, record a nav-history entry, clear sidebar filters, + // reveal the card, and stamp focus recency — per the design doc rule + // "Every user-initiated worktree switch must route through + // activateAndRevealWorktree". Bypassing it (direct setActiveWorktree + + // markWorktreeVisited) silently skipped cross-repo activation and + // back/forward history for clicks from inline agent rows. + activateAndRevealWorktree(worktreeId) const tabs = useAppStore.getState().tabsByWorktree[worktreeId] ?? [] if (tabs.some((t) => t.id === tabId)) { setActiveTab(tabId) } }, - [ - worktreeId, - setActiveWorktree, - setActiveTab, - setActiveView, - acknowledgeAgents, - markWorktreeVisited - ] + [worktreeId, setActiveTab, acknowledgeAgents] ) const handleToggleCollapsed = useCallback( diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 4e8a4ccf3b6..57b10152722 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -779,14 +779,28 @@ describe('markWorktreeVisited', () => { expect(store.getState().lastVisitedAtByWorktreeId['wt-1']).toBe(existing) }) - it('pruneLastVisitedTimestamps drops entries for unknown worktree IDs', () => { + it('pruneLastVisitedTimestamps drops entries for unknown worktree IDs within hydrated repos', () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a' }) store.setState({ worktreesByRepo: { repo1: [wt] }, - lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'stale-id': 200 } + lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'repo1::/gone': 200 } } as Partial) store.getState().pruneLastVisitedTimestamps() expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ 'repo1::/a': 100 }) }) + + it('pruneLastVisitedTimestamps preserves entries for not-yet-hydrated repos (e.g. SSH pre-connect)', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a' }) + store.setState({ + worktreesByRepo: { repo1: [wt] }, + lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'ssh-repo::/b': 200 } + } as Partial) + store.getState().pruneLastVisitedTimestamps() + expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ + 'repo1::/a': 100, + 'ssh-repo::/b': 200 + }) + }) }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 037f801cd98..e59e277562f 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -584,16 +584,36 @@ export const createWorktreeSlice: StateCreator pruneLastVisitedTimestamps: () => { set((s) => { - const validIds = new Set() - for (const list of Object.values(s.worktreesByRepo)) { + // Why: scope pruning per-repo. SSH-backed repos cannot enumerate + // worktrees until their connection is established, so at hydration + // time worktreesByRepo[sshRepoId] is empty/undefined. If we pruned + // globally based on the union of all repos' worktrees, we would wipe + // every persisted focus-recency entry for SSH worktrees — precisely + // the set this feature exists to preserve. Instead, only drop entries + // whose repo has a populated worktree list: a missing repoId means + // "not yet hydrated" (defer), a repoId with an empty list after a + // successful listing means the worktree really is gone (drop). + // The ssh:state-changed 'connected' handler re-fetches worktrees and + // a follow-up prune runs from the same site if needed. + const validIdsByRepo = new Map>() + for (const [repoId, list] of Object.entries(s.worktreesByRepo)) { + const ids = new Set() for (const w of list) { - validIds.add(w.id) + ids.add(w.id) } + validIdsByRepo.set(repoId, ids) } let changed = false const next: Record = {} for (const [id, ts] of Object.entries(s.lastVisitedAtByWorktreeId)) { - if (validIds.has(id)) { + const repoId = getRepoIdFromWorktreeId(id) + const repoIds = validIdsByRepo.get(repoId) + if (!repoIds) { + // Repo not yet hydrated (e.g. SSH not connected). Keep the entry. + next[id] = ts + continue + } + if (repoIds.has(id)) { next[id] = ts } else { changed = true diff --git a/src/shared/workspace-session-schema.test.ts b/src/shared/workspace-session-schema.test.ts index 0b24fe5eb17..4a25933f4aa 100644 --- a/src/shared/workspace-session-schema.test.ts +++ b/src/shared/workspace-session-schema.test.ts @@ -95,4 +95,25 @@ describe('parseWorkspaceSession', () => { expect(parseWorkspaceSession('garbage').ok).toBe(false) expect(parseWorkspaceSession(42).ok).toBe(false) }) + + it('drops bad lastVisitedAtByWorktreeId entries rather than failing the session', () => { + const result = parseWorkspaceSession({ + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + lastVisitedAtByWorktreeId: { + good: 1_700_000_000_000, + nan: Number.NaN, + infinite: Number.POSITIVE_INFINITY, + negative: -5, + string: 'nope' + } + }) + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value.lastVisitedAtByWorktreeId).toEqual({ good: 1_700_000_000_000 }) + } + }) }) diff --git a/src/shared/workspace-session-schema.ts b/src/shared/workspace-session-schema.ts index 30ebae27044..66b1f3b3c39 100644 --- a/src/shared/workspace-session-schema.ts +++ b/src/shared/workspace-session-schema.ts @@ -207,9 +207,29 @@ export const workspaceSessionStateSchema: z.ZodType = z.o remoteSessionIdsByTabId: z.record(z.string(), z.string()).optional(), // Why: the sort comparator in order-empty-query-worktrees.ts would produce // NaN (undefined sort order) if a corrupted session file carried NaN or - // Infinity here. Constrain to finite non-negative numbers so a bad on-disk - // value is rejected at parse time rather than silently breaking Cmd+J. - lastVisitedAtByWorktreeId: z.record(z.string(), z.number().finite().nonnegative()).optional() + // Infinity here. Parse leniently: drop individual bad entries rather than + // failing the entire session. A strict record() rejection here would cause + // parseWorkspaceSession to fall back to defaults for the ENTIRE session + // (terminals, editors, browsers, layouts) on a single corrupted timestamp + // — a blast radius far larger than "Cmd+J falls back to activity recency", + // which is all this field gates. + lastVisitedAtByWorktreeId: z + .preprocess( + (raw) => { + if (raw == null || typeof raw !== 'object') { + return raw + } + const cleaned: Record = {} + for (const [k, v] of Object.entries(raw as Record)) { + if (typeof v === 'number' && Number.isFinite(v) && v >= 0) { + cleaned[k] = v + } + } + return cleaned + }, + z.record(z.string(), z.number().finite().nonnegative()) + ) + .optional() }) export type ParsedWorkspaceSession = From a595c1f84b37e915fdfd29dfbbfb08fe6bb78567 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 22:25:11 +0000 Subject: [PATCH 04/46] release: v1.3.35-rc.0 [rc-slot:2026-05-04-15] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a20272b3ea6..cedff512124 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.3.34-rc.1", + "version": "1.3.35-rc.0", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", From 9b3b2a5486bb9b9ccc028feffa6c088836984dd1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 4 May 2026 15:43:14 -0700 Subject: [PATCH 05/46] feat(source-control): full-row hover, tooltips, and staged discard-all (#1407) Co-authored-by: Orca --- .../right-sidebar/ActionButton.test.tsx | 151 +++++++ .../right-sidebar/SourceControl.tsx | 404 ++++++++++++++---- .../discard-all-sequence.test.ts | 284 ++++++++++++ .../right-sidebar/discard-all-sequence.ts | 122 ++++++ 4 files changed, 880 insertions(+), 81 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/ActionButton.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts create mode 100644 src/renderer/src/components/right-sidebar/discard-all-sequence.ts diff --git a/src/renderer/src/components/right-sidebar/ActionButton.test.tsx b/src/renderer/src/components/right-sidebar/ActionButton.test.tsx new file mode 100644 index 00000000000..55353d29850 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/ActionButton.test.tsx @@ -0,0 +1,151 @@ +import { describe, expect, it, vi } from 'vitest' +import { Plus } from 'lucide-react' +import { ActionButton } from './SourceControl' +import { Button } from '@/components/ui/button' + +type ReactElementLike = { + type: unknown + props: Record +} + +function visit(node: unknown, cb: (node: ReactElementLike) => void): void { + if (node == null || typeof node === 'string' || typeof node === 'number') { + return + } + if (Array.isArray(node)) { + node.forEach((entry) => visit(entry, cb)) + return + } + const element = node as ReactElementLike + cb(element) + if (element.props?.children) { + visit(element.props.children, cb) + } +} + +function findInnerButton(node: unknown): ReactElementLike { + let found: ReactElementLike | null = null + visit(node, (entry) => { + if (entry.type === Button) { + found = entry + } + }) + if (!found) { + throw new Error('inner Button not found') + } + return found +} + +function findTooltipContentText(node: unknown): string { + const texts: string[] = [] + visit(node, (entry) => { + const typeName = + typeof entry.type === 'function' || typeof entry.type === 'object' + ? ((entry.type as { displayName?: string; name?: string }).displayName ?? + (entry.type as { displayName?: string; name?: string }).name ?? + '') + : '' + if (typeName === 'TooltipContent') { + const children = entry.props?.children + if (typeof children === 'string') { + texts.push(children) + } + } + }) + return texts.join(' ') +} + +// Why: ActionButton's onClick handler only touches these three fields, so a +// narrow event type is both correct and avoids a forbidden `as unknown as X` +// double-cast on the event value. +type MinimalMouseEvent = Pick< + React.MouseEvent, + 'preventDefault' | 'stopPropagation' | 'defaultPrevented' +> + +function makeClickEvent(): { + event: MinimalMouseEvent + preventDefault: ReturnType +} { + const preventDefault = vi.fn() + const event: MinimalMouseEvent = { + preventDefault, + stopPropagation: vi.fn(), + defaultPrevented: false + } + return { event, preventDefault } +} + +const baseProps = { + icon: Plus, + title: 'Stage all', + onClick: vi.fn() +} + +describe('ActionButton', () => { + it('forwards the title as aria-label on the inner button', () => { + const element = ActionButton({ ...baseProps, onClick: vi.fn() }) + const button = findInnerButton(element) + expect(button.props['aria-label']).toBe('Stage all') + }) + + it('renders the title as tooltip content (non-native, Radix)', () => { + const element = ActionButton({ ...baseProps, onClick: vi.fn() }) + // Why: the Radix TooltipContent is what renders the visual tooltip. + // Native `title` was removed because its styling diverges from the + // rest of the sidebar chrome. + expect(findTooltipContentText(element)).toContain('Stage all') + }) + + it('calls the onClick handler when enabled', () => { + const onClick = vi.fn() + const element = ActionButton({ ...baseProps, onClick }) + const button = findInnerButton(element) + const { event } = makeClickEvent() + ;(button.props.onClick as (e: MinimalMouseEvent) => void)(event) + expect(onClick).toHaveBeenCalledWith(event) + }) + + it('does NOT render the native disabled prop on the inner button', () => { + // Why: Radix TooltipTrigger on a DOM-disabled - ) : ( - - ) + <> + {/* Why: bulk action buttons are hover-only on + pointer devices to avoid cluttering the section + header with persistent icons. On no-hover + pointers (touch, and SSH sessions where hover + state is unreliable — see AGENTS.md "SSH Use + Case"), force them visible so they're reachable + without tabbing. One outer wrapper so that + focusing any action reveals all three siblings — + otherwise keyboard users tab into an invisible + next stop. */} +
+ {canRevertAll && ( + { + event.stopPropagation() + void handleRevertAllInArea(area) + }} + disabled={isExecutingBulk} + /> + )} + {canStageAll && ( + { + event.stopPropagation() + if (area === 'unstaged' || area === 'untracked') { + void handleStageAllInArea(area) + } + }} + disabled={isExecutingBulk} + /> + )} + {canUnstageAll && ( + { + event.stopPropagation() + void handleUnstageAll() + }} + disabled={isExecutingBulk} + /> + )} +
+ {items.some((entry) => entry.conflictStatus === 'unresolved') ? ( + + ) : ( + + )} + } /> {!isCollapsed && @@ -1420,25 +1621,31 @@ function SectionHeader({ onToggle: () => void actions?: React.ReactNode }): React.JSX.Element { + // Why: wrap the toggle button and actions in a shared rounded container + // so the hover background spans the entire row instead of clipping around + // the label. The outer div keeps the vertical spacing that separates + // sections; the inner wrapper owns the hover rectangle. return ( -
- -
{actions}
+
+
+ +
{actions}
+
) } @@ -1928,26 +2135,61 @@ function EmptyState({ ) } -function ActionButton({ +export function ActionButton({ icon: Icon, title, - onClick + onClick, + disabled }: { icon: React.ComponentType<{ className?: string }> title: string onClick: (event: React.MouseEvent) => void + disabled?: boolean }): React.JSX.Element { + // Why: use the Radix Tooltip instead of the native `title` attribute so the + // label matches the rest of the sidebar chrome (consistent styling, no OS + // delay quirks, dismissible on pointer leave). + // + // Why (no local TooltipProvider): the app root mounts a single + // TooltipProvider (see App.tsx); nesting another one here gives this subtree + // its own delay-timing state and breaks Radix's "skip the open delay when + // moving between adjacent tooltip triggers" handoff between sibling action + // buttons in the section header. + // + // Why (disabled handling): Radix's TooltipTrigger asChild on a disabled + // + + + + + + {title} + + ) } diff --git a/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts b/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts new file mode 100644 index 00000000000..f7a2dcb5f7c --- /dev/null +++ b/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts @@ -0,0 +1,284 @@ +import { describe, expect, it, vi } from 'vitest' +import { + getDiscardAllPaths, + getStageAllPaths, + getUnstageAllPaths, + runDiscardAllForArea, + type DiscardAllArea +} from './discard-all-sequence' +import type { GitStatusEntry } from '../../../../shared/types' + +function entry(partial: Partial & { path: string }): GitStatusEntry { + return { + status: 'modified', + area: 'unstaged', + ...partial + } +} + +describe('getDiscardAllPaths', () => { + it('returns only paths in the requested area', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'a.ts', area: 'staged' }), + entry({ path: 'b.ts', area: 'unstaged' }), + entry({ path: 'c.ts', area: 'untracked', status: 'untracked' }) + ] + expect(getDiscardAllPaths(entries, 'staged')).toEqual(['a.ts']) + expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['b.ts']) + expect(getDiscardAllPaths(entries, 'untracked')).toEqual(['c.ts']) + }) + + it('skips entries with an unresolved conflict', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'clean.ts', area: 'unstaged' }), + entry({ + path: 'conflict.ts', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }) + ] + // Why: `git restore --worktree --source=HEAD` on an unresolved conflict + // clears the `u` record silently before the user has reviewed it, which + // is why the per-row Stage/Discard buttons also suppress this case. + expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['clean.ts']) + }) + + it('skips entries resolved locally but not yet re-staged', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'clean.ts', area: 'unstaged' }), + entry({ + path: 'resolved.ts', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'resolved_locally' + }) + ] + // Why: discarding a locally-resolved file loses the resolution. The user + // would have to re-resolve from scratch — treat it as too dangerous to + // include in a bulk action. + expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['clean.ts']) + }) + + it('returns an empty array when nothing matches', () => { + expect(getDiscardAllPaths([], 'staged')).toEqual([]) + expect(getDiscardAllPaths([entry({ path: 'a.ts', area: 'staged' })], 'unstaged')).toEqual([]) + }) +}) + +describe('getStageAllPaths', () => { + it('returns only paths in the requested area', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'a.ts', area: 'staged' }), + entry({ path: 'b.ts', area: 'unstaged' }), + entry({ path: 'c.ts', area: 'untracked', status: 'untracked' }) + ] + expect(getStageAllPaths(entries, 'unstaged')).toEqual(['b.ts']) + expect(getStageAllPaths(entries, 'untracked')).toEqual(['c.ts']) + }) + + it('skips entries with an unresolved conflict', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'clean.ts', area: 'unstaged' }), + entry({ + path: 'conflict.ts', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }) + ] + // Why: `git add` on an unresolved conflict silently clears the `u` + // record before the user has reviewed it — same hazard the per-row + // Stage button guards against. + expect(getStageAllPaths(entries, 'unstaged')).toEqual(['clean.ts']) + }) + + it('includes entries that are resolved locally', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'clean.ts', area: 'unstaged' }), + entry({ + path: 'resolved.ts', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'resolved_locally' + }) + ] + // Why: staging a locally-resolved file is the normal resolution + // workflow — it marks the conflict as finished. Unlike discard, this + // must NOT be filtered out. + expect(getStageAllPaths(entries, 'unstaged')).toEqual(['clean.ts', 'resolved.ts']) + }) + + it('returns an empty array when nothing matches', () => { + expect(getStageAllPaths([], 'unstaged')).toEqual([]) + expect(getStageAllPaths([entry({ path: 'a.ts', area: 'staged' })], 'unstaged')).toEqual([]) + }) +}) + +describe('getUnstageAllPaths', () => { + it('returns only staged-area paths', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'a.ts', area: 'staged' }), + entry({ path: 'b.ts', area: 'unstaged' }), + entry({ path: 'c.ts', area: 'untracked', status: 'untracked' }) + ] + expect(getUnstageAllPaths(entries)).toEqual(['a.ts']) + }) + + it('includes staged conflict rows', () => { + const entries: GitStatusEntry[] = [ + entry({ path: 'clean.ts', area: 'staged' }), + entry({ + path: 'conflict.ts', + area: 'staged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }), + entry({ + path: 'resolved.ts', + area: 'staged', + conflictKind: 'both_modified', + conflictStatus: 'resolved_locally' + }) + ] + // Why: `git reset HEAD` on a staged conflict row is safe and mirrors + // the per-row Unstage action — no conflict filter here. + expect(getUnstageAllPaths(entries)).toEqual(['clean.ts', 'conflict.ts', 'resolved.ts']) + }) + + it('returns an empty array when nothing is staged', () => { + expect(getUnstageAllPaths([])).toEqual([]) + expect(getUnstageAllPaths([entry({ path: 'a.ts', area: 'unstaged' })])).toEqual([]) + }) +}) + +describe('runDiscardAllForArea', () => { + function makeDeps( + overrides: { + bulkUnstageError?: unknown + discardOneError?: (path: string) => unknown + } = {} + ) { + const bulkUnstageCalls: string[][] = [] + const discardOneCalls: string[] = [] + const errors: unknown[] = [] + + const bulkUnstage = vi.fn(async (paths: string[]) => { + bulkUnstageCalls.push([...paths]) + if (overrides.bulkUnstageError !== undefined) { + throw overrides.bulkUnstageError + } + }) + const discardOne = vi.fn(async (path: string) => { + discardOneCalls.push(path) + if (overrides.discardOneError) { + const err = overrides.discardOneError(path) + if (err !== undefined) { + throw err + } + } + }) + const onError = vi.fn((error: unknown) => { + errors.push(error) + }) + + return { + deps: { bulkUnstage, discardOne, onError }, + bulkUnstageCalls, + discardOneCalls, + errors, + bulkUnstage, + discardOne, + onError + } + } + + it('no-ops when the path list is empty', async () => { + const ctx = makeDeps() + const result = await runDiscardAllForArea('staged', [], ctx.deps) + expect(result).toEqual({ discarded: [], failed: [], aborted: false }) + expect(ctx.bulkUnstage).not.toHaveBeenCalled() + expect(ctx.discardOne).not.toHaveBeenCalled() + }) + + it('discards unstaged paths one-by-one without bulk-unstaging', async () => { + const ctx = makeDeps() + const result = await runDiscardAllForArea('unstaged', ['a.ts', 'b.ts'], ctx.deps) + expect(result).toEqual({ discarded: ['a.ts', 'b.ts'], failed: [], aborted: false }) + expect(ctx.bulkUnstage).not.toHaveBeenCalled() + expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts']) + }) + + it('discards untracked paths one-by-one without bulk-unstaging', async () => { + const ctx = makeDeps() + const result = await runDiscardAllForArea('untracked', ['new.ts'], ctx.deps) + expect(result).toEqual({ discarded: ['new.ts'], failed: [], aborted: false }) + expect(ctx.bulkUnstage).not.toHaveBeenCalled() + expect(ctx.discardOneCalls).toEqual(['new.ts']) + }) + + it('bulk-unstages staged paths before the per-file discard loop', async () => { + const ctx = makeDeps() + const result = await runDiscardAllForArea('staged', ['a.ts', 'b.ts'], ctx.deps) + expect(result).toEqual({ discarded: ['a.ts', 'b.ts'], failed: [], aborted: false }) + expect(ctx.bulkUnstageCalls).toEqual([['a.ts', 'b.ts']]) + expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts']) + // Why: bulk unstage MUST happen strictly before any discard, otherwise + // the index would still hold the staged delta when the worktree was + // reset and the files would reappear as inverse changes. + expect(ctx.bulkUnstage.mock.invocationCallOrder[0]).toBeLessThan( + ctx.discardOne.mock.invocationCallOrder[0] + ) + }) + + it('aborts and skips the discard loop if bulk-unstage rejects', async () => { + const error = new Error('index locked') + const ctx = makeDeps({ bulkUnstageError: error }) + const result = await runDiscardAllForArea('staged', ['a.ts', 'b.ts'], ctx.deps) + expect(result).toEqual({ discarded: [], failed: [], aborted: true }) + // Why: a failed unstage + successful discard would leave the index with + // the staged delta and the worktree at HEAD — a worse state than we + // started in. The discard loop must not run. + expect(ctx.discardOne).not.toHaveBeenCalled() + expect(ctx.errors).toEqual([error]) + }) + + it('continues past a per-file discard failure and records it in `failed`', async () => { + const error = new Error('EPERM') + const ctx = makeDeps({ + discardOneError: (path) => (path === 'b.ts' ? error : undefined) + }) + const result = await runDiscardAllForArea('unstaged', ['a.ts', 'b.ts', 'c.ts'], ctx.deps) + // Why: best-effort continuation — one stuck file shouldn't block the + // rest of a bulk action the user explicitly triggered. + expect(result).toEqual({ + discarded: ['a.ts', 'c.ts'], + failed: ['b.ts'], + aborted: false + }) + expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts', 'c.ts']) + // Why: `aborted` is reserved for the pre-step (bulk unstage) failing — + // per-file failures don't trip it, otherwise callers couldn't + // distinguish "nothing ran" from "some ran, some didn't". + expect(ctx.errors).toEqual([error]) + }) + + it('does not invoke the error callback on a happy-path staged run', async () => { + const ctx = makeDeps() + await runDiscardAllForArea('staged', ['a.ts'], ctx.deps) + expect(ctx.onError).not.toHaveBeenCalled() + }) + + it('does not bulk-unstage for non-staged areas even if the dep is provided', async () => { + const ctx = makeDeps() + const areas: DiscardAllArea[] = ['unstaged', 'untracked'] + for (const area of areas) { + await runDiscardAllForArea(area, ['x.ts'], ctx.deps) + } + // Why: the unstage step is specific to the staged area's two-step + // reset. Accidentally invoking it for unstaged/untracked would be a + // no-op for unstaged entries but could mask a regression where staged + // entries leak into those paths. + expect(ctx.bulkUnstage).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/right-sidebar/discard-all-sequence.ts b/src/renderer/src/components/right-sidebar/discard-all-sequence.ts new file mode 100644 index 00000000000..9425344bca2 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/discard-all-sequence.ts @@ -0,0 +1,122 @@ +import type { GitStatusEntry } from '../../../../shared/types' + +export type DiscardAllArea = 'staged' | 'unstaged' | 'untracked' + +/** + * Collect the paths a "Discard all" bulk action should operate on for a given + * area. Unresolved and locally-resolved conflicts are excluded — discarding + * those can silently re-create the conflict or lose the resolution. + */ +export function getDiscardAllPaths( + entries: readonly GitStatusEntry[], + area: DiscardAllArea +): string[] { + return entries + .filter( + (entry) => + entry.area === area && + entry.conflictStatus !== 'unresolved' && + entry.conflictStatus !== 'resolved_locally' + ) + .map((entry) => entry.path) +} + +export type StageAllArea = 'unstaged' | 'untracked' + +/** + * Collect the paths a "Stage all" action should operate on. + * Unresolved conflict rows are excluded — `git add` on a conflicted file + * silently clears the `u` record before the user has reviewed it. + * `resolved_locally` rows are intentionally INCLUDED: staging them is how the + * user finalises the resolution and mirrors the per-row Stage button. + */ +export function getStageAllPaths(entries: readonly GitStatusEntry[], area: StageAllArea): string[] { + return entries + .filter((entry) => entry.area === area && entry.conflictStatus !== 'unresolved') + .map((entry) => entry.path) +} + +/** + * Collect the paths an "Unstage all" action should operate on. + * Every staged row is eligible — `git reset HEAD` on a staged conflict + * row is safe and mirrors the per-row Unstage action. + */ +export function getUnstageAllPaths(entries: readonly GitStatusEntry[]): string[] { + return entries.filter((entry) => entry.area === 'staged').map((entry) => entry.path) +} + +export type DiscardAllDeps = { + /** Unstage the given paths in one IPC round-trip. Only called for 'staged'. */ + bulkUnstage: (paths: string[]) => Promise + /** Discard a single path (restore working-tree to HEAD, or rm if untracked). */ + discardOne: (path: string) => Promise + /** + * Called when either the pre-step (bulkUnstage) rejects OR an individual + * `discardOne` rejects. Invoked once per failure so callers can surface + * each error (e.g. a toast per stuck file) rather than swallowing them. + */ + onError?: (error: unknown) => void +} + +export type DiscardAllResult = { + /** Paths whose `discardOne` call resolved successfully. */ + discarded: string[] + /** Paths whose `discardOne` call rejected. Best-effort: the loop continues past these. */ + failed: string[] + /** + * True only when the pre-step (bulk unstage for the 'staged' area) failed + * and we never entered the per-file discard loop. Per-file failures do + * NOT set this flag — they are reported via `failed`. + */ + aborted: boolean +} + +/** + * Run the "Discard all" sequence for a given area. + * + * For 'staged', this first bulk-unstages the paths — without that step, + * `discardOne` (which maps to `git restore --worktree --source=HEAD`) would + * reset the working tree to HEAD but leave the index carrying the staged + * delta, producing phantom inverse "Changes" rows the user thought they just + * discarded. If the unstage fails we MUST skip the discard loop entirely for + * the same reason: a stale index with a clean worktree is a worse state than + * the one the user started in. + * + * Per-file `discardOne` failures are best-effort: we continue past a failed + * file so a single stuck path does not block the rest of the bulk action. + * Failed paths are reported via `failed`, `onError` is invoked once per + * failure, and `aborted` remains `false` because the loop ran end-to-end. + */ +export async function runDiscardAllForArea( + area: DiscardAllArea, + paths: readonly string[], + deps: DiscardAllDeps +): Promise { + if (paths.length === 0) { + return { discarded: [], failed: [], aborted: false } + } + + if (area === 'staged') { + try { + await deps.bulkUnstage([...paths]) + } catch (error) { + deps.onError?.(error) + return { discarded: [], failed: [], aborted: true } + } + } + + const discarded: string[] = [] + const failed: string[] = [] + for (const path of paths) { + try { + await deps.discardOne(path) + discarded.push(path) + } catch (error) { + // Best-effort: record and continue so one stuck file doesn't block the + // rest of the bulk action. + failed.push(path) + deps.onError?.(error) + } + } + return { discarded, failed, aborted: false } +} From 85b125f6bf40cd035cf48f8b5d2200ec28fc191b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 4 May 2026 16:09:34 -0700 Subject: [PATCH 06/46] feat(opencode): map question.asked to waiting state (#1406) Co-authored-by: Orca --- src/main/agent-hooks/server.test.ts | 16 ++++++++++++++++ src/main/agent-hooks/server.ts | 10 ++++++---- src/main/opencode/hook-service.test.ts | 15 +++++++++++++++ src/main/opencode/hook-service.ts | 12 ++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/main/agent-hooks/server.test.ts b/src/main/agent-hooks/server.test.ts index d12c7375435..ba321dc1799 100644 --- a/src/main/agent-hooks/server.test.ts +++ b/src/main/agent-hooks/server.test.ts @@ -660,6 +660,22 @@ describe('OpenCode hook normalization', () => { expect(result?.payload.state).toBe('waiting') }) + it('AskUserQuestion maps to waiting', () => { + // Why: OpenCode emits `question.asked` when the agent uses an ask-the-user + // tool (distinct from `permission.asked`, which blocks on tool approval). + // Both leave the agent idle-but-waiting on a human, so both must render + // the same red "needs attention" indicator. Without this mapping the pane + // silently stays in `working` and the user has no visual cue that the + // agent is waiting on them. + const result = _internals.normalizeHookPayload( + 'opencode', + buildBody({ hook_event_name: 'AskUserQuestion' }), + 'production' + ) + expect(result?.payload.state).toBe('waiting') + expect(result?.payload.agentType).toBe('opencode') + }) + it('unknown event name returns null', () => { const result = _internals.normalizeHookPayload( 'opencode', diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index aee9c83e780..92db2823b2b 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -844,11 +844,13 @@ function normalizeCodexEvent( // Why: OpenCode has no declarative hook surface — it exposes in-process plugin // events (session.status busy/idle, session.idle, permission.asked, -// message.updated, message.part.updated). The bundled plugin (see -// opencode/hook-service) pre-maps those to our stable hook_event_name +// question.asked, message.updated, message.part.updated). The bundled plugin +// (see opencode/hook-service) pre-maps those to our stable hook_event_name // vocabulary before POSTing so this normalizer can share the same switch // shape as Claude/Codex/Gemini. SessionBusy = turn started, SessionIdle = -// turn finished, PermissionRequest = blocked on user approval, MessagePart = +// turn finished, PermissionRequest = blocked on user approval, AskUserQuestion = +// blocked on user reply to an ask-the-user tool (both map to `waiting` so the +// sidebar renders the red "needs attention" indicator), MessagePart = // incremental text from user prompt or assistant reply (stays in `working` // because streaming chunks must not flip the row to done mid-turn). function normalizeOpenCodeEvent( @@ -862,7 +864,7 @@ function normalizeOpenCodeEvent( ? 'working' : eventName === 'SessionIdle' ? 'done' - : eventName === 'PermissionRequest' + : eventName === 'PermissionRequest' || eventName === 'AskUserQuestion' ? 'waiting' : null diff --git a/src/main/opencode/hook-service.test.ts b/src/main/opencode/hook-service.test.ts index b1ed29622b5..21fb9229ab5 100644 --- a/src/main/opencode/hook-service.test.ts +++ b/src/main/opencode/hook-service.test.ts @@ -80,6 +80,21 @@ describe('OpenCode hook plugin source', () => { expect(source).toContain('cachedEndpointValues = null;') }) + it('forwards question.asked as AskUserQuestion so the pane flips to waiting', () => { + // Why: OpenCode exposes two separate plugin events for human-in-the-loop + // moments — `permission.asked` (blocks on tool approval) and + // `question.asked` (the agent called an ask-the-user tool). The plugin + // must forward both so the server-side normalizer can map each to + // `waiting` and render the red indicator. Dropping `question.asked` + // leaves the pane stuck in `working` while the agent is actually idle, + // waiting on a human reply — exactly the bug three other OpenCode + // integrations (cmux, t3code, open-vibe-island) all handle. + const source = _internals.getOpenCodePluginSource() + + expect(source).toContain('if (event.type === "question.asked")') + expect(source).toContain('await post("AskUserQuestion", event.properties || {});') + }) + it('guards endpoint-file parse warnings with a process-lifetime latch', () => { // Why: ENOENT is the normal pre-install case and must stay silent, but a // malformed/unreadable file (EACCES, EIO, parse error) would otherwise diff --git a/src/main/opencode/hook-service.ts b/src/main/opencode/hook-service.ts index 5990fa667ae..b789b2146e4 100644 --- a/src/main/opencode/hook-service.ts +++ b/src/main/opencode/hook-service.ts @@ -259,6 +259,18 @@ function getOpenCodePluginSource(): string { ' return;', ' }', '', + ' if (event.type === "question.asked") {', + ' // Why: question.asked fires when OpenCode uses an ask-the-user tool', + ' // (distinct from permission.asked, which blocks on tool approval).', + ' // The agent is idle-but-waiting on a human reply, not running, so we', + ' // must flip the pane to the same red "needs attention" state used for', + ' // permission requests. Like permission.asked, do not touch lastStatus', + ' // so the next SessionBusy/SessionIdle after the user answers still', + ' // fires and restores the normal working/done flow.', + ' await post("AskUserQuestion", event.properties || {});', + ' return;', + ' }', + '', ' if (event.type === "message.updated") {', ' // Why: role is already cached above the isChildSession await so the', ' // back-to-back message.part.updated for the same messageID is not', From 42ea9f33bace2748e2a821438ece7c0864110357 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 4 May 2026 17:10:03 -0700 Subject: [PATCH 07/46] fix(browser): default webview background to white (#1414) Co-authored-by: Orca --- src/renderer/src/components/browser-pane/BrowserPane.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index 8d151ce9190..a421e727375 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -933,7 +933,11 @@ function BrowserPagePane({ webview.style.width = '100%' webview.style.height = '100%' webview.style.border = 'none' - webview.style.background = 'transparent' + // Why: default to white so sites that don't set an html/body background + // (e.g. httpbin.org/html) don't show through to Orca's dark chrome. Real + // browsers paint the viewport white by default; sites that specify their + // own background (including dark ones) still override this. + webview.style.background = '#ffffff' webviewRegistry.set(browserTab.id, webview) container.appendChild(webview) needsInitialNavigation = true From 7489b9ddff5b08dbd02aab222f46765c7f76153a Mon Sep 17 00:00:00 2001 From: Kaylee <65376239+KayleeWilliams@users.noreply.github.com> Date: Mon, 4 May 2026 20:31:47 -0700 Subject: [PATCH 08/46] Preserve workspace name casing (#1417) --- .../src/components/sidebar/WorktreeList.tsx | 2 +- .../sidebar/worktree-list-groups.test.ts | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 4d90a4ac37f..0ef0a28160d 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -417,7 +417,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
-
+
{row.label}
diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts index 129be5f9205..9500cec1af6 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts @@ -1,3 +1,5 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' import { buildRows, getPRGroupKey } from './worktree-list-groups' import type { Repo, Worktree } from '../../../../shared/types' @@ -91,4 +93,22 @@ describe('buildRows with pinned worktrees', () => { const rows = buildRows('none', [pinned, allPinned], repoMap, null, new Set()) expect(rows.some((r) => r.type === 'header' && r.key === 'all')).toBe(false) }) + + it('preserves repo display casing in group labels', () => { + const lowercaseRepo = { ...repo, displayName: 'c15t' } + const rows = buildRows('repo', [worktree], new Map([[repo.id, lowercaseRepo]]), null, new Set()) + + expect(rows[0]).toMatchObject({ type: 'header', label: 'c15t' }) + }) +}) + +describe('WorktreeList header styles', () => { + it('does not title-case workspace group labels', () => { + const source = readFileSync( + fileURLToPath(new URL('./WorktreeList.tsx', import.meta.url)), + 'utf8' + ) + + expect(source).not.toContain('leading-none capitalize') + }) }) From 7db2cd1dd5f03abdf0b0ca9b9ed56ee04319b96c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 4 May 2026 20:42:03 -0700 Subject: [PATCH 09/46] Link CLAUDE.md to AGENTS.md Updated CLAUDE.md to reference AGENTS.md. --- CLAUDE.md | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 656f0c42715..43c994c2d36 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,24 +1 @@ -# CLAUDE.md - -See also: [AGENTS.md](./AGENTS.md) for agent-specific rules. - -## Code Comments: Document the "Why" - -When implementing behavior driven by a design doc, spec, or non-obvious constraint, **add a comment explaining why** the code does what it does — not just what it does. This is especially important for: - -- **Safety constraints** — e.g., suppressing an action because it could silently erase a signal, re-create a conflict, or mislead the user. -- **Fallback/error-handling choices** — e.g., defaulting to `'modified'` on fs error because it's the least misleading option. -- **Architectural boundaries** — e.g., why state lives in the renderer and never crosses IPC, or why a feature belongs to Source Control and not Checks. -- **Compatibility shims** — e.g., when a field exists purely for downstream plumbing and does not carry semantic meaning. -- **Intentional omissions** — e.g., skipping submodule conflicts or not providing rename metadata because the data source doesn't support it. - -A future maintainer who hasn't read the design doc should be able to understand from the comment alone why the code must not be changed casually. - -## Package Manager - -This project uses **pnpm**. Never use `npm` or `yarn`. - -- Use `pnpm install` (not `npm install`) -- Use `pnpm add ` (not `npm install `) -- Use `pnpm run