A paired desktop shows a server's browser tab without opening a secon… · tmchow/orca@ad269f4 · GitHub
Skip to content

Commit ad269f4

Browse files
Jinwoo-HJinwoo-H
authored
A paired desktop shows a server's browser tab without opening a second page (dark, layout-owner build only) (stablyai#31509)
* fix(browser): a paired desktop shows a server's browser tab without opening a second page (switch-on builds only) With the layout-owner switch on, a server browser tab drawn from the server's layout reaches the desktop without a saved page handle. Showing it made the desktop create a new page on the server, so the server went from one page to two. Before creating, the desktop now asks the server for its browser pages and adopts the one already named by the tab's page or browser tab id. It creates a page only when the server has none, when the lookup fails, or when the server gives no list. A pane that moved on during the lookup adopts nothing. Switch-off builds keep creating directly. * test(browser): explain the fake runtime call's cast in the page session test * fix(browser): adopt a server's browser page by the id its layout already names, not by listing pages (switch-on only) This replaces the tab-list lookup. A tab list misses saved pages a restarted server has not started, wakes the active saved page as a side effect, and can match a page another client hosts. The server's layout already records the page id on the browser tab. Desktop hydration ignored it whenever the page record had not landed yet, and made up a new id on every frame build. With the switch on, hydration now uses the tab's recorded page id, so the desktop row is named by the server's page from the first frame and stays named by it. A frame-drawn row still has no saved page handle: hydration has none for server-hosted pages, and the server environment is known only to the pane. So, with the switch on, the pane probes its row id with browser.tabShow. tabShow also starts a saved page the server has not opened since a restart. On success the pane stores the handle and streams that page. On not-found, or any other error, it creates a page as before and closes nothing. A pane that moved on during the probe adopts nothing. Why not reorder the server to save the page record before opening the tab: it would only fix the id, not the missing handle, so the probe is needed either way. The page record write also depends on the tab main writes asynchronously, so the order cannot be made certain from the server alone. * test(browser): explain the lifecycle harness's fake runtime call cast * fix(browser): create a server browser page only when the server lacks it, and prefer the tab's active page id The switch-on probe created a page after any tabShow failure, so a timeout or a brief connection error could duplicate a page the server still runs. Now only a page-missing answer creates one. Any other error is raised, as the stored-handle path already does, and the pane shows its stopped state with Reconnect. Hydration now names a tab's page by its recorded active page before its first page id, which matches the page the server starts for a saved tab.
1 parent 5dd5294 commit ad269f4

7 files changed

Lines changed: 304 additions & 5 deletions

‎src/main/runtime/orca-runtime-serve-browser-restore.test.ts‎

Lines changed: 14 additions & 0 deletions

