From 5c3ba7be91461c9eda8e88cd6a0a80556beaf5e5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 28 Jun 2026 11:38:23 -0700 Subject: [PATCH 01/12] Improve inactive workspace cleanup UX (#3600) Co-authored-by: Orca --- .github/workflows/pullfrog.yml | 4 +- src/main/git/git-runtime-options.ts | 9 +- src/main/git/runner.ts | 14 + src/main/git/status.ts | 4 + src/main/git/worktree.ts | 9 +- src/main/ipc/workspace-cleanup-candidate.ts | 213 +++++ .../ipc/workspace-cleanup-disconnected-ssh.ts | 92 ++ .../ipc/workspace-cleanup-git-evidence.ts | 119 +++ .../ipc/workspace-cleanup-scan-primitives.ts | 102 ++ src/main/ipc/workspace-cleanup-scan.ts | 259 ++++++ src/main/ipc/workspace-cleanup.test.ts | 137 ++- src/main/ipc/workspace-cleanup.ts | 600 +----------- src/main/providers/ssh-git-provider.ts | 7 +- src/main/providers/types.ts | 1 + src/main/repo-worktrees.ts | 1 + src/preload/api-types.ts | 6 +- src/preload/index.ts | 20 +- .../WorkspaceCleanupDialog.tsx | 872 +++++++++++++----- .../workspace-cleanup-filter-sort.ts | 251 +++++ ...workspace-cleanup-presentation-fixtures.ts | 107 +++ .../workspace-cleanup-presentation.test.ts | 228 +++++ .../workspace-cleanup-presentation.ts | 127 +++ .../store/slices/workspace-cleanup.test.ts | 371 +++++++- .../src/store/slices/workspace-cleanup.ts | 354 ++++++- src/shared/workspace-cleanup.ts | 8 + 25 files changed, 3033 insertions(+), 882 deletions(-) create mode 100644 src/main/ipc/workspace-cleanup-candidate.ts create mode 100644 src/main/ipc/workspace-cleanup-disconnected-ssh.ts create mode 100644 src/main/ipc/workspace-cleanup-git-evidence.ts create mode 100644 src/main/ipc/workspace-cleanup-scan-primitives.ts create mode 100644 src/main/ipc/workspace-cleanup-scan.ts create mode 100644 src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-sort.ts create mode 100644 src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation-fixtures.ts create mode 100644 src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.test.ts create mode 100644 src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.ts diff --git a/.github/workflows/pullfrog.yml b/.github/workflows/pullfrog.yml index 6516ae2e07d..19401170c0e 100644 --- a/.github/workflows/pullfrog.yml +++ b/.github/workflows/pullfrog.yml @@ -42,14 +42,14 @@ jobs: MOONSHOT_API_KEY: ${{ secrets.MOONSHOT_API_KEY }} OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }} - + # for Amazon Bedrock (https://docs.pullfrog.com/bedrock) # AWS_BEARER_TOKEN_BEDROCK: ${{ secrets.AWS_BEARER_TOKEN_BEDROCK }} # AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} # AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} # AWS_REGION: us-east-1 # BEDROCK_MODEL_ID: - + # for Google Vertex AI (https://docs.pullfrog.com/vertex) # VERTEX_SERVICE_ACCOUNT_JSON: ${{ secrets.VERTEX_SERVICE_ACCOUNT_JSON }} # GOOGLE_CLOUD_PROJECT: my-project diff --git a/src/main/git/git-runtime-options.ts b/src/main/git/git-runtime-options.ts index cf897a7b395..ca0155f07f6 100644 --- a/src/main/git/git-runtime-options.ts +++ b/src/main/git/git-runtime-options.ts @@ -1,10 +1,15 @@ export type GitRuntimeOptions = { wslDistro?: string + signal?: AbortSignal } export function gitOptionsForWorktree( cwd: string, options: GitRuntimeOptions = {} -): { cwd: string; wslDistro?: string } { - return options.wslDistro ? { cwd, wslDistro: options.wslDistro } : { cwd } +): { cwd: string; wslDistro?: string; signal?: AbortSignal } { + return { + cwd, + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), + ...(options.signal ? { signal: options.signal } : {}) + } } diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index beef7c5e6be..4f455618569 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -836,6 +836,7 @@ type GitStreamOptions = { cwd: string env?: NodeJS.ProcessEnv wslDistro?: string + signal?: AbortSignal /** Byte backstop; defaults to DEFAULT_GIT_MAX_BUFFER. */ maxBuffer?: number /** @@ -863,6 +864,10 @@ export async function gitStreamStdout( const maxBuffer = options.maxBuffer ?? DEFAULT_GIT_MAX_BUFFER return withGitSpan({ args, cwd: options.cwd }, async () => { return new Promise((resolve, reject) => { + if (options.signal?.aborted) { + reject(createAbortError()) + return + } const child = gitSpawn(args, { cwd: options.cwd, env: nonInteractiveGitEnv(options.env), @@ -887,6 +892,7 @@ export async function gitStreamStdout( child.stderr?.off('data', onStderrData) child.off('error', onError) child.off('close', onClose) + options.signal?.removeEventListener('abort', onAbort) // Flush any bytes the decoders were holding for an incomplete sequence. stdoutDecoder.end() stderrDecoder.end() @@ -953,11 +959,19 @@ export async function gitStreamStdout( } finish(new Error(`git exited with ${code}: ${stderr}`)) } + function onAbort(): void { + killSpawnedCommandTree(child) + finish(createAbortError()) + } child.stdout?.on('data', onStdoutData) child.stderr?.on('data', onStderrData) child.on('error', onError) child.on('close', onClose) + options.signal?.addEventListener('abort', onAbort, { once: true }) + if (options.signal?.aborted) { + onAbort() + } }) }) } diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 63917212baa..4902a23e59e 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -109,6 +109,9 @@ export async function getStatus( options: GetStatusOptions = {} ): Promise { gitDiffReadDedupe.clear() + if (options.signal) { + return runGetStatus(worktreePath, options) + } // Why: dedupe only concurrent identical reads; after settle, callers must // execute a fresh status read rather than observing a cached result. const cacheKey = getStatusReadKey(worktreePath, options) @@ -190,6 +193,7 @@ async function runGetStatus( // Why: status polling is read-like; avoid refreshing the index and racing // terminal Git commands on `.git/worktrees/*/index.lock`. env: gitOptionalLocksDisabledEnv(), + signal: options.signal, onStdout: (chunk) => parser.update(chunk, limit) }) if (!stoppedEarly) { diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index af1546396c0..f27ae1df9a7 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -29,6 +29,7 @@ type SparseWorktreeCreateError = Error & { export type GitWorktreeExecOptions = { wslDistro?: string + signal?: AbortSignal } export type AddWorktreeOptions = GitWorktreeExecOptions & { @@ -69,8 +70,12 @@ const SPARSE_CHECKOUT_DETECTION_CONCURRENCY = 8 function gitExecOptions( cwd: string, options: GitWorktreeExecOptions = {} -): { cwd: string; wslDistro?: string } { - return options.wslDistro ? { cwd, wslDistro: options.wslDistro } : { cwd } +): { cwd: string; wslDistro?: string; signal?: AbortSignal } { + return { + cwd, + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), + ...(options.signal ? { signal: options.signal } : {}) + } } function getErrorCode(error: unknown): string | undefined { diff --git a/src/main/ipc/workspace-cleanup-candidate.ts b/src/main/ipc/workspace-cleanup-candidate.ts new file mode 100644 index 00000000000..e25af90964c --- /dev/null +++ b/src/main/ipc/workspace-cleanup-candidate.ts @@ -0,0 +1,213 @@ +import type { IGitProvider } from '../providers/types' +import { isFolderRepo } from '../../shared/repo-kind' +import type { Repo, Worktree } from '../../shared/types' +import { + applyWorkspaceCleanupPolicy, + createWorkspaceCleanupFingerprint, + getWorkspaceCleanupInactivityReasons, + isWorkspaceOldForCleanup, + type WorkspaceCleanupBlocker, + type WorkspaceCleanupCandidate, + type WorkspaceCleanupReason +} from '../../shared/workspace-cleanup' +import { + createEmptyWorkspaceCleanupGitEvidence, + readWorkspaceCleanupGitEvidence +} from './workspace-cleanup-git-evidence' +import { appendWorkspaceCleanupItems } from './workspace-cleanup-scan-primitives' + +export async function buildWorkspaceCleanupCandidate(args: { + repo: Repo + worktree: Worktree + scannedAt: number + provider: IGitProvider | null + skipGit: boolean + forceGitCheck: boolean +}): Promise { + const { repo, worktree, scannedAt, provider, skipGit, forceGitCheck } = args + const blockers: WorkspaceCleanupBlocker[] = [] + const reasons = getWorkspaceCleanupInactivityReasonsForWorkspace(worktree, scannedAt) + const repoIsFolder = isFolderRepo(repo) + + if (worktree.isMainWorktree) { + blockers.push('main-worktree') + } + if (repoIsFolder) { + blockers.push('folder-repo') + } + if (worktree.isPinned) { + blockers.push('pinned') + } + + const localContext = buildWorkspaceCleanupLocalContext(worktree) + const shouldReadGit = shouldReadWorkspaceCleanupGitEvidence({ + repoIsFolder, + blockers, + worktree, + skipGit, + forceGitCheck + }) + + const gitEvidence = !shouldReadGit + ? createEmptyWorkspaceCleanupGitEvidence() + : await readWorkspaceCleanupGitEvidence(worktree, repo, provider) + appendWorkspaceCleanupItems(blockers, gitEvidence.blockers) + + const candidateWithoutFingerprint: WorkspaceCleanupCandidate = { + worktreeId: worktree.id, + repoId: repo.id, + repoName: repo.displayName, + connectionId: repo.connectionId ?? null, + displayName: worktree.displayName, + branch: shortWorkspaceCleanupBranchName(worktree.branch), + path: worktree.path, + tier: 'review', + selectedByDefault: false, + reasons, + blockers: uniqueWorkspaceCleanupBlockers(blockers), + lastActivityAt: worktree.lastActivityAt, + ...(worktree.createdAt !== undefined ? { createdAt: worktree.createdAt } : {}), + localContext, + git: { + clean: gitEvidence.clean, + upstreamAhead: gitEvidence.upstreamAhead, + upstreamBehind: gitEvidence.upstreamBehind, + checkedAt: gitEvidence.checkedAt + }, + fingerprint: '' + } + + const fingerprint = createWorkspaceCleanupFingerprint({ + branch: candidateWithoutFingerprint.branch, + head: worktree.head, + gitClean: gitEvidence.clean, + lastActivityAt: worktree.lastActivityAt + }) + + return applyWorkspaceCleanupPolicy({ + ...candidateWithoutFingerprint, + reasons: uniqueWorkspaceCleanupReasons(reasons), + blockers: uniqueWorkspaceCleanupBlockers(blockers), + fingerprint + }) +} + +export function buildWorkspaceCleanupCandidateFromError( + repo: Repo, + worktree: Worktree, + scannedAt: number +): WorkspaceCleanupCandidate { + return applyWorkspaceCleanupPolicy({ + worktreeId: worktree.id, + repoId: repo.id, + repoName: repo.displayName, + connectionId: repo.connectionId ?? null, + displayName: worktree.displayName, + branch: shortWorkspaceCleanupBranchName(worktree.branch), + path: worktree.path, + tier: 'protected', + selectedByDefault: false, + reasons: getWorkspaceCleanupInactivityReasonsForWorkspace(worktree, scannedAt), + blockers: ['git-status-error'], + lastActivityAt: worktree.lastActivityAt, + ...(worktree.createdAt !== undefined ? { createdAt: worktree.createdAt } : {}), + localContext: buildWorkspaceCleanupLocalContext(worktree), + git: { + clean: null, + upstreamAhead: null, + upstreamBehind: null, + checkedAt: scannedAt + }, + fingerprint: createWorkspaceCleanupFingerprint({ + branch: shortWorkspaceCleanupBranchName(worktree.branch), + head: worktree.head, + gitClean: null, + lastActivityAt: worktree.lastActivityAt + }) + }) +} + +export function buildWorkspaceCleanupLocalContext( + worktree: Pick +): WorkspaceCleanupCandidate['localContext'] { + return { + terminalTabCount: 0, + cleanEditorTabCount: 0, + browserTabCount: 0, + diffCommentCount: worktree.diffComments?.length ?? 0, + newestDiffCommentAt: getNewestWorkspaceCleanupDiffCommentAt(worktree.diffComments), + retainedDoneAgentCount: 0 + } +} + +export function getNewestWorkspaceCleanupDiffCommentAt( + diffComments: Worktree['diffComments'] | undefined +): number | null { + if (!diffComments || diffComments.length === 0) { + return null + } + // Why: persisted diff notes can grow large enough for spread-based Math.max + // to exceed the JavaScript argument limit during cleanup scans. + let newest = diffComments[0]?.createdAt ?? null + for (let index = 1; index < diffComments.length; index += 1) { + const createdAt = diffComments[index]?.createdAt + if (createdAt !== undefined && (newest === null || createdAt > newest)) { + newest = createdAt + } + } + return newest +} + +export function isWorkspaceInactiveForCleanup( + workspace: Pick, + scannedAt: number +): boolean { + return isWorkspaceOldForCleanup(workspace, scannedAt) +} + +export function getWorkspaceCleanupInactivityReasonsForWorkspace( + workspace: Pick, + scannedAt: number +): WorkspaceCleanupReason[] { + return getWorkspaceCleanupInactivityReasons(workspace, scannedAt) +} + +export function shortWorkspaceCleanupBranchName(branch: string): string { + return branch.replace(/^refs\/heads\//, '') || 'HEAD' +} + +function shouldReadWorkspaceCleanupGitEvidence(args: { + repoIsFolder: boolean + blockers: WorkspaceCleanupBlocker[] + worktree: Worktree + skipGit: boolean + forceGitCheck: boolean +}): boolean { + const { repoIsFolder, blockers, worktree, skipGit, forceGitCheck } = args + if ((skipGit && !forceGitCheck) || repoIsFolder || worktree.isMainWorktree) { + return false + } + if ( + blockers.includes('pinned') || + blockers.includes('main-worktree') || + blockers.includes('folder-repo') + ) { + return false + } + + // Why: inactivity is the only recommendation signal now. Git is read only + // to keep the destructive path from deleting dirty or local-only branch work. + return true +} + +function uniqueWorkspaceCleanupBlockers( + blockers: WorkspaceCleanupBlocker[] +): WorkspaceCleanupBlocker[] { + return [...new Set(blockers)] +} + +function uniqueWorkspaceCleanupReasons( + reasons: WorkspaceCleanupReason[] +): WorkspaceCleanupReason[] { + return [...new Set(reasons)] +} diff --git a/src/main/ipc/workspace-cleanup-disconnected-ssh.ts b/src/main/ipc/workspace-cleanup-disconnected-ssh.ts new file mode 100644 index 00000000000..bd549a9b6ee --- /dev/null +++ b/src/main/ipc/workspace-cleanup-disconnected-ssh.ts @@ -0,0 +1,92 @@ +import { basename } from 'node:path' +import type { Store } from '../persistence' +import type { Repo, WorktreeMeta } from '../../shared/types' +import { + applyWorkspaceCleanupPolicy, + createWorkspaceCleanupFingerprint, + type WorkspaceCleanupCandidate +} from '../../shared/workspace-cleanup' +import { splitWorktreeId } from '../../shared/worktree-id' +import { + getNewestWorkspaceCleanupDiffCommentAt, + getWorkspaceCleanupInactivityReasonsForWorkspace, + isWorkspaceInactiveForCleanup +} from './workspace-cleanup-candidate' + +export function synthesizeDisconnectedSshCleanupCandidates( + store: Store, + repo: Repo, + scannedAt: number, + targetWorktreeId?: string +): WorkspaceCleanupCandidate[] { + const repoWorktreePrefix = `${repo.id}::` + if (targetWorktreeId) { + if (!targetWorktreeId.startsWith(repoWorktreePrefix)) { + return [] + } + // Why: focused delete preflight names one workspace already; walking all + // persisted metadata is unnecessary for disconnected SSH repos. + const meta = store.getWorktreeMeta(targetWorktreeId) + return meta ? [createDisconnectedSshCandidate(repo, scannedAt, targetWorktreeId, meta)] : [] + } + + const candidates: WorkspaceCleanupCandidate[] = [] + const allMeta = store.getAllWorktreeMeta() + for (const worktreeId in allMeta) { + if (!Object.hasOwn(allMeta, worktreeId) || !worktreeId.startsWith(repoWorktreePrefix)) { + continue + } + const meta = allMeta[worktreeId] + if (!meta || !isWorkspaceInactiveForCleanup(meta, scannedAt)) { + continue + } + candidates.push(createDisconnectedSshCandidate(repo, scannedAt, worktreeId, meta)) + } + return candidates +} + +function createDisconnectedSshCandidate( + repo: Repo, + scannedAt: number, + worktreeId: string, + meta: WorktreeMeta +): WorkspaceCleanupCandidate { + const parsed = splitWorktreeId(worktreeId) + const path = parsed?.worktreePath ?? worktreeId + const reasons = getWorkspaceCleanupInactivityReasonsForWorkspace(meta, scannedAt) + return applyWorkspaceCleanupPolicy({ + worktreeId, + repoId: repo.id, + repoName: repo.displayName, + connectionId: repo.connectionId ?? null, + displayName: meta.displayName || basename(path), + branch: basename(path), + path, + tier: 'protected', + selectedByDefault: false, + reasons, + blockers: ['ssh-disconnected'], + lastActivityAt: meta.lastActivityAt, + ...(meta.createdAt !== undefined ? { createdAt: meta.createdAt } : {}), + localContext: { + terminalTabCount: 0, + cleanEditorTabCount: 0, + browserTabCount: 0, + diffCommentCount: meta.diffComments?.length ?? 0, + newestDiffCommentAt: getNewestWorkspaceCleanupDiffCommentAt(meta.diffComments), + retainedDoneAgentCount: 0 + }, + git: { + clean: null, + upstreamAhead: null, + upstreamBehind: null, + checkedAt: null + }, + fingerprint: createWorkspaceCleanupFingerprint({ + branch: basename(path), + head: '', + gitClean: null, + lastActivityAt: meta.lastActivityAt + }) + }) +} diff --git a/src/main/ipc/workspace-cleanup-git-evidence.ts b/src/main/ipc/workspace-cleanup-git-evidence.ts new file mode 100644 index 00000000000..90e6058f598 --- /dev/null +++ b/src/main/ipc/workspace-cleanup-git-evidence.ts @@ -0,0 +1,119 @@ +import { getStatus } from '../git/status' +import { gitExecFileAsync } from '../git/runner' +import type { IGitProvider } from '../providers/types' +import type { GitStatusResult, Repo, Worktree } from '../../shared/types' +import type { WorkspaceCleanupBlocker } from '../../shared/workspace-cleanup' +import { + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + withWorkspaceCleanupTimeout +} from './workspace-cleanup-scan-primitives' + +export type WorkspaceCleanupGitEvidence = { + clean: boolean | null + upstreamAhead: number | null + upstreamBehind: number | null + checkedAt: number | null + blockers: WorkspaceCleanupBlocker[] +} + +export function createEmptyWorkspaceCleanupGitEvidence(): WorkspaceCleanupGitEvidence { + return { + clean: null, + upstreamAhead: null, + upstreamBehind: null, + checkedAt: null, + blockers: [] + } +} + +export async function readWorkspaceCleanupGitEvidence( + worktree: Worktree, + repo: Repo, + provider: IGitProvider | null +): Promise { + const blockers: WorkspaceCleanupBlocker[] = [] + let status: GitStatusResult + const checkedAt = Date.now() + + try { + status = await withWorkspaceCleanupTimeout( + (signal) => + repo.connectionId + ? provider!.getStatus(worktree.path, { signal }) + : getStatus(worktree.path, { signal }), + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + 'Timed out reading git status.' + ) + } catch { + return { + ...createEmptyWorkspaceCleanupGitEvidence(), + blockers: ['git-status-error'] + } + } + + if (status.upstreamStatus === undefined) { + return { + ...createEmptyWorkspaceCleanupGitEvidence(), + blockers: ['git-status-error'] + } + } + + const clean = status.entries.length === 0 + if (!clean) { + blockers.push('dirty-files') + } + + const upstreamAhead = status.upstreamStatus.hasUpstream ? status.upstreamStatus.ahead : null + const upstreamBehind = status.upstreamStatus.hasUpstream ? status.upstreamStatus.behind : null + if (upstreamAhead !== null && upstreamAhead > 0) { + blockers.push('unpushed-commits') + } + if (clean && upstreamAhead === null) { + const unpushedCommitCount = await readUnpushedCommitCount(worktree, repo, provider) + if (unpushedCommitCount === null) { + blockers.push('unknown-base') + } else if (unpushedCommitCount > 0) { + blockers.push('unpushed-commits') + } + } + + return { + clean, + upstreamAhead, + upstreamBehind, + checkedAt, + blockers: uniqueWorkspaceCleanupGitBlockers(blockers) + } +} + +async function readUnpushedCommitCount( + worktree: Worktree, + repo: Repo, + provider: IGitProvider | null +): Promise { + try { + const result = await withWorkspaceCleanupTimeout( + (signal) => + repo.connectionId + ? provider!.exec(['rev-list', '--count', 'HEAD', '--not', '--remotes'], worktree.path, { + signal + }) + : gitExecFileAsync(['rev-list', '--count', 'HEAD', '--not', '--remotes'], { + cwd: worktree.path, + signal + }), + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + 'Timed out checking unpushed commits.' + ) + const count = Number.parseInt(result.stdout.trim(), 10) + return Number.isFinite(count) ? count : null + } catch { + return null + } +} + +function uniqueWorkspaceCleanupGitBlockers( + blockers: WorkspaceCleanupBlocker[] +): WorkspaceCleanupBlocker[] { + return [...new Set(blockers)] +} diff --git a/src/main/ipc/workspace-cleanup-scan-primitives.ts b/src/main/ipc/workspace-cleanup-scan-primitives.ts new file mode 100644 index 00000000000..4f0a84c8a86 --- /dev/null +++ b/src/main/ipc/workspace-cleanup-scan-primitives.ts @@ -0,0 +1,102 @@ +import { basename } from 'node:path' +import type { Repo } from '../../shared/types' +import type { WorkspaceCleanupScanError } from '../../shared/workspace-cleanup' + +export const WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS = 8_000 + +export function appendWorkspaceCleanupItems(target: T[], entries: readonly T[]): void { + // Why: cleanup can aggregate generated-size worktree batches; spreading + // those batches into push can exceed JavaScript's argument limit. + for (const entry of entries) { + target.push(entry) + } +} + +export async function mapWorkspaceCleanupWithConcurrency( + items: readonly T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const results: R[] = [] + let nextIndex = 0 + const workerCount = Math.min(limit, items.length) + await Promise.all( + Array.from({ length: workerCount }, async () => { + while (nextIndex < items.length) { + const index = nextIndex + nextIndex += 1 + results[index] = await fn(items[index]) + } + }) + ) + return results +} + +export async function withWorkspaceCleanupTimeout( + run: (signal: AbortSignal) => Promise, + timeoutMs: number, + message: string +): Promise { + const controller = new AbortController() + let timeoutId: NodeJS.Timeout | undefined + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + controller.abort() + reject(new Error(message)) + }, timeoutMs) + }) + try { + return await Promise.race([run(controller.signal), timeoutPromise]) + } finally { + if (timeoutId) { + clearTimeout(timeoutId) + } + } +} + +export function toWorkspaceCleanupErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +export function createWorkspaceCleanupScanError( + repo: Repo, + message: string +): WorkspaceCleanupScanError { + return { + repoId: repo.id, + repoName: repo.displayName || basename(repo.path), + message + } +} + +// Why: git errors often include absolute paths or command output. Keep the +// cause useful without leaking raw local/remote filesystem details to the UI. +export function toSafeWorkspaceCleanupRepoScanError(error: unknown): string { + const message = toWorkspaceCleanupErrorMessage(error) + if (message === 'Timed out listing SSH worktrees.') { + return 'Timed out listing remote worktrees.' + } + if (message === 'Timed out listing worktrees.') { + return 'Timed out listing worktrees.' + } + if (message.startsWith('Timed out ')) { + return message.replace(/\.$/, '') + } + + const lower = message.toLowerCase() + if (lower.includes('not a git repository') || lower.includes('not a git worktree')) { + return 'Repository is not a git checkout.' + } + if ( + lower.includes('enoent') || + lower.includes('no such file') || + lower.includes('cannot find') || + lower.includes('does not exist') + ) { + return 'Repository folder was not found.' + } + if (lower.includes('eacces') || lower.includes('eperm') || lower.includes('permission denied')) { + return 'Repository folder is not accessible.' + } + return 'Git could not list worktrees.' +} diff --git a/src/main/ipc/workspace-cleanup-scan.ts b/src/main/ipc/workspace-cleanup-scan.ts new file mode 100644 index 00000000000..33866e6cd73 --- /dev/null +++ b/src/main/ipc/workspace-cleanup-scan.ts @@ -0,0 +1,259 @@ +import type { Store } from '../persistence' +import { listRepoWorktrees, createFolderWorktree } from '../repo-worktrees' +import { getSshGitProvider } from '../providers/ssh-git-dispatch' +import type { IGitProvider } from '../providers/types' +import { isFolderRepo } from '../../shared/repo-kind' +import type { GitWorktreeInfo, Repo } from '../../shared/types' +import { mergeWorktree } from './worktree-logic' +import { splitWorktreeId } from '../../shared/worktree-id' +import type { + WorkspaceCleanupCandidate, + WorkspaceCleanupScanError, + WorkspaceCleanupScanArgs, + WorkspaceCleanupScanProgress, + WorkspaceCleanupScanResult +} from '../../shared/workspace-cleanup' +import { + buildWorkspaceCleanupCandidate, + buildWorkspaceCleanupCandidateFromError, + isWorkspaceInactiveForCleanup +} from './workspace-cleanup-candidate' +import { synthesizeDisconnectedSshCleanupCandidates } from './workspace-cleanup-disconnected-ssh' +import { + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + appendWorkspaceCleanupItems, + createWorkspaceCleanupScanError, + mapWorkspaceCleanupWithConcurrency, + toSafeWorkspaceCleanupRepoScanError, + withWorkspaceCleanupTimeout +} from './workspace-cleanup-scan-primitives' + +const WORKTREE_SCAN_CONCURRENCY = 3 + +type WorkspaceCleanupScanOptions = { + onProgress?: (progress: WorkspaceCleanupScanProgress) => void +} + +type WorkspaceCleanupScanRepoProgress = { + onWorktreesDiscovered?: (count: number) => void + onCandidateScanned?: (candidate: WorkspaceCleanupCandidate) => void + onErrors?: (errors: WorkspaceCleanupScanError[]) => void +} + +type WorkspaceCleanupProgressEmitter = { + addDiscovered: (count: number) => void + addCandidate: (candidate: WorkspaceCleanupCandidate) => void + addErrors: (errors: WorkspaceCleanupScanError[]) => void +} + +export async function scanWorkspaceCleanup( + store: Store, + args: WorkspaceCleanupScanArgs = {}, + options: WorkspaceCleanupScanOptions = {} +): Promise { + const scannedAt = Date.now() + const parsedTarget = args.worktreeId ? splitWorktreeId(args.worktreeId) : null + if (args.worktreeId && !parsedTarget) { + return { scannedAt, candidates: [], errors: [] } + } + const repos = parsedTarget + ? store.getRepos().filter((repo) => repo.id === parsedTarget.repoId) + : store.getRepos() + const progress = createProgressEmitter(args.scanId, scannedAt, options) + const errors: WorkspaceCleanupScanResult['errors'] = [] + const candidates: WorkspaceCleanupCandidate[] = [] + + for (const repo of repos) { + const result = await scanRepoWorkspaces({ + store, + repo, + scannedAt, + targetWorktreeId: args.worktreeId, + skipGitWorktreeIds: new Set(args.skipGitWorktreeIds ?? []), + onWorktreesDiscovered: progress.addDiscovered, + onCandidateScanned: progress.addCandidate, + onErrors: progress.addErrors + }) + appendWorkspaceCleanupItems(candidates, result.candidates) + appendWorkspaceCleanupItems(errors, result.errors) + } + + return { scannedAt, candidates, errors } +} + +async function scanRepoWorkspaces( + args: { + store: Store + repo: Repo + scannedAt: number + targetWorktreeId?: string + skipGitWorktreeIds: Set + } & WorkspaceCleanupScanRepoProgress +): Promise { + const { + store, + repo, + scannedAt, + targetWorktreeId, + skipGitWorktreeIds, + onWorktreesDiscovered, + onCandidateScanned, + onErrors + } = args + const errors: WorkspaceCleanupScanResult['errors'] = [] + const repoIsFolder = isFolderRepo(repo) + let provider: IGitProvider | null = null + let gitWorktrees: GitWorktreeInfo[] = [] + + try { + const discovered = await listCleanupGitWorktrees(repo, repoIsFolder) + provider = discovered.provider + gitWorktrees = discovered.gitWorktrees + } catch (error) { + return handleRepoWorktreeListError({ repo, targetWorktreeId, scannedAt, error, onErrors }) + } + + if (repo.connectionId && !provider) { + const candidates = targetWorktreeId + ? synthesizeDisconnectedSshCleanupCandidates(store, repo, scannedAt, targetWorktreeId) + : [] + onWorktreesDiscovered?.(candidates.length) + for (const candidate of candidates) { + onCandidateScanned?.(candidate) + } + return { scannedAt, candidates, errors: [] } + } + + const worktrees = gitWorktrees + .map((gitWorktree) => { + const worktreeId = `${repo.id}::${gitWorktree.path}` + const meta = store.getWorktreeMeta(worktreeId) + return mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) + }) + .filter((worktree) => { + if (targetWorktreeId) { + return worktree.id === targetWorktreeId + } + return ( + !repoIsFolder && + !worktree.isMainWorktree && + isWorkspaceInactiveForCleanup(worktree, scannedAt) + ) + }) + + onWorktreesDiscovered?.(worktrees.length) + + const candidates = await mapWorkspaceCleanupWithConcurrency( + worktrees, + WORKTREE_SCAN_CONCURRENCY, + async (worktree) => { + const candidate = await buildWorkspaceCleanupCandidate({ + repo, + worktree, + scannedAt, + provider, + skipGit: skipGitWorktreeIds.has(worktree.id), + forceGitCheck: Boolean(targetWorktreeId) + }).catch((error) => { + console.error('Workspace cleanup candidate scan failed', error) + return buildWorkspaceCleanupCandidateFromError(repo, worktree, scannedAt) + }) + onCandidateScanned?.(candidate) + return candidate + } + ) + + return { scannedAt, candidates, errors } +} + +async function listCleanupGitWorktrees( + repo: Repo, + repoIsFolder: boolean +): Promise<{ provider: IGitProvider | null; gitWorktrees: GitWorktreeInfo[] }> { + if (repoIsFolder) { + return { provider: null, gitWorktrees: [createFolderWorktree(repo)] } + } + if (repo.connectionId) { + const provider = getSshGitProvider(repo.connectionId) ?? null + if (!provider) { + // Why: cleanup should reflect only workspaces Orca can currently inspect. + return { provider: null, gitWorktrees: [] } + } + return { + provider, + gitWorktrees: await withWorkspaceCleanupTimeout( + (signal) => provider.listWorktrees(repo.path, { signal }), + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + 'Timed out listing SSH worktrees.' + ) + } + } + return { + provider: null, + gitWorktrees: await withWorkspaceCleanupTimeout( + (signal) => listRepoWorktrees(repo, { signal }), + WORKSPACE_CLEANUP_GIT_READ_TIMEOUT_MS, + 'Timed out listing worktrees.' + ) + } +} + +function handleRepoWorktreeListError(args: { + repo: Repo + targetWorktreeId?: string + scannedAt: number + error: unknown + onErrors?: (errors: WorkspaceCleanupScanError[]) => void +}): WorkspaceCleanupScanResult { + const { repo, targetWorktreeId, scannedAt, error, onErrors } = args + console.error('Workspace cleanup repo scan failed', error) + if (repo.connectionId && !targetWorktreeId) { + // Why: broad cleanup only shows remote workspaces Orca can inspect now. + // A connected SSH repo that fails mid-scan is omitted, not bannered. + return { scannedAt, candidates: [], errors: [] } + } + const errors = [createWorkspaceCleanupScanError(repo, toSafeWorkspaceCleanupRepoScanError(error))] + onErrors?.(errors) + return { scannedAt, candidates: [], errors } +} + +function createProgressEmitter( + scanId: string | undefined, + scannedAt: number, + options: WorkspaceCleanupScanOptions +): WorkspaceCleanupProgressEmitter { + const errors: WorkspaceCleanupScanError[] = [] + let totalWorktreeCount = 0 + let scannedWorktreeCount = 0 + const emit = ( + candidates: WorkspaceCleanupCandidate[], + candidateMode: WorkspaceCleanupScanProgress['candidateMode'] = 'snapshot' + ): void => { + if (!scanId) { + return + } + options.onProgress?.({ + scanId, + scannedAt, + totalWorktreeCount, + scannedWorktreeCount, + candidates, + errors: [...errors], + candidateMode + }) + } + return { + addDiscovered: (count) => { + totalWorktreeCount += count + emit([], 'append') + }, + addCandidate: (candidate) => { + scannedWorktreeCount += 1 + emit([candidate], 'append') + }, + addErrors: (newErrors) => { + appendWorkspaceCleanupItems(errors, newErrors) + emit([], 'append') + } + } +} diff --git a/src/main/ipc/workspace-cleanup.test.ts b/src/main/ipc/workspace-cleanup.test.ts index 0694ac5e9a5..6fe5320b1b1 100644 --- a/src/main/ipc/workspace-cleanup.test.ts +++ b/src/main/ipc/workspace-cleanup.test.ts @@ -10,6 +10,7 @@ import type { Repo, WorktreeMeta } from '../../shared/types' +import type { WorkspaceCleanupScanProgress } from '../../shared/workspace-cleanup' const { listRepoWorktreesMock, @@ -171,6 +172,58 @@ describe('workspace cleanup scan', () => { }) }) + it('reports cleanup scan progress as inactive workspaces are checked', async () => { + listRepoWorktreesMock.mockResolvedValue([ + { + path: '/repo-feature-a', + head: 'abc123', + branch: 'refs/heads/feature-a', + isBare: false, + isMainWorktree: false + }, + { + path: '/repo-feature-b', + head: 'def456', + branch: 'refs/heads/feature-b', + isBare: false, + isMainWorktree: false + } + ]) + const progress: unknown[] = [] + + const result = await scanWorkspaceCleanup( + makeStore(), + { scanId: 'scan-1' }, + { onProgress: (event) => progress.push(event) } + ) + + expect(result.candidates).toHaveLength(2) + expect(progress[0]).toMatchObject({ + scanId: 'scan-1', + scannedWorktreeCount: 0, + totalWorktreeCount: 2, + candidates: [] + }) + const candidateProgress = progress.filter( + (event): event is WorkspaceCleanupScanProgress => + (event as WorkspaceCleanupScanProgress).candidateMode === 'append' && + (event as WorkspaceCleanupScanProgress).candidates.length > 0 + ) + expect(candidateProgress).toHaveLength(2) + expect(candidateProgress.every((event) => event.candidates.length === 1)).toBe(true) + const progressWorktreeIds = candidateProgress.flatMap((event) => + event.candidates.map((candidate) => candidate.worktreeId) + ) + expect(progressWorktreeIds).toEqual( + expect.arrayContaining(['repo-1::/repo-feature-a', 'repo-1::/repo-feature-b']) + ) + expect(progress.at(-1)).toMatchObject({ + scanId: 'scan-1', + scannedWorktreeCount: 2, + totalWorktreeCount: 2 + }) + }) + it('keeps raw scan errors out of renderer-facing results', async () => { listRepoWorktreesMock.mockRejectedValue(new Error('fatal: path /Users/alice/private failed')) @@ -185,6 +238,30 @@ describe('workspace cleanup scan', () => { ]) }) + it('aborts a hung worktree list when the cleanup timeout fires', async () => { + let signal: AbortSignal | undefined + listRepoWorktreesMock.mockImplementation((_repo: Repo, options?: { signal?: AbortSignal }) => { + signal = options?.signal + return new Promise(() => {}) + }) + + const scan = scanWorkspaceCleanup(makeStore()) + await vi.advanceTimersByTimeAsync(8_000) + + await expect(scan).resolves.toEqual({ + scannedAt: NOW, + candidates: [], + errors: [ + { + repoId: 'repo-1', + repoName: 'Repo', + message: 'Timed out listing worktrees.' + } + ] + }) + expect(signal?.aborted).toBe(true) + }) + it('skips disconnected remote workspaces without a scan warning', async () => { const result = await scanWorkspaceCleanup( makeStore({ @@ -263,8 +340,12 @@ describe('workspace cleanup scan', () => { }) ) - expect(provider.listWorktrees).toHaveBeenCalledWith('/repo') - expect(provider.getStatus).toHaveBeenCalledWith('/remote/repo-feature') + expect(provider.listWorktrees).toHaveBeenCalledWith('/repo', { + signal: expect.any(AbortSignal) + }) + expect(provider.getStatus).toHaveBeenCalledWith('/remote/repo-feature', { + signal: expect.any(AbortSignal) + }) expect(result.errors).toEqual([]) expect(result.candidates[0]).toMatchObject({ connectionId: 'ssh-1', @@ -287,7 +368,9 @@ describe('workspace cleanup scan', () => { }) ) - expect(provider.listWorktrees).toHaveBeenCalledWith('/repo') + expect(provider.listWorktrees).toHaveBeenCalledWith('/repo', { + signal: expect.any(AbortSignal) + }) expect(result.errors).toEqual([]) expect(result.candidates).toEqual([]) }) @@ -324,6 +407,52 @@ describe('workspace cleanup scan', () => { }) }) + it('only inspects the target repo during focused scans', async () => { + const repoTwo = { ...REPO, id: 'repo-2', path: '/repo-two', displayName: 'Repo Two' } + listRepoWorktreesMock.mockImplementation((repo: Repo) => + Promise.resolve([ + { + path: `${repo.path}-feature`, + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + ) + + const result = await scanWorkspaceCleanup( + makeStore({ + repos: [REPO, repoTwo] + }), + { worktreeId: 'repo-2::/repo-two-feature' } + ) + + expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1) + expect(listRepoWorktreesMock).toHaveBeenCalledWith(repoTwo, { + signal: expect.any(AbortSignal) + }) + expect(result.candidates).toHaveLength(1) + expect(result.candidates[0]).toMatchObject({ + worktreeId: 'repo-2::/repo-two-feature', + repoId: 'repo-2' + }) + }) + + it('returns no focused scan rows when the encoded repo id is unknown', async () => { + const result = await scanWorkspaceCleanup(makeStore(), { + worktreeId: 'missing-repo::/repo-feature' + }) + + expect(listRepoWorktreesMock).not.toHaveBeenCalled() + expect(getStatusMock).not.toHaveBeenCalled() + expect(result).toEqual({ + scannedAt: NOW, + candidates: [], + errors: [] + }) + }) + it('honors renderer git deferrals without hiding the workspace', async () => { const result = await scanWorkspaceCleanup(makeStore(), { skipGitWorktreeIds: ['repo-1::/repo-feature'] @@ -353,7 +482,7 @@ describe('workspace cleanup scan', () => { expect(getStatusMock).toHaveBeenCalledTimes(1) expect(gitExecFileAsyncMock).toHaveBeenCalledWith( ['rev-list', '--count', 'HEAD', '--not', '--remotes'], - { cwd: '/repo-feature' } + { cwd: '/repo-feature', signal: expect.any(AbortSignal) } ) expect(result.candidates[0]).toMatchObject({ tier: 'ready', diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index 7a6bdbf2b66..229f4c4177f 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -1,52 +1,20 @@ -/* eslint-disable max-lines */ import { ipcMain } from 'electron' -import { basename } from 'node:path' import type { Store } from '../persistence' -import { getStatus } from '../git/status' -import { gitExecFileAsync } from '../git/runner' -import { listRepoWorktrees, createFolderWorktree } from '../repo-worktrees' -import { getSshGitProvider } from '../providers/ssh-git-dispatch' -import type { IGitProvider, IPtyProvider } from '../providers/types' +import type { IPtyProvider } from '../providers/types' import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { listRegisteredPtys } from '../memory/pty-registry' import { getSshPtyProvider } from './pty' -import { isFolderRepo } from '../../shared/repo-kind' -import type { - GitStatusResult, - GitWorktreeInfo, - Repo, - Worktree, - WorktreeMeta -} from '../../shared/types' -import { mergeWorktree } from './worktree-logic' -import { splitWorktreeId } from '../../shared/worktree-id' import { WORKSPACE_CLEANUP_CLASSIFIER_VERSION, - applyWorkspaceCleanupPolicy, - createWorkspaceCleanupFingerprint, - getWorkspaceCleanupInactivityReasons, - isWorkspaceOldForCleanup, - type WorkspaceCleanupBlocker, - type WorkspaceCleanupCandidate, type WorkspaceCleanupDismissArgs, type WorkspaceCleanupLocalProcessArgs, type WorkspaceCleanupLocalProcessResult, - type WorkspaceCleanupReason, - type WorkspaceCleanupScanError, type WorkspaceCleanupScanArgs, type WorkspaceCleanupScanResult } from '../../shared/workspace-cleanup' +import { scanWorkspaceCleanup } from './workspace-cleanup-scan' -const GIT_READ_TIMEOUT_MS = 8_000 -const WORKTREE_SCAN_CONCURRENCY = 3 - -type GitEvidence = { - clean: boolean | null - upstreamAhead: number | null - upstreamBehind: number | null - checkedAt: number | null - blockers: WorkspaceCleanupBlocker[] -} +export { scanWorkspaceCleanup } type WorkspaceCleanupHandlerDeps = { runtime?: OrcaRuntimeService @@ -64,8 +32,12 @@ export function registerWorkspaceCleanupHandlers( ipcMain.handle( 'workspaceCleanup:scan', - (_event, args?: WorkspaceCleanupScanArgs): Promise => - scanWorkspaceCleanup(store, args ?? {}) + (event, args?: WorkspaceCleanupScanArgs): Promise => + scanWorkspaceCleanup(store, args ?? {}, { + onProgress: args?.scanId + ? (progress) => event.sender.send('workspaceCleanup:scanProgress', progress) + : undefined + }) ) ipcMain.handle('workspaceCleanup:dismiss', (_event, args: WorkspaceCleanupDismissArgs) => { @@ -187,557 +159,3 @@ function normalizeRemotePath(path: string): string { function isPathWithin(candidatePath: string, parentPath: string): boolean { return candidatePath === parentPath || candidatePath.startsWith(`${parentPath}/`) } - -export async function scanWorkspaceCleanup( - store: Store, - args: WorkspaceCleanupScanArgs = {} -): Promise { - const scannedAt = Date.now() - const repos = store.getRepos() - const errors: WorkspaceCleanupScanResult['errors'] = [] - const candidates: WorkspaceCleanupCandidate[] = [] - - for (const repo of repos) { - const result = await scanRepoWorkspaces({ - store, - repo, - scannedAt, - targetWorktreeId: args.worktreeId, - skipGitWorktreeIds: new Set(args.skipGitWorktreeIds ?? []) - }) - appendListItems(candidates, result.candidates) - appendListItems(errors, result.errors) - } - - return { scannedAt, candidates, errors } -} - -async function scanRepoWorkspaces(args: { - store: Store - repo: Repo - scannedAt: number - targetWorktreeId?: string - skipGitWorktreeIds: Set -}): Promise { - const { store, repo, scannedAt, targetWorktreeId, skipGitWorktreeIds } = args - const errors: WorkspaceCleanupScanResult['errors'] = [] - let provider: IGitProvider | null = null - let gitWorktrees: GitWorktreeInfo[] = [] - const repoIsFolder = isFolderRepo(repo) - - try { - if (repoIsFolder) { - gitWorktrees = [createFolderWorktree(repo)] - } else if (repo.connectionId) { - provider = getSshGitProvider(repo.connectionId) ?? null - if (!provider) { - // Why: cleanup should reflect only workspaces Orca can currently - // inspect. Disconnected SSH repos are skipped in broad scans. - return { - scannedAt, - candidates: targetWorktreeId - ? synthesizeDisconnectedSshCandidates(store, repo, scannedAt, targetWorktreeId) - : [], - errors: [] - } - } - gitWorktrees = await withTimeout( - provider.listWorktrees(repo.path), - GIT_READ_TIMEOUT_MS, - 'Timed out listing SSH worktrees.' - ) - } else { - gitWorktrees = await withTimeout( - listRepoWorktrees(repo), - GIT_READ_TIMEOUT_MS, - 'Timed out listing worktrees.' - ) - } - } catch (error) { - console.error('Workspace cleanup repo scan failed', error) - if (repo.connectionId && !targetWorktreeId) { - // Why: broad cleanup only shows remote workspaces Orca can inspect now. - // A connected SSH repo that fails mid-scan is omitted, not bannered. - return { scannedAt, candidates: [], errors: [] } - } - errors.push(createScanError(repo, toSafeRepoScanError(error))) - return { scannedAt, candidates: [], errors } - } - - const worktrees = gitWorktrees - .map((gitWorktree) => { - const worktreeId = `${repo.id}::${gitWorktree.path}` - const meta = store.getWorktreeMeta(worktreeId) - return mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) - }) - .filter((worktree) => { - if (targetWorktreeId) { - return worktree.id === targetWorktreeId - } - return ( - !repoIsFolder && - !worktree.isMainWorktree && - isWorkspaceInactiveForCleanup(worktree, scannedAt) - ) - }) - - const candidates = await mapWithConcurrency(worktrees, WORKTREE_SCAN_CONCURRENCY, (worktree) => - buildCandidate({ - repo, - worktree, - scannedAt, - provider, - skipGit: skipGitWorktreeIds.has(worktree.id), - forceGitCheck: Boolean(targetWorktreeId) - }).catch((error) => { - console.error('Workspace cleanup candidate scan failed', error) - return buildCandidateFromError(repo, worktree, scannedAt, toErrorMessage(error)) - }) - ) - - return { scannedAt, candidates, errors } -} - -async function buildCandidate(args: { - repo: Repo - worktree: Worktree - scannedAt: number - provider: IGitProvider | null - skipGit: boolean - forceGitCheck: boolean -}): Promise { - const { repo, worktree, scannedAt, provider, skipGit, forceGitCheck } = args - const blockers: WorkspaceCleanupBlocker[] = [] - const reasons = getInactivityReasons(worktree, scannedAt) - const repoIsFolder = isFolderRepo(repo) - - if (worktree.isMainWorktree) { - blockers.push('main-worktree') - } - if (repoIsFolder) { - blockers.push('folder-repo') - } - if (worktree.isPinned) { - blockers.push('pinned') - } - - const localContext = buildLocalContext(worktree) - const shouldReadGit = shouldReadGitEvidence({ - repoIsFolder, - blockers, - worktree, - skipGit, - forceGitCheck - }) - - const gitEvidence = !shouldReadGit - ? createEmptyGitEvidence() - : await readGitEvidence(worktree, repo, provider) - appendListItems(blockers, gitEvidence.blockers) - - const candidateWithoutFingerprint: WorkspaceCleanupCandidate = { - worktreeId: worktree.id, - repoId: repo.id, - repoName: repo.displayName, - connectionId: repo.connectionId ?? null, - displayName: worktree.displayName, - branch: shortBranchName(worktree.branch), - path: worktree.path, - tier: 'review', - selectedByDefault: false, - reasons, - blockers: uniqueBlockers(blockers), - lastActivityAt: worktree.lastActivityAt, - ...(worktree.createdAt !== undefined ? { createdAt: worktree.createdAt } : {}), - localContext, - git: { - clean: gitEvidence.clean, - upstreamAhead: gitEvidence.upstreamAhead, - upstreamBehind: gitEvidence.upstreamBehind, - checkedAt: gitEvidence.checkedAt - }, - fingerprint: '' - } - - const fingerprint = createWorkspaceCleanupFingerprint({ - branch: candidateWithoutFingerprint.branch, - head: worktree.head, - gitClean: gitEvidence.clean, - lastActivityAt: worktree.lastActivityAt - }) - - return applyWorkspaceCleanupPolicy({ - ...candidateWithoutFingerprint, - reasons: uniqueReasons(reasons), - blockers: uniqueBlockers(blockers), - fingerprint - }) -} - -function shouldReadGitEvidence(args: { - repoIsFolder: boolean - blockers: WorkspaceCleanupBlocker[] - worktree: Worktree - skipGit: boolean - forceGitCheck: boolean -}): boolean { - const { repoIsFolder, blockers, worktree, skipGit, forceGitCheck } = args - if ((skipGit && !forceGitCheck) || repoIsFolder || worktree.isMainWorktree) { - return false - } - if ( - blockers.includes('pinned') || - blockers.includes('main-worktree') || - blockers.includes('folder-repo') - ) { - return false - } - - // Why: inactivity is the only recommendation signal now. Git is read only - // to keep the destructive path from deleting dirty or local-only branch work. - return true -} - -function appendListItems(target: T[], entries: readonly T[]): void { - // Why: cleanup can aggregate generated-size worktree batches; spreading - // those batches into push can exceed JavaScript's argument limit. - for (const entry of entries) { - target.push(entry) - } -} - -async function readGitEvidence( - worktree: Worktree, - repo: Repo, - provider: IGitProvider | null -): Promise { - const blockers: WorkspaceCleanupBlocker[] = [] - let status: GitStatusResult - const checkedAt = Date.now() - - try { - status = await withTimeout( - repo.connectionId ? provider!.getStatus(worktree.path) : getStatus(worktree.path), - GIT_READ_TIMEOUT_MS, - 'Timed out reading git status.' - ) - } catch { - return { - ...createEmptyGitEvidence(), - blockers: ['git-status-error'] - } - } - - if (status.upstreamStatus === undefined) { - return { - ...createEmptyGitEvidence(), - blockers: ['git-status-error'] - } - } - - const clean = status.entries.length === 0 - if (!clean) { - blockers.push('dirty-files') - } - - const upstreamAhead = status.upstreamStatus.hasUpstream ? status.upstreamStatus.ahead : null - const upstreamBehind = status.upstreamStatus.hasUpstream ? status.upstreamStatus.behind : null - if (upstreamAhead !== null && upstreamAhead > 0) { - blockers.push('unpushed-commits') - } - if (clean && upstreamAhead === null) { - const unpushedCommitCount = await readUnpushedCommitCount(worktree, repo, provider) - if (unpushedCommitCount === null) { - blockers.push('unknown-base') - } else if (unpushedCommitCount > 0) { - blockers.push('unpushed-commits') - } - } - - const evidence: GitEvidence = { - clean, - upstreamAhead, - upstreamBehind, - checkedAt, - blockers - } - - return { ...evidence, blockers: uniqueBlockers(blockers) } -} - -async function readUnpushedCommitCount( - worktree: Worktree, - repo: Repo, - provider: IGitProvider | null -): Promise { - try { - const result = await withTimeout( - repo.connectionId - ? provider!.exec(['rev-list', '--count', 'HEAD', '--not', '--remotes'], worktree.path) - : gitExecFileAsync(['rev-list', '--count', 'HEAD', '--not', '--remotes'], { - cwd: worktree.path - }), - GIT_READ_TIMEOUT_MS, - 'Timed out checking unpushed commits.' - ) - const count = Number.parseInt(result.stdout.trim(), 10) - return Number.isFinite(count) ? count : null - } catch { - return null - } -} - -function buildCandidateFromError( - repo: Repo, - worktree: Worktree, - scannedAt: number, - _message: string -): WorkspaceCleanupCandidate { - return applyWorkspaceCleanupPolicy({ - worktreeId: worktree.id, - repoId: repo.id, - repoName: repo.displayName, - connectionId: repo.connectionId ?? null, - displayName: worktree.displayName, - branch: shortBranchName(worktree.branch), - path: worktree.path, - tier: 'protected', - selectedByDefault: false, - reasons: getInactivityReasons(worktree, scannedAt), - blockers: ['git-status-error'], - lastActivityAt: worktree.lastActivityAt, - ...(worktree.createdAt !== undefined ? { createdAt: worktree.createdAt } : {}), - localContext: buildLocalContext(worktree), - git: { - clean: null, - upstreamAhead: null, - upstreamBehind: null, - checkedAt: scannedAt - }, - fingerprint: createWorkspaceCleanupFingerprint({ - branch: shortBranchName(worktree.branch), - head: worktree.head, - gitClean: null, - lastActivityAt: worktree.lastActivityAt - }) - }) -} - -function synthesizeDisconnectedSshCandidates( - store: Store, - repo: Repo, - scannedAt: number, - targetWorktreeId?: string -): WorkspaceCleanupCandidate[] { - const repoWorktreePrefix = `${repo.id}::` - if (targetWorktreeId) { - if (!targetWorktreeId.startsWith(repoWorktreePrefix)) { - return [] - } - // Why: focused delete preflight names one workspace already; walking all - // persisted metadata is unnecessary for disconnected SSH repos. - const meta = store.getWorktreeMeta(targetWorktreeId) - return meta ? [createDisconnectedSshCandidate(repo, scannedAt, targetWorktreeId, meta)] : [] - } - - const candidates: WorkspaceCleanupCandidate[] = [] - const allMeta = store.getAllWorktreeMeta() - for (const worktreeId in allMeta) { - if (!Object.hasOwn(allMeta, worktreeId) || !worktreeId.startsWith(repoWorktreePrefix)) { - continue - } - const meta = allMeta[worktreeId] - if (!meta || !isWorkspaceInactiveForCleanup(meta, scannedAt)) { - continue - } - candidates.push(createDisconnectedSshCandidate(repo, scannedAt, worktreeId, meta)) - } - return candidates -} - -function createDisconnectedSshCandidate( - repo: Repo, - scannedAt: number, - worktreeId: string, - meta: WorktreeMeta -): WorkspaceCleanupCandidate { - const parsed = splitWorktreeId(worktreeId) - const path = parsed?.worktreePath ?? worktreeId - const reasons = getInactivityReasons(meta, scannedAt) - return applyWorkspaceCleanupPolicy({ - worktreeId, - repoId: repo.id, - repoName: repo.displayName, - connectionId: repo.connectionId ?? null, - displayName: meta.displayName || basename(path), - branch: basename(path), - path, - tier: 'protected', - selectedByDefault: false, - reasons, - blockers: ['ssh-disconnected'], - lastActivityAt: meta.lastActivityAt, - ...(meta.createdAt !== undefined ? { createdAt: meta.createdAt } : {}), - localContext: { - terminalTabCount: 0, - cleanEditorTabCount: 0, - browserTabCount: 0, - diffCommentCount: meta.diffComments?.length ?? 0, - newestDiffCommentAt: getNewestDiffCommentAt(meta.diffComments), - retainedDoneAgentCount: 0 - }, - git: { - clean: null, - upstreamAhead: null, - upstreamBehind: null, - checkedAt: null - }, - fingerprint: createWorkspaceCleanupFingerprint({ - branch: basename(path), - head: '', - gitClean: null, - lastActivityAt: meta.lastActivityAt - }) - }) -} - -function buildLocalContext(worktree: Worktree): WorkspaceCleanupCandidate['localContext'] { - return { - terminalTabCount: 0, - cleanEditorTabCount: 0, - browserTabCount: 0, - diffCommentCount: worktree.diffComments?.length ?? 0, - newestDiffCommentAt: getNewestDiffCommentAt(worktree.diffComments), - retainedDoneAgentCount: 0 - } -} - -function getNewestDiffCommentAt(diffComments: Worktree['diffComments'] | undefined): number | null { - if (!diffComments || diffComments.length === 0) { - return null - } - // Why: persisted diff notes can grow large enough for spread-based Math.max - // to exceed the JavaScript argument limit during cleanup scans. - let newest = diffComments[0]?.createdAt ?? null - for (let index = 1; index < diffComments.length; index += 1) { - const createdAt = diffComments[index]?.createdAt - if (createdAt !== undefined && (newest === null || createdAt > newest)) { - newest = createdAt - } - } - return newest -} - -function createEmptyGitEvidence(): GitEvidence { - return { - clean: null, - upstreamAhead: null, - upstreamBehind: null, - checkedAt: null, - blockers: [] - } -} - -function shortBranchName(branch: string): string { - return branch.replace(/^refs\/heads\//, '') || 'HEAD' -} - -function uniqueBlockers(blockers: WorkspaceCleanupBlocker[]): WorkspaceCleanupBlocker[] { - return [...new Set(blockers)] -} - -function uniqueReasons(reasons: WorkspaceCleanupReason[]): WorkspaceCleanupReason[] { - return [...new Set(reasons)] -} - -function isWorkspaceInactiveForCleanup( - workspace: Pick, - scannedAt: number -): boolean { - return isWorkspaceOldForCleanup(workspace, scannedAt) -} - -function getInactivityReasons( - workspace: Pick, - scannedAt: number -): WorkspaceCleanupReason[] { - return getWorkspaceCleanupInactivityReasons(workspace, scannedAt) -} - -async function mapWithConcurrency( - items: readonly T[], - limit: number, - fn: (item: T) => Promise -): Promise { - const results: R[] = [] - let nextIndex = 0 - const workerCount = Math.min(limit, items.length) - await Promise.all( - Array.from({ length: workerCount }, async () => { - while (nextIndex < items.length) { - const index = nextIndex - nextIndex += 1 - results[index] = await fn(items[index]) - } - }) - ) - return results -} - -async function withTimeout(promise: Promise, timeoutMs: number, message: string): Promise { - let timeoutId: NodeJS.Timeout | undefined - const timeoutPromise = new Promise((_, reject) => { - timeoutId = setTimeout(() => reject(new Error(message)), timeoutMs) - }) - try { - return await Promise.race([promise, timeoutPromise]) - } finally { - if (timeoutId) { - clearTimeout(timeoutId) - } - } -} - -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} - -function createScanError(repo: Repo, message: string): WorkspaceCleanupScanError { - return { - repoId: repo.id, - repoName: repo.displayName || basename(repo.path), - message - } -} - -// Why: git errors often include absolute paths or command output. Keep the -// cause useful without leaking raw local/remote filesystem details to the UI. -function toSafeRepoScanError(error: unknown): string { - const message = toErrorMessage(error) - if (message === 'Timed out listing SSH worktrees.') { - return 'Timed out listing remote worktrees.' - } - if (message === 'Timed out listing worktrees.') { - return 'Timed out listing worktrees.' - } - if (message.startsWith('Timed out ')) { - return message.replace(/\.$/, '') - } - - const lower = message.toLowerCase() - if (lower.includes('not a git repository') || lower.includes('not a git worktree')) { - return 'Repository is not a git checkout.' - } - if ( - lower.includes('enoent') || - lower.includes('no such file') || - lower.includes('cannot find') || - lower.includes('does not exist') - ) { - return 'Repository folder was not found.' - } - if (lower.includes('eacces') || lower.includes('eperm') || lower.includes('permission denied')) { - return 'Repository folder is not accessible.' - } - return 'Git could not list worktrees.' -} diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 751ebc1ef7c..563f3e60b59 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -103,11 +103,14 @@ export class SshGitProvider implements IGitProvider { const upstreamCacheBypassArgs = options?.bypassEffectiveUpstreamNegativeCache ? { bypassEffectiveUpstreamNegativeCache: true } : {} - return (await this.mux.request('git.status', { + const request = { worktreePath, ...includeIgnoredArgs, ...upstreamCacheBypassArgs - })) as GitStatusResult + } + return (await (options?.signal + ? this.mux.request('git.status', request, { signal: options.signal }) + : this.mux.request('git.status', request))) as GitStatusResult } async checkIgnoredPaths(worktreePath: string, relativePaths: string[]): Promise { diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index c8a090e59fa..3a3ebdd94a4 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -168,6 +168,7 @@ export type IFilesystemProvider = { export type GitProviderStatusOptions = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean + signal?: AbortSignal } export type IGitProvider = { diff --git a/src/main/repo-worktrees.ts b/src/main/repo-worktrees.ts index ca3bcf359c9..e02ba35f9c7 100644 --- a/src/main/repo-worktrees.ts +++ b/src/main/repo-worktrees.ts @@ -6,6 +6,7 @@ import { areWorktreePathsEqual } from './ipc/worktree-logic' type LocalRepoWorktreeListOptions = { wslDistro?: string + signal?: AbortSignal } export function isRepoRoot(repos: Repo[], resolvedTarget: string): boolean { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 17945e471d8..1f13ad234d7 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -391,6 +391,7 @@ import type { WorkspaceCleanupLocalProcessArgs, WorkspaceCleanupLocalProcessResult, WorkspaceCleanupScanArgs, + WorkspaceCleanupScanProgress, WorkspaceCleanupScanResult } from '../shared/workspace-cleanup' import type { KeybindingActionId, KeybindingFileSnapshot } from '../shared/keybindings' @@ -989,7 +990,10 @@ export type PreloadApi = { ) => () => void } workspaceCleanup: { - scan: (args?: WorkspaceCleanupScanArgs) => Promise + scan: ( + args?: WorkspaceCleanupScanArgs, + onProgress?: (progress: WorkspaceCleanupScanProgress) => void + ) => Promise dismiss: (args: WorkspaceCleanupDismissArgs) => Promise clearDismissals: () => Promise hasKillableLocalProcesses: ( diff --git a/src/preload/index.ts b/src/preload/index.ts index 96f6797ffb8..d29a1b701c7 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -79,6 +79,7 @@ import type { RateLimitState } from '../shared/rate-limit-types' import type { WorkspaceSpaceScanProgress } from '../shared/workspace-space-types' +import type { WorkspaceCleanupScanProgress } from '../shared/workspace-cleanup' import type { WorkspacePortAdvertisedUrlChangedEvent } from '../shared/workspace-ports' import type { GhAuthDiagnostic } from '../shared/github-auth-types' import type { TaskSourceContext } from '../shared/task-source-context' @@ -662,7 +663,24 @@ const api = { } satisfies PreloadApi['worktrees'], workspaceCleanup: { - scan: (args) => ipcRenderer.invoke('workspaceCleanup:scan', args), + scan: (args, onProgress) => { + if (!onProgress) { + return ipcRenderer.invoke('workspaceCleanup:scan', args) + } + const scanId = args?.scanId ?? crypto.randomUUID() + const listener = ( + _event: Electron.IpcRendererEvent, + progress: WorkspaceCleanupScanProgress + ): void => { + if (progress.scanId === scanId) { + onProgress(progress) + } + } + ipcRenderer.on('workspaceCleanup:scanProgress', listener) + return ipcRenderer + .invoke('workspaceCleanup:scan', { ...args, scanId }) + .finally(() => ipcRenderer.removeListener('workspaceCleanup:scanProgress', listener)) + }, dismiss: (args) => ipcRenderer.invoke('workspaceCleanup:dismiss', args), clearDismissals: () => ipcRenderer.invoke('workspaceCleanup:clearDismissals'), hasKillableLocalProcesses: (args) => diff --git a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx index df9f81dfa37..11ca5d3c2f3 100644 --- a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx +++ b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx @@ -1,19 +1,20 @@ -/* eslint-disable max-lines -- Why: cleanup scanning, safety review, and - confirmation stay together so destructive workspace deletion remains - auditable. */ -import React, { useCallback, useEffect, useMemo, useState } from 'react' +/* eslint-disable max-lines -- Why: the cleanup dialog keeps scan status, + filters, row actions, localized review copy, and force-aware confirmation + in one modal flow. */ +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { AlertTriangle, Check, EyeOff, Loader2, - Minus, RefreshCcw, Search, + SlidersHorizontal, Trash2, X } from 'lucide-react' import { toast } from 'sonner' +import { useShallow } from 'zustand/react/shallow' import { Dialog, DialogContent, @@ -23,6 +24,20 @@ import { DialogTitle } from '@/components/ui/dialog' import { Button } from '@/components/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuSeparator, + DropdownMenuSub, + DropdownMenuSubContent, + DropdownMenuSubTrigger, + DropdownMenuTrigger +} from '@/components/ui/dropdown-menu' +import { Input } from '@/components/ui/input' import RepoMultiCombobox from '@/components/ui/repo-multi-combobox' import { ScrollArea } from '@/components/ui/scroll-area' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' @@ -36,8 +51,23 @@ import { type WorkspaceCleanupBlocker, type WorkspaceCleanupCandidate, type WorkspaceCleanupScanError, - type WorkspaceCleanupTier + type WorkspaceCleanupScanProgress } from '../../../../shared/workspace-cleanup' +import { + filterWorkspaceCleanupCandidates, + getWorkspaceCleanupGitLabel, + getWorkspaceCleanupReviewInfo, + hasWorkspaceCleanupLocalContext, + sortWorkspaceCleanupCandidates, + type WorkspaceCleanupContextFilter, + type WorkspaceCleanupFilters, + type WorkspaceCleanupGitFilter, + type WorkspaceCleanupReviewFilter, + type WorkspaceCleanupReviewInfo, + type WorkspaceCleanupSortDirection, + type WorkspaceCleanupSortKey, + type WorkspaceCleanupTimeFilter +} from './workspace-cleanup-presentation' import { resolveWorkspaceCleanupActiveView, type WorkspaceCleanupView, @@ -45,10 +75,20 @@ import { } from './workspace-cleanup-view-selection' import { translate } from '@/i18n/i18n' -const TIER_LABELS: Record = { - ready: 'Suggested cleanup', - review: 'Needs a closer look', - protected: 'Not suggested for cleanup' +const DEFAULT_FILTERS: WorkspaceCleanupFilters = { + query: '', + time: 'all', + review: 'all', + git: 'all', + context: 'all' +} + +const EMPTY_REVIEW_INFO: WorkspaceCleanupReviewInfo = { + hasReview: false, + label: null, + state: null, + provider: null, + title: null } const BLOCKER_LABELS: Record = { @@ -142,45 +182,23 @@ function formatScanErrorReason(message: string | undefined): string { return message.replace(/\.$/, '') } -function isOldWorkspaceCandidate(candidate: WorkspaceCleanupCandidate): boolean { - if (candidate.blockers.includes('main-worktree') || candidate.blockers.includes('folder-repo')) { - return false - } - return candidate.reasons.includes('archived') || candidate.reasons.includes('idle-clean') -} - -function compareCleanupCandidates( - a: WorkspaceCleanupCandidate, - b: WorkspaceCleanupCandidate -): number { - const priorityA = getCleanupCandidatePriority(a) - const priorityB = getCleanupCandidatePriority(b) - if (priorityA !== priorityB) { - return priorityA - priorityB - } - return a.lastActivityAt - b.lastActivityAt -} - -function getCleanupCandidatePriority(candidate: WorkspaceCleanupCandidate): number { - if (candidate.tier === 'ready') { - return 0 - } - if (candidate.reasons.length > 0) { - return 1 - } - if (isOldWorkspaceCandidate(candidate)) { - return 2 - } - return 3 -} - export default function WorkspaceCleanupDialog(): React.JSX.Element { const activeModal = useAppStore((s) => s.activeModal) + const openModal = useAppStore((s) => s.openModal) const closeModal = useAppStore((s) => s.closeModal) const scan = useAppStore((s) => s.workspaceCleanupScan) + const scanProgress = useAppStore((s) => s.workspaceCleanupProgress) const loading = useAppStore((s) => s.workspaceCleanupLoading) const error = useAppStore((s) => s.workspaceCleanupError) const repos = useAppStore((s) => s.repos) + const reviewStateInputs = useAppStore( + useShallow((s) => ({ + worktreesByRepo: s.worktreesByRepo, + hostedReviewCache: s.hostedReviewCache, + repos: s.repos, + settings: s.settings + })) + ) const scanWorkspaceCleanup = useAppStore((s) => s.scanWorkspaceCleanup) const markCandidateViewed = useAppStore((s) => s.markWorkspaceCleanupCandidateViewed) const dismissCandidates = useAppStore((s) => s.dismissWorkspaceCleanupCandidates) @@ -188,35 +206,104 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { const removeCandidates = useAppStore((s) => s.removeWorkspaceCleanupCandidates) const open = activeModal === 'workspace-cleanup' + const openRef = useRef(open) const [selectedIds, setSelectedIds] = useState>(() => new Set()) const [activeView, setActiveView] = useState('ready') const [confirming, setConfirming] = useState(false) const [removing, setRemoving] = useState(false) const [rowFailures, setRowFailures] = useState>({}) const [repoSelection, setRepoSelection] = useState>(() => new Set()) + const [filters, setFilters] = useState(DEFAULT_FILTERS) + const [sortKey, setSortKey] = useState('activity') + const [sortDirection, setSortDirection] = useState('asc') + const selectedDefaultsScanAtRef = useRef(null) + const autoScanAttemptedForOpenRef = useRef(false) + const latestReadyToastScanAtRef = useRef(null) + const wasOpenRef = useRef(false) const mountedRef = useMountedRef() const eligibleRepos = useMemo(() => repos.filter((repo) => isGitRepoKind(repo)), [repos]) const eligibleRepoIds = useMemo(() => eligibleRepos.map((repo) => repo.id), [eligibleRepos]) useEffect(() => { - if (open) { + openRef.current = open + }, [open]) + + const startWorkspaceCleanupScan = useCallback( + (options: { notifyWhenReady?: boolean } = {}) => { setRowFailures({}) - setActiveView('ready') - void scanWorkspaceCleanup().catch((err: unknown) => { - if (mountedRef.current) { - toast.error( + void scanWorkspaceCleanup() + .then((result) => { + if (!mountedRef.current || !options.notifyWhenReady || openRef.current) { + return + } + if (latestReadyToastScanAtRef.current === result.scannedAt) { + return + } + latestReadyToastScanAtRef.current = result.scannedAt + const suggestedCount = result.candidates.filter( + (candidate) => candidate.selectedByDefault + ).length + toast.success( translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.662b8ec3f8', - 'Workspace cleanup scan failed' + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.0e2d235c63', + 'Inactive workspace scan ready' ), { - description: err instanceof Error ? err.message : String(err) + description: formatWorkspaceCleanupReadyToastDescription( + result.candidates.length, + suggestedCount + ), + action: { + label: translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.4a35c08764', + 'Review' + ), + onClick: () => openModal('workspace-cleanup') + } } ) - } - }) + }) + .catch((err: unknown) => { + if (mountedRef.current) { + toast.error( + translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.662b8ec3f8', + 'Workspace cleanup scan failed' + ), + { + description: err instanceof Error ? err.message : String(err) + } + ) + } + }) + }, + [mountedRef, openModal, scanWorkspaceCleanup] + ) + + useEffect(() => { + if (!open) { + wasOpenRef.current = false + autoScanAttemptedForOpenRef.current = false + return } - }, [mountedRef, open, scanWorkspaceCleanup]) + if (!wasOpenRef.current) { + wasOpenRef.current = true + autoScanAttemptedForOpenRef.current = false + setActiveView('ready') + setConfirming(false) + setRowFailures({}) + setFilters(DEFAULT_FILTERS) + setSortKey('activity') + setSortDirection('asc') + if (scan) { + setSelectedIds(getDefaultSelectedWorkspaceCleanupIds(scan.candidates)) + } + } + if (!loading && !autoScanAttemptedForOpenRef.current) { + autoScanAttemptedForOpenRef.current = true + startWorkspaceCleanupScan({ notifyWhenReady: true }) + } + }, [loading, open, scan, startWorkspaceCleanupScan]) useEffect(() => { if (!open) { @@ -226,6 +313,13 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { }, [eligibleRepoIds, open]) const candidates = useMemo(() => scan?.candidates ?? [], [scan?.candidates]) + const reviewInfoByWorktreeId = useMemo(() => { + const infos = new Map() + for (const candidate of candidates) { + infos.set(candidate.worktreeId, getWorkspaceCleanupReviewInfo(candidate, reviewStateInputs)) + } + return infos + }, [candidates, reviewStateInputs]) const effectiveRepoSelection = useMemo>(() => { if (repoSelection.size > 0 || eligibleRepoIds.length === 0) { return repoSelection @@ -243,29 +337,28 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { }, [candidates, effectiveRepoSelection, eligibleRepoIds.length]) useEffect(() => { - if (!open || !scan) { + if (!scan || selectedDefaultsScanAtRef.current === scan.scannedAt) { return } - setSelectedIds( - new Set( - candidates - .filter((candidate) => candidate.selectedByDefault) - .map((candidate) => candidate.worktreeId) - ) - ) + selectedDefaultsScanAtRef.current = scan.scannedAt + setSelectedIds(getDefaultSelectedWorkspaceCleanupIds(scan.candidates)) setConfirming(false) - }, [open, scan, scan?.scannedAt, candidates]) + setRowFailures({}) + }, [scan]) const visibleCandidates = useMemo(() => { const rows = filteredCandidates.filter((candidate) => !candidate.blockers.includes('dismissed')) - return [...rows].sort(compareCleanupCandidates) - }, [filteredCandidates]) + return sortWorkspaceCleanupCandidates(rows, 'activity', 'asc', reviewInfoByWorktreeId) + }, [filteredCandidates, reviewInfoByWorktreeId]) const hiddenCandidates = useMemo( () => - filteredCandidates - .filter((candidate) => candidate.blockers.includes('dismissed')) - .sort(compareCleanupCandidates), - [filteredCandidates] + sortWorkspaceCleanupCandidates( + filteredCandidates.filter((candidate) => candidate.blockers.includes('dismissed')), + 'activity', + 'asc', + reviewInfoByWorktreeId + ), + [filteredCandidates, reviewInfoByWorktreeId] ) const groups = useMemo( () => ({ @@ -275,19 +368,6 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { }), [visibleCandidates] ) - const selectedCandidates = useMemo(() => { - const byId = new Map(filteredCandidates.map((candidate) => [candidate.worktreeId, candidate])) - return [...selectedIds] - .map((id) => byId.get(id)) - .filter( - (candidate): candidate is WorkspaceCleanupCandidate => - candidate != null && canQueueWorkspaceCleanupCandidate(candidate) - ) - }, [filteredCandidates, selectedIds]) - - const hiddenByKeepCount = filteredCandidates.filter((candidate) => - candidate.blockers.includes('dismissed') - ).length const cleanupViewCounts = useMemo( () => ({ ready: groups.ready.length, @@ -316,28 +396,50 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { () => formatScanNoticeMessage(selectedScanErrors, repoNameById), [repoNameById, selectedScanErrors] ) - const readyCount = groups.ready.length - const protectedCount = groups.protected.length - const inactiveCount = filteredCandidates.length const hasAnyCandidates = candidates.length > 0 - const initialLoading = loading && !scan - const activeRows = resolvedActiveView === 'hidden' ? hiddenCandidates : groups[resolvedActiveView] - const activeQueueableRows = useMemo( - () => activeRows.filter(canQueueWorkspaceCleanupCandidate), + const initialLoading = loading && !hasAnyCandidates + const activeBaseRows = + resolvedActiveView === 'hidden' ? hiddenCandidates : groups[resolvedActiveView] + const activeRows = useMemo( + () => + sortWorkspaceCleanupCandidates( + filterWorkspaceCleanupCandidates( + activeBaseRows, + filters, + reviewInfoByWorktreeId, + scan?.scannedAt ?? Date.now() + ), + sortKey, + sortDirection, + reviewInfoByWorktreeId + ), + [activeBaseRows, filters, reviewInfoByWorktreeId, scan?.scannedAt, sortDirection, sortKey] + ) + const activeRowIds = useMemo( + () => new Set(activeRows.map((candidate) => candidate.worktreeId)), [activeRows] ) - const activeQueueableSelected = useMemo( - () => activeQueueableRows.filter((candidate) => selectedIds.has(candidate.worktreeId)).length, - [activeQueueableRows, selectedIds] - ) - const allActiveQueueableSelected = - activeQueueableRows.length > 0 && activeQueueableSelected === activeQueueableRows.length - const someActiveQueueableSelected = activeQueueableSelected > 0 - const activeSelectionState = allActiveQueueableSelected - ? 'checked' - : someActiveQueueableSelected - ? 'mixed' - : 'unchecked' + const activeFilters = hasActiveWorkspaceCleanupFilters(filters) + const selectedCandidates = useMemo(() => { + const byId = new Map(activeRows.map((candidate) => [candidate.worktreeId, candidate])) + return [...selectedIds] + .map((id) => byId.get(id)) + .filter( + (candidate): candidate is WorkspaceCleanupCandidate => + candidate != null && canQueueWorkspaceCleanupCandidate(candidate) + ) + }, [activeRows, selectedIds]) + useEffect(() => { + if (!open || confirming) { + return + } + // Why: destructive selection must stay scoped to the rows the user can + // currently review after tier/filter changes. + setSelectedIds((current) => { + const next = new Set([...current].filter((id) => activeRowIds.has(id))) + return next.size === current.size ? current : next + }) + }, [activeRowIds, confirming, open]) const handleOpenChange = useCallback( (nextOpen: boolean) => { @@ -349,40 +451,8 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { ) const refresh = useCallback(() => { - setRowFailures({}) - void scanWorkspaceCleanup().catch((err: unknown) => { - if (mountedRef.current) { - toast.error( - translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.662b8ec3f8', - 'Workspace cleanup scan failed' - ), - { - description: err instanceof Error ? err.message : String(err) - } - ) - } - }) - }, [mountedRef, scanWorkspaceCleanup]) - - const toggleActiveSelection = useCallback(() => { - if (activeQueueableRows.length === 0) { - return - } - setSelectedIds((current) => { - const next = new Set(current) - if (allActiveQueueableSelected) { - for (const candidate of activeQueueableRows) { - next.delete(candidate.worktreeId) - } - } else { - for (const candidate of activeQueueableRows) { - next.add(candidate.worktreeId) - } - } - return next - }) - }, [activeQueueableRows, allActiveQueueableSelected]) + startWorkspaceCleanupScan({ notifyWhenReady: true }) + }, [startWorkspaceCleanupScan]) const ignoreCandidate = useCallback( (candidate: WorkspaceCleanupCandidate) => { @@ -492,12 +562,6 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { 'Delete Inactive Workspaces' )} - - {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.e0b5a4deaa', - 'Review inactive workspaces before deleting their local files and Orca state.' - )} -
@@ -545,15 +609,18 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
{translate( 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.7eee951968', - 'Checking workspace safety' + 'Checking inactive workspaces' )}
{translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.8b74d4ea6e', - 'Scanning worktrees and git state, then combining open tab, terminal, live agent, and remote availability signals before suggesting deletions.' + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.47123d0108', + 'Scanning inactive workspaces. You can close this and come back.' )}
+
+ {formatWorkspaceCleanupProgress(scanProgress)} +
) : hasAnyCandidates ? ( @@ -566,40 +633,6 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { 'selected' )} - - {inactiveCount}{' '} - {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.2b31bf68de', - 'inactive' - )} - - {readyCount > 0 ? ( - - {readyCount}{' '} - {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.b299f201b9', - 'safe to remove' - )} - - ) : null} - {groups.review.length > 0 ? ( - - {groups.review.length}{' '} - {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.1b18868569', - 'need review' - )} - - ) : null} - {protectedCount > 0 ? ( - - {protectedCount}{' '} - {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.37ab28277e', - 'not suggested' - )} - - ) : null}
{eligibleRepos.length > 1 ? ( @@ -629,6 +662,23 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
) : null} + {loading && scan && hasAnyCandidates ? ( +
+
+ + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.9a3be9f2df', + 'Scanning inactive workspaces. New rows appear here as they finish. You can close this and come back.' + )} + + + {formatWorkspaceCleanupProgress(scanProgress)} + +
+
+ ) : null} + {error ? (
{error} @@ -647,68 +697,20 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { onViewChange={setActiveView} />
-
-
- {resolvedActiveView !== 'hidden' && activeQueueableRows.length > 0 ? ( - - ) : null} -
- {resolvedActiveView === 'hidden' - ? translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.0c6672f5e3', - 'Ignored cleanup suggestions' - ) - : TIER_LABELS[resolvedActiveView]} -
-
- {resolvedActiveView === 'hidden' && hiddenByKeepCount > 0 ? ( - - ) : ( -
- {translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.592fbab446', - 'Sorted by oldest activity' - )} -
- )} -
+ {filteredCandidates.length > 0 ? ( + 0 + } + sortKey={sortKey} + sortDirection={sortDirection} + onFiltersChange={setFilters} + onSortKeyChange={setSortKey} + onSortDirectionChange={setSortDirection} + onRestoreIgnored={() => void resetDismissals()} + /> + ) : null}
{initialLoading ? : null} @@ -754,7 +756,28 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { onAction={() => setActiveView('hidden')} /> ) : null} - {!loading && scan && activeRows.length === 0 && visibleCandidates.length > 0 ? ( + {!loading && + scan && + activeRows.length === 0 && + activeBaseRows.length > 0 && + activeFilters ? ( + setFilters(DEFAULT_FILTERS)} + /> + ) : null} + {!loading && + scan && + activeRows.length === 0 && + visibleCandidates.length > 0 && + !activeFilters ? ( setConfirming(false)} onConfirm={() => void confirmRemove()} @@ -826,6 +853,233 @@ function StatusPill({ ) } +function WorkspaceCleanupFilterToolbar({ + filters, + showRestoreIgnored, + sortKey, + sortDirection, + onFiltersChange, + onSortKeyChange, + onSortDirectionChange, + onRestoreIgnored +}: { + filters: WorkspaceCleanupFilters + showRestoreIgnored: boolean + sortKey: WorkspaceCleanupSortKey + sortDirection: WorkspaceCleanupSortDirection + onFiltersChange: (filters: WorkspaceCleanupFilters) => void + onSortKeyChange: (sortKey: WorkspaceCleanupSortKey) => void + onSortDirectionChange: (direction: WorkspaceCleanupSortDirection) => void + onRestoreIgnored: () => void +}): React.JSX.Element { + const updateFilter = ( + key: K, + value: WorkspaceCleanupFilters[K] + ): void => { + onFiltersChange({ ...filters, [key]: value }) + } + const hasHiddenControls = hasActiveWorkspaceCleanupPanelControls(filters, sortKey, sortDirection) + const resetPanelControls = (): void => { + onFiltersChange({ + ...filters, + time: 'all', + review: 'all', + git: 'all', + context: 'all' + }) + onSortKeyChange('activity') + onSortDirectionChange('asc') + } + + return ( +
+
+ + updateFilter('query', event.target.value)} + placeholder="Search workspaces" + className="h-8 pl-8 text-xs" + /> +
+ + + + + + + + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.efb3843e75', + 'Filter and sort workspaces' + )} + + + + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.93b7381d50', + 'Filters' + )} + + + label="Age" + value={filters.time} + options={[ + ['all', 'Any age'], + ['30d', '30d+'], + ['90d', '90d+'], + ['archived', 'Archived'] + ]} + onChange={(value) => updateFilter('time', value)} + /> + + label="Review" + value={filters.review} + options={[ + ['all', 'Any review'], + ['no-review', 'No PR/MR'], + ['has-review', 'Has PR/MR'], + ['open-review', 'Open'], + ['closed-review', 'Closed'] + ]} + onChange={(value) => updateFilter('review', value)} + /> + + label="Git" + value={filters.git} + options={[ + ['all', 'Any git'], + ['clean', 'Clean'], + ['dirty', 'Dirty'], + ['unpushed', 'Unpushed'], + ['unknown', 'Unknown'] + ]} + onChange={(value) => updateFilter('git', value)} + /> + + label="Context" + value={filters.context} + options={[ + ['all', 'Any context'], + ['has-context', 'Has context'], + ['no-context', 'No context'] + ]} + onChange={(value) => updateFilter('context', value)} + /> + + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.a615e24679', + 'Sort' + )} + + + label="Sort by" + value={sortKey} + options={[ + ['activity', 'Activity'], + ['name', 'Name'], + ['repo', 'Repo'], + ['review', 'Review'], + ['git', 'Git'] + ]} + onChange={onSortKeyChange} + /> + + label="Direction" + value={sortDirection} + options={[ + ['asc', 'Ascending'], + ['desc', 'Descending'] + ]} + onChange={onSortDirectionChange} + /> + {showRestoreIgnored ? ( + <> + + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.aaee139eab', + 'Restore ignored suggestions' + )} + + + ) : null} + {hasHiddenControls ? ( + <> + + + {translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.e94b1f8bb4', + 'Clear filters' + )} + + + ) : null} + + +
+ ) +} + +function WorkspaceCleanupMenuSub({ + label, + value, + options, + onChange +}: { + label: string + value: T + options: readonly (readonly [T, string])[] + onChange: (value: T) => void +}): React.JSX.Element { + const valueLabel = options.find(([optionValue]) => optionValue === value)?.[1] ?? value + return ( + + + + {label} + + {valueLabel} + + + + + onChange(next as T)}> + {options.map(([optionValue, optionLabel]) => ( + event.preventDefault()} + > + {optionLabel} + + ))} + + + + ) +} + function CleanupViewNav({ activeView, counts, @@ -890,6 +1144,7 @@ function CleanupViewNav({ function CandidateRow({ candidate, + reviewInfo, last, selected, failure, @@ -899,6 +1154,7 @@ function CandidateRow({ onRemove }: { candidate: WorkspaceCleanupCandidate + reviewInfo: WorkspaceCleanupReviewInfo last: boolean selected: boolean failure?: string @@ -913,6 +1169,8 @@ function CandidateRow({ const contextDetails = formatContextDetails(candidate) const branchSafetyDetails = formatBranchSafetyDetails(candidate) const status = getCandidateStatus(candidate) + const gitLabel = getWorkspaceCleanupGitLabel(candidate) + const contextPillLabel = getContextPillLabel(candidate) return (
{candidate.displayName} {status.label} + {reviewInfo.label ? ( + {reviewInfo.label} + ) : null} + {gitLabel} + {contextPillLabel ? {contextPillLabel} : null} {translate( 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.352f15d6fc', @@ -1122,11 +1385,16 @@ function getCandidateStatus(candidate: WorkspaceCleanupCandidate): { } function formatGitStatus(candidate: WorkspaceCleanupCandidate): string { - if (candidate.git.clean === true) { - return 'Clean git' - } - if (candidate.git.clean === false) { - return 'Dirty git' + const label = getWorkspaceCleanupGitLabel(candidate) + switch (label) { + case 'Clean': + return 'Clean git' + case 'Dirty': + return 'Dirty git' + case 'Unpushed': + return 'Unpushed commits' + case 'Unknown': + return 'Git unknown' } return 'Git unknown' } @@ -1187,11 +1455,13 @@ function formatContextDetails(candidate: WorkspaceCleanupCandidate): string | nu function ConfirmRemove({ candidates, + reviewInfoByWorktreeId, removing, onCancel, onConfirm }: { candidates: WorkspaceCleanupCandidate[] + reviewInfoByWorktreeId: ReadonlyMap removing: boolean onCancel: () => void onConfirm: () => void @@ -1243,6 +1513,7 @@ function ConfirmRemove({ ))} @@ -1270,13 +1541,17 @@ function ConfirmRemove({ function ConfirmRemoveRow({ candidate, + reviewInfo, last }: { candidate: WorkspaceCleanupCandidate + reviewInfo: WorkspaceCleanupReviewInfo last: boolean }): React.JSX.Element { const dirtyLabel = getDirtyGitLabel(candidate) const branchDiffersFromName = candidate.branch !== candidate.displayName + const contextPillLabel = getContextPillLabel(candidate) + const status = getCandidateStatus(candidate) return (
@@ -1288,6 +1563,11 @@ function ConfirmRemoveRow({ )}{' '} {formatRelativeTime(candidate.lastActivityAt)} + {status.label} + {reviewInfo.label ? ( + {reviewInfo.label} + ) : null} + {contextPillLabel ? {contextPillLabel} : null} {dirtyLabel ? {dirtyLabel} : null}
@@ -1307,6 +1587,14 @@ function ConfirmRemoveRow({ } function getDirtyGitLabel(candidate: WorkspaceCleanupCandidate): string | null { + if (candidate.blockers.includes('unpushed-commits')) { + if (candidate.git.upstreamAhead && candidate.git.upstreamAhead > 0) { + return `${candidate.git.upstreamAhead} unpushed commit${ + candidate.git.upstreamAhead === 1 ? '' : 's' + }` + } + return 'Unpushed commits' + } if (candidate.git.upstreamAhead && candidate.git.upstreamAhead > 0) { return `${candidate.git.upstreamAhead} unpushed commit${ candidate.git.upstreamAhead === 1 ? '' : 's' @@ -1315,12 +1603,104 @@ function getDirtyGitLabel(candidate: WorkspaceCleanupCandidate): string | null { if (candidate.git.clean === false) { return 'Uncommitted changes' } - if (candidate.git.clean == null) { + if ( + candidate.git.clean == null || + candidate.blockers.includes('unknown-base') || + candidate.blockers.includes('git-status-error') + ) { return 'Git status unknown' } return null } +function getReviewPillTone( + reviewInfo: WorkspaceCleanupReviewInfo +): 'neutral' | 'ready' | 'review' | 'destructive' { + if (reviewInfo.state === 'open' || reviewInfo.state === 'draft') { + return 'review' + } + return 'neutral' +} + +function getContextPillLabel(candidate: WorkspaceCleanupCandidate): string | null { + if (!hasWorkspaceCleanupLocalContext(candidate)) { + return null + } + const count = + candidate.localContext.terminalTabCount + + candidate.localContext.cleanEditorTabCount + + candidate.localContext.browserTabCount + + candidate.localContext.diffCommentCount + + candidate.localContext.retainedDoneAgentCount + return `${count} context` +} + +function hasActiveWorkspaceCleanupFilters(filters: WorkspaceCleanupFilters): boolean { + return ( + filters.query.trim() !== '' || + filters.time !== 'all' || + filters.review !== 'all' || + filters.git !== 'all' || + filters.context !== 'all' + ) +} + +function hasActiveWorkspaceCleanupPanelControls( + filters: WorkspaceCleanupFilters, + sortKey: WorkspaceCleanupSortKey, + sortDirection: WorkspaceCleanupSortDirection +): boolean { + return ( + filters.time !== 'all' || + filters.review !== 'all' || + filters.git !== 'all' || + filters.context !== 'all' || + sortKey !== 'activity' || + sortDirection !== 'asc' + ) +} + +function getDefaultSelectedWorkspaceCleanupIds( + candidates: readonly WorkspaceCleanupCandidate[] +): Set { + return new Set( + candidates + .filter((candidate) => candidate.selectedByDefault) + .map((candidate) => candidate.worktreeId) + ) +} + +function formatWorkspaceCleanupReadyToastDescription( + inactiveCount: number, + suggestedCount: number +): string { + if (inactiveCount === 0) { + return 'No inactive workspaces found.' + } + const inactiveNoun = inactiveCount === 1 ? 'workspace' : 'workspaces' + const suggestedNoun = suggestedCount === 1 ? 'suggestion' : 'suggestions' + return `${inactiveCount} inactive ${inactiveNoun} found, with ${suggestedCount} cleanup ${suggestedNoun}.` +} + +function formatWorkspaceCleanupProgress(progress: WorkspaceCleanupScanProgress | null): string { + if (!progress || progress.totalWorktreeCount === 0) { + return translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.4cc5b73efe', + 'Finding inactive workspaces...' + ) + } + const noun = progress.totalWorktreeCount === 1 ? 'workspace' : 'workspaces' + return translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.5bf2e88480', + '{{value0}}/{{value1}} {{value2}} scanned', + { + value0: progress.scannedWorktreeCount, + value1: progress.totalWorktreeCount, + value2: noun + } + ) +} + function SkeletonRows(): React.JSX.Element { return (
diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-sort.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-sort.ts new file mode 100644 index 00000000000..ff597290bfd --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-sort.ts @@ -0,0 +1,251 @@ +import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' +import type { + WorkspaceCleanupContextFilter, + WorkspaceCleanupFilters, + WorkspaceCleanupGitFilter, + WorkspaceCleanupReviewFilter, + WorkspaceCleanupReviewInfo, + WorkspaceCleanupSortDirection, + WorkspaceCleanupSortKey, + WorkspaceCleanupTimeFilter +} from './workspace-cleanup-presentation' + +const DAY_MS = 24 * 60 * 60 * 1000 + +const EMPTY_REVIEW_INFO: WorkspaceCleanupReviewInfo = { + hasReview: false, + label: null, + state: null, + provider: null, + title: null +} + +const EMPTY_REVIEW_INFO_MAP = new Map() + +export function hasWorkspaceCleanupLocalContext(candidate: WorkspaceCleanupCandidate): boolean { + return ( + candidate.localContext.terminalTabCount > 0 || + candidate.localContext.cleanEditorTabCount > 0 || + candidate.localContext.browserTabCount > 0 || + candidate.localContext.diffCommentCount > 0 || + candidate.localContext.retainedDoneAgentCount > 0 + ) +} + +export function getWorkspaceCleanupSearchText( + candidate: WorkspaceCleanupCandidate, + reviewInfo: WorkspaceCleanupReviewInfo = EMPTY_REVIEW_INFO +): string { + return [ + candidate.displayName, + candidate.repoName, + candidate.branch, + candidate.path, + reviewInfo.label, + reviewInfo.title, + getWorkspaceCleanupGitLabel(candidate), + hasWorkspaceCleanupLocalContext(candidate) ? 'has context' : 'no context' + ] + .filter(Boolean) + .join(' ') + .toLowerCase() +} + +export function filterWorkspaceCleanupCandidates( + candidates: readonly WorkspaceCleanupCandidate[], + filters: WorkspaceCleanupFilters, + reviewInfoByWorktreeId: ReadonlyMap = EMPTY_REVIEW_INFO_MAP, + now: number = Date.now() +): WorkspaceCleanupCandidate[] { + const normalizedQuery = filters.query.trim().toLowerCase() + return candidates.filter((candidate) => { + const reviewInfo = reviewInfoByWorktreeId.get(candidate.worktreeId) ?? EMPTY_REVIEW_INFO + if ( + normalizedQuery && + !getWorkspaceCleanupSearchText(candidate, reviewInfo).includes(normalizedQuery) + ) { + return false + } + return ( + matchesTimeFilter(candidate, filters.time, now) && + matchesReviewFilter(reviewInfo, filters.review) && + matchesGitFilter(candidate, filters.git) && + matchesContextFilter(candidate, filters.context) + ) + }) +} + +export function sortWorkspaceCleanupCandidates( + candidates: readonly WorkspaceCleanupCandidate[], + sortKey: WorkspaceCleanupSortKey, + direction: WorkspaceCleanupSortDirection, + reviewInfoByWorktreeId: ReadonlyMap = EMPTY_REVIEW_INFO_MAP +): WorkspaceCleanupCandidate[] { + const multiplier = direction === 'asc' ? 1 : -1 + return [...candidates].sort((left, right) => { + const primary = + compareWorkspaceCleanupCandidates(left, right, sortKey, reviewInfoByWorktreeId) * multiplier + return ( + primary || + left.lastActivityAt - right.lastActivityAt || + left.repoName.localeCompare(right.repoName) || + left.displayName.localeCompare(right.displayName) + ) + }) +} + +export function getWorkspaceCleanupGitLabel(candidate: WorkspaceCleanupCandidate): string { + if (hasUnpushedCommits(candidate)) { + return 'Unpushed' + } + if (isGitStatusUnknown(candidate)) { + return 'Unknown' + } + if (candidate.git.clean === true) { + return 'Clean' + } + if (candidate.git.clean === false) { + return 'Dirty' + } + return 'Unknown' +} + +function compareWorkspaceCleanupCandidates( + left: WorkspaceCleanupCandidate, + right: WorkspaceCleanupCandidate, + sortKey: WorkspaceCleanupSortKey, + reviewInfoByWorktreeId: ReadonlyMap +): number { + switch (sortKey) { + case 'activity': + return left.lastActivityAt - right.lastActivityAt + case 'name': + return left.displayName.localeCompare(right.displayName) + case 'repo': + return ( + left.repoName.localeCompare(right.repoName) || + left.displayName.localeCompare(right.displayName) + ) + case 'review': + return ( + getReviewSortRank(reviewInfoByWorktreeId.get(left.worktreeId) ?? EMPTY_REVIEW_INFO) - + getReviewSortRank(reviewInfoByWorktreeId.get(right.worktreeId) ?? EMPTY_REVIEW_INFO) || + (reviewInfoByWorktreeId.get(left.worktreeId)?.label ?? '').localeCompare( + reviewInfoByWorktreeId.get(right.worktreeId)?.label ?? '' + ) + ) + case 'git': + return getGitSortRank(left) - getGitSortRank(right) + } +} + +function matchesTimeFilter( + candidate: WorkspaceCleanupCandidate, + filter: WorkspaceCleanupTimeFilter, + now: number +): boolean { + switch (filter) { + case 'all': + return true + case '30d': + return now - candidate.lastActivityAt >= 30 * DAY_MS + case '90d': + return now - candidate.lastActivityAt >= 90 * DAY_MS + case 'archived': + return candidate.reasons.includes('archived') + } +} + +function matchesReviewFilter( + reviewInfo: WorkspaceCleanupReviewInfo, + filter: WorkspaceCleanupReviewFilter +): boolean { + switch (filter) { + case 'all': + return true + case 'no-review': + return !reviewInfo.hasReview + case 'has-review': + return reviewInfo.hasReview + case 'open-review': + return reviewInfo.hasReview && (reviewInfo.state === 'open' || reviewInfo.state === 'draft') + case 'closed-review': + return ( + reviewInfo.hasReview && (reviewInfo.state === 'closed' || reviewInfo.state === 'merged') + ) + } +} + +function matchesGitFilter( + candidate: WorkspaceCleanupCandidate, + filter: WorkspaceCleanupGitFilter +): boolean { + switch (filter) { + case 'all': + return true + case 'clean': + return ( + candidate.git.clean === true && + !hasUnpushedCommits(candidate) && + !isGitStatusUnknown(candidate) + ) + case 'dirty': + return candidate.git.clean === false + case 'unpushed': + return hasUnpushedCommits(candidate) + case 'unknown': + return isGitStatusUnknown(candidate) + } +} + +function matchesContextFilter( + candidate: WorkspaceCleanupCandidate, + filter: WorkspaceCleanupContextFilter +): boolean { + switch (filter) { + case 'all': + return true + case 'has-context': + return hasWorkspaceCleanupLocalContext(candidate) + case 'no-context': + return !hasWorkspaceCleanupLocalContext(candidate) + } +} + +function getReviewSortRank(reviewInfo: WorkspaceCleanupReviewInfo): number { + if (!reviewInfo.hasReview) { + return 0 + } + if (reviewInfo.state === 'open' || reviewInfo.state === 'draft') { + return 3 + } + if (reviewInfo.state === 'unknown') { + return 2 + } + return 1 +} + +function getGitSortRank(candidate: WorkspaceCleanupCandidate): number { + if (hasUnpushedCommits(candidate)) { + return 4 + } + if (candidate.git.clean === false) { + return 3 + } + if (isGitStatusUnknown(candidate)) { + return 2 + } + return 1 +} + +function hasUnpushedCommits(candidate: WorkspaceCleanupCandidate): boolean { + return (candidate.git.upstreamAhead ?? 0) > 0 || candidate.blockers.includes('unpushed-commits') +} + +function isGitStatusUnknown(candidate: WorkspaceCleanupCandidate): boolean { + return ( + candidate.git.clean === null || + candidate.blockers.includes('git-status-error') || + candidate.blockers.includes('unknown-base') + ) +} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation-fixtures.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation-fixtures.ts new file mode 100644 index 00000000000..a9f6b5362da --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation-fixtures.ts @@ -0,0 +1,107 @@ +import type { AppState } from '@/store/types' +import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { Repo, Worktree } from '../../../../shared/types' +import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' +import type { WorkspaceCleanupFilters } from './workspace-cleanup-presentation' + +export const NOW = 1_700_000_000_000 + +const REPO: Repo = { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: NOW +} + +export const DEFAULT_FILTERS: WorkspaceCleanupFilters = { + query: '', + time: 'all', + review: 'all', + git: 'all', + context: 'all' +} + +export function makeCandidate( + overrides: Partial = {} +): WorkspaceCleanupCandidate { + return { + worktreeId: 'repo-1::/repo/alpha', + repoId: 'repo-1', + repoName: 'Repo', + connectionId: null, + displayName: 'alpha', + branch: 'alpha', + path: '/repo/alpha', + tier: 'ready', + selectedByDefault: true, + reasons: ['idle-clean'], + blockers: [], + lastActivityAt: NOW - 40 * 24 * 60 * 60 * 1000, + localContext: { + terminalTabCount: 0, + cleanEditorTabCount: 0, + browserTabCount: 0, + diffCommentCount: 0, + newestDiffCommentAt: null, + retainedDoneAgentCount: 0 + }, + git: { + clean: true, + upstreamAhead: 0, + upstreamBehind: 0, + checkedAt: NOW + }, + fingerprint: 'fingerprint', + ...overrides + } +} + +export function makeWorktree(overrides: Partial = {}): Worktree { + return { + id: 'repo-1::/repo/alpha', + repoId: 'repo-1', + displayName: 'alpha', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: NOW, + path: '/repo/alpha', + head: 'abc', + branch: 'refs/heads/alpha', + isBare: false, + isMainWorktree: false, + ...overrides + } +} + +export function makeReview(overrides: Partial = {}): HostedReviewInfo { + return { + provider: 'github', + number: 42, + title: 'Review alpha cleanup', + state: 'open', + url: 'https://example.test/review/42', + status: 'neutral', + updatedAt: new Date(NOW).toISOString(), + mergeable: 'UNKNOWN', + ...overrides + } +} + +export function makeState(overrides: Partial = {}): AppState { + return { + worktreesByRepo: { + 'repo-1': [makeWorktree()] + }, + repos: [REPO], + hostedReviewCache: {}, + settings: {}, + ...overrides + } as AppState & Partial +} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.test.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.test.ts new file mode 100644 index 00000000000..f6e1aab4488 --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.test.ts @@ -0,0 +1,228 @@ +import { describe, expect, it } from 'vitest' +import { getHostedReviewCacheKey } from '@/store/slices/hosted-review' +import { + filterWorkspaceCleanupCandidates, + getWorkspaceCleanupGitLabel, + getWorkspaceCleanupReviewInfo, + getWorkspaceCleanupSearchText, + hasWorkspaceCleanupLocalContext, + sortWorkspaceCleanupCandidates, + type WorkspaceCleanupReviewInfo +} from './workspace-cleanup-presentation' +import { + DEFAULT_FILTERS, + NOW, + makeCandidate, + makeReview, + makeState +} from './workspace-cleanup-presentation-fixtures' + +describe('workspace cleanup presentation', () => { + it('finds hosted review details from renderer state', () => { + const cacheKey = getHostedReviewCacheKey('/repo', 'alpha', {}, 'repo-1', undefined) + const state = makeState({ + hostedReviewCache: { + [cacheKey]: { data: makeReview(), fetchedAt: NOW } + } + }) + + expect(getWorkspaceCleanupReviewInfo(makeCandidate(), state)).toMatchObject({ + hasReview: true, + label: 'PR #42', + state: 'open', + title: 'Review alpha cleanup' + }) + }) + + it('filters by time, review, git, and context', () => { + const openReview: WorkspaceCleanupReviewInfo = { + hasReview: true, + label: 'PR #7', + state: 'open', + provider: 'github', + title: 'Review row' + } + const rows = [ + makeCandidate({ worktreeId: 'repo-1::/repo/clean', displayName: 'clean' }), + makeCandidate({ + worktreeId: 'repo-1::/repo/dirty-review', + displayName: 'dirty-review', + git: { clean: false, upstreamAhead: 0, upstreamBehind: 0, checkedAt: NOW }, + lastActivityAt: NOW - 100 * 24 * 60 * 60 * 1000, + localContext: { + terminalTabCount: 1, + cleanEditorTabCount: 0, + browserTabCount: 0, + diffCommentCount: 0, + newestDiffCommentAt: null, + retainedDoneAgentCount: 0 + } + }), + makeCandidate({ + worktreeId: 'repo-1::/repo/unknown', + displayName: 'unknown', + git: { clean: null, upstreamAhead: null, upstreamBehind: null, checkedAt: null }, + blockers: ['git-status-error'] + }) + ] + const reviewInfo = new Map([ + ['repo-1::/repo/dirty-review', openReview] + ]) + + expect( + filterWorkspaceCleanupCandidates( + rows, + { + ...DEFAULT_FILTERS, + time: '90d', + review: 'open-review', + git: 'dirty', + context: 'has-context' + }, + reviewInfo, + NOW + ).map((row) => row.displayName) + ).toEqual(['dirty-review']) + + expect( + filterWorkspaceCleanupCandidates( + rows, + { ...DEFAULT_FILTERS, git: 'unknown' }, + reviewInfo, + NOW + ).map((row) => row.displayName) + ).toEqual(['unknown']) + }) + + it('treats unknown-base clean rows as unknown git risk', () => { + const candidate = makeCandidate({ + git: { clean: true, upstreamAhead: null, upstreamBehind: null, checkedAt: NOW }, + blockers: ['unknown-base'] + }) + + expect(getWorkspaceCleanupGitLabel(candidate)).toBe('Unknown') + expect( + filterWorkspaceCleanupCandidates( + [candidate], + { ...DEFAULT_FILTERS, git: 'clean' }, + new Map(), + NOW + ) + ).toEqual([]) + expect( + filterWorkspaceCleanupCandidates( + [candidate], + { ...DEFAULT_FILTERS, git: 'unknown' }, + new Map(), + NOW + ) + ).toEqual([candidate]) + }) + + it('search includes repo, branch, path, review, git, and context labels', () => { + const candidate = makeCandidate({ + repoName: 'Search Repo', + branch: 'feature/search', + path: '/repo/search-target', + git: { clean: null, upstreamAhead: null, upstreamBehind: null, checkedAt: null }, + localContext: { + terminalTabCount: 0, + cleanEditorTabCount: 0, + browserTabCount: 1, + diffCommentCount: 0, + newestDiffCommentAt: null, + retainedDoneAgentCount: 0 + } + }) + const reviewInfo: WorkspaceCleanupReviewInfo = { + hasReview: true, + label: 'MR #9', + state: 'closed', + provider: 'gitlab', + title: 'Searchable review' + } + const text = getWorkspaceCleanupSearchText(candidate, reviewInfo) + + expect(text).toContain('search repo') + expect(text).toContain('feature/search') + expect(text).toContain('/repo/search-target') + expect(text).toContain('mr #9') + expect(text).toContain('unknown') + expect(text).toContain('has context') + }) + + it('sorts by activity, name, repo, review, and git', () => { + const rows = [ + makeCandidate({ + worktreeId: 'repo-1::/repo/bravo', + displayName: 'bravo', + repoName: 'Repo B', + lastActivityAt: NOW - 5, + git: { clean: false, upstreamAhead: 0, upstreamBehind: 0, checkedAt: NOW } + }), + makeCandidate({ + worktreeId: 'repo-1::/repo/alpha', + displayName: 'alpha', + repoName: 'Repo A', + lastActivityAt: NOW - 10, + git: { clean: true, upstreamAhead: 0, upstreamBehind: 0, checkedAt: NOW } + }), + makeCandidate({ + worktreeId: 'repo-1::/repo/charlie', + displayName: 'charlie', + repoName: 'Repo C', + lastActivityAt: NOW - 1, + git: { clean: true, upstreamAhead: 2, upstreamBehind: 0, checkedAt: NOW }, + blockers: ['unpushed-commits'] + }) + ] + const reviewInfo = new Map([ + [ + 'repo-1::/repo/charlie', + { + hasReview: true, + label: 'PR #3', + state: 'open', + provider: 'github', + title: null + } + ] + ]) + + expect( + sortWorkspaceCleanupCandidates(rows, 'activity', 'asc').map((row) => row.displayName) + ).toEqual(['alpha', 'bravo', 'charlie']) + expect( + sortWorkspaceCleanupCandidates(rows, 'name', 'asc').map((row) => row.displayName) + ).toEqual(['alpha', 'bravo', 'charlie']) + expect( + sortWorkspaceCleanupCandidates(rows, 'repo', 'asc').map((row) => row.displayName) + ).toEqual(['alpha', 'bravo', 'charlie']) + expect( + sortWorkspaceCleanupCandidates(rows, 'review', 'desc', reviewInfo).map( + (row) => row.displayName + ) + ).toEqual(['charlie', 'alpha', 'bravo']) + expect( + sortWorkspaceCleanupCandidates(rows, 'git', 'desc').map((row) => row.displayName) + ).toEqual(['charlie', 'bravo', 'alpha']) + }) + + it('detects local context', () => { + expect(hasWorkspaceCleanupLocalContext(makeCandidate())).toBe(false) + expect( + hasWorkspaceCleanupLocalContext( + makeCandidate({ + localContext: { + terminalTabCount: 0, + cleanEditorTabCount: 1, + browserTabCount: 0, + diffCommentCount: 0, + newestDiffCommentAt: null, + retainedDoneAgentCount: 0 + } + }) + ) + ).toBe(true) + }) +}) diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.ts new file mode 100644 index 00000000000..ad860e1f7e2 --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-presentation.ts @@ -0,0 +1,127 @@ +import { getWorktreeMapFromState } from '@/store/selectors' +import { getHostedReviewCacheKey } from '@/store/slices/hosted-review' +import type { AppState } from '@/store/types' +import type { HostedReviewInfo, HostedReviewProvider } from '../../../../shared/hosted-review' +import type { Repo, Worktree } from '../../../../shared/types' +import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' + +export type WorkspaceCleanupSortKey = 'activity' | 'name' | 'repo' | 'review' | 'git' +export type WorkspaceCleanupSortDirection = 'asc' | 'desc' +export type WorkspaceCleanupTimeFilter = 'all' | '30d' | '90d' | 'archived' +export type WorkspaceCleanupReviewFilter = + | 'all' + | 'no-review' + | 'has-review' + | 'open-review' + | 'closed-review' +export type WorkspaceCleanupGitFilter = 'all' | 'clean' | 'dirty' | 'unpushed' | 'unknown' +export type WorkspaceCleanupContextFilter = 'all' | 'has-context' | 'no-context' + +export type WorkspaceCleanupFilters = { + query: string + time: WorkspaceCleanupTimeFilter + review: WorkspaceCleanupReviewFilter + git: WorkspaceCleanupGitFilter + context: WorkspaceCleanupContextFilter +} + +export type WorkspaceCleanupReviewInfo = { + hasReview: boolean + label: string | null + state: 'open' | 'closed' | 'merged' | 'draft' | 'unknown' | null + provider: HostedReviewProvider | null + title: string | null +} + +export type WorkspaceCleanupRendererStateInputs = Pick< + AppState, + 'worktreesByRepo' | 'hostedReviewCache' | 'repos' | 'settings' +> + +export { + filterWorkspaceCleanupCandidates, + getWorkspaceCleanupGitLabel, + getWorkspaceCleanupSearchText, + hasWorkspaceCleanupLocalContext, + sortWorkspaceCleanupCandidates +} from './workspace-cleanup-filter-sort' + +export function getWorkspaceCleanupReviewInfo( + candidate: WorkspaceCleanupCandidate, + state: WorkspaceCleanupRendererStateInputs +): WorkspaceCleanupReviewInfo { + const worktree = getWorktreeMapFromState(state).get(candidate.worktreeId) ?? null + const repo = state.repos.find((entry) => entry.id === candidate.repoId) ?? null + const hostedReview = getCachedHostedReview(candidate, worktree, repo, state) + if (hostedReview) { + return { + hasReview: true, + label: `${getReviewShortLabel(hostedReview.provider)} #${hostedReview.number}`, + state: hostedReview.state, + provider: hostedReview.provider, + title: hostedReview.title + } + } + + const linkedReview = getLinkedReviewFallback(worktree) + if (linkedReview) { + return { + hasReview: true, + label: linkedReview.label, + state: 'unknown', + provider: linkedReview.provider, + title: null + } + } + + return { + hasReview: false, + label: null, + state: null, + provider: null, + title: null + } +} + +function getCachedHostedReview( + candidate: WorkspaceCleanupCandidate, + worktree: Worktree | null, + repo: Repo | null, + state: WorkspaceCleanupRendererStateInputs +): HostedReviewInfo | null { + if (!repo) { + return null + } + const cacheKey = getHostedReviewCacheKey( + repo.path, + getBranchDisplayName(worktree?.branch ?? candidate.branch), + state.settings, + repo.id, + repo.connectionId + ) + return state.hostedReviewCache[cacheKey]?.data ?? null +} + +function getLinkedReviewFallback(worktree: Worktree | null): { + label: string + provider: HostedReviewProvider +} | null { + if (!worktree) { + return null + } + if (worktree.linkedGitLabMR != null) { + return { label: `MR #${worktree.linkedGitLabMR}`, provider: 'gitlab' } + } + if (worktree.linkedPR != null) { + return { label: `PR #${worktree.linkedPR}`, provider: 'github' } + } + return null +} + +function getReviewShortLabel(provider: HostedReviewProvider): string { + return provider === 'gitlab' ? 'MR' : 'PR' +} + +function getBranchDisplayName(branch: string): string { + return branch.replace(/^refs\/heads\//, '') || 'HEAD' +} diff --git a/src/renderer/src/store/slices/workspace-cleanup.test.ts b/src/renderer/src/store/slices/workspace-cleanup.test.ts index 70dc8e2afed..116ec974d10 100644 --- a/src/renderer/src/store/slices/workspace-cleanup.test.ts +++ b/src/renderer/src/store/slices/workspace-cleanup.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from 'vitest' import type { AppState } from '../types' import type { WorkspaceCleanupCandidate, + WorkspaceCleanupScanProgress, WorkspaceCleanupScanResult } from '../../../../shared/workspace-cleanup' import { createWorkspaceCleanupSlice, enrichWorkspaceCleanupCandidates } from './workspace-cleanup' @@ -85,7 +86,368 @@ function createCleanupTestStore(removeWorktree = vi.fn()) { ) } +function installWorkspaceCleanupApi(scan: ReturnType) { + ;(globalThis as { window: unknown }).window = { + api: { + workspaceCleanup: { + scan, + dismiss: vi.fn().mockResolvedValue(undefined), + clearDismissals: vi.fn().mockResolvedValue(undefined), + hasKillableLocalProcesses: vi.fn().mockResolvedValue({ + hasKillableProcesses: false + }) + } + } + } +} + +function deferred(): { + promise: Promise + resolve: (value: T) => void + reject: (error: unknown) => void +} { + let resolve!: (value: T) => void + let reject!: (error: unknown) => void + const promise = new Promise((innerResolve, innerReject) => { + resolve = innerResolve + reject = innerReject + }) + return { promise, resolve, reject } +} + describe('workspace cleanup viewed rows', () => { + it('joins duplicate broad cleanup scans', async () => { + const pending = deferred() + const scan = vi.fn().mockReturnValue(pending.promise) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + + const first = store.getState().scanWorkspaceCleanup() + const second = store.getState().scanWorkspaceCleanup() + + expect(scan).toHaveBeenCalledTimes(1) + expect(store.getState().workspaceCleanupLoading).toBe(true) + + const result = { scannedAt: NOW, candidates: [makeCandidate()], errors: [] } + pending.resolve(result) + + await expect(Promise.all([first, second])).resolves.toEqual([result, result]) + expect(store.getState().workspaceCleanupScan?.candidates).toHaveLength(1) + expect(store.getState().workspaceCleanupLoading).toBe(false) + }) + + it('does not leave cleanup loading stuck when a reopen joins a just-settled scan', async () => { + const pending = deferred() + const result = { scannedAt: NOW, candidates: [makeCandidate()], errors: [] } + const scan = vi.fn().mockReturnValue(pending.promise) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + let joinedScan: Promise | null = null + + const unsubscribe = store.subscribe((state, previousState) => { + if ( + previousState.workspaceCleanupLoading && + !state.workspaceCleanupLoading && + joinedScan === null + ) { + joinedScan = state.scanWorkspaceCleanup() + } + }) + + const firstScan = store.getState().scanWorkspaceCleanup() + pending.resolve(result) + + await expect(firstScan).resolves.toEqual(result) + await expect(joinedScan).resolves.toEqual(result) + unsubscribe() + + expect(scan).toHaveBeenCalledTimes(1) + expect(store.getState().workspaceCleanupLoading).toBe(false) + }) + + it('shows scanned cleanup candidates before the final broad scan resolves', async () => { + const pending = deferred() + let onProgress: ((progress: WorkspaceCleanupScanProgress) => void) | undefined + const partialCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/partial' }) + const finalCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/final' }) + const scan = vi.fn((_args, progressCallback) => { + onProgress = progressCallback + return pending.promise + }) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + + const scanPromise = store.getState().scanWorkspaceCleanup() + onProgress?.({ + scanId: 'scan-1', + scannedAt: NOW, + scannedWorktreeCount: 1, + totalWorktreeCount: 2, + candidates: [partialCandidate], + errors: [] + }) + + expect(store.getState().workspaceCleanupLoading).toBe(true) + await vi.waitFor(() => { + expect(store.getState().workspaceCleanupProgress).toMatchObject({ + scannedWorktreeCount: 1, + totalWorktreeCount: 2 + }) + }) + expect(store.getState().workspaceCleanupScan?.candidates).toEqual([partialCandidate]) + + pending.resolve({ + scannedAt: NOW, + candidates: [partialCandidate, finalCandidate], + errors: [] + }) + + await expect(scanPromise).resolves.toEqual({ + scannedAt: NOW, + candidates: [partialCandidate, finalCandidate], + errors: [] + }) + expect(store.getState().workspaceCleanupLoading).toBe(false) + expect(store.getState().workspaceCleanupProgress).toMatchObject({ + scannedWorktreeCount: 2, + totalWorktreeCount: 2 + }) + }) + + it('does not re-probe previously enriched rows during cumulative progress updates', async () => { + const pending = deferred() + let onProgress: ((progress: WorkspaceCleanupScanProgress) => void) | undefined + const terminalCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/terminal' }) + const laterCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/later' }) + const scan = vi.fn((_args, progressCallback) => { + onProgress = progressCallback + return pending.promise + }) + installWorkspaceCleanupApi(scan) + const hasChildProcesses = vi.fn().mockResolvedValue(false) + const getForegroundProcess = vi.fn().mockResolvedValue('zsh') + ;( + globalThis.window as unknown as { + api: { + pty?: { + hasChildProcesses: typeof hasChildProcesses + getForegroundProcess: typeof getForegroundProcess + } + } + } + ).api.pty = { hasChildProcesses, getForegroundProcess } + const store = createCleanupTestStore() + store.setState({ + tabsByWorktree: { + 'repo1::/tmp/terminal': [ + { id: 'tab-1', title: 'zsh' } + ] as AppState['tabsByWorktree'][string] + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] } + } as Partial) + + const scanPromise = store.getState().scanWorkspaceCleanup() + onProgress?.({ + scanId: 'scan-1', + scannedAt: NOW, + scannedWorktreeCount: 1, + totalWorktreeCount: 2, + candidates: [terminalCandidate], + errors: [], + candidateMode: 'append' + }) + + await vi.waitFor(() => { + expect(store.getState().workspaceCleanupProgress?.scannedWorktreeCount).toBe(1) + }) + expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(getForegroundProcess).toHaveBeenCalledTimes(1) + + onProgress?.({ + scanId: 'scan-1', + scannedAt: NOW, + scannedWorktreeCount: 2, + totalWorktreeCount: 2, + candidates: [laterCandidate], + errors: [], + candidateMode: 'append' + }) + + await vi.waitFor(() => { + expect(store.getState().workspaceCleanupProgress?.scannedWorktreeCount).toBe(2) + }) + expect(store.getState().workspaceCleanupScan?.candidates).toHaveLength(2) + expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(getForegroundProcess).toHaveBeenCalledTimes(1) + + pending.resolve({ + scannedAt: NOW, + candidates: [terminalCandidate, laterCandidate], + errors: [] + }) + await scanPromise + + expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(getForegroundProcess).toHaveBeenCalledTimes(1) + }) + + it('does not join broad cleanup scans with different explicit args', async () => { + const firstPending = deferred() + const secondPending = deferred() + const firstResult = { + scannedAt: NOW, + candidates: [makeCandidate({ worktreeId: 'repo1::/tmp/first' })], + errors: [] + } + const secondResult = { + scannedAt: NOW + 1, + candidates: [makeCandidate({ worktreeId: 'repo1::/tmp/second' })], + errors: [] + } + const scan = vi.fn((args?: { skipGitWorktreeIds?: string[] }) => + args?.skipGitWorktreeIds?.includes('repo1::/tmp/first') + ? firstPending.promise + : secondPending.promise + ) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + + const first = store + .getState() + .scanWorkspaceCleanup({ skipGitWorktreeIds: ['repo1::/tmp/first'] }) + const second = store + .getState() + .scanWorkspaceCleanup({ skipGitWorktreeIds: ['repo1::/tmp/second'] }) + + expect(scan).toHaveBeenCalledTimes(2) + secondPending.resolve(secondResult) + await second + expect(store.getState().workspaceCleanupScan).toMatchObject(secondResult) + + firstPending.resolve(firstResult) + await expect(Promise.all([first, second])).resolves.toEqual([firstResult, secondResult]) + expect(store.getState().workspaceCleanupScan).toMatchObject(secondResult) + }) + + it('keeps stale cleanup results visible after a broad refresh failure', async () => { + const previous = { scannedAt: NOW, candidates: [makeCandidate()], errors: [] } + const scan = vi + .fn() + .mockResolvedValueOnce(previous) + .mockRejectedValueOnce(new Error('scan failed')) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + + await store.getState().scanWorkspaceCleanup() + await expect(store.getState().scanWorkspaceCleanup()).rejects.toThrow('scan failed') + + expect(store.getState().workspaceCleanupScan).toMatchObject(previous) + expect(store.getState().workspaceCleanupError).toBe('scan failed') + expect(store.getState().workspaceCleanupLoading).toBe(false) + }) + + it('keeps focused cleanup preflight scans separate from broad scans', async () => { + const broad = deferred() + const scan = vi.fn((args?: { worktreeId?: string }) => { + if (args?.worktreeId) { + return Promise.resolve({ + scannedAt: NOW + 1, + candidates: [makeCandidate({ worktreeId: args.worktreeId })], + errors: [] + } satisfies WorkspaceCleanupScanResult) + } + return broad.promise + }) + installWorkspaceCleanupApi(scan) + const store = createCleanupTestStore() + + const broadScan = store.getState().scanWorkspaceCleanup() + const focusedScan = await store.getState().scanWorkspaceCleanup({ worktreeId: WORKTREE_ID }) + + expect(scan).toHaveBeenCalledTimes(2) + expect(focusedScan.candidates[0]?.worktreeId).toBe(WORKTREE_ID) + expect(store.getState().workspaceCleanupScan).toBeNull() + + broad.resolve({ scannedAt: NOW, candidates: [], errors: [] }) + await broadScan + expect(store.getState().workspaceCleanupScan?.scannedAt).toBe(NOW) + }) + + it('preflights cleanup removals concurrently and deletes nested workspaces globally deepest first', async () => { + let activePreflights = 0 + let maxActivePreflights = 0 + let activeDeletes = 0 + let maxActiveDeletes = 0 + const deleteOrder: string[] = [] + const candidates = [ + makeCandidate({ + worktreeId: 'repo-a::/repo/parent', + repoId: 'repo-a', + path: '/repo/parent', + displayName: 'parent' + }), + makeCandidate({ + worktreeId: 'repo-b::/repo/parent/child', + repoId: 'repo-b', + path: '/repo/parent/child', + displayName: 'child' + }), + makeCandidate({ + worktreeId: 'repo-c::/other', + repoId: 'repo-c', + path: '/other', + displayName: 'other', + git: { clean: null, upstreamAhead: null, upstreamBehind: null, checkedAt: null }, + blockers: ['git-status-error'] + }) + ] + const candidateById = new Map(candidates.map((candidate) => [candidate.worktreeId, candidate])) + const scan = vi.fn(async (args?: { worktreeId?: string }) => { + activePreflights += 1 + maxActivePreflights = Math.max(maxActivePreflights, activePreflights) + await new Promise((resolve) => setTimeout(resolve, 5)) + activePreflights -= 1 + return { + scannedAt: NOW, + candidates: args?.worktreeId ? [candidateById.get(args.worktreeId)!] : [], + errors: [] + } satisfies WorkspaceCleanupScanResult + }) + installWorkspaceCleanupApi(scan) + + const removeWorktree = vi.fn(async (worktreeId: string) => { + deleteOrder.push(worktreeId) + activeDeletes += 1 + maxActiveDeletes = Math.max(maxActiveDeletes, activeDeletes) + await new Promise((resolve) => setTimeout(resolve, 5)) + activeDeletes -= 1 + return { ok: true as const } + }) + const store = createCleanupTestStore(removeWorktree) + store.setState({ + workspaceCleanupScan: { scannedAt: NOW, candidates, errors: [] } + } as Partial) + + await expect( + store + .getState() + .removeWorkspaceCleanupCandidates(candidates.map((candidate) => candidate.worktreeId)) + ).resolves.toEqual({ + removedIds: expect.arrayContaining(candidates.map((candidate) => candidate.worktreeId)), + failures: [] + }) + + expect(maxActivePreflights).toBeGreaterThan(1) + expect(maxActiveDeletes).toBe(1) + expect(deleteOrder).toEqual([ + 'repo-b::/repo/parent/child', + 'repo-a::/repo/parent', + 'repo-c::/other' + ]) + expect(removeWorktree).toHaveBeenCalledWith('repo-c::/other', true) + expect(store.getState().workspaceCleanupScan?.candidates).toEqual([]) + }) + it('demotes an active suggested workspace when it was not viewed from cleanup', async () => { const [candidate] = await enrichWorkspaceCleanupCandidates( [makeCandidate()], @@ -230,9 +592,12 @@ describe('workspace cleanup viewed rows', () => { await store.getState().scanWorkspaceCleanup() - expect(scan).toHaveBeenCalledWith({ - skipGitWorktreeIds: expect.arrayContaining([WORKTREE_ID, 'repo1::/tmp/terminal-workspace']) - }) + expect(scan).toHaveBeenCalledWith( + { + skipGitWorktreeIds: expect.arrayContaining([WORKTREE_ID, 'repo1::/tmp/terminal-workspace']) + }, + expect.any(Function) + ) }) it('does not defer git checks for focused remove preflights', async () => { diff --git a/src/renderer/src/store/slices/workspace-cleanup.ts b/src/renderer/src/store/slices/workspace-cleanup.ts index 159e0b50b40..357e4e74a74 100644 --- a/src/renderer/src/store/slices/workspace-cleanup.ts +++ b/src/renderer/src/store/slices/workspace-cleanup.ts @@ -1,4 +1,6 @@ -/* eslint-disable max-lines */ +/* eslint-disable max-lines -- Why: cleanup scan persistence, renderer safety + enrichment, dismissals, and destructive preflight/delete orchestration share + one store state contract. */ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import { @@ -16,6 +18,7 @@ import { type WorkspaceCleanupCandidate, type WorkspaceCleanupDismissal, type WorkspaceCleanupScanArgs, + type WorkspaceCleanupScanProgress, type WorkspaceCleanupScanResult } from '../../../../shared/workspace-cleanup' import { detectAgentStatusFromTitle, isExplicitAgentStatusFresh } from '@/lib/agent-status' @@ -40,6 +43,7 @@ type WorkspaceCleanupViewedCandidate = { export type WorkspaceCleanupSlice = { workspaceCleanupScan: WorkspaceCleanupScanResult | null + workspaceCleanupProgress: WorkspaceCleanupScanProgress | null workspaceCleanupLoading: boolean workspaceCleanupError: string | null workspaceCleanupDismissals: Record @@ -59,8 +63,25 @@ type EnrichOptions = { applyDismissals?: boolean } +type WorkspaceCleanupEnrichmentCacheEntry = { + inputSignature: string + localSignature: string + candidate: WorkspaceCleanupCandidate +} + const RECENT_VISIBLE_CONTEXT_MS = 24 * 60 * 60 * 1000 const VIEWED_FROM_CLEANUP_MS = 2 * 60 * 60 * 1000 +const WORKSPACE_CLEANUP_PREFLIGHT_CONCURRENCY = 4 + +let inFlightWorkspaceCleanupScan: { + key: string + promise: Promise +} | null = null +let latestWorkspaceCleanupScanToken = 0 +let workspaceCleanupEnrichmentCache: { + scanToken: number + entries: Map +} | null = null const SHELL_PROCESS_NAMES = new Set([ 'bash', @@ -96,35 +117,90 @@ export const createWorkspaceCleanupSlice: StateCreator ({ workspaceCleanupScan: null, + workspaceCleanupProgress: null, workspaceCleanupLoading: false, workspaceCleanupError: null, workspaceCleanupDismissals: {}, workspaceCleanupViewedCandidates: {}, scanWorkspaceCleanup: async (args) => { - set({ workspaceCleanupLoading: true, workspaceCleanupError: null }) - try { - const scanArgs = - args?.worktreeId !== undefined - ? args - : { - ...args, - skipGitWorktreeIds: [ - ...new Set([ - ...(args?.skipGitWorktreeIds ?? []), - ...getInitialWorkspaceCleanupGitDeferrals(get()) - ]) - ] - } - const scan = await window.api.workspaceCleanup.scan(scanArgs) - const enriched = await enrichWorkspaceCleanupCandidates(scan.candidates, get()) + if (args?.worktreeId !== undefined) { + const scan = await window.api.workspaceCleanup.scan(args) + const enriched = await enrichWorkspaceCleanupCandidates(scan.candidates, get(), { + applyDismissals: false + }) + return { ...scan, candidates: enriched } + } + + const scanArgs = { + ...args, + skipGitWorktreeIds: [ + ...new Set([ + ...(args?.skipGitWorktreeIds ?? []), + ...getInitialWorkspaceCleanupGitDeferrals(get()) + ]) + ] + } + const scanKey = getWorkspaceCleanupScanKey(scanArgs) + + if (inFlightWorkspaceCleanupScan?.key === scanKey) { + set({ workspaceCleanupLoading: true, workspaceCleanupError: null }) + try { + return await inFlightWorkspaceCleanupScan.promise + } finally { + if (!inFlightWorkspaceCleanupScan) { + set({ workspaceCleanupLoading: false }) + } + } + } + + set({ + workspaceCleanupLoading: true, + workspaceCleanupProgress: null, + workspaceCleanupError: null + }) + const scanToken = ++latestWorkspaceCleanupScanToken + workspaceCleanupEnrichmentCache = { scanToken, entries: new Map() } + const promise = (async () => { + const scan = await window.api.workspaceCleanup.scan(scanArgs, (progress) => { + void applyWorkspaceCleanupProgress(progress, scanToken, get, set) + }) + const enriched = await enrichWorkspaceCleanupCandidatesForScan( + scan.candidates, + get(), + scanToken + ) const result = { ...scan, candidates: enriched } - set({ workspaceCleanupScan: result, workspaceCleanupLoading: false }) + if (scanToken === latestWorkspaceCleanupScanToken) { + set({ + workspaceCleanupScan: result, + workspaceCleanupProgress: { + scanId: get().workspaceCleanupProgress?.scanId ?? '', + scannedAt: result.scannedAt, + scannedWorktreeCount: result.candidates.length, + totalWorktreeCount: result.candidates.length, + candidates: result.candidates, + errors: result.errors + }, + workspaceCleanupLoading: false + }) + } return result + })() + inFlightWorkspaceCleanupScan = { key: scanKey, promise } + + try { + return await promise } catch (error) { const message = error instanceof Error ? error.message : String(error) - set({ workspaceCleanupError: message, workspaceCleanupLoading: false }) + if (scanToken === latestWorkspaceCleanupScanToken) { + set({ workspaceCleanupError: message, workspaceCleanupLoading: false }) + } throw error + } finally { + if (inFlightWorkspaceCleanupScan?.promise === promise) { + inFlightWorkspaceCleanupScan = null + } } }, @@ -194,35 +270,47 @@ export const createWorkspaceCleanupSlice: StateCreator preflightWorkspaceCleanupCandidate(worktreeId, get) + ) + const candidatesToRemove: WorkspaceCleanupCandidate[] = [] + + for (const preflight of preflights) { if (!preflight.ok) { failures.push(preflight.failure) continue } + candidatesToRemove.push(preflight.candidate) + } + // Why: nested workspaces can belong to different repos; parent removal must + // not race child cleanup hooks, PTY teardown, or metadata deletion. + for (const candidate of [...candidatesToRemove].sort((a, b) => b.path.length - a.path.length)) { const result = await get().removeWorktree( - worktreeId, - shouldForceWorkspaceCleanupRemoval(preflight.candidate) + candidate.worktreeId, + shouldForceWorkspaceCleanupRemoval(candidate) ) if (result.ok) { - removedIds.push(worktreeId) + removedIds.push(candidate.worktreeId) } else { failures.push({ - worktreeId, - displayName: preflight.candidate.displayName, + worktreeId: candidate.worktreeId, + displayName: candidate.displayName, message: result.error }) } } if (removedIds.length > 0) { + const removedIdSet = new Set(removedIds) set((state) => ({ workspaceCleanupScan: state.workspaceCleanupScan ? { ...state.workspaceCleanupScan, candidates: state.workspaceCleanupScan.candidates.filter( - (candidate) => !removedIds.includes(candidate.worktreeId) + (candidate) => !removedIdSet.has(candidate.worktreeId) ) } : state.workspaceCleanupScan @@ -233,6 +321,116 @@ export const createWorkspaceCleanupSlice: StateCreator AppState, + setState: ( + partial: Partial | ((state: AppState) => Partial), + replace?: false + ) => void +): Promise { + const state = getState() + const previousCandidates = + progress.candidateMode === 'append' && + state.workspaceCleanupProgress?.scanId === progress.scanId + ? state.workspaceCleanupProgress.candidates + : [] + const enrichedProgressCandidates = await enrichWorkspaceCleanupCandidatesForScan( + progress.candidates, + state, + scanToken + ) + const candidates = + progress.candidateMode === 'append' + ? appendWorkspaceCleanupProgressCandidates(previousCandidates, enrichedProgressCandidates) + : enrichedProgressCandidates + if (scanToken !== latestWorkspaceCleanupScanToken) { + return + } + setState((state) => { + if ( + state.workspaceCleanupProgress?.scanId === progress.scanId && + state.workspaceCleanupProgress.scannedWorktreeCount > progress.scannedWorktreeCount + ) { + return {} + } + return { + workspaceCleanupScan: { + scannedAt: progress.scannedAt, + candidates, + errors: progress.errors + }, + workspaceCleanupProgress: { ...progress, candidates } + } + }) +} + +async function enrichWorkspaceCleanupCandidatesForScan( + candidates: readonly WorkspaceCleanupCandidate[], + state: AppState, + scanToken: number +): Promise { + if (workspaceCleanupEnrichmentCache?.scanToken !== scanToken) { + workspaceCleanupEnrichmentCache = { scanToken, entries: new Map() } + } + return enrichWorkspaceCleanupCandidatesWithCache( + candidates, + state, + workspaceCleanupEnrichmentCache.entries + ) +} + +function appendWorkspaceCleanupProgressCandidates( + previousCandidates: readonly WorkspaceCleanupCandidate[], + nextCandidates: readonly WorkspaceCleanupCandidate[] +): WorkspaceCleanupCandidate[] { + if (nextCandidates.length === 0) { + return [...previousCandidates] + } + + const indexesByWorktreeId = new Map( + previousCandidates.map((candidate, index) => [candidate.worktreeId, index]) + ) + const merged = [...previousCandidates] + for (const candidate of nextCandidates) { + const existingIndex = indexesByWorktreeId.get(candidate.worktreeId) + if (existingIndex === undefined) { + indexesByWorktreeId.set(candidate.worktreeId, merged.length) + merged.push(candidate) + continue + } + merged[existingIndex] = candidate + } + return merged +} + +async function mapWithConcurrency( + items: readonly T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const results: R[] = [] + let nextIndex = 0 + const workerCount = Math.min(limit, items.length) + await Promise.all( + Array.from({ length: workerCount }, async () => { + while (nextIndex < items.length) { + const index = nextIndex + nextIndex += 1 + results[index] = await fn(items[index]) + } + }) + ) + return results +} + function getInitialWorkspaceCleanupGitDeferrals(state: AppState): string[] { const ids = new Set() if (state.activeWorktreeId) { @@ -288,6 +486,106 @@ export async function enrichWorkspaceCleanupCandidates( ) } +async function enrichWorkspaceCleanupCandidatesWithCache( + candidates: readonly WorkspaceCleanupCandidate[], + state: AppState, + cache: Map, + options: EnrichOptions = {} +): Promise { + return Promise.all( + candidates.map(async (candidate) => { + const inputSignature = getWorkspaceCleanupCandidateInputSignature(candidate) + const localSignature = getWorkspaceCleanupLocalStateSignature( + candidate.worktreeId, + state, + options + ) + const cached = cache.get(candidate.worktreeId) + if (cached?.inputSignature === inputSignature && cached.localSignature === localSignature) { + return cached.candidate + } + + const enriched = await enrichWorkspaceCleanupCandidate(candidate, state, options) + cache.set(candidate.worktreeId, { + inputSignature, + localSignature, + candidate: enriched + }) + return enriched + }) + ) +} + +function getWorkspaceCleanupCandidateInputSignature(candidate: WorkspaceCleanupCandidate): string { + return JSON.stringify({ + fingerprint: candidate.fingerprint, + blockers: candidate.blockers, + reasons: candidate.reasons, + git: candidate.git, + lastActivityAt: candidate.lastActivityAt, + createdAt: candidate.createdAt, + path: candidate.path, + branch: candidate.branch + }) +} + +function getWorkspaceCleanupLocalStateSignature( + worktreeId: string, + state: AppState, + options: EnrichOptions +): string { + const tabs = state.tabsByWorktree[worktreeId] ?? [] + const tabIds = tabs.map((tab) => tab.id) + const tabIdSet = new Set(tabIds) + const openFiles = state.openFiles + .filter((file) => file.worktreeId === worktreeId) + .map((file) => ({ + id: file.id, + isDirty: file.isDirty, + hasDraft: state.editorDrafts[file.id] !== undefined + })) + const retainedDoneAgentPaneKeys = Object.entries(state.retainedAgentsByPaneKey) + .filter(([, entry]) => entry.worktreeId === worktreeId && entry.entry.state === 'done') + .map(([paneKey]) => paneKey) + .sort() + const agentStatuses = Object.values(state.agentStatusByPaneKey) + .filter((entry) => tabIdSet.has(getPaneKeyTabId(entry.paneKey))) + .map((entry) => ({ + paneKey: entry.paneKey, + state: entry.state, + updatedAt: entry.updatedAt + })) + .sort((a, b) => a.paneKey.localeCompare(b.paneKey)) + const ptyIdsByTabId = Object.fromEntries( + tabIds.map((tabId) => [tabId, state.ptyIdsByTabId[tabId] ?? []]) + ) + const runtimePaneTitlesByTabId = Object.fromEntries( + tabIds.map((tabId) => [tabId, state.runtimePaneTitlesByTabId[tabId] ?? {}]) + ) + const terminalLayoutsByTabId = Object.fromEntries( + tabIds.map((tabId) => [tabId, state.terminalLayoutsByTabId?.[tabId]?.ptyIdsByLeafId ?? {}]) + ) + const dismissal = + options.applyDismissals === false + ? null + : (state.workspaceCleanupDismissals[worktreeId] ?? null) + + return JSON.stringify({ + active: state.activeWorktreeId === worktreeId, + tabs: tabs.map((tab) => ({ id: tab.id, title: tab.title })), + ptyIdsByTabId, + runtimePaneTitlesByTabId, + terminalLayoutsByTabId, + openFiles, + browserTabCount: (state.browserTabsByWorktree[worktreeId] ?? []).length, + retainedDoneAgentPaneKeys, + agentStatuses, + lastVisitedAt: state.lastVisitedAtByWorktreeId[worktreeId] ?? 0, + viewed: state.workspaceCleanupViewedCandidates[worktreeId] ?? null, + dismissal + }) +} + async function enrichWorkspaceCleanupCandidate( candidate: WorkspaceCleanupCandidate, state: AppState, @@ -413,7 +711,7 @@ async function preflightWorkspaceCleanupCandidate( displayName: candidate.displayName, message: candidate.blockers.length ? candidate.blockers.join(', ') - : 'Workspace is no longer safe to remove.' + : 'Workspace needs another look before removal.' } } } diff --git a/src/shared/workspace-cleanup.ts b/src/shared/workspace-cleanup.ts index f751b7738a5..46ae7d1dd15 100644 --- a/src/shared/workspace-cleanup.ts +++ b/src/shared/workspace-cleanup.ts @@ -74,6 +74,7 @@ export type WorkspaceCleanupCandidate = { export type WorkspaceCleanupScanArgs = { worktreeId?: string skipGitWorktreeIds?: string[] + scanId?: string } export type WorkspaceCleanupLocalProcessArgs = { @@ -94,6 +95,13 @@ export type WorkspaceCleanupScanResult = { errors: WorkspaceCleanupScanError[] } +export type WorkspaceCleanupScanProgress = WorkspaceCleanupScanResult & { + scanId: string + scannedWorktreeCount: number + totalWorktreeCount: number + candidateMode?: 'append' | 'snapshot' +} + export type WorkspaceCleanupLocalProcessResult = { hasKillableProcesses: boolean | null } From 6015ad07a09934e3192ab9caa6f68c43f53ac1f9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 28 Jun 2026 11:46:59 -0700 Subject: [PATCH 02/12] Show working tree in file explorer for repos using a separate Git directory (#6482) Co-authored-by: Orca --- .../git/worktree-separate-git-dir.test.ts | 192 ++++++++++++++++++ src/main/git/worktree.ts | 112 +++++++++- src/relay/git-handler.test.ts | 91 +++++++++ src/relay/git-handler.ts | 122 ++++++++++- 4 files changed, 513 insertions(+), 4 deletions(-) create mode 100644 src/main/git/worktree-separate-git-dir.test.ts diff --git a/src/main/git/worktree-separate-git-dir.test.ts b/src/main/git/worktree-separate-git-dir.test.ts new file mode 100644 index 00000000000..2febdb09a76 --- /dev/null +++ b/src/main/git/worktree-separate-git-dir.test.ts @@ -0,0 +1,192 @@ +import { execFileSync } from 'child_process' +import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'fs/promises' +import { tmpdir } from 'os' +import * as path from 'path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type * as GitRunner from './runner' + +// Why: spy on the git runner so we can count rev-parse invocations while still +// running real git, proving the main-entry-equals-repoPath early return skips +// the extra subprocess for an ordinary repo scanned at its own root. +const revParseTopLevelCalls = { count: 0 } +vi.mock('./runner', async () => { + const actual = await vi.importActual('./runner') + return { + ...actual, + gitExecFileAsync: (args: string[], options?: unknown) => { + if (args[0] === 'rev-parse' && args.includes('--show-toplevel')) { + revParseTopLevelCalls.count += 1 + } + return (actual.gitExecFileAsync as (a: string[], o?: unknown) => Promise)( + args, + options + ) + } + } +}) + +// Imported after vi.mock so worktree.ts binds to the spied runner. +const { listWorktrees } = await import('./worktree') + +const tempRoots: string[] = [] + +function git(cwd: string, args: string[]): string { + return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }) +} + +async function createCommittedRepo(root: string, name: string): Promise { + const repoPath = path.join(root, name) + execFileSync('git', ['init', '--quiet', repoPath]) + git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main']) + git(repoPath, ['config', 'user.email', 'test@example.com']) + git(repoPath, ['config', 'user.name', 'Test User']) + await writeFile(path.join(repoPath, 'README.md'), `${name}\n`) + git(repoPath, ['add', 'README.md']) + git(repoPath, ['commit', '--quiet', '-m', 'initial']) + return realpath(repoPath) +} + +async function createSeparateGitDirRepo(): Promise<{ + gitDirPath: string + worktreePath: string +}> { + const root = await mkdtemp(path.join(tmpdir(), 'orca-separate-git-dir-')) + tempRoots.push(root) + const sourcePath = await createCommittedRepo(root, 'source') + const requestedWorktreePath = path.join(root, 'worktree') + const requestedGitDirPath = path.join(root, 'git-store', 'project.git') + await mkdir(path.dirname(requestedGitDirPath), { recursive: true }) + + execFileSync('git', [ + 'clone', + '--quiet', + `--separate-git-dir=${requestedGitDirPath}`, + sourcePath, + requestedWorktreePath + ]) + + return { + gitDirPath: await realpath(requestedGitDirPath), + worktreePath: await realpath(requestedWorktreePath) + } +} + +async function createNormalRepo(): Promise { + const root = await mkdtemp(path.join(tmpdir(), 'orca-normal-worktree-')) + tempRoots.push(root) + return createCommittedRepo(root, 'repo') +} + +async function createBareRepo(): Promise { + const root = await mkdtemp(path.join(tmpdir(), 'orca-bare-worktree-')) + tempRoots.push(root) + const repoPath = path.join(root, 'repo.git') + execFileSync('git', ['init', '--bare', '--quiet', repoPath]) + return realpath(repoPath) +} + +afterEach(async () => { + revParseTopLevelCalls.count = 0 + await Promise.all(tempRoots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('git worktree separate git dir paths', () => { + it.skipIf(process.platform === 'win32')( + 'normalizes the main worktree path for a separate-git-dir repo', + async () => { + const { gitDirPath, worktreePath } = await createSeparateGitDirRepo() + + const worktrees = await listWorktrees(worktreePath) + const mainWorktree = worktrees.find((worktree) => worktree.isMainWorktree) + + expect(mainWorktree).toMatchObject({ + path: worktreePath, + isMainWorktree: true + }) + expect(mainWorktree?.path).not.toBe(gitDirPath) + } + ) + + it.skipIf(process.platform === 'win32')( + 'fires exactly one rev-parse for a separate-git-dir repo', + async () => { + const { worktreePath } = await createSeparateGitDirRepo() + + await listWorktrees(worktreePath) + + expect(revParseTopLevelCalls.count).toBe(1) + } + ) + + it.skipIf(process.platform === 'win32')('leaves a normal repo main path unchanged', async () => { + const repoPath = await createNormalRepo() + + const worktrees = await listWorktrees(repoPath) + const mainWorktree = worktrees.find((worktree) => worktree.isMainWorktree) + + expect(mainWorktree).toMatchObject({ + path: repoPath, + isMainWorktree: true + }) + }) + + it.skipIf(process.platform === 'win32')( + 'leaves an ordinary repo reached via a symlinked path unchanged', + async () => { + // A symlink alias of the checkout root defeats the path-string gate + // (git reports the realpath toplevel), so the git-common-dir gate must + // still skip the rewrite since the main entry is a real working root. + const repoPath = await createNormalRepo() + const linkRoot = await mkdtemp(path.join(tmpdir(), 'orca-symlink-worktree-')) + tempRoots.push(linkRoot) + const linkedRepoPath = path.join(linkRoot, 'linked-repo') + await symlink(repoPath, linkedRepoPath) + + const worktrees = await listWorktrees(linkedRepoPath) + const mainWorktree = worktrees.find((worktree) => worktree.isMainWorktree) + + expect(mainWorktree).toMatchObject({ + path: repoPath, + isMainWorktree: true + }) + } + ) + + it.skipIf(process.platform === 'win32')( + 'leaves the main entry unchanged when scanned via a linked worktree', + async () => { + // A linked worktree has a `.git` *pointer file* just like a + // separate-git-dir checkout, but its porcelain main entry is the real + // main working root — never the Git directory. The git-common-dir gate + // must skip the rewrite so the main entry is not overwritten with the + // linked worktree's own toplevel (which would collide / drop the main). + const repoPath = await createNormalRepo() + const linkedWorktreePath = path.join(path.dirname(repoPath), 'linked') + git(repoPath, ['worktree', 'add', '--quiet', linkedWorktreePath, '-b', 'feature']) + const resolvedLinked = await realpath(linkedWorktreePath) + + const worktrees = await listWorktrees(resolvedLinked) + const mainWorktree = worktrees.find((worktree) => worktree.isMainWorktree) + + expect(mainWorktree).toMatchObject({ + path: repoPath, + isMainWorktree: true + }) + // The main entry must not be rewritten to the linked worktree's path. + expect(mainWorktree?.path).not.toBe(resolvedLinked) + } + ) + + it.skipIf(process.platform === 'win32')('does not throw for a bare repo', async () => { + const repoPath = await createBareRepo() + + const worktrees = await listWorktrees(repoPath) + const mainWorktree = worktrees.find((worktree) => worktree.isMainWorktree) + + expect(mainWorktree).toMatchObject({ + path: repoPath, + isBare: true, + isMainWorktree: true + }) + }) +}) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index f27ae1df9a7..d225c380b31 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -14,6 +14,7 @@ import type { RemoveWorktreeResult } from '../../shared/types' import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output' +import { parseWslUncPath } from '../../shared/wsl-paths' import { gitExecFileAsync, translateWslOutputPaths } from './runner' import { resolveGitDir } from './status' import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe' @@ -108,6 +109,12 @@ function isUnsupportedWorktreeListZError(error: unknown): boolean { ) } +function isUnsupportedRevParsePathFormatError(error: unknown): boolean { + return /(?:unknown|invalid|unrecognized).*(?:--path-format|path-format)/i.test( + getErrorText(error) + ) +} + function isBranchCheckedOutInWorktreeError(error: unknown): boolean { return /cannot delete branch .*(?:used by worktree|checked out)|branch .*is checked out/i.test( getErrorText(error) @@ -353,6 +360,103 @@ function looksLikeWindowsPath(pathValue: string): boolean { return /^[A-Za-z]:[\\/]/.test(pathValue) || pathValue.startsWith('\\\\') } +function resolveRevParsePath(repoPath: string, value: string): string { + if (posix.isAbsolute(value) || win32.isAbsolute(value)) { + return value + } + // Old git ignores `--path-format=absolute`, so a relative toplevel/git-dir + // must be resolved against the scanned repo path. + return looksLikeWindowsPath(repoPath) + ? win32.resolve(repoPath, value) + : posix.resolve(repoPath, value) +} + +type RepoLocation = { topLevel: string; commonDir: string } + +function parseRepoLocation(repoPath: string, output: string): RepoLocation | undefined { + // Old git (pre `--path-format`) echoes the unrecognized flag to stdout and + // exits 0 rather than erroring, so drop any echoed `-`-prefixed lines and + // read the two trailing path lines (toplevel, then git-common-dir). Strip only + // the trailing CR, not surrounding spaces — git paths may legitimately start + // or end with a space. + const lines = output + .split('\n') + .map((line) => (line.endsWith('\r') ? line.slice(0, -1) : line)) + .filter((line) => line.length > 0 && !line.startsWith('-')) + if (lines.length < 2) { + return undefined + } + const [topLevel, commonDir] = lines.slice(-2) + return { + topLevel: resolveRevParsePath(repoPath, topLevel), + commonDir: resolveRevParsePath(repoPath, commonDir) + } +} + +async function readRepoLocation( + repoPath: string, + resolveBasePath: string, + options: GitWorktreeExecOptions = {} +): Promise { + try { + const { stdout } = await gitExecFileAsync( + ['rev-parse', '--path-format=absolute', '--show-toplevel', '--git-common-dir'], + gitExecOptions(repoPath, options) + ) + return parseRepoLocation(resolveBasePath, stdout) + } catch (error) { + if (!isUnsupportedRevParsePathFormatError(error)) { + return undefined + } + } + + try { + const { stdout } = await gitExecFileAsync( + ['rev-parse', '--show-toplevel', '--git-common-dir'], + gitExecOptions(repoPath, options) + ) + return parseRepoLocation(resolveBasePath, stdout) + } catch { + return undefined + } +} + +async function normalizeMainWorktreePath( + repoPath: string, + worktrees: GitWorktreeInfo[], + options: GitWorktreeExecOptions = {} +): Promise { + const mainIndex = worktrees.findIndex((worktree) => worktree.isMainWorktree) + const mainWorktree = worktrees[mainIndex] + // Compare in the Git-output space: under WSL the porcelain/rev-parse paths are + // Linux while repoPath is a UNC path, so without translating, a UNC repoPath + // never matches the early-return and fires a needless rev-parse on every poll. + // Use the platform-independent UNC parser so the comparison holds regardless + // of host OS; the runner still receives the original repoPath for WSL routing. + const wslRepo = parseWslUncPath(repoPath) + const comparablePath = wslRepo ? wslRepo.linuxPath : repoPath + if (!mainWorktree || areWorktreePathsEqual(mainWorktree.path, comparablePath)) { + return worktrees + } + + const location = await readRepoLocation(repoPath, comparablePath, options) + if (!location) { + return worktrees + } + + // Why: only a separate-git-dir/submodule main worktree reports the Git + // directory as the main entry — i.e. the main entry equals git-common-dir. + // A linked worktree's main entry is a real working root, so gating on this + // equality avoids overwriting it with the linked worktree's own toplevel. + if (!areWorktreePathsEqual(mainWorktree.path, location.commonDir)) { + return worktrees + } + + const normalized = [...worktrees] + normalized[mainIndex] = { ...mainWorktree, path: location.topLevel } + return normalized +} + /** * Parse the porcelain output of `git worktree list --porcelain`. */ @@ -446,7 +550,11 @@ async function readWorktreeList( cwd: repoPath, ...options }) - return parseWorktreeList(stdout, { nulDelimited: true }) + return normalizeMainWorktreePath( + repoPath, + parseWorktreeList(stdout, { nulDelimited: true }), + options + ) } catch (error) { if (!isUnsupportedWorktreeListZError(error)) { throw error @@ -459,7 +567,7 @@ async function readWorktreeList( cwd: repoPath, ...options }) - return parseWorktreeList(stdout) + return normalizeMainWorktreePath(repoPath, parseWorktreeList(stdout), options) } async function readTranslatedWorktreeGraph( diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index 8e1c3ba65dd..90b516a947e 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -1660,6 +1660,97 @@ describe('GitHandler', () => { expect(result[0].isMainWorktree).toBe(true) }) + it.skipIf(process.platform === 'win32')( + 'normalizes the main worktree path for a separate-git-dir repo', + async () => { + const sourcePath = path.join(tmpDir, 'source') + const worktreePath = path.join(tmpDir, 'worktree') + const gitDirPath = path.join(tmpDir, 'git-store', 'project.git') + mkdirSync(sourcePath) + mkdirSync(path.dirname(gitDirPath), { recursive: true }) + gitInit(sourcePath) + writeFileSync(path.join(sourcePath, 'file.txt'), 'hello') + gitCommit(sourcePath, 'initial') + + execFileSync('git', [ + 'clone', + '--quiet', + `--separate-git-dir=${gitDirPath}`, + sourcePath, + worktreePath + ]) + + const result = (await dispatcher.callRequest('git.listWorktrees', { + repoPath: await fs.realpath(worktreePath) + })) as Record[] + const mainWorktree = result.find((worktree) => worktree.isMainWorktree === true) + + expect(mainWorktree).toMatchObject({ + path: await fs.realpath(worktreePath), + isMainWorktree: true + }) + expect(mainWorktree?.path).not.toBe(await fs.realpath(gitDirPath)) + } + ) + + it.skipIf(process.platform === 'win32')( + 'leaves an ordinary repo reached via a symlinked path unchanged', + async () => { + // A symlink alias defeats the path-string gate (git reports the + // realpath toplevel); the git-common-dir gate must still skip the + // rewrite for an ordinary repo so the reported path is untouched. + const repoPath = path.join(tmpDir, 'plain-repo') + mkdirSync(repoPath) + gitInit(repoPath) + writeFileSync(path.join(repoPath, 'file.txt'), 'hello') + gitCommit(repoPath, 'initial') + const linkedRepoPath = path.join(tmpDir, 'linked-repo') + symlinkSync(repoPath, linkedRepoPath) + + const result = (await dispatcher.callRequest('git.listWorktrees', { + repoPath: linkedRepoPath + })) as Record[] + const mainWorktree = result.find((worktree) => worktree.isMainWorktree === true) + + expect(mainWorktree).toMatchObject({ + path: await fs.realpath(repoPath), + isMainWorktree: true + }) + } + ) + + it.skipIf(process.platform === 'win32')( + 'leaves the main entry unchanged when scanned via a linked worktree', + async () => { + // A linked worktree has a `.git` pointer file like a separate-git-dir + // checkout, but its porcelain main entry is the real main working root. + // The git-common-dir gate must skip the rewrite so the main entry is + // not overwritten with the linked worktree's own toplevel. + const repoPath = path.join(tmpDir, 'main-repo') + mkdirSync(repoPath) + gitInit(repoPath) + writeFileSync(path.join(repoPath, 'file.txt'), 'hello') + gitCommit(repoPath, 'initial') + const linkedWorktreePath = path.join(tmpDir, 'linked-wt') + execFileSync('git', ['worktree', 'add', '--quiet', linkedWorktreePath, '-b', 'feature'], { + cwd: repoPath, + stdio: 'pipe' + }) + const resolvedLinked = await fs.realpath(linkedWorktreePath) + + const result = (await dispatcher.callRequest('git.listWorktrees', { + repoPath: resolvedLinked + })) as Record[] + const mainWorktree = result.find((worktree) => worktree.isMainWorktree === true) + + expect(mainWorktree).toMatchObject({ + path: await fs.realpath(repoPath), + isMainWorktree: true + }) + expect(mainWorktree?.path).not.toBe(resolvedLinked) + } + ) + it.skipIf(process.platform === 'win32')( 'lists worktrees whose paths contain newlines', async () => { diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index e3310f969bc..31c796418f3 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -20,6 +20,7 @@ import { } from './git-handler-ops' import { commitCompare as commitCompareOp, commitDiffEntry } from './git-handler-commit-diff-ops' import { + areRelayWorktreePathsEqual, commitChangesRelay, addWorktreeOp, removeWorktreeOp, @@ -52,6 +53,64 @@ const execFileAsync = promisify(execFile) const MAX_GIT_BUFFER = 10 * 1024 * 1024 const BULK_CHUNK_SIZE = 100 +function getErrorText(error: unknown): string { + if (typeof error === 'object' && error !== null) { + const parts: string[] = [] + if ('message' in error && typeof error.message === 'string') { + parts.push(error.message) + } + if ('stderr' in error && typeof error.stderr === 'string') { + parts.push(error.stderr) + } + return parts.join('\n') + } + return String(error) +} + +function isUnsupportedRevParsePathFormatError(error: unknown): boolean { + return /(?:unknown|invalid|unrecognized).*(?:--path-format|path-format)/i.test( + getErrorText(error) + ) +} + +function isWindowsAbsolutePath(value: string): boolean { + return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith('\\\\') +} + +function resolveRelayPath(repoPath: string, value: string): string { + if (path.posix.isAbsolute(value) || path.win32.isAbsolute(value)) { + return value + } + // Old git ignores `--path-format=absolute`, so a relative toplevel/git-dir + // must be resolved against the scanned repo path. Mirror worktree.ts's + // resolveRevParsePath: pick the win32/posix resolver from the repoPath shape. + return isWindowsAbsolutePath(repoPath) + ? path.win32.resolve(repoPath, value) + : path.posix.resolve(repoPath, value) +} + +type RelayRepoLocation = { topLevel: string; commonDir: string } + +function parseRelayRepoLocation(repoPath: string, output: string): RelayRepoLocation | undefined { + // Old git (pre `--path-format`) echoes the unrecognized flag to stdout and + // exits 0 rather than erroring, so drop any echoed `-`-prefixed lines and + // read the two trailing path lines (toplevel, then git-common-dir). Strip only + // the trailing CR, not surrounding spaces — git paths may legitimately start + // or end with a space. + const lines = output + .split('\n') + .map((line) => (line.endsWith('\r') ? line.slice(0, -1) : line)) + .filter((line) => line.length > 0 && !line.startsWith('-')) + if (lines.length < 2) { + return undefined + } + const [topLevel, commonDir] = lines.slice(-2) + return { + topLevel: resolveRelayPath(repoPath, topLevel), + commonDir: resolveRelayPath(repoPath, commonDir) + } +} + function execFileWithStdin( command: string, args: string[], @@ -998,11 +1057,70 @@ export class GitHandler { } } + private async readRepoLocation(repoPath: string): Promise { + try { + const { stdout } = await this.git( + ['rev-parse', '--path-format=absolute', '--show-toplevel', '--git-common-dir'], + repoPath + ) + return parseRelayRepoLocation(repoPath, stdout) + } catch (error) { + if (!isUnsupportedRevParsePathFormatError(error)) { + return undefined + } + } + + try { + const { stdout } = await this.git( + ['rev-parse', '--show-toplevel', '--git-common-dir'], + repoPath + ) + return parseRelayRepoLocation(repoPath, stdout) + } catch { + return undefined + } + } + + private async normalizeMainWorktreePath( + repoPath: string, + worktrees: Record[] + ): Promise[]> { + const mainIndex = worktrees.findIndex((worktree) => worktree.isMainWorktree === true) + const mainWorktree = worktrees[mainIndex] + const mainPath = typeof mainWorktree?.path === 'string' ? mainWorktree.path : '' + // Expand `~` so the early-return matches git's absolute porcelain path for + // legacy SSH repos stored with a tilde, sparing them a rev-parse per poll. + const resolvedRepoPath = expandTilde(repoPath) + if (!mainPath || areRelayWorktreePathsEqual(mainPath, resolvedRepoPath)) { + return worktrees + } + + const location = await this.readRepoLocation(resolvedRepoPath) + if (!location) { + return worktrees + } + + // Why: only a separate-git-dir/submodule main worktree reports the Git + // directory as the main entry — i.e. the main entry equals git-common-dir. + // A linked worktree's main entry is a real working root, so gating on this + // equality avoids overwriting it with the linked worktree's own toplevel. + if (!areRelayWorktreePathsEqual(mainPath, location.commonDir)) { + return worktrees + } + + const normalized = [...worktrees] + normalized[mainIndex] = { ...mainWorktree, path: location.topLevel } + return normalized + } + private async listWorktrees(params: Record) { const repoPath = params.repoPath as string try { const { stdout } = await this.git(['worktree', 'list', '--porcelain', '-z'], repoPath) - return parseWorktreeList(stdout, { nulDelimited: true }) + return this.normalizeMainWorktreePath( + repoPath, + parseWorktreeList(stdout, { nulDelimited: true }) + ) } catch (error) { if (!isUnsupportedWorktreeListZError(error)) { return [] @@ -1013,7 +1131,7 @@ export class GitHandler { // Git rejects it. Fall back to the original line-block parser there. try { const { stdout } = await this.git(['worktree', 'list', '--porcelain'], repoPath) - return parseWorktreeList(stdout) + return this.normalizeMainWorktreePath(repoPath, parseWorktreeList(stdout)) } catch { return [] } From e58a5e9e1710bd072a1b4126daafc6916abd67e2 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 28 Jun 2026 11:50:29 -0700 Subject: [PATCH 03/12] Simplify Appearance settings (#6459) Co-authored-by: Orca --- config/scripts/trim-windows-icon-source.mjs | 5 +- src/main/ipc/pty.test.ts | 3 +- .../local-pty-utils-windows-fallback.test.ts | 3 +- .../windows-powershell-executable.test.ts | 6 +- .../packaged-cli-entry-redirect.test.ts | 26 +- .../settings/AppearanceAdvancedDisclosure.tsx | 56 ++ .../settings/AppearanceInterfaceSection.tsx | 241 ++++++ .../settings/AppearancePane.test.tsx | 163 ++++- .../components/settings/AppearancePane.tsx | 691 +++++------------- .../components/settings/AppearanceSection.tsx | 78 ++ .../settings/AppearanceShortcutHintList.tsx | 34 + .../AppearanceWindowSidebarSection.tsx | 307 ++++++++ .../settings/SettingsFormControls.tsx | 22 +- .../TerminalAdvancedTypographyControls.tsx | 188 +++++ .../TerminalAppearanceSection.ghostty.test.ts | 90 ++- .../settings/TerminalAppearanceSection.tsx | 261 +++++-- .../TerminalCursorAppearanceSection.tsx | 20 +- .../settings/TerminalFontSizeSetting.tsx | 11 +- .../TerminalPaneAppearanceSection.tsx | 16 +- .../settings/TerminalThemeSections.tsx | 32 +- .../settings/TerminalWindowSection.tsx | 502 +++++++------ .../components/settings/appearance-search.ts | 20 + .../settings/appearance-sidebar-search.ts | 2 +- .../settings/terminal-search.test.ts | 2 + .../components/settings/terminal-search.ts | 1 + .../settings/terminal-typography-search.ts | 10 +- .../terminal-pane/pty-connection.test.ts | 343 +++++++++ .../terminal-pane/pty-connection.ts | 146 +++- src/renderer/src/i18n/locales/en.json | 20 +- src/renderer/src/i18n/locales/es.json | 22 +- src/renderer/src/i18n/locales/ja.json | 22 +- src/renderer/src/i18n/locales/ko.json | 20 +- src/renderer/src/i18n/locales/zh.json | 22 +- src/shared/agent-process-recognition.test.ts | 4 +- src/shared/keybindings.test.ts | 6 +- 35 files changed, 2393 insertions(+), 1002 deletions(-) create mode 100644 src/renderer/src/components/settings/AppearanceAdvancedDisclosure.tsx create mode 100644 src/renderer/src/components/settings/AppearanceInterfaceSection.tsx create mode 100644 src/renderer/src/components/settings/AppearanceSection.tsx create mode 100644 src/renderer/src/components/settings/AppearanceShortcutHintList.tsx create mode 100644 src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx create mode 100644 src/renderer/src/components/settings/TerminalAdvancedTypographyControls.tsx diff --git a/config/scripts/trim-windows-icon-source.mjs b/config/scripts/trim-windows-icon-source.mjs index 90a1914e733..a6208965715 100644 --- a/config/scripts/trim-windows-icon-source.mjs +++ b/config/scripts/trim-windows-icon-source.mjs @@ -214,6 +214,9 @@ function main() { console.log(` -> ${outputIco} (filled multi-size ICO, safe-area inset trimmed)`) } -if (import.meta.url === `file://${process.argv[1]}` || process.argv[1]?.endsWith('trim-windows-icon-source.mjs')) { +if ( + import.meta.url === `file://${process.argv[1]}` || + process.argv[1]?.endsWith('trim-windows-icon-source.mjs') +) { main() } diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index d20e5905ecb..4b9ce9508b7 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -204,8 +204,7 @@ const POWERSHELL_OSC133_ARGS = [ // real absolute executable before handing it to ConPTY (PR #6537 / issue // #5161) — a bare/alias `pwsh.exe` makes CreateProcessW fail with error code 5. // These match the deterministic install roots pinned in the win32 beforeEach. -const RESOLVED_WINDOWS_POWERSHELL = - 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' +const RESOLVED_WINDOWS_POWERSHELL = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' const RESOLVED_PWSH7 = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe' const TEST_CODEX_HOME = process.platform === 'win32' diff --git a/src/main/providers/local-pty-utils-windows-fallback.test.ts b/src/main/providers/local-pty-utils-windows-fallback.test.ts index 97fd3b6ea8f..48b35cd27c0 100644 --- a/src/main/providers/local-pty-utils-windows-fallback.test.ts +++ b/src/main/providers/local-pty-utils-windows-fallback.test.ts @@ -39,8 +39,7 @@ function makeAttempt( // error code 5 == ERROR_ACCESS_DENIED from CreateProcessW inside ConPTY when a // bare/alias pwsh.exe is handed to node-pty. -const ACCESS_DENIED_5 = - 'Cannot create process, error code: 5' +const ACCESS_DENIED_5 = 'Cannot create process, error code: 5' describe('spawnShellWithFallback on Windows', () => { it('repro: recovers when the primary PowerShell spawn fails with error code 5', () => { diff --git a/src/main/providers/windows-powershell-executable.test.ts b/src/main/providers/windows-powershell-executable.test.ts index 28400332c01..dbdb652ae65 100644 --- a/src/main/providers/windows-powershell-executable.test.ts +++ b/src/main/providers/windows-powershell-executable.test.ts @@ -14,12 +14,10 @@ const WIN_ENV: NodeJS.ProcessEnv = { } const PWSH7 = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe' -const WINDOWS_POWERSHELL = - 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' +const WINDOWS_POWERSHELL = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' // The Microsoft Store App Execution Alias stub for pwsh — a zero-byte reparse // point under WindowsApps that ConPTY's CreateProcessW rejects with error 5. -const PWSH_STORE_ALIAS = - 'C:\\Users\\dev\\AppData\\Local\\Microsoft\\WindowsApps\\pwsh.exe' +const PWSH_STORE_ALIAS = 'C:\\Users\\dev\\AppData\\Local\\Microsoft\\WindowsApps\\pwsh.exe' describe('resolveWindowsPowerShellExecutablePath', () => { it('returns null on non-Windows platforms', () => { diff --git a/src/main/startup/packaged-cli-entry-redirect.test.ts b/src/main/startup/packaged-cli-entry-redirect.test.ts index f892ae6c4da..4f91657eca0 100644 --- a/src/main/startup/packaged-cli-entry-redirect.test.ts +++ b/src/main/startup/packaged-cli-entry-redirect.test.ts @@ -29,7 +29,9 @@ describe('packaged CLI entry redirect', () => { }) it('does not match the entrypoint on non-Windows platforms', () => { - expect(getPackagedCliEntryArgs([execPath, cliEntryPath, 'status'], cliEntryPath, 'linux')).toBeNull() + expect( + getPackagedCliEntryArgs([execPath, cliEntryPath, 'status'], cliEntryPath, 'linux') + ).toBeNull() }) it('spawns the in-package CLI in Electron node mode before the single-instance lock can win', () => { @@ -50,19 +52,15 @@ describe('packaged CLI entry redirect', () => { }) expect(result).toEqual({ redirected: true, status: 0 }) - expect(spawn).toHaveBeenCalledWith( - execPath, - [cliEntryPath, 'status', '--json'], - { - env: expect.objectContaining({ - ELECTRON_RUN_AS_NODE: '1', - ORCA_PACKAGED_CLI_ENTRY_REDIRECTED: '1', - ORCA_NODE_OPTIONS: '--inspect', - ORCA_NODE_REPL_EXTERNAL_MODULE: 'external-loader' - }), - stdio: 'inherit' - } - ) + expect(spawn).toHaveBeenCalledWith(execPath, [cliEntryPath, 'status', '--json'], { + env: expect.objectContaining({ + ELECTRON_RUN_AS_NODE: '1', + ORCA_PACKAGED_CLI_ENTRY_REDIRECTED: '1', + ORCA_NODE_OPTIONS: '--inspect', + ORCA_NODE_REPL_EXTERNAL_MODULE: 'external-loader' + }), + stdio: 'inherit' + }) const spawnOptions = spawn.mock.calls[0]?.[2] as { env: NodeJS.ProcessEnv } | undefined expect(spawnOptions?.env).not.toHaveProperty('NODE_OPTIONS') expect(spawnOptions?.env).not.toHaveProperty('NODE_REPL_EXTERNAL_MODULE') diff --git a/src/renderer/src/components/settings/AppearanceAdvancedDisclosure.tsx b/src/renderer/src/components/settings/AppearanceAdvancedDisclosure.tsx new file mode 100644 index 00000000000..73f5c668208 --- /dev/null +++ b/src/renderer/src/components/settings/AppearanceAdvancedDisclosure.tsx @@ -0,0 +1,56 @@ +import type React from 'react' +import { useState } from 'react' +import { ChevronRight } from 'lucide-react' +import { cn } from '@/lib/utils' +import { useAppStore } from '../../store' +import { normalizeSettingsSearchQuery } from './settings-search' +import { translate } from '@/i18n/i18n' + +type AppearanceAdvancedDisclosureProps = { + /** Optional override label; defaults to "Advanced". */ + label?: string + showTopBorder?: boolean + className?: string + contentClassName?: string + children: React.ReactNode +} + +/** Inline "Advanced" disclosure for low-frequency controls. An active settings + * search force-opens it so matching controls stay reachable instead of being + * hidden behind a collapsed trigger. */ +export function AppearanceAdvancedDisclosure({ + label, + showTopBorder = true, + className, + contentClassName, + children +}: AppearanceAdvancedDisclosureProps): React.JSX.Element { + const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const isSearching = normalizeSettingsSearchQuery(searchQuery).length > 0 + const [open, setOpen] = useState(false) + const expanded = open || isSearching + + return ( +
+ + {expanded ?
{children}
: null} +
+ ) +} diff --git a/src/renderer/src/components/settings/AppearanceInterfaceSection.tsx b/src/renderer/src/components/settings/AppearanceInterfaceSection.tsx new file mode 100644 index 00000000000..0f691a072dd --- /dev/null +++ b/src/renderer/src/components/settings/AppearanceInterfaceSection.tsx @@ -0,0 +1,241 @@ +import type React from 'react' + +import type { GlobalSettings } from '../../../../shared/types' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select' +import { UIZoomControl } from './UIZoomControl' +import { SearchableSetting } from './SearchableSetting' +import { AppearanceAdvancedDisclosure } from './AppearanceAdvancedDisclosure' +import { useAppStore } from '../../store' +import { useShortcutKeyComboDetails } from '@/hooks/useShortcutLabel' +import { ShortcutHintList } from './AppearanceShortcutHintList' +import { + FontAutocomplete, + SettingsRow, + SettingsSegmentedControl, + SettingsSwitchRow +} from './SettingsFormControls' +import { DEFAULT_APP_FONT_FAMILY } from '../../../../shared/constants' +import { + getLanguageEntries, + getSystemTrayEntries, + getThemeEntries, + getTitlebarEntries, + getTypographyEntries, + getZoomEntries +} from './appearance-search' +import { + getUiLanguageChoiceLabel, + SHOW_UI_LANGUAGE_SETTING, + UI_LANGUAGE_CHOICES +} from '@/i18n/supported-languages' +import { translate } from '@/i18n/i18n' +import type { UiLanguage } from '../../../../shared/ui-language' +import { matchesSettingsSearch, normalizeSettingsSearchQuery } from './settings-search' + +type AppearanceInterfaceSectionProps = { + settings: GlobalSettings + updateSettings: (updates: Partial) => void + applyTheme: (theme: 'system' | 'dark' | 'light') => void + fontSuggestions: string[] + isDesktopWindows: boolean + forceVisiblePrimary?: boolean +} + +export function AppearanceInterfaceSection({ + settings, + updateSettings, + applyTheme, + fontSuggestions, + isDesktopWindows, + forceVisiblePrimary = false +}: AppearanceInterfaceSectionProps): React.JSX.Element { + const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const isSearching = normalizeSettingsSearchQuery(searchQuery).length > 0 + const zoomInKeyCombos = useShortcutKeyComboDetails('zoom.in') + const zoomOutKeyCombos = useShortcutKeyComboDetails('zoom.out') + const languageEntry = getLanguageEntries()[0] + const systemTrayEntry = getSystemTrayEntries({ showSystemTray: true })[0] + const themeEntry = getThemeEntries()[0] + const themeLabel = translate('auto.components.settings.AppearancePane.932ff1fbff', 'Theme') + const titlebarEntry = getTitlebarEntries()[0] + const typographyEntry = getTypographyEntries()[0] + const zoomEntry = getZoomEntries()[0] + const advancedEntries = [ + ...(SHOW_UI_LANGUAGE_SETTING ? getLanguageEntries() : []), + ...getTitlebarEntries(), + ...getSystemTrayEntries({ showSystemTray: isDesktopWindows }) + ] + const showAdvanced = !isSearching || matchesSettingsSearch(searchQuery, advancedEntries) + + return ( +
+ + { + updateSettings({ theme: option }) + applyTheme(option) + }} + options={[ + { + value: 'system', + label: translate('auto.components.settings.AppearancePane.fb0e0b4453', 'System') + }, + { + value: 'dark', + label: translate('auto.components.settings.AppearancePane.7d26ccabe8', 'Dark') + }, + { + value: 'light', + label: translate('auto.components.settings.AppearancePane.fd89b5487c', 'Light') + } + ]} + /> + } + /> + + + + + /{' '} + {' '} + {translate( + 'auto.components.settings.AppearancePane.ef89200c1f', + 'when not in a terminal pane.' + )} + + } + control={} + /> + + + + + updateSettings({ appFontFamily: value.trim() || DEFAULT_APP_FONT_FAMILY }) + } + /> + } + /> + + + {showAdvanced ? ( + +
+ {SHOW_UI_LANGUAGE_SETTING ? ( + + updateSettings({ uiLanguage: value as UiLanguage })} + > + + + + + {UI_LANGUAGE_CHOICES.map((choice) => ( + + {getUiLanguageChoiceLabel(choice, translate)} + + ))} + + + } + /> + + ) : null} + + + + updateSettings({ showTitlebarAppName: !settings.showTitlebarAppName }) + } + /> + + + {isDesktopWindows ? ( + + + updateSettings({ minimizeToTrayOnClose: !settings.minimizeToTrayOnClose }) + } + /> + + ) : null} +
+ + ) : null} +
+ ) +} diff --git a/src/renderer/src/components/settings/AppearancePane.test.tsx b/src/renderer/src/components/settings/AppearancePane.test.tsx index a5d0c0cdea9..e26cffcccde 100644 --- a/src/renderer/src/components/settings/AppearancePane.test.tsx +++ b/src/renderer/src/components/settings/AppearancePane.test.tsx @@ -10,10 +10,18 @@ import type { GlobalSettings } from '../../../../shared/types' const mocks = vi.hoisted(() => ({ state: { + availableStatusBarToggles: [] as { + description: string + id: 'ports' + keywords: string[] + title: string + toggleDescription: string + }[], settingsSearchQuery: 'automations', statusBarItems: [], toggleStatusBarItem: vi.fn(), - recordFeatureInteraction: vi.fn() + recordFeatureInteraction: vi.fn(), + setWorktreeCardMode: vi.fn() } })) @@ -26,7 +34,7 @@ vi.mock('@/hooks/useShortcutLabel', () => ({ })) vi.mock('../status-bar/use-available-status-bar-toggles', () => ({ - useAvailableStatusBarToggles: () => [] + useAvailableStatusBarToggles: () => mocks.state.availableStatusBarToggles })) vi.mock('./TerminalAppearanceSection', () => ({ @@ -85,6 +93,7 @@ vi.mock('../ui/select', async () => { }) import { AppearancePane } from './AppearancePane' +import { TooltipProvider } from '../ui/tooltip' const mountedRoots: Root[] = [] @@ -130,16 +139,18 @@ async function renderAppearancePane( await act(async () => { root.render( - + + + ) }) @@ -159,7 +170,22 @@ describe('AppearancePane', () => { beforeEach(() => { vi.clearAllMocks() + mocks.state.availableStatusBarToggles = [] mocks.state.settingsSearchQuery = 'automations' + // UIZoomControl reads window.api.ui on mount; the inline-expansion pane can + // render the full Interface section, so provide a minimal renderer bridge + // without clobbering happy-dom's window.location. + ;(window as unknown as { api: unknown }).api = { + ui: { + getZoomLevel: () => 0, + onTerminalZoom: () => () => {}, + set: vi.fn() + } + } + }) + + afterEach(() => { + delete (window as unknown as { api?: unknown }).api }) it('renders the language dropdown with system, english, chinese, korean, japanese, and spanish options', async () => { @@ -236,4 +262,117 @@ describe('AppearancePane', () => { expect(updateSettings).toHaveBeenCalledWith({ showAutomationsButton: true }) }) + + it('changes workspace card layout from the Appearance sidebar controls', async () => { + mocks.state.settingsSearchQuery = 'workspace card layout' + const settings = { + ...getDefaultSettings('/tmp'), + compactWorktreeCards: false + } + + const container = await renderAppearancePane(settings) + const compactButton = Array.from( + container.querySelectorAll('button[role="radio"]') + ).find((button) => button.textContent === 'Compact') + + expect(compactButton).toBeDefined() + + await act(async () => { + compactButton?.dispatchEvent(new MouseEvent('click', { bubbles: true })) + }) + + expect(mocks.state.setWorktreeCardMode).toHaveBeenCalledWith('Compact') + }) + + it('renders the three top-level section rows and no Code & Markdown row when not searching', async () => { + mocks.state.settingsSearchQuery = '' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + expect(container.textContent).toContain('Interface') + expect(container.textContent).toContain('Terminal') + expect(container.textContent).toContain('Window & Sidebar') + // Code & Markdown is intentionally omitted — Orca has no Appearance-level + // code/markdown settings, so the row would be empty. + expect(container.textContent).not.toContain('Code & Markdown') + }) + + it('keeps the app icon control at the bottom of the pane, after the section rows', async () => { + mocks.state.settingsSearchQuery = '' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + const buttons = Array.from(container.querySelectorAll('button')) + const interfaceRow = buttons.find((button) => button.textContent?.includes('Interface')) + const appIconImage = container.querySelector('img[alt="Selected app icon"]') + + expect(interfaceRow).toBeDefined() + expect(appIconImage).not.toBeNull() + // The App Icon block sits after the Interface section row in document order. + expect( + interfaceRow && + appIconImage && + interfaceRow.compareDocumentPosition(appIconImage) & Node.DOCUMENT_POSITION_FOLLOWING + ).toBeTruthy() + }) + + it('reveals an advanced sidebar control when its search matches, even though it is hidden by default', async () => { + // The Show Tasks Button toggle lives behind the Window & Sidebar Advanced + // disclosure; with no search it stays collapsed, but a matching query must + // force the disclosure open so the control is reachable. + mocks.state.settingsSearchQuery = '' + const collapsedContainer = await renderAppearancePane(getDefaultSettings('/tmp')) + expect( + collapsedContainer.querySelector('button[role="switch"][aria-label="Show Tasks Button"]') + ).toBeNull() + + mocks.state.settingsSearchQuery = 'tasks' + const searchedContainer = await renderAppearancePane(getDefaultSettings('/tmp')) + expect( + searchedContainer.querySelector('button[role="switch"][aria-label="Show Tasks Button"]') + ).not.toBeNull() + }) + + it('keeps description-only search matches visible after helper text is hidden', async () => { + mocks.state.settingsSearchQuery = 'app window' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + expect(container.textContent).toContain('Theme') + expect(container.textContent).not.toContain('Advanced') + }) + + it('shows useful primary rows for a Window & Sidebar section-label search', async () => { + mocks.state.settingsSearchQuery = 'Window & Sidebar' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + expect(container.textContent).toContain('Left Sidebar Appearance') + expect(container.textContent).toContain('Status Bar') + expect(container.textContent).not.toContain('Advanced') + }) + + it('expands status bar controls for a section-label search', async () => { + mocks.state.availableStatusBarToggles = [ + { + id: 'ports', + title: 'Ports', + description: 'Show live workspace ports in the status bar.', + toggleDescription: 'Show Ports in the status bar.', + keywords: ['status bar', 'ports'] + } + ] + mocks.state.settingsSearchQuery = 'status bar' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + expect(container.querySelector('button[role="switch"][aria-label="Ports"]')).not.toBeNull() + }) + + it('collapses sibling sections so only the Interface section is expanded by default', async () => { + mocks.state.settingsSearchQuery = '' + const container = await renderAppearancePane(getDefaultSettings('/tmp')) + + const expanded = Array.from( + container.querySelectorAll('button[aria-expanded="true"]') + ).filter((button) => button.getAttribute('aria-controls')?.startsWith('appearance-section-')) + + expect(expanded).toHaveLength(1) + expect(expanded[0]?.textContent).toContain('Interface') + }) }) diff --git a/src/renderer/src/components/settings/AppearancePane.tsx b/src/renderer/src/components/settings/AppearancePane.tsx index 8a392e30515..f7a2dd52c13 100644 --- a/src/renderer/src/components/settings/AppearancePane.tsx +++ b/src/renderer/src/components/settings/AppearancePane.tsx @@ -1,36 +1,22 @@ -/* eslint-disable max-lines -- Why: AppearancePane keeps theme, typography, zoom, and status-bar - visibility settings together so the searchable settings rows share one filtered surface. */ import type React from 'react' +import { useState } from 'react' +import { AppWindow, PanelLeft, TerminalSquare } from 'lucide-react' import type { GlobalSettings } from '../../../../shared/types' -import { Separator } from '../ui/separator' -import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select' -import { UIZoomControl } from './UIZoomControl' +import { AppearanceSection } from './AppearanceSection' +import { AppearanceInterfaceSection } from './AppearanceInterfaceSection' +import { AppearanceWindowSidebarSection } from './AppearanceWindowSidebarSection' import { SearchableSetting } from './SearchableSetting' -import { matchesSettingsSearch } from './settings-search' +import { matchesSettingsSearch, normalizeSettingsSearchQuery } from './settings-search' import { useAppStore } from '../../store' -import { useShortcutKeyComboDetails, type ShortcutKeyComboDetails } from '@/hooks/useShortcutLabel' -import { ShortcutKeyCombo } from '../ShortcutKeyCombo' -import { - FontAutocomplete, - SettingsRow, - SettingsSegmentedControl, - SettingsSubsectionHeader, - SettingsSwitchRow -} from './SettingsFormControls' -import { DEFAULT_APP_FONT_FAMILY } from '../../../../shared/constants' -import { normalizeAppIconId } from '../../../../shared/app-icon' -import { useAvailableStatusBarToggles } from '../status-bar/use-available-status-bar-toggles' import { getAppIconEntries, getAppearancePaneSearchEntries, getLanguageEntries, getLayoutEntries, - getLeftSidebarAppearanceEntry, getSidebarEntries, getStatusBarEntries, - getStatusBarToggles, getSystemTrayEntries, getThemeEntries, getTitlebarEntries, @@ -42,17 +28,15 @@ import { TerminalAppearanceSection } from './TerminalAppearanceSection' import type { UseGhosttyImportReturn } from './useGhosttyImport' import type { UseWarpThemeImportReturn } from './useWarpThemeImport' import { AppIconSelector } from './AppIconSelector' +import { normalizeAppIconId } from '../../../../shared/app-icon' import { getRendererAppPlatform } from '@/lib/renderer-app-platform' import { isWebClientLocation } from '@/lib/web-client-location' -import { - getUiLanguageChoiceLabel, - SHOW_UI_LANGUAGE_SETTING, - UI_LANGUAGE_CHOICES -} from '@/i18n/supported-languages' +import { SHOW_UI_LANGUAGE_SETTING } from '@/i18n/supported-languages' import { translate } from '@/i18n/i18n' -import type { UiLanguage } from '../../../../shared/ui-language' -import { LeftSidebarAppearanceSetting } from './LeftSidebarAppearanceSetting' -import { getWorkspaceCardLayoutEntry } from './appearance-sidebar-search' +import { + getLeftSidebarAppearanceEntry, + getWorkspaceCardLayoutEntry +} from './appearance-sidebar-search' export { getAppearancePaneSearchEntries } type AppearancePaneProps = { @@ -66,28 +50,16 @@ type AppearancePaneProps = { warpThemes: UseWarpThemeImportReturn } -function ShortcutHintList({ combos }: { combos: ShortcutKeyComboDetails[] }): React.JSX.Element { - if (combos.length === 0) { - return ( - - {translate('auto.components.settings.AppearancePane.3057983501', 'Unassigned')} - - ) - } +type AppearanceSectionKey = 'interface' | 'terminal' | 'window' - return ( - - {combos.map((combo) => ( - - ))} - - ) +function resolveThemeSummary(theme: GlobalSettings['theme']): string { + if (theme === 'system') { + return translate('auto.components.settings.AppearancePane.fb0e0b4453', 'System') + } + if (theme === 'light') { + return translate('auto.components.settings.AppearancePane.fd89b5487c', 'Light') + } + return translate('auto.components.settings.AppearancePane.7d26ccabe8', 'Dark') } export function AppearancePane({ @@ -101,479 +73,165 @@ export function AppearancePane({ warpThemes }: AppearancePaneProps): React.JSX.Element { const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const isSearching = normalizeSettingsSearchQuery(searchQuery).length > 0 const isWebClient = isWebClientLocation() // Why: the system tray behavior is desktop-Electron Windows-only; a Windows // browser web client has no local tray to control. const isDesktopWindows = getRendererAppPlatform() === 'win32' && !isWebClient - const zoomInKeyCombos = useShortcutKeyComboDetails('zoom.in') - const zoomOutKeyCombos = useShortcutKeyComboDetails('zoom.out') - const statusBarItems = useAppStore((state) => state.statusBarItems) - const toggleStatusBarItem = useAppStore((state) => state.toggleStatusBarItem) - const recordFeatureInteraction = useAppStore((state) => state.recordFeatureInteraction) - const visibleStatusBarToggles = useAvailableStatusBarToggles(getStatusBarToggles()) - const terminalAppearanceSearchEntries = getTerminalAppearanceSearchEntries({ - showWarpImport: !isWebClient + + const [manuallyOpenSection, setManuallyOpenSection] = useState( + 'interface' + ) + const interfaceTitle = translate( + 'auto.components.settings.AppearancePane.interfaceTitle', + 'Interface' + ) + const terminalTitle = translate( + 'auto.components.settings.AppearancePane.terminalTitle', + 'Terminal' + ) + const windowSidebarTitle = translate( + 'auto.components.settings.AppearancePane.windowSidebarTitle', + 'Window & Sidebar' + ) + const windowSidebarSummary = translate( + 'auto.components.settings.AppearancePane.windowSidebarSummary', + 'Sidebar, status bar, and file explorer' + ) + + // Search-entry buckets per section so a query can force-open the matching one. + const interfaceSearchEntries = [ + { title: interfaceTitle }, + ...getThemeEntries(), + ...getZoomEntries(), + ...getTypographyEntries(), + ...(SHOW_UI_LANGUAGE_SETTING ? getLanguageEntries() : []), + ...getTitlebarEntries(), + ...getSystemTrayEntries({ showSystemTray: isDesktopWindows }) + ] + const terminalSearchEntries = [ + { title: terminalTitle }, + ...getTerminalAppearanceSearchEntries({ showWarpImport: !isWebClient }) + ] + const windowSearchEntries = [ + { + title: windowSidebarTitle, + description: windowSidebarSummary + }, + ...getStatusBarEntries(), + ...getSidebarEntries(), + ...getLayoutEntries(), + getLeftSidebarAppearanceEntry(), + getWorkspaceCardLayoutEntry() + ] + + const interfaceMatches = matchesSettingsSearch(searchQuery, interfaceSearchEntries) + const terminalMatches = matchesSettingsSearch(searchQuery, terminalSearchEntries) + const windowMatches = matchesSettingsSearch(searchQuery, windowSearchEntries) + const interfaceLabelMatches = matchesSettingsSearch(searchQuery, { title: interfaceTitle }) + const terminalLabelMatches = matchesSettingsSearch(searchQuery, { title: terminalTitle }) + const windowLabelMatches = matchesSettingsSearch(searchQuery, { + title: windowSidebarTitle, + description: windowSidebarSummary }) - const systemTrayEntries = getSystemTrayEntries({ showSystemTray: isDesktopWindows }) - const leftSidebarAppearanceEntry = getLeftSidebarAppearanceEntry() - const workspaceCardLayoutEntry = getWorkspaceCardLayoutEntry() - const visibleSections = [ - matchesSettingsSearch(searchQuery, getThemeEntries()) || - (SHOW_UI_LANGUAGE_SETTING && matchesSettingsSearch(searchQuery, getLanguageEntries())) || - matchesSettingsSearch(searchQuery, getZoomEntries()) || - matchesSettingsSearch(searchQuery, getTypographyEntries()) ? ( -
- {matchesSettingsSearch(searchQuery, getThemeEntries()) ? ( - - { - updateSettings({ theme: option }) - applyTheme(option) - }} - options={[ - { - value: 'system', - label: translate( - 'auto.components.settings.AppearancePane.fb0e0b4453', - 'System' - ) - }, - { - value: 'dark', - label: translate('auto.components.settings.AppearancePane.7d26ccabe8', 'Dark') - }, - { - value: 'light', - label: translate( - 'auto.components.settings.AppearancePane.fd89b5487c', - 'Light' - ) - } - ]} - /> - } - /> - - ) : null} + const appIconMatches = matchesSettingsSearch(searchQuery, getAppIconEntries()) - {SHOW_UI_LANGUAGE_SETTING && matchesSettingsSearch(searchQuery, getLanguageEntries()) ? ( - - updateSettings({ uiLanguage: value as UiLanguage })} - > - - - - - {UI_LANGUAGE_CHOICES.map((choice) => ( - - {getUiLanguageChoiceLabel(choice, translate)} - - ))} - - - } - /> - - ) : null} + // While searching, force-open every section that contains a match so its + // controls (including advanced ones) are revealed; otherwise the accordion + // shows exactly one manually-chosen section. + function isSectionOpen(key: AppearanceSectionKey): boolean { + if (isSearching) { + return key === 'interface' + ? interfaceMatches + : key === 'terminal' + ? terminalMatches + : windowMatches + } + return manuallyOpenSection === key + } - {matchesSettingsSearch(searchQuery, getZoomEntries()) ? ( - - - {translate( - 'auto.components.settings.AppearancePane.f687711a9b', - 'Scale the entire application interface. Use' - )}{' '} - /{' '} - {' '} - {translate( - 'auto.components.settings.AppearancePane.ef89200c1f', - 'when not in a terminal pane.' - )} - - } - control={} - /> - - ) : null} + function toggleSection(key: AppearanceSectionKey): void { + setManuallyOpenSection((current) => (current === key ? null : key)) + } - {matchesSettingsSearch(searchQuery, getTypographyEntries()) ? ( - - - updateSettings({ appFontFamily: value.trim() || DEFAULT_APP_FONT_FAMILY }) - } - /> - } - /> - - ) : null} -
- ) : null, - matchesSettingsSearch(searchQuery, terminalAppearanceSearchEntries) ? ( - - ) : null, - matchesSettingsSearch(searchQuery, getLayoutEntries()) ? ( -
- + const interfaceSummary = `${resolveThemeSummary(settings.theme)} · ${ + settings.appFontFamily || + translate('auto.components.settings.AppearancePane.interfaceDefaultFont', 'Default font') + }` + const terminalSummary = `${ + settings.terminalFontFamily || + translate('auto.components.settings.AppearancePane.terminalDefaultFont', 'Default font') + } · ${settings.terminalFontSize}px` -
- - - updateSettings({ showGitIgnoredFiles: !(settings.showGitIgnoredFiles ?? true) }) - } - /> - -
-
- ) : null, - matchesSettingsSearch(searchQuery, getTitlebarEntries()) ? ( -
- + return ( +
+ {interfaceMatches ? ( + + ) : null} -
- - - updateSettings({ showTitlebarAppName: !settings.showTitlebarAppName }) - } - /> - -
-
- ) : null, - isDesktopWindows && matchesSettingsSearch(searchQuery, systemTrayEntries) ? ( -
- + {/* Why: Code & Markdown is intentionally omitted. Orca has no Appearance-level + code/markdown settings — the Monaco editor reuses the terminal font and + there is no markdown-style or line-number setting — so a fourth row would + be empty. We surface only the three sections that hold real controls + rather than fabricate settings. */} -
- - - updateSettings({ minimizeToTrayOnClose: !settings.minimizeToTrayOnClose }) - } - /> - -
-
- ) : null, - matchesSettingsSearch(searchQuery, getStatusBarEntries()) ? ( -
- + {terminalMatches ? ( + + ) : null} -
- {visibleStatusBarToggles.map((toggle) => { - const enabled = statusBarItems.includes(toggle.id) - return ( - - { - if (toggle.id === 'resource-usage') { - recordFeatureInteraction('resource-manager') - } else if (toggle.id === 'ports') { - recordFeatureInteraction('ports') - } else if (toggle.id === 'ssh') { - recordFeatureInteraction('ssh') - } else if ( - toggle.id === 'claude' || - toggle.id === 'codex' || - toggle.id === 'gemini' || - toggle.id === 'opencode-go' - ) { - recordFeatureInteraction('usage-tracking') - } - toggleStatusBarItem(toggle.id) - }} - ariaLabel={toggle.title} - /> - - ) - })} -
-
- ) : null, - matchesSettingsSearch(searchQuery, getSidebarEntries()) ? ( -
- + {windowMatches ? ( + + ) : null} -
- - - - - {/* Why: this setting lives with the sidebar layout controls; Settings only - points people to it so we do not create a second stateful control. */} - - Card layout > Compact.' - )} - control={null} - /> - - - - - updateSettings({ showTasksButton: !(settings.showTasksButton !== false) }) - } - /> - - - - - updateSettings({ - showAutomationsButton: !(settings.showAutomationsButton !== false) - }) - } - /> - - - - - updateSettings({ showMobileButton: !(settings.showMobileButton !== false) }) - } - /> - -
-
- ) : null, - matchesSettingsSearch(searchQuery, getAppIconEntries()) ? ( -
+ {/* App icon stays at the bottom of Appearance as a small easter egg, + matching production — not buried inside Interface advanced. */} + {appIconMatches ? ( updateSettings({ appIcon })} /> -
- ) : null - ].filter(Boolean) - - return ( -
- {visibleSections.map((section, index) => ( -
- {index > 0 ? : null} - {section} -
- ))} + ) : null}
) } diff --git a/src/renderer/src/components/settings/AppearanceSection.tsx b/src/renderer/src/components/settings/AppearanceSection.tsx new file mode 100644 index 00000000000..faa7dde2f03 --- /dev/null +++ b/src/renderer/src/components/settings/AppearanceSection.tsx @@ -0,0 +1,78 @@ +import type React from 'react' +import { ChevronRight } from 'lucide-react' +import { cn } from '@/lib/utils' + +type AppearanceSectionProps = { + /** Stable id used for the accordion toggle + aria wiring. */ + id: string + icon: React.ReactNode + title: React.ReactNode + /** Plain-language current value shown in the collapsed summary row. */ + summary: React.ReactNode + open: boolean + onToggle: () => void + children: React.ReactNode +} + +/** Compact summary row that expands its section inline. The parent owns the + * open state so opening one row can collapse the previously open one + * (accordion behavior) and search can force a section open. */ +export function AppearanceSection({ + id, + icon, + title, + summary, + open, + onToggle, + children +}: AppearanceSectionProps): React.JSX.Element { + const contentId = `appearance-section-${id}` + return ( +
+ +
+
+
+ {children} +
+
+
+
+ ) +} diff --git a/src/renderer/src/components/settings/AppearanceShortcutHintList.tsx b/src/renderer/src/components/settings/AppearanceShortcutHintList.tsx new file mode 100644 index 00000000000..8f9b33cb259 --- /dev/null +++ b/src/renderer/src/components/settings/AppearanceShortcutHintList.tsx @@ -0,0 +1,34 @@ +import type React from 'react' + +import { type ShortcutKeyComboDetails } from '@/hooks/useShortcutLabel' +import { ShortcutKeyCombo } from '../ShortcutKeyCombo' +import { translate } from '@/i18n/i18n' + +/** Renders the primary keyboard shortcut combo inline, or an "Unassigned" + * hint when the action has no binding. Platform-aware glyphs come from + * ShortcutKeyCombo. */ +export function ShortcutHintList({ + combos +}: { + combos: ShortcutKeyComboDetails[] +}): React.JSX.Element { + if (combos.length === 0) { + return ( + + {translate('auto.components.settings.AppearancePane.3057983501', 'Unassigned')} + + ) + } + const primaryCombo = combos[0] + + return ( + + + + ) +} diff --git a/src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx b/src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx new file mode 100644 index 00000000000..065056fbe6f --- /dev/null +++ b/src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx @@ -0,0 +1,307 @@ +import type React from 'react' + +import type { GlobalSettings, StatusBarItem } from '../../../../shared/types' +import type { FeatureInteractionId } from '../../../../shared/feature-interaction-catalog' +import { SearchableSetting } from './SearchableSetting' +import { AppearanceAdvancedDisclosure } from './AppearanceAdvancedDisclosure' +import { useAppStore } from '../../store' +import { + SettingsRow, + SettingsSegmentedControl, + SettingsSubsectionHeader, + SettingsSwitchRow +} from './SettingsFormControls' +import { useAvailableStatusBarToggles } from '../status-bar/use-available-status-bar-toggles' +import { getLayoutEntries, getSidebarEntries, getStatusBarToggles } from './appearance-search' +import { LeftSidebarAppearanceSetting } from './LeftSidebarAppearanceSetting' +import { + getLeftSidebarAppearanceEntry, + getWorkspaceCardLayoutEntry +} from './appearance-sidebar-search' +import { translate } from '@/i18n/i18n' +import { matchesSettingsSearch, normalizeSettingsSearchQuery } from './settings-search' + +type AppearanceWindowSidebarSectionProps = { + settings: GlobalSettings + updateSettings: (updates: Partial) => void + forceVisiblePrimary?: boolean +} + +function recordStatusBarToggleInteraction( + id: StatusBarItem, + recordFeatureInteraction: (feature: FeatureInteractionId) => void +): void { + if (id === 'resource-usage') { + recordFeatureInteraction('resource-manager') + } else if (id === 'ports') { + recordFeatureInteraction('ports') + } else if (id === 'ssh') { + recordFeatureInteraction('ssh') + } else if (id === 'claude' || id === 'codex' || id === 'gemini' || id === 'opencode-go') { + recordFeatureInteraction('usage-tracking') + } +} + +export function AppearanceWindowSidebarSection({ + settings, + updateSettings, + forceVisiblePrimary = false +}: AppearanceWindowSidebarSectionProps): React.JSX.Element { + const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const isSearching = normalizeSettingsSearchQuery(searchQuery).length > 0 + const statusBarItems = useAppStore((state) => state.statusBarItems) + const toggleStatusBarItem = useAppStore((state) => state.toggleStatusBarItem) + const recordFeatureInteraction = useAppStore((state) => state.recordFeatureInteraction) + const setWorktreeCardMode = useAppStore((state) => state.setWorktreeCardMode) + const visibleStatusBarToggles = useAvailableStatusBarToggles(getStatusBarToggles()) + const leftSidebarAppearanceEntry = getLeftSidebarAppearanceEntry() + const sidebarEntries = getSidebarEntries() + const workspaceCardLayoutEntry = getWorkspaceCardLayoutEntry() + const layoutEntries = getLayoutEntries() + const statusBarTitle = translate( + 'auto.components.settings.AppearancePane.3e4175e5c6', + 'Status Bar' + ) + const statusBarDescription = translate( + 'auto.components.settings.AppearancePane.statusBarDescription', + 'Choose which indicators appear in the status bar.' + ) + const statusBarKeywords = ['status bar', 'indicators'] + const statusBarSectionMatches = matchesSettingsSearch(searchQuery, { + title: statusBarTitle, + description: statusBarDescription, + keywords: statusBarKeywords + }) + const statusBarControlMatches = visibleStatusBarToggles.some((toggle) => + matchesSettingsSearch(searchQuery, { + title: toggle.title, + description: toggle.description, + keywords: toggle.keywords + }) + ) + const sidebarAdvancedMatches = matchesSettingsSearch(searchQuery, [ + workspaceCardLayoutEntry, + ...sidebarEntries + ]) + const fileExplorerAdvancedMatches = matchesSettingsSearch(searchQuery, layoutEntries) + const showStatusBarControls = !isSearching || statusBarSectionMatches || statusBarControlMatches + const showSidebarAdvanced = !isSearching || sidebarAdvancedMatches + const showFileExplorerAdvanced = !isSearching || fileExplorerAdvancedMatches + const showAdvanced = showSidebarAdvanced || showFileExplorerAdvanced + + return ( +
+
+ + + + + + + {showStatusBarControls ? ( +
+ {visibleStatusBarToggles.map((toggle) => { + const enabled = statusBarItems.includes(toggle.id) + return ( + + { + recordStatusBarToggleInteraction(toggle.id, recordFeatureInteraction) + toggleStatusBarItem(toggle.id) + }} + ariaLabel={toggle.title} + /> + + ) + })} +
+ ) : null} + +
+ + {showAdvanced ? ( + +
+ {showSidebarAdvanced ? ( +
+ +
+ {/* Why: this setting lives with the sidebar layout controls; Settings only + names that ownership so we do not create a second stateful control. */} + + + setWorktreeCardMode(value === 'compact' ? 'Compact' : 'Default') + } + ariaLabel={workspaceCardLayoutEntry.title} + options={[ + { + value: 'detailed', + label: translate( + 'auto.components.sidebar.SidebarWorkspaceOptionsMenu.cc17bd443b', + 'Detailed' + ) + }, + { + value: 'compact', + label: translate( + 'auto.components.sidebar.SidebarWorkspaceOptionsMenu.25105b28cb', + 'Compact' + ) + } + ]} + /> + } + /> + + + + + updateSettings({ showTasksButton: !(settings.showTasksButton !== false) }) + } + /> + + + + + updateSettings({ + showAutomationsButton: !(settings.showAutomationsButton !== false) + }) + } + /> + + + + + updateSettings({ showMobileButton: !(settings.showMobileButton !== false) }) + } + /> + +
+
+ ) : null} + + {showFileExplorerAdvanced ? ( +
+ +
+ + + updateSettings({ + showGitIgnoredFiles: !(settings.showGitIgnoredFiles ?? true) + }) + } + /> + +
+
+ ) : null} +
+ + ) : null} +
+ ) +} diff --git a/src/renderer/src/components/settings/SettingsFormControls.tsx b/src/renderer/src/components/settings/SettingsFormControls.tsx index d60a72c6900..bc51edc5bde 100644 --- a/src/renderer/src/components/settings/SettingsFormControls.tsx +++ b/src/renderer/src/components/settings/SettingsFormControls.tsx @@ -76,11 +76,19 @@ export function SettingsRow({ }: SettingsRowProps): React.JSX.Element { return (
-
- - {description ?

{description}

: null} +
+ + {description ? ( +

{description}

+ ) : null}
{control}
@@ -218,16 +226,18 @@ type SettingsSubsectionHeaderProps = { title: React.ReactNode description?: React.ReactNode action?: React.ReactNode + className?: string } /** Consistent subsection header: h3 text-sm font-semibold + optional muted description. */ export function SettingsSubsectionHeader({ title, description, - action + action, + className }: SettingsSubsectionHeaderProps): React.JSX.Element { return ( -
+

{title}

{description ?

{description}

: null} diff --git a/src/renderer/src/components/settings/TerminalAdvancedTypographyControls.tsx b/src/renderer/src/components/settings/TerminalAdvancedTypographyControls.tsx new file mode 100644 index 00000000000..5acfee0d190 --- /dev/null +++ b/src/renderer/src/components/settings/TerminalAdvancedTypographyControls.tsx @@ -0,0 +1,188 @@ +import type { GlobalSettings } from '../../../../shared/types' +import { + DEFAULT_TERMINAL_FONT_WEIGHT, + TERMINAL_FONT_WEIGHT_MAX, + TERMINAL_FONT_WEIGHT_MIN, + TERMINAL_FONT_WEIGHT_STEP, + normalizeTerminalFontWeight +} from '../../../../shared/terminal-fonts' +import { + fontFamilyHasKnownLigatures, + resolveTerminalLigaturesEnabled +} from '../../../../shared/terminal-ligatures' +import { NumberField, SettingsRow, SettingsSegmentedControl } from './SettingsFormControls' +import { SearchableSetting } from './SearchableSetting' +import { clampNumber } from '@/lib/terminal-theme' +import { translate } from '@/i18n/i18n' +import { getTerminalAdvancedTypographySearchEntries } from './terminal-typography-search' + +type TerminalAdvancedTypographyControlsProps = { + settings: GlobalSettings + updateSettings: (updates: Partial) => void +} + +/** Low-frequency terminal typography knobs (weight, line height, ligatures). + * Split out of the primary font controls so the default Terminal scan stays + * compact while these stay searchable inside the Advanced disclosure. */ +export function TerminalAdvancedTypographyControls({ + settings, + updateSettings +}: TerminalAdvancedTypographyControlsProps): React.JSX.Element { + const searchEntries = getTerminalAdvancedTypographySearchEntries() + + return ( +
+ + + updateSettings({ terminalFontWeight: normalizeTerminalFontWeight(value) }) + } + /> + + + + updateSettings({ terminalLineHeight: clampNumber(value, 1, 3) })} + /> + + + + updateSettings({ terminalLigatures: option })} + options={[ + { + value: 'auto', + label: translate( + 'auto.components.settings.TerminalAppearanceSection.bc9ff84d61', + 'Auto' + ) + }, + { + value: 'on', + label: translate( + 'auto.components.settings.TerminalAppearanceSection.84bd22f2cd', + 'On' + ) + }, + { + value: 'off', + label: translate( + 'auto.components.settings.TerminalAppearanceSection.870377082f', + 'Off' + ) + } + ]} + /> + } + /> +

+ {translate( + 'auto.components.settings.TerminalAppearanceSection.31f6e61085', + 'Ligatures are currently' + )}{' '} + {resolveTerminalLigaturesEnabled(settings.terminalLigatures, settings.terminalFontFamily) + ? translate('auto.components.settings.TerminalAppearanceSection.4e7d41a9f0', 'enabled') + : translate( + 'auto.components.settings.TerminalAppearanceSection.4415beb958', + 'disabled' + )} + . +

+ +
+ ) +} diff --git a/src/renderer/src/components/settings/TerminalAppearanceSection.ghostty.test.ts b/src/renderer/src/components/settings/TerminalAppearanceSection.ghostty.test.ts index deb30721179..d5f2c28f73a 100644 --- a/src/renderer/src/components/settings/TerminalAppearanceSection.ghostty.test.ts +++ b/src/renderer/src/components/settings/TerminalAppearanceSection.ghostty.test.ts @@ -97,8 +97,8 @@ vi.mock('./SettingsFormControls', () => ({ }) { return options?.map((option) => option.label) ?? null }, - SettingsSubsectionHeader: function SettingsSubsectionHeader() { - return null + SettingsSubsectionHeader: function SettingsSubsectionHeader({ action }: { action?: unknown }) { + return action ?? null }, SettingsSwitchRow: function SettingsSwitchRow() { return null @@ -322,6 +322,30 @@ function findWarpThemeImportModal(node: unknown): ReactElementLike | null { return null } +function findComponentByTypeName(node: unknown, targetTypeName: string): ReactElementLike | null { + if (node == null) { + return null + } + if (Array.isArray(node)) { + for (const child of node) { + const found = findComponentByTypeName(child, targetTypeName) + if (found) { + return found + } + } + return null + } + const el = node as ReactElementLike + const typeName = typeof el.type === 'function' ? el.type.name : String(el.type) + if (typeName === targetTypeName) { + return el + } + if (el.props?.children) { + return findComponentByTypeName(el.props.children, targetTypeName) + } + return null +} + describe('TerminalAppearanceSection ghostty import wiring', () => { beforeEach(() => { mockStateValues.length = 0 @@ -421,6 +445,68 @@ describe('TerminalAppearanceSection ghostty import wiring', () => { expect(findTerminalThemeCatalogSection(darkPhraseElement)?.props.preferredTarget).toBe('dark') }) + it('does not open advanced typography for primary terminal font searches', () => { + mockSettingsSearchQuery = 'font size' + + const element = TerminalAppearanceSection({ + settings: {} as never, + updateSettings: () => {}, + systemPrefersDark: true, + terminalFontSuggestions: [], + ghostty: ghosttyMock, + warpThemes: warpThemesMock + }) + + expect(findComponentByTypeName(element, 'TerminalAdvancedTypographyControls')).toBeNull() + }) + + it('does not show primary typography chrome for unrelated terminal searches', () => { + mockSettingsSearchQuery = 'cursor opacity' + + const element = TerminalAppearanceSection({ + settings: {} as never, + updateSettings: () => {}, + systemPrefersDark: true, + terminalFontSuggestions: [], + ghostty: ghosttyMock, + warpThemes: warpThemesMock + }) + + expect(findComponentByTypeName(element, 'TerminalFontSizeSetting')).toBeNull() + expect(findButtons(element).some((button) => button.text === 'Import from Ghostty')).toBe(false) + }) + + it('shows the Ghostty import button for Ghostty-only searches', () => { + mockSettingsSearchQuery = 'ghostty' + + const element = TerminalAppearanceSection({ + settings: {} as never, + updateSettings: () => {}, + systemPrefersDark: true, + terminalFontSuggestions: [], + ghostty: ghosttyMock, + warpThemes: warpThemesMock + }) + + expect(findButtons(element).some((button) => button.text === 'Import from Ghostty')).toBe(true) + }) + + it('opens typography advanced inside the typography section for advanced searches', () => { + mockSettingsSearchQuery = 'line height' + + const element = TerminalAppearanceSection({ + settings: {} as never, + updateSettings: () => {}, + systemPrefersDark: true, + terminalFontSuggestions: [], + ghostty: ghosttyMock, + warpThemes: warpThemesMock + }) + + expect(findComponentByTypeName(element, 'TerminalAdvancedTypographyControls')).not.toBeNull() + expect(findComponentByTypeName(element, 'TerminalFontSizeSetting')).not.toBeNull() + }) + it('hides the theme import affordance on paired web clients', () => { vi.stubGlobal('window', { __ORCA_WEB_CLIENT__: true, diff --git a/src/renderer/src/components/settings/TerminalAppearanceSection.tsx b/src/renderer/src/components/settings/TerminalAppearanceSection.tsx index e790673835a..4503eb545e0 100644 --- a/src/renderer/src/components/settings/TerminalAppearanceSection.tsx +++ b/src/renderer/src/components/settings/TerminalAppearanceSection.tsx @@ -2,32 +2,42 @@ import { useState } from 'react' import type { GlobalSettings } from '../../../../shared/types' import { matchesSettingsSearch, + normalizeSettingsSearchQuery, scoreSettingsSearch, type SettingsSearchEntry } from './settings-search' import { useAppStore } from '../../store' import { + getTerminalAdvancedTypographySearchEntries, getTerminalCursorSearchEntries, getTerminalDarkThemeSearchEntries, getTerminalGhosttyImportSearchEntries, getTerminalLightThemeSearchEntries, getTerminalPaneAppearanceSearchEntries, getTerminalThemeTargetSearchEntries, - getTerminalTypographySearchEntries, getTerminalWarpImportSearchEntries, - getTerminalWindowSearchEntries, - getTerminalYamlImportSearchEntries + getTerminalYamlImportSearchEntries, + getTerminalTypographySearchEntries, + getTerminalWindowSearchEntries } from './terminal-search' +import { Button } from '../ui/button' +import { SettingsRow, SettingsSubsectionHeader } from './SettingsFormControls' +import { SearchableSetting } from './SearchableSetting' +import { FontAutocomplete } from './SettingsFormControls' +import { TerminalFontSizeSetting } from './TerminalFontSizeSetting' +import { TerminalAdvancedTypographyControls } from './TerminalAdvancedTypographyControls' import { TerminalThemeCatalogSection } from './TerminalThemeSections' import { TerminalWindowSection } from './TerminalWindowSection' -import { TerminalTypographyAppearanceSection } from './TerminalTypographyAppearanceSection' import { TerminalCursorAppearanceSection } from './TerminalCursorAppearanceSection' import { TerminalPaneAppearanceSection } from './TerminalPaneAppearanceSection' +import { AppearanceAdvancedDisclosure } from './AppearanceAdvancedDisclosure' import { GhosttyImportModal } from './GhosttyImportModal' import type { UseGhosttyImportReturn } from './useGhosttyImport' import { WarpThemeImportModal } from './WarpThemeImportModal' import type { UseWarpThemeImportReturn } from './useWarpThemeImport' import { isWebClientLocation } from '@/hooks/useSettingsNavigationMetadata' +import ghosttyIcon from '../../../../../resources/ghostty.svg' +import { translate } from '@/i18n/i18n' type TerminalAppearanceSectionProps = { settings: GlobalSettings @@ -36,6 +46,7 @@ type TerminalAppearanceSectionProps = { terminalFontSuggestions: string[] ghostty: UseGhosttyImportReturn warpThemes: UseWarpThemeImportReturn + forceVisiblePrimary?: boolean } type TerminalThemeTarget = 'dark' | 'light' @@ -64,88 +75,194 @@ export function TerminalAppearanceSection({ systemPrefersDark, terminalFontSuggestions, ghostty, - warpThemes + warpThemes, + forceVisiblePrimary = false }: TerminalAppearanceSectionProps): React.JSX.Element { const searchQuery = useAppStore((state) => state.settingsSearchQuery) + const isSearching = normalizeSettingsSearchQuery(searchQuery).length > 0 const [themeSearch, setThemeSearch] = useState('') const [previewFontFamily, setPreviewFontFamily] = useState(null) const showWarpThemeImport = !isWebClientLocation() const darkThemeSearchEntries = getTerminalDarkThemeSearchEntries() const lightThemeSearchEntries = getTerminalLightThemeSearchEntries() - const darkThemeSearchScore = scoreSettingsSearch(searchQuery, darkThemeSearchEntries) - const lightThemeSearchScore = scoreSettingsSearch(searchQuery, lightThemeSearchEntries) + const terminalTypographyEntries = getTerminalTypographySearchEntries() + const ghosttyImportEntries = getTerminalGhosttyImportSearchEntries() + const themeCatalogSearchEntries = [ + ...getTerminalThemeTargetSearchEntries(), + ...darkThemeSearchEntries, + ...lightThemeSearchEntries, + ...(showWarpThemeImport + ? [...getTerminalWarpImportSearchEntries(), ...getTerminalYamlImportSearchEntries()] + : []) + ] const darkThemeTargetScore = scoreThemeTargetIntent(searchQuery, darkThemeSearchEntries) const lightThemeTargetScore = scoreThemeTargetIntent(searchQuery, lightThemeSearchEntries) - const darkThemeMatches = darkThemeSearchScore > 0 - const lightThemeMatches = lightThemeSearchScore > 0 - const themeTargetMatches = matchesSettingsSearch( - searchQuery, - getTerminalThemeTargetSearchEntries() - ) - const themeImportMatches = - showWarpThemeImport && - (matchesSettingsSearch(searchQuery, getTerminalWarpImportSearchEntries()) || - matchesSettingsSearch(searchQuery, getTerminalYamlImportSearchEntries())) - const showTerminalThemeCatalog = - darkThemeMatches || lightThemeMatches || themeTargetMatches || themeImportMatches const preferredThemeTarget = getPreferredThemeTarget(darkThemeTargetScore, lightThemeTargetScore) - const visibleSections = [ - matchesSettingsSearch(searchQuery, getTerminalGhosttyImportSearchEntries()) || - matchesSettingsSearch(searchQuery, getTerminalTypographySearchEntries()) ? ( - - ) : null, - matchesSettingsSearch(searchQuery, getTerminalCursorSearchEntries()) ? ( - - ) : null, - matchesSettingsSearch(searchQuery, getTerminalPaneAppearanceSearchEntries()) ? ( - - ) : null, - matchesSettingsSearch(searchQuery, getTerminalWindowSearchEntries()) ? ( - - ) : null, - showTerminalThemeCatalog ? ( - - ) : null - ].filter(Boolean) + // Why: low-frequency knobs are force-opened during search; render each group + // only when its own search matches so an active query never leaves a dangling header. + const typographyMatches = matchesSettingsSearch( + searchQuery, + getTerminalAdvancedTypographySearchEntries() + ) + const cursorMatches = matchesSettingsSearch(searchQuery, getTerminalCursorSearchEntries()) + const paneMatches = matchesSettingsSearch(searchQuery, getTerminalPaneAppearanceSearchEntries()) + const windowMatches = matchesSettingsSearch(searchQuery, getTerminalWindowSearchEntries()) + const themeCatalogMatches = matchesSettingsSearch(searchQuery, themeCatalogSearchEntries) + const previewAdvancedMatches = cursorMatches || paneMatches || windowMatches + const showThemeCatalog = !isSearching || themeCatalogMatches || previewAdvancedMatches + const primaryTypographyMatches = matchesSettingsSearch( + searchQuery, + terminalTypographyEntries.slice(0, 2) + ) + const ghosttyImportMatches = matchesSettingsSearch(searchQuery, ghosttyImportEntries) + const showPrimaryTypography = + !isSearching || + forceVisiblePrimary || + primaryTypographyMatches || + typographyMatches || + ghosttyImportMatches + const showGhosttyImport = !isSearching || forceVisiblePrimary || ghosttyImportMatches + const showTypographyAdvancedDisclosure = !isSearching || typographyMatches - return ( -
- {visibleSections.map((section, index) => ( -
- {index > 0 ?
: null} - {section} + const advancedGroups = [ + cursorMatches + ? { + key: 'cursor', + node: ( + + ) + } + : null, + paneMatches + ? { + key: 'pane', + node: ( + + ) + } + : null, + windowMatches + ? { + key: 'window', + node: + } + : null + ].filter((group): group is { key: string; node: React.JSX.Element } => group !== null) + const showAdvancedDisclosure = !isSearching || advancedGroups.length > 0 + const previewAdvancedContent = showAdvancedDisclosure ? ( + + {advancedGroups.map((group, index) => ( +
0 ? 'mt-2 border-t border-border/60 pt-4' : undefined} + > + {group.node}
))} + + ) : null + + return ( +
+ {/* Primary: font + theme + previews. The expanded section column is far + narrower than the xl breakpoint, so the preview grids inside the + theme catalog already stack full-width below their controls. */} + {showPrimaryTypography ? ( +
+ void ghostty.handleClick()} + > + + {translate( + 'auto.components.settings.TerminalAppearanceSection.855a76343a', + 'Import from Ghostty' + )} + + ) : null + } + /> + +
+ + + + updateSettings({ terminalFontFamily: value })} + onPreviewFontFamily={setPreviewFontFamily} + /> + } + /> + +
+ + {showTypographyAdvancedDisclosure ? ( +
+ + + +
+ ) : null} +
+ ) : null} + + {showThemeCatalog ? ( + + ) : null} + -
+
+ {/* Why: Bar/Block/Underline options convey the meaning; helper text pruned. */} updateSettings({ terminalCursorBlink: !settings.terminalCursorBlink })} /> @@ -130,10 +119,7 @@ export function TerminalCursorAppearanceSection({ 'auto.components.settings.TerminalAppearanceSection.b9f1804422', 'Cursor Opacity' )} - description={translate( - 'auto.components.settings.TerminalAppearanceSection.04cdf85dec', - 'Opacity of the terminal cursor.' - )} + description="" value={settings.terminalCursorOpacity ?? 1} defaultValue={1} min={0} diff --git a/src/renderer/src/components/settings/TerminalFontSizeSetting.tsx b/src/renderer/src/components/settings/TerminalFontSizeSetting.tsx index e3964394a11..cc601aa9980 100644 --- a/src/renderer/src/components/settings/TerminalFontSizeSetting.tsx +++ b/src/renderer/src/components/settings/TerminalFontSizeSetting.tsx @@ -8,10 +8,12 @@ import { translate } from '@/i18n/i18n' export function TerminalFontSizeSetting({ settings, - updateSettings + updateSettings, + forceVisible = false }: { settings: GlobalSettings updateSettings: (updates: Partial) => void + forceVisible?: boolean }): React.JSX.Element { return ( + {/* Why: helper text dropped per the copy audit — "Font Size" + px control + is self-evident; the search index keeps the longer description. */} -
- - {blurPendingRestart ? ( -
-
-

+ +

+
+

{translate( - 'auto.components.settings.TerminalWindowSection.53ce336e15', - 'Restart Orca to apply the window blur change.' + 'auto.components.settings.TerminalWindowSection.97950bb087', + 'Apply background blur to the terminal window. Requires restart.' )}

- + +
- ) : null} - - - +
+

+ {translate( + 'auto.components.settings.TerminalWindowSection.c65bb9ce63', + 'Restart required' + )} +

+

+ {translate( + 'auto.components.settings.TerminalWindowSection.53ce336e15', + 'Restart Orca to apply the window blur change.' + )} +

+
+ +
+ ) : null} + + + updateSettings({ terminalPaddingX: Math.max(0, value) })} - /> - + keywords={['padding', 'horizontal', 'spacing', 'margin']} + > + updateSettings({ terminalPaddingX: Math.max(0, value) })} + /> + - - updateSettings({ terminalPaddingY: Math.max(0, value) })} - /> - - - -
- -

- {translate( - 'auto.components.settings.TerminalWindowSection.1d1920dc8a', - 'Hide the mouse cursor when typing in the terminal.' - )} -

-
- - - - -
- + + + +
+ +
+
+ {COLOR_OVERRIDE_GROUPS.map((group) => ( +
+

{group.label}

+
+ {group.keys.map((item) => ( + + updateSettings({ + terminalColorOverrides: { + ...settings.terminalColorOverrides, + [item.key]: value || undefined + } + }) + } + /> + ))} +
-
- ))} - + ))} + +
-
- + +
) } diff --git a/src/renderer/src/components/settings/appearance-search.ts b/src/renderer/src/components/settings/appearance-search.ts index b0d3d8eb8dd..7284b5db315 100644 --- a/src/renderer/src/components/settings/appearance-search.ts +++ b/src/renderer/src/components/settings/appearance-search.ts @@ -245,6 +245,25 @@ export function getSystemTrayEntries(options: SystemTraySearchOptions = {}): Set return shouldShowSystemTrayEntries(options) ? getSystemTrayEntryCatalog() : [] } +const getAppearanceSectionEntries = createLocalizedCatalog((): SettingsSearchEntry[] => [ + { + title: translate('auto.components.settings.AppearancePane.interfaceTitle', 'Interface') + }, + { + title: translate('auto.components.settings.AppearancePane.terminalTitle', 'Terminal') + }, + { + title: translate( + 'auto.components.settings.AppearancePane.windowSidebarTitle', + 'Window & Sidebar' + ), + description: translate( + 'auto.components.settings.AppearancePane.windowSidebarSummary', + 'Sidebar, status bar, and file explorer' + ) + } +]) + type AppearancePaneSearchOptions = { showWarpImport?: boolean showSystemTray?: boolean @@ -254,6 +273,7 @@ function buildAppearancePaneSearchEntries( options: AppearancePaneSearchOptions ): SettingsSearchEntry[] { return [ + ...getAppearanceSectionEntries(), ...getThemeEntries(), ...(SHOW_UI_LANGUAGE_SETTING ? getLanguageEntries() : []), ...getTypographyEntries(), diff --git a/src/renderer/src/components/settings/appearance-sidebar-search.ts b/src/renderer/src/components/settings/appearance-sidebar-search.ts index d723e3c4a0e..fd66f0f00a9 100644 --- a/src/renderer/src/components/settings/appearance-sidebar-search.ts +++ b/src/renderer/src/components/settings/appearance-sidebar-search.ts @@ -43,7 +43,7 @@ export const getWorkspaceCardLayoutEntry = createLocalizedCatalog( ), description: translate( 'auto.components.settings.appearance.search.workspaceCardLayout.description', - 'Switch between compact and detailed workspace cards from the workspace sidebar options menu.' + 'Workspace cards can use compact or detailed layouts.' ), keywords: [ ...translateSearchKeyword( diff --git a/src/renderer/src/components/settings/terminal-search.test.ts b/src/renderer/src/components/settings/terminal-search.test.ts index 88041fdf80b..60ff53fef0b 100644 --- a/src/renderer/src/components/settings/terminal-search.test.ts +++ b/src/renderer/src/components/settings/terminal-search.test.ts @@ -180,6 +180,8 @@ describe('getTerminalPaneSearchEntries', () => { expect(getSidebarEntries()).toContainEqual(entry) expect(getAppearancePaneSearchEntries()).toContainEqual(entry) + expect(entry.description).toBe('Workspace cards can use compact or detailed layouts.') + expect(entry.description).not.toContain('options menu') }) it.each(['compact', 'compact display', 'workspace cards', 'sidebar', 'card layout'])( diff --git a/src/renderer/src/components/settings/terminal-search.ts b/src/renderer/src/components/settings/terminal-search.ts index 3b06d4a3b1c..783f7085d2a 100644 --- a/src/renderer/src/components/settings/terminal-search.ts +++ b/src/renderer/src/components/settings/terminal-search.ts @@ -34,6 +34,7 @@ import { import { createLocalizedCatalog } from '@/i18n/localized-catalog' export { + getTerminalAdvancedTypographySearchEntries, getTerminalTypographySearchEntries, getTerminalRenderingSearchEntries, getTerminalCursorSearchEntries diff --git a/src/renderer/src/components/settings/terminal-typography-search.ts b/src/renderer/src/components/settings/terminal-typography-search.ts index ed04dcc07be..02fe124d25f 100644 --- a/src/renderer/src/components/settings/terminal-typography-search.ts +++ b/src/renderer/src/components/settings/terminal-typography-search.ts @@ -2,7 +2,7 @@ import { translate } from '@/i18n/i18n' import { translateSearchKeyword } from './settings-search-keywords' import { createLocalizedCatalog } from '@/i18n/localized-catalog' -export const getTerminalTypographySearchEntries = createLocalizedCatalog(() => [ +const getTerminalTypographySearchEntryCatalog = createLocalizedCatalog(() => [ { title: translate('auto.components.settings.terminal.search.5930244899', 'Font Size'), description: translate( @@ -100,6 +100,14 @@ export const getTerminalTypographySearchEntries = createLocalizedCatalog(() => [ } ]) +export const getTerminalTypographySearchEntries = createLocalizedCatalog(() => [ + ...getTerminalTypographySearchEntryCatalog() +]) + +export const getTerminalAdvancedTypographySearchEntries = createLocalizedCatalog(() => + getTerminalTypographySearchEntryCatalog().slice(2) +) + export const getTerminalRenderingSearchEntries = createLocalizedCatalog(() => [ { title: translate('auto.components.settings.terminal.search.13a2502dfc', 'GPU Acceleration'), diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 5d7fec5c1a0..ef0430160cf 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -6215,6 +6215,249 @@ describe('connectPanePty', () => { disposable.dispose() }) + it('abandons a stalled hidden restore and drains pending foreground chunks warning-first', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + const snapshot = createDeferred<{ data: string; cols: number; rows: number; seq: number }>() + getMainBufferSnapshot.mockReturnValue(snapshot.promise) + const hidden = 'hidden-codex-output\r\n' + const firstLive = 'first-live-output\r\n' + const secondLive = 'second-live-output\r\n' + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + vi.useFakeTimers() + capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(firstLive, { + seq: hidden.length + firstLive.length, + rawLength: firstLive.length + }) + await flushAsyncTicks(4) + capturedDataCallback.current?.(secondLive, { + seq: hidden.length + firstLive.length + secondLive.length, + rawLength: secondLive.length + }) + + expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 }) + expect(pane.terminal.write).not.toHaveBeenCalledWith(firstLive, expect.any(Function)) + expect(pane.terminal.write).not.toHaveBeenCalledWith(secondLive, expect.any(Function)) + + vi.advanceTimersByTime(749) + await flushAsyncTicks(4) + expect(pane.terminal.write).not.toHaveBeenCalledWith( + expect.stringContaining('main recovery was unavailable'), + expect.any(Function) + ) + + vi.advanceTimersByTime(1) + vi.advanceTimersByTime(0) + await flushAsyncTicks(10) + + const written = pane.terminal.write.mock.calls.map(([data]) => data as string) + const warningIndex = written.findIndex((data) => data.includes('main recovery was unavailable')) + const combinedLiveIndex = written.indexOf(firstLive + secondLive) + expect(warningIndex).toBeGreaterThanOrEqual(0) + expect(combinedLiveIndex).toBeGreaterThan(warningIndex) + + snapshot.resolve({ + data: 'late-snapshot-state\r\n', + cols: 100, + rows: 30, + seq: hidden.length + firstLive.length + secondLive.length + }) + await flushAsyncTicks(20) + + expect(pane.terminal.write).not.toHaveBeenCalledWith( + 'late-snapshot-state\r\n', + expect.any(Function) + ) + disposable.dispose() + }) + + it('falls back after repeated null hidden restore retries and drains blocked foreground', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockResolvedValue(null) + const hidden = 'hidden-codex-output\r\n' + const live = 'visible-after-null-retries\r\n' + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + vi.useFakeTimers() + capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(live, { + seq: hidden.length + live.length, + rawLength: live.length + }) + await flushAsyncTicks(10) + + for (let attempt = 0; attempt < 3; attempt++) { + expect(pane.terminal.write).not.toHaveBeenCalledWith(live, expect.any(Function)) + vi.advanceTimersByTime(50) + vi.advanceTimersByTime(0) + await flushAsyncTicks(10) + } + + const written = pane.terminal.write.mock.calls.map(([data]) => data as string) + const warningIndex = written.findIndex((data) => data.includes('main recovery was unavailable')) + const liveIndex = written.indexOf(live) + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(4) + expect(warningIndex).toBeGreaterThanOrEqual(0) + expect(liveIndex).toBeGreaterThan(warningIndex) + disposable.dispose() + }) + + it('drops pending foreground overflow when a stalled hidden restore falls back', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockReturnValue( + createDeferred<{ data: string; cols: number; rows: number; seq: number }>().promise + ) + const hidden = 'hidden-codex-output\r\n' + const liveOverflow = 'v'.repeat(512 * 1024 + 1) + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + vi.useFakeTimers() + capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(liveOverflow, { + seq: hidden.length + liveOverflow.length, + rawLength: liveOverflow.length + }) + await flushAsyncTicks(4) + + vi.advanceTimersByTime(750) + await vi.runAllTimersAsync() + await flushAsyncTicks(20) + vi.advanceTimersByTime(0) + await flushAsyncTicks(10) + + expect(pane.terminal.write).toHaveBeenCalledWith( + expect.stringContaining('main recovery was unavailable'), + expect.any(Function) + ) + expect(pane.terminal.write).not.toHaveBeenCalledWith(liveOverflow, expect.any(Function)) + disposable.dispose() + }) + + it('coalesces tiny pending foreground chunks when stalled hidden restore falls back', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + getMainBufferSnapshot.mockReturnValue( + createDeferred<{ data: string; cols: number; rows: number; seq: number }>().promise + ) + const hidden = 'hidden-codex-output\r\n' + const chunkCount = 2_000 + const liveChunk = 'x' + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + vi.useFakeTimers() + capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length }) + ;(deps.isVisibleRef as { current: boolean }).current = true + for (let index = 0; index < chunkCount; index += 1) { + capturedDataCallback.current?.(liveChunk, { + seq: hidden.length + index + 1, + rawLength: liveChunk.length + }) + } + await flushAsyncTicks(4) + + vi.advanceTimersByTime(750) + vi.advanceTimersByTime(0) + await flushAsyncTicks(10) + + const written = pane.terminal.write.mock.calls.map(([data]) => data as string) + const warningIndex = written.findIndex((data) => data.includes('main recovery was unavailable')) + const combinedLive = liveChunk.repeat(chunkCount) + const liveWrites = written.filter((data) => data === combinedLive) + expect(warningIndex).toBeGreaterThanOrEqual(0) + expect(liveWrites).toHaveLength(1) + expect(written.indexOf(combinedLive)).toBeGreaterThan(warningIndex) + + disposable.dispose() + expect(vi.getTimerCount()).toBe(0) + }) + it('keeps foreground output when hidden-backlog snapshot recovery is unavailable', async () => { const pendingTimeouts: { canceled: boolean @@ -6324,6 +6567,106 @@ describe('connectPanePty', () => { } }) + it('keeps a newer same-PTY hidden restore after a timed-out snapshot resolves late', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-id') + const capturedDataCallback: { + current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType< + typeof vi.fn + > + const firstSnapshot = createDeferred<{ + data: string + cols: number + rows: number + seq: number + }>() + const secondSnapshot = createDeferred<{ + data: string + cols: number + rows: number + seq: number + }>() + getMainBufferSnapshot + .mockReturnValueOnce(firstSnapshot.promise) + .mockReturnValueOnce(secondSnapshot.promise) + const firstHidden = 'first-hidden-output\r\n' + const firstLive = 'first-live-output\r\n' + const secondHidden = 'second-hidden-output\r\n' + const secondLive = 'second-live-output\r\n' + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false }, + startup: { command: 'codex' } + }) + const disposable = connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + vi.useFakeTimers() + capturedDataCallback.current?.(firstHidden, { + seq: firstHidden.length, + rawLength: firstHidden.length + }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(firstLive, { + seq: firstHidden.length + firstLive.length, + rawLength: firstLive.length + }) + await flushAsyncTicks(4) + vi.advanceTimersByTime(750) + vi.advanceTimersByTime(0) + await flushAsyncTicks(10) + + pane.terminal.write.mockClear() + ;(deps.isVisibleRef as { current: boolean }).current = false + capturedDataCallback.current?.(secondHidden, { + seq: firstHidden.length + firstLive.length + secondHidden.length, + rawLength: secondHidden.length + }) + ;(deps.isVisibleRef as { current: boolean }).current = true + capturedDataCallback.current?.(secondLive, { + seq: firstHidden.length + firstLive.length + secondHidden.length + secondLive.length, + rawLength: secondLive.length + }) + await flushAsyncTicks(4) + expect(getMainBufferSnapshot).toHaveBeenCalledTimes(2) + + firstSnapshot.resolve({ + data: 'stale-first-snapshot\r\n', + cols: 100, + rows: 30, + seq: firstHidden.length + firstLive.length + }) + await flushAsyncTicks(10) + secondSnapshot.resolve({ + data: 'fresh-second-snapshot\r\n', + cols: 100, + rows: 30, + seq: firstHidden.length + firstLive.length + secondHidden.length + secondLive.length + }) + await flushAsyncTicks(20) + + expect(pane.terminal.write).not.toHaveBeenCalledWith( + 'stale-first-snapshot\r\n', + expect.any(Function) + ) + expect(pane.terminal.write).toHaveBeenCalledWith( + 'fresh-second-snapshot\r\n', + expect.any(Function) + ) + expect(pane.terminal.write).not.toHaveBeenCalledWith(secondLive, expect.any(Function)) + disposable.dispose() + }) + it('ignores an async hidden-backlog snapshot if the pane changes PTYs first', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('old-pty-id') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 64bbae2443e..7475a1d2634 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -141,6 +141,7 @@ const HIDDEN_OUTPUT_RESTORE_SCROLLBACK_ROWS = 5000 const HIDDEN_OUTPUT_RESTORE_PENDING_CHARS = 512 * 1024 const HIDDEN_OUTPUT_RESTORE_DEFERRED_RETRY_MS = 50 const HIDDEN_OUTPUT_RESTORE_DEFERRED_RETRY_MAX = 3 +const HIDDEN_OUTPUT_RESTORE_FOREGROUND_TIMEOUT_MS = 750 const TERMINAL_RENDERER_RISK_SCAN_TAIL_CHARS = 256 const CURSOR_SHOW_SEQUENCE = '\x1b[?25h' const CURSOR_HIDE_SEQUENCE = '\x1b[?25l' @@ -811,6 +812,7 @@ export function connectPanePty( let unregisterBacklogRecovery: (() => void) | null = null let unregisterDocumentVisibilityRecovery: (() => void) | null = null let cleanupHiddenOutputRestoreDeferredRetry = (): void => {} + let cleanupHiddenOutputRestoreForegroundDeadline = (): void => {} let unregisterE2ePtyDataInjection = (): void => {} let startupInjectTimer: ReturnType | null = null let sshShellReadyFallbackTimer: ReturnType | null = null @@ -2619,6 +2621,7 @@ export function connectPanePty( let hiddenOutputRestoreRetryDeferred = false let hiddenOutputRestoreScheduled = false let hiddenOutputRestoreDeferredRetryTimer: ReturnType | null = null + let hiddenOutputRestoreForegroundDeadlineTimer: ReturnType | null = null let hiddenOutputRestoreDeferredRetryAttempts = 0 // Why: hidden recovery state belongs to one PTY stream. Reattach/restart // can reuse the pane object for a different session before visibility. @@ -2856,7 +2859,8 @@ export function connectPanePty( (synchronizedForegroundOutput || nativeWindowsCursorRestore || foregroundRenderRefreshNeeded), - followupForegroundRefresh: nativeWindowsCursorRestore || nativeWindowsInPlaceRewriteFollowup, + followupForegroundRefresh: + nativeWindowsCursorRestore || nativeWindowsInPlaceRewriteFollowup, stripTransientCursorShows: shouldProtectNativeWindowsSynchronizedOutput && foreground, coalesceForeground: synchronizedForegroundOutput && synchronizedOutputEnded, holdForeground: synchronizedForegroundOutput && nextSynchronizedForegroundOutputActive @@ -3030,6 +3034,7 @@ export function connectPanePty( hiddenOutputRestorePendingChunks = [] hiddenOutputRestorePendingChars = 0 hiddenOutputRestorePendingOverflow = true + armHiddenOutputRestoreForegroundDeadline() return } const pending: PendingHiddenOutputRestoreChunk = { data } @@ -3041,6 +3046,7 @@ export function connectPanePty( } hiddenOutputRestorePendingChunks.push(pending) hiddenOutputRestorePendingChars += data.length + armHiddenOutputRestoreForegroundDeadline() } function getChunkDataAfterSnapshot( @@ -3109,34 +3115,10 @@ export function connectPanePty( hiddenOutputRestoreScheduled = false cancelScheduledHiddenOutputRestore(pane.terminal) clearHiddenOutputRestoreDeferredRetryTimer() + clearHiddenOutputRestoreForegroundDeadlineTimer() hiddenOutputRestoreDeferredRetryAttempts = 0 } - function drainPendingLiveChunksWithoutSnapshot(): void { - if (hiddenOutputRestorePendingOverflow) { - hiddenOutputRestorePendingChunks = [] - hiddenOutputRestorePendingChars = 0 - hiddenOutputRestorePendingOverflow = false - return - } - // Why: once snapshot retries are exhausted, these bounded chunks are the - // only known visible-era PTY bytes; replay them without overlap trimming. - while (hiddenOutputRestorePendingChunks.length > 0) { - const chunks = hiddenOutputRestorePendingChunks - hiddenOutputRestorePendingChunks = [] - hiddenOutputRestorePendingChars = 0 - for (const chunk of chunks) { - writePtyOutputToXterm(chunk.data, true) - } - if (hiddenOutputRestorePendingOverflow) { - hiddenOutputRestorePendingChunks = [] - hiddenOutputRestorePendingChars = 0 - hiddenOutputRestorePendingOverflow = false - return - } - } - } - function clearHiddenOutputRestoreDeferredRetryTimer(): void { if (hiddenOutputRestoreDeferredRetryTimer === null) { return @@ -3146,6 +3128,81 @@ export function connectPanePty( } cleanupHiddenOutputRestoreDeferredRetry = clearHiddenOutputRestoreDeferredRetryTimer + function clearHiddenOutputRestoreForegroundDeadlineTimer(): void { + if (hiddenOutputRestoreForegroundDeadlineTimer === null) { + return + } + clearTimeout(hiddenOutputRestoreForegroundDeadlineTimer) + hiddenOutputRestoreForegroundDeadlineTimer = null + } + cleanupHiddenOutputRestoreForegroundDeadline = clearHiddenOutputRestoreForegroundDeadlineTimer + + function armHiddenOutputRestoreForegroundDeadline(): void { + if ( + disposed || + hiddenOutputRestoreForegroundDeadlineTimer !== null || + !shouldWritePtyOutputForeground(deps.isVisibleRef.current) || + (hiddenOutputRestorePendingChunks.length === 0 && !hiddenOutputRestorePendingOverflow) + ) { + return + } + const ptyId = hiddenOutputRestorePtyId + if (ptyId === null || transport.getPtyId() !== ptyId) { + return + } + const deadlineGeneration = hiddenOutputRestoreGeneration + // Why: only foreground-visible output blocked behind recovery gets a + // deadline; hidden-time restore work can continue without user impact. + hiddenOutputRestoreForegroundDeadlineTimer = setTimeout(() => { + hiddenOutputRestoreForegroundDeadlineTimer = null + if ( + disposed || + hiddenOutputRestoreGeneration !== deadlineGeneration || + hiddenOutputRestorePtyId !== ptyId || + !shouldWritePtyOutputForeground(deps.isVisibleRef.current) + ) { + return + } + abandonHiddenOutputRestoreAndDrainPendingForeground(ptyId) + }, HIDDEN_OUTPUT_RESTORE_FOREGROUND_TIMEOUT_MS) + } + + function abandonHiddenOutputRestoreAndDrainPendingForeground(expectedPtyId: string): void { + if (transport.getPtyId() !== expectedPtyId || hiddenOutputRestorePtyId !== expectedPtyId) { + resetHiddenOutputRestoreIfPtyChanged() + return + } + const pendingChunks = hiddenOutputRestorePendingOverflow + ? [] + : hiddenOutputRestorePendingChunks.slice() + const hadPendingOverflow = hiddenOutputRestorePendingOverflow + hiddenOutputRestoreGeneration += 1 + hiddenOutputRestoreInFlight = null + hiddenOutputRestoreNeeded = false + hiddenOutputRestorePtyId = null + hiddenOutputRestorePendingChunks = [] + hiddenOutputRestorePendingChars = 0 + hiddenOutputRestorePendingOverflow = false + hiddenOutputRestoreFreshSnapshotNeeded = false + hiddenOutputRestoreRetryDeferred = false + hiddenOutputRestoreScheduled = false + hiddenStartupRendererQueryPending = '' + hiddenRendererStateDirty = false + cancelScheduledHiddenOutputRestore(pane.terminal) + clearHiddenOutputRestoreDeferredRetryTimer() + clearHiddenOutputRestoreForegroundDeadlineTimer() + hiddenOutputRestoreDeferredRetryAttempts = 0 + + writeRestoreUnavailableWarning() + if (hadPendingOverflow) { + return + } + const pendingData = pendingChunks.map((chunk) => chunk.data).join('') + if (pendingData) { + writePtyOutputToXterm(pendingData, true) + } + } + function scheduleHiddenOutputRestoreDeferredRetry(): void { if ( disposed || @@ -3155,9 +3212,13 @@ export function connectPanePty( return } if (hiddenOutputRestoreDeferredRetryAttempts >= HIDDEN_OUTPUT_RESTORE_DEFERRED_RETRY_MAX) { - writeRestoreUnavailableWarning() - drainPendingLiveChunksWithoutSnapshot() - clearHiddenOutputRestoreState() + const ptyId = hiddenOutputRestorePtyId + if (ptyId !== null) { + abandonHiddenOutputRestoreAndDrainPendingForeground(ptyId) + } else { + clearHiddenOutputRestoreState() + writeRestoreUnavailableWarning() + } return } hiddenOutputRestoreDeferredRetryAttempts += 1 @@ -3283,6 +3344,7 @@ export function connectPanePty( } hiddenOutputRestorePtyId = ptyId if (hiddenOutputRestoreInFlight) { + armHiddenOutputRestoreForegroundDeadline() return true } if (!opts?.bypassScheduler) { @@ -3356,14 +3418,15 @@ export function connectPanePty( if (disposed) { return } - if ( - hiddenOutputRestoreGeneration !== restoreGeneration || - transport.getPtyId() !== currentPtyId || - hiddenOutputRestorePtyId !== currentPtyId - ) { + const restoreGenerationChanged = hiddenOutputRestoreGeneration !== restoreGeneration + const restorePtyChanged = + transport.getPtyId() !== currentPtyId || hiddenOutputRestorePtyId !== currentPtyId + if (restoreGenerationChanged || restorePtyChanged) { // Why: the snapshot belongs to the requested PTY; after reattach, // replaying it would show stale/cleared output in the new terminal. - if (hiddenOutputRestorePtyId === currentPtyId) { + // A stale generation may be an abandoned timeout while a newer + // restore for the same PTY owns the current hidden-recovery state. + if (restorePtyChanged && hiddenOutputRestorePtyId === currentPtyId) { clearHiddenOutputRestoreState() } return @@ -3382,6 +3445,7 @@ export function connectPanePty( if (drainPendingLiveChunksAfterSnapshot(snapshot.seq) && !needsFreshSnapshot) { hiddenOutputRestoreNeeded = false hiddenOutputRestorePtyId = null + clearHiddenOutputRestoreForegroundDeadlineTimer() return } if (!shouldWritePtyOutputForeground(deps.isVisibleRef.current)) { @@ -3393,10 +3457,16 @@ export function connectPanePty( } hiddenOutputRestoreNeeded = true } - })().finally(() => { - hiddenOutputRestoreInFlight = null + })() + const hiddenOutputRestoreTask = hiddenOutputRestoreInFlight + let trackedHiddenOutputRestore: Promise + trackedHiddenOutputRestore = hiddenOutputRestoreTask.finally(() => { + if (hiddenOutputRestoreInFlight === trackedHiddenOutputRestore) { + hiddenOutputRestoreInFlight = null + } if (hiddenOutputRestorePendingChunks.length > 0 || hiddenOutputRestorePendingOverflow) { hiddenOutputRestoreNeeded = true + armHiddenOutputRestoreForegroundDeadline() } if ( !hiddenOutputRestoreRetryDeferred && @@ -3406,6 +3476,7 @@ export function connectPanePty( requestHiddenOutputRestoreIfNeeded() } }) + hiddenOutputRestoreInFlight = trackedHiddenOutputRestore return true } @@ -4300,6 +4371,7 @@ export function connectPanePty( clearTerminalBellNotificationTimer() clearReattachIdleAgentCursorResetTimer() cleanupHiddenOutputRestoreDeferredRetry() + cleanupHiddenOutputRestoreForegroundDeadline() unregisterBacklogRecovery?.() unregisterBacklogRecovery = null unregisterDocumentVisibilityRecovery?.() diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 0e4ae069858..632f976a53d 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -4597,7 +4597,16 @@ "tintOpacity": "Tint Strength", "tintOpacityDescription": "Controls how strongly the tint is mixed into the sidebar." }, - "workspaceCardLayoutGuidance": "Use the workspace sidebar options menu > Card layout > Compact." + "workspaceCardLayoutGuidance": "Managed from the workspace sidebar.", + "interfaceDefaultFont": "Default font", + "terminalDefaultFont": "Default font", + "interfaceTitle": "Interface", + "terminalTitle": "Terminal", + "windowSidebarTitle": "Window & Sidebar", + "windowSidebarSummary": "Sidebar, status bar, and file explorer", + "statusBarCount": "{{value0}} indicators visible.", + "gitIgnoredGlossary": "Files matched by .gitignore.", + "statusBarDescription": "Choose which indicators appear in the status bar." }, "AutoRenameBranchFromWorkSetting": { "1626524572": "Nautilus", @@ -6248,7 +6257,9 @@ "4415beb958": "disabled", "4e7d41a9f0": "enabled", "e90afcc44f": "off", - "16c471ee03": "on" + "16c471ee03": "on", + "typographyAdvanced": "Typography", + "dimUnfocusedPanes": "Dim unfocused panes." }, "TerminalFontSizeSetting": { "9b5252c85a": "px", @@ -6778,7 +6789,7 @@ }, "workspaceCardLayout": { "title": "Workspace Card Layout", - "description": "Switch between compact and detailed workspace cards from the workspace sidebar options menu.", + "description": "Workspace cards can use compact or detailed layouts.", "compact": "compact", "compactDisplay": "compact display", "workspaceCards": "workspace cards", @@ -8360,6 +8371,9 @@ "wslUnavailable": "WSL is not available on this machine.", "distroRequired": "Choose a WSL distro before projects can inherit WSL.", "wslDescription": "Detect and launch agents in {{value0}} via WSL for projects that do not override their runtime." + }, + "AppearanceAdvancedDisclosure": { + "advanced": "Advanced" } }, "right": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 8a79c6e3aa2..281be816a65 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -4594,10 +4594,19 @@ "tintOpacity": "Intensidad del tinte", "tintOpacityDescription": "Controla con qué fuerza se mezcla el tinte en la barra lateral." }, - "workspaceCardLayoutGuidance": "Usa el menú de opciones de la barra lateral de espacios de trabajo > Diseño de tarjeta > Compacto.", + "workspaceCardLayoutGuidance": "Gestionado desde la barra lateral del espacio de trabajo.", "872af9556e": "Bandeja del sistema", "2edf606c46": "Minimizar a la bandeja al cerrar", - "b707773a0d": "Cuando está activado, cerrar la ventana mantiene Orca en ejecución en la bandeja del sistema en lugar de salir." + "b707773a0d": "Cuando está activado, cerrar la ventana mantiene Orca en ejecución en la bandeja del sistema en lugar de salir.", + "interfaceDefaultFont": "Fuente predeterminada", + "terminalDefaultFont": "Fuente predeterminada", + "interfaceTitle": "Interfaz", + "terminalTitle": "Terminal", + "windowSidebarTitle": "Ventana y barra lateral", + "windowSidebarSummary": "Barra lateral, barra de estado y explorador de archivos", + "statusBarCount": "Indicadores visibles: {{value0}}.", + "gitIgnoredGlossary": "Archivos coincidentes con .gitignore.", + "statusBarDescription": "Choose which indicators appear in the status bar." }, "AutoRenameBranchFromWorkSetting": { "1626524572": "Nautilus", @@ -6211,7 +6220,9 @@ "4415beb958": "desactivado", "4e7d41a9f0": "activado", "e90afcc44f": "apagado", - "16c471ee03": "en" + "16c471ee03": "activado", + "typographyAdvanced": "Tipografía", + "dimUnfocusedPanes": "Atenuar paneles sin foco." }, "TerminalFontSizeSetting": { "9b5252c85a": "píxeles", @@ -6739,7 +6750,7 @@ }, "workspaceCardLayout": { "title": "Diseño de tarjetas de espacios de trabajo", - "description": "Cambia entre tarjetas de espacios de trabajo compactas y detalladas desde el menú de opciones de la barra lateral de espacios de trabajo.", + "description": "Las tarjetas de espacios de trabajo pueden usar diseños compactos o detallados.", "compact": "compacto", "compactDisplay": "vista compacta", "workspaceCards": "tarjetas de espacios de trabajo", @@ -8360,6 +8371,9 @@ "wslUnavailable": "WSL is not available on this machine.", "distroRequired": "Choose a WSL distro before projects can inherit WSL.", "wslDescription": "Detect and launch agents in {{value0}} via WSL for projects that do not override their runtime." + }, + "AppearanceAdvancedDisclosure": { + "advanced": "Avanzado" } }, "right": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 09040f48da7..4e14b21d7dd 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -4579,10 +4579,19 @@ "tintOpacity": "色合いの強さ", "tintOpacityDescription": "サイドバーに色合いをどの程度強く混ぜるかを調整します。" }, - "workspaceCardLayoutGuidance": "ワークスペースサイドバーのオプションメニュー > カードレイアウト > コンパクト を使用します。", + "workspaceCardLayoutGuidance": "ワークスペースサイドバーで管理されます。", "872af9556e": "システムトレイ", "2edf606c46": "閉じるときにトレイへ最小化", - "b707773a0d": "有効にすると、ウィンドウを閉じてもOrcaは終了せず、システムトレイで実行を続けます。" + "b707773a0d": "有効にすると、ウィンドウを閉じてもOrcaは終了せず、システムトレイで実行を続けます。", + "interfaceDefaultFont": "デフォルトフォント", + "terminalDefaultFont": "デフォルトフォント", + "interfaceTitle": "インターフェイス", + "terminalTitle": "ターミナル", + "windowSidebarTitle": "ウィンドウとサイドバー", + "windowSidebarSummary": "サイドバー、ステータスバー、ファイルエクスプローラー", + "statusBarCount": "{{value0}} 個のインジケーターが表示中。", + "gitIgnoredGlossary": ".gitignore に一致するファイル。", + "statusBarDescription": "Choose which indicators appear in the status bar." }, "AutoRenameBranchFromWorkSetting": { "1626524572": "Nautilus", @@ -6233,7 +6242,9 @@ "4415beb958": "無効", "4e7d41a9f0": "有効", "e90afcc44f": "オフ", - "16c471ee03": "の上" + "16c471ee03": "オン", + "typographyAdvanced": "タイポグラフィ", + "dimUnfocusedPanes": "フォーカスされていないペインを暗くします。" }, "TerminalFontSizeSetting": { "9b5252c85a": "ピクセル", @@ -6761,7 +6772,7 @@ }, "workspaceCardLayout": { "title": "ワークスペースカードのレイアウト", - "description": "ワークスペースサイドバーのオプションメニューから、コンパクト表示と詳細表示のワークスペースカードを切り替えます。", + "description": "ワークスペースカードはコンパクトまたは詳細レイアウトを使用できます。", "compact": "コンパクト", "compactDisplay": "コンパクト表示", "workspaceCards": "ワークスペースカード", @@ -8360,6 +8371,9 @@ "wslUnavailable": "WSL is not available on this machine.", "distroRequired": "Choose a WSL distro before projects can inherit WSL.", "wslDescription": "Detect and launch agents in {{value0}} via WSL for projects that do not override their runtime." + }, + "AppearanceAdvancedDisclosure": { + "advanced": "詳細設定" } }, "right": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 0dc37894aac..64591eea997 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -4582,7 +4582,16 @@ "tintOpacity": "색조 강도", "tintOpacityDescription": "사이드바에 색조를 얼마나 강하게 섞을지 조절합니다." }, - "workspaceCardLayoutGuidance": "워크스페이스 사이드바 옵션 메뉴 > 카드 레이아웃 > 컴팩트를 사용하세요." + "workspaceCardLayoutGuidance": "워크스페이스 사이드바에서 관리됩니다.", + "interfaceDefaultFont": "기본 글꼴", + "terminalDefaultFont": "기본 글꼴", + "interfaceTitle": "인터페이스", + "terminalTitle": "터미널", + "windowSidebarTitle": "창 및 사이드바", + "windowSidebarSummary": "사이드바, 상태 표시줄 및 파일 탐색기", + "statusBarCount": "{{value0}}개 표시기가 보입니다.", + "gitIgnoredGlossary": ".gitignore와 일치하는 파일.", + "statusBarDescription": "Choose which indicators appear in the status bar." }, "AutoRenameBranchFromWorkSetting": { "1626524572": "Nautilus", @@ -6196,7 +6205,9 @@ "4415beb958": "비활성", "4e7d41a9f0": "활성화됨", "e90afcc44f": "끔", - "16c471ee03": "켜짐" + "16c471ee03": "켜짐", + "typographyAdvanced": "타이포그래피", + "dimUnfocusedPanes": "포커스되지 않은 창을 흐리게 표시합니다." }, "TerminalFontSizeSetting": { "9b5252c85a": "px", @@ -6726,7 +6737,7 @@ "4d5b9427b5": "활성화하면 창을 닫아도 Orca가 종료되지 않고 시스템 트레이에서 계속 실행됩니다.", "workspaceCardLayout": { "title": "워크스페이스 카드 레이아웃", - "description": "워크스페이스 사이드바 옵션 메뉴에서 워크스페이스 카드를 컴팩트 또는 상세 보기로 전환합니다.", + "description": "워크스페이스 카드는 컴팩트 또는 상세 레이아웃을 사용할 수 있습니다.", "compact": "컴팩트", "compactDisplay": "컴팩트 보기", "workspaceCards": "워크스페이스 카드", @@ -8360,6 +8371,9 @@ "wslUnavailable": "WSL is not available on this machine.", "distroRequired": "Choose a WSL distro before projects can inherit WSL.", "wslDescription": "Detect and launch agents in {{value0}} via WSL for projects that do not override their runtime." + }, + "AppearanceAdvancedDisclosure": { + "advanced": "고급" } }, "right": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 089b4e141dd..aea5fd20e0f 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -4579,10 +4579,19 @@ "tintOpacity": "色调强度", "tintOpacityDescription": "控制色调混入边栏的强度。" }, - "workspaceCardLayoutGuidance": "使用工作区侧边栏选项菜单 > 卡片布局 > 紧凑。", + "workspaceCardLayoutGuidance": "从工作区侧边栏管理。", "872af9556e": "系统托盘", "2edf606c46": "关闭时最小化到托盘", - "b707773a0d": "启用后，关闭窗口会让 Orca 继续在系统托盘中运行，而不是退出。" + "b707773a0d": "启用后，关闭窗口会让 Orca 继续在系统托盘中运行，而不是退出。", + "interfaceDefaultFont": "默认字体", + "terminalDefaultFont": "默认字体", + "interfaceTitle": "界面", + "terminalTitle": "终端", + "windowSidebarTitle": "窗口和侧边栏", + "windowSidebarSummary": "侧边栏、状态栏和文件浏览器", + "statusBarCount": "显示 {{value0}} 个指示器。", + "gitIgnoredGlossary": "与 .gitignore 匹配的文件。", + "statusBarDescription": "Choose which indicators appear in the status bar." }, "AutoRenameBranchFromWorkSetting": { "1626524572": "Nautilus", @@ -6196,7 +6205,9 @@ "4415beb958": "已禁用", "4e7d41a9f0": "已启用", "e90afcc44f": "关", - "16c471ee03": "开" + "16c471ee03": "开", + "typographyAdvanced": "排版", + "dimUnfocusedPanes": "调暗未聚焦的窗格。" }, "TerminalFontSizeSetting": { "9b5252c85a": "像素", @@ -6724,7 +6735,7 @@ }, "workspaceCardLayout": { "title": "工作区卡片布局", - "description": "从工作区侧边栏选项菜单在紧凑和详细工作区卡片之间切换。", + "description": "工作区卡片可以使用紧凑或详细布局。", "compact": "紧凑", "compactDisplay": "紧凑显示", "workspaceCards": "工作区卡片", @@ -8360,6 +8371,9 @@ "wslUnavailable": "WSL is not available on this machine.", "distroRequired": "Choose a WSL distro before projects can inherit WSL.", "wslDescription": "Detect and launch agents in {{value0}} via WSL for projects that do not override their runtime." + }, + "AppearanceAdvancedDisclosure": { + "advanced": "高级" } }, "right": { diff --git a/src/shared/agent-process-recognition.test.ts b/src/shared/agent-process-recognition.test.ts index 865d5105c25..297e8e405f9 100644 --- a/src/shared/agent-process-recognition.test.ts +++ b/src/shared/agent-process-recognition.test.ts @@ -140,9 +140,7 @@ describe('agent process recognition', () => { agent: 'qwen-code', processName: 'qwen' }) - expect( - recognizeAgentProcess(String.raw`C:\Users\dev\AppData\Roaming\npm\qwen.cmd`) - ).toEqual({ + expect(recognizeAgentProcess(String.raw`C:\Users\dev\AppData\Roaming\npm\qwen.cmd`)).toEqual({ agent: 'qwen-code', processName: 'qwen' }) diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index 6dcfcbba44f..4d724b630ab 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -850,11 +850,7 @@ describe('keybindings', () => { // Ctrl+Shift+C on the same layout (terminal copy) must match too. expect( - keybindingMatchesAction( - 'terminal.copySelection', - { ...cyrillicCtrlC, shift: true }, - 'win32' - ) + keybindingMatchesAction('terminal.copySelection', { ...cyrillicCtrlC, shift: true }, 'win32') ).toBe(true) // Greek layout: physical P produces 'π' (U+03C0); Ctrl+P must still match. From d6f3f514d6b555c49a3cb0a6b6f3e7082af23921 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 28 Jun 2026 11:58:02 -0700 Subject: [PATCH 04/12] Fix agent session row drag restore (#6463) Co-authored-by: Neil --- .../right-sidebar/AiVaultSessionRow.tsx | 19 ++++- .../SessionRowTrailingActions.tsx | 74 +------------------ 2 files changed, 19 insertions(+), 74 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx index eab39c0f2d3..c6b209b02f3 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx @@ -17,6 +17,7 @@ import { useRepoById } from '@/store/selectors' import { resolveRepoBadgeColor } from '../../../../shared/repo-badge-color' import { splitWorktreeIdForFilesystem } from '../../../../shared/worktree-id' import { + AI_VAULT_SESSION_DRAG_END_EVENT, AI_VAULT_SESSION_DRAG_START_EVENT, writeAiVaultSessionDragData } from '@/lib/ai-vault-session-drag' @@ -82,8 +83,13 @@ export function VaultSessionRow({ ? translate('auto.components.right.sidebar.AiVaultSessionRow.hideDetails', 'Hide Details') : translate('auto.components.right.sidebar.AiVaultSessionRow.showDetails', 'Show Details') const startResumeDrag = useCallback( - (event: React.DragEvent): void => { + (event: React.DragEvent): void => { event.stopPropagation() + const target = event.target + if (target instanceof Element && target.closest('[data-ai-vault-session-actions]')) { + event.preventDefault() + return + } if (resumeDisabled) { event.preventDefault() return @@ -106,12 +112,20 @@ export function VaultSessionRow({