mirror of
https://github.com/stablyai/orca.git
synced 2026-11-04 02:57:36 +00:00
fix(browser): keep slept workspace browser tabs off the reopen-closed list
Sleeping a workspace closed its browser tabs as if the user had closed them, so Reopen Closed Tab could bring back a tab that was only slept. Sleep now closes them with a 'sleep' reason that skips the reopen stack while still releasing host pages, doc grants and webviews as before.
This commit is contained in:
@@ -88,7 +88,8 @@ describe('runSleepWorktree', () => {
|
||||
// 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')
|
||||
// Why the reason: a slept tab must not land on the reopen-closed list.
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-1', { reason: 'sleep' })
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true
|
||||
})
|
||||
@@ -300,7 +301,7 @@ describe('runSleepWorktree', () => {
|
||||
expect(mocks.state.shutdownWorktreeTerminals).not.toHaveBeenCalledWith('wt-1', {
|
||||
keepIdentifiers: true
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-2')
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-2', { reason: 'sleep' })
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-2', {
|
||||
keepIdentifiers: true
|
||||
})
|
||||
@@ -333,11 +334,15 @@ describe('runSleepWorktree', () => {
|
||||
|
||||
expect(mocks.state.setActiveWorktree).toHaveBeenCalledTimes(1)
|
||||
expect(mocks.state.setActiveWorktree).toHaveBeenCalledWith(null)
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(1, 'wt-1')
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(1, 'wt-1', {
|
||||
reason: 'sleep'
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenNthCalledWith(1, 'wt-1', {
|
||||
keepIdentifiers: true
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(2, 'wt-2')
|
||||
expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenNthCalledWith(2, 'wt-2', {
|
||||
reason: 'sleep'
|
||||
})
|
||||
expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenNthCalledWith(2, 'wt-2', {
|
||||
keepIdentifiers: true
|
||||
})
|
||||
|
||||
@@ -197,7 +197,9 @@ export async function sleepWorktreesReportingFailures(
|
||||
// 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 withWorktreeSleepTeardown(worktreeId, () => shutdownWorktreeBrowsers(worktreeId))
|
||||
await withWorktreeSleepTeardown(worktreeId, () =>
|
||||
shutdownWorktreeBrowsers(worktreeId, { reason: 'sleep' })
|
||||
)
|
||||
} catch (err) {
|
||||
console.error('[sleep-worktree] browser shutdown failed', { worktreeId, error: err })
|
||||
failedWorktreeIds.add(worktreeId)
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as AgentStatusModule from '@/lib/agent-status'
|
||||
import type * as RuntimeRpcClientModule from '@/runtime/runtime-rpc-client'
|
||||
import { createTestStore, makeWorktree, seedStore } from './store-test-helpers'
|
||||
import { createStoreCascadesMockApi } from './store-cascades-test-harness'
|
||||
|
||||
const mockCallRuntimeRpc = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() }
|
||||
}))
|
||||
|
||||
vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({
|
||||
restorePtyDataHandlersAfterFailedShutdown: vi.fn(),
|
||||
unregisterPtyDataHandlers: vi.fn(() => [])
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-status', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof AgentStatusModule>()
|
||||
return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) }
|
||||
})
|
||||
|
||||
vi.mock('@/runtime/runtime-rpc-client', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof RuntimeRpcClientModule>()
|
||||
return { ...actual, callRuntimeRpc: mockCallRuntimeRpc }
|
||||
})
|
||||
|
||||
createStoreCascadesMockApi()
|
||||
|
||||
const WT = 'repo1::/path/wt1'
|
||||
|
||||
function storeWithRemoteBrowserTab(): {
|
||||
store: ReturnType<typeof createTestStore>
|
||||
workspaceId: string
|
||||
} {
|
||||
const store = createTestStore()
|
||||
seedStore(store, {
|
||||
worktreesByRepo: { repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' })] },
|
||||
activeWorktreeId: WT,
|
||||
activeTabType: 'terminal'
|
||||
})
|
||||
const workspace = store.getState().createBrowserTab(WT, 'about:blank', {
|
||||
activate: true,
|
||||
browserPageId: 'remote-page-1',
|
||||
browserRuntimeEnvironmentId: 'env-1'
|
||||
})
|
||||
const pageId = store.getState().browserPagesByWorkspace[workspace.id]?.[0]?.id ?? ''
|
||||
store.getState().setRemoteBrowserPageHandle(pageId, {
|
||||
environmentId: 'env-1',
|
||||
remotePageId: 'remote-page-1',
|
||||
staged: true
|
||||
})
|
||||
return { store, workspaceId: workspace.id }
|
||||
}
|
||||
|
||||
describe('browser tabs closed by workspace sleep', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockCallRuntimeRpc.mockResolvedValue({})
|
||||
})
|
||||
|
||||
// Why: reopen-closed is the user's undo history; a slept tab was never closed by the user.
|
||||
it('does not enter the reopen stack', async () => {
|
||||
const { store } = storeWithRemoteBrowserTab()
|
||||
|
||||
await store.getState().shutdownWorktreeBrowsers(WT, { reason: 'sleep' })
|
||||
|
||||
const s = store.getState()
|
||||
expect(s.browserTabsByWorktree[WT] ?? []).toEqual([])
|
||||
expect(s.recentlyClosedBrowserTabsByWorktree[WT] ?? []).toEqual([])
|
||||
expect(s.recentlyClosedTabKindsByWorktree[WT] ?? []).toEqual([])
|
||||
expect(s.reopenClosedBrowserTab(WT)).toBeNull()
|
||||
})
|
||||
|
||||
// Why: unlike an unwound create, a slept tab's host page exists and must still be released.
|
||||
it('still closes the host page', async () => {
|
||||
const { store } = storeWithRemoteBrowserTab()
|
||||
|
||||
await store.getState().shutdownWorktreeBrowsers(WT, { reason: 'sleep' })
|
||||
|
||||
expect(mockCallRuntimeRpc).toHaveBeenCalledWith(
|
||||
{ kind: 'environment', environmentId: 'env-1' },
|
||||
'browser.tabClose',
|
||||
expect.objectContaining({ page: 'remote-page-1' }),
|
||||
expect.anything()
|
||||
)
|
||||
})
|
||||
|
||||
it('leaves the reopen stack for a user close', () => {
|
||||
const { store, workspaceId } = storeWithRemoteBrowserTab()
|
||||
|
||||
store.getState().closeBrowserTab(workspaceId)
|
||||
|
||||
expect(store.getState().recentlyClosedBrowserTabsByWorktree[WT]).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -25,6 +25,8 @@ export function createBrowserCloseActions(
|
||||
// Why: a cleanup close unwinds a tab that never finished being created — it owns no host
|
||||
// page to close and must not enter the reopen stack as if the user had closed something.
|
||||
const isCleanup = options?.reason === 'cleanup'
|
||||
// Why: a slept tab is released, not closed by the user, so it must not be reopenable.
|
||||
const recordsReopen = !isCleanup && options?.reason !== 'sleep'
|
||||
let remotePagesToClose: { worktreeId: string; handle: RemoteBrowserPageHandle }[] = []
|
||||
// Why collected rather than released inside the reducer: revoking is main-process work, and a
|
||||
// grant is the only authority the preview scheme honors — a closed document must stop being
|
||||
@@ -114,7 +116,7 @@ export function createBrowserCloseActions(
|
||||
}
|
||||
|
||||
const nextRecentlyClosedBrowserTabsByWorktree = { ...s.recentlyClosedBrowserTabsByWorktree }
|
||||
if (!isCleanup) {
|
||||
if (recordsReopen) {
|
||||
const existingSnapshots = nextRecentlyClosedBrowserTabsByWorktree[owningWorktreeId] ?? []
|
||||
const position = getRecentlyClosedTabPosition(s, owningWorktreeId, tabId)
|
||||
nextRecentlyClosedBrowserTabsByWorktree[owningWorktreeId] = [
|
||||
@@ -126,13 +128,13 @@ export function createBrowserCloseActions(
|
||||
...existingSnapshots.filter((entry) => entry.workspace.id !== closedWorkspace.id)
|
||||
].slice(0, 10)
|
||||
}
|
||||
const nextRecentlyClosedTabKindsByWorktree = isCleanup
|
||||
? s.recentlyClosedTabKindsByWorktree
|
||||
: pushRecentlyClosedTabKind(
|
||||
const nextRecentlyClosedTabKindsByWorktree = recordsReopen
|
||||
? pushRecentlyClosedTabKind(
|
||||
s.recentlyClosedTabKindsByWorktree,
|
||||
owningWorktreeId,
|
||||
'browser'
|
||||
)
|
||||
: s.recentlyClosedTabKindsByWorktree
|
||||
|
||||
const nextRecentlyClosedBrowserPagesByWorkspace = {
|
||||
...s.recentlyClosedBrowserPagesByWorkspace
|
||||
@@ -196,7 +198,7 @@ export function createBrowserCloseActions(
|
||||
get(),
|
||||
workspaceItem.worktreeId,
|
||||
tabId,
|
||||
isCleanup ? 'cleanup' : 'user'
|
||||
recordsReopen ? 'user' : 'cleanup'
|
||||
)
|
||||
}
|
||||
if (workspaceItem) {
|
||||
@@ -237,13 +239,13 @@ export function createBrowserCloseActions(
|
||||
}
|
||||
},
|
||||
|
||||
shutdownWorktreeBrowsers: async (worktreeId) => {
|
||||
shutdownWorktreeBrowsers: async (worktreeId, options) => {
|
||||
const workspaces = get().browserTabsByWorktree[worktreeId] ?? []
|
||||
// Why: snapshot before the loop — closeBrowserTab empties the array, so set() below couldn't recompute hadBrowserTabs.
|
||||
const hadBrowserTabs = workspaces.length > 0
|
||||
for (const workspace of workspaces) {
|
||||
const browserPagesByWorkspace = get().browserPagesByWorkspace
|
||||
get().closeBrowserTab(workspace.id)
|
||||
get().closeBrowserTab(workspace.id, options)
|
||||
destroyWorkspaceWebviews(browserPagesByWorkspace, workspace.id)
|
||||
}
|
||||
set((s) => {
|
||||
|
||||
@@ -32,6 +32,9 @@ import type { ExecutionHostId } from '../../../../../shared/execution-host'
|
||||
import type { TabSplitTarget } from '../tabs/tabs-slice-contract'
|
||||
import type { RuntimeBrowserPlacement } from '../../../../../shared/runtime-browser-placement'
|
||||
|
||||
/** A close the user did not ask for. */
|
||||
export type BrowserTabCloseReason = 'cleanup' | 'sleep'
|
||||
|
||||
export type CreateBrowserTabOptions = {
|
||||
activate?: boolean
|
||||
browserPageId?: string
|
||||
@@ -148,8 +151,13 @@ export type BrowserSlice = {
|
||||
url: string,
|
||||
profileId: string | null
|
||||
) => Promise<boolean>
|
||||
closeBrowserTab: (tabId: string, options?: { reason?: 'cleanup' }) => void
|
||||
shutdownWorktreeBrowsers: (worktreeId: string) => Promise<void>
|
||||
/** `cleanup` unwinds an unfinished create; `sleep` releases a slept workspace's tab. Neither
|
||||
* enters the reopen-closed list. */
|
||||
closeBrowserTab: (tabId: string, options?: { reason?: BrowserTabCloseReason }) => void
|
||||
shutdownWorktreeBrowsers: (
|
||||
worktreeId: string,
|
||||
options?: { reason?: BrowserTabCloseReason }
|
||||
) => Promise<void>
|
||||
reopenClosedBrowserTab: (worktreeId: string) => BrowserWorkspace | null
|
||||
/** Activation uses the tab's workspace unless a target is supplied; global selection
|
||||
* moves only when that workspace is active. */
|
||||
|
||||
Reference in New Issue
Block a user