‎src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-session.test.ts‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { describe, expect, it, vi } from 'vitest'
2+
import {
3+
RemoteBrowserPageSession,
4+
type RemoteBrowserPageHandle,
5+
type RemoteBrowserRpcCall
6+
} from './remote-browser-page-session'
7+
import { RemoteBrowserOperationTokens } from './remote-browser-stream-tokens'
8+
9+
const layoutOwner = vi.hoisted(() => ({ isMain: true }))
10+
vi.mock('../../../../../shared/layout-owner-switch', () => ({
11+
get ORCA_LAYOUT_OWNER_IS_MAIN() {
12+
return layoutOwner.isMain
13+
}
14+
}))
15+
16+
/** What the host answers to tabShow for the row's own id. */
17+
type ShowReply = 'runs' | 'not-found' | 'other-error'
18+
19+
function rpcError(code: string): Error {
20+
return Object.assign(new Error(code), { code })
21+
}
22+
23+
function setup(args: { show: ShowReply; switchOn?: boolean; onShow?: () => void }) {
24+
layoutOwner.isMain = args.switchOn ?? true
25+
const tokens = new RemoteBrowserOperationTokens({
26+
isMounted: () => true,
27+
isActive: () => true,
28+
// A frame-drawn row is named by the host's page id.
29+
getTabId: () => 'host-page-1',
30+
getEnvironmentId: () => 'env-1',
31+
browserPageExists: () => true
32+
})
33+
const calls: { method: string; params: unknown }[] = []
34+
const closedMissing: (string | null)[] = []
35+
let stored: RemoteBrowserPageHandle | null = null
36+
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake answers each method with the reply shape its caller reads.
37+
const callRpc = (async (_target: unknown, method: string, params?: unknown) => {
38+
calls.push({ method, params })
39+
if (method === 'browser.tabShow') {
40+
args.onShow?.()
41+
if (args.show === 'not-found') {
42+
throw rpcError('browser_tab_not_found')
43+
}
44+
if (args.show === 'other-error') {
45+
throw rpcError('request_timeout')
46+
}
47+
return { tab: { url: 'https://example.test/', title: 'Example' } }
48+
}
49+
if (method === 'browser.tabCreate') {
50+
return { browserPageId: 'created-page' }
51+
}
52+
return {}
53+
}) as unknown as RemoteBrowserRpcCall
54+
const session = new RemoteBrowserPageSession({
55+
tokens,
56+
callRpc,
57+
getWorktreeSelector: () => 'id:wt-1',
58+
getCurrentUrl: () => 'about:blank',
59+
readStoredHandle: () => stored,
60+
writeStoredHandle: (handle) => {
61+
stored = handle
62+
},
63+
removeStoredHandle: () => {
64+
stored = null
65+
},
66+
applyTabInfo: () => {},
67+
closeMissingRemotePage: (id) => closedMissing.push(id)
68+
})
69+
return {
70+
tokens,
71+
calls,
72+
closedMissing,
73+
stored: () => stored,
74+
ensure: () => session.ensureRemotePage(tokens.createOperationToken()!),
75+
methods: () => calls.map((call) => call.method)
76+
}
77+
}
78+
79+
describe('RemoteBrowserPageSession.ensureRemotePage without a stored handle', () => {
80+
// A saved page a restarted host has not started is woken by this same tabShow (see
81+
// orca-runtime-serve-browser-restore.test.ts).
82+
it('adopts the page the host runs under the row id instead of creating a second one', async () => {
83+
const harness = setup({ show: 'runs' })
84+
85+
await expect(harness.ensure()).resolves.toBe('host-page-1')
86+
87+
expect(harness.methods()).toEqual(['browser.tabShow'])
88+
expect(harness.calls[0]?.params).toEqual({ worktree: 'id:wt-1', page: 'host-page-1' })
89+
expect(harness.tokens.remotePage).toBe('host-page-1')
90+
expect(harness.stored()).toEqual({ environmentId: 'env-1', remotePageId: 'host-page-1' })
91+
})
92+
93+
it('creates a page when the host has none under the row id, closing nothing', async () => {
94+
const harness = setup({ show: 'not-found' })
95+
96+
await expect(harness.ensure()).resolves.toBe('created-page')
97+
98+
expect(harness.methods()).toEqual(['browser.tabShow', 'browser.tabCreate'])
99+
expect(harness.closedMissing).toEqual([])
100+
expect(harness.stored()).toEqual({ environmentId: 'env-1', remotePageId: 'created-page' })
101+
})
102+
103+
// A timeout or transient failure says nothing about the page; creating would duplicate a live one.
104+
it('surfaces any other probe failure without creating a page', async () => {
105+
const harness = setup({ show: 'other-error' })
106+
107+
await expect(harness.ensure()).rejects.toMatchObject({ code: 'request_timeout' })
108+
109+
expect(harness.methods()).toEqual(['browser.tabShow'])
110+
expect(harness.tokens.remotePage).toBeNull()
111+
expect(harness.stored()).toBeNull()
112+
expect(harness.closedMissing).toEqual([])
113+
})
114+
115+
it('adopts nothing when the pane moved on during the probe', async () => {
116+
let harness: ReturnType<typeof setup> | null = null
117+
harness = setup({ show: 'runs', onShow: () => harness?.tokens.supersedeOperations() })
118+
119+
await expect(harness.ensure()).resolves.toBeNull()
120+
121+
expect(harness.methods()).toEqual(['browser.tabShow'])
122+
expect(harness.stored()).toBeNull()
123+
expect(harness.tokens.remotePage).toBeNull()
124+
})
125+
126+
it('keeps creating directly with the layout-owner switch off', async () => {
127+
const harness = setup({ show: 'runs', switchOn: false })
128+
129+
await expect(harness.ensure()).resolves.toBe('created-page')
130+
131+
expect(harness.methods()).toEqual(['browser.tabCreate'])
132+
})
133+
})

