From 0645729f68e12526f8227369747cc931dc920a85 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:16 -0700 Subject: [PATCH 01/31] perf(worktrees): resolve watcher targets concurrently (#17166) --- ...ktree-base-directory-watch-targets.test.ts | 232 ++++++++++++++++++ .../worktree-base-directory-watch-targets.ts | 54 +++- 2 files changed, 276 insertions(+), 10 deletions(-) create mode 100644 src/main/ipc/worktree-base-directory-watch-targets.test.ts diff --git a/src/main/ipc/worktree-base-directory-watch-targets.test.ts b/src/main/ipc/worktree-base-directory-watch-targets.test.ts new file mode 100644 index 00000000000..250b3d7625d --- /dev/null +++ b/src/main/ipc/worktree-base-directory-watch-targets.test.ts @@ -0,0 +1,232 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join, sep } from 'node:path' +import type { GlobalSettings } from '../../shared/global-settings-types' +import type { Repo } from '../../shared/repo-types' + +const { getSshFilesystemProviderMock, readFileMock, realpathMock, statMock } = vi.hoisted(() => ({ + getSshFilesystemProviderMock: vi.fn(), + readFileMock: vi.fn(), + realpathMock: vi.fn(), + statMock: vi.fn() +})) + +vi.mock('node:fs/promises', () => ({ + readFile: readFileMock, + realpath: realpathMock, + stat: statMock +})) + +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + getSshFilesystemProvider: getSshFilesystemProviderMock +})) + +import { + buildWorktreeBaseDirectoryWatchTargets, + clearWorktreeBaseDirectoryWatchTargetWarnings, + WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY +} from './worktree-base-directory-watch-targets' + +const absolutePath = (...parts: string[]): string => join(sep, ...parts) +const WORKTREE_ROOT = absolutePath('workspace', 'worktrees') +const PROJECT_ROOT = absolutePath('workspace', 'projects') +const localDirectoryStat = { isDirectory: () => true } +const remoteDirectoryStat = { type: 'directory', size: 0, mtime: 0 } +const settings = { + workspaceDir: WORKTREE_ROOT, + nestWorkspaces: true +} as GlobalSettings + +function makeRepo(index: number, overrides: Partial = {}): Repo { + return { + id: `repo-${index}`, + path: join(PROJECT_ROOT, `project-${index}`), + displayName: `Project ${index}`, + badgeColor: '#000000', + addedAt: index, + ...overrides + } as Repo +} + +function makeStore(repos: Repo[]) { + return { + getSettings: () => settings, + getRepos: () => repos + } +} + +function makeRemoteProvider() { + return { + stat: vi.fn(async () => remoteDirectoryStat), + realpath: vi.fn(async (path: string) => path), + readFile: vi.fn(async () => ({ content: '', isBinary: false })) + } +} + +describe('worktree base directory watch target resolution', () => { + let warnSpy: ReturnType + + beforeEach(() => { + statMock.mockReset().mockResolvedValue(localDirectoryStat) + realpathMock.mockReset().mockImplementation(async (path: string) => path) + readFileMock.mockReset().mockResolvedValue('') + getSshFilesystemProviderMock.mockReset().mockReturnValue(undefined) + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(() => { + clearWorktreeBaseDirectoryWatchTargetWarnings() + warnSpy.mockRestore() + }) + + it('bounds resolution concurrency and starts one successor per released slot', async () => { + const repos = Array.from( + { length: WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY + 4 }, + (_, index) => makeRepo(index) + ) + const gates = repos.map(() => Promise.withResolvers()) + let startedRootStats = 0 + let activeRootStats = 0 + let peakRootStats = 0 + statMock.mockImplementation(async (path: string) => { + if (path === WORKTREE_ROOT) { + const gate = gates[startedRootStats++] + activeRootStats++ + peakRootStats = Math.max(peakRootStats, activeRootStats) + await gate.promise + activeRootStats-- + } + return localDirectoryStat + }) + + const resultPromise = buildWorktreeBaseDirectoryWatchTargets(makeStore(repos) as never) + await vi.waitFor(() => { + expect(startedRootStats).toBe(WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY) + }) + expect(peakRootStats).toBe(WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY) + + gates[0].resolve() + await vi.waitFor(() => { + expect(startedRootStats).toBe(WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY + 1) + }) + expect(peakRootStats).toBe(WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY) + + for (const gate of gates) { + gate.resolve() + } + await resultPromise + expect(startedRootStats).toBe(repos.length) + expect(peakRootStats).toBe(WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY) + }) + + it('merges targets and shared repo membership in store order after reverse completion', async () => { + const repos = Array.from({ length: 4 }, (_, index) => makeRepo(index)) + const gates = repos.map(() => Promise.withResolvers()) + const gitStatStarted = new Set() + const completionOrder: number[] = [] + statMock.mockImplementation(async (path: string) => { + const index = repos.findIndex((repo) => path === join(repo.path, '.git')) + if (index !== -1) { + gitStatStarted.add(index) + await gates[index].promise + completionOrder.push(index) + } + return localDirectoryStat + }) + + const resultPromise = buildWorktreeBaseDirectoryWatchTargets(makeStore(repos) as never) + await vi.waitFor(() => expect(gitStatStarted.size).toBe(repos.length)) + for (let index = repos.length - 1; index >= 0; index--) { + gates[index].resolve() + await vi.waitFor(() => expect(completionOrder).toContain(index)) + } + const targets = await resultPromise + + expect(completionOrder).toEqual([3, 2, 1, 0]) + expect([...targets.values()].map((target) => [target.kind, target.path])).toEqual([ + ['base', WORKTREE_ROOT], + ...repos.map((repo) => ['git-common', join(repo.path, '.git')]) + ]) + expect([...([...targets.values()][0]?.repos.keys() ?? [])]).toEqual( + repos.map((repo) => repo.id) + ) + }) + + it('isolates missing paths to the affected repo', async () => { + const missing = Object.assign(new Error('missing'), { code: 'ENOENT' }) + const goodBefore = makeRepo(0, { worktreeBasePath: absolutePath('worktrees', 'before') }) + const unavailable = makeRepo(1, { worktreeBasePath: absolutePath('worktrees', 'missing') }) + const goodAfter = makeRepo(2, { worktreeBasePath: absolutePath('worktrees', 'after') }) + statMock.mockImplementation(async (path: string) => { + if (path === unavailable.worktreeBasePath || path === join(unavailable.path, '.git')) { + throw missing + } + return localDirectoryStat + }) + + const targets = await buildWorktreeBaseDirectoryWatchTargets( + makeStore([goodBefore, unavailable, goodAfter]) as never + ) + const repoIds = [...targets.values()].flatMap((target) => [...target.repos.keys()]) + + expect(repoIds).toContain(goodBefore.id) + expect(repoIds).not.toContain(unavailable.id) + expect(repoIds).toContain(goodAfter.id) + }) + + it('keeps identical SSH paths isolated by connection and provider', async () => { + const providerA = makeRemoteProvider() + const providerB = makeRemoteProvider() + getSshFilesystemProviderMock.mockImplementation((connectionId: string) => + connectionId === 'ssh-a' ? providerA : connectionId === 'ssh-b' ? providerB : undefined + ) + const sharedPath = '/srv/project' + const repos = [ + makeRepo(0, { path: sharedPath, connectionId: 'ssh-a' }), + makeRepo(1, { path: sharedPath, connectionId: 'ssh-b' }) + ] + + const targets = await buildWorktreeBaseDirectoryWatchTargets(makeStore(repos) as never) + + expect(providerA.stat).toHaveBeenCalledTimes(2) + expect(providerA.realpath).toHaveBeenCalledTimes(2) + expect(providerB.stat).toHaveBeenCalledTimes(2) + expect(providerB.realpath).toHaveBeenCalledTimes(2) + expect([...targets.values()].map((target) => target.connectionId)).toEqual([ + 'ssh-a', + 'ssh-a', + 'ssh-b', + 'ssh-b' + ]) + expect(new Set(targets.keys()).size).toBe(4) + }) + + it('skips folder workspaces, unavailable SSH providers, and WSL UNC roots', async () => { + const folder = makeRepo(0, { kind: 'folder' }) + const unavailableSsh = makeRepo(1, { connectionId: 'missing', path: '/srv/missing' }) + const wslPath = '\\\\wsl.localhost\\Ubuntu\\home\\alice\\project' + const wsl = makeRepo(2, { path: wslPath, worktreeBasePath: wslPath }) + + const targets = await buildWorktreeBaseDirectoryWatchTargets( + makeStore([folder, unavailableSsh, wsl]) as never + ) + + expect(targets.size).toBe(0) + expect(statMock).not.toHaveBeenCalled() + expect(getSshFilesystemProviderMock).toHaveBeenCalledWith('missing') + }) + + it('does not publish ordered partial targets when a repo resolver rejects unexpectedly', async () => { + const completed = makeRepo(0) + const broken = makeRepo(1) + Object.defineProperty(broken, 'path', { + get: () => { + throw new Error('unexpected resolver failure') + } + }) + + await expect( + buildWorktreeBaseDirectoryWatchTargets(makeStore([completed, broken]) as never) + ).rejects.toThrow('unexpected resolver failure') + expect(statMock).toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/worktree-base-directory-watch-targets.ts b/src/main/ipc/worktree-base-directory-watch-targets.ts index cfa825e09ab..5300475593d 100644 --- a/src/main/ipc/worktree-base-directory-watch-targets.ts +++ b/src/main/ipc/worktree-base-directory-watch-targets.ts @@ -15,6 +15,7 @@ import { resolveRuntimePath } from '../../shared/cross-platform-path' import { isWslUncPath } from '../../shared/wsl-paths' +import { mapWithConcurrency } from '../../shared/map-with-concurrency' import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { computeWorkspaceRoot, @@ -32,6 +33,9 @@ import type { const missingRootWarnings = new Set() const skippedWslWarnings = new Set() +// Why: match existing worktree probe caps while bounding aggregate SSH filesystem RPC pressure. +export const WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY = 8 + function normalizeWatchKey(pathValue: string): string { return normalizeRuntimePathForComparison(normalize(pathValue)) } @@ -177,21 +181,51 @@ async function maybeAddBaseTarget( } } +async function resolveRepoTargets( + repo: Repo, + settings: GlobalSettings +): Promise> { + const targets = new Map() + if (isFolderRepo(repo)) { + return targets + } + const executionHostId = getRepoExecutionHostId(repo) + if (executionHostId === LOCAL_EXECUTION_HOST_ID) { + await maybeAddBaseTarget(targets, repo, settings) + } else if (repo.connectionId) { + await maybeAddBaseTarget(targets, repo, settings, repo.connectionId) + } + return targets +} + +function mergeRepoTargets( + targets: Map, + repoTargets: Map +): void { + for (const [key, target] of repoTargets) { + const existing = targets.get(key) + if (!existing) { + targets.set(key, target) + continue + } + for (const [repoId, config] of target.repos) { + existing.repos.set(repoId, config) + } + } +} + export async function buildWorktreeBaseDirectoryWatchTargets( store: Store ): Promise> { const settings = store.getSettings() + const resolvedRepoTargets = await mapWithConcurrency( + store.getRepos(), + WORKTREE_BASE_TARGET_RESOLUTION_CONCURRENCY, + (repo) => resolveRepoTargets(repo, settings) + ) const targets = new Map() - for (const repo of store.getRepos()) { - if (isFolderRepo(repo)) { - continue - } - const executionHostId = getRepoExecutionHostId(repo) - if (executionHostId === LOCAL_EXECUTION_HOST_ID) { - await maybeAddBaseTarget(targets, repo, settings) - } else if (repo.connectionId) { - await maybeAddBaseTarget(targets, repo, settings, repo.connectionId) - } + for (const repoTargets of resolvedRepoTargets) { + mergeRepoTargets(targets, repoTargets) } return targets } From bf884ea8667817727f7dce6a86aa2c0e68c42be6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:20 -0700 Subject: [PATCH 02/31] perf(startup): skip disabled diagnostic serialization (#17176) --- ...rsistence-loading-store-extraction.test.ts | 81 ++++++++++++++++++- .../loading-store/loaded-state-parsing.ts | 14 ++-- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/main/persistence-loading-store-extraction.test.ts b/src/main/persistence-loading-store-extraction.test.ts index dafb6deadee..e48971d6c5c 100644 --- a/src/main/persistence-loading-store-extraction.test.ts +++ b/src/main/persistence-loading-store-extraction.test.ts @@ -2,14 +2,22 @@ import { afterEach, beforeEach, describe, expect, expectTypeOf, it, vi } from 'v import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { getDefaultWorkspaceSession } from '../shared/constants' +import { getDefaultPersistedState, getDefaultWorkspaceSession } from '../shared/constants' import type { PersistedState } from '../shared/persisted-state-types' +import type * as StartupDiagnosticsModule from './startup/startup-diagnostics' import type { Store as PersistenceStore } from './persistence/loading-store/store' -import { createStore, dataFile, makeRepo, testState } from './persistence-test-harness' +import { + createStore, + dataFile, + makeRepo, + testState, + writeDataFile +} from './persistence-test-harness' -const { trackMock, getCohortAtEmitMock } = vi.hoisted(() => ({ +const { trackMock, getCohortAtEmitMock, logStartupDiagnosticMock } = vi.hoisted(() => ({ trackMock: vi.fn(), - getCohortAtEmitMock: vi.fn(() => ({ nth_repo_added: 2 })) + getCohortAtEmitMock: vi.fn(() => ({ nth_repo_added: 2 })), + logStartupDiagnosticMock: vi.fn() })) vi.mock('electron', () => ({ @@ -33,6 +41,10 @@ vi.mock('./ssh/ssh-config-parser', () => ({ loadUserSshConfig: vi.fn(() => ({ hosts: [] })), sshConfigHostsToTargets: vi.fn(() => []) })) +vi.mock('./startup/startup-diagnostics', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, logStartupDiagnostic: logStartupDiagnosticMock } +}) describe('loading Store extraction seams', () => { beforeEach(() => { @@ -40,9 +52,70 @@ describe('loading Store extraction seams', () => { }) afterEach(() => { + vi.unstubAllEnvs() + logStartupDiagnosticMock.mockReset() rmSync(testState.dir, { recursive: true, force: true }) }) + it('does not serialize the workspace session when startup diagnostics are disabled', () => { + const sentinel = 'startup-diagnostics-workspace-session-sentinel-disabled' + vi.stubEnv('ORCA_STARTUP_DIAGNOSTICS', '') + const state = getDefaultPersistedState(testState.dir) + state.workspaceSession = { ...state.workspaceSession, activeTabId: sentinel } + writeDataFile(state) + + const stringifySpy = vi.spyOn(JSON, 'stringify') + const store = createStore() + store.freezeWrites() + + const workspaceSessionStringifyCalls = stringifySpy.mock.calls.filter( + ([value]) => + value && + typeof value === 'object' && + (value as { activeTabId?: unknown }).activeTabId === sentinel + ) + stringifySpy.mockRestore() + + expect(store.getWorkspaceSession().activeTabId).toBe(sentinel) + expect(workspaceSessionStringifyCalls).toHaveLength(0) + expect( + logStartupDiagnosticMock.mock.calls.some(([event]) => event === 'persistence-load-done') + ).toBe(false) + }) + + it('reports the unchanged workspace-session byte count when startup diagnostics are enabled', () => { + const sentinel = 'startup-diagnostics-workspace-session-sentinel-enabled' + vi.stubEnv('ORCA_STARTUP_DIAGNOSTICS', '1') + const state = getDefaultPersistedState(testState.dir) + state.workspaceSession = { ...state.workspaceSession, activeTabId: sentinel } + writeDataFile(state) + + const stringifySpy = vi.spyOn(JSON, 'stringify') + const store = createStore() + store.freezeWrites() + + const workspaceSessionStringifyCalls = stringifySpy.mock.calls.filter( + ([value]) => + value && + typeof value === 'object' && + (value as { activeTabId?: unknown }).activeTabId === sentinel + ) + stringifySpy.mockRestore() + + expect(store.getWorkspaceSession().activeTabId).toBe(sentinel) + expect(workspaceSessionStringifyCalls).toHaveLength(1) + const loadDoneCall = logStartupDiagnosticMock.mock.calls.find( + ([event]) => event === 'persistence-load-done' + ) + expect(loadDoneCall).toBeDefined() + const details = loadDoneCall?.[1] as Record | undefined + expect(details).toEqual({ + t: expect.any(Number), + repos: state.repos.length, + workspaceSessionBytes: Buffer.byteLength(JSON.stringify(store.getWorkspaceSession())) + }) + }) + it('accepts the first JSON-parseable backup even when an older backup has richer state', async () => { mkdirSync(testState.dir, { recursive: true }) writeFileSync(dataFile(), '{{corrupt-primary', 'utf-8') diff --git a/src/main/persistence/loading-store/loaded-state-parsing.ts b/src/main/persistence/loading-store/loaded-state-parsing.ts index 2f28c771508..5bd6e572ab1 100644 --- a/src/main/persistence/loading-store/loaded-state-parsing.ts +++ b/src/main/persistence/loading-store/loaded-state-parsing.ts @@ -42,13 +42,17 @@ import { prepareLoadedTerminalSettings } from './prepare-loaded-terminal-setting import { prepareLoadedProfileSettings } from './prepare-loaded-profile-settings' import { normalizeLoadedProfileState } from './normalize-loaded-profile-state' +type PersistenceStartupDetails = Record | (() => Record) + function logPersistenceStartupMilestone( event: string, - details: Record = {} + details: PersistenceStartupDetails = {} ): void { - if (isStartupDiagnosticsEnabled()) { - logStartupDiagnostic(event, { t: Math.round(performance.now()), ...details }) + if (!isStartupDiagnosticsEnabled()) { + return } + const resolvedDetails = typeof details === 'function' ? details() : details + logStartupDiagnostic(event, { t: Math.round(performance.now()), ...resolvedDetails }) } import type { StoreRuntimeState } from './store-runtime-state' @@ -279,10 +283,10 @@ export class LoadedStateParsingOperations { migrated.githubCache = readGithubCacheSnapshot(this.runtime.dataFile) ?? migrated.githubCache } - logPersistenceStartupMilestone('persistence-load-done', { + logPersistenceStartupMilestone('persistence-load-done', () => ({ repos: migrated.repos.length, workspaceSessionBytes: Buffer.byteLength(JSON.stringify(migrated.workspaceSession)) - }) + })) return migrated } } From 80a0595449d78f12b9610c704da80b5ff40df6a9 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:23 -0700 Subject: [PATCH 03/31] perf(startup): stop rescanning persisted pane tabs (#17179) * perf(startup): index persisted pane tabs once * perf(startup): lazily resolve persisted pane tabs --- .../pane-identity-migration.ts | 66 ++++++- ...workspace-pane-normalization-index.test.ts | 184 ++++++++++++++++++ .../workspace-pane-normalization.ts | 9 +- 3 files changed, 247 insertions(+), 12 deletions(-) create mode 100644 src/main/persistence/restoring-sessions/workspace-pane-normalization-index.test.ts diff --git a/src/main/persistence/restoring-sessions/pane-identity-migration.ts b/src/main/persistence/restoring-sessions/pane-identity-migration.ts index e07e9995286..98d97a649a3 100644 --- a/src/main/persistence/restoring-sessions/pane-identity-migration.ts +++ b/src/main/persistence/restoring-sessions/pane-identity-migration.ts @@ -1,5 +1,5 @@ import type { LegacyPaneKeyAliasEntry } from '../../../shared/persisted-state-types' -import type { TerminalLayoutSnapshot } from '../../../shared/terminal-tab-types' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../shared/terminal-tab-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { isTerminalLeafId, makePaneKey } from '../../../shared/stable-pane-id' import { agentHookServer } from '../../agent-hooks/server' @@ -17,23 +17,69 @@ export function findWorktreeIdForTab( return undefined } +export type TerminalTabLookup = { + get(tabId: string): TerminalTab | undefined +} + +/** Resolves tabs on demand while retaining first-match ordering and scan progress. */ +export function createLazyTerminalTabLookup(session: WorkspaceSessionState): TerminalTabLookup { + const tabsByWorktree = Object.entries(session.tabsByWorktree ?? {}) + const tabsById = new Map() + let worktreeIndex = 0 + let tabIndex = 0 + let exhausted = false + + return { + get(requestedTabId: string): TerminalTab | undefined { + if (tabsById.has(requestedTabId)) { + return tabsById.get(requestedTabId) + } + if (exhausted) { + return undefined + } + + while (worktreeIndex < tabsByWorktree.length) { + const tabs = tabsByWorktree[worktreeIndex][1] + while (tabIndex < tabs.length) { + const currentIndex = tabIndex + tabIndex += 1 + // Array#some, used by the prior lookup, skips sparse holes. + if (!(currentIndex in tabs)) { + continue + } + const tab = tabs[currentIndex] + const tabId = tab.id + // Preserve first-match behavior when persisted IDs collide. + if (!tabsById.has(tabId)) { + tabsById.set(tabId, tab) + } + if (tabId === requestedTabId) { + return tabsById.get(requestedTabId) + } + } + worktreeIndex += 1 + tabIndex = 0 + } + + exhausted = true + return undefined + } + } +} + /** Bridges a tab's legacy numeric pane keys to stable ones; returns the alias rows worth persisting. */ export function registerLegacyPaneKeyAliasesForTab(args: { - session: WorkspaceSessionState tabId: string + tab: TerminalTab | undefined inputLayout: TerminalLayoutSnapshot normalizedLayout: TerminalLayoutSnapshot leafIdByInputLeafId: Map }): LegacyPaneKeyAliasEntry[] { - const worktreeId = findWorktreeIdForTab(args.session, args.tabId) - const tab = worktreeId - ? args.session.tabsByWorktree?.[worktreeId]?.find((entry) => entry.id === args.tabId) - : undefined const legacyPaneKeyAliasEntries: LegacyPaneKeyAliasEntry[] = [] const registeredLegacyPaneKeys = new Set() const hasLeafPtyBindings = Object.keys(args.inputLayout.ptyIdsByLeafId ?? {}).length > 0 const fallbackPtyId = - !hasLeafPtyBindings && typeof tab?.ptyId === 'string' ? tab.ptyId : undefined + !hasLeafPtyBindings && typeof args.tab?.ptyId === 'string' ? args.tab.ptyId : undefined const registerLegacyAlias = (inputLeafId: string, leafId: string, ptyId?: string): boolean => { if (!isTerminalLeafId(leafId)) { return false @@ -80,7 +126,7 @@ export function registerLegacyPaneKeyAliasesForTab(args: { ) } } - if (tab?.ptyId && !hasLeafPtyBindings) { + if (args.tab?.ptyId && !hasLeafPtyBindings) { const fallbackLeafId = args.normalizedLayout.activeLeafId ?? firstLayoutLeafId(args.normalizedLayout.root) let paneKey: string | undefined @@ -96,9 +142,9 @@ export function registerLegacyPaneKeyAliasesForTab(args: { if (registeredLegacyPaneKeys.has(legacyPaneKey)) { continue } - agentHookServer.registerPaneKeyAlias(legacyPaneKey, paneKey, tab.ptyId) + agentHookServer.registerPaneKeyAlias(legacyPaneKey, paneKey, args.tab.ptyId) legacyPaneKeyAliasEntries.push({ - ptyId: tab.ptyId, + ptyId: args.tab.ptyId, legacyPaneKey, stablePaneKey: paneKey, updatedAt: Date.now() diff --git a/src/main/persistence/restoring-sessions/workspace-pane-normalization-index.test.ts b/src/main/persistence/restoring-sessions/workspace-pane-normalization-index.test.ts new file mode 100644 index 00000000000..9cebe041544 --- /dev/null +++ b/src/main/persistence/restoring-sessions/workspace-pane-normalization-index.test.ts @@ -0,0 +1,184 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { createLazyTerminalTabLookup } from './pane-identity-migration' +import { normalizeWorkspaceSessionPaneIdentities } from './workspace-pane-normalization' + +const { registerPaneKeyAliasMock } = vi.hoisted(() => ({ + registerPaneKeyAliasMock: vi.fn() +})) + +vi.mock('../../agent-hooks/server', () => ({ + agentHookServer: { + registerPaneKeyAlias: registerPaneKeyAliasMock + } +})) + +const STABLE_LEAF_ID = '00000000-0000-4000-8000-000000000001' + +function terminalTab(id: string, worktreeId: string, ptyId: string | null): TerminalTab { + return { + id, + worktreeId, + ptyId, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function workspaceSession( + tabsByWorktree: Record, + terminalLayoutsByTabId: Record +): WorkspaceSessionState { + return { + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree, + terminalLayoutsByTabId + } +} + +function legacyLayout(): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: 'pane:1' }, + activeLeafId: 'pane:1', + expandedLeafId: null + } +} + +function stableLayout(): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: STABLE_LEAF_ID }, + activeLeafId: STABLE_LEAF_ID, + expandedLeafId: null + } +} + +describe('workspace pane normalization tab index', () => { + beforeEach(() => { + registerPaneKeyAliasMock.mockReset() + }) + + it('keeps the first tab when duplicate ids occur across worktrees', () => { + const result = normalizeWorkspaceSessionPaneIdentities( + workspaceSession( + { + first: [terminalTab('duplicate-tab', 'first', 'first-pty')], + second: [terminalTab('duplicate-tab', 'second', 'second-pty')] + }, + { 'duplicate-tab': legacyLayout() } + ) + ) + + expect(result.legacyPaneKeyAliasEntries).not.toHaveLength(0) + expect(result.legacyPaneKeyAliasEntries.every((entry) => entry.ptyId === 'first-pty')).toBe( + true + ) + }) + + it('keeps missing tab ids without inventing persisted aliases', () => { + const result = normalizeWorkspaceSessionPaneIdentities( + workspaceSession({}, { 'missing-tab': legacyLayout() }) + ) + + expect(result.legacyPaneKeyAliasEntries).toEqual([]) + }) + + it('reads each tab id once regardless of the number of layouts', () => { + const tabCount = 256 + let idReads = 0 + const tabs = Array.from({ length: tabCount }, (_, index) => { + const id = `tab-${index}` + const tab = terminalTab(id, 'worktree', null) + Object.defineProperty(tab, 'id', { + enumerable: true, + get: () => { + idReads += 1 + return id + } + }) + return tab + }) + const layouts = Object.fromEntries( + Array.from({ length: tabCount }, (_, index) => [`tab-${index}`, stableLayout()]) + ) + + normalizeWorkspaceSessionPaneIdentities(workspaceSession({ worktree: tabs }, layouts)) + + expect(idReads).toBe(tabCount) + }) + + it('does not inspect tabs after an early match', () => { + const first = terminalTab('first', 'worktree', 'first-pty') + const later = terminalTab('later', 'worktree', 'later-pty') + Object.defineProperty(later, 'id', { + enumerable: true, + get: () => { + throw new Error('later tab should not be inspected') + } + }) + + const lookup = createLazyTerminalTabLookup(workspaceSession({ worktree: [first, later] }, {})) + + expect(lookup.get('first')).toBe(first) + }) + + it('resumes scanning and makes later misses constant-time', () => { + const reads: string[] = [] + const tabs = ['first', 'second', 'third'].map((id) => { + const tab = terminalTab(id, 'worktree', null) + Object.defineProperty(tab, 'id', { + enumerable: true, + get: () => { + reads.push(id) + return id + } + }) + return tab + }) + const lookup = createLazyTerminalTabLookup(workspaceSession({ worktree: tabs }, {})) + + expect(lookup.get('first')).toBe(tabs[0]) + expect(reads).toEqual(['first']) + expect(lookup.get('second')).toBe(tabs[1]) + expect(reads).toEqual(['first', 'second']) + expect(lookup.get('missing')).toBeUndefined() + expect(reads).toEqual(['first', 'second', 'third']) + expect(lookup.get('missing')).toBeUndefined() + expect(reads).toEqual(['first', 'second', 'third']) + }) + + it('skips sparse tab-array holes', () => { + const tabs: TerminalTab[] = [] + tabs.length = 3 + tabs[1] = terminalTab('target', 'worktree', null) + const lookup = createLazyTerminalTabLookup(workspaceSession({ worktree: tabs }, {})) + + expect(lookup.get('target')).toBe(tabs[1]) + expect(lookup.get('missing')).toBeUndefined() + }) + + it('does not read tab ids when every layout is skipped', () => { + const tab = terminalTab('skipped', 'worktree', null) + let idReads = 0 + Object.defineProperty(tab, 'id', { + enumerable: true, + get: () => { + idReads += 1 + return 'skipped' + } + }) + + normalizeWorkspaceSessionPaneIdentities( + workspaceSession({ worktree: [tab] }, { skipped: legacyLayout() }), + {}, + { skipAliasTabIds: new Set(['skipped']) } + ) + + expect(idReads).toBe(0) + }) +}) diff --git a/src/main/persistence/restoring-sessions/workspace-pane-normalization.ts b/src/main/persistence/restoring-sessions/workspace-pane-normalization.ts index a977134bd0e..2bfafebae9f 100644 --- a/src/main/persistence/restoring-sessions/workspace-pane-normalization.ts +++ b/src/main/persistence/restoring-sessions/workspace-pane-normalization.ts @@ -10,7 +10,10 @@ import { import type { SshRemotePtyLease } from '../../../shared/ssh-types' import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id' import { findCrossHostPaneTabIds, withoutPaneTabIds } from './cross-host-pane-tab-ids' -import { registerLegacyPaneKeyAliasesForTab } from './pane-identity-migration' +import { + createLazyTerminalTabLookup, + registerLegacyPaneKeyAliasesForTab +} from './pane-identity-migration' import { normalizeTerminalLayoutSnapshotForPersistence } from './terminal-layout-normalization' import { legacyMigrationUnsupportedRowsToAliasEntries, @@ -40,6 +43,7 @@ export function normalizeWorkspaceSessionPaneIdentities( const migrationUnsupportedEntries: MigrationUnsupportedPtyEntry[] = [] const legacyPaneKeyAliasEntries: LegacyPaneKeyAliasEntry[] = [] const terminalLayoutsByTabId: Record = {} + let tabsById: ReturnType | null = null for (const [tabId, layout] of Object.entries(session.terminalLayoutsByTabId ?? {})) { const normalized = normalizeTerminalLayoutSnapshotForPersistence( layout, @@ -48,9 +52,10 @@ export function normalizeWorkspaceSessionPaneIdentities( terminalLayoutsByTabId[tabId] = normalized.snapshot leafIdByInputLeafIdByTabId.set(tabId, normalized.leafIdByInputLeafId) if (!options.skipAliasTabIds?.has(tabId)) { + tabsById ??= createLazyTerminalTabLookup(session) const tabAliasEntries = registerLegacyPaneKeyAliasesForTab({ - session, tabId, + tab: tabsById.get(tabId), inputLayout: layout, normalizedLayout: normalized.snapshot, leafIdByInputLeafId: normalized.leafIdByInputLeafId From eb6cd6721965f959476cedc550a0fb2e69fa715e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:27 -0700 Subject: [PATCH 04/31] perf(editor): skip unchanged review-note reflows (#17212) --- .../DiffCommentCard.resize.test.tsx | 51 ++++++++++++++++++- .../diff-comments/DiffCommentCard.tsx | 18 ++++--- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/renderer/src/components/diff-comments/DiffCommentCard.resize.test.tsx b/src/renderer/src/components/diff-comments/DiffCommentCard.resize.test.tsx index f17dfbb7613..648b33357d4 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentCard.resize.test.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentCard.resize.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment happy-dom -import { act, cleanup, render } from '@testing-library/react' +import { act, cleanup, fireEvent, render } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { DiffCommentCard } from './DiffCommentCard' @@ -44,6 +44,7 @@ describe('DiffCommentCard content resize', () => { afterEach(() => { cleanup() + vi.restoreAllMocks() vi.unstubAllGlobals() }) @@ -101,4 +102,52 @@ describe('DiffCommentCard content resize', () => { expect(constructObserver).not.toHaveBeenCalled() expect(onContentResize).not.toHaveBeenCalled() }) + + it('requests rail reflow only when the editing textarea height changes', () => { + const onContentResize = vi.fn() + let scrollHeight = 60 + vi.spyOn(HTMLTextAreaElement.prototype, 'scrollHeight', 'get').mockImplementation( + () => scrollHeight + ) + const view = render( + true} + /> + ) + + fireEvent.click(view.getByRole('button', { name: 'Edit note' })) + expect(onContentResize).toHaveBeenCalledOnce() + + const textarea = view.getByRole('textbox') + expect(textarea.style.height).toBe('60px') + fireEvent.change(textarea, { target: { value: 'A saved note!' } }) + expect(onContentResize).toHaveBeenCalledOnce() + + for (let index = 0; index < 50; index++) { + fireEvent.change(textarea, { target: { value: `A saved note! ${index}` } }) + } + expect(textarea.style.height).toBe('60px') + expect(onContentResize).toHaveBeenCalledOnce() + + scrollHeight = 400 + fireEvent.change(textarea, { target: { value: 'A much taller saved note' } }) + expect(textarea.style.height).toBe('240px') + expect(onContentResize).toHaveBeenCalledTimes(2) + + scrollHeight = 480 + fireEvent.change(textarea, { target: { value: 'A still taller saved note' } }) + expect(textarea.style.height).toBe('240px') + expect(onContentResize).toHaveBeenCalledTimes(2) + + scrollHeight = 80 + fireEvent.change(textarea, { target: { value: 'A shorter saved note' } }) + expect(textarea.style.height).toBe('80px') + expect(onContentResize).toHaveBeenCalledTimes(3) + + fireEvent.click(view.getByRole('button', { name: 'Cancel' })) + expect(onContentResize).toHaveBeenCalledTimes(4) + }) }) diff --git a/src/renderer/src/components/diff-comments/DiffCommentCard.tsx b/src/renderer/src/components/diff-comments/DiffCommentCard.tsx index 9e7ddf229ce..6becfe1fb34 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentCard.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentCard.tsx @@ -35,6 +35,13 @@ type Props = { headerActions?: ReactNode } +function resizeDiffCommentTextarea(textarea: HTMLTextAreaElement): boolean { + const previousHeight = textarea.style.height + textarea.style.height = 'auto' + textarea.style.height = `${Math.min(textarea.scrollHeight, 240)}px` + return textarea.style.height !== previousHeight +} + export function DiffCommentCard({ lineNumber, startLine, @@ -116,8 +123,7 @@ export function DiffCommentCard({ if (!el) { return } - el.style.height = 'auto' - el.style.height = `${Math.min(el.scrollHeight, 240)}px` + resizeDiffCommentTextarea(el) el.focus() el.setSelectionRange(el.value.length, el.value.length) onContentResizeRef.current?.() @@ -281,10 +287,10 @@ export function DiffCommentCard({ value={draft} onChange={(e) => { setDraft(e.target.value) - const el = e.currentTarget - el.style.height = 'auto' - el.style.height = `${Math.min(el.scrollHeight, 240)}px` - onContentResizeRef.current?.() + // Why: rich-review layout measures every note; skip it when this card stayed put. + if (resizeDiffCommentTextarea(e.currentTarget)) { + onContentResizeRef.current?.() + } }} onKeyDown={(e) => { if (e.key === 'Escape') { From 7879dead69b1101301c9b861e494702800ee8220 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:29 -0700 Subject: [PATCH 05/31] perf(ssh): cache legacy worktree clean fallback (#17213) --- .../ssh-git-provider-worktree.test.ts | 225 ++++++++++++++++-- .../providers/ssh-git-worktree-provider.ts | 69 +++--- 2 files changed, 247 insertions(+), 47 deletions(-) diff --git a/src/main/providers/ssh-git-provider-worktree.test.ts b/src/main/providers/ssh-git-provider-worktree.test.ts index 10f36acc8fd..6ddff0a1f68 100644 --- a/src/main/providers/ssh-git-provider-worktree.test.ts +++ b/src/main/providers/ssh-git-provider-worktree.test.ts @@ -1,6 +1,16 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' import { SshGitProvider } from './ssh-git-provider' -import { createMockMux, type MockMultiplexer } from './ssh-git-provider-test-harness' +import { + createMockMux, + waitForRequestCount, + type MockMultiplexer +} from './ssh-git-provider-test-harness' + +function methodNotFound(method: string): Error & { code: number } { + return Object.assign(new Error(`Method not found: ${method}`), { code: -32601 }) +} + +const CLEAN_STATUS = { entries: [], conflictOperation: 'unknown' } describe('SshGitProvider', () => { let mux: MockMultiplexer @@ -67,6 +77,40 @@ describe('SshGitProvider', () => { expect(result).toEqual(cleanResult) }) + it('keeps supported clean checks on the preferred relay RPC', async () => { + let resolveProbe!: (result: { clean: boolean }) => void + let resolveRemaining!: (result: { clean: boolean }) => void + const probe = new Promise<{ clean: boolean }>((resolve) => { + resolveProbe = resolve + }) + const remaining = new Promise<{ clean: boolean }>((resolve) => { + resolveRemaining = resolve + }) + let preferredRequestCount = 0 + mux.request.mockImplementation((method) => { + expect(method).toBe('git.worktreeIsClean') + preferredRequestCount += 1 + return preferredRequestCount === 1 ? probe : remaining + }) + const checks = Array.from({ length: 10 }, (_, index) => + provider.worktreeIsClean(`/repo/worktree-${index}`) + ) + + await waitForRequestCount(mux.request, 1) + expect(mux.request).toHaveBeenCalledOnce() + resolveProbe({ clean: true }) + await waitForRequestCount(mux.request, 10) + expect(mux.request).toHaveBeenCalledTimes(10) + resolveRemaining({ clean: true }) + await expect(Promise.all(checks)).resolves.toEqual( + Array.from({ length: 10 }, () => ({ clean: true })) + ) + expect( + mux.request.mock.calls.filter(([method]) => method === 'git.worktreeIsClean') + ).toHaveLength(10) + expect(mux.request.mock.calls.some(([method]) => method === 'git.status')).toBe(false) + }) + it('worktreeIsClean can ignore untracked files', async () => { const cleanResult = { clean: true } mux.request.mockResolvedValue(cleanResult) @@ -113,10 +157,7 @@ describe('SshGitProvider', () => { }) it('worktreeIsClean falls back to git.status for old relays', async () => { - const methodNotFound = Object.assign(new Error('Method not found: git.worktreeIsClean'), { - code: -32601 - }) - mux.request.mockRejectedValueOnce(methodNotFound).mockResolvedValueOnce({ + mux.request.mockRejectedValueOnce(methodNotFound('git.worktreeIsClean')).mockResolvedValueOnce({ entries: [{ path: 'scratch.txt', status: 'untracked', area: 'untracked' }], conflictOperation: 'unknown' }) @@ -140,26 +181,180 @@ describe('SshGitProvider', () => { } }) - it('worktreeIsClean filters untracked entries in old-relay fallback', async () => { - const methodNotFound = Object.assign(new Error('Method not found: git.worktreeIsClean'), { - code: -32601 + it('probes an old relay once across sequential clean checks', async () => { + mux.request.mockImplementation(async (method) => { + if (method === 'git.worktreeIsClean') { + throw methodNotFound(method) + } + return CLEAN_STATUS }) - mux.request.mockRejectedValueOnce(methodNotFound).mockResolvedValueOnce({ - entries: [{ path: 'scratch.txt', status: 'untracked', area: 'untracked' }], - conflictOperation: 'unknown' + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const worktreePaths = Array.from({ length: 10 }, (_, index) => `/repo/worktree-${index}`) + + try { + for (const worktreePath of worktreePaths) { + await expect(provider.worktreeIsClean(worktreePath)).resolves.toEqual({ clean: true }) + } + + expect( + mux.request.mock.calls.filter(([method]) => method === 'git.worktreeIsClean') + ).toHaveLength(1) + expect(mux.request.mock.calls.filter(([method]) => method === 'git.status')).toHaveLength(10) + expect(mux.request).toHaveBeenCalledTimes(11) + expect(warnSpy).toHaveBeenCalledOnce() + } finally { + warnSpy.mockRestore() + } + }) + + it('shares one old-relay probe across concurrent clean checks', async () => { + let rejectProbe!: (error: Error) => void + const probe = new Promise((_resolve, reject) => { + rejectProbe = reject + }) + mux.request.mockImplementation((method) => { + if (method === 'git.worktreeIsClean') { + return probe + } + return Promise.resolve(CLEAN_STATUS) + }) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const checks = Array.from({ length: 10 }, (_, index) => + provider.worktreeIsClean(`/repo/worktree-${index}`) + ) + + try { + await waitForRequestCount(mux.request, 1) + rejectProbe(methodNotFound('git.worktreeIsClean')) + await expect(Promise.all(checks)).resolves.toEqual( + Array.from({ length: 10 }, () => ({ clean: true })) + ) + + expect( + mux.request.mock.calls.filter(([method]) => method === 'git.worktreeIsClean') + ).toHaveLength(1) + expect(mux.request.mock.calls.filter(([method]) => method === 'git.status')).toHaveLength(10) + expect(mux.request).toHaveBeenCalledTimes(11) + expect(warnSpy).toHaveBeenCalledOnce() + } finally { + warnSpy.mockRestore() + } + }) + + it('retries the preferred clean RPC after a transport failure', async () => { + const transportError = Object.assign(new Error('Method not found: git.worktreeIsClean'), { + code: -32602 + }) + mux.request.mockRejectedValueOnce(transportError).mockResolvedValueOnce({ clean: true }) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + try { + await expect(provider.worktreeIsClean('/repo/first')).rejects.toBe(transportError) + await expect(provider.worktreeIsClean('/repo/second')).resolves.toEqual({ clean: true }) + expect(mux.request.mock.calls.map(([method]) => method)).toEqual([ + 'git.worktreeIsClean', + 'git.worktreeIsClean' + ]) + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('lets concurrent waiters retry after a transient probe failure', async () => { + const transportError = new Error('connection closed') + let rejectProbe!: (error: Error) => void + let resolveRetries!: (result: { clean: boolean }) => void + const probe = new Promise((_resolve, reject) => { + rejectProbe = reject + }) + const retries = new Promise<{ clean: boolean }>((resolve) => { + resolveRetries = resolve + }) + let preferredRequestCount = 0 + mux.request.mockImplementation((method) => { + expect(method).toBe('git.worktreeIsClean') + preferredRequestCount += 1 + return preferredRequestCount === 1 ? probe : retries + }) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const checks = Array.from({ length: 3 }, (_, index) => + provider.worktreeIsClean(`/repo/worktree-${index}`) + ) + const settledChecks = Promise.allSettled(checks) + + try { + await waitForRequestCount(mux.request, 1) + rejectProbe(transportError) + await waitForRequestCount(mux.request, 3) + resolveRetries({ clean: true }) + + const results = await settledChecks + expect(results[0]).toEqual({ status: 'rejected', reason: transportError }) + expect(results.slice(1)).toEqual([ + { status: 'fulfilled', value: { clean: true } }, + { status: 'fulfilled', value: { clean: true } } + ]) + await expect(provider.worktreeIsClean('/repo/later')).resolves.toEqual({ clean: true }) + expect(mux.request).toHaveBeenCalledTimes(4) + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('re-probes clean support after the provider is replaced', async () => { + mux.request.mockImplementation(async (method) => { + if (method === 'git.worktreeIsClean') { + throw methodNotFound(method) + } + return CLEAN_STATUS }) const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) try { + await expect(provider.worktreeIsClean('/repo/first')).resolves.toEqual({ clean: true }) + const replacement = new SshGitProvider('conn-1', mux as never) + await expect(replacement.worktreeIsClean('/repo/second')).resolves.toEqual({ clean: true }) + + expect( + mux.request.mock.calls.filter(([method]) => method === 'git.worktreeIsClean') + ).toHaveLength(2) + expect(warnSpy).toHaveBeenCalledTimes(2) + } finally { + warnSpy.mockRestore() + } + }) + + it('worktreeIsClean filters untracked entries in old-relay fallback', async () => { + mux.request.mockImplementation(async (method) => { + if (method === 'git.worktreeIsClean') { + throw methodNotFound(method) + } + return { + entries: [{ path: 'scratch.txt', status: 'untracked', area: 'untracked' }], + conflictOperation: 'unknown' + } + }) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + try { + await expect(provider.worktreeIsClean('/home/user/feat')).resolves.toEqual({ + clean: false, + stdout: 'untracked untracked: scratch.txt' + }) await expect( provider.worktreeIsClean('/home/user/feat', { includeUntracked: false }) ).resolves.toEqual({ clean: true }) expect(mux.request).toHaveBeenNthCalledWith( - 2, + 3, 'git.status', { worktreePath: '/home/user/feat' }, { signal: expect.any(AbortSignal) } ) + expect( + mux.request.mock.calls.filter(([method]) => method === 'git.worktreeIsClean') + ).toHaveLength(1) } finally { warnSpy.mockRestore() } @@ -183,11 +378,7 @@ describe('SshGitProvider', () => { }) it('forceDeletePreservedBranch maps old relays to the reconnect message', async () => { - const methodNotFound = Object.assign( - new Error('Method not found: git.forceDeletePreservedBranch'), - { code: -32601 } - ) - mux.request.mockRejectedValueOnce(methodNotFound) + mux.request.mockRejectedValueOnce(methodNotFound('git.forceDeletePreservedBranch')) await expect( provider.forceDeletePreservedBranch('/home/user/repo', 'you/fix-auth', 'abc123') diff --git a/src/main/providers/ssh-git-worktree-provider.ts b/src/main/providers/ssh-git-worktree-provider.ts index ce509ccf847..d5cf9b5a5e1 100644 --- a/src/main/providers/ssh-git-worktree-provider.ts +++ b/src/main/providers/ssh-git-worktree-provider.ts @@ -1,9 +1,12 @@ import type { GitStatusResult } from '../../shared/git-status-types' import type { RemoveWorktreeResult } from '../../shared/worktree/create-types' import type { GitWorktreeInfo } from '../../shared/worktree/types' +import { CapabilityProbeCache } from '../../shared/capability-probe-cache' import { isJsonRpcMethodNotFoundError } from './ssh-git-relay-errors' import { SshGitReviewHeadProvider } from './ssh-git-review-head-provider' +const WORKTREE_IS_CLEAN_CAPABILITY = 'git.worktreeIsClean' as const + function formatStatusEntriesForCleanCheck(entries: GitStatusResult['entries']): string | undefined { if (entries.length === 0) { return undefined @@ -20,6 +23,10 @@ function filterUntrackedPorcelainStatus(stdout: string | undefined): string | un export class SshGitWorktreeProvider extends SshGitReviewHeadProvider { private loggedWorktreeIsCleanFallback = false + // Why: reconnect replaces this provider, so an upgraded relay is naturally re-probed. + private readonly worktreeIsCleanCapabilityCache = new CapabilityProbeCache< + typeof WORKTREE_IS_CLEAN_CAPABILITY + >(Number.POSITIVE_INFINITY) async listWorktrees( repoPath: string, @@ -67,37 +74,39 @@ export class SshGitWorktreeProvider extends SshGitReviewHeadProvider { worktreePath: string, options: { includeUntracked?: boolean } = {} ): Promise<{ clean: boolean; stdout?: string }> { - try { - const result = (await this.mux.request('git.worktreeIsClean', { - worktreePath, - ...(options.includeUntracked === false ? { includeUntracked: false } : {}) - })) as { clean: boolean; stdout?: string } - if (options.includeUntracked === false) { - if (!result.clean && result.stdout === undefined) { - return result + return this.worktreeIsCleanCapabilityCache.runWithFallback( + WORKTREE_IS_CLEAN_CAPABILITY, + async () => { + const result = (await this.mux.request('git.worktreeIsClean', { + worktreePath, + ...(options.includeUntracked === false ? { includeUntracked: false } : {}) + })) as { clean: boolean; stdout?: string } + if (options.includeUntracked === false) { + if (!result.clean && result.stdout === undefined) { + return result + } + const trackedStdout = filterUntrackedPorcelainStatus(result.stdout) + return { clean: !trackedStdout, ...(trackedStdout ? { stdout: trackedStdout } : {}) } } - const trackedStdout = filterUntrackedPorcelainStatus(result.stdout) - return { clean: !trackedStdout, ...(trackedStdout ? { stdout: trackedStdout } : {}) } - } - return result - } catch (error) { - if (!isJsonRpcMethodNotFoundError(error)) { - throw error - } - if (!this.loggedWorktreeIsCleanFallback) { - this.loggedWorktreeIsCleanFallback = true - console.warn( - '[ssh-git] Relay does not implement git.worktreeIsClean; falling back to git.status clean check' - ) - } - const status = await this.getStatus(worktreePath) - const entries = - options.includeUntracked === false - ? status.entries.filter((entry) => entry.area !== 'untracked') - : status.entries - const clean = entries.length === 0 - return { clean, stdout: formatStatusEntriesForCleanCheck(entries) } - } + return result + }, + async () => { + if (!this.loggedWorktreeIsCleanFallback) { + this.loggedWorktreeIsCleanFallback = true + console.warn( + '[ssh-git] Relay does not implement git.worktreeIsClean; falling back to git.status clean check' + ) + } + const status = await this.getStatus(worktreePath) + const entries = + options.includeUntracked === false + ? status.entries.filter((entry) => entry.area !== 'untracked') + : status.entries + const clean = entries.length === 0 + return { clean, stdout: formatStatusEntriesForCleanCheck(entries) } + }, + isJsonRpcMethodNotFoundError + ) } async refreshLocalBaseRefForWorktreeCreate(args: { From ad499df8937e11559909205afe8577a38b49b4ee Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:33 -0700 Subject: [PATCH 06/31] perf(markdown): reuse rich eligibility state (#17219) --- ...orContent.markdown-classification.test.tsx | 292 ++++++++++++++++++ .../EditorContent.monaco-lifecycle.test.tsx | 5 + .../components/editor/EditorContent.test.tsx | 4 + .../src/components/editor/EditorContent.tsx | 4 + .../editor/EditorEditFileSurface.tsx | 4 + .../editor/EditorMarkdownFileSurface.tsx | 17 +- .../components/editor/EditorPanelShell.tsx | 1 + .../editor/editor-panel-render-model.ts | 46 ++- .../components/editor/markdown-render-mode.ts | 5 + 9 files changed, 355 insertions(+), 23 deletions(-) create mode 100644 src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx diff --git a/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx b/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx new file mode 100644 index 00000000000..7d8767071be --- /dev/null +++ b/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx @@ -0,0 +1,292 @@ +// @vitest-environment happy-dom +import { cleanup, render } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import type { FileContent } from './editor-panel-content-types' + +const classifiers = vi.hoisted(() => ({ + getUnsupportedMessage: vi.fn<(content: string) => string | null>(), + exceedsSizeLimit: vi.fn<(content: string) => boolean>() +})) + +vi.mock('./markdown-rich-mode', () => ({ + getMarkdownRichModeUnsupportedMessage: classifiers.getUnsupportedMessage +})) + +vi.mock('./markdown-rich-size-limit', () => ({ + exceedsMarkdownRichModeSizeLimit: classifiers.exceedsSizeLimit +})) + +vi.mock('./editor-lazy-views', () => { + const view = (name: string) => () =>
+ return { + MonacoEditor: view('source'), + DiffViewer: view('diff'), + CombinedDiffViewer: view('combined-diff'), + RichMarkdownEditor: view('rich-editor'), + MarkdownPreview: view('preview'), + ImageViewer: view('image'), + ImageDiffViewer: view('image-diff'), + MermaidViewer: view('mermaid'), + CsvViewer: view('csv'), + IpynbViewer: view('notebook') + } +}) + +vi.mock('./RichMarkdownErrorBoundary', () => ({ + RichMarkdownErrorBoundary: ({ children }: { children: React.ReactNode }) => children +})) + +vi.mock('./useMarkdownDocuments', () => ({ + useMarkdownDocuments: () => ({ + markdownDocuments: [], + onOpenDocLink: () => {}, + previewProps: { markdownDocuments: [], onOpenDocument: async () => {} }, + mdSave: async () => true + }) +})) + +vi.mock('./useEditorConflictNavigation', () => ({ + useEditorConflictNavigation: () => () => undefined +})) + +vi.mock('@/store', () => { + const state = { + markdownRichModeSizeOverride: {}, + setMarkdownRichModeSizeOverride: () => {}, + reloadOpenCheckRunDetailsTab: () => {} + } + return { + useAppStore: Object.assign( + (selector: (storeState: typeof state) => unknown) => selector(state), + { getState: () => state } + ) + } +}) + +import { EditorContent } from './EditorContent' +import { getEditorPanelRenderModel } from './editor-panel-render-model' + +function openFile( + language: 'markdown' | 'typescript' = 'markdown', + mode: 'edit' | 'markdown-preview' = 'edit' +): OpenFile { + const extension = language === 'markdown' ? 'md' : 'ts' + return { + id: `/repo/notes.${extension}`, + filePath: `/repo/notes.${extension}`, + relativePath: `notes.${extension}`, + worktreeId: 'wt-1', + language, + mode, + isDirty: false + } +} + +function renderEditPath({ + content, + language = 'markdown', + viewMode = 'rich', + mode = 'edit' +}: { + content: string + language?: 'markdown' | 'typescript' + viewMode?: 'source' | 'rich' | 'preview' + mode?: 'edit' | 'markdown-preview' +}) { + const activeFile = openFile(language, mode) + const fileContents = { + [activeFile.id]: { content: '# Saved', isBinary: false } + } + const editorDrafts = { [activeFile.id]: content } + const model = getEditorPanelRenderModel({ + activeFile, + fileContents, + editorDrafts, + gitStatusEntries: undefined, + gitBranchEntries: undefined, + markdownViewMode: { [activeFile.id]: viewMode }, + markdownRichModeSizeOverride: {}, + isChangesMode: false, + canOpenWorkspaceFileBrowser: true + }) + const view = render( + + ) + + return { model, view } +} + +function getGuardedRenderModel({ + activeFileOverrides, + fileContent = { content: '# Saved', isBinary: false }, + includeFileContent = true, + isChangesMode = false +}: { + activeFileOverrides?: Partial + fileContent?: FileContent + includeFileContent?: boolean + isChangesMode?: boolean +}) { + const activeFile = { ...openFile(), ...activeFileOverrides } + return getEditorPanelRenderModel({ + activeFile, + fileContents: includeFileContent ? { [activeFile.id]: fileContent } : {}, + editorDrafts: { [activeFile.id]: '# Draft' }, + gitStatusEntries: undefined, + gitBranchEntries: undefined, + markdownViewMode: { [activeFile.id]: 'rich' }, + markdownRichModeSizeOverride: {}, + isChangesMode, + canOpenWorkspaceFileBrowser: true + }) +} + +beforeEach(() => { + classifiers.getUnsupportedMessage.mockImplementation((content) => + content.includes('[reference]:') ? 'Reference links require source mode.' : null + ) + classifiers.exceedsSizeLimit.mockImplementation((content) => content.includes('oversized')) +}) + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +describe('inline Markdown render classification', () => { + it('classifies each rich draft once, including after content changes', () => { + const first = renderEditPath({ content: '# First draft' }) + + expect(first.view.container.innerHTML).toContain('data-editor-view="rich-editor"') + expect(classifiers.getUnsupportedMessage).toHaveBeenCalledTimes(1) + expect(classifiers.getUnsupportedMessage).toHaveBeenLastCalledWith('# First draft') + expect(classifiers.exceedsSizeLimit).toHaveBeenCalledTimes(1) + expect(classifiers.exceedsSizeLimit).toHaveBeenLastCalledWith('# First draft') + + first.view.unmount() + const second = renderEditPath({ content: '# Changed draft' }) + + expect(second.view.container.innerHTML).toContain('data-editor-view="rich-editor"') + expect(classifiers.getUnsupportedMessage).toHaveBeenCalledTimes(2) + expect(classifiers.getUnsupportedMessage).toHaveBeenLastCalledWith('# Changed draft') + expect(classifiers.exceedsSizeLimit).toHaveBeenCalledTimes(2) + expect(classifiers.exceedsSizeLimit).toHaveBeenLastCalledWith('# Changed draft') + }) + + it.each([ + { + name: 'source Markdown', + args: { content: '# Source', viewMode: 'source' as const }, + expectedView: 'source', + canExport: false + }, + { + name: 'Markdown preview', + args: { content: '# Preview', mode: 'markdown-preview' as const }, + expectedView: 'preview', + canExport: true + }, + { + name: 'TypeScript', + args: { content: 'const value = 1', language: 'typescript' as const }, + expectedView: 'source', + canExport: false + } + ])('skips rich eligibility scans for $name', ({ args, expectedView, canExport }) => { + const result = renderEditPath(args) + + expect(result.view.container.innerHTML).toContain(`data-editor-view="${expectedView}"`) + expect(result.model.canExportMarkdownToPdf).toBe(canExport) + expect(classifiers.getUnsupportedMessage).not.toHaveBeenCalled() + expect(classifiers.exceedsSizeLimit).not.toHaveBeenCalled() + }) + + it.each([ + { name: 'Changes mode', args: { isChangesMode: true } }, + { name: 'content that is still loading', args: { includeFileContent: false } }, + { + name: 'binary content', + args: { fileContent: { content: '', isBinary: true } } + }, + { + name: 'content with a load error', + args: { fileContent: { content: '', isBinary: false, loadError: 'Read failed' } } + }, + { + name: 'read-only Markdown', + args: { activeFileOverrides: { readOnly: true } } + }, + { + name: 'an unresolved conflict', + args: { + activeFileOverrides: { + conflict: { + kind: 'conflict-editable' as const, + conflictKind: 'both_modified' as const, + conflictStatus: 'unresolved' as const, + conflictStatusSource: 'git' as const + } + } + } + }, + { + name: 'a conflict placeholder', + args: { + activeFileOverrides: { + conflict: { + kind: 'conflict-placeholder' as const, + conflictKind: 'both_deleted' as const, + conflictStatus: 'unresolved' as const, + conflictStatusSource: 'git' as const + } + } + } + } + ])('skips rich eligibility scans for $name', ({ args }) => { + const model = getGuardedRenderModel(args) + + expect(model.inlineMarkdownRenderState).toBeNull() + expect(classifiers.getUnsupportedMessage).not.toHaveBeenCalled() + expect(classifiers.exceedsSizeLimit).not.toHaveBeenCalled() + }) + + it('preserves unsupported and oversized rich-mode fallbacks', () => { + const unsupported = renderEditPath({ content: '[reference]: https://example.com' }) + + expect(unsupported.model.canExportMarkdownToPdf).toBe(false) + expect(unsupported.view.getByText('Reference links require source mode.')).toBeTruthy() + expect(unsupported.view.queryByText('Open anyway')).toBeNull() + unsupported.view.unmount() + + const oversized = renderEditPath({ content: '# oversized' }) + + expect(oversized.model.canExportMarkdownToPdf).toBe(false) + expect(oversized.view.getByText(/File is larger than the .* rich editing limit/)).toBeTruthy() + expect(oversized.view.getByText('Open anyway')).toBeTruthy() + }) +}) diff --git a/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx b/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx index 53eec01d0d8..00ded9527d2 100644 --- a/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx +++ b/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx @@ -184,6 +184,7 @@ function props(activeFile: OpenFile, content: string) { isCsv: false, isNotebook: false, mdViewMode: 'rich' as const, + inlineMarkdownRenderState: null, isChangesMode: false, sideBySide: false, pendingEditorReveal: null, @@ -326,6 +327,10 @@ describe('EditorContent Monaco lifecycle boundary', () => { resolvedLanguage="markdown" isMarkdown mdViewMode="rich" + inlineMarkdownRenderState={{ + renderMode: 'rich-editor', + richModeUnsupportedMessage: null + }} /> ) diff --git a/src/renderer/src/components/editor/EditorContent.test.tsx b/src/renderer/src/components/editor/EditorContent.test.tsx index 130c92abe35..8086851a81c 100644 --- a/src/renderer/src/components/editor/EditorContent.test.tsx +++ b/src/renderer/src/components/editor/EditorContent.test.tsx @@ -67,6 +67,7 @@ describe('EditorContent', () => { isCsv={false} isNotebook mdViewMode="rich" + inlineMarkdownRenderState={null} isChangesMode={false} sideBySide={false} pendingEditorReveal={null} @@ -108,6 +109,7 @@ describe('EditorContent', () => { isCsv={false} isNotebook={false} mdViewMode="rich" + inlineMarkdownRenderState={null} isChangesMode={false} sideBySide={false} pendingEditorReveal={null} @@ -153,6 +155,7 @@ describe('EditorContent', () => { isCsv={false} isNotebook={false} mdViewMode="rich" + inlineMarkdownRenderState={null} isChangesMode={false} sideBySide={false} pendingEditorReveal={null} @@ -200,6 +203,7 @@ describe('EditorContent', () => { isCsv={false} isNotebook={false} mdViewMode="preview" + inlineMarkdownRenderState={null} isChangesMode={false} sideBySide={false} pendingEditorReveal={null} diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index ca12823e846..9fab1cd5665 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -12,6 +12,7 @@ import type { FileContent } from './editor-panel-content-types' import { translate } from '@/i18n/i18n' import { useEditorConflictNavigation } from './useEditorConflictNavigation' import { useMarkdownDocuments } from './useMarkdownDocuments' +import type { MarkdownRenderState } from './markdown-render-mode' const noopCloseMarkdownTableOfContents = (): void => {} @@ -47,6 +48,7 @@ export function EditorContent({ isCsv, isNotebook, mdViewMode, + inlineMarkdownRenderState, isChangesMode, sideBySide, showMarkdownTableOfContents = false, @@ -74,6 +76,7 @@ export function EditorContent({ isCsv: boolean isNotebook: boolean mdViewMode: MarkdownViewMode + inlineMarkdownRenderState: MarkdownRenderState | null isChangesMode: boolean sideBySide: boolean showMarkdownTableOfContents?: boolean @@ -238,6 +241,7 @@ export function EditorContent({ isCsv={isCsv} isNotebook={isNotebook} mdViewMode={mdViewMode} + inlineMarkdownRenderState={inlineMarkdownRenderState} isChangesMode={isChangesMode} sideBySide={sideBySide} showMarkdownTableOfContents={showMarkdownTableOfContents} diff --git a/src/renderer/src/components/editor/EditorEditFileSurface.tsx b/src/renderer/src/components/editor/EditorEditFileSurface.tsx index d0b83539ee6..06a7bb3f909 100644 --- a/src/renderer/src/components/editor/EditorEditFileSurface.tsx +++ b/src/renderer/src/components/editor/EditorEditFileSurface.tsx @@ -17,6 +17,7 @@ import type { FileContent } from './editor-panel-content-types' import { ExternalFileChangeBanner } from './ExternalFileChangeBanner' import type { useMarkdownDocuments } from './useMarkdownDocuments' import { EditorMarkdownFileSurface } from './EditorMarkdownFileSurface' +import type { MarkdownRenderState } from './markdown-render-mode' const noopEditorContentChange = (_content: string): void => {} const noopEditorSave = async (_content: string): Promise => false @@ -39,6 +40,7 @@ export function EditorEditFileSurface({ isCsv, isNotebook, mdViewMode, + inlineMarkdownRenderState, isChangesMode, sideBySide, showMarkdownTableOfContents, @@ -69,6 +71,7 @@ export function EditorEditFileSurface({ isCsv: boolean isNotebook: boolean mdViewMode: MarkdownViewMode + inlineMarkdownRenderState: MarkdownRenderState | null isChangesMode: boolean sideBySide: boolean showMarkdownTableOfContents: boolean @@ -210,6 +213,7 @@ export function EditorEditFileSurface({ editorViewStateKey={editorViewStateKey} currentContent={currentContent} mdViewMode={mdViewMode} + inlineMarkdownRenderState={inlineMarkdownRenderState} showMarkdownTableOfContents={showMarkdownTableOfContents} showMarkdownFrontmatter={showMarkdownFrontmatter} onCloseMarkdownTableOfContents={onCloseMarkdownTableOfContents} diff --git a/src/renderer/src/components/editor/EditorMarkdownFileSurface.tsx b/src/renderer/src/components/editor/EditorMarkdownFileSurface.tsx index 6d5cfea1bdf..fda81b98462 100644 --- a/src/renderer/src/components/editor/EditorMarkdownFileSurface.tsx +++ b/src/renderer/src/components/editor/EditorMarkdownFileSurface.tsx @@ -5,10 +5,8 @@ import { Button } from '@/components/ui/button' import { RICH_MARKDOWN_MAX_SIZE_BYTES } from '../../../../shared/constants' import { formatBytes } from '../status-bar/workspace-space-format' import { MarkdownPreview, RichMarkdownEditor } from './editor-lazy-views' -import { exceedsMarkdownRichModeSizeLimit } from './markdown-rich-size-limit' import { extractFrontMatter, prependFrontMatter } from './markdown-frontmatter' -import { getMarkdownRenderMode } from './markdown-render-mode' -import { getMarkdownRichModeUnsupportedMessage } from './markdown-rich-mode' +import type { MarkdownRenderState } from './markdown-render-mode' import { RichMarkdownErrorBoundary } from './RichMarkdownErrorBoundary' import type { useMarkdownDocuments } from './useMarkdownDocuments' @@ -20,6 +18,7 @@ export function EditorMarkdownFileSurface({ editorViewStateKey, currentContent, mdViewMode, + inlineMarkdownRenderState, showMarkdownTableOfContents, showMarkdownFrontmatter, onCloseMarkdownTableOfContents, @@ -35,6 +34,7 @@ export function EditorMarkdownFileSurface({ editorViewStateKey: string currentContent: string mdViewMode: MarkdownViewMode + inlineMarkdownRenderState: MarkdownRenderState | null showMarkdownTableOfContents: boolean showMarkdownFrontmatter: boolean onCloseMarkdownTableOfContents: () => void @@ -45,18 +45,15 @@ export function EditorMarkdownFileSurface({ handleDirtyStateHint: (dirty: boolean) => void monacoEditor: React.JSX.Element }): React.JSX.Element { - const sizeOverridden = useAppStore((s) => s.markdownRichModeSizeOverride[activeFile.id] === true) const setSizeOverride = useAppStore((s) => s.setMarkdownRichModeSizeOverride) - const richModeUnsupportedMessage = getMarkdownRichModeUnsupportedMessage(currentContent) - const renderMode = getMarkdownRenderMode({ - exceedsRichModeSizeLimit: !sizeOverridden && exceedsMarkdownRichModeSizeLimit(currentContent), - hasRichModeUnsupportedContent: richModeUnsupportedMessage !== null, - viewMode: mdViewMode - }) if (activeFile.conflict?.conflictStatus === 'unresolved') { return
{monacoEditor}
} + if (!inlineMarkdownRenderState) { + return
{monacoEditor}
+ } + const { renderMode, richModeUnsupportedMessage } = inlineMarkdownRenderState if (renderMode === 'source' && mdViewMode === 'rich') { // Why: only a size fallback is recoverable — unsupported syntax would round-trip badly, so it gets no override. const isSizeFallback = richModeUnsupportedMessage === null diff --git a/src/renderer/src/components/editor/EditorPanelShell.tsx b/src/renderer/src/components/editor/EditorPanelShell.tsx index 62b399c3b3c..dd15231cefe 100644 --- a/src/renderer/src/components/editor/EditorPanelShell.tsx +++ b/src/renderer/src/components/editor/EditorPanelShell.tsx @@ -149,6 +149,7 @@ export function EditorPanelShell({ isCsv={model.isCsv} isNotebook={model.isNotebook} mdViewMode={model.mdViewMode} + inlineMarkdownRenderState={model.inlineMarkdownRenderState} isChangesMode={model.isDiffSurface && !model.isSingleDiff} sideBySide={sideBySide} pendingEditorReveal={pendingEditorReveal} diff --git a/src/renderer/src/components/editor/editor-panel-render-model.ts b/src/renderer/src/components/editor/editor-panel-render-model.ts index 2dc9fe87382..8a479794f83 100644 --- a/src/renderer/src/components/editor/editor-panel-render-model.ts +++ b/src/renderer/src/components/editor/editor-panel-render-model.ts @@ -12,7 +12,7 @@ import { getEditorHeaderOpenFileState } from './editor-header' import type { EditorToggleValue } from './EditorViewToggle' import type { FileContent } from './editor-panel-content-types' import { canUseChangesModeForFile } from './editor-panel-file-mode' -import { getMarkdownRenderMode } from './markdown-render-mode' +import { getMarkdownRenderMode, type MarkdownRenderState } from './markdown-render-mode' import { getMarkdownRichModeUnsupportedMessage } from './markdown-rich-mode' import { exceedsMarkdownRichModeSizeLimit } from './markdown-rich-size-limit' @@ -129,17 +129,36 @@ export function getEditorPanelRenderModel({ const shouldShowMarkdownExportAction = viewerLanguage === 'markdown' && (activeFile.mode === 'edit' || activeFile.mode === 'markdown-preview') - const inlineMarkdownRenderMode = - activeFile.mode === 'edit' && inlineMarkdownContent !== null - ? getMarkdownRenderMode({ - exceedsRichModeSizeLimit: - markdownRichModeSizeOverride[activeFile.id] !== true && - exceedsMarkdownRichModeSizeLimit(inlineMarkdownContent), - hasRichModeUnsupportedContent: - getMarkdownRichModeUnsupportedMessage(inlineMarkdownContent) !== null, - viewMode: mdViewMode - }) + const inlineFileContent = activeFile.mode === 'edit' ? fileContents[activeFile.id] : undefined + const canRenderInlineMarkdown = + viewerLanguage === 'markdown' && + activeFile.mode === 'edit' && + inlineMarkdownContent !== null && + !isChangesMode && + inlineFileContent !== undefined && + inlineFileContent.isBinary !== true && + !inlineFileContent.loadError && + activeFile.conflict?.kind !== 'conflict-placeholder' && + activeFile.conflict?.conflictStatus !== 'unresolved' + let inlineMarkdownRenderState: MarkdownRenderState | null = null + if (canRenderInlineMarkdown) { + const shouldClassifyRichMode = mdViewMode === 'rich' + const exceedsRichModeSizeLimit = + shouldClassifyRichMode && + markdownRichModeSizeOverride[activeFile.id] !== true && + exceedsMarkdownRichModeSizeLimit(inlineMarkdownContent) + const richModeUnsupportedMessage = shouldClassifyRichMode + ? getMarkdownRichModeUnsupportedMessage(inlineMarkdownContent) : null + inlineMarkdownRenderState = { + renderMode: getMarkdownRenderMode({ + exceedsRichModeSizeLimit, + hasRichModeUnsupportedContent: richModeUnsupportedMessage !== null, + viewMode: mdViewMode + }), + richModeUnsupportedMessage + } + } const canExportMarkdownToPdf = shouldShowMarkdownExportAction && ((activeFile.mode === 'markdown-preview' && @@ -149,8 +168,8 @@ export function getEditorPanelRenderModel({ (activeFile.mode === 'edit' && fileContents[activeFile.id] !== undefined && !isChangesMode && - inlineMarkdownRenderMode !== null && - inlineMarkdownRenderMode !== 'source' && + inlineMarkdownRenderState !== null && + inlineMarkdownRenderState.renderMode !== 'source' && fileContents[activeFile.id]?.isBinary !== true && !fileContents[activeFile.id]?.loadError && activeFile.conflict?.conflictStatus !== 'unresolved')) @@ -180,6 +199,7 @@ export function getEditorPanelRenderModel({ isMarkdownTableOfContentsDisabled: hasViewModeToggle && mdViewMode === 'source', shouldShowMarkdownExportAction, canExportMarkdownToPdf, + inlineMarkdownRenderState, canShowMarkdownTableOfContents: viewerLanguage === 'markdown' && (hasViewModeToggle || activeFile.mode === 'markdown-preview'), diff --git a/src/renderer/src/components/editor/markdown-render-mode.ts b/src/renderer/src/components/editor/markdown-render-mode.ts index b8cbc499986..f188dcbb814 100644 --- a/src/renderer/src/components/editor/markdown-render-mode.ts +++ b/src/renderer/src/components/editor/markdown-render-mode.ts @@ -2,6 +2,11 @@ import type { MarkdownViewMode } from '@/store/slices/editor' export type MarkdownRenderMode = 'source' | 'rich-editor' | 'preview' +export type MarkdownRenderState = { + renderMode: MarkdownRenderMode + richModeUnsupportedMessage: string | null +} + export function getMarkdownRenderMode({ exceedsRichModeSizeLimit, hasRichModeUnsupportedContent, From 5e3c4f391cc2588728e99d79fea30c92e51c795d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:36 -0700 Subject: [PATCH 07/31] perf(terminal): bound stalled setup-split polling (#17228) --- .../terminal-startup-grid-settle.test.ts | 24 +++++++++++++++++++ .../terminal-startup-grid-settle.ts | 15 ++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.test.ts b/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.test.ts index ebe5064be3c..dd7e56ab328 100644 --- a/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.test.ts @@ -115,6 +115,30 @@ describe('waitForStableStartupGrid', () => { expect(onSettled).toHaveBeenCalledWith({ cols: 88, rows: 50 }) }) + it('stops polling at the default cap when the external split gate never opens', () => { + const scheduler = createFrameScheduler() + const onSettled = vi.fn() + const measure = vi.fn(() => ({ cols: 88, rows: 50 })) + + waitForStableStartupGrid({ + isAlive: () => true, + isReadyToSettle: () => false, + measure, + onSettled, + requestFrame: scheduler.requestFrame, + cancelFrame: scheduler.cancelFrame + }) + + expect(scheduler.run(119)).toBe(119) + expect(scheduler.pending()).toBe(1) + expect(onSettled).not.toHaveBeenCalled() + + expect(scheduler.run(10)).toBe(1) + expect(scheduler.pending()).toBe(0) + expect(measure).toHaveBeenCalledTimes(120) + expect(onSettled).toHaveBeenCalledWith({ cols: 88, rows: 50 }) + }) + it('uses the latest usable grid at the frame cap when dimensions keep changing', () => { const scheduler = createFrameScheduler() const onSettled = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.ts b/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.ts index 5ed9d704670..8df3d42d6c6 100644 --- a/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.ts +++ b/src/renderer/src/components/terminal-pane/terminal-startup-grid-settle.ts @@ -13,6 +13,7 @@ export type TerminalStartupGridSettleOptions = { minFrames?: number stableFrames?: number maxFrames?: number + maxReadinessWaitFrames?: number } export type TerminalStartupGridSettleHandle = { @@ -22,6 +23,8 @@ export type TerminalStartupGridSettleHandle = { const DEFAULT_MIN_FRAMES = 6 const DEFAULT_STABLE_FRAMES = 2 const DEFAULT_MAX_FRAMES = 12 +// Why: ten settle windows tolerate slow split layout without permitting permanent polling. +const DEFAULT_MAX_READINESS_WAIT_FRAMES = DEFAULT_MAX_FRAMES * 10 function usableDimensions( dimensions: TerminalStartupGridDimensions | null @@ -42,6 +45,10 @@ export function waitForStableStartupGrid( const minFrames = Math.max(1, options.minFrames ?? DEFAULT_MIN_FRAMES) const stableFrames = Math.max(1, options.stableFrames ?? DEFAULT_STABLE_FRAMES) const maxFrames = Math.max(minFrames, options.maxFrames ?? DEFAULT_MAX_FRAMES) + const maxReadinessWaitFrames = Math.max( + 1, + options.maxReadinessWaitFrames ?? DEFAULT_MAX_READINESS_WAIT_FRAMES + ) let frame = 0 let stableFrameCount = 0 let previous: TerminalStartupGridDimensions | null = null @@ -50,6 +57,7 @@ export function waitForStableStartupGrid( let pendingFrame: number | null = null let cancelled = false let readyFrame = 0 + let latestReadinessWaitUsable: TerminalStartupGridDimensions | null = null const usesReadinessGate = options.isReadyToSettle !== undefined const settle = (dimensions: TerminalStartupGridDimensions | null): void => { @@ -73,6 +81,13 @@ export function waitForStableStartupGrid( const measured = options.measure() const readyToSettle = options.isReadyToSettle?.() ?? true if (!readyToSettle) { + if (usableDimensions(measured)) { + latestReadinessWaitUsable = measured + } + if (frame >= maxReadinessWaitFrames) { + settle(latestReadinessWaitUsable) + return + } previous = null stableFrameCount = 0 observedGridChange = false From be68aa271887865ef5567bd5ea97548c38a5ef16 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:39 -0700 Subject: [PATCH 08/31] perf(browser): release popup origin bar contents (#17234) --- .../browser/popup-origin-bar-window.test.ts | 45 +++++++++++++++++++ src/main/browser/popup-origin-bar-window.ts | 19 +++++--- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/main/browser/popup-origin-bar-window.test.ts b/src/main/browser/popup-origin-bar-window.test.ts index 3e816e74b07..fedcd4dfdd0 100644 --- a/src/main/browser/popup-origin-bar-window.test.ts +++ b/src/main/browser/popup-origin-bar-window.test.ts @@ -285,6 +285,7 @@ describe('openPopupWithOriginBar', () => { it('closes the window when the popup content is destroyed, without re-closing the contents', () => { const adopted = createFakeWebContents() openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/') + const { bar } = lastViews() adopted.emit('destroyed') @@ -292,6 +293,7 @@ describe('openPopupWithOriginBar', () => { // The window's closed handler must not call close() on already-destroyed // contents — that throws in real Electron. expect(adopted.close).not.toHaveBeenCalled() + expect(bar.webContents.close).toHaveBeenCalledTimes(1) }) it('re-asserts the origin when the popup finishes loading', () => { @@ -326,11 +328,54 @@ describe('openPopupWithOriginBar', () => { const adopted = createFakeWebContents() const onClosed = vi.fn() const popup = openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/') + const { bar } = lastViews() popup.onClosed(onClosed) popup.close() expect(adopted.close).toHaveBeenCalledTimes(1) + expect(bar.webContents.close).toHaveBeenCalledTimes(1) expect(onClosed).toHaveBeenCalledTimes(1) }) + + it('releases every origin bar across repeated popup lifecycles', () => { + const cycleCount = 25 + const originBars: FakeWebContents[] = [] + + for (let cycle = 0; cycle < cycleCount; cycle += 1) { + const popup = openPopupWithOriginBar({}, `https://example.com/${cycle}`) + originBars.push(lastViews().bar.webContents) + popup.close() + } + + expect(originBars.filter((contents) => contents.close.mock.calls.length === 1)).toHaveLength( + cycleCount + ) + }) + + it('tolerates an origin bar destroyed before its window closes', () => { + const adopted = createFakeWebContents() + const onClosed = vi.fn() + const popup = openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/') + popup.onClosed(onClosed) + const { bar } = lastViews() + const barWebContents = bar.webContents + barWebContents.emit('destroyed') + Object.defineProperty(bar, 'webContents', { get: () => undefined }) + + expect(() => popup.close()).not.toThrow() + expect(adopted.close).toHaveBeenCalledTimes(1) + expect(barWebContents.close).not.toHaveBeenCalled() + expect(onClosed).toHaveBeenCalledTimes(1) + }) + + it('closes both child contents when popup preparation fails', () => { + const adopted = createFakeWebContents() + + openPopupWithOriginBar({ webContents: adopted as never }, 'https://example.com/', () => false) + + expect(adopted.close).toHaveBeenCalledTimes(1) + expect(lastViews().bar.webContents.close).toHaveBeenCalledTimes(1) + expect(lastWindow().close).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/main/browser/popup-origin-bar-window.ts b/src/main/browser/popup-origin-bar-window.ts index 376c318d1ae..cfbe3ef3d3b 100644 --- a/src/main/browser/popup-origin-bar-window.ts +++ b/src/main/browser/popup-origin-bar-window.ts @@ -92,7 +92,8 @@ function clampPopupContentSize(options: PopupChildWindowOptions): { // caller's popup bookkeeping cannot leak an entry for a window that no longer exists. function closeUnpreparedPopup( window: BaseWindow, - contentWebContents: Electron.WebContents + contentWebContents: Electron.WebContents, + originBarWebContents: Electron.WebContents ): PopupOriginBarWindow { const closedListeners: (() => void)[] = [] let closed = false @@ -105,6 +106,9 @@ function closeUnpreparedPopup( if (!contentWebContents.isDestroyed()) { contentWebContents.close() } + if (!originBarWebContents.isDestroyed()) { + originBarWebContents.close() + } if (!window.isDestroyed()) { window.close() } @@ -159,6 +163,7 @@ export function openPopupWithOriginBar( const originBarView = new WebContentsView({ webPreferences: { contextIsolation: true, nodeIntegration: false, sandbox: true } }) + const originBarWebContents = originBarView.webContents const contentView = new WebContentsView({ // Why: Electron rejects an explicitly undefined webContents; omitting it // lets WebContentsView create contents for Cmd/Ctrl-click popups. @@ -188,7 +193,7 @@ export function openPopupWithOriginBar( const contentWebContents = contentView.webContents if (prepareContent && !prepareContent(contentWebContents)) { - return closeUnpreparedPopup(window, contentWebContents) + return closeUnpreparedPopup(window, contentWebContents, originBarWebContents) } let currentUrl = initialUrl const renderOrigin = (): void => { @@ -202,15 +207,15 @@ export function openPopupWithOriginBar( } // Why: textContent + JSON encoding — the URL is attacker-controlled and // must never be interpolated into the bar's markup. - void originBarView.webContents + void originBarWebContents .executeJavaScript( `document.body.classList.toggle('insecure', ${insecure ? 'true' : 'false'});` + `document.getElementById('origin').textContent = ${JSON.stringify(label)};` ) .catch(() => {}) } - originBarView.webContents.once('did-finish-load', renderOrigin) - void originBarView.webContents.loadURL( + originBarWebContents.once('did-finish-load', renderOrigin) + void originBarWebContents.loadURL( `data:text/html;charset=utf-8,${encodeURIComponent(ORIGIN_BAR_HTML)}` ) @@ -253,6 +258,10 @@ export function openPopupWithOriginBar( // pages often notify the opener from unload. contentWebContents.close() } + // Why: BaseWindow does not destroy child WebContentsView contents when it closes. + if (!originBarWebContents.isDestroyed()) { + originBarWebContents.close() + } for (const listener of closedListeners) { listener() } From 79aea811734962d35a0177f5cf734b817543dea2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:01:42 -0700 Subject: [PATCH 09/31] perf(renderer): index unread completion tabs (#17246) --- .../terminal-tab-activity-status.test.ts | 55 +++++++++++++++++++ .../tab-bar/terminal-tab-activity-status.ts | 50 ++++++++++++----- 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts index d2bed923686..bf367f93667 100644 --- a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts +++ b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts @@ -3,6 +3,7 @@ import type { AgentStatusEntry } from '../../../../shared/agent-status-types' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { hasUnreadAgentCompletionForTerminalTab, + resetUnreadAgentCompletionTabIdsCacheForTest, resetTerminalTabActivityFlagsCacheForTest, resolveTerminalTabActivityStatus, resolveTerminalTabAttentionBadge, @@ -41,6 +42,7 @@ const LIVE_PTY = { [TAB_ID]: ['pty-1'] } beforeEach(() => { resetTerminalTabActivityFlagsCacheForTest() + resetUnreadAgentCompletionTabIdsCacheForTest() vi.useFakeTimers() vi.setSystemTime(NOW) }) @@ -48,6 +50,7 @@ beforeEach(() => { afterEach(() => { vi.useRealTimers() resetTerminalTabActivityFlagsCacheForTest() + resetUnreadAgentCompletionTabIdsCacheForTest() }) describe('resolveTerminalTabActivityStatus', () => { @@ -267,6 +270,7 @@ describe('hasUnreadAgentCompletionForTerminalTab', () => { expect( hasUnreadAgentCompletionForTerminalTab({ [`tab-2:${SECOND_LEAF_ID}`]: true }, TAB_ID) ).toBe(false) + expect(hasUnreadAgentCompletionForTerminalTab(undefined, TAB_ID)).toBe(false) }) // Why: the param accepts boolean maps, so a cleared-to-`false` marker must not read as unread. @@ -275,6 +279,57 @@ describe('hasUnreadAgentCompletionForTerminalTab', () => { hasUnreadAgentCompletionForTerminalTab({ [`${TAB_ID}:${FIRST_LEAF_ID}`]: false }, TAB_ID) ).toBe(false) }) + + it('preserves first-colon ownership for legacy and malformed pane keys', () => { + expect(hasUnreadAgentCompletionForTerminalTab({ [TAB_ID]: true }, TAB_ID)).toBe(true) + expect(hasUnreadAgentCompletionForTerminalTab({ [`${TAB_ID}:3`]: true }, TAB_ID)).toBe(true) + expect( + hasUnreadAgentCompletionForTerminalTab( + { [`${TAB_ID}:${FIRST_LEAF_ID}:suffix`]: true }, + TAB_ID + ) + ).toBe(true) + expect( + hasUnreadAgentCompletionForTerminalTab({ [`${TAB_ID}0:${SECOND_LEAF_ID}`]: true }, TAB_ID) + ).toBe(false) + }) + + it('changes only the owning tab when immutable marker snapshots add and clear unread', () => { + const before = { [`tab-2:${SECOND_LEAF_ID}`]: true } + const added = { ...before, [`${TAB_ID}:${FIRST_LEAF_ID}`]: true } + const cleared = { ...added, [`${TAB_ID}:${FIRST_LEAF_ID}`]: false } + + expect(hasUnreadAgentCompletionForTerminalTab(before, TAB_ID)).toBe(false) + expect(hasUnreadAgentCompletionForTerminalTab(added, TAB_ID)).toBe(true) + expect(hasUnreadAgentCompletionForTerminalTab(cleared, TAB_ID)).toBe(false) + expect(hasUnreadAgentCompletionForTerminalTab(before, 'tab-2')).toBe(true) + expect(hasUnreadAgentCompletionForTerminalTab(added, 'tab-2')).toBe(true) + expect(hasUnreadAgentCompletionForTerminalTab(cleared, 'tab-2')).toBe(true) + }) + + it('indexes one immutable marker snapshot once across all mounted tab lookups', () => { + const ownKeys = vi.fn(Reflect.ownKeys) + let valueReads = 0 + const markers: Record = Object.fromEntries( + Array.from({ length: 1_000 }, (_, index) => [`owner-${index}:leaf`, true] as const) + ) + const unread = new Proxy>(markers, { + ownKeys, + get: (target, property, receiver) => { + valueReads += 1 + return Reflect.get(target, property, receiver) + } + }) + + for (let publication = 0; publication < 5; publication += 1) { + for (let tab = 0; tab < 500; tab += 1) { + expect(hasUnreadAgentCompletionForTerminalTab(unread, `absent-${tab}`)).toBe(false) + } + } + + expect(ownKeys).toHaveBeenCalledTimes(1) + expect(valueReads).toBe(1_000) + }) }) describe('resolveTerminalTabAttentionBadge', () => { diff --git a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts index 97f511a1d41..a801855a588 100644 --- a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts +++ b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts @@ -256,29 +256,49 @@ export function terminalTabHasUnreadActivity({ ) } +// Why: production writes replace this map; WeakMap supports retained snapshots without pinning them. +let unreadAgentCompletionTabIdsBySnapshot = new WeakMap< + Record, + ReadonlySet +>() + +function getUnreadAgentCompletionTabIds( + unreadAgentCompletionPanes: Record +): ReadonlySet { + const cached = unreadAgentCompletionTabIdsBySnapshot.get(unreadAgentCompletionPanes) + if (cached) { + return cached + } + + // Why: every mounted tab runs this selector per store write; index each immutable marker snapshot once. + const tabIds = new Set() + for (const paneKey of Object.keys(unreadAgentCompletionPanes)) { + if (!unreadAgentCompletionPanes[paneKey]) { + continue + } + const separatorIndex = paneKey.indexOf(':') + tabIds.add(separatorIndex === -1 ? paneKey : paneKey.slice(0, separatorIndex)) + } + unreadAgentCompletionTabIdsBySnapshot.set(unreadAgentCompletionPanes, tabIds) + return tabIds +} + /** Match pane-level unread completion markers to their owning terminal tab. */ export function hasUnreadAgentCompletionForTerminalTab( unreadAgentCompletionPanes: Record | undefined, tabId: string ): boolean { - for (const [paneKey, unread] of Object.entries(unreadAgentCompletionPanes ?? {})) { - // Why entries, not keys: the widened value type lets a cleared marker linger as `false`. - if (!unread) { - continue - } - // paneKey is `${tabId}:${leafId}` and tab ids never contain ":", so the - // prefix up to the first ":" is the owning tab id (see - // selectFloatingWorkspaceHasUnread). Prefix-match to keep legacy keys. - const separatorIndex = paneKey.indexOf(':') - const owningTabId = separatorIndex === -1 ? paneKey : paneKey.slice(0, separatorIndex) - if (owningTabId === tabId) { - return true - } - } - return false + return unreadAgentCompletionPanes + ? getUnreadAgentCompletionTabIds(unreadAgentCompletionPanes).has(tabId) + : false } /** Test-only: clear the memoized per-tab flag cache between cases. */ export function resetTerminalTabActivityFlagsCacheForTest(): void { flagsCache = null } + +/** Test-only: clear the unread marker snapshot index between cases. */ +export function resetUnreadAgentCompletionTabIdsCacheForTest(): void { + unreadAgentCompletionTabIdsBySnapshot = new WeakMap() +} From e25878588445931b42db2fb05323e62733e7a646 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:02:10 -0700 Subject: [PATCH 10/31] perf(terminal): index accepted input pane ownership (#17287) --- .../runtime-terminal-inspection.test.ts | 204 ++++++++++++++++++ .../runtime/runtime-terminal-inspection.ts | 78 +++++-- 2 files changed, 266 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/runtime/runtime-terminal-inspection.test.ts b/src/renderer/src/runtime/runtime-terminal-inspection.test.ts index 4223f6d2b82..766e80f3f6e 100644 --- a/src/renderer/src/runtime/runtime-terminal-inspection.test.ts +++ b/src/renderer/src/runtime/runtime-terminal-inspection.test.ts @@ -209,6 +209,53 @@ describe('runtime terminal owner routing', () => { }) }) + it('attributes a delayed runtime acknowledgement to the current layout owner', async () => { + const pendingSend = Promise.withResolvers<{ + ok: true + result: { send: { handle: string; accepted: true; bytesWritten: number } } + _meta: { runtimeId: string } + }>() + runtimeCall.mockReturnValue(pendingSend.promise) + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'remote:env-1@@terminal-1' } + } + } + }) + recordRuntimeTerminalInputForPtyId('remote:env-1@@terminal-1', 123) + useAppStore.setState({ lastTerminalInputAtByPaneKey: {} }) + + expect( + sendRuntimePtyInput({ activeRuntimeEnvironmentId: 'env-2' }, 'remote:env-1@@terminal-1', 'x') + ).toBe(true) + const nextLeafId = '22222222-2222-4222-8222-222222222222' + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-2': { + root: { type: 'leaf', leafId: nextLeafId }, + activeLeafId: nextLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { [nextLeafId]: 'remote:env-1@@terminal-1' } + } + } + }) + pendingSend.resolve({ + ok: true, + result: { send: { handle: 'terminal-1', accepted: true, bytesWritten: 1 } }, + _meta: { runtimeId: 'runtime-1' } + }) + + await vi.waitFor(() => { + expect(useAppStore.getState().lastTerminalInputAtByPaneKey).toEqual({ + [`tab-2:${nextLeafId}`]: expect.any(Number) + }) + }) + }) + it('does not record declined fire-and-forget runtime input', async () => { runtimeCall.mockResolvedValue({ ok: true, @@ -479,6 +526,163 @@ describe('runtime terminal owner routing', () => { expect(useAppStore.getState().lastTerminalInputAtByPaneKey[PANE_KEY]).toBe(123) }) + it('indexes a stable layout identity once across repeated terminal input', () => { + const layoutCount = 500 + let layoutEnumerations = 0 + let leafEnumerations = 0 + const layouts = Object.fromEntries( + Array.from({ length: layoutCount }, (_, index) => { + const leafId = `00000000-0000-4000-8000-${index.toString(16).padStart(12, '0')}` + const ptyIdsByLeafId = new Proxy( + { [leafId]: `pty-${index}` }, + { + ownKeys: (target) => { + leafEnumerations += 1 + return Reflect.ownKeys(target) + } + } + ) + return [ + `tab-${index}`, + { + root: { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId + } + ] + }) + ) + const observedLayouts = new Proxy(layouts, { + ownKeys: (target) => { + layoutEnumerations += 1 + return Reflect.ownKeys(target) + } + }) + useAppStore.setState({ terminalLayoutsByTabId: observedLayouts }) + + recordRuntimeTerminalInputForPtyId('pty-0', 10_000) + expect({ layoutEnumerations, leafEnumerations }).toEqual({ + layoutEnumerations: 1, + leafEnumerations: 1 + }) + layoutEnumerations = 0 + leafEnumerations = 0 + + for (let timestamp = 1; timestamp <= 100; timestamp += 1) { + recordRuntimeTerminalInputForPtyId(`pty-${layoutCount - 1}`, timestamp * 10_000) + } + + expect({ layoutEnumerations, leafEnumerations }).toEqual({ + layoutEnumerations: 1, + leafEnumerations: layoutCount + }) + expect( + useAppStore.getState().lastTerminalInputAtByPaneKey[ + `tab-${layoutCount - 1}:00000000-0000-4000-8000-${(layoutCount - 1) + .toString(16) + .padStart(12, '0')}` + ] + ).toBe(1_000_000) + }) + + it('reindexes a PTY when the immutable layout identity changes', () => { + const nextLeafId = '22222222-2222-4222-8222-222222222222' + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'rebound-pty' } + } + } + }) + recordRuntimeTerminalInputForPtyId('rebound-pty', 123) + + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-2': { + root: { type: 'leaf', leafId: nextLeafId }, + activeLeafId: nextLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { [nextLeafId]: 'rebound-pty' } + } + } + }) + recordRuntimeTerminalInputForPtyId('rebound-pty', 1_000) + + expect(useAppStore.getState().lastTerminalInputAtByPaneKey).toEqual({ + [PANE_KEY]: 123, + [`tab-2:${nextLeafId}`]: 1_000 + }) + }) + + it('reindexes stale owners without caching misses within the same layout identity', () => { + const nextLeafId = '22222222-2222-4222-8222-222222222222' + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'rebound-pty' } + } + } + }) + const layouts = useAppStore.getState().terminalLayoutsByTabId + recordRuntimeTerminalInputForPtyId('rebound-pty', 123) + recordRuntimeTerminalInputForPtyId('late-pty', 456) + + const firstLayout = layouts['tab-1'] + expect(firstLayout).toBeDefined() + if (!firstLayout) { + return + } + firstLayout.ptyIdsByLeafId = Object.create({ [LEAF_ID]: 'rebound-pty' }) + Object.setPrototypeOf(layouts, { 'tab-1': firstLayout }) + delete layouts['tab-1'] + layouts['tab-2'] = { + root: { type: 'leaf', leafId: nextLeafId }, + activeLeafId: nextLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { [nextLeafId]: 'rebound-pty', [LEAF_ID]: 'late-pty' } + } + recordRuntimeTerminalInputForPtyId('rebound-pty', 1_000) + recordRuntimeTerminalInputForPtyId('late-pty', 2_000) + + expect(useAppStore.getState().terminalLayoutsByTabId).toBe(layouts) + expect(useAppStore.getState().lastTerminalInputAtByPaneKey).toEqual({ + [PANE_KEY]: 123, + [`tab-2:${nextLeafId}`]: 1_000, + [`tab-2:${LEAF_ID}`]: 2_000 + }) + }) + + it('preserves a malformed first match instead of routing a duplicate PTY', () => { + const nextLeafId = '22222222-2222-4222-8222-222222222222' + useAppStore.setState({ + terminalLayoutsByTabId: { + 'legacy:tab': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'duplicate-pty' } + }, + 'tab-2': { + root: { type: 'leaf', leafId: nextLeafId }, + activeLeafId: nextLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { [nextLeafId]: 'duplicate-pty' } + } + } + }) + + recordRuntimeTerminalInputForPtyId('duplicate-pty', 123) + + expect(useAppStore.getState().lastTerminalInputAtByPaneKey).toEqual({}) + }) + it('reports success after fallback fire-and-forget writes when local acceptance cannot be verified', async () => { localWriteAccepted.mockResolvedValue(false) diff --git a/src/renderer/src/runtime/runtime-terminal-inspection.ts b/src/renderer/src/runtime/runtime-terminal-inspection.ts index 29be1aed435..7578c161116 100644 --- a/src/renderer/src/runtime/runtime-terminal-inspection.ts +++ b/src/renderer/src/runtime/runtime-terminal-inspection.ts @@ -1,6 +1,6 @@ import type { GlobalSettings } from '../../../shared/global-settings-types' import type { RuntimeTerminalSend } from '../../../shared/runtime-types' -import { makePaneKey } from '../../../shared/stable-pane-id' +import { makePaneKey, type PaneKey } from '../../../shared/stable-pane-id' import { isTerminalInputTooLargeWithDeferredMeasurement } from '../../../shared/terminal-input' import { useAppStore } from '../store' import { RuntimeRpcCallError, callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' @@ -18,6 +18,56 @@ export type RuntimeTerminalProcessInspection = { const REMOTE_PTY_ID_PREFIX = 'remote:' const DESKTOP_RUNTIME_CLIENT = { id: 'orca-desktop', type: 'desktop' } as const +type TerminalLayoutsByTabId = ReturnType['terminalLayoutsByTabId'] +type TerminalPaneOwner = { + tabId: string + leafId: string + paneKey: PaneKey +} + +const paneOwnersByPtyIdByLayoutIdentity = new WeakMap< + TerminalLayoutsByTabId, + Map +>() + +function resolvePaneKeyForPtyId(layouts: TerminalLayoutsByTabId, ptyId: string): PaneKey | null { + let paneOwnersByPtyId = paneOwnersByPtyIdByLayoutIdentity.get(layouts) + if (!paneOwnersByPtyId) { + paneOwnersByPtyId = new Map() + paneOwnersByPtyIdByLayoutIdentity.set(layouts, paneOwnersByPtyId) + } + const cachedOwner = paneOwnersByPtyId.get(ptyId) + if (cachedOwner) { + const layout = Object.prototype.propertyIsEnumerable.call(layouts, cachedOwner.tabId) + ? layouts[cachedOwner.tabId] + : undefined + const ptyIdsByLeafId = layout?.ptyIdsByLeafId + if ( + ptyIdsByLeafId && + Object.prototype.propertyIsEnumerable.call(ptyIdsByLeafId, cachedOwner.leafId) && + ptyIdsByLeafId[cachedOwner.leafId] === ptyId + ) { + return cachedOwner.paneKey + } + paneOwnersByPtyId.delete(ptyId) + } + for (const [tabId, layout] of Object.entries(layouts)) { + for (const [leafId, leafPtyId] of Object.entries(layout?.ptyIdsByLeafId ?? {})) { + if (leafPtyId !== ptyId) { + continue + } + try { + const paneKey = makePaneKey(tabId, leafId) + paneOwnersByPtyId.set(ptyId, { tabId, leafId, paneKey }) + return paneKey + } catch { + // Preserve first-match behavior for malformed legacy layout rows. + return null + } + } + } + return null +} function isRuntimePtyInputTooLarge(data: string): boolean | Promise { return isTerminalInputTooLargeWithDeferredMeasurement(data) @@ -49,21 +99,17 @@ function isTerminalGoneError(error: unknown): boolean { export function recordRuntimeTerminalInputForPtyId(ptyId: string, timestamp = Date.now()): void { const state = useAppStore.getState() - for (const [tabId, layout] of Object.entries(state.terminalLayoutsByTabId)) { - for (const [leafId, leafPtyId] of Object.entries(layout?.ptyIdsByLeafId ?? {})) { - if (leafPtyId !== ptyId) { - continue - } - try { - // Why: paired/runtime sends can bypass xterm.onData, so hibernation - // needs the same user-input marker from the PTY-id route. - state.recordTerminalInput(makePaneKey(tabId, leafId), timestamp) - } catch { - // Ignore malformed legacy layout data; the planner will stay - // conservative when a live PTY cannot be matched to an eligible pane. - } - return - } + const paneKey = resolvePaneKeyForPtyId(state.terminalLayoutsByTabId, ptyId) + if (!paneKey) { + return + } + try { + // Why: paired/runtime sends can bypass xterm.onData, so hibernation + // needs the same user-input marker from the PTY-id route. + state.recordTerminalInput(paneKey, timestamp) + } catch { + // Ignore malformed legacy layout data; the planner will stay + // conservative when a live PTY cannot be matched to an eligible pane. } } From 1fe558dd284477f03dcacbed2128feca49af2d59 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:02:29 -0700 Subject: [PATCH 11/31] perf(automations): narrow status observer updates (#17249) --- ...tomation-agent-status-entry-change.test.ts | 66 ++++++++++++++ .../automation-agent-status-entry-change.ts | 15 ++++ ...automation-dispatch-observer-test-probe.ts | 30 +++++++ .../hooks/useAutomationDispatchEvents.test.ts | 4 +- .../src/hooks/useAutomationDispatchEvents.ts | 89 +++++++++++-------- 5 files changed, 164 insertions(+), 40 deletions(-) create mode 100644 src/renderer/src/hooks/automation-agent-status-entry-change.test.ts create mode 100644 src/renderer/src/hooks/automation-agent-status-entry-change.ts create mode 100644 src/renderer/src/hooks/automation-dispatch-observer-test-probe.ts diff --git a/src/renderer/src/hooks/automation-agent-status-entry-change.test.ts b/src/renderer/src/hooks/automation-agent-status-entry-change.test.ts new file mode 100644 index 00000000000..7079f81af8d --- /dev/null +++ b/src/renderer/src/hooks/automation-agent-status-entry-change.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import type { AgentStatusEntry } from '../../../shared/agent-status-types' +import { + selectAutomationAgentStatusEntryChange, + UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY +} from './automation-agent-status-entry-change' + +function makeEntry(paneKey: string): AgentStatusEntry { + return { + paneKey, + state: 'working', + prompt: 'turn', + updatedAt: 1, + stateStartedAt: 1, + stateHistory: [] + } +} + +describe('selectAutomationAgentStatusEntryChange', () => { + it('reads only the target and skips an unchanged entry', () => { + const targetPaneKey = 'target-tab:leaf' + const targetEntry = makeEntry(targetPaneKey) + const entries = Object.fromEntries( + Array.from({ length: 499 }, (_, index) => { + const paneKey = `other-tab:${index}` + return [paneKey, makeEntry(paneKey)] + }) + ) + entries[targetPaneKey] = targetEntry + let enumerations = 0 + const measuredEntries = new Proxy(entries, { + ownKeys: (target) => { + enumerations += 1 + return Reflect.ownKeys(target) + } + }) + + expect(selectAutomationAgentStatusEntryChange(measuredEntries, targetPaneKey, undefined)).toBe( + targetEntry + ) + expect( + selectAutomationAgentStatusEntryChange(measuredEntries, targetPaneKey, targetEntry) + ).toBe(UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY) + expect(enumerations).toBe(0) + }) + + it('reports removal and ignores inherited or non-enumerable entries', () => { + const targetPaneKey = 'target-tab:leaf' + const previousEntry = makeEntry(targetPaneKey) + const inheritedEntries = Object.create({ [targetPaneKey]: previousEntry }) as Record< + string, + AgentStatusEntry + > + const nonEnumerableEntries = Object.defineProperty({}, targetPaneKey, { + value: previousEntry + }) as Record + + expect(selectAutomationAgentStatusEntryChange({}, targetPaneKey, previousEntry)).toBeUndefined() + expect(selectAutomationAgentStatusEntryChange(inheritedEntries, targetPaneKey, undefined)).toBe( + UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY + ) + expect( + selectAutomationAgentStatusEntryChange(nonEnumerableEntries, targetPaneKey, undefined) + ).toBe(UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY) + }) +}) diff --git a/src/renderer/src/hooks/automation-agent-status-entry-change.ts b/src/renderer/src/hooks/automation-agent-status-entry-change.ts new file mode 100644 index 00000000000..1c919e6b417 --- /dev/null +++ b/src/renderer/src/hooks/automation-agent-status-entry-change.ts @@ -0,0 +1,15 @@ +import type { AgentStatusEntry } from '../../../shared/agent-status-types' + +export const UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY = Symbol('unchanged-agent-status-entry') + +export function selectAutomationAgentStatusEntryChange( + entries: Readonly>, + targetPaneKey: string, + previousEntry: AgentStatusEntry | undefined +): AgentStatusEntry | undefined | typeof UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY { + const entry = Object.prototype.propertyIsEnumerable.call(entries, targetPaneKey) + ? entries[targetPaneKey] + : undefined + // Why: status writes replace entries; unchanged identity proves an unrelated publication. + return entry === previousEntry ? UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY : entry +} diff --git a/src/renderer/src/hooks/automation-dispatch-observer-test-probe.ts b/src/renderer/src/hooks/automation-dispatch-observer-test-probe.ts new file mode 100644 index 00000000000..302c052782c --- /dev/null +++ b/src/renderer/src/hooks/automation-dispatch-observer-test-probe.ts @@ -0,0 +1,30 @@ +import type { AgentStatusEntry } from '../../../shared/agent-status-types' + +const TARGET_PANE_KEY = 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d' + +export function countUnchangedObserverHistoryReads( + state: { agentStatusByPaneKey: Record }, + subscriber: (() => void) | null +): number { + if (!subscriber) { + throw new Error('agent status observer was not registered') + } + let historyReads = 0 + state.agentStatusByPaneKey = { + [TARGET_PANE_KEY]: { + paneKey: TARGET_PANE_KEY, + state: 'working', + prompt: 'turn', + updatedAt: Date.now() + 1, + stateStartedAt: Date.now() + 1, + get stateHistory() { + historyReads += 1 + return [] + } + } + } + subscriber() + historyReads = 0 + subscriber() + return historyReads +} diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts index 81fc23a5728..b8c1cb49c17 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts @@ -1,6 +1,7 @@ import type * as ReactModule from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { AUTOMATIONS_CHANGED_EVENT } from '@/lib/automations-changed-window-event' +import { countUnchangedObserverHistoryReads } from './automation-dispatch-observer-test-probe' const mockDispatchEvent = vi.fn() @@ -620,10 +621,11 @@ describe('useAutomationDispatchEvents setup launch', () => { await vi.waitFor(() => expect(mockFinalizeTerminalOwnership).toHaveBeenCalledOnce()) }) - it('persists assistant output from a batched working→done→working transition', async () => { + it('skips unchanged status and persists batched working→done→working output', async () => { const paneKey = 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d' await registerAndDispatch() + expect(countUnchangedObserverHistoryReads(state, latestStoreSubscriber)).toBe(0) const transitionStartedAt = Date.now() + 1 state.agentStatusByPaneKey = { [paneKey]: { diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.ts index 1e46b315f28..e06eefa6e61 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.ts @@ -32,8 +32,12 @@ import { toSshExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' -import type { AgentStateHistoryEntry } from '../../../shared/agent-status-types' +import type { AgentStateHistoryEntry, AgentStatusEntry } from '../../../shared/agent-status-types' import { resolveFolderWorkspaceHost } from '../../../shared/folder-workspace-execution-host' +import { + selectAutomationAgentStatusEntryChange, + UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY +} from './automation-agent-status-entry-change' const activeReuseDispatchTabIds = new Set() @@ -450,57 +454,64 @@ export function useAutomationDispatchEvents(): void { ): void => { let sawWorkingAfterStart = false let observedStateHistory: AgentStateHistoryEntry[] = [] + let observedEntry: AgentStatusEntry | undefined const checkCurrentStatus = (): void => { - const { agentStatusByPaneKey } = useAppStore.getState() - for (const [paneKey, entry] of Object.entries(agentStatusByPaneKey)) { - if (paneKey !== targetPaneKey || entry.updatedAt < startedAfter) { + const entryChange = selectAutomationAgentStatusEntryChange( + useAppStore.getState().agentStatusByPaneKey, + targetPaneKey, + observedEntry + ) + if (entryChange === UNCHANGED_AUTOMATION_AGENT_STATUS_ENTRY) { + return + } + const entry = entryChange + observedEntry = entry + if (!entry || entry.updatedAt < startedAfter) { + return + } + const historyOverlap = getAgentStateHistoryOverlap( + observedStateHistory, + entry.stateHistory + ) + // Why: sawWorkingAfterStart stays monotonic — a recreated entry + // (transport loss, PTY exit, cap eviction) arrives with an empty + // stateHistory, so clearing it here would strand reuseSession runs + // with nothing left to re-derive the working edge from. + for (const historicalState of entry.stateHistory.slice(historyOverlap)) { + if (historicalState.startedAt < startedAfter) { continue } - const historyOverlap = getAgentStateHistoryOverlap( - observedStateHistory, - entry.stateHistory - ) - // Why: sawWorkingAfterStart stays monotonic — a recreated entry - // (transport loss, PTY exit, cap eviction) arrives with an empty - // stateHistory, so clearing it here would strand reuseSession runs - // with nothing left to re-derive the working edge from. - for (const historicalState of entry.stateHistory.slice(historyOverlap)) { - if (historicalState.startedAt < startedAfter) { - continue - } - if (historicalState.state === 'working') { - sawWorkingAfterStart = true - } - if ( - historicalState.state === 'done' && - (!options?.requireWorkingAfterStart || sawWorkingAfterStart) - ) { - // Why: this `done` already rolled out of the live entry, so its output - // survives only in the entry-level completed slot. - latestAssistantMessage = - entry.lastCompletedAssistantMessage?.trim() || latestAssistantMessage - handleAgentDone() - return - } - } - observedStateHistory = [...entry.stateHistory] - if (entry.state === 'working') { + if (historicalState.state === 'working') { sawWorkingAfterStart = true } if ( - entry.state === 'done' && - // Why: a session-boundary done is the agent CONNECTING (Claude SessionStart - // fires at launch, before the argv prompt submits) — completing here would - // close the tab and record an empty run result. - entry.sessionBoundary !== true && + historicalState.state === 'done' && (!options?.requireWorkingAfterStart || sawWorkingAfterStart) ) { + // Why: this `done` already rolled out of the live entry, so its output + // survives only in the entry-level completed slot. latestAssistantMessage = - entry.lastAssistantMessage?.trim() || latestAssistantMessage + entry.lastCompletedAssistantMessage?.trim() || latestAssistantMessage handleAgentDone() return } } + observedStateHistory = [...entry.stateHistory] + if (entry.state === 'working') { + sawWorkingAfterStart = true + } + if ( + entry.state === 'done' && + // Why: a session-boundary done is the agent CONNECTING (Claude SessionStart + // fires at launch, before the argv prompt submits) — completing here would + // close the tab and record an empty run result. + entry.sessionBoundary !== true && + (!options?.requireWorkingAfterStart || sawWorkingAfterStart) + ) { + latestAssistantMessage = + entry.lastAssistantMessage?.trim() || latestAssistantMessage + handleAgentDone() + } } // Why: Codex/Claude completion normally arrives through the global // hook IPC listener, not the hidden PTY OSC fallback. From 9c777ca89e175169fad79ef34bccfdbb36613822 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:03:35 -0700 Subject: [PATCH 12/31] perf(renderer): gate pending agent-status retries (#17254) --- .../ipc-events/agent-status-ipc-bridge.ts | 11 +- .../agent-status-pending-retry-gate.test.ts | 191 ++++++++++++++++++ .../agent-status-pending-retry-gate.ts | 29 +++ ...Events-agent-status-queue-ordering.test.ts | 27 ++- ...vents-agent-status-snapshot-replay.test.ts | 6 +- 5 files changed, 249 insertions(+), 15 deletions(-) create mode 100644 src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts create mode 100644 src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts diff --git a/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts index 3a99527b1d3..26058a3e9e0 100644 --- a/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts @@ -13,6 +13,7 @@ import type { AgentStatusBatchEvent, PendingAgentStatusEvent } from './agent-status-bridge-types' +import { shouldRetryPendingAgentStatusesAfterStoreUpdate } from './agent-status-pending-retry-gate' const PENDING_AGENT_STATUS_RETRY_MS = 100 const PENDING_AGENT_STATUS_TTL_MS = 15_000 @@ -94,8 +95,8 @@ export function registerAgentStatusIpcBridge(unsubs: (() => void)[]): AgentStatu } } finally { isFlushingAgentStatuses = false + schedulePendingAgentStatusFlush() } - schedulePendingAgentStatusFlush() } const applyAgentStatus = createAgentStatusEventApplicator({ @@ -261,7 +262,13 @@ export function registerAgentStatusIpcBridge(unsubs: (() => void)[]): AgentStatu requestAgentStatusSnapshotIfReady() const unsubscribeAgentStatusStore = useAppStore.subscribe((state, previousState) => { requestAgentStatusSnapshotIfReady() - flushPendingAgentStatuses() + // Why: the timer covers module-owned rekeys; unrelated store writes cannot change attribution and must not rebuild its routing index. + if ( + pendingAgentStatusEvents.length > 0 && + shouldRetryPendingAgentStatusesAfterStoreUpdate(state, previousState) + ) { + flushPendingAgentStatuses() + } syncAgentHookCompletionNotificationsForStoreUpdate(state, previousState) }) diff --git a/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts new file mode 100644 index 00000000000..51e8161a8fc --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts @@ -0,0 +1,191 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { AgentStatusBatchUpdate } from '../../store/slices/agent-status' +import { + buildStoreState, + FUTURE_LEAF_ID, + FUTURE_PANE_KEY, + type AgentStatusSetData, + type StoreLike, + type StoreSubscribeListener +} from '../ipc-events-agent-status-store-test-fixtures' +import { shouldRetryPendingAgentStatusesAfterStoreUpdate } from './agent-status-pending-retry-gate' + +type RetryState = Parameters[0] + +function createRetryState(): RetryState { + return { + workspaceSessionReady: true, + tabsByWorktree: {}, + unifiedTabsByWorktree: {}, + terminalLayoutsByTabId: {}, + worktreesByRepo: {}, + repos: [], + recentlyClosedAgentStatusTabIds: {}, + recentlyRetiredAgentStatusPaneKeys: {} + } +} + +type PendingRetryHarness = { + bridge: { disposeAsyncState: () => void; unsubscribeStore: () => void } + emitPendingStatus: () => void + publish: (mutate: (state: StoreLike) => void) => void + setAgentStatuses: ReturnType + transactAgentStatuses: ReturnType +} + +async function createPendingRetryHarness(): Promise { + const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const setAgentStatuses = vi.fn((updates: readonly AgentStatusBatchUpdate[]) => + updates.map(() => true) + ) + const storeState = buildStoreState({ + setAgentStatuses, + workspaceSessionReady: true, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + settings: { terminalFontSize: 13, notifications: { enabled: false } } + }) + const transactImplementation = storeState.transactAgentStatuses as (...args: unknown[]) => unknown + const transactAgentStatuses = vi.fn(transactImplementation) + storeState.transactAgentStatuses = transactAgentStatuses + + vi.doMock('../../store', () => ({ + useAppStore: { + subscribe: vi.fn((listener: StoreSubscribeListener) => { + subscribeListenerRef.current = listener + return () => { + subscribeListenerRef.current = null + } + }), + getState: () => storeState + } + })) + vi.doMock('../agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn(), + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() + })) + vi.stubGlobal('window', { + api: { + agentStatus: { + onSet: (listener: (data: AgentStatusSetData) => void) => { + onSetListenerRef.current = listener + return () => { + onSetListenerRef.current = null + } + } + } + } + }) + + const { registerAgentStatusIpcBridge } = await import('./agent-status-ipc-bridge') + const bridge = registerAgentStatusIpcBridge([]) + if (!subscribeListenerRef.current || !onSetListenerRef.current) { + throw new Error('Expected agent-status bridge listeners') + } + + return { + bridge, + emitPendingStatus: () => { + onSetListenerRef.current?.({ + paneKey: FUTURE_PANE_KEY, + state: 'working', + prompt: 'pending attribution', + agentType: 'claude', + receivedAt: 1_700_000_000_000, + stateStartedAt: 1_700_000_000_000 + }) + }, + publish: (mutate) => { + const previousState = { ...storeState } + mutate(storeState) + subscribeListenerRef.current?.(storeState, previousState) + }, + setAgentStatuses, + transactAgentStatuses + } +} + +describe('agent status pending retry gate', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + vi.setSystemTime(1_700_000_100_000) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('tracks every store-owned input that can resolve or retire pending attribution', () => { + const previous = createRetryState() + const changes: RetryState[] = [ + { ...previous, workspaceSessionReady: false }, + { ...previous, tabsByWorktree: {} }, + { ...previous, unifiedTabsByWorktree: {} }, + { ...previous, terminalLayoutsByTabId: {} }, + { ...previous, worktreesByRepo: {} }, + { ...previous, repos: [] }, + { ...previous, recentlyClosedAgentStatusTabIds: {} }, + { ...previous, recentlyRetiredAgentStatusPaneKeys: {} } + ] + + expect(shouldRetryPendingAgentStatusesAfterStoreUpdate({ ...previous }, previous)).toBe(false) + for (const current of changes) { + expect(shouldRetryPendingAgentStatusesAfterStoreUpdate(current, previous)).toBe(true) + } + }) + + it('skips retry transactions for unrelated publications and retries on hydration', async () => { + const harness = await createPendingRetryHarness() + harness.emitPendingStatus() + + for (let updateIndex = 0; updateIndex < 2_400; updateIndex += 1) { + harness.publish((state) => { + state.cosmeticUpdateIndex = updateIndex + }) + } + + expect(harness.transactAgentStatuses).not.toHaveBeenCalled() + + harness.publish((state) => { + state.tabsByWorktree = { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future' }] + } + state.terminalLayoutsByTabId = { + 'tab-future': { + root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, + activeLeafId: FUTURE_LEAF_ID, + expandedLeafId: null + } + } + }) + + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + expect(harness.setAgentStatuses).toHaveBeenCalledWith([ + expect.objectContaining({ paneKey: FUTURE_PANE_KEY }) + ]) + vi.advanceTimersByTime(100) + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + harness.bridge.unsubscribeStore() + harness.bridge.disposeAsyncState() + }) + + it('keeps the timer fallback when routing references stay unchanged', async () => { + const harness = await createPendingRetryHarness() + harness.emitPendingStatus() + harness.publish((state) => { + state.cosmeticUpdateIndex = 1 + }) + + expect(harness.transactAgentStatuses).not.toHaveBeenCalled() + vi.advanceTimersByTime(100) + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + expect(harness.setAgentStatuses).toHaveBeenCalledWith([]) + harness.bridge.unsubscribeStore() + harness.bridge.disposeAsyncState() + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts new file mode 100644 index 00000000000..4b8dcc36a9e --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts @@ -0,0 +1,29 @@ +import type { AppState } from '../../store/types' + +type AgentStatusPendingRetryState = Pick< + AppState, + | 'workspaceSessionReady' + | 'tabsByWorktree' + | 'unifiedTabsByWorktree' + | 'terminalLayoutsByTabId' + | 'worktreesByRepo' + | 'repos' + | 'recentlyClosedAgentStatusTabIds' + | 'recentlyRetiredAgentStatusPaneKeys' +> + +export function shouldRetryPendingAgentStatusesAfterStoreUpdate( + current: AgentStatusPendingRetryState, + previous: AgentStatusPendingRetryState +): boolean { + return ( + current.workspaceSessionReady !== previous.workspaceSessionReady || + current.tabsByWorktree !== previous.tabsByWorktree || + current.unifiedTabsByWorktree !== previous.unifiedTabsByWorktree || + current.terminalLayoutsByTabId !== previous.terminalLayoutsByTabId || + current.worktreesByRepo !== previous.worktreesByRepo || + current.repos !== previous.repos || + current.recentlyClosedAgentStatusTabIds !== previous.recentlyClosedAgentStatusTabIds || + current.recentlyRetiredAgentStatusPaneKeys !== previous.recentlyRetiredAgentStatusPaneKeys + ) +} diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts index 322e253e541..eb613f21e70 100644 --- a/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentStatusUpdate } from '../store/slices/agent-status' import { YOLO_TUI_AGENT_ARGS } from '../../../shared/tui-agent-permissions' import { @@ -22,6 +22,10 @@ describe('useIpcEvents agent status snapshot integration', () => { vi.unstubAllGlobals() }) + afterEach(() => { + vi.useRealTimers() + }) + it('preserves queued set-clear order for working removal and done retention', async () => { vi.useFakeTimers() let storeState: StoreLike @@ -163,7 +167,7 @@ describe('useIpcEvents agent status snapshot integration', () => { it('does not recurse when flushing a pending status re-enters via the store subscriber', async () => { // Repro for crash 9fc89529 (RangeError: Maximum call stack size exceeded): - // the store subscriber calls flushPendingAgentStatuses() on every update. + // the store subscriber retries pending statuses synchronously on hydration. // flush -> applyAgentStatus -> store.setAgentStatus notifies subscribers // synchronously (like Zustand) -> subscriber -> flush again while the same // event is still queued -> infinite recursion. Model setAgentStatus with a @@ -173,10 +177,10 @@ describe('useIpcEvents agent status snapshot integration', () => { current: null } let setAgentStatusCalls = 0 - const notify = (): void => { + const notify = (previousState: StoreLike = storeState): void => { const listener = subscribeListenerRef.current if (listener) { - listener(storeState, storeState) + listener(storeState, previousState) } } const storeState: StoreLike = buildStoreState({ @@ -243,6 +247,7 @@ describe('useIpcEvents agent status snapshot integration', () => { // Tab hydrates; the next store update flushes the pending event. Without the // re-entrancy guard this overflows the stack instead of applying once. + const beforeHydration = { ...storeState } storeState.tabsByWorktree = { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] } @@ -254,7 +259,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } - expect(() => notify()).not.toThrow() + expect(() => notify(beforeHydration)).not.toThrow() // Applied exactly once — the re-entrant flush is a no-op, not a loop. expect(setAgentStatusCalls).toBe(1) }) @@ -263,6 +268,7 @@ describe('useIpcEvents agent status snapshot integration', () => { // buffered event permanently. Before batching the queue was only replaced after the loop, // so a throw left it intact — keep that. it('keeps pending statuses queued when the retry fold throws', async () => { + vi.useFakeTimers() const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { current: null @@ -284,8 +290,6 @@ describe('useIpcEvents agent status snapshot integration', () => { tabsByWorktree: {}, terminalLayoutsByTabId: {} }) - const notify = (): void => subscribeListenerRef.current?.(storeState, storeState) - stubReactSyncEffect() vi.doMock('../store', () => ({ useAppStore: { @@ -325,7 +329,10 @@ describe('useIpcEvents agent status snapshot integration', () => { stateStartedAt: 1_700_000_000_100 }) - // Tab hydrates, so the next store update flushes the pending event — and throws. + // The timer-owned retry throws after clearing its handle. + expect(() => vi.advanceTimersByTime(100)).toThrow('fold blew up') + + // Hydrate without publishing so only the re-armed timer can recover the event. storeState.tabsByWorktree = { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] } @@ -336,10 +343,8 @@ describe('useIpcEvents agent status snapshot integration', () => { expandedLeafId: null } } - expect(() => notify()).toThrow('fold blew up') - // The event must still be queued, so the next flush replays it. - notify() + vi.advanceTimersByTime(100) const replayedAfterThrow = setAgentStatuses.mock.calls .slice(1) .flatMap((call) => call[0].map((update) => update.payload.prompt)) diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts index cbb4992e7f2..e26448f0b63 100644 --- a/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts @@ -87,6 +87,7 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() + const beforeHydration = { ...storeState } Object.assign(storeState, { tabsByWorktree: { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'SSH Tab' }] @@ -99,7 +100,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } }) - subscribeListenerRef.current?.(storeState, storeState) + subscribeListenerRef.current?.(storeState, beforeHydration) expect(setAgentStatus).toHaveBeenCalledTimes(1) expect(setAgentStatus).toHaveBeenCalledWith( @@ -192,6 +193,7 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() + const beforeHydration = { ...storeState } Object.assign(storeState, { tabsByWorktree: { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'SSH Tab' }] @@ -204,7 +206,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } }) - subscribeListenerRef.current?.(storeState, storeState) + subscribeListenerRef.current?.(storeState, beforeHydration) expect(setAgentStatus).toHaveBeenCalledWith( FUTURE_PANE_KEY, From 7ae916cebd7b14f9d1e55f3866c042102e51583b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:06:15 -0700 Subject: [PATCH 13/31] perf(worktrees): batch-prune stale local metadata (#17278) --- .../ipc/registered-worktree-roots-cache.ts | 16 + ...thoritative-local-metadata-pruning.test.ts | 611 ++++++++++++++++++ src/main/ipc/worktrees-test-harness.ts | 9 +- src/main/ipc/worktrees-test-ipc-surface.ts | 4 + .../worktrees-test-metadata-pruning-store.ts | 35 + src/main/ipc/worktrees-windows.test.ts | 16 + ...itative-local-worktree-metadata-pruning.ts | 141 ++++ .../listing/detected-provider-listing.ts | 23 +- .../listing/detected-worktree-scan-cache.ts | 118 +++- .../register-worktree-catalog-handlers.ts | 29 +- src/main/local-worktree-path-presence.test.ts | 219 +++++++ src/main/local-worktree-path-presence.ts | 72 +++ src/main/local-worktree-scan-generation.ts | 25 + src/main/persistence-initial-load.test.ts | 11 +- src/main/persistence-repo-lifecycle.test.ts | 39 ++ src/main/persistence-settings-update.test.ts | 19 + src/main/persistence-update-repo.test.ts | 23 + .../applying-settings/settings-update.ts | 11 + .../metadata-lineage-batch-pruning.test.ts | 64 ++ .../metadata-lineage-operations.ts | 31 + .../loading-store/profile-preferences.ts | 2 + .../project-collection-operations.ts | 2 + .../repo-lifecycle-operations.ts | 15 + .../worktree-identity-metadata.ts | 13 +- .../session-worktree-ownership.test.ts | 166 +++++ .../session-worktree-ownership.ts | 246 +++++++ ...ktree-metadata-batch-pruning.bench.test.ts | 106 +++ ...worktree-metadata-scan-expectation.test.ts | 242 +++++++ ...ocal-worktree-metadata-scan-expectation.ts | 243 +++++++ ...ng-local-worktree-metadata-pruning.test.ts | 477 ++++++++++++++ ...missing-local-worktree-metadata-pruning.ts | 153 +++++ .../tracking-repos/project-host-operations.ts | 4 + .../tracking-repos/repo-update-operations.ts | 6 + src/main/worktree-lineage-pruning.test.ts | 137 +++- src/main/worktree-lineage-pruning.ts | 35 +- .../worktree-retention-path-comparison.ts | 24 + src/shared/priority-semaphore.test.ts | 15 + src/shared/priority-semaphore.ts | 26 +- 38 files changed, 3382 insertions(+), 46 deletions(-) create mode 100644 src/main/ipc/worktrees-authoritative-local-metadata-pruning.test.ts create mode 100644 src/main/ipc/worktrees-test-metadata-pruning-store.ts create mode 100644 src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts create mode 100644 src/main/local-worktree-path-presence.test.ts create mode 100644 src/main/local-worktree-path-presence.ts create mode 100644 src/main/local-worktree-scan-generation.ts create mode 100644 src/main/persistence/loading-store/metadata-lineage-batch-pruning.test.ts create mode 100644 src/main/persistence/restoring-sessions/session-worktree-ownership.test.ts create mode 100644 src/main/persistence/restoring-sessions/session-worktree-ownership.ts create mode 100644 src/main/persistence/tracking-repos/local-worktree-metadata-batch-pruning.bench.test.ts create mode 100644 src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.test.ts create mode 100644 src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.ts create mode 100644 src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.test.ts create mode 100644 src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.ts create mode 100644 src/main/worktree-retention-path-comparison.ts diff --git a/src/main/ipc/registered-worktree-roots-cache.ts b/src/main/ipc/registered-worktree-roots-cache.ts index 8ca19984298..92ae4fc334f 100644 --- a/src/main/ipc/registered-worktree-roots-cache.ts +++ b/src/main/ipc/registered-worktree-roots-cache.ts @@ -7,6 +7,9 @@ import { isDescendantOrEqual, normalizeExistingPath } from './filesystem-path-co const registeredWorktreeRoots = new Set() const registeredWorktreeRootsByRepo = new Map>() const registeredWorktreeRootRepoIds = new Set() +const registeredWorktreeRootsRevisionByRepo = new Map() +let registeredWorktreeRootsRevisionSequence = 0 +let registeredWorktreeRootsBaseRevision = 0 let registeredWorktreeRootsDirty = true let registeredWorktreeRootsRefresh: Promise | null = null const AUTHORIZED_ROOTS_REBUILD_CONCURRENCY = 8 @@ -17,6 +20,8 @@ export function invalidateAuthorizedRootsCache(): void { registeredWorktreeRoots.clear() registeredWorktreeRootsByRepo.clear() registeredWorktreeRootRepoIds.clear() + registeredWorktreeRootsBaseRevision = ++registeredWorktreeRootsRevisionSequence + registeredWorktreeRootsRevisionByRepo.clear() } export async function rebuildAuthorizedRootsCache(store: Store): Promise { @@ -55,6 +60,8 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { registeredWorktreeRootRepoIds.add(repoId) } registeredWorktreeRootsDirty = false + registeredWorktreeRootsBaseRevision = ++registeredWorktreeRootsRevisionSequence + registeredWorktreeRootsRevisionByRepo.clear() } async function mapWithConcurrency( @@ -87,6 +94,10 @@ export function registerWorktreeRootsForRepo( if (!localRepoIds.has(registeredRepoId)) { registeredWorktreeRootsByRepo.delete(registeredRepoId) registeredWorktreeRootRepoIds.delete(registeredRepoId) + registeredWorktreeRootsRevisionByRepo.set( + registeredRepoId, + ++registeredWorktreeRootsRevisionSequence + ) } } @@ -98,10 +109,15 @@ export function registerWorktreeRootsForRepo( registeredWorktreeRootsByRepo.set(repoId, new Set(worktreeRoots.map((root) => resolve(root)))) registeredWorktreeRootRepoIds.add(repoId) + registeredWorktreeRootsRevisionByRepo.set(repoId, ++registeredWorktreeRootsRevisionSequence) refreshRegisteredWorktreeRoots() registeredWorktreeRootsDirty = !allLocalRepoRootsRegistered(localRepoIds) } +export function getRegisteredWorktreeRootsRevision(repoId: string): number { + return registeredWorktreeRootsRevisionByRepo.get(repoId) ?? registeredWorktreeRootsBaseRevision +} + export async function ensureAuthorizedRootsCache(store: Store): Promise { if (!registeredWorktreeRootsDirty) { return diff --git a/src/main/ipc/worktrees-authoritative-local-metadata-pruning.test.ts b/src/main/ipc/worktrees-authoritative-local-metadata-pruning.test.ts new file mode 100644 index 00000000000..aa3fff56f77 --- /dev/null +++ b/src/main/ipc/worktrees-authoritative-local-metadata-pruning.test.ts @@ -0,0 +1,611 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { GitWorktreeInfo } from '../../shared/worktree/types' +import type { NativeLocalWorktreeMetadataScanExpectation } from '../persistence/tracking-repos/missing-local-worktree-metadata-pruning' +import { notifyWorktreesChanged } from './worktree-remote' +import { + listWorktreesMock, + pruneCleanupScanSnapshotsMock, + pruneSpaceAnalysisSnapshotsMock +} from './worktrees-test-module-mocks' +import { handlers, mainWindow, setupWorktreeHandlers, store } from './worktrees-test-harness' +import { mockSelectedWslProjectRuntime } from './worktrees-test-fixtures' +import { pruneMetadataMissingFromAuthoritativeLocalScan } from './worktrees/listing/authoritative-local-worktree-metadata-pruning' +import { listDetectedWorktreesForCapturedRepo } from './worktrees/listing/detected-provider-listing' +import { getLocalWorktreeScanGeneration } from '../local-worktree-scan-generation' +import { + isRegisteredWorktreePath, + registerWorktreeRootsForRepo +} from './registered-worktree-roots-cache' + +const localWorktreePathPresenceMock = vi.hoisted(() => vi.fn()) + +vi.mock('electron', async () => + (await import('./worktrees-test-module-mocks')).electronModuleMock() +) +vi.mock('../git/worktree', async () => + (await import('./worktrees-test-module-mocks')).gitWorktreeModuleMock() +) +vi.mock('../git/runner', async () => + (await import('./worktrees-test-module-mocks')).gitRunnerModuleMock() +) +vi.mock('../git/repo', async () => + (await import('./worktrees-test-module-mocks')).gitRepoModuleMock() +) +vi.mock('../git/git-username', async (importOriginal) => ({ + ...(await importOriginal>()), + resolveLocalGitUsername: (await import('./worktrees-test-module-mocks')) + .resolveLocalGitUsernameMock +})) +vi.mock('../github/client', async () => + (await import('./worktrees-test-module-mocks')).githubClientModuleMock() +) +vi.mock('../source-control/hosted-review', async () => + (await import('./worktrees-test-module-mocks')).hostedReviewModuleMock() +) +vi.mock('../providers/ssh-git-dispatch', async () => + (await import('./worktrees-test-module-mocks')).sshGitDispatchModuleMock() +) +vi.mock('../providers/ssh-filesystem-dispatch', async () => + (await import('./worktrees-test-module-mocks')).sshFilesystemDispatchModuleMock() +) +vi.mock('./worktree-symlinks', async () => + (await import('./worktrees-test-module-mocks')).worktreeSymlinksModuleMock() +) +vi.mock('./ssh', async () => (await import('./worktrees-test-module-mocks')).sshModuleMock()) +vi.mock('../ssh/ssh-target-registry', async () => + (await import('./worktrees-test-module-mocks')).sshTargetRegistryModuleMock() +) +vi.mock('../hooks', async () => (await import('./worktrees-test-module-mocks')).hooksModuleMock()) +vi.mock('../setup-runner-script-text', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).setupRunnerScriptTextModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../worktree-runner-script', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).worktreeRunnerScriptModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../effective-hook-config', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).effectiveHookConfigModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../setup-hook-env-vars', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).setupHookEnvVarsModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('./worktree-logic', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).worktreeLogicModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../terminal-history-deletion', async () => + (await import('./worktrees-test-module-mocks')).terminalHistoryDeletionModuleMock() +) +vi.mock('../ports/advertised-url-watcher', async () => + (await import('./worktrees-test-module-mocks')).advertisedUrlWatcherModuleMock() +) +vi.mock('../workspace-cleanup-scan-snapshot', async () => + (await import('./worktrees-test-module-mocks')).workspaceCleanupScanSnapshotModuleMock() +) +vi.mock('../workspace-space-analysis-snapshot', async () => + (await import('./worktrees-test-module-mocks')).workspaceSpaceAnalysisSnapshotModuleMock() +) +vi.mock('../workspace-cleanup-removal-snapshot-prune', async () => + (await import('./worktrees-test-module-mocks')).workspaceCleanupRemovalSnapshotPruneModuleMock() +) +vi.mock('../local-worktree-path-presence', async (importOriginal) => ({ + ...(await importOriginal>()), + localWorktreePathsExistOrAreUnverifiable: localWorktreePathPresenceMock +})) +vi.mock('../runtime/worktree-teardown', async () => + (await import('./worktrees-test-module-mocks')).worktreeTeardownModuleMock() +) +vi.mock('./pty', async () => (await import('./worktrees-test-module-mocks')).ptyModuleMock()) + +const REPO_ID = 'repo-1' +const REPO_PATH = '/workspace/repo' +const LOCAL_HOST_ID = 'local' + +function worktree(path: string, overrides: Partial = {}): GitWorktreeInfo { + return { + path, + head: path, + branch: `refs/heads/${path.split('/').at(-1) ?? 'main'}`, + isBare: false, + isMainWorktree: path === REPO_PATH, + ...overrides + } +} + +function scanExpectation( + worktreeIds: readonly string[] +): NativeLocalWorktreeMetadataScanExpectation { + const expectedRepo = { + id: REPO_ID, + path: REPO_PATH, + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 + } + return { + repo: { + id: REPO_ID, + path: REPO_PATH, + kind: 'git', + expectedRepo + }, + routing: { + expectedProject: undefined, + expectedProjectUpdatedAt: undefined, + expectedSettings: + {} as NativeLocalWorktreeMetadataScanExpectation['routing']['expectedSettings'] + }, + metadata: worktreeIds.map((worktreeId) => ({ + worktreeId, + expectedLegacy: { + expectedPresent: true, + expectedMeta: undefined, + expectedSerialized: undefined, + expectedInstanceId: undefined, + expectedHostId: LOCAL_HOST_ID + }, + expectedAliases: [] + })) + } +} + +function listDetected(): Promise { + return handlers['worktrees:listDetected'](null, { repoId: REPO_ID }) as Promise +} + +const localListingCalls = [ + ['worktrees:listDetected', listDetected], + [ + 'worktrees:list', + () => handlers['worktrees:list'](null, { repoId: REPO_ID }) as Promise + ], + ['worktrees:listAll', () => handlers['worktrees:listAll'](null, undefined) as Promise] +] as const + +describe('authoritative local worktree metadata pruning integration', () => { + beforeEach(() => { + setupWorktreeHandlers() + localWorktreePathPresenceMock.mockReset() + localWorktreePathPresenceMock.mockImplementation( + async (pathValues: readonly string[]) => + new Map(pathValues.map((pathValue) => [pathValue, false])) + ) + }) + + it('captures metadata expectations before starting the Git scan', async () => { + const order: string[] = [] + store.captureNativeLocalWorktreeMetadataScanExpectation.mockImplementation(() => { + order.push('capture') + return scanExpectation([]) + }) + listWorktreesMock.mockImplementation(() => { + order.push('scan') + return Promise.resolve([worktree(REPO_PATH)]) + }) + + await listDetected() + + expect(order).toEqual(['capture', 'scan']) + }) + + it('prunes once for the fresh producer, while coalesced and cached callers do nothing', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + const rows = [worktree(REPO_PATH), worktree('/workspace/live')] + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockImplementation(async () => { + await Promise.resolve() + return rows + }) + + await Promise.all([listDetected(), listDetected(), listDetected()]) + await listDetected() + + expect(store.captureNativeLocalWorktreeMetadataScanExpectation).toHaveBeenCalledTimes(1) + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).toHaveBeenCalledTimes(1) + const firstPruneCall = store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mock + .calls[0] as [unknown, readonly { worktreeId: string }[]] | undefined + expect(firstPruneCall?.[1].map(({ worktreeId }) => worktreeId)).toEqual([staleId]) + expect(pruneCleanupScanSnapshotsMock).toHaveBeenCalledTimes(1) + expect(pruneCleanupScanSnapshotsMock).toHaveBeenCalledWith('/profile-a', [ + { worktreeId: staleId, executionHostId: LOCAL_HOST_ID } + ]) + expect(pruneSpaceAnalysisSnapshotsMock).toHaveBeenCalledTimes(1) + expect(pruneSpaceAnalysisSnapshotsMock).toHaveBeenCalledWith('/profile-a', [ + { worktreeId: staleId, executionHostId: LOCAL_HOST_ID } + ]) + }) + + it('does not prune when the authoritative scan is invalidated while in flight', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + let resolveScan: (rows: GitWorktreeInfo[]) => void = () => {} + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolveScan = resolve as (rows: GitWorktreeInfo[]) => void + }) + ) + + const pending = listDetected() + await Promise.resolve() + notifyWorktreesChanged(mainWindow as never, REPO_ID) + resolveScan([worktree(REPO_PATH), worktree('/workspace/live')]) + await pending + + expect(store.captureNativeLocalWorktreeMetadataScanExpectation).toHaveBeenCalledTimes(1) + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + expect(pruneCleanupScanSnapshotsMock).not.toHaveBeenCalled() + expect(pruneSpaceAnalysisSnapshotsMock).not.toHaveBeenCalled() + }) + + it('does not prune when the completed scan is invalidated before its caller resumes', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + let resolveScan: (rows: GitWorktreeInfo[]) => void = () => {} + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolveScan = resolve as (rows: GitWorktreeInfo[]) => void + }) + ) + + const pending = listDetected() + await Promise.resolve() + resolveScan([worktree(REPO_PATH)]) + queueMicrotask(() => notifyWorktreesChanged(mainWindow as never, REPO_ID)) + await pending + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + }) + + it.each(localListingCalls)( + 'skips stale WSL root and lineage side effects after %s caller resumption', + async (_channel, listWorktrees) => { + const orphanId = `${REPO_ID}::/workspace/orphan` + let resolveScan: (rows: GitWorktreeInfo[]) => void = () => {} + mockSelectedWslProjectRuntime() + store.getAllWorktreeLineage.mockReturnValue({ + [orphanId]: { + worktreeId: orphanId, + worktreeInstanceId: 'orphan-instance', + parentWorktreeId: `${REPO_ID}::${REPO_PATH}`, + parentWorktreeInstanceId: 'main-instance', + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 0 + } + }) + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolveScan = resolve as (rows: GitWorktreeInfo[]) => void + }) + ) + + const pending = listWorktrees() + await Promise.resolve() + resolveScan([worktree(REPO_PATH)]) + queueMicrotask(() => notifyWorktreesChanged(mainWindow as never, REPO_ID)) + await pending + + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(isRegisteredWorktreePath(REPO_PATH)).toBe(false) + } + ) + + it('preserves WSL roots and lineage created while an older Git scan is pending', async () => { + const newPath = '/workspace/new-wsl-worktree' + const newId = `${REPO_ID}::${newPath}` + let resolveScan: (rows: GitWorktreeInfo[]) => void = () => {} + mockSelectedWslProjectRuntime() + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolveScan = resolve as (rows: GitWorktreeInfo[]) => void + }) + ) + + const pending = listDetected() + await Promise.resolve() + + resolveScan([worktree(REPO_PATH)]) + queueMicrotask(() => { + registerWorktreeRootsForRepo(store as never, REPO_ID, [REPO_PATH, newPath]) + store.setWorktreeMeta(newId, { hostId: LOCAL_HOST_ID, instanceId: 'new-instance' }) + store.getAllWorktreeLineage.mockReturnValue({ + [newId]: { + worktreeId: newId, + worktreeInstanceId: 'new-instance', + parentWorktreeId: `${REPO_ID}::${REPO_PATH}`, + parentWorktreeInstanceId: 'main-instance', + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 0 + } + }) + }) + await pending + + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(isRegisteredWorktreePath(newPath)).toBe(true) + }) + + it.each(['scan generation', 'caller request'] as const)( + 'skips metadata, root, and lineage mutations when the %s is invalidated during path probes', + async (invalidation) => { + const staleId = `${REPO_ID}::/workspace/stale` + const orphanId = `${REPO_ID}::/workspace/orphan` + let callerCurrent = true + let resolvePresence: (presence: ReadonlyMap) => void = () => {} + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + store.getAllWorktreeLineage.mockReturnValue({ + [orphanId]: { + worktreeId: orphanId, + worktreeInstanceId: 'orphan-instance', + parentWorktreeId: `${REPO_ID}::${REPO_PATH}`, + parentWorktreeInstanceId: 'main-instance', + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 0 + } + }) + localWorktreePathPresenceMock.mockImplementationOnce( + () => + new Promise((resolve) => { + resolvePresence = resolve + }) + ) + listWorktreesMock.mockResolvedValue([worktree(REPO_PATH)]) + + const pending = listDetectedWorktreesForCapturedRepo( + store as never, + store.getRepo(REPO_ID) as never, + () => callerCurrent + ) + await vi.waitFor(() => expect(localWorktreePathPresenceMock).toHaveBeenCalledTimes(1)) + if (invalidation === 'scan generation') { + notifyWorktreesChanged(mainWindow as never, REPO_ID) + } else { + callerCurrent = false + } + resolvePresence(new Map([['/workspace/stale', false]])) + await pending + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(isRegisteredWorktreePath(REPO_PATH)).toBe(false) + expect(pruneCleanupScanSnapshotsMock).not.toHaveBeenCalled() + expect(pruneSpaceAnalysisSnapshotsMock).not.toHaveBeenCalled() + } + ) + + it('preserves roots and lineage created while an older path probe is pending', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + const newPath = '/workspace/new-worktree' + const newId = `${REPO_ID}::${newPath}` + let resolvePresence: (presence: ReadonlyMap) => void = () => {} + const metadata = new Map() + store.getWorktreeMeta.mockImplementation((worktreeId) => metadata.get(worktreeId)) + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + localWorktreePathPresenceMock.mockImplementationOnce( + () => + new Promise((resolve) => { + resolvePresence = resolve + }) + ) + listWorktreesMock.mockResolvedValue([worktree(REPO_PATH)]) + + const pending = listDetected() + await vi.waitFor(() => expect(localWorktreePathPresenceMock).toHaveBeenCalledTimes(1)) + + registerWorktreeRootsForRepo(store as never, REPO_ID, [REPO_PATH, newPath]) + const newMetadata = { hostId: LOCAL_HOST_ID, instanceId: 'new-instance' } + metadata.set(newId, newMetadata) + store.setWorktreeMeta(newId, newMetadata) + store.getAllWorktreeLineage.mockReturnValue({ + [newId]: { + worktreeId: newId, + worktreeInstanceId: newMetadata.instanceId, + parentWorktreeId: `${REPO_ID}::${REPO_PATH}`, + parentWorktreeInstanceId: 'main-instance', + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 0 + } + }) + resolvePresence(new Map([['/workspace/stale', false]])) + await pending + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).toHaveBeenCalledTimes(1) + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(isRegisteredWorktreePath(newPath)).toBe(true) + }) + + it('does not capture or prune on an initial WSL scan', async () => { + mockSelectedWslProjectRuntime() + listWorktreesMock.mockResolvedValue([worktree(REPO_PATH)]) + + await listDetected() + + expect(listWorktreesMock).toHaveBeenCalledWith(REPO_PATH, { wslDistro: 'Ubuntu' }) + expect(store.captureNativeLocalWorktreeMetadataScanExpectation).not.toHaveBeenCalled() + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + }) + + it('does not let a native scan that changes to WSL routing become destructive', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + const resolvers: ((rows: GitWorktreeInfo[]) => void)[] = [] + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolvers.push(resolve as (rows: GitWorktreeInfo[]) => void) + }) + ) + + const nativePending = listDetected() + await Promise.resolve() + mockSelectedWslProjectRuntime() + const wslPending = listDetected() + await Promise.resolve() + + resolvers[1]?.([worktree(REPO_PATH)]) + await wslPending + resolvers[0]?.([worktree(REPO_PATH), worktree('/workspace/live')]) + await nativePending + + expect(store.captureNativeLocalWorktreeMetadataScanExpectation).toHaveBeenCalledTimes(1) + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + expect(pruneCleanupScanSnapshotsMock).not.toHaveBeenCalled() + expect(pruneSpaceAnalysisSnapshotsMock).not.toHaveBeenCalled() + }) + + it.each([ + ['empty', [] as GitWorktreeInfo[]], + ['failed', new Error('git worktree list failed')] + ] as const)('does nothing for an %s authoritative scan', async (_label, result) => { + const staleId = `${REPO_ID}::/workspace/stale` + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + if (result instanceof Error) { + listWorktreesMock.mockRejectedValue(result) + } else { + listWorktreesMock.mockResolvedValue(result) + } + + await listDetected() + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).not.toHaveBeenCalled() + expect(pruneCleanupScanSnapshotsMock).not.toHaveBeenCalled() + expect(pruneSpaceAnalysisSnapshotsMock).not.toHaveBeenCalled() + }) + + it('keeps prunable Git registrations live and does not probe their paths', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + const liveId = `${REPO_ID}::/workspace/live` + const prunableId = `${REPO_ID}::/workspace/prunable` + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId, liveId, prunableId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockResolvedValue([ + worktree(REPO_PATH), + worktree('/workspace/live'), + worktree('/workspace/prunable', { prunable: true }) + ]) + + await listDetected() + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).toHaveBeenCalledTimes(1) + const missing = store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mock.calls[0]?.[1] as + | readonly { worktreeId: string }[] + | undefined + expect(missing?.map(({ worktreeId }) => worktreeId)).toEqual([staleId]) + expect(missing?.map(({ worktreeId }) => worktreeId)).not.toContain(liveId) + expect(missing?.map(({ worktreeId }) => worktreeId)).not.toContain(prunableId) + expect(localWorktreePathPresenceMock).toHaveBeenCalledWith(['/workspace/stale'], { + signal: undefined + }) + }) + + it('keeps the configured repo path live when Git reports a canonical spelling', async () => { + const configuredMainId = `${REPO_ID}::${REPO_PATH}` + const staleId = `${REPO_ID}::/workspace/stale` + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([configuredMainId, staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + listWorktreesMock.mockResolvedValue([worktree('/canonical/workspace/repo')]) + + await listDetected() + + const missing = store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mock.calls[0]?.[1] as + | readonly { worktreeId: string }[] + | undefined + expect(missing?.map(({ worktreeId }) => worktreeId)).toEqual([staleId]) + expect(missing?.map(({ worktreeId }) => worktreeId)).not.toContain(configuredMainId) + }) + + it('keeps a linked worktree whose persisted symlink spelling still exists', async () => { + const aliasId = `${REPO_ID}::/workspace/alias/linked` + const staleId = `${REPO_ID}::/workspace/stale` + const scan = scanExpectation([aliasId, staleId]) + const repo = scan.repo.expectedRepo! + const prune = vi.fn( + ( + _scan: NativeLocalWorktreeMetadataScanExpectation, + _missing: readonly { worktreeId: string }[] + ) => [] as string[] + ) + + await pruneMetadataMissingFromAuthoritativeLocalScan({ + store: { + getRepos: () => [repo], + pruneSessionlessMissingLocalWorktreeMetadataForRepo: prune + } as never, + repo, + gitWorktrees: [worktree('/workspace/real/linked')], + scan, + scanGeneration: getLocalWorktreeScanGeneration(REPO_ID), + pathsExistOrAreUnverifiable: async (pathValues) => + new Map(pathValues.map((pathValue) => [pathValue, pathValue === '/workspace/alias/linked'])) + }) + + expect(prune.mock.calls[0]?.[1].map(({ worktreeId }) => worktreeId)).toEqual([staleId]) + }) + + it('does not rematerialize a removed lineage parent metadata row', async () => { + const staleId = `${REPO_ID}::/workspace/stale` + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReturnValue( + scanExpectation([staleId]) + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([staleId]) + store.getAllWorktreeLineage.mockReturnValue({ + [`${REPO_ID}::/workspace/live-child`]: { + worktreeId: `${REPO_ID}::/workspace/live-child`, + worktreeInstanceId: 'child-instance', + parentWorktreeId: staleId, + parentWorktreeInstanceId: 'stale-instance', + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 0 + } + }) + listWorktreesMock.mockResolvedValue([worktree(REPO_PATH), worktree('/workspace/live')]) + + await listDetected() + + expect(store.pruneSessionlessMissingLocalWorktreeMetadataForRepo).toHaveBeenCalledTimes(1) + expect(store.setWorktreeMeta.mock.calls.some(([worktreeId]) => worktreeId === staleId)).toBe( + false + ) + }) +}) diff --git a/src/main/ipc/worktrees-test-harness.ts b/src/main/ipc/worktrees-test-harness.ts index 0eacf52e472..1888156a692 100644 --- a/src/main/ipc/worktrees-test-harness.ts +++ b/src/main/ipc/worktrees-test-harness.ts @@ -7,6 +7,7 @@ import { resetRetirementCollisionKeyCacheForTests } from '../worktree-name-retir import { resetSshProviderAuthorities } from '../ssh/ssh-provider-authority' import { createWorktreeRuntimeStub, type WorktreeRuntimeStub } from './worktrees-test-runtime-stub' import { handlers, mainWindow, store } from './worktrees-test-ipc-surface' +import { configureMetadataPruningStoreMocks } from './worktrees-test-metadata-pruning-store' import { ORIGINAL_PLATFORM, setPlatform, @@ -129,10 +130,12 @@ export function setupWorktreeHandlers(): WorktreeRuntimeStub { store.getWorktreeMeta, store.getWorktreeMetaForHost, store.getAllWorktreeMeta, + store.captureNativeLocalWorktreeMetadataScanExpectation, store.setWorktreeMeta, store.setWorktreeMetaForHost, store.getProjectHostSetups, store.removeWorktreeMeta, + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo, store.removeWorkspaceSessionStateForWorktree, store.getAllWorktreeLineage, store.removeWorktreeLineage, @@ -187,12 +190,13 @@ export function setupWorktreeHandlers(): WorktreeRuntimeStub { store.getRepo.mockReturnValue({ ...repo, worktreeBaseRef: null }) store.getProjects.mockReturnValue([]) store.getSparsePresets.mockReturnValue([]) - store.getSettings.mockReturnValue({ + const settings = { branchPrefix: 'none', nestWorkspaces: false, refreshLocalBaseRefOnWorktreeCreate: false, workspaceDir: '/workspace' - }) + } + store.getSettings.mockReturnValue(settings) store.getWorktreeMeta.mockReturnValue(undefined) // Host-qualified accessors delegate by default so payload assertions stay on one spy; // host routing itself is covered by worktree-identity-persistence.test.ts. @@ -200,6 +204,7 @@ export function setupWorktreeHandlers(): WorktreeRuntimeStub { store.getWorktreeMeta(args[0] as string) ) store.getAllWorktreeMeta.mockReturnValue({}) + configureMetadataPruningStoreMocks(store, settings) store.getRetiredWorktreeNameRegistry.mockReturnValue({ exhaustedTiers: 0, names: [] }) resetRetirementCollisionKeyCacheForTests() store.setWorktreeMeta.mockReturnValue({}) diff --git a/src/main/ipc/worktrees-test-ipc-surface.ts b/src/main/ipc/worktrees-test-ipc-surface.ts index cc4b497b1bc..a858aacf6b8 100644 --- a/src/main/ipc/worktrees-test-ipc-surface.ts +++ b/src/main/ipc/worktrees-test-ipc-surface.ts @@ -24,10 +24,12 @@ export type TestStore = { getWorktreeMeta: KeyedStoreMock getWorktreeMetaForHost: StoreMock getAllWorktreeMeta: StoreMock + captureNativeLocalWorktreeMetadataScanExpectation: StoreMock setWorktreeMeta: KeyedStoreWriteMock setWorktreeMetaForHost: StoreMock getProjectHostSetups: StoreMock removeWorktreeMeta: KeyedStoreMock + pruneSessionlessMissingLocalWorktreeMetadataForRepo: StoreMock removeWorkspaceSessionStateForWorktree: KeyedStoreMock getAllWorktreeLineage: StoreMock removeWorktreeLineage: KeyedStoreMock @@ -58,10 +60,12 @@ export const store: TestStore = { getWorktreeMeta: vi.fn(), getWorktreeMetaForHost: vi.fn(), getAllWorktreeMeta: vi.fn(), + captureNativeLocalWorktreeMetadataScanExpectation: vi.fn(), setWorktreeMeta: vi.fn(), setWorktreeMetaForHost: vi.fn(), getProjectHostSetups: vi.fn(), removeWorktreeMeta: vi.fn(), + pruneSessionlessMissingLocalWorktreeMetadataForRepo: vi.fn(), removeWorkspaceSessionStateForWorktree: vi.fn(), getAllWorktreeLineage: vi.fn(), removeWorktreeLineage: vi.fn(), diff --git a/src/main/ipc/worktrees-test-metadata-pruning-store.ts b/src/main/ipc/worktrees-test-metadata-pruning-store.ts new file mode 100644 index 00000000000..dd60acaa027 --- /dev/null +++ b/src/main/ipc/worktrees-test-metadata-pruning-store.ts @@ -0,0 +1,35 @@ +import type { TestStore } from './worktrees-test-ipc-surface' + +export function configureMetadataPruningStoreMocks( + store: TestStore, + expectedSettings: unknown +): void { + store.captureNativeLocalWorktreeMetadataScanExpectation.mockImplementation( + (...args: unknown[]) => { + const repo = args[0] as { + id: string + path: string + connectionId?: string | null + executionHostId?: string | null + kind?: string + } + return { + repo: { + id: repo.id, + path: repo.path, + connectionId: repo.connectionId, + executionHostId: repo.executionHostId, + kind: repo.kind === 'folder' ? 'folder' : 'git', + expectedRepo: repo + }, + routing: { + expectedProject: undefined, + expectedProjectUpdatedAt: undefined, + expectedSettings + }, + metadata: [] + } + } + ) + store.pruneSessionlessMissingLocalWorktreeMetadataForRepo.mockReturnValue([]) +} diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index b22d7decd84..3826f5493a8 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -166,6 +166,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { getSettings: vi.fn(), getWorktreeMeta: vi.fn(), getAllWorktreeMeta: vi.fn(), + captureNativeLocalWorktreeMetadataScanExpectation: vi.fn(), setWorktreeMeta: vi.fn(), removeWorktreeMeta: vi.fn(), addRetiredWorktreeName: vi.fn(), @@ -211,6 +212,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { store.getSettings.mockReset() store.getWorktreeMeta.mockReset() store.getAllWorktreeMeta.mockReset() + store.captureNativeLocalWorktreeMetadataScanExpectation.mockReset() store.setWorktreeMeta.mockReset() store.removeWorktreeMeta.mockReset() store.addRetiredWorktreeName.mockReset() @@ -254,6 +256,20 @@ describe('registerWorktreeHandlers – Windows path handling', () => { resolveSetupRunnerShellMock.mockReturnValue(undefined) store.getWorktreeMeta.mockReturnValue(undefined) store.getAllWorktreeMeta.mockReturnValue({}) + store.captureNativeLocalWorktreeMetadataScanExpectation.mockImplementation((repo) => ({ + repo: { + id: repo.id, + path: repo.path, + kind: 'git', + expectedRepo: repo + }, + routing: { + expectedProject: undefined, + expectedProjectUpdatedAt: undefined, + expectedSettings: store.getSettings() + }, + metadata: [] + })) store.getRetiredWorktreeNameRegistry.mockReturnValue({ exhaustedTiers: 0, names: [] }) store.setWorktreeMeta.mockReturnValue({}) resolveLocalGitUsernameMock.mockResolvedValue('') diff --git a/src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts b/src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts new file mode 100644 index 00000000000..c5ceb1acafb --- /dev/null +++ b/src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts @@ -0,0 +1,141 @@ +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' +import { isWindowsAbsolutePathLike } from '../../../../shared/cross-platform-path' +import { isFolderRepo } from '../../../../shared/repo-kind' +import type { Repo } from '../../../../shared/repo-types' +import { + FOLDER_WORKSPACE_INSTANCE_SEPARATOR, + splitWorktreeId +} from '../../../../shared/worktree/id' +import type { GitWorktreeInfo } from '../../../../shared/worktree/types' +import { isWslUncPath } from '../../../../shared/wsl-paths' +import type { Store } from '../../../persistence/loading-store/store' +import type { NativeLocalWorktreeMetadataScanExpectation } from '../../../persistence/tracking-repos/missing-local-worktree-metadata-pruning' +import { pruneWorkspaceCleanupScanSnapshots } from '../../../workspace-cleanup-scan-snapshot' +import { pruneWorkspaceSpaceAnalysisSnapshots } from '../../../workspace-space-analysis-snapshot' +import { isLocalWorktreeScanGenerationCurrent } from '../../../local-worktree-scan-generation' +import { localWorktreePathsExistOrAreUnverifiable } from '../../../local-worktree-path-presence' +import { worktreeRetentionPathComparisonKey } from '../../../worktree-retention-path-comparison' + +type AuthoritativeLocalMetadataPruneArgs = { + store: Store + repo: Repo + gitWorktrees: readonly GitWorktreeInfo[] + scan: NativeLocalWorktreeMetadataScanExpectation + scanGeneration: number + platform?: NodeJS.Platform + pathsExistOrAreUnverifiable?: ( + pathValues: readonly string[], + options?: { signal?: AbortSignal } + ) => Promise> + isCallerCurrent?: () => boolean + signal?: AbortSignal +} + +export type AuthoritativeLocalMetadataPruneResult = Readonly<{ + removedWorktreeIds: readonly string[] + preservedMetadataCandidateIds: ReadonlySet + scanGenerationCurrent: boolean +}> + +export async function pruneMetadataMissingFromAuthoritativeLocalScan({ + store, + repo, + gitWorktrees, + scan, + scanGeneration, + platform = process.platform, + pathsExistOrAreUnverifiable = localWorktreePathsExistOrAreUnverifiable, + isCallerCurrent = () => true, + signal +}: AuthoritativeLocalMetadataPruneArgs): Promise { + const capturedCandidateIds = scan.metadata.map(({ worktreeId }) => worktreeId) + const result = ( + removedWorktreeIds: readonly string[], + scanGenerationCurrent: boolean + ): AuthoritativeLocalMetadataPruneResult => { + const removedIds = new Set(removedWorktreeIds) + return { + removedWorktreeIds, + preservedMetadataCandidateIds: new Set( + capturedCandidateIds.filter((worktreeId) => !removedIds.has(worktreeId)) + ), + scanGenerationCurrent + } + } + const generationCurrent = () => isLocalWorktreeScanGenerationCurrent(repo.id, scanGeneration) + const repoOwners = store.getRepos().filter((candidate) => candidate.id === repo.id) + if ( + !generationCurrent() || + !isCallerCurrent() || + gitWorktrees.length === 0 || + repoOwners.length !== 1 || + repo.id !== scan.repo.id || + repo.path !== scan.repo.path || + repo.connectionId || + isFolderRepo(repo) || + getRepoExecutionHostId(repo) !== LOCAL_EXECUTION_HOST_ID || + (platform === 'win32' && + (isWslUncPath(repo.path) || gitWorktrees.some(({ path }) => isWslUncPath(path)))) + ) { + return result([], generationCurrent()) + } + + const livePathKeys = new Set([ + // Why: Git can canonicalize a symlinked main checkout differently from the configured path. + worktreeRetentionPathComparisonKey(repo.path, platform), + ...gitWorktrees.map((worktree) => worktreeRetentionPathComparisonKey(worktree.path, platform)) + ]) + const probeCandidates = scan.metadata.flatMap((metadata) => { + const { worktreeId } = metadata + const parsed = splitWorktreeId(worktreeId) + const nativeAbsolute = parsed + ? platform === 'win32' + ? isWindowsAbsolutePathLike(parsed.worktreePath) + : parsed.worktreePath.startsWith('/') + : false + if ( + parsed?.repoId !== repo.id || + !nativeAbsolute || + isWslUncPath(parsed.worktreePath) || + worktreeId.includes(FOLDER_WORKSPACE_INSTANCE_SEPARATOR) || + livePathKeys.has(worktreeRetentionPathComparisonKey(parsed.worktreePath, platform)) + ) { + return [] + } + return [{ metadata, pathValue: parsed.worktreePath }] + }) + const presenceByPath = await pathsExistOrAreUnverifiable( + probeCandidates.map(({ pathValue }) => pathValue), + { signal } + ) + if (!generationCurrent() || !isCallerCurrent()) { + return result([], generationCurrent()) + } + const missingMetadata = probeCandidates.flatMap(({ metadata, pathValue }) => + presenceByPath.get(pathValue) === false ? [metadata] : [] + ) + if (missingMetadata.length === 0) { + return result([], generationCurrent()) + } + + if (!generationCurrent() || !isCallerCurrent()) { + return result([], generationCurrent()) + } + const removedIds = store.pruneSessionlessMissingLocalWorktreeMetadataForRepo( + scan, + missingMetadata + ) + if (removedIds.length > 0) { + const snapshotDirectory = store.getProfileStorageDirectory() + const targets: { + worktreeId: string + executionHostId: typeof LOCAL_EXECUTION_HOST_ID + }[] = removedIds.map((worktreeId) => ({ + worktreeId, + executionHostId: LOCAL_EXECUTION_HOST_ID + })) + void pruneWorkspaceCleanupScanSnapshots(snapshotDirectory, targets) + void pruneWorkspaceSpaceAnalysisSnapshots(snapshotDirectory, targets) + } + return result(removedIds, generationCurrent()) +} diff --git a/src/main/ipc/worktrees/listing/detected-provider-listing.ts b/src/main/ipc/worktrees/listing/detected-provider-listing.ts index d855e582161..45ccd4183bb 100644 --- a/src/main/ipc/worktrees/listing/detected-provider-listing.ts +++ b/src/main/ipc/worktrees/listing/detected-provider-listing.ts @@ -4,7 +4,6 @@ import { getSshGitProvider } from '../../../providers/ssh-git-dispatch' import type { DetectedWorktreeListResult, GitWorktreeInfo } from '../../../../shared/worktree/types' import { isFolderRepo } from '../../../../shared/repo-kind' import { projectResolvedWorktreeLineage } from '../../../../shared/resolved-worktree-lineage' -import { pruneLineageForMissingRepoWorktrees } from '../../../worktree-lineage-pruning' import type { DirectSshDetectedWorktreeRequest } from '../../../../shared/detected-worktree-provider-contract' import { isAdmissibleDirectSshAuthority } from '../../../../shared/ssh-retained-payload-admission' import type { ListDesktopLineageForHostArgs } from '../../../../shared/host-lineage-contract' @@ -20,8 +19,10 @@ import { import { isFolderWorkspaceIdForRepo } from '../folder-workspace-model' import { hasConflictingStoredWorktreeOwner } from './worktree-host-ownership' import { + applyFreshDetectedWorktreeScanSideEffects, listDetectedGitWorktrees, - rememberLocalWorktreeRoots + type DetectedWorktreeMetadataPrune, + type DetectedWorktreeSideEffectToken } from './detected-worktree-scan-cache' import { loggedWorktreeListFailures, warnOnce } from './worktree-listing-diagnostics' import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' @@ -48,6 +49,8 @@ export async function listDetectedWorktreesForCapturedRepo( try { let gitWorktrees: GitWorktreeInfo[] let freshScan = true + let sideEffectToken: DetectedWorktreeSideEffectToken | undefined + let metadataPrune: DetectedWorktreeMetadataPrune | undefined if (isFolderRepo(repo)) { if (!isCurrent()) { return null @@ -97,6 +100,8 @@ export async function listDetectedWorktreesForCapturedRepo( const scan = await listDetectedGitWorktrees(store, repo) gitWorktrees = scan.gitWorktrees freshScan = scan.fresh + sideEffectToken = scan.sideEffectToken + metadataPrune = scan.metadataPrune } const aborted = abortedResult() if (aborted) { @@ -115,8 +120,18 @@ export async function listDetectedWorktreesForCapturedRepo( } } if (freshScan) { - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) + await applyFreshDetectedWorktreeScanSideEffects(store, repo, gitWorktrees, metadataPrune, { + isCurrent: () => isCurrent() && !providerAbort?.signal.aborted, + sideEffectToken, + signal: providerAbort?.signal + }) + const aborted = abortedResult() + if (aborted) { + return aborted + } + if (!isCurrent()) { + return null + } } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return { diff --git a/src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts b/src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts index 55b3d75e3ed..fea200f1a8b 100644 --- a/src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts +++ b/src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts @@ -4,7 +4,19 @@ import type { Repo } from '../../../../shared/repo-types' import { getLocalProjectWorktreeGitOptions } from '../../../project-runtime-git-options' import { isFolderRepo } from '../../../../shared/repo-kind' import { listRepoWorktrees } from '../../../repo-worktrees' -import { registerWorktreeRootsForRepo } from '../../registered-worktree-roots-cache' +import { + getRegisteredWorktreeRootsRevision, + registerWorktreeRootsForRepo +} from '../../registered-worktree-roots-cache' +import type { NativeLocalWorktreeMetadataScanExpectation } from '../../../persistence/tracking-repos/missing-local-worktree-metadata-pruning' +import { + bumpLocalWorktreeScanGeneration, + getLocalWorktreeScanGeneration, + isLocalWorktreeScanGenerationCurrent, + resetLocalWorktreeScanGenerationsForTests +} from '../../../local-worktree-scan-generation' +import { pruneLineageForMissingRepoWorktrees } from '../../../worktree-lineage-pruning' +import { pruneMetadataMissingFromAuthoritativeLocalScan } from './authoritative-local-worktree-metadata-pruning' // Why: absorb renderer polling bursts while bounding external worktree-change lag to one short refresh window. export const DETECTED_WORKTREE_SCAN_CACHE_TTL_MS = 5_000 @@ -17,17 +29,31 @@ export type DetectedWorktreeScanCacheEntry = { export type DetectedWorktreeScan = { invalidated: boolean promise: Promise + sideEffectToken: DetectedWorktreeSideEffectToken + metadataPrune?: DetectedWorktreeMetadataPrune } +export type DetectedWorktreeSideEffectToken = Readonly<{ + generation: number + authorizedRootsRevision: number +}> + +export type DetectedWorktreeMetadataPrune = Readonly<{ + expectation: NativeLocalWorktreeMetadataScanExpectation +}> + export type DetectedWorktreeScanResult = { gitWorktrees: GitWorktreeInfo[] fresh: boolean + sideEffectToken?: DetectedWorktreeSideEffectToken + metadataPrune?: DetectedWorktreeMetadataPrune } export const detectedWorktreeScanCache = new Map() export const detectedWorktreeScanInFlight = new Map() export function invalidateDetectedWorktreeScanCache(repoId: string): void { + bumpLocalWorktreeScanGeneration(repoId) const keyPrefix = `${repoId}\0` for (const key of new Set([ ...detectedWorktreeScanCache.keys(), @@ -53,6 +79,7 @@ export function __resetDetectedWorktreeScanCacheForTests(): void { } detectedWorktreeScanCache.clear() detectedWorktreeScanInFlight.clear() + resetLocalWorktreeScanGenerationsForTests() } export function __getDetectedWorktreeScanCacheStatsForTests(): { @@ -88,21 +115,47 @@ export async function listDetectedGitWorktrees( return { gitWorktrees: await inFlight.promise, fresh: false } } + // Why: capture before invoking Git because listing can mutate synchronously before its first await. + // WSL listings can use UNC paths while legacy metadata keeps Linux paths; v1 cannot prove + // those aliases equivalent, so only native-host scans carry destructive expectations. + const generation = getLocalWorktreeScanGeneration(repo.id) + const authorizedRootsRevision = getRegisteredWorktreeRootsRevision(repo.id) + const metadataPruneExpectation = localWorktreeGitOptions.wslDistro + ? undefined + : store.captureNativeLocalWorktreeMetadataScanExpectation(repo) const scan: DetectedWorktreeScan = { invalidated: false, - promise: listRepoWorktrees(repo, localWorktreeGitOptions) + promise: listRepoWorktrees(repo, localWorktreeGitOptions), + sideEffectToken: { generation, authorizedRootsRevision }, + ...(metadataPruneExpectation + ? { + metadataPrune: { + expectation: metadataPruneExpectation + } + } + : {}) } detectedWorktreeScanInFlight.set(cacheKey, scan) try { const gitWorktrees = await scan.promise + const routingUnchanged = + getDetectedWorktreeScanCacheKey(repo.id, getLocalProjectWorktreeGitOptions(store, repo)) === + cacheKey // Why: a create/remove notification can invalidate mid-scan; don't let that stale scan repopulate the cache afterward. - if (!scan.invalidated) { + const generationCurrent = isLocalWorktreeScanGenerationCurrent(repo.id, generation) + if (!scan.invalidated && routingUnchanged && generationCurrent) { detectedWorktreeScanCache.set(cacheKey, { worktrees: gitWorktrees, expiresAt: Date.now() + DETECTED_WORKTREE_SCAN_CACHE_TTL_MS }) } - return { gitWorktrees, fresh: !scan.invalidated } + const fresh = !scan.invalidated && routingUnchanged && generationCurrent + return { + gitWorktrees, + fresh, + ...(fresh ? { sideEffectToken: scan.sideEffectToken } : {}), + ...(fresh && scan.metadataPrune ? { metadataPrune: scan.metadataPrune } : {}) + } } finally { if (detectedWorktreeScanInFlight.get(cacheKey) === scan) { detectedWorktreeScanInFlight.delete(cacheKey) @@ -110,6 +163,63 @@ export async function listDetectedGitWorktrees( } } +export async function applyFreshDetectedWorktreeScanSideEffects( + store: Store, + repo: Repo, + gitWorktrees: GitWorktreeInfo[], + metadataPrune?: DetectedWorktreeMetadataPrune, + options: { + isCurrent?: () => boolean + sideEffectToken?: DetectedWorktreeSideEffectToken + signal?: AbortSignal + } = {} +): Promise { + const { isCurrent = () => true, sideEffectToken, signal } = options + const generationCurrent = () => + sideEffectToken === undefined || + isLocalWorktreeScanGenerationCurrent(repo.id, sideEffectToken.generation) + if (!generationCurrent() || !isCurrent()) { + return false + } + let preservedMetadataCandidateIds: ReadonlySet | undefined + if (metadataPrune) { + if (!sideEffectToken) { + return false + } + const pruneResult = await pruneMetadataMissingFromAuthoritativeLocalScan({ + store, + repo, + gitWorktrees, + scan: metadataPrune.expectation, + scanGeneration: sideEffectToken.generation, + isCallerCurrent: isCurrent, + signal + }) + if (!pruneResult.scanGenerationCurrent || !generationCurrent() || !isCurrent()) { + return false + } + preservedMetadataCandidateIds = pruneResult.preservedMetadataCandidateIds + } + if (!generationCurrent() || !isCurrent()) { + return false + } + + if ( + sideEffectToken && + getRegisteredWorktreeRootsRevision(repo.id) !== sideEffectToken.authorizedRootsRevision + ) { + return false + } + rememberLocalWorktreeRoots(store, repo, gitWorktrees) + pruneLineageForMissingRepoWorktrees( + store, + repo, + gitWorktrees, + preservedMetadataCandidateIds ? { preservedMetadataCandidateIds } : undefined + ) + return true +} + export function getDetectedWorktreeScanCacheKey( repoId: string, localWorktreeGitOptions: { wslDistro?: string } = {} diff --git a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts index ac98130b5eb..4a882ff24b7 100644 --- a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts +++ b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts @@ -2,7 +2,6 @@ import { ipcMain } from 'electron' import { isFolderRepo } from '../../../../shared/repo-kind' import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { getSshGitProvider } from '../../../providers/ssh-git-dispatch' -import { pruneLineageForMissingRepoWorktrees } from '../../../worktree-lineage-pruning' import { EMPTY_RETIRED_NAME_REGISTRY } from '../../../../shared/worktree/retired-name-registry' import { getRetiredNameRegistryForRepo } from '../../../worktree-name-retirement' import { @@ -13,8 +12,10 @@ import { } from './ssh-worktree-fallback' import { listVisibleFolderWorkspaces } from './folder-workspace-catalog' import { + applyFreshDetectedWorktreeScanSideEffects, listDetectedGitWorktrees, - rememberLocalWorktreeRoots + type DetectedWorktreeMetadataPrune, + type DetectedWorktreeSideEffectToken } from './detected-worktree-scan-cache' import { loggedUnavailableSshGitProviders, @@ -88,6 +89,8 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo try { let gitWorktrees let freshScan = true + let sideEffectToken: DetectedWorktreeSideEffectToken | undefined + let metadataPrune: DetectedWorktreeMetadataPrune | undefined if (isFolderRepo(repo)) { return listVisibleFolderWorkspaces(store, repo) } else if (repo.connectionId) { @@ -116,10 +119,19 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo const scan = await listDetectedGitWorktrees(store, repo) gitWorktrees = scan.gitWorktrees freshScan = scan.fresh + sideEffectToken = scan.sideEffectToken + metadataPrune = scan.metadataPrune } if (freshScan) { - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) + await applyFreshDetectedWorktreeScanSideEffects( + store, + repo, + gitWorktrees, + metadataPrune, + { + sideEffectToken + } + ) } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) const metadata = metadataForRepo(repo) @@ -169,6 +181,8 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo try { let gitWorktrees let freshScan = true + let sideEffectToken: DetectedWorktreeSideEffectToken | undefined + let metadataPrune: DetectedWorktreeMetadataPrune | undefined if (isFolderRepo(repo)) { return listVisibleFolderWorkspaces(store, repo) } else if (repo.connectionId) { @@ -197,10 +211,13 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo const scan = await listDetectedGitWorktrees(store, repo) gitWorktrees = scan.gitWorktrees freshScan = scan.fresh + sideEffectToken = scan.sideEffectToken + metadataPrune = scan.metadataPrune } if (freshScan) { - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) + await applyFreshDetectedWorktreeScanSideEffects(store, repo, gitWorktrees, metadataPrune, { + sideEffectToken + }) } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) const metadata = allMeta ?? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) diff --git a/src/main/local-worktree-path-presence.test.ts b/src/main/local-worktree-path-presence.test.ts new file mode 100644 index 00000000000..d54284f7c6b --- /dev/null +++ b/src/main/local-worktree-path-presence.test.ts @@ -0,0 +1,219 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { statMock } = vi.hoisted(() => ({ + statMock: vi.fn() +})) + +vi.mock('node:fs/promises', () => ({ stat: statMock })) + +import { + LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS, + localWorktreePathExistsOrIsUnverifiable, + localWorktreePathsExistOrAreUnverifiable +} from './local-worktree-path-presence' + +function fsError(code: string): NodeJS.ErrnoException { + return Object.assign(new Error(code), { code }) +} + +function installFakeAbortSignalTimeout(): AbortController[] { + const controllers: AbortController[] = [] + vi.spyOn(AbortSignal, 'timeout').mockImplementation((timeoutMs) => { + const controller = new AbortController() + controllers.push(controller) + setTimeout( + () => controller.abort(new DOMException('The operation timed out.', 'TimeoutError')), + timeoutMs + ) + return controller.signal + }) + return controllers +} + +describe('local worktree path presence', () => { + beforeEach(() => { + statMock.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it('returns one result per distinct path and only treats ENOENT/ENOTDIR as missing', async () => { + statMock.mockImplementation(async (pathValue: string) => { + if (pathValue === '/missing') { + throw fsError('ENOENT') + } + if (pathValue === '/not-a-directory') { + throw fsError('ENOTDIR') + } + if (pathValue === '/permission-denied') { + throw fsError('EACCES') + } + return {} + }) + + const result = await localWorktreePathsExistOrAreUnverifiable([ + '/present', + '/missing', + '/not-a-directory', + '/permission-denied', + '/present' + ]) + + expect([...result.entries()]).toEqual([ + ['/present', true], + ['/missing', false], + ['/not-a-directory', false], + ['/permission-denied', true] + ]) + expect(statMock).toHaveBeenCalledTimes(4) + }) + + it('does not allocate a deadline when there are no paths to probe', async () => { + const timeout = vi.spyOn(AbortSignal, 'timeout') + + await expect(localWorktreePathsExistOrAreUnverifiable([])).resolves.toEqual(new Map()) + + expect(timeout).not.toHaveBeenCalled() + expect(statMock).not.toHaveBeenCalled() + }) + + it('globally bounds stat calls across simultaneous bulk probes', async () => { + let active = 0 + let maximumActive = 0 + const pending: (() => void)[] = [] + statMock.mockImplementation( + () => + new Promise((resolve) => { + active += 1 + maximumActive = Math.max(maximumActive, active) + pending.push(() => { + active -= 1 + resolve({}) + }) + }) + ) + + const first = localWorktreePathsExistOrAreUnverifiable([ + '/first-1', + '/first-2', + '/first-3', + '/first-4' + ]) + const second = localWorktreePathsExistOrAreUnverifiable([ + '/second-1', + '/second-2', + '/second-3', + '/second-4' + ]) + + let resolved = 0 + while (resolved < 8) { + while (pending.length === 0) { + await Promise.resolve() + } + pending.shift()?.() + resolved += 1 + await Promise.resolve() + } + + await Promise.all([first, second]) + + expect(maximumActive).toBe(2) + expect(statMock).toHaveBeenCalledTimes(8) + }) + + it('uses the same fail-closed classification for a single probe', async () => { + statMock.mockRejectedValueOnce(fsError('ENOENT')) + statMock.mockRejectedValueOnce(fsError('ENOTDIR')) + statMock.mockRejectedValueOnce(fsError('EIO')) + + await expect(localWorktreePathExistsOrIsUnverifiable('/missing')).resolves.toBe(false) + await expect(localWorktreePathExistsOrIsUnverifiable('/parent-missing')).resolves.toBe(false) + await expect(localWorktreePathExistsOrIsUnverifiable('/unverifiable')).resolves.toBe(true) + }) + + it('settles timed-out batches without exceeding or leaking the global stat bound', async () => { + vi.useFakeTimers() + const timeoutControllers = installFakeAbortSignalTimeout() + const releaseStats: (() => void)[] = [] + let returnMissing = false + statMock.mockImplementation(() => + returnMissing + ? Promise.reject(fsError('ENOENT')) + : new Promise((resolve) => releaseStats.push(() => resolve({}))) + ) + + try { + const firstPaths = ['/first-1', '/first-2', '/first-3', '/first-4'] + const first = localWorktreePathsExistOrAreUnverifiable(firstPaths) + let firstSettled = false + void first.then(() => { + firstSettled = true + }) + await vi.advanceTimersByTimeAsync(0) + + expect(statMock).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS - 1) + expect(firstSettled).toBe(false) + await vi.advanceTimersByTimeAsync(1) + await expect(first).resolves.toEqual( + new Map(firstPaths.map((pathValue) => [pathValue, true])) + ) + expect(statMock).toHaveBeenCalledTimes(2) + + const secondPaths = ['/second-1', '/second-2', '/second-3'] + const second = localWorktreePathsExistOrAreUnverifiable(secondPaths) + await vi.advanceTimersByTimeAsync(LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS) + await expect(second).resolves.toEqual( + new Map(secondPaths.map((pathValue) => [pathValue, true])) + ) + expect(statMock).toHaveBeenCalledTimes(2) + + returnMissing = true + releaseStats.splice(0).forEach((release) => release()) + await vi.advanceTimersByTimeAsync(0) + + await expect(localWorktreePathsExistOrAreUnverifiable(['/after-release'])).resolves.toEqual( + new Map([['/after-release', false]]) + ) + expect(statMock).toHaveBeenCalledTimes(3) + } finally { + timeoutControllers.forEach((controller) => controller.abort()) + releaseStats.splice(0).forEach((release) => release()) + await Promise.resolve() + await Promise.resolve() + } + }) + + it('settles promptly and fail closed when the caller aborts hung probes', async () => { + vi.useFakeTimers() + const timeoutControllers = installFakeAbortSignalTimeout() + const releaseStats: (() => void)[] = [] + statMock.mockImplementation( + () => new Promise((resolve) => releaseStats.push(() => resolve({}))) + ) + const controller = new AbortController() + const paths = ['/abort-1', '/abort-2', '/abort-3'] + + try { + const result = localWorktreePathsExistOrAreUnverifiable(paths, { + signal: controller.signal + }) + await vi.advanceTimersByTimeAsync(0) + expect(statMock).toHaveBeenCalledTimes(2) + + controller.abort() + + await expect(result).resolves.toEqual(new Map(paths.map((pathValue) => [pathValue, true]))) + expect(vi.getTimerCount()).toBeGreaterThan(0) + } finally { + timeoutControllers.forEach((timeoutController) => timeoutController.abort()) + releaseStats.splice(0).forEach((release) => release()) + await Promise.resolve() + await Promise.resolve() + } + }) +}) diff --git a/src/main/local-worktree-path-presence.ts b/src/main/local-worktree-path-presence.ts new file mode 100644 index 00000000000..673305d9513 --- /dev/null +++ b/src/main/local-worktree-path-presence.ts @@ -0,0 +1,72 @@ +import { stat } from 'node:fs/promises' +import { mapWithConcurrency } from '../shared/map-with-concurrency' +import { PrioritySemaphore } from '../shared/priority-semaphore' + +// Why: share one bound across polling callers so slow mounts cannot multiply libuv work. +const LOCAL_WORKTREE_PATH_PROBE_CONCURRENCY = 2 +export const LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS = 2_000 +const localWorktreePathProbeSemaphore = new PrioritySemaphore(LOCAL_WORKTREE_PATH_PROBE_CONCURRENCY) + +function probeDeadline(signal?: AbortSignal): AbortSignal { + const timeout = AbortSignal.timeout(LOCAL_WORKTREE_PATH_PROBE_TIMEOUT_MS) + return signal ? AbortSignal.any([signal, timeout]) : timeout +} + +async function probeLocalWorktreePath(pathValue: string, signal: AbortSignal): Promise { + let release: () => void + try { + release = await localWorktreePathProbeSemaphore.acquire(0, signal) + } catch { + return true + } + if (signal.aborted) { + release() + return true + } + const presence = stat(pathValue) + .then( + () => true, + (error) => { + const code = (error as NodeJS.ErrnoException | undefined)?.code + return code !== 'ENOENT' && code !== 'ENOTDIR' + } + ) + .finally(release) + if (signal.aborted) { + return true + } + return new Promise((resolve) => { + const finish = (value: boolean): void => { + signal.removeEventListener('abort', onAbort) + resolve(value) + } + const onAbort = (): void => finish(true) + signal.addEventListener('abort', onAbort, { once: true }) + void presence.then(finish) + }) +} + +/** A non-absence filesystem error cannot prove a native-local worktree disappeared. */ +export function localWorktreePathExistsOrIsUnverifiable( + pathValue: string, + options: { signal?: AbortSignal } = {} +): Promise { + return probeLocalWorktreePath(pathValue, probeDeadline(options.signal)) +} + +export async function localWorktreePathsExistOrAreUnverifiable( + pathValues: readonly string[], + options: { signal?: AbortSignal } = {} +): Promise> { + const uniquePaths = [...new Set(pathValues)] + if (uniquePaths.length === 0) { + return new Map() + } + const signal = probeDeadline(options.signal) + const results = await mapWithConcurrency( + uniquePaths, + LOCAL_WORKTREE_PATH_PROBE_CONCURRENCY, + async (pathValue) => [pathValue, await probeLocalWorktreePath(pathValue, signal)] as const + ) + return new Map(results) +} diff --git a/src/main/local-worktree-scan-generation.ts b/src/main/local-worktree-scan-generation.ts new file mode 100644 index 00000000000..c8cc3bd0ff9 --- /dev/null +++ b/src/main/local-worktree-scan-generation.ts @@ -0,0 +1,25 @@ +const generationByRepoId = new Map() +let generationSequence = 0 + +export function getLocalWorktreeScanGeneration(repoId: string): number { + const existing = generationByRepoId.get(repoId) + if (existing !== undefined) { + return existing + } + const generation = ++generationSequence + generationByRepoId.set(repoId, generation) + return generation +} + +export function bumpLocalWorktreeScanGeneration(repoId: string): void { + generationByRepoId.set(repoId, ++generationSequence) +} + +export function isLocalWorktreeScanGenerationCurrent(repoId: string, generation: number): boolean { + return getLocalWorktreeScanGeneration(repoId) === generation +} + +export function resetLocalWorktreeScanGenerationsForTests(): void { + generationSequence += 1 + generationByRepoId.clear() +} diff --git a/src/main/persistence-initial-load.test.ts b/src/main/persistence-initial-load.test.ts index e1926a67877..6d0c0f11d5e 100644 --- a/src/main/persistence-initial-load.test.ts +++ b/src/main/persistence-initial-load.test.ts @@ -16,6 +16,10 @@ import { makeProjectHostSetup } from './persistence-test-harness' import { TEST_LEAF_1 } from './persistence-session-fixtures' +import { + getLocalWorktreeScanGeneration, + isLocalWorktreeScanGenerationCurrent +} from './local-worktree-scan-generation' // Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config. const { loadUserSshConfigMock, sshConfigHostsToTargetsMock } = vi.hoisted(() => ({ @@ -304,12 +308,13 @@ describe('Store', () => { it('updates and persists a project Windows runtime preference', async () => { const project = makeProject({ - id: 'project-1', + id: 'repo:r1', sourceRepoIds: ['r1'], localWindowsRuntimePreference: { kind: 'inherit-global' } }) writeDataFile({ ...getDefaultPersistedState(testState.dir), + repos: [makeRepo({ id: 'r1' })], projects: [project], projectHostSetups: [ makeProjectHostSetup({ @@ -320,12 +325,14 @@ describe('Store', () => { ] }) const store = await createStore() + const scanGeneration = getLocalWorktreeScanGeneration('r1') - const updated = store.updateProject('project-1', { + const updated = store.updateProject(project.id, { localWindowsRuntimePreference: { kind: 'wsl', distro: 'Ubuntu' } }) expect(updated?.localWindowsRuntimePreference).toEqual({ kind: 'wsl', distro: 'Ubuntu' }) + expect(isLocalWorktreeScanGenerationCurrent('r1', scanGeneration)).toBe(false) store.flush() const reloaded = await createStore() expect(reloaded.getProjects()[0]?.localWindowsRuntimePreference).toEqual({ diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index 53b759a7d01..1653f78297e 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -14,6 +14,10 @@ import { makeTerminalTab, makeWorktreeLineage } from './persistence-test-harness' +import { + getLocalWorktreeScanGeneration, + isLocalWorktreeScanGenerationCurrent +} from './local-worktree-scan-generation' // Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config. const { loadUserSshConfigMock, sshConfigHostsToTargetsMock } = vi.hoisted(() => ({ @@ -79,6 +83,23 @@ describe('Store', () => { expect(fetched!.gitUsername).toBe('') }) + it('invalidates local scans across add, remove, and same-id re-add', async () => { + const store = await createStore() + const repoId = 'scan-lifecycle' + const beforeAdd = getLocalWorktreeScanGeneration(repoId) + + store.addRepo(makeRepo({ id: repoId })) + expect(isLocalWorktreeScanGenerationCurrent(repoId, beforeAdd)).toBe(false) + + const beforeRemove = getLocalWorktreeScanGeneration(repoId) + store.removeProject(repoId) + expect(isLocalWorktreeScanGenerationCurrent(repoId, beforeRemove)).toBe(false) + + const beforeReAdd = getLocalWorktreeScanGeneration(repoId) + store.addRepo(makeRepo({ id: repoId, path: '/replacement' })) + expect(isLocalWorktreeScanGenerationCurrent(repoId, beforeReAdd)).toBe(false) + }) + it('setResolvedRepoGitUsername persists the enriched username for hydration', async () => { const store = await createStore() store.addRepo(makeRepo()) @@ -369,6 +390,24 @@ describe('Store', () => { // ── 6b. removeProjectForHost is host-scoped ─────────────────────────── + it('invalidates local scans when one host registration is removed', async () => { + const store = await createStore() + store.addRepo(makeRepo({ id: 'shared', path: '/local/repo' })) + store.addRepo( + makeRepo({ + id: 'shared', + path: '/remote/repo', + connectionId: 'ssh-old', + executionHostId: 'ssh:ssh-old' + }) + ) + const beforeRemove = getLocalWorktreeScanGeneration('shared') + + store.removeProjectForHost('shared', 'ssh:ssh-old') + + expect(isLocalWorktreeScanGenerationCurrent('shared', beforeRemove)).toBe(false) + }) + it('removeProjectForHost removes only the target host row for a shared repo id', async () => { const store = await createStore() // Same repo id on both local and an SSH host. diff --git a/src/main/persistence-settings-update.test.ts b/src/main/persistence-settings-update.test.ts index 330386a48a2..dc3a3c7ebb5 100644 --- a/src/main/persistence-settings-update.test.ts +++ b/src/main/persistence-settings-update.test.ts @@ -14,6 +14,10 @@ import { makeRepo, makeTerminalTab } from './persistence-test-harness' +import { + getLocalWorktreeScanGeneration, + isLocalWorktreeScanGenerationCurrent +} from './local-worktree-scan-generation' // Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config. const { loadUserSshConfigMock, sshConfigHostsToTargetsMock } = vi.hoisted(() => ({ @@ -227,6 +231,21 @@ describe('Store', () => { expect(listener).not.toHaveBeenCalled() }) + it('invalidates local worktree scans when the global Windows runtime changes', async () => { + writeDataFile({ repos: [makeRepo()] }) + const store = await createStore() + const generation = getLocalWorktreeScanGeneration('r1') + + store.updateSettings({ theme: 'dark' }) + expect(isLocalWorktreeScanGenerationCurrent('r1', generation)).toBe(true) + + store.updateSettings({ + localWindowsRuntimeDefault: { kind: 'wsl', distro: 'Ubuntu' } + }) + + expect(isLocalWorktreeScanGenerationCurrent('r1', generation)).toBe(false) + }) + it('migrates missing terminal scrollback rows to the row default and writes back rows only', async () => { writeDataFile({ settings: {} }) diff --git a/src/main/persistence-update-repo.test.ts b/src/main/persistence-update-repo.test.ts index d2266103615..a89378d6c71 100644 --- a/src/main/persistence-update-repo.test.ts +++ b/src/main/persistence-update-repo.test.ts @@ -11,6 +11,10 @@ import { makeProject, makeProjectHostSetup } from './persistence-test-harness' +import { + getLocalWorktreeScanGeneration, + isLocalWorktreeScanGenerationCurrent +} from './local-worktree-scan-generation' // Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config. const { loadUserSshConfigMock, sshConfigHostsToTargetsMock } = vi.hoisted(() => ({ @@ -75,6 +79,25 @@ describe('Store', () => { expect(store.getRepo('r1')!.displayName).toBe('renamed') }) + it('invalidates local worktree scans across a git-folder-git update round trip', async () => { + const store = await createStore() + store.addRepo(makeRepo({ id: 'scan-round-trip', kind: 'git' })) + + const initialGeneration = getLocalWorktreeScanGeneration('scan-round-trip') + expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', initialGeneration)).toBe(true) + + const folder = store.updateRepo('scan-round-trip', { kind: 'folder' }) + expect(folder?.kind).toBe('folder') + expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', initialGeneration)).toBe(false) + + const folderGeneration = getLocalWorktreeScanGeneration('scan-round-trip') + expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', folderGeneration)).toBe(true) + + const git = store.updateRepo('scan-round-trip', { kind: 'git' }) + expect(git?.kind).toBe('git') + expect(isLocalWorktreeScanGenerationCurrent('scan-round-trip', folderGeneration)).toBe(false) + }) + it('updateRepo targets one host when repo ids collide', async () => { const store = await createStore() store.addRepo(makeRepo({ id: 'shared', path: '/local/repo' })) diff --git a/src/main/persistence/applying-settings/settings-update.ts b/src/main/persistence/applying-settings/settings-update.ts index c4ded8da753..20614061bdd 100644 --- a/src/main/persistence/applying-settings/settings-update.ts +++ b/src/main/persistence/applying-settings/settings-update.ts @@ -41,6 +41,7 @@ import { export type SettingsMutationOperations = { state: PersistedState + bumpLocalWorktreeScanGeneration: (repoId: string) => void removeRetainedBlob: ( slot: Parameters[0] ) => void @@ -242,6 +243,16 @@ export function updateSettings( }), ...(mergedTelemetry !== undefined ? { telemetry: mergedTelemetry } : {}) } + if ( + !Object.is( + previousSettings.localWindowsRuntimeDefault, + operations.state.settings.localWindowsRuntimeDefault + ) + ) { + for (const repoId of new Set(operations.state.repos.map(({ id }) => id))) { + operations.bumpLocalWorktreeScanGeneration(repoId) + } + } operations.scheduleSave() const changedUpdates = {} as Partial & Record for (const key of Object.keys(sanitizedUpdates) as (keyof GlobalSettings)[]) { diff --git a/src/main/persistence/loading-store/metadata-lineage-batch-pruning.test.ts b/src/main/persistence/loading-store/metadata-lineage-batch-pruning.test.ts new file mode 100644 index 00000000000..9e7b71734b6 --- /dev/null +++ b/src/main/persistence/loading-store/metadata-lineage-batch-pruning.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultPersistedState } from '../../../shared/constants' +import type { Repo } from '../../../shared/repo-types' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import type * as WriteSchedulingModule from './write-scheduling' +import { MetadataLineageOperations } from './metadata-lineage-operations' + +const { scheduleSaveMock } = vi.hoisted(() => ({ scheduleSaveMock: vi.fn() })) + +vi.mock('./write-scheduling', async (importOriginal) => ({ + ...(await importOriginal()), + scheduleSave: scheduleSaveMock +})) + +const REPO: Repo = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 +} + +function makeMeta(worktreeId: string): WorktreeMeta { + return { + instanceId: `instance-${worktreeId}`, + hostId: 'local', + displayName: worktreeId, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } +} + +describe('MetadataLineageOperations batch metadata pruning', () => { + it('schedules one save for thousands of removals and none for a no-op retry', () => { + const state = getDefaultPersistedState('/home/test') + state.repos = [REPO] + const staleIds = Array.from( + { length: 2_709 }, + (_, index) => `${REPO.id}::/workspace/stale-${index}` + ) + for (const worktreeId of staleIds) { + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + } + const operations = new MetadataLineageOperations({ state } as never, {} as never, {} as never) + const scan = operations.captureNativeLocalWorktreeMetadataScanExpectation(REPO) + + expect( + operations.pruneSessionlessMissingLocalWorktreeMetadataForRepo(scan, scan.metadata) + ).toHaveLength(staleIds.length) + expect(scheduleSaveMock).toHaveBeenCalledTimes(1) + + expect( + operations.pruneSessionlessMissingLocalWorktreeMetadataForRepo(scan, scan.metadata) + ).toEqual([]) + expect(scheduleSaveMock).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/persistence/loading-store/metadata-lineage-operations.ts b/src/main/persistence/loading-store/metadata-lineage-operations.ts index f12af5d1583..7d4867ce476 100644 --- a/src/main/persistence/loading-store/metadata-lineage-operations.ts +++ b/src/main/persistence/loading-store/metadata-lineage-operations.ts @@ -1,6 +1,7 @@ import type { WorkspaceKey } from '../../../shared/folder-workspace-types' import type { WorkspaceLineage, WorktreeLineage } from '../../../shared/worktree/lineage-types' import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import type { Repo } from '../../../shared/repo-types' import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import { hasWorktreeRemovalRepoOwnerOnOtherHost } from '../../worktree-removal-repo-owner' @@ -28,6 +29,12 @@ import { setWorktreeMetaForHost as setWorktreeMetaForHostOperation } from './worktree-identity-metadata' import { mergeWorktreeMetaForWrite } from './worktree-meta-write-normalization' +import { + captureNativeLocalWorktreeMetadataScanExpectation as captureNativeLocalWorktreeMetadataScanExpectationOperation, + pruneSessionlessMissingLocalWorktreeMetadataForRepo as pruneSessionlessMissingLocalWorktreeMetadataForRepoOperation, + type LocalWorktreeMetadataPruneExpectation, + type NativeLocalWorktreeMetadataScanExpectation +} from '../tracking-repos/missing-local-worktree-metadata-pruning' type MetadataLineageOperationsRuntime = Pick @@ -89,6 +96,15 @@ export class MetadataLineageOperations { ) } + captureNativeLocalWorktreeMetadataScanExpectation( + repo: Repo + ): NativeLocalWorktreeMetadataScanExpectation { + return captureNativeLocalWorktreeMetadataScanExpectationOperation( + this[metadataLineageOperationsContext].runtime.state, + repo + ) + } + setWorktreeMeta(worktreeId: string, meta: Partial): WorktreeMeta { const state = this[metadataLineageOperationsContext].runtime.state const stored = state.worktreeMeta[worktreeId] @@ -184,6 +200,21 @@ export class MetadataLineageOperations { scheduleSave(this[metadataLineageOperationsContext].scheduling) } + pruneSessionlessMissingLocalWorktreeMetadataForRepo( + scan: NativeLocalWorktreeMetadataScanExpectation, + missingMetadata: readonly LocalWorktreeMetadataPruneExpectation[] + ): string[] { + const removed = pruneSessionlessMissingLocalWorktreeMetadataForRepoOperation( + this[metadataLineageOperationsContext].runtime.state, + scan, + missingMetadata + ) + if (removed.length > 0) { + scheduleSave(this[metadataLineageOperationsContext].scheduling) + } + return removed + } + getWorktreeLineage(worktreeId: string): WorktreeLineage | undefined { return this[metadataLineageOperationsContext].runtime.state.worktreeLineageById[worktreeId] } diff --git a/src/main/persistence/loading-store/profile-preferences.ts b/src/main/persistence/loading-store/profile-preferences.ts index 228229d2584..0ec4e383c34 100644 --- a/src/main/persistence/loading-store/profile-preferences.ts +++ b/src/main/persistence/loading-store/profile-preferences.ts @@ -17,6 +17,7 @@ import { import type { StoreRuntimeState } from './store-runtime-state' import type { WriteSchedulingOperations } from './write-scheduling' import { scheduleSave } from './write-scheduling' +import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation' type ProfilePreferencesRuntime = Pick< StoreRuntimeState, @@ -155,6 +156,7 @@ export function getSettingsMutationOperations( ): SettingsMutationOperations { return { state: owner[profilePreferencesContext].runtime.state, + bumpLocalWorktreeScanGeneration, removeRetainedBlob: (slot) => owner[profilePreferencesContext].runtime.protectedSecrets.removeRetainedBlob(slot), scheduleSave: () => scheduleSave(owner[profilePreferencesContext].scheduling), diff --git a/src/main/persistence/loading-store/project-collection-operations.ts b/src/main/persistence/loading-store/project-collection-operations.ts index e7100478037..ecb1130072f 100644 --- a/src/main/persistence/loading-store/project-collection-operations.ts +++ b/src/main/persistence/loading-store/project-collection-operations.ts @@ -28,6 +28,7 @@ import { } from './repo-lifecycle-operations' import { scheduleSave } from './write-scheduling' import { removeWorkspaceLineageForFolderParent } from './metadata-lineage-operations' +import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation' type ProjectCollectionOperationsRuntime = Pick< StoreRuntimeState, @@ -159,6 +160,7 @@ export function getProjectHostOperations( new ProjectHostPersistenceOperations({ state: owner[projectCollectionOperationsContext].runtime.state, gitUsernameCache: owner[projectCollectionOperationsContext].runtime.gitUsernameCache, + bumpLocalWorktreeScanGeneration, hydrateRepo: (repo) => hydrateRepo(owner[projectCollectionOperationsContext].repos, repo), updateRepoBackedProjectHostSetup: (setup, repo, updates) => updateRepoBackedProjectHostSetup( diff --git a/src/main/persistence/loading-store/repo-lifecycle-operations.ts b/src/main/persistence/loading-store/repo-lifecycle-operations.ts index bdd3718d9a6..095090ee1ca 100644 --- a/src/main/persistence/loading-store/repo-lifecycle-operations.ts +++ b/src/main/persistence/loading-store/repo-lifecycle-operations.ts @@ -15,6 +15,7 @@ import { pruneWorktreeStateForRepo as pruneWorktreeStateForRepoOperation } from import { hydrateRepo as hydrateRepoOperation } from '../tracking-repos/repo-hydration' import { RepoUpdatePersistenceOperations } from '../tracking-repos/repo-update-operations' import { ProjectHostSetupPersistenceOperations } from '../tracking-repos/project-host-setup-update' +import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation' import type { StoreRuntimeState } from './store-runtime-state' import type { WriteSchedulingOperations } from './write-scheduling' @@ -43,6 +44,7 @@ export class RepoLifecycleOperations { } addRepo(repo: Repo): void { + bumpLocalWorktreeScanGeneration(repo.id) getRepoOrderOperations(this).addRepo(repo) } @@ -55,9 +57,15 @@ export class RepoLifecycleOperations { } removeProject(id: string): void { + const repoRemoved = this[repoLifecycleOperationsContext].runtime.state.repos.some( + (repo) => repo.id === id + ) this[repoLifecycleOperationsContext].runtime.state.repos = this[ repoLifecycleOperationsContext ].runtime.state.repos.filter((r) => r.id !== id) + if (repoRemoved) { + bumpLocalWorktreeScanGeneration(id) + } syncProjectHostSetupCompatibilityState(this) // Why: presets are repo-scoped and unreachable once the repo is gone, so drop them with it. delete this[repoLifecycleOperationsContext].runtime.state.sparsePresetsByRepo[id] @@ -77,9 +85,15 @@ export class RepoLifecycleOperations { } removeProjectForHost(id: string, hostId: ExecutionHostId): void { + const repoRemoved = this[repoLifecycleOperationsContext].runtime.state.repos.some( + (repo) => repo.id === id && getRepoExecutionHostId(repo) === hostId + ) this[repoLifecycleOperationsContext].runtime.state.repos = this[ repoLifecycleOperationsContext ].runtime.state.repos.filter((r) => !(r.id === id && getRepoExecutionHostId(r) === hostId)) + if (repoRemoved) { + bumpLocalWorktreeScanGeneration(id) + } const idStillPresent = this[repoLifecycleOperationsContext].runtime.state.repos.some( (r) => r.id === id ) @@ -204,6 +218,7 @@ export function getRepoUpdateOperations( owner[repoLifecycleOperationsContext].runtime.repoUpdateOperations ??= new RepoUpdatePersistenceOperations({ state: owner[repoLifecycleOperationsContext].runtime.state, + bumpLocalWorktreeScanGeneration, syncProjectHostSetupCompatibilityState: () => syncProjectHostSetupCompatibilityState(owner), scheduleSave: () => scheduleSave(owner[repoLifecycleOperationsContext].scheduling), hydrateRepo: (repo) => hydrateRepo(owner, repo) diff --git a/src/main/persistence/loading-store/worktree-identity-metadata.ts b/src/main/persistence/loading-store/worktree-identity-metadata.ts index 396c2ffae85..21fea4ac15c 100644 --- a/src/main/persistence/loading-store/worktree-identity-metadata.ts +++ b/src/main/persistence/loading-store/worktree-identity-metadata.ts @@ -36,11 +36,18 @@ function resolveAliasIdentityKey(state: PersistedState, alias: string): string | } /** Drop identity metadata no alias points at any more. */ -export function pruneUnreferencedWorktreeIdentityMeta(state: PersistedState): boolean { +export function pruneUnreferencedWorktreeIdentityMeta( + state: PersistedState, + candidates?: ReadonlySet +): boolean { const referenced = new Set(Object.values(state.worktreeIdentityAliases ?? {}).flat()) let changed = false - for (const identityKey of Object.keys(state.worktreeMetaByIdentity ?? {})) { - if (!referenced.has(identityKey)) { + const identityKeys = candidates ?? Object.keys(state.worktreeMetaByIdentity ?? {}) + for (const identityKey of identityKeys) { + if ( + Object.hasOwn(state.worktreeMetaByIdentity ?? {}, identityKey) && + !referenced.has(identityKey) + ) { delete state.worktreeMetaByIdentity?.[identityKey] changed = true } diff --git a/src/main/persistence/restoring-sessions/session-worktree-ownership.test.ts b/src/main/persistence/restoring-sessions/session-worktree-ownership.test.ts new file mode 100644 index 00000000000..1e84cf4c506 --- /dev/null +++ b/src/main/persistence/restoring-sessions/session-worktree-ownership.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultPersistedState, getDefaultWorkspaceSession } from '../../../shared/constants' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import { + collectPersistedSessionWorktreeOwners, + collectWorkspaceSessionWorktreeOwners +} from './session-worktree-ownership' + +const TARGET = 'repo-1::/workspace/target' +const OTHER = 'repo-1::/workspace/other' + +function sessionWith(field: keyof WorkspaceSessionState, value: unknown): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), [field]: value } as WorkspaceSessionState +} + +function collected(session: WorkspaceSessionState): string[] { + return [...collectWorkspaceSessionWorktreeOwners(session, new Set([TARGET]))] +} + +const OWNER_KEYED_FIELD_VALUES = [ + ['tabsByWorktree', []], + ['openFilesByWorktree', []], + ['activeFileIdByWorktree', null], + ['browserTabsByWorktree', []], + ['activeBrowserTabIdByWorktree', null], + ['clientHostedBrowserPagesByWorktree', []], + ['activeTabTypeByWorktree', 'terminal'], + ['activeTabIdByWorktree', null], + ['unifiedTabs', []], + ['tabGroups', []], + ['tabGroupLayouts', { type: 'leaf', groupId: 'group-1' }], + ['activeGroupIdByWorktree', 'group-1'], + ['lastVisitedAtByWorktreeId', 0], + ['defaultTerminalTabsAppliedByWorktreeId', true] +] as const satisfies readonly [keyof WorkspaceSessionState, unknown][] + +describe('workspace session worktree ownership', () => { + it.each(OWNER_KEYED_FIELD_VALUES)( + 'treats a key in %s as ownership even when its bucket is empty or null', + (field, value) => { + expect(collected(sessionWith(field, { [TARGET]: value }))).toEqual([TARGET]) + } + ) + + it.each([ + ['active worktree', sessionWith('activeWorktreeId', TARGET)], + ['active workspace', sessionWith('activeWorkspaceKey', worktreeWorkspaceKey(TARGET))], + ['shutdown owner', sessionWith('activeWorktreeIdsOnShutdown', [TARGET])], + [ + 'host-qualified recency owner', + sessionWith('lastVisitedAtByWorktreeId', { [`ssh:builder|${TARGET}`]: 1 }) + ] + ])('collects the %s', (_label, session) => { + expect(collected(session)).toEqual([TARGET]) + }) + + it.each([ + ['terminal tab', 'tabsByWorktree', { [OTHER]: [{ worktreeId: TARGET }] }], + ['open file', 'openFilesByWorktree', { [OTHER]: [{ worktreeId: TARGET }] }], + ['unified tab', 'unifiedTabs', { [OTHER]: [{ worktreeId: TARGET }] }], + ['tab group', 'tabGroups', { [OTHER]: [{ worktreeId: TARGET }] }], + ['browser workspace', 'browserTabsByWorktree', { [OTHER]: [{ worktreeId: TARGET }] }], + [ + 'browser workspace document', + 'browserTabsByWorktree', + { [OTHER]: [{ worktreeId: OTHER, docLocation: { worktreeId: TARGET } }] } + ], + ['browser page', 'browserPagesByWorkspace', { page: [{ worktreeId: TARGET }] }], + [ + 'browser page document', + 'browserPagesByWorkspace', + { page: [{ worktreeId: OTHER, docLocation: { worktreeId: TARGET } }] } + ], + [ + 'browser close intent', + 'clientHostedBrowserCloseIntentsByEnvironment', + { environment: [{ worktreeId: TARGET }] } + ], + ['sleeping agent', 'sleepingAgentSessionsByPaneKey', { pane: { worktreeId: TARGET } }], + ['terminal tombstone', 'terminalSurfaceTombstonesByPaneKey', { pane: { worktreeId: TARGET } }], + [ + 'closed-terminal tombstone', + 'closedTerminalTabTombstonesByTabId', + { tab: { worktreeId: TARGET } } + ] + ] as const)( + 'collects a %s value even when its enclosing key names something else', + (_label, field, value) => { + expect(collected(sessionWith(field, value))).toEqual([TARGET]) + } + ) + + it('collects ownership from every persisted host partition', () => { + const state = getDefaultPersistedState('/home/test') + state.workspaceSessionsByHostId = { + 'ssh:builder': sessionWith('tabsByWorktree', { [TARGET]: [] }), + 'runtime:environment': sessionWith('activeWorktreeIdsOnShutdown', [OTHER]) + } + + expect( + [...collectPersistedSessionWorktreeOwners(state, new Set([TARGET, OTHER]))].sort() + ).toEqual([OTHER, TARGET]) + }) + + it('prefers an exact candidate id over workspace-key prefix parsing', () => { + const prefixedId = 'folder::/workspace/target' + const session = sessionWith('tabsByWorktree', { [prefixedId]: [] }) + + expect([...collectWorkspaceSessionWorktreeOwners(session, new Set([prefixedId]))]).toEqual([ + prefixedId + ]) + }) + + it('matches canonically equivalent Unicode worktree owners', () => { + const candidateId = 'repo-1::/workspace/Café'.normalize('NFC') + const ownerId = candidateId.normalize('NFD') + const session = sessionWith('activeWorktreeId', ownerId) + + expect([...collectWorkspaceSessionWorktreeOwners(session, new Set([candidateId]))]).toEqual([ + candidateId + ]) + }) + + it('matches Windows worktree owners across drive case and slash spelling', () => { + const candidateId = 'repo-1::D:/Agentic/game2' + const session = sessionWith('activeWorktreeId', 'repo-1::d:\\agentic\\game2') + + expect([...collectWorkspaceSessionWorktreeOwners(session, new Set([candidateId]))]).toEqual([ + candidateId + ]) + }) + + it('matches macOS /private/tmp session owners to /tmp metadata', () => { + const candidateId = 'repo-1::/private/tmp/orca/target' + const session = sessionWith('activeWorktreeId', 'repo-1::/tmp/orca/target') + + expect([ + ...collectWorkspaceSessionWorktreeOwners(session, new Set([candidateId]), 'darwin') + ]).toEqual([candidateId]) + }) + + it('preserves every candidate that shares an owner comparison key', () => { + const nfcId = 'repo-1::/workspace/Café'.normalize('NFC') + const nfdId = nfcId.normalize('NFD') + const session = sessionWith('activeWorktreeId', nfcId) + + expect( + [...collectWorkspaceSessionWorktreeOwners(session, new Set([nfcId, nfdId]))].sort() + ).toEqual([nfcId, nfdId].sort()) + }) + + it('ignores fields whose keys cannot identify a worktree owner', () => { + const session = { + ...getDefaultWorkspaceSession(), + activeRepoId: TARGET, + activeTabId: TARGET, + terminalLayoutsByTabId: { [TARGET]: { root: null } }, + remoteSessionIdsByTabId: { [TARGET]: TARGET }, + terminalPtyIncarnationsByPaneKey: { [TARGET]: TARGET }, + terminalTopologyRevisionByRepoId: { [TARGET]: 1 } + } as unknown as WorkspaceSessionState + + expect(collected(session)).toEqual([]) + }) +}) diff --git a/src/main/persistence/restoring-sessions/session-worktree-ownership.ts b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts new file mode 100644 index 00000000000..dc572e79278 --- /dev/null +++ b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts @@ -0,0 +1,246 @@ +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { parseWorkspaceKey } from '../../../shared/workspace-scope' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' +import { worktreeRetentionIdComparisonKey } from '../../worktree-retention-path-comparison' + +type WorkspaceSessionWorktreeReferenceKind = + | 'none' + | 'direct' + | 'owner-keyed' + | 'owner-keyed-row-arrays' + | 'owner-keyed-browser-row-arrays' + | 'worktree-id-array' + | 'row-record' + | 'row-arrays' + | 'browser-row-arrays' + +/** Every persisted session field must state how, if at all, it can name a worktree owner. */ +export const WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND = { + activeRepoId: 'none', + activeWorkspaceKey: 'direct', + activeWorkspaceExecutionHostId: 'none', + activeWorktreeId: 'direct', + activeTabId: 'none', + tabsByWorktree: 'owner-keyed-row-arrays', + terminalLayoutsByTabId: 'none', + activeWorktreeIdsOnShutdown: 'worktree-id-array', + openFilesByWorktree: 'owner-keyed-row-arrays', + activeFileIdByWorktree: 'owner-keyed', + markdownFrontmatterVisible: 'none', + browserTabsByWorktree: 'owner-keyed-browser-row-arrays', + browserPagesByWorkspace: 'browser-row-arrays', + activeBrowserTabIdByWorktree: 'owner-keyed', + clientHostedBrowserPagesByWorktree: 'owner-keyed', + clientHostedBrowserCloseIntentsByEnvironment: 'row-arrays', + activeTabTypeByWorktree: 'owner-keyed', + browserUrlHistory: 'none', + activeTabIdByWorktree: 'owner-keyed', + unifiedTabs: 'owner-keyed-row-arrays', + tabGroups: 'owner-keyed-row-arrays', + tabGroupLayouts: 'owner-keyed', + activeGroupIdByWorktree: 'owner-keyed', + activeConnectionIdsAtShutdown: 'none', + remoteSessionIdsByTabId: 'none', + lastVisitedAtByWorktreeId: 'owner-keyed', + defaultTerminalTabsAppliedByWorktreeId: 'owner-keyed', + sleepingAgentSessionsByPaneKey: 'row-record', + terminalPtyIncarnationsByPaneKey: 'none', + terminalTopologyRevisionByRepoId: 'none', + terminalSurfaceTombstonesByPaneKey: 'row-record', + closedTerminalTabTombstonesByTabId: 'row-record' +} as const satisfies Record + +type WorktreeRow = { worktreeId?: string | null } +type BrowserWorktreeRow = WorktreeRow & { + docLocation?: { worktreeId?: string | null } | null +} + +export type WorktreeOwnerCandidateCollector = Readonly<{ + owners: Set + addOwner: (ownerKey: string | null | undefined) => void +}> + +export function getPersistedWorktreeOwnerId(ownerKey: string | null | undefined): string | null { + if (!ownerKey) { + return null + } + if (isWorktreeHostIdentity(ownerKey)) { + return getWorktreeIdFromHostIdentity(ownerKey) || null + } + const scope = parseWorkspaceKey(ownerKey) + if (scope?.type === 'folder') { + return null + } + return scope?.type === 'worktree' ? scope.worktreeId : ownerKey +} + +export function createWorktreeOwnerCandidateCollector( + candidateIds: ReadonlySet, + platform: NodeJS.Platform = process.platform, + owners: Set = new Set() +): WorktreeOwnerCandidateCollector { + const candidateIdsByComparisonKey = new Map() + for (const candidateId of candidateIds) { + const comparisonKey = worktreeRetentionIdComparisonKey(candidateId, platform) + if (!comparisonKey) { + continue + } + const matches = candidateIdsByComparisonKey.get(comparisonKey) ?? [] + matches.push(candidateId) + candidateIdsByComparisonKey.set(comparisonKey, matches) + } + return { + owners, + addOwner: (ownerKey) => { + const worktreeId = + ownerKey && candidateIds.has(ownerKey) ? ownerKey : getPersistedWorktreeOwnerId(ownerKey) + if (!worktreeId) { + return + } + const comparisonKey = worktreeRetentionIdComparisonKey(worktreeId, platform) + const equivalentCandidates = comparisonKey + ? candidateIdsByComparisonKey.get(comparisonKey) + : undefined + if (equivalentCandidates) { + for (const candidateId of equivalentCandidates) { + owners.add(candidateId) + } + } else if (candidateIds.has(worktreeId)) { + owners.add(worktreeId) + } + } + } +} + +function collectRecordKeys( + value: unknown, + add: (ownerKey: string | null | undefined) => void +): void { + for (const ownerKey of Object.keys((value ?? {}) as Record)) { + add(ownerKey) + } +} + +function collectWorktreeRows( + rows: unknown, + add: (ownerKey: string | null | undefined) => void +): void { + if (!Array.isArray(rows)) { + return + } + for (const row of rows as WorktreeRow[]) { + add(row?.worktreeId) + } +} + +function collectBrowserRows( + rows: unknown, + add: (ownerKey: string | null | undefined) => void +): void { + if (!Array.isArray(rows)) { + return + } + for (const row of rows as BrowserWorktreeRow[]) { + add(row?.worktreeId) + add(row?.docLocation?.worktreeId) + } +} + +function collectSessionFieldOwners( + value: unknown, + kind: WorkspaceSessionWorktreeReferenceKind, + add: (ownerKey: string | null | undefined) => void +): void { + switch (kind) { + case 'none': + return + case 'direct': + add(typeof value === 'string' ? value : null) + return + case 'owner-keyed': + collectRecordKeys(value, add) + return + case 'owner-keyed-row-arrays': + case 'owner-keyed-browser-row-arrays': + for (const [ownerKey, rows] of Object.entries((value ?? {}) as Record)) { + add(ownerKey) + if (kind === 'owner-keyed-browser-row-arrays') { + collectBrowserRows(rows, add) + } else { + collectWorktreeRows(rows, add) + } + } + return + case 'worktree-id-array': + for (const ownerKey of (value ?? []) as string[]) { + add(ownerKey) + } + return + case 'row-record': + for (const row of Object.values((value ?? {}) as Record)) { + add(row?.worktreeId) + } + return + case 'row-arrays': + case 'browser-row-arrays': + for (const rows of Object.values((value ?? {}) as Record)) { + if (kind === 'browser-row-arrays') { + collectBrowserRows(rows, add) + } else { + collectWorktreeRows(rows, add) + } + } + } +} + +export function collectWorkspaceSessionWorktreeOwners( + session: WorkspaceSessionState, + candidateIds: ReadonlySet, + platform: NodeJS.Platform = process.platform, + owners: Set = new Set() +): Set { + const collector = createWorktreeOwnerCandidateCollector(candidateIds, platform, owners) + addWorkspaceSessionWorktreeOwners(session, collector) + return owners +} + +function addWorkspaceSessionWorktreeOwners( + session: WorkspaceSessionState, + collector: WorktreeOwnerCandidateCollector +): void { + for (const field of Object.keys( + WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND + ) as (keyof WorkspaceSessionState)[]) { + collectSessionFieldOwners( + session[field], + WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND[field], + collector.addOwner + ) + } +} + +export function addPersistedSessionWorktreeOwners( + state: Pick, + collector: WorktreeOwnerCandidateCollector +): void { + addWorkspaceSessionWorktreeOwners(state.workspaceSession, collector) + for (const session of Object.values(state.workspaceSessionsByHostId ?? {})) { + if (session) { + addWorkspaceSessionWorktreeOwners(session, collector) + } + } +} + +export function collectPersistedSessionWorktreeOwners( + state: Pick, + candidateIds: ReadonlySet, + platform: NodeJS.Platform = process.platform +): Set { + const collector = createWorktreeOwnerCandidateCollector(candidateIds, platform) + addPersistedSessionWorktreeOwners(state, collector) + return collector.owners +} diff --git a/src/main/persistence/tracking-repos/local-worktree-metadata-batch-pruning.bench.test.ts b/src/main/persistence/tracking-repos/local-worktree-metadata-batch-pruning.bench.test.ts new file mode 100644 index 00000000000..0f5a85a80c6 --- /dev/null +++ b/src/main/persistence/tracking-repos/local-worktree-metadata-batch-pruning.bench.test.ts @@ -0,0 +1,106 @@ +// Opt in: ORCA_LOCAL_METADATA_PRUNE_BENCH=1 pnpm test src/main/persistence/tracking-repos/local-worktree-metadata-batch-pruning.bench.test.ts +import { performance } from 'node:perf_hooks' +import { describe, expect, it, vi } from 'vitest' +import { getDefaultPersistedState } from '../../../shared/constants' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { Repo } from '../../../shared/repo-types' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import { removeWorkspaceSessionOwner } from '../restoring-sessions/session-owner-removal' +import { + pruneUnreferencedWorktreeIdentityMeta, + removeWorktreeMetadataForHost +} from '../loading-store/worktree-identity-metadata' +import { + captureNativeLocalWorktreeMetadataScanExpectation, + pruneSessionlessMissingLocalWorktreeMetadataForRepo +} from './missing-local-worktree-metadata-pruning' + +const describeBench = process.env.ORCA_LOCAL_METADATA_PRUNE_BENCH ? describe : describe.skip +const ROW_COUNT = 2_709 +const REPO: Repo = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 +} + +function makeMeta(index: number): WorktreeMeta { + return { + instanceId: `instance-${index}`, + hostId: 'local', + displayName: `stale-${index}`, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: index, + lastActivityAt: 0 + } +} + +function makeState(): { state: PersistedState; staleIds: string[] } { + const state = getDefaultPersistedState('/home/test') + state.repos = [REPO] + state.worktreeMetaByIdentity = {} + state.worktreeIdentityAliases = {} + const staleIds = Array.from( + { length: ROW_COUNT }, + (_, index) => `${REPO.id}::/workspace/stale-${index}` + ) + for (const [index, worktreeId] of staleIds.entries()) { + const meta = makeMeta(index) + const identityKey = `identity-${index}` + state.worktreeMeta[worktreeId] = meta + state.worktreeMetaByIdentity[identityKey] = meta + state.worktreeIdentityAliases[`local|${worktreeId}`] = [identityKey] + } + return { state, staleIds } +} + +describeBench('authoritative local metadata batch pruning', () => { + it('measures the reported 2,709-row sequential and batch shapes', () => { + const legacy = makeState() + const batch = makeState() + const cloneSpy = vi.spyOn(globalThis, 'structuredClone') + const legacyStartedAt = performance.now() + for (const worktreeId of legacy.staleIds) { + removeWorktreeMetadataForHost(legacy.state, worktreeId, undefined) + delete legacy.state.worktreeMeta[worktreeId] + legacy.state.workspaceSession = removeWorkspaceSessionOwner( + legacy.state.workspaceSession, + worktreeId + )! + } + pruneUnreferencedWorktreeIdentityMeta(legacy.state) + const legacyMs = performance.now() - legacyStartedAt + const legacySessionClones = cloneSpy.mock.calls.length + cloneSpy.mockClear() + + const batchStartedAt = performance.now() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(batch.state, REPO) + const removed = pruneSessionlessMissingLocalWorktreeMetadataForRepo( + batch.state, + scan, + scan.metadata + ) + const batchMs = performance.now() - batchStartedAt + const batchSessionClones = cloneSpy.mock.calls.length + cloneSpy.mockRestore() + + expect(removed).toHaveLength(ROW_COUNT) + expect(Object.keys(legacy.state.worktreeMeta)).toHaveLength(0) + expect(Object.keys(batch.state.worktreeMeta)).toHaveLength(0) + expect(legacySessionClones).toBe(ROW_COUNT) + expect(batchSessionClones).toBe(0) + + console.log( + `[bench] rows=${ROW_COUNT} metadata+session=${legacyMs.toFixed(2)}ms -> ${batchMs.toFixed(2)}ms; ` + + `gitScans=${ROW_COUNT} -> 1; sessionClones=${legacySessionClones} -> ${batchSessionClones}; ` + + `minimumSaveSchedules=${ROW_COUNT * 2} -> 1` + ) + }) +}) diff --git a/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.test.ts b/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.test.ts new file mode 100644 index 00000000000..85c883fe666 --- /dev/null +++ b/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.test.ts @@ -0,0 +1,242 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultPersistedState } from '../../../shared/constants' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { Repo } from '../../../shared/repo-types' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import { + captureNativeLocalWorktreeMetadataScanExpectation, + pruneSessionlessMissingLocalWorktreeMetadataForRepo +} from './missing-local-worktree-metadata-pruning' + +const REPO_ID = 'repo-1' +const WORKTREE_ID = `${REPO_ID}::/workspace/stale` +const LOCAL_ALIAS = `local|${WORKTREE_ID}` +const IDENTITY_KEY = 'local-identity' + +function makeRepo(id = REPO_ID): Repo { + return { + id, + path: `/workspace/${id}`, + displayName: id, + badgeColor: '#000', + addedAt: 0 + } +} + +function makeMeta(worktreeId = WORKTREE_ID): WorktreeMeta { + return { + instanceId: `instance-${worktreeId}`, + hostId: 'local', + displayName: worktreeId, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } +} + +function makeState(): PersistedState { + const state = getDefaultPersistedState('/home/test') + state.repos = [makeRepo()] + return state +} + +function makeCanonicalOnlyState(): { + state: PersistedState + meta: WorktreeMeta +} { + const state = makeState() + const meta = makeMeta() + state.worktreeMetaByIdentity = { [IDENTITY_KEY]: meta } + state.worktreeIdentityAliases = { [LOCAL_ALIAS]: [IDENTITY_KEY] } + return { state, meta } +} + +function pruneCaptured(state: PersistedState): string[] { + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + return pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata) +} + +describe('local worktree metadata scan expectations', () => { + it('captures canonical-only local metadata and removes its orphaned identity row', () => { + const { state } = makeCanonicalOnlyState() + + expect(pruneCaptured(state)).toEqual([WORKTREE_ID]) + expect(state.worktreeMeta[WORKTREE_ID]).toBeUndefined() + expect(state.worktreeIdentityAliases?.[LOCAL_ALIAS]).toBeUndefined() + expect(state.worktreeMetaByIdentity?.[IDENTITY_KEY]).toBeUndefined() + }) + + it('removes equal legacy and canonical projections after a persistence round trip', () => { + const state = makeState() + const meta = makeMeta() + state.worktreeMeta[WORKTREE_ID] = structuredClone(meta) + state.worktreeMetaByIdentity = { [IDENTITY_KEY]: structuredClone(meta) } + state.worktreeIdentityAliases = { [LOCAL_ALIAS]: [IDENTITY_KEY] } + + expect(pruneCaptured(state)).toEqual([WORKTREE_ID]) + expect(state.worktreeMeta[WORKTREE_ID]).toBeUndefined() + expect(state.worktreeIdentityAliases[LOCAL_ALIAS]).toBeUndefined() + expect(state.worktreeMetaByIdentity[IDENTITY_KEY]).toBeUndefined() + }) + + it('does not sweep an unrelated unaliased canonical row during pruning', () => { + const { state, meta } = makeCanonicalOnlyState() + const unrelated = makeMeta(`${REPO_ID}::/workspace/unrelated`) + state.worktreeMetaByIdentity!.unrelated = unrelated + + expect(pruneCaptured(state)).toEqual([WORKTREE_ID]) + expect(state.worktreeMetaByIdentity).toEqual({ unrelated }) + expect(state.worktreeMetaByIdentity?.[IDENTITY_KEY]).not.toBeDefined() + expect(meta).not.toBe(unrelated) + }) + + it('allocates expectations only for the scanned repository', () => { + const state = makeState() + const otherRepo = makeRepo('repo-2') + const otherId = `${otherRepo.id}::/workspace/other` + const canonicalId = `${REPO_ID}::/workspace/canonical` + const canonicalKey = 'canonical-key' + state.repos.push(otherRepo) + state.worktreeMeta[WORKTREE_ID] = makeMeta() + state.worktreeMeta[otherId] = makeMeta(otherId) + state.worktreeMetaByIdentity = { + [canonicalKey]: makeMeta(canonicalId), + 'other-key': makeMeta(otherId) + } + state.worktreeIdentityAliases = { + [`local|${canonicalId}`]: [canonicalKey], + [`local|${otherId}`]: ['other-key'] + } + + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + + expect(scan.metadata.map(({ worktreeId }) => worktreeId).sort()).toEqual( + [WORKTREE_ID, canonicalId].sort() + ) + }) + + it('preserves metadata when an alias array is replaced with identical contents', () => { + const { state } = makeCanonicalOnlyState() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + state.worktreeIdentityAliases![LOCAL_ALIAS] = [IDENTITY_KEY] + + expect(pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata)).toEqual( + [] + ) + expect(state.worktreeMetaByIdentity?.[IDENTITY_KEY]).toBeDefined() + }) + + it('preserves metadata when the captured alias array is mutated in place', () => { + const { state } = makeCanonicalOnlyState() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + state.worktreeIdentityAliases![LOCAL_ALIAS]!.push('new-identity') + state.worktreeMetaByIdentity!['new-identity'] = makeMeta() + + expect(pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata)).toEqual( + [] + ) + }) + + it.each([ + [ + 'replaced', + (state: PersistedState) => { + state.worktreeMetaByIdentity![IDENTITY_KEY] = { + ...state.worktreeMetaByIdentity![IDENTITY_KEY]! + } + } + ], + [ + 'host-mutated', + (state: PersistedState) => { + state.worktreeMetaByIdentity![IDENTITY_KEY]!.hostId = 'ssh:builder' + } + ], + [ + 'instance-mutated', + (state: PersistedState) => { + state.worktreeMetaByIdentity![IDENTITY_KEY]!.instanceId = 'new-instance' + } + ], + [ + 'content-mutated', + (state: PersistedState) => { + state.worktreeMetaByIdentity![IDENTITY_KEY]!.comment = 'new comment' + } + ] + ] as const)('preserves metadata when a canonical row is %s', (_label, mutate) => { + const { state } = makeCanonicalOnlyState() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + mutate(state) + + expect(pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata)).toEqual( + [] + ) + }) + + it.each([ + [ + 'added', + (state: PersistedState) => { + state.worktreeIdentityAliases![`ssh:builder|${WORKTREE_ID}`] = [IDENTITY_KEY] + } + ], + [ + 'removed', + (state: PersistedState) => { + delete state.worktreeIdentityAliases![LOCAL_ALIAS] + } + ] + ] as const)('preserves metadata when an alias is %s during the scan', (_label, mutate) => { + const { state } = makeCanonicalOnlyState() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + mutate(state) + + expect(pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata)).toEqual( + [] + ) + expect(state.worktreeMetaByIdentity?.[IDENTITY_KEY]).toBeDefined() + }) + + it('preserves a legacy row created after a canonical-only scan capture', () => { + const { state } = makeCanonicalOnlyState() + const scan = captureNativeLocalWorktreeMetadataScanExpectation(state, state.repos[0]!) + state.worktreeMeta[WORKTREE_ID] = makeMeta() + + expect(pruneSessionlessMissingLocalWorktreeMetadataForRepo(state, scan, scan.metadata)).toEqual( + [] + ) + expect(state.worktreeMeta[WORKTREE_ID]).toBeDefined() + }) + + it.each([WORKTREE_ID, `bogus|${WORKTREE_ID}`, `ssh:builder|${WORKTREE_ID}`])( + 'preserves metadata with an unqualified or foreign alias: %s', + (alias) => { + const state = makeState() + state.worktreeMeta[WORKTREE_ID] = makeMeta() + state.worktreeMetaByIdentity = { [IDENTITY_KEY]: state.worktreeMeta[WORKTREE_ID]! } + state.worktreeIdentityAliases = { [alias]: [IDENTITY_KEY] } + + expect(pruneCaptured(state)).toEqual([]) + expect(state.worktreeMeta[WORKTREE_ID]).toBeDefined() + } + ) + + it('preserves an unqualified alias whose worktree path contains a pipe', () => { + const state = makeState() + const pipedId = `${REPO_ID}::/workspace/a|b` + state.worktreeMeta[pipedId] = makeMeta(pipedId) + state.worktreeMetaByIdentity = { [IDENTITY_KEY]: state.worktreeMeta[pipedId]! } + state.worktreeIdentityAliases = { [pipedId]: [IDENTITY_KEY] } + + expect(pruneCaptured(state)).toEqual([]) + expect(state.worktreeMeta[pipedId]).toBeDefined() + expect(state.worktreeIdentityAliases[pipedId]).toEqual([IDENTITY_KEY]) + }) +}) diff --git a/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.ts b/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.ts new file mode 100644 index 00000000000..fca925d0986 --- /dev/null +++ b/src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.ts @@ -0,0 +1,243 @@ +import { isDeepStrictEqual } from 'node:util' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { GlobalSettings } from '../../../shared/global-settings-types' +import type { Project } from '../../../shared/project-types' +import type { Repo } from '../../../shared/repo-types' +import { getRepoKind } from '../../../shared/repo-kind' +import { splitWorktreeId } from '../../../shared/worktree/id' +import { + composeWorktreeHostIdentity, + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import { LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' + +type CapturedLocalRepoIdentity = Readonly< + Pick & { + kind: ReturnType + expectedRepo: Repo | undefined + } +> + +type CapturedLocalRoutingIdentity = Readonly<{ + expectedProject: Project | undefined + expectedProjectUpdatedAt: number | undefined + expectedSettings: GlobalSettings +}> + +type MetadataRowExpectation = Readonly<{ + expectedPresent: boolean + expectedMeta: WorktreeMeta | undefined + expectedSerialized: string | null | undefined + expectedInstanceId: string | undefined + expectedHostId: WorktreeMeta['hostId'] +}> + +type MetadataAliasExpectation = Readonly<{ + alias: string + expectedAlias: string[] + expectedIdentityKeys: readonly string[] + expectedIdentityRows: readonly (MetadataRowExpectation & { identityKey: string })[] +}> + +export type LocalWorktreeMetadataPruneExpectation = Readonly<{ + worktreeId: string + expectedLegacy: MetadataRowExpectation + expectedAliases: readonly MetadataAliasExpectation[] +}> + +export type NativeLocalWorktreeMetadataScanExpectation = Readonly<{ + repo: CapturedLocalRepoIdentity + routing: CapturedLocalRoutingIdentity + metadata: readonly LocalWorktreeMetadataPruneExpectation[] +}> + +type MetadataAliasEntry = readonly [alias: string, identityKeys: string[]] + +function serializeMetadataRow(meta: WorktreeMeta | undefined): string | null | undefined { + if (!meta) { + return undefined + } + try { + return JSON.stringify(meta) + } catch { + return null + } +} + +function captureRow( + record: Readonly> | undefined, + key: string +): MetadataRowExpectation { + const expectedPresent = Object.hasOwn(record ?? {}, key) + const expectedMeta = record?.[key] + return { + expectedPresent, + expectedMeta, + expectedSerialized: serializeMetadataRow(expectedMeta), + expectedInstanceId: expectedMeta?.instanceId, + expectedHostId: expectedMeta?.hostId + } +} + +export function indexMetadataAliasesForWorktreeIds( + state: PersistedState, + worktreeIds: ReadonlySet +): Map { + const indexed = new Map() + for (const [alias, identityKeys] of Object.entries(state.worktreeIdentityAliases ?? {})) { + const matches = new Set() + if (worktreeIds.has(alias)) { + matches.add(alias) + } + const separator = alias.indexOf('|') + const suffix = separator === -1 ? alias : alias.slice(separator + 1) + if (worktreeIds.has(suffix)) { + matches.add(suffix) + } + for (const worktreeId of matches) { + const entries = indexed.get(worktreeId) ?? [] + entries.push([alias, identityKeys]) + indexed.set(worktreeId, entries) + } + } + return indexed +} + +export function captureNativeLocalWorktreeMetadataScanExpectation( + state: PersistedState, + repo: Repo +): NativeLocalWorktreeMetadataScanExpectation { + const matchingRepos = state.repos.filter((entry) => entry.id === repo.id) + const expectedRepo = matchingRepos.length === 1 ? matchingRepos[0] : undefined + const expectedProject = state.projects.find((project) => project.sourceRepoIds.includes(repo.id)) + const candidateIds = new Set() + for (const [worktreeId, meta] of Object.entries(state.worktreeMeta)) { + if ( + splitWorktreeId(worktreeId)?.repoId === repo.id && + (!meta.hostId || meta.hostId === LOCAL_EXECUTION_HOST_ID) + ) { + candidateIds.add(worktreeId) + } + } + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + if (!isWorktreeHostIdentity(alias)) { + continue + } + const worktreeId = getWorktreeIdFromHostIdentity(alias) + if ( + alias === composeWorktreeHostIdentity(LOCAL_EXECUTION_HOST_ID, worktreeId) && + splitWorktreeId(worktreeId)?.repoId === repo.id + ) { + candidateIds.add(worktreeId) + } + } + const aliasesByWorktreeId = indexMetadataAliasesForWorktreeIds(state, candidateIds) + return { + repo: { + id: repo.id, + path: repo.path, + connectionId: repo.connectionId, + executionHostId: repo.executionHostId, + kind: getRepoKind(repo), + expectedRepo + }, + routing: { + expectedProject, + expectedProjectUpdatedAt: expectedProject?.updatedAt, + expectedSettings: state.settings + }, + metadata: [...candidateIds].map((worktreeId) => ({ + worktreeId, + expectedLegacy: captureRow(state.worktreeMeta, worktreeId), + expectedAliases: (aliasesByWorktreeId.get(worktreeId) ?? []).map(([alias, identityKeys]) => ({ + alias, + expectedAlias: identityKeys, + expectedIdentityKeys: [...identityKeys], + expectedIdentityRows: identityKeys.map((identityKey) => ({ + identityKey, + ...captureRow(state.worktreeMetaByIdentity, identityKey) + })) + })) + })) + } +} + +function rowStillMatches( + record: Readonly> | undefined, + key: string, + expected: MetadataRowExpectation +): boolean { + const current = record?.[key] + return ( + Object.hasOwn(record ?? {}, key) === expected.expectedPresent && + current === expected.expectedMeta && + expected.expectedSerialized !== null && + serializeMetadataRow(current) === expected.expectedSerialized && + current?.instanceId === expected.expectedInstanceId && + current?.hostId === expected.expectedHostId + ) +} + +function aliasesStillMatch( + state: PersistedState, + expected: LocalWorktreeMetadataPruneExpectation, + currentAliases: readonly MetadataAliasEntry[] +): boolean { + if (currentAliases.length !== expected.expectedAliases.length) { + return false + } + const currentByAlias = new Map(currentAliases) + return expected.expectedAliases.every((aliasExpectation) => { + const current = currentByAlias.get(aliasExpectation.alias) + return Boolean( + current && + current === aliasExpectation.expectedAlias && + current.length === aliasExpectation.expectedIdentityKeys.length && + current.every((key, index) => key === aliasExpectation.expectedIdentityKeys[index]) && + aliasExpectation.expectedIdentityRows.every(({ identityKey, ...row }) => + rowStillMatches(state.worktreeMetaByIdentity, identityKey, row) + ) + ) + }) +} + +export function removeRevalidatedLocalWorktreeMetadata( + state: PersistedState, + expected: LocalWorktreeMetadataPruneExpectation, + currentAliases: readonly MetadataAliasEntry[], + removedIdentityKeys?: Set +): boolean { + if ( + !rowStillMatches(state.worktreeMeta, expected.worktreeId, expected.expectedLegacy) || + !aliasesStillMatch(state, expected, currentAliases) + ) { + return false + } + const localAlias = composeWorktreeHostIdentity(LOCAL_EXECUTION_HOST_ID, expected.worktreeId) + if (currentAliases.some(([alias]) => alias !== localAlias)) { + return false + } + const legacy = state.worktreeMeta[expected.worktreeId] + const identityKeys = state.worktreeIdentityAliases?.[localAlias] + if (identityKeys && identityKeys.length !== 1) { + return false + } + const canonical = identityKeys?.[0] ? state.worktreeMetaByIdentity?.[identityKeys[0]] : undefined + if ( + (legacy?.hostId && legacy.hostId !== LOCAL_EXECUTION_HOST_ID) || + (canonical?.hostId && canonical.hostId !== LOCAL_EXECUTION_HOST_ID) || + (identityKeys && !canonical) || + (legacy && canonical && !isDeepStrictEqual(legacy, canonical)) || + (!legacy && !canonical) + ) { + return false + } + if (identityKeys?.[0]) { + removedIdentityKeys?.add(identityKeys[0]) + } + delete state.worktreeMeta[expected.worktreeId] + delete state.worktreeIdentityAliases?.[localAlias] + return true +} diff --git a/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.test.ts b/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.test.ts new file mode 100644 index 00000000000..cdad14034e2 --- /dev/null +++ b/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.test.ts @@ -0,0 +1,477 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultPersistedState, getDefaultWorkspaceSession } from '../../../shared/constants' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { Project } from '../../../shared/project-types' +import type { Repo } from '../../../shared/repo-types' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import { + captureNativeLocalWorktreeMetadataScanExpectation, + pruneSessionlessMissingLocalWorktreeMetadataForRepo +} from './missing-local-worktree-metadata-pruning' + +const REPO_ID = 'repo-1' +const LIVE_ID = `${REPO_ID}::/workspace/live` + +function makeRepo(overrides: Partial = {}): Repo { + return { + id: REPO_ID, + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + ...overrides + } +} + +function makeMeta(worktreeId: string): WorktreeMeta { + return { + instanceId: `instance-${worktreeId}`, + hostId: 'local', + displayName: worktreeId, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } +} + +function makeState(repo = makeRepo()): PersistedState { + const state = getDefaultPersistedState('/home/test') + state.repos = [repo] + return state +} + +function capture(state: PersistedState, repo = state.repos[0]!) { + return captureNativeLocalWorktreeMetadataScanExpectation(state, repo) +} + +function pruneCaptured( + state: PersistedState, + scan: ReturnType, + ids: readonly string[], + platform?: NodeJS.Platform +): string[] { + const wanted = new Set(ids) + return pruneSessionlessMissingLocalWorktreeMetadataForRepo( + state, + scan, + scan.metadata.filter(({ worktreeId }) => wanted.has(worktreeId)), + platform + ) +} + +describe('pruneSessionlessMissingLocalWorktreeMetadataForRepo', () => { + it('removes 2,709 sessionless rows in one batch without cloning or changing session state', () => { + const state = makeState() + const staleIds = Array.from( + { length: 2_709 }, + (_, index) => `${REPO_ID}::/workspace/stale-${index}` + ) + const allIds = [LIVE_ID, ...staleIds] + state.worktreeMetaByIdentity = {} + state.worktreeIdentityAliases = {} + for (const [index, worktreeId] of allIds.entries()) { + const meta = makeMeta(worktreeId) + const identityKey = `identity-${index}` + state.worktreeMeta[worktreeId] = meta + state.worktreeMetaByIdentity[identityKey] = meta + state.worktreeIdentityAliases[`local|${worktreeId}`] = [identityKey] + } + state.worktreeLineageById[staleIds[0]!] = { worktreeId: staleIds[0] } as never + state.workspaceLineageByChildKey[worktreeWorkspaceKey(staleIds[0]!)] = { + childWorkspaceKey: worktreeWorkspaceKey(staleIds[0]!) + } as never + state.workspaceSession.terminalTopologyRevisionByRepoId = { [REPO_ID]: 7 } + const hostSession = { + ...getDefaultWorkspaceSession(), + terminalTopologyRevisionByRepoId: { [REPO_ID]: 11 } + } + state.workspaceSessionsByHostId = { 'runtime:mirror': hostSession } + const scan = capture(state) + const session = state.workspaceSession + const sessionSnapshot = structuredClone(session) + const hostSessionSnapshot = structuredClone(hostSession) + const cloneSpy = vi.spyOn(globalThis, 'structuredClone') + + const removed = pruneCaptured(state, scan, staleIds) + + expect(removed).toHaveLength(staleIds.length) + expect(cloneSpy).not.toHaveBeenCalled() + cloneSpy.mockRestore() + expect(Object.keys(state.worktreeMeta)).toEqual([LIVE_ID]) + expect(Object.keys(state.worktreeMetaByIdentity ?? {})).toEqual(['identity-0']) + expect(Object.keys(state.worktreeIdentityAliases ?? {})).toEqual([`local|${LIVE_ID}`]) + expect(state.worktreeLineageById[staleIds[0]!]).toBeUndefined() + expect(state.workspaceLineageByChildKey[worktreeWorkspaceKey(staleIds[0]!)]).toBeUndefined() + expect(state.workspaceSession).toBe(session) + expect(state.workspaceSession).toEqual(sessionSnapshot) + expect(state.workspaceSessionsByHostId?.['runtime:mirror']).toBe(hostSession) + expect(state.workspaceSessionsByHostId?.['runtime:mirror']).toEqual(hostSessionSnapshot) + }) + + it('fails closed when metadata is mutated or replaced after the scan starts', () => { + const inPlaceState = makeState() + const inPlaceId = `${REPO_ID}::/workspace/in-place` + inPlaceState.worktreeMeta[inPlaceId] = makeMeta(inPlaceId) + const inPlaceScan = capture(inPlaceState) + inPlaceState.worktreeMeta[inPlaceId]!.instanceId = 'replacement-instance' + + expect(pruneCaptured(inPlaceState, inPlaceScan, [inPlaceId])).toEqual([]) + + const replacedState = makeState() + const replacedId = `${REPO_ID}::/workspace/replaced` + const original = makeMeta(replacedId) + replacedState.worktreeMeta[replacedId] = original + const replacedScan = capture(replacedState) + replacedState.worktreeMeta[replacedId] = { ...original } + + expect(pruneCaptured(replacedState, replacedScan, [replacedId])).toEqual([]) + expect(replacedState.worktreeMeta[replacedId]).not.toBe(original) + }) + + it('fails closed when an identical repo row is removed and re-added during the scan', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/re-added-repo` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + const replacement = { ...state.repos[0]! } + state.repos.splice(0, 1, replacement) + + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + expect(state.repos[0]).toBe(replacement) + expect(state.worktreeMeta[worktreeId]).toBeDefined() + }) + + it('uses the effective Git kind when the raw legacy repo omitted kind', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/legacy-git-kind` + const rawRepo = state.repos[0]! + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state, { ...rawRepo, kind: 'git' }) + + expect(rawRepo.kind).toBeUndefined() + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([worktreeId]) + }) + + it.each([ + 'tabsByWorktree', + 'browserTabsByWorktree', + 'openFilesByWorktree', + 'activeFileIdByWorktree', + 'activeBrowserTabIdByWorktree', + 'clientHostedBrowserPagesByWorktree', + 'activeTabTypeByWorktree', + 'activeTabIdByWorktree', + 'unifiedTabs', + 'tabGroups', + 'tabGroupLayouts', + 'activeGroupIdByWorktree', + 'lastVisitedAtByWorktreeId', + 'defaultTerminalTabsAppliedByWorktreeId' + ] as const)('preserves an owner named by workspaceSession.%s', (field) => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/${field}` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + const ownerKey = field === 'lastVisitedAtByWorktreeId' ? `local|${worktreeId}` : worktreeId + ;(state.workspaceSession as unknown as Record)[field] = { + [ownerKey]: [] + } + + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + expect(state.worktreeMeta[worktreeId]).toBeDefined() + }) + + it('preserves scalar, nested, remote-partition, mobile, and relay recovery owners', () => { + const ownerMutations: ((state: PersistedState, worktreeId: string) => void)[] = [ + (state, worktreeId) => { + state.workspaceSession.activeWorktreeId = worktreeId + }, + (state, worktreeId) => { + state.workspaceSession.activeWorkspaceKey = worktreeWorkspaceKey(worktreeId) + }, + (state, worktreeId) => { + state.workspaceSession.activeWorktreeIdsOnShutdown = [worktreeId] + }, + (state, worktreeId) => { + state.ui.lastActiveWorktreeId = worktreeId + }, + (state, worktreeId) => { + state.workspaceSession.sleepingAgentSessionsByPaneKey = { + pane: { worktreeId } as never + } + }, + (state, worktreeId) => { + state.workspaceSession.terminalSurfaceTombstonesByPaneKey = { + pane: { worktreeId } as never + } + }, + (state, worktreeId) => { + state.workspaceSession.closedTerminalTabTombstonesByTabId = { + tab: { worktreeId, closedAt: 1 } + } + }, + (state, worktreeId) => { + state.workspaceSession.clientHostedBrowserCloseIntentsByEnvironment = { + runtime: [{ worktreeId, browserPageId: 'page', closedAt: 1 }] + } + }, + (state, worktreeId) => { + state.workspaceSession.browserPagesByWorkspace = { + browser: [{ worktreeId } as never] + } + }, + (state, worktreeId) => { + state.workspaceSessionsByHostId = { + 'ssh:builder': { + ...getDefaultWorkspaceSession(), + lastVisitedAtByWorktreeId: { [worktreeId]: 1 } + } + } + }, + (state, worktreeId) => { + state.mobileClientTabSelectionsByDeviceId = { + phone: { [worktreeId]: {} as never } + } + }, + (state, worktreeId) => { + state.sshRemotePtyLeases = [ + { + targetId: 'builder', + ptyId: 'pty', + worktreeId, + state: 'detached', + createdAt: 1, + updatedAt: 1 + } + ] + }, + (state, worktreeId) => { + state.migrationUnsupportedPtyEntries = [ + { + ptyId: 'pty', + worktreeId, + reason: 'legacy-numeric-pane-key', + source: 'local', + updatedAt: 1 + } + ] + }, + (state, worktreeId) => { + state.automations = [ + { + enabled: true, + workspaceMode: 'existing', + workspaceId: worktreeId + } as never + ] + }, + (state, worktreeId) => { + state.automationRuns = [{ status: 'dispatching', workspaceId: worktreeId } as never] + } + ] + + for (const [index, mutate] of ownerMutations.entries()) { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/owner-${index}` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + mutate(state, worktreeId) + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + } + }) + + it('preserves canonically equivalent session and top-level owners', () => { + const candidateId = `${REPO_ID}::/workspace/Café`.normalize('NFC') + const ownerId = candidateId.normalize('NFD') + const ownerMutations: ((state: PersistedState) => void)[] = [ + (state) => { + state.workspaceSession.activeWorktreeId = ownerId + }, + (state) => { + state.ui.lastActiveWorktreeId = ownerId + } + ] + + for (const mutate of ownerMutations) { + const state = makeState() + state.worktreeMeta[candidateId] = makeMeta(candidateId) + const scan = capture(state) + mutate(state) + + expect(pruneCaptured(state, scan, [candidateId])).toEqual([]) + expect(state.worktreeMeta[candidateId]).toBeDefined() + } + }) + + it('removes a row on a later pass after its persisted owner is cleared', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/two-phase` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + state.workspaceSession.lastVisitedAtByWorktreeId = { [worktreeId]: 1 } + const scan = capture(state) + + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + delete state.workspaceSession.lastVisitedAtByWorktreeId![worktreeId] + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([worktreeId]) + }) + + it('treats dotfile visibility and cleanup dismissals as non-owning preferences', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/non-owning-preferences` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const showDotfilesByWorktree = { [worktreeId]: true } + const workspaceCleanup = { + dismissals: { + [worktreeId]: { + worktreeId, + dismissedAt: 1, + fingerprint: 'fingerprint', + classifierVersion: 1 + } + } + } + state.ui.showDotfilesByWorktree = showDotfilesByWorktree + state.ui.workspaceCleanup = workspaceCleanup + const scan = capture(state) + + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([worktreeId]) + expect(state.ui.showDotfilesByWorktree).toBe(showDotfilesByWorktree) + expect(state.ui.workspaceCleanup).toBe(workspaceCleanup) + }) + + it('preserves foreign aliases, ambiguous local aliases, and divergent projections', () => { + const foreignState = makeState() + const foreignId = `${REPO_ID}::/workspace/foreign-alias` + const localMeta = makeMeta(foreignId) + const remoteMeta = { ...localMeta, hostId: 'ssh:builder' as const } + foreignState.worktreeMeta[foreignId] = localMeta + foreignState.worktreeMetaByIdentity = { local: localMeta, remote: remoteMeta } + foreignState.worktreeIdentityAliases = { + [`local|${foreignId}`]: ['local'], + [`ssh:builder|${foreignId}`]: ['remote'] + } + const foreignScan = capture(foreignState) + + expect(pruneCaptured(foreignState, foreignScan, [foreignId])).toEqual([]) + expect(foreignState.worktreeMetaByIdentity).toEqual({ local: localMeta, remote: remoteMeta }) + + const ambiguousState = makeState() + const ambiguousId = `${REPO_ID}::/workspace/ambiguous` + const first = makeMeta(ambiguousId) + ambiguousState.worktreeMeta[ambiguousId] = first + ambiguousState.worktreeMetaByIdentity = { first, second: { ...first } } + ambiguousState.worktreeIdentityAliases = { [`local|${ambiguousId}`]: ['first', 'second'] } + expect(pruneCaptured(ambiguousState, capture(ambiguousState), [ambiguousId])).toEqual([]) + + const divergentState = makeState() + const divergentId = `${REPO_ID}::/workspace/divergent` + const legacy = makeMeta(divergentId) + const canonical = { ...legacy, comment: 'newer canonical comment' } + divergentState.worktreeMeta[divergentId] = legacy + divergentState.worktreeMetaByIdentity = { canonical } + divergentState.worktreeIdentityAliases = { [`local|${divergentId}`]: ['canonical'] } + const divergentScan = capture(divergentState) + expect(pruneCaptured(divergentState, divergentScan, [divergentId])).toEqual([]) + }) + + it('fails closed for repo replacement, host collisions, remote, runtime, and folder repos', () => { + const mutations: ((state: PersistedState) => void)[] = [ + (state) => { + state.repos[0] = makeRepo({ path: '/workspace/replacement' }) + }, + (state) => { + state.repos.push(makeRepo({ connectionId: 'builder', executionHostId: 'ssh:builder' })) + }, + (state) => { + state.repos[0] = makeRepo({ connectionId: 'builder', executionHostId: 'ssh:builder' }) + }, + (state) => { + state.repos[0] = makeRepo({ executionHostId: 'runtime:environment' }) + }, + (state) => { + state.repos[0] = makeRepo({ kind: 'folder' }) + } + ] + for (const mutate of mutations) { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/stale` + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + mutate(state) + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + } + }) + + it('fails closed when project runtime routing changes away and back during the scan', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/project-routing-race` + const project: Project = { + id: 'project-1', + displayName: 'project', + badgeColor: '#000', + localWindowsRuntimePreference: { kind: 'windows-host' }, + sourceRepoIds: [REPO_ID], + createdAt: 1, + updatedAt: 1 + } + state.projects = [project] + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + project.localWindowsRuntimePreference = { kind: 'wsl', distro: 'Ubuntu' } + project.updatedAt += 1 + project.localWindowsRuntimePreference = { kind: 'windows-host' } + project.updatedAt += 1 + + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + expect(state.worktreeMeta[worktreeId]).toBeDefined() + }) + + it('fails closed when global runtime routing changes away and back during the scan', () => { + const state = makeState() + const worktreeId = `${REPO_ID}::/workspace/settings-routing-race` + const originalDefault = state.settings.localWindowsRuntimeDefault + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + const scan = capture(state) + state.settings = { + ...state.settings, + localWindowsRuntimeDefault: { kind: 'wsl', distro: 'Ubuntu' } + } + state.settings = { + ...state.settings, + localWindowsRuntimeDefault: originalDefault + } + + expect(state.settings.localWindowsRuntimeDefault).toEqual(originalDefault) + expect(pruneCaptured(state, scan, [worktreeId])).toEqual([]) + expect(state.worktreeMeta[worktreeId]).toBeDefined() + }) + + it('rejects malformed, folder-instance, WSL, and non-native Windows candidate paths', () => { + const state = makeState() + const foreignWindowsId = `${REPO_ID}::C:\\workspace\\stale` + const ids = [ + `${REPO_ID}::relative/path`, + `${REPO_ID}::`, + `wrong::/workspace/stale`, + `${REPO_ID}::/workspace/folder::workspace:11111111-1111-4111-8111-111111111111`, + `${REPO_ID}::\\\\wsl.localhost\\Ubuntu\\home\\gone`, + `${REPO_ID}::/home/user/wsl-legacy`, + foreignWindowsId + ] + for (const worktreeId of ids) { + state.worktreeMeta[worktreeId] = makeMeta(worktreeId) + } + const scan = capture(state) + + expect(pruneCaptured(state, scan, ids.slice(0, -2))).toEqual([]) + expect(pruneCaptured(state, scan, [ids.at(-2)!], 'win32')).toEqual([]) + expect(pruneCaptured(state, scan, [foreignWindowsId], 'linux')).toEqual([]) + }) +}) diff --git a/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.ts b/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.ts new file mode 100644 index 00000000000..3ab380bdd8e --- /dev/null +++ b/src/main/persistence/tracking-repos/missing-local-worktree-metadata-pruning.ts @@ -0,0 +1,153 @@ +import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path' +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' +import type { PersistedState } from '../../../shared/persisted-state-types' +import { getRepoKind } from '../../../shared/repo-kind' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR, splitWorktreeId } from '../../../shared/worktree/id' +import { isWslUncPath } from '../../../shared/wsl-paths' +import { isFinalAutomationRunStatus } from '../../../shared/automations-types' +import { pruneUnreferencedWorktreeIdentityMeta } from '../loading-store/worktree-identity-metadata' +import { + addPersistedSessionWorktreeOwners, + createWorktreeOwnerCandidateCollector +} from '../restoring-sessions/session-worktree-ownership' +import { + indexMetadataAliasesForWorktreeIds, + removeRevalidatedLocalWorktreeMetadata, + type LocalWorktreeMetadataPruneExpectation, + type NativeLocalWorktreeMetadataScanExpectation +} from './local-worktree-metadata-scan-expectation' + +export { captureNativeLocalWorktreeMetadataScanExpectation } from './local-worktree-metadata-scan-expectation' +export type { + LocalWorktreeMetadataPruneExpectation, + NativeLocalWorktreeMetadataScanExpectation +} from './local-worktree-metadata-scan-expectation' + +function collectPersistedWorkspaceOwners( + state: PersistedState, + candidateIds: ReadonlySet, + platform: NodeJS.Platform +): Set { + const collector = createWorktreeOwnerCandidateCollector(candidateIds, platform) + addPersistedSessionWorktreeOwners(state, collector) + const add = collector.addOwner + add(state.ui.lastActiveWorktreeId) + for (const selections of Object.values(state.mobileClientTabSelectionsByDeviceId ?? {})) { + for (const worktreeId of Object.keys(selections)) { + add(worktreeId) + } + } + for (const lease of state.sshRemotePtyLeases) { + add(lease.worktreeId) + } + for (const entry of state.migrationUnsupportedPtyEntries) { + add(entry.worktreeId) + } + for (const automation of state.automations) { + if (automation.enabled && automation.workspaceMode === 'existing') { + add(automation.workspaceId) + } + } + for (const run of state.automationRuns) { + if (!isFinalAutomationRunStatus(run.status)) { + add(run.workspaceId) + } + } + return collector.owners +} + +function repoStillMatches( + state: PersistedState, + expected: NativeLocalWorktreeMetadataScanExpectation['repo'] +): boolean { + const owners = state.repos.filter((repo) => repo.id === expected.id) + const current = owners[0] + return Boolean( + owners.length === 1 && + current && + current === expected.expectedRepo && + current.path === expected.path && + current.connectionId === expected.connectionId && + current.executionHostId === expected.executionHostId && + getRepoKind(current) === expected.kind && + expected.kind === 'git' && + !current.connectionId && + getRepoExecutionHostId(current) === LOCAL_EXECUTION_HOST_ID + ) +} + +function routingStillMatches( + state: PersistedState, + expected: NativeLocalWorktreeMetadataScanExpectation +): boolean { + const currentProject = state.projects.find((project) => + project.sourceRepoIds.includes(expected.repo.id) + ) + return ( + currentProject === expected.routing.expectedProject && + currentProject?.updatedAt === expected.routing.expectedProjectUpdatedAt && + state.settings === expected.routing.expectedSettings + ) +} + +function isValidCandidateId( + repoId: string, + worktreeId: string, + platform: NodeJS.Platform +): boolean { + const parsed = splitWorktreeId(worktreeId) + return Boolean( + parsed?.repoId === repoId && + parsed.worktreePath.length > 0 && + !isWslUncPath(parsed.worktreePath) && + (platform === 'win32' + ? isWindowsAbsolutePathLike(parsed.worktreePath) + : parsed.worktreePath.startsWith('/')) && + !worktreeId.includes(FOLDER_WORKSPACE_INSTANCE_SEPARATOR) + ) +} + +export function pruneSessionlessMissingLocalWorktreeMetadataForRepo( + state: PersistedState, + scan: NativeLocalWorktreeMetadataScanExpectation, + missingMetadata: readonly LocalWorktreeMetadataPruneExpectation[], + platform = process.platform +): string[] { + if ( + missingMetadata.length === 0 || + !repoStillMatches(state, scan.repo) || + !routingStillMatches(state, scan) + ) { + return [] + } + + const candidateIds = new Set(missingMetadata.map(({ worktreeId }) => worktreeId)) + const sessionOwners = collectPersistedWorkspaceOwners(state, candidateIds, platform) + const aliasesByWorktreeId = indexMetadataAliasesForWorktreeIds(state, candidateIds) + const removedIdentityKeys = new Set() + const removedIds: string[] = [] + for (const expectation of missingMetadata) { + const { worktreeId } = expectation + if ( + !isValidCandidateId(scan.repo.id, worktreeId, platform) || + sessionOwners.has(worktreeId) || + !removeRevalidatedLocalWorktreeMetadata( + state, + expectation, + aliasesByWorktreeId.get(worktreeId) ?? [], + removedIdentityKeys + ) + ) { + continue + } + delete state.worktreeLineageById[worktreeId] + delete state.workspaceLineageByChildKey[worktreeWorkspaceKey(worktreeId)] + removedIds.push(worktreeId) + } + if (removedIds.length > 0) { + // Why: a global sweep could delete unrelated unaliased rows from another repo or host. + pruneUnreferencedWorktreeIdentityMeta(state, removedIdentityKeys) + } + return removedIds +} diff --git a/src/main/persistence/tracking-repos/project-host-operations.ts b/src/main/persistence/tracking-repos/project-host-operations.ts index c6a8441f58a..605c258f4f7 100644 --- a/src/main/persistence/tracking-repos/project-host-operations.ts +++ b/src/main/persistence/tracking-repos/project-host-operations.ts @@ -19,6 +19,7 @@ import { repoGitUsernameCacheKey } from './repo-hydration' export type ProjectHostMutationOperations = { state: PersistedState gitUsernameCache: Map + bumpLocalWorktreeScanGeneration: (repoId: string) => void hydrateRepo: (repo: Repo) => Repo updateRepoBackedProjectHostSetup: ( setup: ProjectHostSetup, @@ -87,6 +88,9 @@ export class ProjectHostPersistenceOperations { return null } if ('localWindowsRuntimePreference' in updates) { + for (const repoId of project.sourceRepoIds) { + this.operations.bumpLocalWorktreeScanGeneration(repoId) + } if (updates.localWindowsRuntimePreference === undefined) { delete project.localWindowsRuntimePreference } else { diff --git a/src/main/persistence/tracking-repos/repo-update-operations.ts b/src/main/persistence/tracking-repos/repo-update-operations.ts index f39e0e1e26f..6ad6f23903b 100644 --- a/src/main/persistence/tracking-repos/repo-update-operations.ts +++ b/src/main/persistence/tracking-repos/repo-update-operations.ts @@ -9,6 +9,7 @@ import { sanitizeRepoUpdatesForPersistence } from './repo-sanitization' export type RepoUpdateMutationOperations = { state: PersistedState + bumpLocalWorktreeScanGeneration: (repoId: string) => void syncProjectHostSetupCompatibilityState: () => void scheduleSave: () => void hydrateRepo: (repo: Repo) => Repo @@ -21,6 +22,10 @@ export class RepoUpdatePersistenceOperations { return this.operations.state } + private bumpLocalWorktreeScanGeneration(repoId: string): void { + this.operations.bumpLocalWorktreeScanGeneration(repoId) + } + private syncProjectHostSetupCompatibilityState(): void { this.operations.syncProjectHostSetupCompatibilityState() } @@ -177,6 +182,7 @@ export class RepoUpdatePersistenceOperations { } } Object.assign(repo, sanitizedUpdates) + this.bumpLocalWorktreeScanGeneration(id) this.syncProjectHostSetupCompatibilityState() this.scheduleSave() return this.hydrateRepo(repo) diff --git a/src/main/worktree-lineage-pruning.test.ts b/src/main/worktree-lineage-pruning.test.ts index db393e864d5..22b34f70104 100644 --- a/src/main/worktree-lineage-pruning.test.ts +++ b/src/main/worktree-lineage-pruning.test.ts @@ -57,6 +57,82 @@ function createStore( } describe('pruneLineageForMissingRepoWorktrees', () => { + it('keeps lineage stored under another spelling of a live worktree path', () => { + const parentId = 'repo-1::/repo' + const childId = 'repo-1::/repo/./child' + const edge = lineage(childId, parentId) + const worktreeLineageById = { [childId]: edge } + const workspaceLineageByChildKey = { + [worktreeWorkspaceKey(childId)]: workspaceLineage(childId, parentId) + } + const metaById = { + [parentId]: { instanceId: edge.parentWorktreeInstanceId } as WorktreeMeta + } + const store = createStore(worktreeLineageById, workspaceLineageByChildKey, metaById) + + pruneLineageForMissingRepoWorktrees( + store as never, + repo, + [ + { path: '/repo', head: 'a', branch: 'main', isBare: false, isMainWorktree: true }, + { path: '/repo/child/', head: 'b', branch: 'child', isBare: false, isMainWorktree: false } + ], + { platform: 'linux' } + ) + + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(store.removeWorkspaceLineage).not.toHaveBeenCalled() + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(worktreeLineageById[childId]).toBe(edge) + }) + + it('keeps captured metadata lineage not removed after Git canonicalizes paths', () => { + const parentId = 'repo-1::/alias/parent' + const childId = 'repo-1::/alias/child' + const edge = lineage(childId, parentId) + const worktreeLineageById = { [childId]: edge } + const workspaceLineageByChildKey = { + [worktreeWorkspaceKey(childId)]: workspaceLineage(childId, parentId) + } + const metaById = { + [childId]: { instanceId: edge.worktreeInstanceId } as WorktreeMeta, + [parentId]: { instanceId: edge.parentWorktreeInstanceId } as WorktreeMeta + } + const store = createStore(worktreeLineageById, workspaceLineageByChildKey, metaById) + + pruneLineageForMissingRepoWorktrees( + store as never, + repo, + [ + { + path: '/canonical/parent', + head: 'a', + branch: 'parent', + isBare: false, + isMainWorktree: false + }, + { + path: '/canonical/child', + head: 'b', + branch: 'child', + isBare: false, + isMainWorktree: false + } + ], + { + platform: 'linux', + preservedMetadataCandidateIds: new Set([parentId, childId]) + } + ) + + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(store.removeWorkspaceLineage).not.toHaveBeenCalled() + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(worktreeLineageById[childId]).toBe(edge) + expect(workspaceLineageByChildKey[worktreeWorkspaceKey(childId)]).toBeDefined() + expect(metaById[parentId].instanceId).toBe(edge.parentWorktreeInstanceId) + }) + it('refuses an empty scan when the repo still has registered lineage', () => { const parentId = 'repo-1::/repo/parent' const childId = 'repo-1::/repo/child' @@ -78,7 +154,7 @@ describe('pruneLineageForMissingRepoWorktrees', () => { expect(workspaceLineageByChildKey[worktreeWorkspaceKey(childId)]).toBe(workspaceEdge) }) - it('prunes missing children and rotates missing parents after a trusted non-empty scan', () => { + it('prunes and rotates missing IDs absent from preserved metadata candidates', () => { const liveParentId = 'repo-1::/repo/live-parent' const missingChildId = 'repo-1::/repo/missing-child' const liveChildId = 'repo-1::/repo/live-child' @@ -99,25 +175,31 @@ describe('pruneLineageForMissingRepoWorktrees', () => { } const store = createStore(worktreeLineageById, workspaceLineageByChildKey, metaById) - pruneLineageForMissingRepoWorktrees(store as never, repo, [ - { - path: '/repo/live-parent', - head: 'a', - branch: 'main', - isBare: false, - isMainWorktree: false - }, - { - path: '/repo/live-child', - head: 'b', - branch: 'child', - isBare: false, - isMainWorktree: false - } - ]) + pruneLineageForMissingRepoWorktrees( + store as never, + repo, + [ + { + path: '/repo/live-parent', + head: 'a', + branch: 'main', + isBare: false, + isMainWorktree: false + }, + { + path: '/repo/live-child', + head: 'b', + branch: 'child', + isBare: false, + isMainWorktree: false + } + ], + { preservedMetadataCandidateIds: new Set([liveParentId, liveChildId]) } + ) expect(store.removeWorktreeLineage).toHaveBeenCalledWith(missingChildId) expect(store.removeWorktreeLineage).not.toHaveBeenCalledWith(liveChildId) + expect(store.removeWorkspaceLineage).toHaveBeenCalledWith(worktreeWorkspaceKey(missingChildId)) expect(store.setWorktreeMeta).toHaveBeenCalledWith(missingParentId, { instanceId: expect.any(String) }) @@ -126,4 +208,25 @@ describe('pruneLineageForMissingRepoWorktrees', () => { missingParentEdge.parentWorktreeInstanceId ) }) + + it('does not rematerialize metadata already removed for a missing parent', () => { + const liveChildId = 'repo-1::/repo/live-child' + const removedParentId = 'repo-1::/repo/removed-parent' + const edge = lineage(liveChildId, removedParentId) + const worktreeLineageById = { [liveChildId]: edge } + const store = createStore(worktreeLineageById, {}, {}) + + pruneLineageForMissingRepoWorktrees(store as never, repo, [ + { + path: '/repo/live-child', + head: 'child-head', + branch: 'child', + isBare: false, + isMainWorktree: false + } + ]) + + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(worktreeLineageById[liveChildId]).toBe(edge) + }) }) diff --git a/src/main/worktree-lineage-pruning.ts b/src/main/worktree-lineage-pruning.ts index 34a99ef6e95..6040f25c03d 100644 --- a/src/main/worktree-lineage-pruning.ts +++ b/src/main/worktree-lineage-pruning.ts @@ -4,6 +4,8 @@ import type { WorkspaceLineage, WorktreeLineage } from '../shared/worktree/linea import type { GitWorktreeInfo } from '../shared/worktree/types' import { getRepoExecutionHostId } from '../shared/execution-host' import { isWorkspaceKey, parseWorkspaceKey, worktreeWorkspaceKey } from '../shared/workspace-scope' +import { splitWorktreeId } from '../shared/worktree/id' +import { worktreeRetentionPathComparisonKey } from './worktree-retention-path-comparison' import type { Store } from './persistence' function worktreeIdBelongsToRepo(worktreeId: string, repoPrefix: string): boolean { @@ -37,8 +39,13 @@ function hasStoredRepoLineage( export function pruneLineageForMissingRepoWorktrees( store: Store, repo: Repo, - gitWorktrees: GitWorktreeInfo[] + gitWorktrees: GitWorktreeInfo[], + options: { + platform?: NodeJS.Platform + preservedMetadataCandidateIds?: ReadonlySet + } = {} ): void { + const { platform = process.platform, preservedMetadataCandidateIds } = options if ( typeof store.getAllWorktreeLineage !== 'function' || typeof store.removeWorktreeLineage !== 'function' @@ -56,6 +63,24 @@ export function pruneLineageForMissingRepoWorktrees( return } const liveIds = new Set(gitWorktrees.map((worktree) => `${repo.id}::${worktree.path}`)) + const livePathKeys = new Set( + [repo.path, ...gitWorktrees.map(({ path }) => path)].map((pathValue) => + worktreeRetentionPathComparisonKey(pathValue, platform) + ) + ) + const isLive = (worktreeId: string): boolean => { + if (liveIds.has(worktreeId) || preservedMetadataCandidateIds?.has(worktreeId)) { + return true + } + const worktreePath = splitWorktreeId(worktreeId)?.worktreePath + if (!worktreePath) { + return false + } + if (livePathKeys.has(worktreeRetentionPathComparisonKey(worktreePath, platform))) { + return true + } + return false + } const expectedHostId = getRepoExecutionHostId(repo) const repoOwners = store.getRepos().filter((candidate) => candidate.id === repo.id) const canMutateWorktree = (worktreeId: string): boolean => { @@ -68,7 +93,7 @@ export function pruneLineageForMissingRepoWorktrees( childScope?.type === 'worktree' && worktreeIdBelongsToRepo(childScope.worktreeId, repoPrefix) && canMutateWorktree(childScope.worktreeId) && - !liveIds.has(childScope.worktreeId) && + !isLive(childScope.worktreeId) && isWorkspaceKey(childWorkspaceKey) ) { store.removeWorkspaceLineage?.(childWorkspaceKey) @@ -78,7 +103,7 @@ export function pruneLineageForMissingRepoWorktrees( if ( worktreeIdBelongsToRepo(childId, repoPrefix) && canMutateWorktree(childId) && - !liveIds.has(childId) + !isLive(childId) ) { // Why: a proven-missing path must not transfer its lineage to a future checkout at that path. store.removeWorktreeLineage(childId) @@ -87,10 +112,10 @@ export function pruneLineageForMissingRepoWorktrees( if ( worktreeIdBelongsToRepo(lineage.parentWorktreeId, repoPrefix) && canMutateWorktree(lineage.parentWorktreeId) && - !liveIds.has(lineage.parentWorktreeId) + !isLive(lineage.parentWorktreeId) ) { const parentMeta = store.getWorktreeMeta(lineage.parentWorktreeId) - if (!parentMeta || parentMeta.instanceId === lineage.parentWorktreeInstanceId) { + if (parentMeta?.instanceId === lineage.parentWorktreeInstanceId) { // Why: rotate a proven-missing parent's identity once so path reuse cannot validate old lineage. store.setWorktreeMeta(lineage.parentWorktreeId, { instanceId: randomUUID() }) } diff --git a/src/main/worktree-retention-path-comparison.ts b/src/main/worktree-retention-path-comparison.ts new file mode 100644 index 00000000000..2ff3fd02990 --- /dev/null +++ b/src/main/worktree-retention-path-comparison.ts @@ -0,0 +1,24 @@ +import { splitWorktreeId } from '../shared/worktree/id' +import { worktreePathComparisonKey } from './ipc/worktree-path-comparison' + +/** Comparison keys used only to retain state; canonical equivalence must never select a delete target. */ +export function worktreeRetentionPathComparisonKey( + pathValue: string, + platform: NodeJS.Platform +): string { + return worktreePathComparisonKey(pathValue.normalize('NFC'), platform) +} + +export function worktreeRetentionIdComparisonKey( + worktreeId: string, + platform: NodeJS.Platform +): string | null { + const parsed = splitWorktreeId(worktreeId) + if (!parsed?.repoId || !parsed.worktreePath) { + return null + } + return JSON.stringify([ + parsed.repoId, + worktreeRetentionPathComparisonKey(parsed.worktreePath, platform) + ]) +} diff --git a/src/shared/priority-semaphore.test.ts b/src/shared/priority-semaphore.test.ts index 035e3ca920d..aeeacda94ef 100644 --- a/src/shared/priority-semaphore.test.ts +++ b/src/shared/priority-semaphore.test.ts @@ -145,4 +145,19 @@ describe('PrioritySemaphore', () => { const r2 = await sem.acquire(0) r2() }) + + it('removes an aborted waiter without consuming the next permit', async () => { + const sem = new PrioritySemaphore(1) + const release = await sem.acquire(0) + const controller = new AbortController() + const reason = new Error('canceled') + const waiting = sem.acquire(0, controller.signal) + + controller.abort(reason) + await expect(waiting).rejects.toBe(reason) + release() + + const nextRelease = await sem.acquire(0) + nextRelease() + }) }) diff --git a/src/shared/priority-semaphore.ts b/src/shared/priority-semaphore.ts index 4d31726937d..b7a0a50e06c 100644 --- a/src/shared/priority-semaphore.ts +++ b/src/shared/priority-semaphore.ts @@ -1,6 +1,8 @@ type Waiter = { priority: number resolve: (release: () => void) => void + signal?: AbortSignal + onAbort?: () => void } export class PrioritySemaphore { @@ -11,7 +13,10 @@ export class PrioritySemaphore { this.available = concurrency } - acquire(priority: number): Promise<() => void> { + acquire(priority: number, signal?: AbortSignal): Promise<() => void> { + if (signal?.aborted) { + return Promise.reject(signal.reason) + } if (this.available > 0) { this.available-- let released = false @@ -24,8 +29,20 @@ export class PrioritySemaphore { }) } - return new Promise<() => void>((resolve) => { - this.waiters.push({ priority, resolve }) + return new Promise<() => void>((resolve, reject) => { + const waiter: Waiter = { priority, resolve, signal } + if (signal) { + waiter.onAbort = () => { + const index = this.waiters.indexOf(waiter) + if (index === -1) { + return + } + this.waiters.splice(index, 1) + reject(signal.reason) + } + signal.addEventListener('abort', waiter.onAbort, { once: true }) + } + this.waiters.push(waiter) }) } @@ -45,6 +62,9 @@ export class PrioritySemaphore { } const waiter = this.waiters.splice(bestIdx, 1)[0] + if (waiter.signal && waiter.onAbort) { + waiter.signal.removeEventListener('abort', waiter.onAbort) + } let released = false waiter.resolve(() => { if (released) { From de851d5ca7b01c0a9bd8d8e2caaea9d1e89635bc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:25:30 -0700 Subject: [PATCH 14/31] perf(terminals): drop disposed queued inspections (#17187) --- ...dinator-queued-inspection-disposal.test.ts | 81 +++++++++++++++++++ .../agent-completion-process-monitor.ts | 1 + .../agent-process-inspection-queue.ts | 11 +++ 3 files changed, 93 insertions(+) create mode 100644 src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts new file mode 100644 index 00000000000..9d83b1d6a93 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + createDeferred, + flushAsyncTicks, + processResult, + useAgentCompletionCoordinatorLifecycle +} from './agent-completion-coordinator-test-harness' +import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' + +describe('agent completion coordinator queued inspections', () => { + useAgentCompletionCoordinatorLifecycle() + + it('drops inspections queued by a disposed coordinator before starting live work', async () => { + const blockers = Array.from({ length: 4 }, () => + createDeferred() + ) + const blockerInspectors = blockers.map((inspection) => vi.fn(() => inspection.promise)) + const blockerCoordinators = blockerInspectors.map((inspectProcess, index) => + createAgentCompletionCoordinator({ + paneKey: `tab-1:blocked-${index}`, + getPtyId: () => `pty-blocked-${index}`, + getSettings: () => null, + inspectProcess, + dispatchCompletion: vi.fn(), + isLive: () => true + }) + ) + + blockerCoordinators.forEach((coordinator) => coordinator.startProcessTracking()) + await vi.advanceTimersByTimeAsync(2_000) + expect( + blockerInspectors.every((inspectProcess) => inspectProcess.mock.calls.length === 1) + ).toBe(true) + + const staleInspectProcesses = Array.from({ length: 8 }, () => + vi.fn(async () => processResult(null, false)) + ) + const staleCoordinators = staleInspectProcesses.map((inspectProcess, index) => + createAgentCompletionCoordinator({ + paneKey: `tab-1:stale-${index}`, + getPtyId: () => `pty-stale-${index}`, + getSettings: () => null, + inspectProcess, + dispatchCompletion: vi.fn(), + isLive: () => true + }) + ) + const liveInspectProcess = vi.fn(async () => processResult(null, false)) + const liveCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:live', + getPtyId: () => 'pty-live', + getSettings: () => null, + inspectProcess: liveInspectProcess, + dispatchCompletion: vi.fn(), + isLive: () => true + }) + + for (const [index, coordinator] of staleCoordinators.entries()) { + coordinator.observeTitle(`Codex working ${index}`) + coordinator.observeTitle(`~/stale-${index}`) + } + liveCoordinator.observeTitle('Codex working') + liveCoordinator.observeTitle('~/live') + staleCoordinators.forEach((coordinator) => coordinator.dispose()) + + blockers.forEach((inspection) => inspection.resolve(processResult(null, false))) + await flushAsyncTicks() + // Existing 100ms pump admits live work after blockers release. + await vi.advanceTimersByTimeAsync(100) + await flushAsyncTicks() + + expect( + staleInspectProcesses.every((inspectProcess) => inspectProcess.mock.calls.length === 0) + ).toBe(true) + expect(liveInspectProcess).toHaveBeenCalledTimes(1) + + blockerCoordinators.forEach((coordinator) => coordinator.dispose()) + liveCoordinator.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts b/src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts index 39ca68e1ad4..f0a0492fdfe 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-process-monitor.ts @@ -146,6 +146,7 @@ export function createAgentCompletionProcessMonitor({ const pendingTitleIdAtRequest = priority === 'pending-title' ? pendingTitle.get()?.id : null enqueueAgentProcessInspection({ priority, + canRun: () => !state.disposed, run: async () => { let inspectedRecognizedAgent = false let inspectionSucceeded = false diff --git a/src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts b/src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts index 5c581ddc652..f7512ad8c60 100644 --- a/src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts +++ b/src/renderer/src/components/terminal-pane/agent-process-inspection-queue.ts @@ -2,6 +2,7 @@ export type InspectionPriority = 'cadence' | 'pending-title' type InspectionTask = { priority: InspectionPriority + canRun: () => boolean run: () => Promise } @@ -37,6 +38,16 @@ function scheduleInspectionPump(delayMs = 0): void { } function pumpInspectionQueue(): void { + // Drop disposed tasks before slot/rate accounting. + for (let index = inspectionQueue.length - 1; index >= 0; index -= 1) { + const task = inspectionQueue[index] + if (task && !task.canRun()) { + inspectionQueue.splice(index, 1) + } + } + if (inspectionQueue.length === 0) { + return + } const now = Date.now() if (!canStartInspection(now)) { scheduleInspectionPump(100) From 0a995cc30d14e980cf738204e49d64dcb8467bd6 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 29 Aug 2026 18:14:03 -0700 Subject: [PATCH 15/31] Fix md file rendering 2 (#17295) * Improve markdown rich mode: distinguish HTML tags from placeholders - Consolidate size limit and unsupported content checks into single function - Refine HTML/JSX detection to validate against rehype-sanitize's known tag names - Allow bare placeholders like `` and `` in rich mode rendering * Add decision explainer for markdown rich-mode rendering fix * rm html explainer --- ...orContent.markdown-classification.test.tsx | 15 ++++-- .../editor/editor-panel-render-model.ts | 17 +++---- .../editor/markdown-rich-mode.test.ts | 44 ++++++++++++++++- .../components/editor/markdown-rich-mode.ts | 49 ++++++++++++++++++- 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx b/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx index 7d8767071be..4a5600192d3 100644 --- a/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx +++ b/src/renderer/src/components/editor/EditorContent.markdown-classification.test.tsx @@ -10,11 +10,16 @@ const classifiers = vi.hoisted(() => ({ })) vi.mock('./markdown-rich-mode', () => ({ - getMarkdownRichModeUnsupportedMessage: classifiers.getUnsupportedMessage -})) - -vi.mock('./markdown-rich-size-limit', () => ({ - exceedsMarkdownRichModeSizeLimit: classifiers.exceedsSizeLimit + getMarkdownRichModeEligibility: ({ + content, + sizeOverridden + }: { + content: string + sizeOverridden: boolean + }) => ({ + exceedsSizeLimit: !sizeOverridden && classifiers.exceedsSizeLimit(content), + unsupportedMessage: classifiers.getUnsupportedMessage(content) + }) })) vi.mock('./editor-lazy-views', () => { diff --git a/src/renderer/src/components/editor/editor-panel-render-model.ts b/src/renderer/src/components/editor/editor-panel-render-model.ts index 8a479794f83..db03e65efe1 100644 --- a/src/renderer/src/components/editor/editor-panel-render-model.ts +++ b/src/renderer/src/components/editor/editor-panel-render-model.ts @@ -13,8 +13,7 @@ import type { EditorToggleValue } from './EditorViewToggle' import type { FileContent } from './editor-panel-content-types' import { canUseChangesModeForFile } from './editor-panel-file-mode' import { getMarkdownRenderMode, type MarkdownRenderState } from './markdown-render-mode' -import { getMarkdownRichModeUnsupportedMessage } from './markdown-rich-mode' -import { exceedsMarkdownRichModeSizeLimit } from './markdown-rich-size-limit' +import { getMarkdownRichModeEligibility } from './markdown-rich-mode' type StoreState = ReturnType @@ -143,16 +142,16 @@ export function getEditorPanelRenderModel({ let inlineMarkdownRenderState: MarkdownRenderState | null = null if (canRenderInlineMarkdown) { const shouldClassifyRichMode = mdViewMode === 'rich' - const exceedsRichModeSizeLimit = - shouldClassifyRichMode && - markdownRichModeSizeOverride[activeFile.id] !== true && - exceedsMarkdownRichModeSizeLimit(inlineMarkdownContent) - const richModeUnsupportedMessage = shouldClassifyRichMode - ? getMarkdownRichModeUnsupportedMessage(inlineMarkdownContent) + const richModeEligibility = shouldClassifyRichMode + ? getMarkdownRichModeEligibility({ + content: inlineMarkdownContent, + sizeOverridden: markdownRichModeSizeOverride[activeFile.id] === true + }) : null + const richModeUnsupportedMessage = richModeEligibility?.unsupportedMessage ?? null inlineMarkdownRenderState = { renderMode: getMarkdownRenderMode({ - exceedsRichModeSizeLimit, + exceedsRichModeSizeLimit: richModeEligibility?.exceedsSizeLimit ?? false, hasRichModeUnsupportedContent: richModeUnsupportedMessage !== null, viewMode: mdViewMode }), diff --git a/src/renderer/src/components/editor/markdown-rich-mode.test.ts b/src/renderer/src/components/editor/markdown-rich-mode.test.ts index e95b6f49c81..25f2e3bd869 100644 --- a/src/renderer/src/components/editor/markdown-rich-mode.test.ts +++ b/src/renderer/src/components/editor/markdown-rich-mode.test.ts @@ -1,5 +1,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { getMarkdownRichModeUnsupportedMessage } from './markdown-rich-mode' +import { RICH_MARKDOWN_MAX_SIZE_BYTES } from '../../../../shared/constants' +import { + getMarkdownRichModeEligibility, + getMarkdownRichModeUnsupportedMessage +} from './markdown-rich-mode' afterEach(() => { vi.restoreAllMocks() @@ -46,6 +50,31 @@ describe('getMarkdownRichModeUnsupportedMessage', () => { ).toBeNull() }) + it('allows bare lowercase placeholders in prose', () => { + expect( + getMarkdownRichModeUnsupportedMessage( + 'Use --host runtime:, --output , and --project .\n' + ) + ).toBeNull() + }) + + it('allows bare lowercase placeholders in large documents', () => { + const content = `${'a'.repeat(50_001)}\nUse --host runtime: and --project .\n` + + expect(getMarkdownRichModeUnsupportedMessage(content)).toBeNull() + }) + + it('still blocks large documents with actual html or jsx', () => { + const prefix = 'a'.repeat(50_001) + + expect(getMarkdownRichModeUnsupportedMessage(`${prefix}\ntext\n`)).not.toBeNull() + expect(getMarkdownRichModeUnsupportedMessage(`${prefix}\ntext\n`)).not.toBeNull() + expect(getMarkdownRichModeUnsupportedMessage(`${prefix}\n\n`)).not.toBeNull() + expect( + getMarkdownRichModeUnsupportedMessage(`${prefix}\n\n`) + ).not.toBeNull() + }) + it('allows block html and mdx-like tags by preserving them as passthrough nodes', () => { expect(getMarkdownRichModeUnsupportedMessage('\n')).toBeNull() expect(getMarkdownRichModeUnsupportedMessage('
block
\n')).toBeNull() @@ -90,4 +119,17 @@ describe('getMarkdownRichModeUnsupportedMessage', () => { ) expect(usedGlobalHtmlFragmentMatch).toBe(false) }) + + it('keeps unsupported content blocked when it also exceeds the size limit', () => { + const content = `${'a'.repeat(RICH_MARKDOWN_MAX_SIZE_BYTES + 1)}` + + expect(getMarkdownRichModeEligibility({ content, sizeOverridden: false })).toEqual({ + exceedsSizeLimit: true, + unsupportedMessage: expect.any(String) + }) + expect(getMarkdownRichModeEligibility({ content, sizeOverridden: true })).toEqual({ + exceedsSizeLimit: false, + unsupportedMessage: expect.any(String) + }) + }) }) diff --git a/src/renderer/src/components/editor/markdown-rich-mode.ts b/src/renderer/src/components/editor/markdown-rich-mode.ts index 694a8a7ee5f..4764892a4ea 100644 --- a/src/renderer/src/components/editor/markdown-rich-mode.ts +++ b/src/renderer/src/components/editor/markdown-rich-mode.ts @@ -1,5 +1,7 @@ +import { defaultSchema } from 'rehype-sanitize' import { getRichMarkdownRoundTripOutput } from './markdown-round-trip' import { extractFrontMatter } from './markdown-frontmatter' +import { exceedsMarkdownRichModeSizeLimit } from './markdown-rich-size-limit' import { translate } from '@/i18n/i18n' export type MarkdownRichModeUnsupportedReason = @@ -14,6 +16,13 @@ type UnsupportedMatch = { pattern: RegExp } +export type MarkdownRichModeEligibility = { + exceedsSizeLimit: boolean + unsupportedMessage: string | null +} + +const KNOWN_MARKDOWN_HTML_TAG_NAMES = new Set(defaultSchema.tagNames ?? []) + const UNSUPPORTED_PATTERNS: UnsupportedMatch[] = [ { reason: 'html-or-jsx', @@ -66,7 +75,7 @@ export function getMarkdownRichModeUnsupportedMessage(content: string): string | // opinion when HTML is detected, to verify the HTML survives the round-trip // before blocking the user from rich mode. const htmlMatcher = UNSUPPORTED_PATTERNS.find((m) => m.reason === 'html-or-jsx') - const hasHtml = htmlMatcher && htmlMatcher.pattern.test(contentWithoutCode) + const hasHtml = htmlMatcher && hasHtmlOrJsx(contentWithoutCode, htmlMatcher.pattern) for (const matcher of UNSUPPORTED_PATTERNS) { if (matcher.reason === 'html-or-jsx') { @@ -91,6 +100,44 @@ export function getMarkdownRichModeUnsupportedMessage(content: string): string | return null } +export function getMarkdownRichModeEligibility({ + content, + sizeOverridden +}: { + content: string + sizeOverridden: boolean +}): MarkdownRichModeEligibility { + const exceedsSizeLimit = !sizeOverridden && exceedsMarkdownRichModeSizeLimit(content) + return { + exceedsSizeLimit, + unsupportedMessage: getMarkdownRichModeUnsupportedMessage(content) + } +} + +function hasHtmlOrJsx(content: string, pattern: RegExp): boolean { + for (const match of content.matchAll(new RegExp(pattern, 'g'))) { + if (isHtmlOrJsxFragment(match[0])) { + return true + } + } + return false +} + +function isHtmlOrJsxFragment(fragment: string): boolean { + if (fragment.startsWith('