From deb0be1c5241eb3a8a6823e9f561f55736c3ff05 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:33:12 -0700 Subject: [PATCH 1/5] fix: recognize working WSL1 without a WSL2 kernel (#19061) * fix: recognize working WSL1 without a WSL2 kernel * fix: recognize unsigned Windows missing-kernel status * fix(wsl): fold the missing-kernel guest probe into wsl-availability The separate wsl-missing-kernel-probe module failed three CI gates: it was not in the web typecheck project (TS6307), it added a new direct wsl.exe spawn outside wsl-runner, and its `catch { return false }` tripped the probe-failure-semantics ratchet. wsl-availability.ts already owns the answer and is already on the invocation allowlist, so the probe lives there now. A guest probe that cannot spawn keeps the real --status failure instead of minting a fresh negative, which is what the ratchet exists to prevent -- and is the more correct semantics. --- .../wsl-availability-missing-kernel.test.ts | 98 +++++++++++++++++++ src/main/wsl-availability.ts | 63 +++++++++++- 2 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 src/main/wsl-availability-missing-kernel.test.ts diff --git a/src/main/wsl-availability-missing-kernel.test.ts b/src/main/wsl-availability-missing-kernel.test.ts new file mode 100644 index 00000000000..3ab7b6d4a26 --- /dev/null +++ b/src/main/wsl-availability-missing-kernel.test.ts @@ -0,0 +1,98 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { execFile, execFileSync } from 'node:child_process' +import { runProcess, runProcessSync, type ProcessResult } from '../shared/child-process/run-process' +import { + _resetWslAvailabilityCacheForTests, + isWslAvailable, + isWslAvailableAsync +} from './wsl-availability' + +vi.mock('node:child_process', () => ({ execFile: vi.fn(), execFileSync: vi.fn() })) +vi.mock('../shared/child-process/run-process', () => ({ + runProcess: vi.fn(), + runProcessSync: vi.fn() +})) +vi.mock('./wsl-interop-spawn-directory', () => ({ + resolveWslInteropSpawnCwd: () => 'C:\\Windows' +})) + +const originalPlatform = process.platform +const success: ProcessResult = { code: 0, signal: null, stdout: '', stderr: '', timedOut: false } + +beforeEach(() => { + vi.resetAllMocks() + Object.defineProperty(process, 'platform', { value: 'win32' }) + _resetWslAvailabilityCacheForTests() +}) +afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }) + _resetWslAvailabilityCacheForTests() +}) + +for (const mode of ['sync', 'async'] as const) { + describe(`${mode} WSL1 availability without WSL2 kernel`, () => { + const probe = () => (mode === 'sync' ? isWslAvailable() : isWslAvailableAsync()) + const guestRunner = () => (mode === 'sync' ? runProcessSync : runProcess) + + function failStatus(code: number): void { + vi.mocked(execFileSync).mockImplementation(() => { + throw { status: code } + }) + vi.mocked(execFile).mockImplementation((...args: unknown[]) => { + const callback = args.at(-1) as (error: unknown) => void + callback({ code }) + return {} as ReturnType + }) + } + function guestResult(result: ProcessResult): void { + vi.mocked(runProcess).mockResolvedValue(result) + vi.mocked(runProcessSync).mockReturnValue(result) + } + + // Node reports the Windows DWORD; the console prints its signed equivalent. + for (const status of [-444, 4_294_966_852]) { + it(`requires guest execution and caches its success for ${status}`, async () => { + failStatus(status) + guestResult(success) + expect(await probe()).toBe(true) + expect(await probe()).toBe(true) + expect(guestRunner()).toHaveBeenCalledTimes(1) + expect(guestRunner()).toHaveBeenCalledWith( + expect.objectContaining({ + program: 'wsl.exe', + args: ['--exec', '/bin/true'], + timeoutMs: 5000, + cwd: 'C:\\Windows' + }) + ) + }) + } + + for (const result of [ + { ...success, code: 1 }, + { ...success, code: null, timedOut: true } + ]) { + it(`keeps a failed guest unavailable: ${JSON.stringify(result)}`, async () => { + failStatus(-444) + guestResult(result) + expect(await probe()).toBe(false) + }) + } + + it('stays unavailable when the guest probe cannot be spawned', async () => { + failStatus(-444) + vi.mocked(runProcess).mockRejectedValue(new Error('EPERM')) + vi.mocked(runProcessSync).mockImplementation(() => { + throw new Error('EPERM') + }) + expect(await probe()).toBe(false) + }) + + it('does not probe a guest for unrelated status failures', async () => { + failStatus(1) + expect(await probe()).toBe(false) + expect(runProcess).not.toHaveBeenCalled() + expect(runProcessSync).not.toHaveBeenCalled() + }) + }) +} diff --git a/src/main/wsl-availability.ts b/src/main/wsl-availability.ts index 1d14f526413..ad5e5645c30 100644 --- a/src/main/wsl-availability.ts +++ b/src/main/wsl-availability.ts @@ -1,4 +1,6 @@ import { execFile, execFileSync } from 'node:child_process' +import { runProcess, runProcessSync, type ProcessSpec } from '../shared/child-process/run-process' +import { buildWslExecArgs } from '../shared/wsl-login-shell-command' import { resolveWslInteropSpawnCwd } from './wsl-interop-spawn-directory' type WslAvailabilityCache = @@ -94,6 +96,55 @@ function cacheWslAvailabilityProbeResult(error: unknown, startedAtGeneration: nu return !error } +// `wsl --status` exits 0x1bc when the WSL2 kernel package is missing -- a package +// a WSL1 distro never needed. Node keeps the Windows DWORD; the console prints the +// signed form, and either spelling can reach us. +function isMissingWsl2KernelStatus(error: unknown): boolean { + const failure = error as { status?: unknown; code?: unknown } | null + return [failure?.status, failure?.code].some((code) => code === -444 || code === 4_294_966_852) +} + +// Cheapest proof the default guest runs: no login shell, no output to parse. +function defaultGuestExecutionProbe(): ProcessSpec { + return { + program: 'wsl.exe', + args: buildWslExecArgs(undefined, ['/bin/true']), + cwd: resolveWslInteropSpawnCwd(), + timeoutMs: WSL_AVAILABILITY_PROBE_TIMEOUT_MS, + maxOutputBytes: 4096 + } +} + +/** + * The `--status` error still worth caching, or null once the guest ran anyway. + * + * Why it returns that error rather than a fresh negative: a guest probe that could not + * spawn means "could not ask", and minting an answer for that is the bug this subsystem + * keeps re-shipping (docs/reference/wsl-probe-failure-semantics.md). + */ +function wslStatusErrorAfterGuestProbe(error: unknown): unknown { + if (!isMissingWsl2KernelStatus(error)) { + return error + } + try { + return runProcessSync(defaultGuestExecutionProbe()).code === 0 ? null : error + } catch { + return error + } +} + +/** Async twin of `wslStatusErrorAfterGuestProbe`; the sync/async pair share one cache. */ +async function wslStatusErrorAfterGuestProbeAsync(error: unknown): Promise { + if (!isMissingWsl2KernelStatus(error)) { + return error + } + try { + return (await runProcess(defaultGuestExecutionProbe())).code === 0 ? null : error + } catch { + return error + } +} + function probeWslStatus(): Promise { return new Promise((resolve, reject) => { execFile( @@ -147,7 +198,10 @@ export function isWslAvailable(): boolean { }) return cacheWslAvailabilityProbeResult(null, startedAtGeneration) } catch (error) { - return cacheWslAvailabilityProbeResult(error, startedAtGeneration) + return cacheWslAvailabilityProbeResult( + wslStatusErrorAfterGuestProbe(error), + startedAtGeneration + ) } } @@ -176,7 +230,12 @@ export function isWslAvailableAsync(): Promise { const startedAtGeneration = wslAvailabilityCacheGeneration wslAvailabilityProbeInFlight = probeWslStatus() .then(() => cacheWslAvailabilityProbeResult(null, startedAtGeneration)) - .catch((error: unknown) => cacheWslAvailabilityProbeResult(error, startedAtGeneration)) + .catch(async (error: unknown) => + cacheWslAvailabilityProbeResult( + await wslStatusErrorAfterGuestProbeAsync(error), + startedAtGeneration + ) + ) .finally(() => { wslAvailabilityProbeInFlight = null }) From 0cba706b01e2e0fef620893d441e272cdac7894e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:33:19 -0700 Subject: [PATCH 2/5] fix(ports): coalesce advertised URL refresh bursts (#19150) --- .../ports/WorkspacePortScanner.test.tsx | 110 ++++++++++++++++++ .../components/ports/WorkspacePortScanner.tsx | 14 ++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx index 0ae53fee931..7741368d025 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx @@ -602,3 +602,113 @@ describe('WorkspacePortScanner', () => { expect(getPublishedRemoteWorktreePorts()).toBeUndefined() }) }) + +describe('advertised URL refresh bursts', () => { + async function mountLocalScanner(): Promise<() => void> { + useAppStore.setState({ settings: getDefaultSettings('/tmp/orca-workspaces') }) + await act(async () => { + root?.render() + await flushPromises() + }) + localScan.mockClear() + return vi.mocked(window.api.workspacePorts.onAdvertisedUrlChanged).mock + .calls[0][0] as () => void + } + + it('coalesces sequential URL changes into one immediate scan and one settled scan', async () => { + const changed = await mountLocalScanner() + for (let index = 0; index < 5; index++) { + await act(async () => { + changed() + await flushPromises() + await vi.advanceTimersByTimeAsync(100) + }) + } + expect(localScan).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.advanceTimersByTimeAsync(1_000) + }) + expect(localScan).toHaveBeenCalledTimes(2) + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(3) + }) + + it('cancels the settled scan on unmount', async () => { + const changed = await mountLocalScanner() + await act(async () => { + changed() + await flushPromises() + }) + act(() => root?.unmount()) + root = null + await vi.advanceTimersByTimeAsync(2_000) + expect(localScan).toHaveBeenCalledTimes(1) + }) + + it('skips the settled scan while hidden and accepts the next visible URL change', async () => { + let visibility: DocumentVisibilityState = 'visible' + const restore = overrideDocumentVisibilityState(() => visibility) + try { + const changed = await mountLocalScanner() + await act(async () => { + changed() + await flushPromises() + }) + visibility = 'hidden' + await act(async () => { + await vi.advanceTimersByTimeAsync(2_000) + }) + expect(localScan).toHaveBeenCalledTimes(1) + visibility = 'visible' + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(2) + } finally { + restore() + } + }) +}) + +it('releases the URL burst when its leading scan finishes while hidden', async () => { + let visibility: DocumentVisibilityState = 'visible' + const restore = overrideDocumentVisibilityState(() => visibility) + try { + useAppStore.setState({ settings: getDefaultSettings('/tmp/orca-workspaces') }) + await act(async () => { + root?.render() + await flushPromises() + }) + const changed = vi.mocked(window.api.workspacePorts.onAdvertisedUrlChanged).mock + .calls[0][0] as () => void + let finish!: (scan: WorkspacePortScanResult) => void + localScan.mockClear() + localScan.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + await act(async () => { + changed() + await flushPromises() + }) + visibility = 'hidden' + await act(async () => { + finish(emptyScan) + await flushPromises() + }) + visibility = 'visible' + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(2) + } finally { + restore() + } +}) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.tsx index 2b12bd86cd8..f8f2d11ee40 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.tsx @@ -280,6 +280,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): return } + let burstRefresh: Promise | null = null let eventSequence = 0 let disposed = false let retryTimer: ReturnType | null = null @@ -296,15 +297,24 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): const sequence = eventSequence clearRetryTimer() if (!isWindowVisible()) { + burstRefresh = null return } - void refresh({ force: true, targets: [runtimeTarget] }).finally(() => { - if (disposed || sequence !== eventSequence || !isWindowVisible()) { + // Keep the leading scan through the quiet window so sequential events share it too. + burstRefresh ??= refresh({ force: true, targets: [runtimeTarget] }) + void burstRefresh.finally(() => { + if (disposed || sequence !== eventSequence) { + return + } + if (!isWindowVisible()) { + burstRefresh = null return } // Why: some dev servers print their URL just before the listener is // visible to lsof/netstat. One quiet settle scan catches that startup race. retryTimer = setTimeout(() => { + retryTimer = null + burstRefresh = null if (disposed || sequence !== eventSequence || !isWindowVisible()) { return } From be10e5455ef3211dd0e424cb0f817768cbfe72a4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:33:44 -0700 Subject: [PATCH 3/5] perf(store): keep recentlyRetiredAgentStatusPaneKeys identity on no-op retirement (#19142) boundRecentlyRetiredAgentStatusPaneKeys always rebuilt the record, replacing its reference even when nothing changed; a probe counted 1,099 such writes across the store suite. Return the existing record when no key would be evicted and the additions are already its tail in the same relative order. Key-set equality is deliberately NOT enough: re-adding a key must move it to the tail because that LRU order decides which key the cap evicts next. Share the LRU bound with boundRecentlyClosedAgentStatusTabIds, which had the same always-rebuild shape. --- .../store/slices/agent-pane-authority.test.ts | 14 +++ .../agent-status-pane-keyed-records.test.ts | 110 ++++++++++++++++++ .../slices/agent-status-pane-keyed-records.ts | 69 +++++++---- 3 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts diff --git a/src/renderer/src/store/slices/agent-pane-authority.test.ts b/src/renderer/src/store/slices/agent-pane-authority.test.ts index 5e97e3c9064..01e99f27d6f 100644 --- a/src/renderer/src/store/slices/agent-pane-authority.test.ts +++ b/src/renderer/src/store/slices/agent-pane-authority.test.ts @@ -74,6 +74,20 @@ describe('agent pane authority', () => { expect(retirePaneAuthority).toHaveBeenCalledWith(TARGET) }) + it('re-retiring an already-retired pane keeps the retired-key map identity and epochs', () => { + const store = createTestStore() + store.getState().setAgentStatus(TARGET, { state: 'working', prompt: 'target' }) + store.getState().retireAgentPaneAuthority(TARGET) + const before = store.getState() + + store.getState().retireAgentPaneAuthority(TARGET) + + const after = store.getState() + expect(after.recentlyRetiredAgentStatusPaneKeys).toBe(before.recentlyRetiredAgentStatusPaneKeys) + expect(after.agentStatusEpoch).toBe(before.agentStatusEpoch) + expect(after.sortEpoch).toBe(before.sortEpoch) + }) + it('retires the pane activity cutoff with the rest of its pane-owned state', () => { const store = createTestStore() store.setState({ diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts new file mode 100644 index 00000000000..5b327561637 --- /dev/null +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest' +import { + RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX, + RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, + boundRecentlyClosedAgentStatusTabIds, + boundRecentlyRetiredAgentStatusPaneKeys +} from './agent-status-pane-keyed-records' + +function keyRecord(keys: readonly string[]): Record { + const record: Record = {} + for (const key of keys) { + record[key] = true + } + return record +} + +function fullRecord(max: number, prefix: string): Record { + return keyRecord(Array.from({ length: max }, (_, i) => `${prefix}${i}`)) +} + +describe('boundRecentlyRetiredAgentStatusPaneKeys', () => { + it('returns the existing record when there is nothing to add', () => { + const existing = keyRecord(['a', 'b']) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, [])).toBe(existing) + const empty = keyRecord([]) + expect(boundRecentlyRetiredAgentStatusPaneKeys(empty, [])).toBe(empty) + }) + + it('returns the existing record when the additions already form its tail in order', () => { + const existing = keyRecord(['a', 'b', 'c']) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['c'])).toBe(existing) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['b', 'c'])).toBe(existing) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['a', 'b', 'c'])).toBe(existing) + }) + + // Why: LRU order decides which key the cap evicts next. A key-set match is not a + // no-op when the re-added key is not already at the tail — it must move there. + it('re-retiring an existing non-tail key changes identity and moves it to the tail', () => { + const existing = keyRecord(['a', 'b', 'c']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['a']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['b', 'c', 'a']) + expect(Object.keys(existing)).toEqual(['a', 'b', 'c']) + }) + + it('tail keys re-added in a different relative order are rebuilt in the new order', () => { + const existing = keyRecord(['a', 'b', 'c']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['c', 'b']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['a', 'c', 'b']) + }) + + it('appends new keys after the existing ones', () => { + const existing = keyRecord(['a']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['b', 'c']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['a', 'b', 'c']) + }) + + it('evicts the oldest keys once the cap is exceeded', () => { + const full = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, 'k') + const next = boundRecentlyRetiredAgentStatusPaneKeys(full, ['fresh']) + const keys = Object.keys(next) + expect(keys).toHaveLength(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) + expect(keys[0]).toBe('k1') + expect(keys.at(-1)).toBe('fresh') + expect(next.k0).toBeUndefined() + }) + + it('re-retiring the oldest key at the cap keeps it fenced and evicts the next oldest', () => { + const full = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, 'k') + const bumped = boundRecentlyRetiredAgentStatusPaneKeys(full, ['k0']) + expect(bumped).not.toBe(full) + expect(Object.keys(bumped).at(-1)).toBe('k0') + const afterFresh = boundRecentlyRetiredAgentStatusPaneKeys(bumped, ['fresh']) + expect(afterFresh.k0).toBe(true) + expect(afterFresh.k1).toBeUndefined() + }) + + it('never returns an over-cap record unchanged', () => { + const over = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX + 1, 'k') + const last = `k${RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX}` + const next = boundRecentlyRetiredAgentStatusPaneKeys(over, [last]) + expect(next).not.toBe(over) + expect(Object.keys(next)).toHaveLength(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) + expect(next.k0).toBeUndefined() + }) +}) + +describe('boundRecentlyClosedAgentStatusTabIds', () => { + it('returns the existing record when the tab is already the most recent', () => { + const existing = keyRecord(['t1', 't2']) + expect(boundRecentlyClosedAgentStatusTabIds(existing, 't2')).toBe(existing) + }) + + it('moves a re-closed tab to the tail', () => { + const existing = keyRecord(['t1', 't2']) + const next = boundRecentlyClosedAgentStatusTabIds(existing, 't1') + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['t2', 't1']) + }) + + it('evicts the oldest tab once the cap is exceeded', () => { + const full = fullRecord(RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX, 't') + const next = boundRecentlyClosedAgentStatusTabIds(full, 'fresh') + expect(Object.keys(next)).toHaveLength(RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) + expect(next.t0).toBeUndefined() + expect(next.fresh).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts index af5cc4e83c7..f9199140c07 100644 --- a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts @@ -3,47 +3,66 @@ export const RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX = 1024 // delete-then-set for LRU recency, then evict oldest keys past the cap (Record iterates // insertion order); safe because a status for a tab closed >MAX tabs ago cannot still arrive. -export function boundRecentlyClosedAgentStatusTabIds( +function boundLruKeyRecord( existing: Record, - tabId: string + additions: ReadonlySet, + max: number ): Record { - const next: Record = {} - for (const key of Object.keys(existing)) { - if (key !== tabId) { - next[key] = true - } + if (isLruKeyRecordUnchanged(existing, additions, max)) { + return existing } - next[tabId] = true - const keys = Object.keys(next) - if (keys.length > RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) { - for (const stale of keys.slice(0, keys.length - RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX)) { - delete next[stale] - } - } - return next -} - -export function boundRecentlyRetiredAgentStatusPaneKeys( - existing: Record, - paneKeys: readonly string[] -): Record { - const additions = new Set(paneKeys) const next: Record = {} for (const key of Object.keys(existing)) { if (!additions.has(key)) { next[key] = true } } - for (const paneKey of additions) { - next[paneKey] = true + for (const key of additions) { + next[key] = true } const keys = Object.keys(next) - for (const stale of keys.slice(0, -RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX)) { + for (const stale of keys.slice(0, -max)) { delete next[stale] } return next } +// The rebuild is a no-op only when nothing would be evicted and the additions are +// already the tail of `existing` in that same relative order. A matching key SET is +// not enough: re-adding a key moves it to the tail, and that order decides which key +// the cap evicts next, so a stale-order hit would un-fence a recently retired pane. +function isLruKeyRecordUnchanged( + existing: Record, + additions: ReadonlySet, + max: number +): boolean { + const keys = Object.keys(existing) + if (keys.length > max || additions.size > keys.length) { + return false + } + let index = keys.length - additions.size + for (const key of additions) { + if (keys[index++] !== key) { + return false + } + } + return true +} + +export function boundRecentlyClosedAgentStatusTabIds( + existing: Record, + tabId: string +): Record { + return boundLruKeyRecord(existing, new Set([tabId]), RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) +} + +export function boundRecentlyRetiredAgentStatusPaneKeys( + existing: Record, + paneKeys: readonly string[] +): Record { + return boundLruKeyRecord(existing, new Set(paneKeys), RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) +} + export function movePaneKeyedRecord( record: Record, fromPaneKey: string, From 373514ef2678410f3ea805fd3600e62778ed202e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:33:55 -0700 Subject: [PATCH 4/5] perf(worktrees): stop worktree teardown replacing arrays and maps it never touched (#19145) * perf(worktrees): stop worktree teardown replacing arrays and maps it never touched Removing a worktree fires three store writes through removed-worktree-renderer-teardown.ts, and each handed back a fresh reference even when it removed nothing: - remove-worktree-store-cleanup filtered openFiles unconditionally. #19058 gave the ~50 record maps in this file identity preservation and missed the one plain array; the sibling purge path already had the guard this copies. openFiles is selected whole by the editor panel, file explorer and git-status polling. - shutdownWorktreeBrowsers spread-then-deleted browserTabsByWorktree and activeBrowserTabIdByWorktree; both now go through omitRecordKeys. - markShutdownPending rebuilt suppressedPtyExitIds and pendingPtyShutdownIds even with no guard ids at all, which is the normal case when the panes already exited. It now returns early, and skips the suppressed map when every id is already true. Same contents, same keys removed; only the reference is reused when nothing changed. * fix(test): use AppState['openFiles'][number] instead of a nonexistent module The test imported OpenFile from shared/editor-types, which does not exist. Vitest passed because a type-only import is erased at runtime; CI typecheck caught it. I had run tsc before adding this file and never re-ran it. * refactor(terminals): reuse copyOnWriteRecord in markShutdownPending and pin its identity contract --- .../slices/browser/browser-close-actions.ts | 14 ++-- .../teardown/remove-worktree-store-cleanup.ts | 8 ++- .../worktree-teardown-array-identity.test.ts | 58 +++++++++++++++ .../terminal-shutdown-guards-identity.test.ts | 72 +++++++++++++++++++ .../terminals/terminal-shutdown-guards.ts | 19 +++-- 5 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts create mode 100644 src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts diff --git a/src/renderer/src/store/slices/browser/browser-close-actions.ts b/src/renderer/src/store/slices/browser/browser-close-actions.ts index c70b93f6eb2..3aa77f9e46c 100644 --- a/src/renderer/src/store/slices/browser/browser-close-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-close-actions.ts @@ -12,6 +12,7 @@ import { getFallbackTabTypeForWorktree, isLocalBrowserPageOwner } from './browse import { closeRemoteBrowserPageInOwningEnvironment } from './browser-remote-close' import { releaseDocPreviewGrant } from '@/lib/doc-preview-grants' import { destroyWorkspaceWebviews } from '../browser-webview-cleanup' +import { omitRecordKeys } from '../worktrees/teardown/record-key-omission' export function createBrowserCloseActions( set: BrowserSliceSet, @@ -231,10 +232,15 @@ export function createBrowserCloseActions( destroyWorkspaceWebviews(browserPagesByWorkspace, workspace.id) } set((s) => { - const nextBrowserTabsByWorktree = { ...s.browserTabsByWorktree } - delete nextBrowserTabsByWorktree[worktreeId] - const nextActiveBrowserTabIdByWorktree = { ...s.activeBrowserTabIdByWorktree } - delete nextActiveBrowserTabIdByWorktree[worktreeId] + const removedWorktreeIds = [worktreeId] + const nextBrowserTabsByWorktree = omitRecordKeys( + s.browserTabsByWorktree, + removedWorktreeIds + ) + const nextActiveBrowserTabIdByWorktree = omitRecordKeys( + s.activeBrowserTabIdByWorktree, + removedWorktreeIds + ) // Why: reset the global browser surface only when the shut-down worktree is the active one AND had tabs. const shouldResetGlobalBrowser = s.activeWorktreeId === worktreeId && hadBrowserTabs return { diff --git a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts index 09ab88fdfbc..415697b883e 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts @@ -35,6 +35,12 @@ export function applyRemoveWorktreeSuccessState( } } const omitByFileId = (m: Record | undefined) => omitRecordKeys(m, removedFileIds) + // Why guarded: a removed worktree usually has no open file, and an unconditional + // filter would hand openFiles a new identity anyway — the sibling purge path + // already does this. + const nextOpenFiles = s.openFiles.some((f) => f.worktreeId === worktreeId) + ? s.openFiles.filter((f) => f.worktreeId !== worktreeId) + : s.openFiles // If the active file belonged to the removed worktree, clear it const activeFileCleared = s.activeFileId ? s.openFiles.some((f) => f.id === s.activeFileId && f.worktreeId === worktreeId) @@ -79,7 +85,7 @@ export function applyRemoveWorktreeSuccessState( ? null : s.activeWorkspaceExecutionHostId, activeTabId: s.activeTabId && tabIds.has(s.activeTabId) ? null : s.activeTabId, - openFiles: s.openFiles.filter((f) => f.worktreeId !== worktreeId), + openFiles: nextOpenFiles, browserTabsByWorktree: omitByWorktree(s.browserTabsByWorktree), // Why: closeBrowserTab records a Cmd+Shift+T undo snapshot, but a deleted worktree's tabs can't be restored; purge it. recentlyClosedBrowserTabsByWorktree: omitByWorktree(s.recentlyClosedBrowserTabsByWorktree), diff --git a/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts b/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts new file mode 100644 index 00000000000..5e122d9dbef --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '../../../types' +import { applyRemoveWorktreeSuccessState } from './remove-worktree-store-cleanup' + +type OpenFile = AppState['openFiles'][number] + +const REMOVED = 'repo-1::/repos/one/removed' +const KEPT = 'repo-1::/repos/one/kept' + +function fileFor(worktreeId: string, id: string): OpenFile { + return { id, worktreeId, path: `${worktreeId}/f.ts`, name: 'f.ts' } as unknown as OpenFile +} + +function buildState(openFiles: OpenFile[]): AppState { + return { + worktreesByRepo: { 'repo-1': [] }, + tabsByWorktree: { [KEPT]: [] }, + openFiles, + everActivatedWorktreeIds: new Set(), + lastVisitedAtByWorktreeId: {}, + deleteStateByWorktreeId: {}, + sortEpoch: 0 + } as unknown as AppState +} + +function removeWorktree(state: AppState): AppState { + let current = state + applyRemoveWorktreeSuccessState( + (update) => { + const patch = typeof update === 'function' ? update(current) : update + current = { ...current, ...patch } + }, + REMOVED, + new Set() + ) + return current +} + +describe('worktree removal openFiles identity', () => { + it('keeps the openFiles reference when the removed worktree had no open file', () => { + // openFiles is selected whole by the editor panel, file explorer and git-status + // polling, so a fresh array here rerenders all of them for no data change. + const before = buildState([fileFor(KEPT, 'kept-file')]) + + const after = removeWorktree(before) + + expect(after.openFiles).toBe(before.openFiles) + }) + + it('still drops the removed worktree files', () => { + const before = buildState([fileFor(KEPT, 'kept-file'), fileFor(REMOVED, 'gone-file')]) + + const after = removeWorktree(before) + + expect(after.openFiles).not.toBe(before.openFiles) + expect(after.openFiles.map((f) => f.id)).toEqual(['kept-file']) + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts b/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts new file mode 100644 index 00000000000..c353991c2c1 --- /dev/null +++ b/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import { createTerminalShutdownGuardController } from './terminal-shutdown-guards' + +vi.mock('@/components/terminal-pane/pty-transport', () => ({ + restorePtyDataHandlersAfterFailedShutdown: vi.fn(), + unregisterPtyDataHandlers: vi.fn(() => []) +})) +vi.mock('@/components/terminal-pane/terminal-parked-watcher-registry', () => ({ + disposeParkedTerminalWatchersForPtyIds: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-shutdown-exit-deferral', () => ({ + clearCommittedPtyShutdownSettlements: vi.fn(), + hasCommittedPtyShutdownSettlement: vi.fn(() => false), + markCommittedPtyShutdowns: vi.fn(), + noteCommittedPtyShutdownSettlements: vi.fn(), + settleDeferredPtyShutdownExits: vi.fn() +})) + +function harness(initial: Partial, exitGuardPtyIds: readonly string[]) { + let current = initial as AppState + const set = vi.fn((update: unknown) => { + const patch = + typeof update === 'function' ? (update as (s: AppState) => object)(current) : update + current = { ...current, ...(patch as object) } + }) + const guards = createTerminalShutdownGuardController({ + exitGuardPtyIds, + get: (() => current) as never, + keepIdentifiers: false, + rendererShutdownPtyIds: exitGuardPtyIds, + runtimeEnvironmentId: null, + set: set as never, + tabs: [] + }) + return { guards, set, state: () => current } +} + +describe('markShutdownPending identity', () => { + it('does not write the store when there is nothing to guard', () => { + const { guards, set } = harness({ suppressedPtyExitIds: {}, pendingPtyShutdownIds: {} }, []) + + guards.markShutdownPending() + + expect(set).not.toHaveBeenCalled() + }) + + it('still counts a pending owner when every id is already suppressed', () => { + const suppressedPtyExitIds: Record = { 'pty-1': true } + const { guards, state } = harness( + { suppressedPtyExitIds, pendingPtyShutdownIds: { 'pty-1': 1 } }, + ['pty-1'] + ) + + guards.markShutdownPending() + + expect(state().suppressedPtyExitIds).toBe(suppressedPtyExitIds) + expect(state().pendingPtyShutdownIds).toEqual({ 'pty-1': 2 }) + }) + + it('suppresses the ids that were not yet suppressed', () => { + const { guards, state } = harness( + { suppressedPtyExitIds: { 'pty-1': true }, pendingPtyShutdownIds: {} }, + ['pty-1', 'pty-2'] + ) + + guards.markShutdownPending() + + expect(state().suppressedPtyExitIds).toEqual({ 'pty-1': true, 'pty-2': true }) + expect(state().pendingPtyShutdownIds).toEqual({ 'pty-1': 1, 'pty-2': 1 }) + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-shutdown-guards.ts b/src/renderer/src/store/terminals/terminal-shutdown-guards.ts index 83342bd5041..0eba0fda927 100644 --- a/src/renderer/src/store/terminals/terminal-shutdown-guards.ts +++ b/src/renderer/src/store/terminals/terminal-shutdown-guards.ts @@ -13,6 +13,7 @@ import { settleDeferredPtyShutdownExits } from '@/components/terminal-pane/pty-shutdown-exit-deferral' import type { TerminalStoreGet, TerminalStoreSet } from './terminal-state' +import { copyOnWriteRecord } from '../copy-on-write-record' export type TerminalShutdownGuardController = { commitHandlerSnapshots: () => void @@ -48,18 +49,22 @@ export function createTerminalShutdownGuardController({ let partialRendererStopSettled = false const markShutdownPending = (): void => { + // Why the early return: tearing down a worktree whose panes already exited passes + // no guard ids, and the spreads below would still hand both maps a new identity. + if (exitGuardPtyIds.length === 0) { + return + } set((state) => { const pendingPtyShutdownIds = { ...state.pendingPtyShutdownIds } + // Why copy-on-write: re-guarding an already-suppressed pty writes the same `true`. + const suppressedPtyExitIds = copyOnWriteRecord(state.suppressedPtyExitIds) for (const ptyId of exitGuardPtyIds) { pendingPtyShutdownIds[ptyId] = (pendingPtyShutdownIds[ptyId] ?? 0) + 1 + if (state.suppressedPtyExitIds[ptyId] !== true) { + suppressedPtyExitIds.set(ptyId, true) + } } - return { - suppressedPtyExitIds: { - ...state.suppressedPtyExitIds, - ...Object.fromEntries(exitGuardPtyIds.map((ptyId) => [ptyId, true] as const)) - }, - pendingPtyShutdownIds - } + return { suppressedPtyExitIds: suppressedPtyExitIds.read(), pendingPtyShutdownIds } }) } From 463cab2f71bc3156443a963ebe793f058c8a3905 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 6 Sep 2026 18:40:43 -0700 Subject: [PATCH 5/5] fix(cmd-j): remove duplicate browser ownership inputs (#19172) --- .../src/components/use-worktree-jump-palette-open-tabs.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts b/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts index 42630be7116..d6c62711670 100644 --- a/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts +++ b/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts @@ -93,7 +93,6 @@ export function useWorktreeJumpPaletteOpenTabs({ worktreeOrder, browserTabsByWorktree, browserPagesByWorkspace, - unifiedTabsByWorktree, activeBrowserTabId, activeWorktreeId, activeWorkspaceExecutionHostId, @@ -110,7 +109,6 @@ export function useWorktreeJumpPaletteOpenTabs({ browserPagesByWorkspace, browserTabsByWorktree, browserSortedWorktrees, - unifiedTabsByWorktree, repoByHostIdentity, repoMap, unifiedTabsByWorktree,