‎src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-session.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { RuntimeClientTarget } from '../../../../../shared/runtime-client-target'
22
import { ORCA_BROWSER_BLANK_URL } from '../../../../../shared/constants'
3+
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../shared/layout-owner-switch'
34
import type { BrowserTabInfo } from '../../../../../shared/runtime-types'
45
import { isRemoteBrowserPageMissingError } from './remote-browser-stream-errors'
56
import type {
@@ -59,6 +60,13 @@ export class RemoteBrowserPageSession {
5960
}
6061
const existingHandle = this.deps.readStoredHandle()
6162
if (existingHandle?.environmentId !== token.environmentId) {
63+
// Switch point: a frame-drawn host tab has no handle, but its row id is the host's page id.
64+
if (ORCA_LAYOUT_OWNER_IS_MAIN) {
65+
const adopted = await this.adoptHostPage(token)
66+
if (adopted !== undefined) {
67+
return adopted
68+
}
69+
}
6270
return this.createRemotePage(token)
6371
}
6472
tokens.setRemotePage(existingHandle.remotePageId)
@@ -115,6 +123,38 @@ export class RemoteBrowserPageSession {
115123
}
116124
}
117125

126+
/** The row's own id when the host runs that page; undefined when the host has none, so create. */
127+
private async adoptHostPage(
128+
token: RemoteBrowserOperationToken
129+
): Promise<string | null | undefined> {
130+
const { tokens } = this.deps
131+
const provisional = token.tabId
132+
const probe = { ...token, remotePageId: provisional }
133+
const release = (): void => {
134+
if (tokens.remotePage === provisional) {
135+
tokens.setRemotePage(null)
136+
}
137+
}
138+
tokens.setRemotePage(provisional)
139+
try {
140+
// tabShow also starts a saved page the host has not opened yet.
141+
if (!(await this.fetchTabInfo(probe)) || !tokens.isCurrent(probe)) {
142+
release()
143+
return null
144+
}
145+
} catch (error) {
146+
release()
147+
// Only a page the host truly lacks is created; a timeout must not duplicate a live page.
148+
if (!isRemoteBrowserPageMissingError(error)) {
149+
throw error
150+
}
151+
// A handle-less row has nothing to close.
152+
return tokens.isCurrent(token) ? undefined : null
153+
}
154+
this.deps.writeStoredHandle({ environmentId: token.environmentId, remotePageId: provisional })
155+
return provisional
156+
}
157+
118158
private async createRemotePage(token: RemoteBrowserOperationToken): Promise<string | null> {
119159
const target: RuntimeClientTarget = { kind: 'environment', environmentId: token.environmentId }
120160
const worktree = this.deps.getWorktreeSelector()

‎src/renderer/src/components/browser-pane/stream-remote/remote-browser-stream-lifecycle-test-harness.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ export function createHarness() {
7272
let subscribeAttempts = 0
7373
let closeBeforeNextSubscribeRejects = false
7474

75-
const callRpc = (async (_target: unknown, method: string) => {
75+
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake answers each method with the reply shape its caller reads.
76+
const callRpc = (async (_target: unknown, method: string, params?: { page?: string }) => {
7677
rpcLog.push(method)
7778
if (method === 'status.get') {
7879
if (statusGate) {
@@ -88,6 +89,10 @@ export function createHarness() {
8889
tabShowGate = null
8990
await gate.wait
9091
}
92+
// The host holds only pages it created or the pane was handed; the row id is not one.
93+
if (params?.page === identity.tabId) {
94+
throw rpcError('browser_tab_not_found', 'Browser page was not found')
95+
}
9196
return { tab: { url: 'https://example.test/', title: 'Example' } }
9297
}
9398
if (method === 'browser.tabCreate') {

‎src/renderer/src/store/slices/browser/browser-hydration-state.test.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { describe, expect, it, vi } from 'vitest'
2+
import type { WorkspaceSessionState } from '../../../../../shared/workspace-session-state-types'
3+
import { projectHydratedBrowserSession } from './browser-hydration-state'
4+
5+
const layoutOwner = vi.hoisted(() => ({ isMain: true }))
6+
vi.mock('../../../../../shared/layout-owner-switch', () => ({
7+
get ORCA_LAYOUT_OWNER_IS_MAIN() {
8+
return layoutOwner.isMain
9+
}
10+
}))
11+
12+
const WT = 'repo-1::/srv/wt'
13+
14+
/** A frame published between the server's openBrowserTab and its page record: the tab names its page. */
15+
function frameBeforePageRecord(
16+
tab: { activePageId?: string; pageIds?: string[] } = { pageIds: ['host-page-1'] }
17+
): WorkspaceSessionState {
18+
return {
19+
activeRepoId: null,
20+
activeWorktreeId: null,
21+
activeTabId: null,
22+
tabsByWorktree: {},
23+
terminalLayoutsByTabId: {},
24+
browserTabsByWorktree: {
25+
[WT]: [
26+
{
27+
id: 'host-page-1',
28+
worktreeId: WT,
29+
url: 'about:blank',
30+
title: 'New Tab',
31+
loading: false,
32+
faviconUrl: null,
33+
canGoBack: false,
34+
canGoForward: false,
35+
loadError: null,
36+
createdAt: 1,
37+
...tab
38+
}
39+
]
40+
},
41+
browserPagesByWorkspace: {}
42+
}
43+
}
44+
45+
function hydratedPageIds(tab?: { activePageId?: string; pageIds?: string[] }): {
46+
pageIds: string[]
47+
workspacePageIds: string[]
48+
} {
49+
const projected = projectHydratedBrowserSession(
50+
{
51+
activeWorktreeId: null,
52+
activeTabType: 'terminal',
53+
activeTabTypeByWorktree: {},
54+
openFiles: [],
55+
tabsByWorktree: {}
56+
},
57+
frameBeforePageRecord(tab),
58+
new Set([WT])
59+
)
60+
return {
61+
pageIds: (projected.browserPagesByWorkspace['host-page-1'] ?? []).map((page) => page.id),
62+
workspacePageIds: projected.browserTabsByWorktree[WT]?.[0]?.pageIds ?? []
63+
}
64+
}
65+
66+
describe('projectHydratedBrowserSession for a tab whose page record has not landed', () => {
67+
it('names the page by the id the tab records, the same on every frame build', () => {
68+
layoutOwner.isMain = true
69+
70+
const first = hydratedPageIds()
71+
const second = hydratedPageIds()
72+
73+
expect(first).toEqual({ pageIds: ['host-page-1'], workspacePageIds: ['host-page-1'] })
74+
expect(second).toEqual(first)
75+
})
76+
77+
// The page a host starts for a saved tab is its active one (ServeBrowserTabs.startableIn).
78+
it('prefers the page the tab records as active', () => {
79+
layoutOwner.isMain = true
80+
81+
const { pageIds } = hydratedPageIds({
82+
activePageId: 'host-page-2',
83+
pageIds: ['host-page-1', 'host-page-2']
84+
})
85+
86+
expect(pageIds).toEqual(['host-page-2'])
87+
})
88+
89+
it('keeps minting a fresh page id with the layout-owner switch off', () => {
90+
layoutOwner.isMain = false
91+
92+
const { pageIds, workspacePageIds } = hydratedPageIds()
93+
94+
expect(pageIds).toHaveLength(1)
95+
expect(pageIds[0]).not.toBe('host-page-1')
96+
expect(workspacePageIds).toEqual(pageIds)
97+
})
98+
})

‎src/renderer/src/store/slices/browser/browser-hydration-state.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { BrowserPage, BrowserWorkspace } from '../../../../../shared/browser-workspace-types'
22
import { createBrowserUuid } from '@/lib/browser-uuid'
3+
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../shared/layout-owner-switch'
34
import { ORCA_BROWSER_BLANK_URL } from '../../../../../shared/constants'
45
import type { WorkspaceSessionState } from '../../../../../shared/workspace-session-state-types'
56
import { normalizeBrowserHistoryEntries } from '../../../../../shared/workspace-session-browser-history'
@@ -43,7 +44,10 @@ export function projectHydratedBrowserSession(
4344
? storedPages
4445
: [
4546
{
46-
id: createBrowserUuid(),
47+
// Switch point: a frame names the page before its record lands; keep that id stable.
48+
id:
49+
(ORCA_LAYOUT_OWNER_IS_MAIN ? (tab.activePageId ?? tab.pageIds?.[0]) : undefined) ??
50+
createBrowserUuid(),
4751
workspaceId: tab.id,
4852
worktreeId,
4953
url: normalizeUrl(tab.url),

‎src/renderer/src/store/slices/store-session-browser-hydration.test.ts‎

Lines changed: 8 additions & 3 deletions

0 commit comments

Comments
 (0)