mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 00:03:20 +00:00
fix(windows): close terminal shutdown safety races
This commit is contained in:
@@ -26,6 +26,12 @@ function buildSessionIds(prefix: string, count: number): string[] {
|
||||
return ids
|
||||
}
|
||||
|
||||
function stubProcessPlatform(platform: NodeJS.Platform): () => void {
|
||||
const original = Object.getOwnPropertyDescriptor(process, 'platform')!
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
|
||||
return () => Object.defineProperty(process, 'platform', original)
|
||||
}
|
||||
|
||||
function createAdapter(
|
||||
label: string,
|
||||
sessions: string[] = [],
|
||||
@@ -145,6 +151,48 @@ describe('DaemonPtyRouter', () => {
|
||||
expect(current.shutdown).toHaveBeenCalledWith('current-session', { immediate: true })
|
||||
})
|
||||
|
||||
it.each(['darwin', 'linux'] as const)(
|
||||
'does not let a dead legacy daemon strand known current sessions on %s',
|
||||
async (platform) => {
|
||||
const restorePlatform = stubProcessPlatform(platform)
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['legacy-session'])
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
|
||||
try {
|
||||
await router.discoverLegacySessions()
|
||||
await router.spawn({ sessionId: 'current-close', cols: 80, rows: 24 })
|
||||
await router.spawn({ sessionId: 'current-sleep', cols: 80, rows: 24 })
|
||||
vi.mocked(legacy.listProcesses).mockRejectedValue(new Error('legacy unavailable'))
|
||||
|
||||
await expect(router.shutdown('current-close', { immediate: true })).resolves.toBeUndefined()
|
||||
await expect(
|
||||
router.shutdown('current-sleep', { immediate: true, keepHistory: true })
|
||||
).resolves.toBeUndefined()
|
||||
current.emitExit('current-sleep', 0)
|
||||
await router.spawn({ sessionId: 'current-sleep', cols: 80, rows: 24 })
|
||||
router.write('legacy-session', 'legacy still routed\n')
|
||||
|
||||
expect(legacy.listProcesses).toHaveBeenCalledOnce()
|
||||
expect(current.shutdown).toHaveBeenCalledWith('current-close', { immediate: true })
|
||||
expect(current.shutdown).toHaveBeenCalledWith('current-sleep', {
|
||||
immediate: true,
|
||||
keepHistory: true
|
||||
})
|
||||
expect(legacy.shutdown).not.toHaveBeenCalled()
|
||||
expect(current.spawn).toHaveBeenLastCalledWith({
|
||||
sessionId: 'current-sleep',
|
||||
cols: 80,
|
||||
rows: 24
|
||||
})
|
||||
expect(legacy.spawn).not.toHaveBeenCalled()
|
||||
expect(legacy.write).toHaveBeenCalledWith('legacy-session', 'legacy still routed\n')
|
||||
} finally {
|
||||
restorePlatform()
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
it('preflights against the authoritative legacy owner', async () => {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['legacy-session'])
|
||||
@@ -447,24 +495,29 @@ describe('DaemonPtyRouter', () => {
|
||||
})
|
||||
|
||||
it('inventories a hidden legacy owner before cached current shutdown', async () => {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['duplicate-session'])
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
const restorePlatform = stubProcessPlatform('win32')
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
vi.mocked(legacy.hasPty).mockReturnValue(false)
|
||||
vi.mocked(legacy.listProcesses).mockRejectedValueOnce(new Error('legacy still starting'))
|
||||
await router.discoverLegacySessions()
|
||||
await router.spawn({ sessionId: 'duplicate-session', cols: 80, rows: 24 })
|
||||
const deadlineMs = Date.now() + 1000
|
||||
try {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['duplicate-session'])
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
vi.mocked(legacy.hasPty).mockReturnValue(false)
|
||||
vi.mocked(legacy.listProcesses).mockRejectedValueOnce(new Error('legacy still starting'))
|
||||
await router.discoverLegacySessions()
|
||||
await router.spawn({ sessionId: 'duplicate-session', cols: 80, rows: 24 })
|
||||
const deadlineMs = Date.now() + 1000
|
||||
|
||||
await expect(
|
||||
router.shutdown('duplicate-session', { immediate: true, deadlineMs })
|
||||
).rejects.toThrow('Ambiguous PTY session ownership')
|
||||
expect(current.listProcesses).toHaveBeenLastCalledWith({ deadlineMs })
|
||||
expect(legacy.listProcesses).toHaveBeenLastCalledWith({ deadlineMs })
|
||||
expect(current.shutdown).not.toHaveBeenCalled()
|
||||
expect(legacy.shutdown).not.toHaveBeenCalled()
|
||||
warn.mockRestore()
|
||||
await expect(
|
||||
router.shutdown('duplicate-session', { immediate: true, deadlineMs })
|
||||
).rejects.toThrow('Ambiguous PTY session ownership')
|
||||
expect(current.listProcesses).toHaveBeenLastCalledWith({ deadlineMs })
|
||||
expect(legacy.listProcesses).toHaveBeenLastCalledWith({ deadlineMs })
|
||||
expect(current.shutdown).not.toHaveBeenCalled()
|
||||
expect(legacy.shutdown).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
restorePlatform()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps duplicate ownership ambiguous when one of three owners exits', async () => {
|
||||
|
||||
@@ -108,7 +108,7 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
id: string,
|
||||
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
|
||||
): Promise<void> {
|
||||
if (this.legacy.length > 0) {
|
||||
if (this.requiresInventoryBeforeTargetedShutdown(id)) {
|
||||
// Why: a recovered legacy daemon may own an id absent from hasPty's local
|
||||
// cache; await authoritative inventories before selecting an adapter.
|
||||
await this.refreshInventories({ deadlineMs: opts.deadlineMs })
|
||||
@@ -140,7 +140,7 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
}
|
||||
|
||||
async getShutdownBlockReason(id: string): Promise<PtyShutdownBlockReason | null> {
|
||||
if (this.legacy.length > 0) {
|
||||
if (this.requiresInventoryBeforeTargetedShutdown(id)) {
|
||||
// Why: preflight must select the same authoritative legacy owner as shutdown.
|
||||
await this.refreshInventories()
|
||||
}
|
||||
@@ -355,6 +355,13 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
return this.sessionRouting.get(sessionId) ?? this.current
|
||||
}
|
||||
|
||||
private requiresInventoryBeforeTargetedShutdown(sessionId: string): boolean {
|
||||
// Why: Windows legacy sessions need a fresh ambiguity check before teardown;
|
||||
// on POSIX, a known route must not be stranded by an unrelated dead daemon.
|
||||
const hasLegacy = this.legacy.length > 0
|
||||
return hasLegacy && (process.platform === 'win32' || !this.sessionRouting.get(sessionId))
|
||||
}
|
||||
|
||||
private refreshInventories(opts?: { deadlineMs?: number }): Promise<{
|
||||
results: PtyProcessInfo[][]
|
||||
ambiguousIds: string[]
|
||||
@@ -362,9 +369,8 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
return this.sessionRouting.refreshInventories(this.allAdapters(), opts)
|
||||
}
|
||||
|
||||
private ambiguousOwnershipError(sessionIds: string[]): Error {
|
||||
return new Error(`Ambiguous PTY session ownership across daemons: ${sessionIds.join(', ')}`)
|
||||
}
|
||||
private ambiguousOwnershipError = (sessionIds: string[]): Error =>
|
||||
new Error(`Ambiguous PTY session ownership across daemons: ${sessionIds.join(', ')}`)
|
||||
|
||||
private allAdapters(): DaemonPtyAdapter[] {
|
||||
return [this.current, ...this.legacy]
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
|
||||
import { WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON } from '../../../../shared/pty-shutdown-safety'
|
||||
import { useAppStore, type AppState } from '@/store'
|
||||
@@ -101,6 +101,11 @@ describe('snapshotKillAllTerminalSurfaceIds', () => {
|
||||
})
|
||||
|
||||
describe('runKillAllTerminalSurfaces', () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal('navigator', { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' })
|
||||
delete (globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__
|
||||
})
|
||||
|
||||
it('preserves unsafe legacy tabs and mappings before daemon kill-all starts', async () => {
|
||||
const current = state({
|
||||
tabsByWorktree: {
|
||||
@@ -577,6 +582,50 @@ describe('runKillAllTerminalSurfaces', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a legacy PTY visible when it attaches during a renderer yield', async () => {
|
||||
let current = state({
|
||||
activeWorktreeId: 'wt',
|
||||
tabsByWorktree: {
|
||||
wt: [terminal('first', 'wt'), terminal('second', 'wt'), terminal('remaining', 'wt')]
|
||||
},
|
||||
ptyIdsByTabId: {
|
||||
first: ['pty-first'],
|
||||
second: ['pty-second'],
|
||||
remaining: ['pty-safe-before-yield']
|
||||
}
|
||||
})
|
||||
const getShutdownBlockReason = vi.fn(async (ptyId: string) =>
|
||||
ptyId === 'pty-v24' ? WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON : null
|
||||
)
|
||||
const closeSurface = vi.fn((targetId: string) => removeSurface(current, targetId))
|
||||
const killPty = vi.fn().mockResolvedValue(undefined)
|
||||
vi.stubGlobal('window', { api: { pty: { getShutdownBlockReason } } })
|
||||
|
||||
await expect(
|
||||
runKillAllTerminalSurfaces(['first', 'second', 'remaining'], {
|
||||
getState: () => current,
|
||||
killDaemonSessions: vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }),
|
||||
closeSurface,
|
||||
killPty,
|
||||
yieldToRenderer: vi.fn(async () => {
|
||||
current = state({
|
||||
activeWorktreeId: 'wt',
|
||||
tabsByWorktree: {
|
||||
wt: [{ ...terminal('remaining', 'wt'), ptyId: 'pty-v24' }]
|
||||
},
|
||||
ptyIdsByTabId: { remaining: ['pty-v24'] }
|
||||
})
|
||||
}),
|
||||
reportSummary: vi.fn()
|
||||
})
|
||||
).rejects.toThrow(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON)
|
||||
|
||||
expect(closeSurface.mock.calls.map(([targetId]) => targetId)).toEqual(['first', 'second'])
|
||||
expect(killPty).not.toHaveBeenCalledWith('pty-v24')
|
||||
expect(snapshotKillAllTerminalSurfaceIds(current)).toEqual(['remaining'])
|
||||
expect(current.ptyIdsByTabId.remaining).toEqual(['pty-v24'])
|
||||
})
|
||||
|
||||
it('records bounded fanout and two-close batches for 100 terminal tabs', async () => {
|
||||
const tabs = Array.from({ length: 100 }, (_, index) => terminal(`tab-${index}`, 'wt'))
|
||||
const ptyIdsByTabId = Object.fromEntries(tabs.map((tab, index) => [tab.id, [`pty-${index}`]]))
|
||||
|
||||
@@ -114,15 +114,6 @@ function getTargetIndex(
|
||||
return { ownerByTargetId, terminalIdsByWorktree }
|
||||
}
|
||||
|
||||
function getNextTerminalId(ids: ReadonlySet<string>, closingId: string): string | null {
|
||||
for (const id of ids) {
|
||||
if (id !== closingId) {
|
||||
return id
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function createDefaultDependencies(): KillAllTerminalSurfaceDependencies {
|
||||
return {
|
||||
getState: useAppStore.getState,
|
||||
@@ -228,7 +219,8 @@ export async function runKillAllTerminalSurfaces(
|
||||
const owningWorktreeId = closeWave.ownerByTargetId.get(targetId)!
|
||||
const remainingTerminalIds =
|
||||
closeWave.terminalIdsByWorktree.get(owningWorktreeId) ?? new Set<string>()
|
||||
const nextTerminalTabId = getNextTerminalId(remainingTerminalIds, targetId)
|
||||
const nextTerminalTabId =
|
||||
[...remainingTerminalIds].find((terminalId) => terminalId !== targetId) ?? null
|
||||
const { plan: retirementPlan, newlyScheduledPtyOwners } = reserveTerminalRetirementTeardowns(
|
||||
closeWave.state,
|
||||
closeWave.retirementPlans.get(targetId)!,
|
||||
@@ -285,8 +277,12 @@ export async function runKillAllTerminalSurfaces(
|
||||
closeBatchStartedAt = deps.now()
|
||||
const stateAfterYield = deps.getState()
|
||||
if (mustReplanAfterYield || stateAfterYield !== stateAfterBatch) {
|
||||
// Why: replanning after a mutable yield prevents killing a new PTY owner.
|
||||
closeWave = createCloseWave(stateAfterYield)
|
||||
// Why: a yield can attach an unsafe legacy owner; revalidate before
|
||||
// rebuilding the wave that will synchronously retire the next target.
|
||||
if (shouldPreflightTargets) {
|
||||
await assertTargetsSafe()
|
||||
}
|
||||
closeWave = createCloseWave(deps.getState())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,6 +47,8 @@ import { runSleepWorktree, runSleepWorktrees } from './sleep-worktree-flow'
|
||||
describe('runSleepWorktree', () => {
|
||||
beforeEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
vi.stubGlobal('navigator', { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' })
|
||||
delete (globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__
|
||||
vi.stubGlobal('window', {
|
||||
api: {
|
||||
ephemeralVm: {
|
||||
@@ -74,25 +76,22 @@ describe('runSleepWorktree', () => {
|
||||
mocks.state.pendingReconnectPtyIdByTabId = {}
|
||||
})
|
||||
|
||||
it('tears down browsers before terminals on the sleep path', async () => {
|
||||
it('starts checked terminal teardown before browsers on the sleep path', async () => {
|
||||
mocks.state.activeWorktreeId = 'wt-1'
|
||||
|
||||
await runSleepWorktree('wt-1')
|
||||
|
||||
// Why: browsers must run first so destroyPersistentWebview can unregister
|
||||
// the Chromium guests while browserTabsByWorktree/browserPagesByWorkspace
|
||||
// are still populated. If terminals ran first and kept its old
|
||||
// browserTabsByWorktree delete, browsers would no-op and leak webviews.
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-1')
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.suspendWorkspace).toHaveBeenCalledWith({ workspaceId: 'wt-1' })
|
||||
const browsersCallOrder = mocks.state.shutdownWorktreeBrowsers.mock.invocationCallOrder[0]
|
||||
const terminalsCallOrder = mocks.state.shutdownWorktreeTerminals.mock.invocationCallOrder[0]
|
||||
const suspendCallOrder = mocks.suspendWorkspace.mock.invocationCallOrder[0]
|
||||
expect(browsersCallOrder).toBeLessThan(terminalsCallOrder)
|
||||
expect(terminalsCallOrder).toBeLessThan(suspendCallOrder)
|
||||
expect(terminalsCallOrder).toBeLessThan(browsersCallOrder)
|
||||
expect(browsersCallOrder).toBeLessThan(suspendCallOrder)
|
||||
})
|
||||
|
||||
it('clears activeWorktreeId before teardown when the slept worktree is active', async () => {
|
||||
@@ -158,6 +157,39 @@ describe('runSleepWorktree', () => {
|
||||
expect(mocks.clearWorktreeSleepIntent).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('retires checked PTY ownership before browser teardown can replace it', async () => {
|
||||
const getShutdownBlockReason = vi.fn().mockResolvedValue(null)
|
||||
vi.stubGlobal('window', {
|
||||
api: {
|
||||
pty: { getShutdownBlockReason },
|
||||
ephemeralVm: { suspendWorkspace: mocks.suspendWorkspace }
|
||||
},
|
||||
requestAnimationFrame: vi.fn()
|
||||
})
|
||||
mocks.state.activeWorktreeId = 'wt-1'
|
||||
mocks.state.tabsByWorktree = { 'wt-1': [{ id: 'tab-1' }] }
|
||||
mocks.state.ptyIdsByTabId = { 'tab-1': ['pty-safe'] }
|
||||
mocks.state.shutdownWorktreeTerminals.mockImplementationOnce(async () => {
|
||||
mocks.state.ptyIdsByTabId = { 'tab-1': [] }
|
||||
})
|
||||
const unsafeReplacement = vi.fn(() => {
|
||||
if ((mocks.state.ptyIdsByTabId['tab-1'] ?? []).length > 0) {
|
||||
mocks.state.ptyIdsByTabId = { 'tab-1': ['pty-v24'] }
|
||||
}
|
||||
})
|
||||
mocks.state.shutdownWorktreeBrowsers.mockImplementationOnce(async () => unsafeReplacement())
|
||||
|
||||
await runSleepWorktree('wt-1')
|
||||
|
||||
expect(getShutdownBlockReason).toHaveBeenCalledTimes(1)
|
||||
expect(getShutdownBlockReason).toHaveBeenCalledWith('pty-safe')
|
||||
expect(unsafeReplacement).toHaveBeenCalledOnce()
|
||||
expect(mocks.state.ptyIdsByTabId['tab-1']).toEqual([])
|
||||
const terminalCall = mocks.state.shutdownWorktreeTerminals.mock.invocationCallOrder[0]
|
||||
const browserCall = mocks.state.shutdownWorktreeBrowsers.mock.invocationCallOrder[0]
|
||||
expect(terminalCall).toBeLessThan(browserCall)
|
||||
})
|
||||
|
||||
it('preserves active row position through section-scoped sidebar row ids', async () => {
|
||||
const requestAnimationFrame = vi.fn(() => 1)
|
||||
const scroller = {
|
||||
@@ -239,7 +271,7 @@ describe('runSleepWorktree', () => {
|
||||
expect(mocks.markWorktreeSleepIntent).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('surfaces a toast and skips terminals when browsers throws', async () => {
|
||||
it('surfaces a toast after a post-terminal browser failure', async () => {
|
||||
mocks.state.activeWorktreeId = 'wt-1'
|
||||
mocks.state.shutdownWorktreeBrowsers.mockRejectedValueOnce(new Error('boom'))
|
||||
mocks.state.tabsByWorktree = { 'wt-1': [{ id: 'tab-1' }] }
|
||||
@@ -247,7 +279,10 @@ describe('runSleepWorktree', () => {
|
||||
|
||||
await runSleepWorktree('wt-1')
|
||||
|
||||
expect(mocks.state.shutdownWorktreeTerminals).not.toHaveBeenCalled()
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.suspendWorkspace).not.toHaveBeenCalled()
|
||||
expect(mocks.clearWorktreeSleepIntent).toHaveBeenCalledWith('wt-1')
|
||||
expect(mocks.toastError).toHaveBeenCalledWith(
|
||||
@@ -266,12 +301,14 @@ describe('runSleepWorktree', () => {
|
||||
|
||||
await runSleepWorktrees(['wt-1', 'wt-2'])
|
||||
|
||||
expect(mocks.state.shutdownWorktreeTerminals).not.toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-2')
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-2', {
|
||||
keepIdentifiers: true
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.suspendWorkspace).toHaveBeenCalledWith({ workspaceId: 'wt-2' })
|
||||
expect(mocks.toastError).toHaveBeenCalledWith(
|
||||
@@ -289,11 +326,13 @@ describe('runSleepWorktree', () => {
|
||||
expect(mocks.state.setActiveWorktree).toHaveBeenCalledWith(null)
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(1, 'wt-1')
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenNthCalledWith(1, 'wt-1', {
|
||||
keepIdentifiers: true
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(2, 'wt-2')
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenNthCalledWith(2, 'wt-2', {
|
||||
keepIdentifiers: true
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
expect(mocks.suspendWorkspace).toHaveBeenNthCalledWith(1, { workspaceId: 'wt-1' })
|
||||
expect(mocks.suspendWorkspace).toHaveBeenNthCalledWith(2, { workspaceId: 'wt-2' })
|
||||
|
||||
@@ -114,50 +114,41 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise
|
||||
if (worktreeIds.length === 0) {
|
||||
return
|
||||
}
|
||||
const worktreeIdSet = new Set(worktreeIds)
|
||||
try {
|
||||
// Why: a legacy refusal must leave activation, browsers, and sleep intent untouched.
|
||||
await assertStableTerminalShutdownSafety({
|
||||
surfaceId: worktreeIds.join(':'),
|
||||
getState: useAppStore.getState,
|
||||
selectTabIds: (state) => selectTerminalTabIdsForWorktrees(state, worktreeIdSet)
|
||||
})
|
||||
} catch {
|
||||
// The safety preflight already explains why the workspace stayed awake.
|
||||
return
|
||||
}
|
||||
const {
|
||||
activeWorktreeId,
|
||||
setActiveWorktree,
|
||||
shutdownWorktreeBrowsers,
|
||||
shutdownWorktreeTerminals
|
||||
} = useAppStore.getState()
|
||||
let activeSleepIntentWorktreeId: string | null = null
|
||||
if (activeWorktreeId && worktreeIds.includes(activeWorktreeId)) {
|
||||
const restoreSidebarPosition = preserveSidebarWorktreePosition(activeWorktreeId)
|
||||
// Why: clearing the active workspace can unmount TerminalPanes before
|
||||
// shutdownWorktreeTerminals writes PTY suppressions. Use a non-rendering
|
||||
// intent marker so those exits do not stamp activity, without inserting an
|
||||
// extra Zustand update that can disturb the sidebar's scroll restoration.
|
||||
markWorktreeSleepIntent(activeWorktreeId)
|
||||
activeSleepIntentWorktreeId = activeWorktreeId
|
||||
setActiveWorktree(null)
|
||||
restoreSidebarPosition()
|
||||
}
|
||||
const errors: string[] = []
|
||||
try {
|
||||
for (const worktreeId of worktreeIds) {
|
||||
try {
|
||||
// Why: sleep mirrors removeWorktree's shutdown sequence — browsers first
|
||||
// so destroyPersistentWebview unregisters the Chromium guests before any
|
||||
// other teardown runs, terminals second so the PTY kill uses the same
|
||||
// ordering on both paths. Without the browser thunk here, sleep leaks
|
||||
// browserPagesByWorkspace entries and live webviews for the slept worktree.
|
||||
await shutdownWorktreeBrowsers(worktreeId)
|
||||
} catch (err) {
|
||||
errors.push(err instanceof Error ? err.message : String(err))
|
||||
// Why: verify each worktree immediately before its first mutation; an
|
||||
// earlier worktree's async teardown must not stale this ownership answer.
|
||||
await assertStableTerminalShutdownSafety({
|
||||
surfaceId: worktreeId,
|
||||
getState: useAppStore.getState,
|
||||
selectTabIds: (state) => selectTerminalTabIdsForWorktrees(state, new Set([worktreeId]))
|
||||
})
|
||||
} catch {
|
||||
// The safety preflight already explains why this workspace stayed awake.
|
||||
continue
|
||||
}
|
||||
|
||||
const {
|
||||
activeWorktreeId,
|
||||
setActiveWorktree,
|
||||
shutdownWorktreeBrowsers,
|
||||
shutdownWorktreeTerminals
|
||||
} = useAppStore.getState()
|
||||
if (activeWorktreeId === worktreeId) {
|
||||
const restoreSidebarPosition = preserveSidebarWorktreePosition(activeWorktreeId)
|
||||
// Why: clearing the active workspace can unmount TerminalPanes before
|
||||
// shutdownWorktreeTerminals writes PTY suppressions. Use a non-rendering
|
||||
// intent marker so those exits do not stamp activity, without inserting an
|
||||
// extra Zustand update that can disturb the sidebar's scroll restoration.
|
||||
markWorktreeSleepIntent(activeWorktreeId)
|
||||
activeSleepIntentWorktreeId = activeWorktreeId
|
||||
setActiveWorktree(null)
|
||||
restoreSidebarPosition()
|
||||
}
|
||||
|
||||
try {
|
||||
// Why: sleep is reversible — the tab record stays in tabsByWorktree, the
|
||||
// layout stays in terminalLayoutsByTabId, only the live PTY processes are
|
||||
@@ -166,7 +157,15 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise
|
||||
// history dir (local) or relay session id (SSH); it also captures
|
||||
// serializer buffers into buffersByLeafId for SSH wake to reseed
|
||||
// scrollback. See DESIGN_DOC_TERMINAL_HISTORY_FIX_V2.md §3.3.c.
|
||||
await shutdownWorktreeTerminals(worktreeId, { keepIdentifiers: true })
|
||||
// Why: start terminal retirement in the same renderer turn as the stable
|
||||
// preflight; browser teardown awaits and can otherwise attach a legacy PTY.
|
||||
await shutdownWorktreeTerminals(worktreeId, {
|
||||
keepIdentifiers: true,
|
||||
shutdownSafetyChecked: true
|
||||
})
|
||||
// Why: terminal shutdown no longer mutates browser maps, so Chromium
|
||||
// guests can be removed afterward without creating a PTY safety gap.
|
||||
await shutdownWorktreeBrowsers(worktreeId)
|
||||
if (typeof window !== 'undefined' && window.api?.ephemeralVm?.suspendWorkspace) {
|
||||
await window.api.ephemeralVm.suspendWorkspace({ workspaceId: worktreeId })
|
||||
}
|
||||
|
||||
@@ -19,6 +19,13 @@ type StableTerminalShutdownPlan = {
|
||||
function getShutdownSafetyPreflight():
|
||||
| ((id: string) => ReturnType<Window['api']['pty']['getShutdownBlockReason']>)
|
||||
| undefined {
|
||||
const isNativeWindowsRenderer =
|
||||
typeof navigator !== 'undefined' &&
|
||||
navigator.userAgent.includes('Windows') &&
|
||||
(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ !== true
|
||||
if (!isNativeWindowsRenderer) {
|
||||
return undefined
|
||||
}
|
||||
return (globalThis.window?.api?.pty as Partial<Window['api']['pty']> | undefined)
|
||||
?.getShutdownBlockReason
|
||||
}
|
||||
|
||||
@@ -60,6 +60,8 @@ function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
|
||||
describe('terminal tab retirement store boundary', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
vi.stubGlobal('navigator', { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' })
|
||||
delete (globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__
|
||||
mockKill.mockResolvedValue(undefined)
|
||||
mockGetShutdownBlockReason.mockResolvedValue(null)
|
||||
mockRuntimeCall.mockResolvedValue({
|
||||
@@ -272,6 +274,39 @@ describe('terminal tab retirement store boundary', () => {
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('keeps ordinary non-Windows close synchronous and skips Windows inventory', () => {
|
||||
vi.stubGlobal('navigator', { userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X)' })
|
||||
const store = createTestStore()
|
||||
seedStore(store, {
|
||||
tabsByWorktree: {
|
||||
'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-1' })]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-1'] }
|
||||
})
|
||||
|
||||
store.getState().closeTab('tab-1')
|
||||
|
||||
expect(store.getState().tabsByWorktree['wt-1']).toEqual([])
|
||||
expect(mockGetShutdownBlockReason).not.toHaveBeenCalled()
|
||||
expect(mockKill).toHaveBeenCalledWith('pty-1')
|
||||
})
|
||||
|
||||
it('does not run native shutdown inventory in a Windows web client', () => {
|
||||
;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true
|
||||
const store = createTestStore()
|
||||
seedStore(store, {
|
||||
tabsByWorktree: {
|
||||
'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'remote:terminal-1' })]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-1': ['remote:terminal-1'] }
|
||||
})
|
||||
|
||||
store.getState().closeTab('tab-1')
|
||||
|
||||
expect(store.getState().tabsByWorktree['wt-1']).toEqual([])
|
||||
expect(mockGetShutdownBlockReason).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a legacy Windows tab and PTY ownership visible when shutdown is unsafe', async () => {
|
||||
const store = createTestStore()
|
||||
mockGetShutdownBlockReason.mockResolvedValueOnce(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON)
|
||||
|
||||
@@ -606,6 +606,8 @@ export type TerminalSlice = {
|
||||
shutdownReason?: AgentStatusWorktreeShutdownReason
|
||||
sleepingPaneKeys?: string[]
|
||||
expectedRuntimePtyIds?: string[]
|
||||
/** Internal handoff from a stable safety check in the same renderer turn. */
|
||||
shutdownSafetyChecked?: boolean
|
||||
}
|
||||
) => Promise<void>
|
||||
shutdownCompletedAgentPaneForHibernation: (
|
||||
@@ -2302,11 +2304,13 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
const shutdownReason: AgentStatusWorktreeShutdownReason =
|
||||
opts?.shutdownReason ?? (keepIdentifiers ? 'manual-sleep' : 'remove-worktree')
|
||||
const worktreeIds = new Set([worktreeId])
|
||||
await assertStableTerminalShutdownSafety({
|
||||
surfaceId: worktreeId,
|
||||
getState: get,
|
||||
selectTabIds: (state) => selectTerminalTabIdsForWorktrees(state, worktreeIds)
|
||||
})
|
||||
if (!opts?.shutdownSafetyChecked) {
|
||||
await assertStableTerminalShutdownSafety({
|
||||
surfaceId: worktreeId,
|
||||
getState: get,
|
||||
selectTabIds: (state) => selectTerminalTabIdsForWorktrees(state, worktreeIds)
|
||||
})
|
||||
}
|
||||
const tabs = get().tabsByWorktree[worktreeId] ?? []
|
||||
const ptyIds = tabs.flatMap((tab) => get().ptyIdsByTabId[tab.id] ?? [])
|
||||
const rendererShutdownPtyIds = sortedUniquePtyIds(ptyIds)
|
||||
|
||||
Reference in New Issue
Block a user