mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
Merge current main application review fixes
This commit is contained in:
@@ -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<typeof execFile>
|
||||
})
|
||||
}
|
||||
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()
|
||||
})
|
||||
})
|
||||
}
|
||||
@@ -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<unknown> {
|
||||
if (!isMissingWsl2KernelStatus(error)) {
|
||||
return error
|
||||
}
|
||||
try {
|
||||
return (await runProcess(defaultGuestExecutionProbe())).code === 0 ? null : error
|
||||
} catch {
|
||||
return error
|
||||
}
|
||||
}
|
||||
|
||||
function probeWslStatus(): Promise<void> {
|
||||
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<boolean> {
|
||||
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
|
||||
})
|
||||
|
||||
@@ -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(<WorkspacePortScanner />)
|
||||
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(<WorkspacePortScanner />)
|
||||
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<WorkspacePortScanResult>((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()
|
||||
}
|
||||
})
|
||||
|
||||
@@ -280,6 +280,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }):
|
||||
return
|
||||
}
|
||||
|
||||
let burstRefresh: Promise<void> | null = null
|
||||
let eventSequence = 0
|
||||
let disposed = false
|
||||
let retryTimer: ReturnType<typeof setTimeout> | 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
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<string, true> {
|
||||
const record: Record<string, true> = {}
|
||||
for (const key of keys) {
|
||||
record[key] = true
|
||||
}
|
||||
return record
|
||||
}
|
||||
|
||||
function fullRecord(max: number, prefix: string): Record<string, true> {
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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<string, true>,
|
||||
tabId: string
|
||||
additions: ReadonlySet<string>,
|
||||
max: number
|
||||
): Record<string, true> {
|
||||
const next: Record<string, true> = {}
|
||||
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<string, true>,
|
||||
paneKeys: readonly string[]
|
||||
): Record<string, true> {
|
||||
const additions = new Set(paneKeys)
|
||||
const next: Record<string, true> = {}
|
||||
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<string, true>,
|
||||
additions: ReadonlySet<string>,
|
||||
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<string, true>,
|
||||
tabId: string
|
||||
): Record<string, true> {
|
||||
return boundLruKeyRecord(existing, new Set([tabId]), RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX)
|
||||
}
|
||||
|
||||
export function boundRecentlyRetiredAgentStatusPaneKeys(
|
||||
existing: Record<string, true>,
|
||||
paneKeys: readonly string[]
|
||||
): Record<string, true> {
|
||||
return boundLruKeyRecord(existing, new Set(paneKeys), RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX)
|
||||
}
|
||||
|
||||
export function movePaneKeyedRecord<T>(
|
||||
record: Record<string, T>,
|
||||
fromPaneKey: string,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -35,6 +35,12 @@ export function applyRemoveWorktreeSuccessState(
|
||||
}
|
||||
}
|
||||
const omitByFileId = <T>(m: Record<string, T> | 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),
|
||||
|
||||
+58
@@ -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<string>(),
|
||||
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<string>()
|
||||
)
|
||||
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'])
|
||||
})
|
||||
})
|
||||
@@ -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<AppState>, 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<string, true> = { '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 })
|
||||
})
|
||||
})
|
||||
@@ -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 }
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user