mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
fix(terminal): ignore PTY binds that land inside the sleep teardown window
A spawn resolving while shutdown was still awaiting the host bound a PTY and cleared the marker, waking every waiting pane mid-sleep; re-marking afterwards could not un-connect them. The sleep flow now scopes each teardown so binds in that window are not wakes. The e2e asserts a deliberate wake yields exactly one PTY, and the dispose test proves the listener is gone.
This commit is contained in:
@@ -44,7 +44,8 @@ vi.mock('@/store', () => ({
|
||||
vi.mock('sonner', () => ({ toast: { error: mocks.toastError } }))
|
||||
vi.mock('@/lib/worktree-sleep-intent', () => ({
|
||||
clearWorktreeSleepIntent: mocks.clearWorktreeSleepIntent,
|
||||
markWorktreeSleepIntent: mocks.markWorktreeSleepIntent
|
||||
markWorktreeSleepIntent: mocks.markWorktreeSleepIntent,
|
||||
withWorktreeSleepTeardown: (_worktreeId: string, teardown: () => Promise<unknown>) => teardown()
|
||||
}))
|
||||
|
||||
import { runSleepWorktree, runSleepWorktrees } from './sleep-worktree-flow'
|
||||
@@ -221,14 +222,6 @@ describe('runSleepWorktree', () => {
|
||||
expect(mocks.clearWorktreeSleepIntent).toHaveBeenLastCalledWith('wt-2')
|
||||
})
|
||||
|
||||
it('re-asserts the marker after teardown so a late PTY bind cannot un-sleep it', async () => {
|
||||
await runSleepWorktree('wt-1')
|
||||
|
||||
const marks = mocks.markWorktreeSleepIntent.mock.invocationCallOrder
|
||||
const terminalShutdown = mocks.state.shutdownWorktreeTerminals.mock.invocationCallOrder[0]
|
||||
expect(marks.some((order) => order > terminalShutdown)).toBe(true)
|
||||
})
|
||||
|
||||
it('marks each worktree only when its own teardown starts', async () => {
|
||||
let releaseFirst: () => void = () => {}
|
||||
mocks.state.shutdownWorktreeBrowsers.mockImplementationOnce(
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { toast } from 'sonner'
|
||||
import { useAppStore } from '@/store'
|
||||
import { clearWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent'
|
||||
import {
|
||||
clearWorktreeSleepIntent,
|
||||
markWorktreeSleepIntent,
|
||||
withWorktreeSleepTeardown
|
||||
} from '@/lib/worktree-sleep-intent'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
@@ -167,7 +171,7 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise
|
||||
// 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)
|
||||
await withWorktreeSleepTeardown(worktreeId, () => shutdownWorktreeBrowsers(worktreeId))
|
||||
} catch (err) {
|
||||
console.error('[sleep-worktree] browser shutdown failed', { worktreeId, error: err })
|
||||
failedWorktreeIds.add(worktreeId)
|
||||
@@ -182,17 +186,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 })
|
||||
if (typeof window !== 'undefined' && window.api?.ephemeralVm?.suspendWorkspace) {
|
||||
await window.api.ephemeralVm.suspendWorkspace({ workspaceId: worktreeId })
|
||||
}
|
||||
// Why: a spawn that resolved during teardown binds a PTY and clears the marker;
|
||||
// the workspace is asleep now, so re-assert it. A workspace the user activated
|
||||
// meanwhile is awake by their choice and must not be left marked.
|
||||
await withWorktreeSleepTeardown(worktreeId, async () => {
|
||||
await shutdownWorktreeTerminals(worktreeId, { keepIdentifiers: true })
|
||||
if (typeof window !== 'undefined' && window.api?.ephemeralVm?.suspendWorkspace) {
|
||||
await window.api.ephemeralVm.suspendWorkspace({ workspaceId: worktreeId })
|
||||
}
|
||||
})
|
||||
// Why: a workspace the user activated during the batch is awake by their choice.
|
||||
if (useAppStore.getState().activeWorktreeId === worktreeId) {
|
||||
clearWorktreeSleepIntent(worktreeId)
|
||||
} else {
|
||||
markWorktreeSleepIntent(worktreeId)
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('[sleep-worktree] terminal or host suspension failed', {
|
||||
|
||||
+6
@@ -313,10 +313,16 @@ describe('deliberate sleep keeps mounted panes cold', () => {
|
||||
const binding = connectPanePty(createPane(1) as never, createManager(1) as never, deps as never)
|
||||
await flushAsyncTicks()
|
||||
binding.dispose()
|
||||
// Why: the connect body already refuses a disposed session, so prove the
|
||||
// listener itself is gone: a wake after dispose reaches no subscriber.
|
||||
const wakeCalls: number[] = []
|
||||
const { onWorktreeSleepIntentCleared } = await import('@/lib/worktree-sleep-intent')
|
||||
onWorktreeSleepIntentCleared('wt-1', () => wakeCalls.push(1))
|
||||
clearWorktreeSleepIntent('wt-1')
|
||||
await flushAsyncTicks()
|
||||
|
||||
expect(transport.connect).not.toHaveBeenCalled()
|
||||
expect(wakeCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('still connects a slept pane that carries a queued startup', async () => {
|
||||
|
||||
@@ -2,14 +2,35 @@
|
||||
// Any pane connect that runs while the marker is set waits here, and the clear
|
||||
// that marks the workspace awake resumes every waiting connect.
|
||||
const sleepingWorktreeIds = new Set<string>()
|
||||
const tearingDownWorktreeIds = new Set<string>()
|
||||
const wakeListenersByWorktreeId = new Map<string, Set<() => void>>()
|
||||
|
||||
export function markWorktreeSleepIntent(worktreeId: string): void {
|
||||
sleepingWorktreeIds.add(worktreeId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Why: a spawn that resolves while the sleep teardown is still awaiting its host
|
||||
* would bind a PTY and clear the marker, waking every waiting pane mid-sleep.
|
||||
* Binds during the teardown window are not wakes.
|
||||
*/
|
||||
export async function withWorktreeSleepTeardown<T>(
|
||||
worktreeId: string,
|
||||
teardown: () => Promise<T>
|
||||
): Promise<T> {
|
||||
tearingDownWorktreeIds.add(worktreeId)
|
||||
try {
|
||||
return await teardown()
|
||||
} finally {
|
||||
tearingDownWorktreeIds.delete(worktreeId)
|
||||
}
|
||||
}
|
||||
|
||||
export function clearWorktreeSleepIntent(worktreeId: string | null): void {
|
||||
if (!worktreeId || !sleepingWorktreeIds.delete(worktreeId)) {
|
||||
if (!worktreeId || tearingDownWorktreeIds.has(worktreeId)) {
|
||||
return
|
||||
}
|
||||
if (!sleepingWorktreeIds.delete(worktreeId)) {
|
||||
return
|
||||
}
|
||||
const listeners = wakeListenersByWorktreeId.get(worktreeId)
|
||||
@@ -27,6 +48,7 @@ export function clearWorktreeSleepIntent(worktreeId: string | null): void {
|
||||
// Why: a purged worktree must not wake its panes; they are being unmounted.
|
||||
export function forgetWorktreeSleepIntent(worktreeId: string): void {
|
||||
sleepingWorktreeIds.delete(worktreeId)
|
||||
tearingDownWorktreeIds.delete(worktreeId)
|
||||
wakeListenersByWorktreeId.delete(worktreeId)
|
||||
}
|
||||
|
||||
|
||||
@@ -120,6 +120,24 @@ describe('worktree sleep intent lifecycle', () => {
|
||||
unsubscribe()
|
||||
})
|
||||
|
||||
it('ignores a PTY bind that lands while the sleep teardown is in flight', async () => {
|
||||
const store = createTestStore()
|
||||
seedWorktree(store)
|
||||
const tab = store.getState().createTab(WORKTREE_ID, undefined, undefined, { activate: false })
|
||||
markWorktreeSleepIntent(WORKTREE_ID)
|
||||
const woke = vi.fn()
|
||||
intent.onWorktreeSleepIntentCleared(WORKTREE_ID, woke)
|
||||
|
||||
await intent.withWorktreeSleepTeardown(WORKTREE_ID, async () => {
|
||||
store.getState().updateTabPtyId(tab.id, 'pty-late-spawn')
|
||||
})
|
||||
|
||||
expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(true)
|
||||
expect(woke).not.toHaveBeenCalled()
|
||||
store.getState().updateTabPtyId(tab.id, 'pty-after-teardown')
|
||||
expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps notifying siblings when one wake listener throws', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const woke = vi.fn()
|
||||
|
||||
@@ -72,7 +72,8 @@ test('remounting a slept hidden pane does not respawn its PTY', async ({ orcaPag
|
||||
expect(remounted, 'remountTerminalTabForRecovery did not find the slept tab').toBe(true)
|
||||
await assertStaysCold(orcaPage, slept)
|
||||
|
||||
// Non-vacuity: a deliberate click must still wake it.
|
||||
// Non-vacuity: a deliberate click must still wake it, and exactly once — the
|
||||
// waiting pane and its remounted successor must not both reattach.
|
||||
await activateWorkspaceByClick(orcaPage, slept)
|
||||
await expect
|
||||
.poll(async () => (await readWorkspaceSample(orcaPage, slept)).livePtyCount, {
|
||||
@@ -80,4 +81,7 @@ test('remounting a slept hidden pane does not respawn its PTY', async ({ orcaPag
|
||||
message: 'the slept workspace never wakes even on deliberate activation'
|
||||
})
|
||||
.toBeGreaterThan(0)
|
||||
await orcaPage.waitForTimeout(3_000)
|
||||
expect((await readWorkspaceSample(orcaPage, slept)).livePtyCount).toBe(1)
|
||||
expect(await readHostLiveTerminalCount(orcaPage, slept)).toBe(1)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user