mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
Address browser automation review findings
This commit is contained in:
parent
19c49e601f
commit
405315dac0
12 changed files with 629 additions and 36 deletions
|
|
@ -28,9 +28,12 @@ Last updated: 2026-04-20
|
|||
Resolution: the close request now carries `worktreeId`, and the renderer falls back to that
|
||||
worktree's active browser workspace instead of the global active browser tab.
|
||||
|
||||
- `open` `src/main/runtime/orca-runtime.ts:2153`
|
||||
Issue: `tab close --page <id>` trusts the page id directly and does not verify that it exists
|
||||
or belongs to the supplied `--worktree`.
|
||||
- `fixed` `src/main/runtime/orca-runtime.ts:2153`, `src/main/runtime/orca-runtime.test.ts`, `src/renderer/src/hooks/useIpcEvents.ts`, `src/renderer/src/hooks/useIpcEvents.test.ts`
|
||||
Issue: `tab close --page <id>` trusted the page id directly, did not verify that it existed in
|
||||
the supplied `--worktree`, and the renderer close path collapsed page ids into workspace closes.
|
||||
Resolution: main now validates explicit page ids against the scoped registered tab set before it
|
||||
sends any close request, and the renderer now closes just the targeted page unless it is the last
|
||||
page in that workspace. Unknown explicit page ids now fail instead of replying success.
|
||||
|
||||
- `fixed` `src/main/browser/agent-browser-bridge.ts:247`, `src/renderer/src/store/slices/browser.ts:598`, `src/renderer/src/store/slices/browser.ts:810`, `src/main/ipc/browser.ts:148`, `src/main/ipc/browser.test.ts`
|
||||
Issue: per-worktree active-tab routing is not updated when the renderer changes the active
|
||||
|
|
@ -38,6 +41,13 @@ Last updated: 2026-04-20
|
|||
Resolution: `browser:activeTabChanged` now forwards the owning worktree id into the bridge so
|
||||
renderer tab switches update both the global active guest and the per-worktree active guest.
|
||||
|
||||
- `fixed` `src/main/browser/agent-browser-bridge.ts`, `src/main/browser/agent-browser-bridge.test.ts`, `src/main/ipc/browser.ts`
|
||||
Issue: per-worktree active-tab routing could also go stale after a tab closed or a guest process
|
||||
swap, leaving the bridge pointed at dead webContents ids.
|
||||
Resolution: the bridge now repairs the per-worktree active guest when a tab closes, updates the
|
||||
owning worktree slot on process swaps, and keeps `tab list` read-only so discovery does not
|
||||
mutate future routing state.
|
||||
|
||||
## Screenshot behavior
|
||||
|
||||
- `fixed` `src/main/browser/agent-browser-bridge.ts:828`, `src/main/browser/agent-browser-bridge.test.ts`
|
||||
|
|
@ -65,6 +75,13 @@ Last updated: 2026-04-20
|
|||
Resolution: screenshot prep no longer forces the host BrowserWindow to the foreground. Guest
|
||||
background throttling remains disabled, so capture prep can avoid stealing app focus entirely.
|
||||
|
||||
- `fixed` `src/main/browser/browser-manager.ts`, `src/main/browser/browser-manager.test.ts`
|
||||
Issue: screenshot prep changed Orca's remembered browser workspace/page even when the visible
|
||||
pane was terminal/editor, so the next browser activation landed on the screenshot target instead
|
||||
of the user's prior hidden browser state.
|
||||
Resolution: restore now reapplies the previous browser workspace/page selection independently of
|
||||
the visible pane type, then restores the non-browser pane focus.
|
||||
|
||||
- `fixed` `src/main/browser/cdp-screenshot.ts:68`, `src/main/browser/cdp-bridge.ts:636`, `src/main/browser/cdp-screenshot.test.ts`
|
||||
Issue: the screenshot timeout fallback ignores clip/full-page geometry, so a timed-out
|
||||
clipped or full-page request can return the wrong image.
|
||||
|
|
@ -130,6 +147,12 @@ Last updated: 2026-04-20
|
|||
|
||||
## Session lifecycle and process swaps
|
||||
|
||||
- `fixed` `src/main/browser/agent-browser-bridge.ts`
|
||||
Issue: packaged macOS builds could fail to resolve the bundled `agent-browser` binary because the
|
||||
path logic assumed a lowercase `resources` directory next to the executable.
|
||||
Resolution: bundled binary lookup now uses `process.resourcesPath` (with a platform-aware test
|
||||
fallback), which matches Electron packaging on macOS and other platforms.
|
||||
|
||||
- `fixed` `src/main/browser/agent-browser-bridge.ts:264`, `src/main/browser/agent-browser-bridge.ts:1148`, `src/main/browser/agent-browser-bridge.ts:1169`, `src/main/browser/agent-browser-bridge.ts:1559`, `src/main/browser/agent-browser-bridge.test.ts`
|
||||
Issue: intercept restore after a process swap replays old intercept patterns after the first
|
||||
successful command on the new session, which can re-enable routes the caller just disabled.
|
||||
|
|
|
|||
|
|
@ -2164,9 +2164,9 @@ Terminals:
|
|||
Browser Automation:
|
||||
tab create Create a new browser tab (navigates to --url)
|
||||
tab list List open browser tabs
|
||||
tab switch Switch the active browser tab by --index
|
||||
tab close Close the active browser tab
|
||||
snapshot Accessibility snapshot with element refs (e.g. e1, e2)
|
||||
tab switch Switch the active browser tab by --index or --page
|
||||
tab close Close a browser tab by --index/--page or the current tab
|
||||
snapshot Accessibility snapshot with element refs (e.g. @e1, @e2)
|
||||
goto Navigate the active tab to --url
|
||||
click Click element by --element ref
|
||||
fill Clear and fill input by --element ref with --value
|
||||
|
|
@ -2174,7 +2174,7 @@ Browser Automation:
|
|||
select Select dropdown option by --element ref and --value
|
||||
hover Hover element by --element ref
|
||||
keypress Press a key (e.g. --key Enter, --key Tab)
|
||||
scroll Scroll --direction (up/down/left/right) by --amount pixels
|
||||
scroll Scroll --direction (up/down) by --amount pixels
|
||||
back Navigate back in browser history
|
||||
reload Reload the active browser tab
|
||||
screenshot Capture viewport screenshot (--format png|jpeg)
|
||||
|
|
@ -2191,7 +2191,7 @@ Browser Automation:
|
|||
scrollintoview Scroll --element into view
|
||||
get Get element property (--what: text, html, value, url, title)
|
||||
is Check element state (--what: visible, enabled, checked)
|
||||
keyboard inserttext Insert text without key events
|
||||
inserttext Insert text without key events
|
||||
mouse move Move mouse to --x --y coordinates
|
||||
mouse down Press mouse button
|
||||
mouse up Release mouse button
|
||||
|
|
@ -2201,7 +2201,7 @@ Browser Automation:
|
|||
set offline Toggle offline mode (--state on|off)
|
||||
set headers Set HTTP headers (--headers '{"key":"val"}')
|
||||
set credentials Set HTTP auth (--user <u> --pass <p>)
|
||||
set media Set color scheme (--scheme dark|light)
|
||||
set media Set color scheme (--color-scheme dark|light)
|
||||
clipboard read Read clipboard contents
|
||||
clipboard write Write --text to clipboard
|
||||
dialog accept Accept browser dialog (--text for prompt response)
|
||||
|
|
@ -2212,8 +2212,8 @@ Browser Automation:
|
|||
storage session get Get sessionStorage value by --key
|
||||
storage session set Set sessionStorage --key --value
|
||||
storage session clear Clear sessionStorage
|
||||
download Download file via --element to --path
|
||||
highlight Highlight --element on page
|
||||
download Download file via --selector to --path
|
||||
highlight Highlight --selector on page
|
||||
exec Run any agent-browser command (--command "...")
|
||||
|
||||
Common Commands:
|
||||
|
|
@ -2274,13 +2274,13 @@ Browser Workflow:
|
|||
(Element refs change after navigation — always re-snapshot before interacting)
|
||||
|
||||
Browser Options:
|
||||
--element <ref> Element ref from snapshot (e.g. e3, not @e3)
|
||||
--element <ref> Element ref from snapshot (e.g. @e3)
|
||||
--url <url> URL to navigate to
|
||||
--value <text> Value to fill or select
|
||||
--input <text> Text to type at current focus (no element needed)
|
||||
--expression <js> JavaScript expression to evaluate
|
||||
--key <key> Key to press (Enter, Tab, Escape, Control+a, etc.)
|
||||
--direction <dir> Scroll direction: up, down, left, right
|
||||
--direction <dir> Scroll direction: up or down
|
||||
--amount <pixels> Scroll distance in pixels (default: viewport height)
|
||||
--index <n> Tab index (from \`tab list\`)
|
||||
--page <id> Stable browser page id (preferred for concurrent workflows)
|
||||
|
|
|
|||
|
|
@ -264,6 +264,25 @@ describe('AgentBrowserBridge', () => {
|
|||
expect(result.tabs[0].browserPageId).toBe('tab-a')
|
||||
expect(result.tabs[0].url).toBe('https://a.com')
|
||||
})
|
||||
|
||||
it('does not mutate active-tab routing when tab-list infers the first live tab', () => {
|
||||
const tabs = new Map([
|
||||
['tab-a', 1],
|
||||
['tab-b', 2]
|
||||
])
|
||||
const wc1 = mockWebContents(1, 'https://a.com', 'A')
|
||||
const wc2 = mockWebContents(2, 'https://b.com', 'B')
|
||||
webContentsFromIdMock.mockImplementation((id: number) => (id === 1 ? wc1 : wc2))
|
||||
|
||||
const b = new AgentBrowserBridge(mockBrowserManager(tabs))
|
||||
|
||||
const result = b.tabList()
|
||||
expect(result.tabs).toMatchObject([
|
||||
{ browserPageId: 'tab-a', active: true },
|
||||
{ browserPageId: 'tab-b', active: false }
|
||||
])
|
||||
expect(b.getActiveWebContentsId()).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
// ── Tab switch ──
|
||||
|
|
@ -672,6 +691,42 @@ describe('AgentBrowserBridge', () => {
|
|||
expect(bridge.getActiveWebContentsId()).toBeNull()
|
||||
})
|
||||
|
||||
it('repairs per-worktree active routing when the active tab closes', async () => {
|
||||
const tabs = new Map([
|
||||
['tab-a', 1],
|
||||
['tab-b', 2]
|
||||
])
|
||||
const worktrees = new Map([
|
||||
['tab-a', 'wt-1'],
|
||||
['tab-b', 'wt-1']
|
||||
])
|
||||
const wc2 = mockWebContents(2, 'https://b.com', 'B')
|
||||
webContentsFromIdMock.mockImplementation((id: number) => (id === 2 ? wc2 : null))
|
||||
|
||||
const b = new AgentBrowserBridge(mockBrowserManager(tabs, worktrees))
|
||||
b.setActiveTab(1, 'wt-1')
|
||||
|
||||
await b.onTabClosed(1)
|
||||
|
||||
expect(b.getActiveWebContentsId()).toBe(2)
|
||||
expect(b.tabList('wt-1').tabs).toMatchObject([{ browserPageId: 'tab-b', active: true }])
|
||||
})
|
||||
|
||||
it('repairs per-worktree active routing when an active tab swaps processes', async () => {
|
||||
const tabs = new Map([['tab-a', 200]])
|
||||
const worktrees = new Map([['tab-a', 'wt-1']])
|
||||
const wc = mockWebContents(200, 'https://a.com', 'A')
|
||||
webContentsFromIdMock.mockImplementation((id: number) => (id === 200 ? wc : null))
|
||||
|
||||
const b = new AgentBrowserBridge(mockBrowserManager(tabs, worktrees))
|
||||
b.setActiveTab(100, 'wt-1')
|
||||
|
||||
await b.onProcessSwap('tab-a', 200, 100)
|
||||
|
||||
expect(b.getActiveWebContentsId()).toBe(200)
|
||||
expect(b.tabList('wt-1').tabs).toMatchObject([{ browserPageId: 'tab-a', active: true }])
|
||||
})
|
||||
|
||||
// ── tabSwitch success ──
|
||||
|
||||
it('switches active tab and returns switched index', async () => {
|
||||
|
|
|
|||
|
|
@ -92,8 +92,15 @@ function agentBrowserNativeName(): string {
|
|||
|
||||
function resolveAgentBrowserBinary(): string {
|
||||
// Why: production builds copy the platform-specific binary into resources/
|
||||
// via electron-builder extraResources. Check there first.
|
||||
const bundled = join(app.getPath('exe'), '..', 'resources', agentBrowserNativeName())
|
||||
// via electron-builder extraResources. Use Electron's resolved resourcesPath
|
||||
// instead of hand-rolling ../resources so packaged macOS builds keep working
|
||||
// on case-sensitive filesystems where Contents/Resources casing matters.
|
||||
const bundledResourcesPath =
|
||||
process.resourcesPath ??
|
||||
(process.platform === 'darwin'
|
||||
? join(app.getPath('exe'), '..', '..', 'Resources')
|
||||
: join(app.getPath('exe'), '..', 'resources'))
|
||||
const bundled = join(bundledResourcesPath, agentBrowserNativeName())
|
||||
if (existsSync(bundled)) {
|
||||
return bundled
|
||||
}
|
||||
|
|
@ -230,6 +237,23 @@ export class AgentBrowserBridge {
|
|||
}
|
||||
}
|
||||
|
||||
private selectFallbackActiveWebContents(
|
||||
worktreeId: string,
|
||||
excludedWebContentsId?: number
|
||||
): number | null {
|
||||
for (const [, wcId] of this.getRegisteredTabs(worktreeId)) {
|
||||
if (wcId === excludedWebContentsId) {
|
||||
continue
|
||||
}
|
||||
if (this.getWebContents(wcId)) {
|
||||
this.activeWebContentsPerWorktree.set(worktreeId, wcId)
|
||||
return wcId
|
||||
}
|
||||
}
|
||||
this.activeWebContentsPerWorktree.delete(worktreeId)
|
||||
return null
|
||||
}
|
||||
|
||||
getActiveWebContentsId(): number | null {
|
||||
return this.activeWebContentsId
|
||||
}
|
||||
|
|
@ -262,27 +286,55 @@ export class AgentBrowserBridge {
|
|||
}
|
||||
|
||||
async onTabClosed(webContentsId: number): Promise<void> {
|
||||
if (this.activeWebContentsId === webContentsId) {
|
||||
this.activeWebContentsId = null
|
||||
}
|
||||
const browserPageId = this.resolveTabIdSafe(webContentsId)
|
||||
const owningWorktreeId = browserPageId
|
||||
? this.browserManager.getWorktreeIdForTab(browserPageId)
|
||||
: undefined
|
||||
let nextWorktreeActiveWebContentsId: number | null = null
|
||||
if (
|
||||
owningWorktreeId &&
|
||||
this.activeWebContentsPerWorktree.get(owningWorktreeId) === webContentsId
|
||||
) {
|
||||
nextWorktreeActiveWebContentsId = this.selectFallbackActiveWebContents(
|
||||
owningWorktreeId,
|
||||
webContentsId
|
||||
)
|
||||
}
|
||||
if (this.activeWebContentsId === webContentsId) {
|
||||
this.activeWebContentsId = nextWorktreeActiveWebContentsId
|
||||
}
|
||||
if (browserPageId) {
|
||||
await this.destroySession(`orca-tab-${browserPageId}`)
|
||||
}
|
||||
}
|
||||
|
||||
async onProcessSwap(browserPageId: string, newWebContentsId: number): Promise<void> {
|
||||
async onProcessSwap(
|
||||
browserPageId: string,
|
||||
newWebContentsId: number,
|
||||
previousWebContentsId?: number
|
||||
): Promise<void> {
|
||||
// Why: Electron process swaps give same browserPageId but new webContentsId.
|
||||
// Old proxy's webContents is destroyed, so destroy session and let next command recreate.
|
||||
const sessionName = `orca-tab-${browserPageId}`
|
||||
const session = this.sessions.get(sessionName)
|
||||
const oldWebContentsId = previousWebContentsId ?? session?.webContentsId
|
||||
const owningWorktreeId = this.browserManager.getWorktreeIdForTab(browserPageId)
|
||||
// Why: save active intercept patterns before destroying so they can be restored
|
||||
// on the new session after the next successful init command.
|
||||
if (session && session.activeInterceptPatterns.length > 0) {
|
||||
this.pendingInterceptRestore.set(sessionName, [...session.activeInterceptPatterns])
|
||||
}
|
||||
await this.destroySession(sessionName)
|
||||
this.activeWebContentsId = newWebContentsId
|
||||
if (oldWebContentsId != null && this.activeWebContentsId === oldWebContentsId) {
|
||||
this.activeWebContentsId = newWebContentsId
|
||||
}
|
||||
if (
|
||||
owningWorktreeId &&
|
||||
oldWebContentsId != null &&
|
||||
this.activeWebContentsPerWorktree.get(owningWorktreeId) === oldWebContentsId
|
||||
) {
|
||||
this.activeWebContentsPerWorktree.set(owningWorktreeId, newWebContentsId)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Worktree-scoped tab queries ──
|
||||
|
|
@ -308,6 +360,8 @@ export class AgentBrowserBridge {
|
|||
const tabs = this.getRegisteredTabs(worktreeId)
|
||||
// Why: use per-worktree active tab for the "active" flag so tab-list is
|
||||
// consistent with what resolveActiveTab would pick for command routing.
|
||||
// Keep this read-only though: discovery commands must not mutate the
|
||||
// active-tab state that later bare commands rely on.
|
||||
let activeWcId =
|
||||
(worktreeId && this.activeWebContentsPerWorktree.get(worktreeId)) ?? this.activeWebContentsId
|
||||
const result: BrowserTabInfo[] = []
|
||||
|
|
@ -329,14 +383,12 @@ export class AgentBrowserBridge {
|
|||
active: wcId === activeWcId
|
||||
})
|
||||
}
|
||||
// Why: if no tab has been explicitly activated (e.g. first use after app
|
||||
// start), auto-activate the first live tab so the active flag is never
|
||||
// all-false when tabs exist.
|
||||
// Why: if no tab has been explicitly activated yet, surface the first live
|
||||
// tab as active in the listing without mutating bridge state. That keeps
|
||||
// `tab list` side-effect free while still showing users which tab a bare
|
||||
// command would select next.
|
||||
if (activeWcId == null && firstLiveWcId !== null) {
|
||||
this.activeWebContentsId = firstLiveWcId
|
||||
if (worktreeId) {
|
||||
this.activeWebContentsPerWorktree.set(worktreeId, firstLiveWcId)
|
||||
}
|
||||
activeWcId = firstLiveWcId
|
||||
if (result.length > 0) {
|
||||
result[0].active = true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -332,6 +332,67 @@ describe('browserManager', () => {
|
|||
expect(restoreScript).toContain('"page-prev"')
|
||||
})
|
||||
|
||||
it('restores remembered browser workspace/page even when the visible pane was terminal', async () => {
|
||||
const rendererExecuteJavaScriptMock = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
prevTabType: 'terminal',
|
||||
prevActiveWorktreeId: 'wt-target',
|
||||
prevActiveBrowserWorkspaceId: 'workspace-prev',
|
||||
prevActiveBrowserPageId: 'page-prev',
|
||||
prevFocusedGroupTabId: 'tab-prev',
|
||||
targetWorktreeId: 'wt-target',
|
||||
targetBrowserWorkspaceId: 'workspace-target',
|
||||
targetBrowserPageId: 'page-target'
|
||||
})
|
||||
.mockResolvedValueOnce(undefined)
|
||||
const guest = {
|
||||
id: 7091,
|
||||
isDestroyed: vi.fn(() => false),
|
||||
getType: vi.fn(() => 'webview'),
|
||||
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
|
||||
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
|
||||
on: guestOnMock,
|
||||
off: guestOffMock,
|
||||
openDevTools: guestOpenDevToolsMock
|
||||
}
|
||||
const renderer = {
|
||||
id: rendererWebContentsId,
|
||||
isDestroyed: vi.fn(() => false),
|
||||
executeJavaScript: rendererExecuteJavaScriptMock
|
||||
}
|
||||
browserWindowFromWebContentsMock.mockReturnValue({ isFocused: vi.fn(() => true) })
|
||||
webContentsFromIdMock.mockImplementation((id: number) => {
|
||||
if (id === guest.id) {
|
||||
return guest
|
||||
}
|
||||
if (id === rendererWebContentsId) {
|
||||
return renderer
|
||||
}
|
||||
return null
|
||||
})
|
||||
|
||||
browserManager.attachGuestPolicies(guest as never)
|
||||
browserManager.registerGuest({
|
||||
browserPageId: 'page-target',
|
||||
workspaceId: 'workspace-target',
|
||||
worktreeId: 'wt-target',
|
||||
webContentsId: guest.id,
|
||||
rendererWebContentsId
|
||||
})
|
||||
|
||||
const restore = await browserManager.ensureWebviewVisible(guest.id)
|
||||
restore()
|
||||
|
||||
const restoreScript = rendererExecuteJavaScriptMock.mock.calls[1]?.[0]
|
||||
expect(restoreScript).toContain('state.setActiveBrowserTab("workspace-prev");')
|
||||
expect(restoreScript).toContain('state.setActiveBrowserPage(')
|
||||
expect(restoreScript).toContain('"workspace-prev"')
|
||||
expect(restoreScript).toContain('"page-prev"')
|
||||
expect(restoreScript).toContain('state.activateTab("tab-prev");')
|
||||
expect(restoreScript).toContain('state.setActiveTabType("terminal");')
|
||||
})
|
||||
|
||||
it('does not focus the Orca window while preparing a screenshot', async () => {
|
||||
const rendererExecuteJavaScriptMock = vi.fn().mockResolvedValueOnce({
|
||||
prevTabType: 'terminal',
|
||||
|
|
|
|||
|
|
@ -288,7 +288,6 @@ export class BrowserManager {
|
|||
state = store.getState();
|
||||
}
|
||||
if (
|
||||
${JSON.stringify(prev?.prevTabType)} === 'browser' &&
|
||||
${JSON.stringify(prev?.prevActiveBrowserWorkspaceId)} &&
|
||||
${JSON.stringify(prev?.prevActiveBrowserWorkspaceId)} !==
|
||||
${JSON.stringify(prev?.targetBrowserWorkspaceId)} &&
|
||||
|
|
@ -298,19 +297,26 @@ export class BrowserManager {
|
|||
state = store.getState();
|
||||
}
|
||||
if (
|
||||
${JSON.stringify(prev?.prevTabType)} === 'browser' &&
|
||||
${JSON.stringify(prev?.prevActiveBrowserWorkspaceId)} &&
|
||||
${JSON.stringify(prev?.prevActiveBrowserPageId)} &&
|
||||
${JSON.stringify(prev?.prevActiveBrowserPageId)} !==
|
||||
${JSON.stringify(prev?.targetBrowserPageId)} &&
|
||||
typeof state.setActiveBrowserPage === 'function'
|
||||
) {
|
||||
// Why: Orca remembers the last browser workspace/page even when
|
||||
// the user is currently in terminal/editor view. Screenshot prep
|
||||
// temporarily switches that hidden browser selection state, so
|
||||
// restore it independently of the visible tab type.
|
||||
state.setActiveBrowserPage(
|
||||
${JSON.stringify(prev?.prevActiveBrowserWorkspaceId)},
|
||||
${JSON.stringify(prev?.prevActiveBrowserPageId)}
|
||||
);
|
||||
state = store.getState();
|
||||
} else if (${JSON.stringify(prev?.prevFocusedGroupTabId)}) {
|
||||
}
|
||||
if (
|
||||
${JSON.stringify(prev?.prevTabType)} !== 'browser' &&
|
||||
${JSON.stringify(prev?.prevFocusedGroupTabId)}
|
||||
) {
|
||||
state.activateTab(${JSON.stringify(prev?.prevFocusedGroupTabId)});
|
||||
}
|
||||
if (${JSON.stringify(prev?.prevTabType)} !== 'browser') {
|
||||
|
|
|
|||
|
|
@ -117,7 +117,7 @@ export function registerBrowserHandlers(): void {
|
|||
rendererWebContentsId: event.sender.id
|
||||
})
|
||||
if (agentBrowserBridgeRef && previousWcId !== null && previousWcId !== args.webContentsId) {
|
||||
agentBrowserBridgeRef.onProcessSwap(args.browserPageId, args.webContentsId)
|
||||
agentBrowserBridgeRef.onProcessSwap(args.browserPageId, args.webContentsId, previousWcId)
|
||||
}
|
||||
const pendingResolve = pendingTabRegistrations.get(args.browserPageId)
|
||||
if (pendingResolve) {
|
||||
|
|
|
|||
|
|
@ -873,5 +873,49 @@ describe('OrcaRuntimeService', () => {
|
|||
).rejects.toThrow('selector_not_found')
|
||||
expect(tabListMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects closing an unknown page id instead of treating it as success', async () => {
|
||||
vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES)
|
||||
const runtime = createRuntime()
|
||||
|
||||
runtime.setAgentBrowserBridge({
|
||||
getRegisteredTabs: vi.fn(() => new Map([['page-1', 1]]))
|
||||
} as never)
|
||||
|
||||
await expect(
|
||||
runtime.browserTabClose({
|
||||
page: 'missing-page'
|
||||
})
|
||||
).rejects.toThrow('Browser page missing-page was not found')
|
||||
})
|
||||
|
||||
it('rejects closing a page outside the explicitly scoped worktree', async () => {
|
||||
vi.mocked(listWorktrees).mockResolvedValue([
|
||||
...MOCK_GIT_WORKTREES,
|
||||
{
|
||||
path: '/tmp/worktree-b',
|
||||
head: 'def',
|
||||
branch: 'feature/bar',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
const runtime = createRuntime()
|
||||
const getRegisteredTabsMock = vi.fn((worktreeId?: string) =>
|
||||
worktreeId === `${TEST_REPO_ID}::/tmp/worktree-b` ? new Map() : new Map([['page-1', 1]])
|
||||
)
|
||||
|
||||
runtime.setAgentBrowserBridge({
|
||||
getRegisteredTabs: getRegisteredTabsMock
|
||||
} as never)
|
||||
|
||||
await expect(
|
||||
runtime.browserTabClose({
|
||||
page: 'page-1',
|
||||
worktree: 'path:/tmp/worktree-b'
|
||||
})
|
||||
).rejects.toThrow('Browser page page-1 was not found in this worktree')
|
||||
expect(getRegisteredTabsMock).toHaveBeenCalledWith(`${TEST_REPO_ID}::/tmp/worktree-b`)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -2148,12 +2148,18 @@ export class OrcaRuntimeService {
|
|||
page?: string
|
||||
worktree?: string
|
||||
}): Promise<{ closed: boolean }> {
|
||||
const win = this.getAuthoritativeWindow()
|
||||
const bridge = this.requireAgentBrowserBridge()
|
||||
const worktreeId = await this.resolveBrowserWorktreeId(params.worktree)
|
||||
|
||||
let tabId: string | null = null
|
||||
if (typeof params.page === 'string' && params.page.length > 0) {
|
||||
if (!bridge.getRegisteredTabs(worktreeId).has(params.page)) {
|
||||
const scope = worktreeId ? ' in this worktree' : ''
|
||||
throw new BrowserError(
|
||||
'browser_tab_not_found',
|
||||
`Browser page ${params.page} was not found${scope}`
|
||||
)
|
||||
}
|
||||
tabId = params.page
|
||||
} else if (params.index !== undefined) {
|
||||
const tabs = bridge.getRegisteredTabs(worktreeId)
|
||||
|
|
@ -2174,6 +2180,7 @@ export class OrcaRuntimeService {
|
|||
}
|
||||
}
|
||||
|
||||
const win = this.getAuthoritativeWindow()
|
||||
const requestId = randomUUID()
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
|
|
|
|||
|
|
@ -392,6 +392,7 @@ describe('useIpcEvents browser tab close routing', () => {
|
|||
|
||||
it('closes the active browser tab for the requested worktree when main does not provide a tab id', async () => {
|
||||
const closeBrowserTab = vi.fn()
|
||||
const closeBrowserPage = vi.fn()
|
||||
const replyTabClose = vi.fn()
|
||||
const tabCloseListenerRef: {
|
||||
current:
|
||||
|
|
@ -447,7 +448,8 @@ describe('useIpcEvents browser tab close routing', () => {
|
|||
'wt-2': [{ id: 'workspace-target' }]
|
||||
},
|
||||
browserPagesByWorkspace: {},
|
||||
closeBrowserTab
|
||||
closeBrowserTab,
|
||||
closeBrowserPage
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
|
@ -547,8 +549,333 @@ describe('useIpcEvents browser tab close routing', () => {
|
|||
})
|
||||
|
||||
expect(closeBrowserTab).toHaveBeenCalledWith('workspace-target')
|
||||
expect(closeBrowserPage).not.toHaveBeenCalled()
|
||||
expect(replyTabClose).toHaveBeenCalledWith({ requestId: 'req-1' })
|
||||
})
|
||||
|
||||
it('closes only the requested browser page when a workspace has multiple pages', async () => {
|
||||
const closeBrowserTab = vi.fn()
|
||||
const closeBrowserPage = vi.fn()
|
||||
const replyTabClose = vi.fn()
|
||||
const tabCloseListenerRef: {
|
||||
current:
|
||||
| ((data: { requestId: string; tabId: string | null; worktreeId?: string }) => void)
|
||||
| null
|
||||
} = {
|
||||
current: null
|
||||
}
|
||||
|
||||
vi.doMock('react', async () => {
|
||||
const actual = await vi.importActual<typeof ReactModule>('react')
|
||||
return {
|
||||
...actual,
|
||||
useEffect: (effect: () => void | (() => void)) => {
|
||||
effect()
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
vi.doMock('../store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => ({
|
||||
setUpdateStatus: vi.fn(),
|
||||
fetchRepos: vi.fn(),
|
||||
fetchWorktrees: vi.fn(),
|
||||
setActiveView: vi.fn(),
|
||||
activeModal: null,
|
||||
closeModal: vi.fn(),
|
||||
openModal: vi.fn(),
|
||||
activeWorktreeId: 'wt-1',
|
||||
activeView: 'terminal',
|
||||
setActiveRepo: vi.fn(),
|
||||
setActiveWorktree: vi.fn(),
|
||||
revealWorktreeInSidebar: vi.fn(),
|
||||
setIsFullScreen: vi.fn(),
|
||||
updateBrowserTabPageState: vi.fn(),
|
||||
activeTabType: 'browser',
|
||||
editorFontZoomLevel: 0,
|
||||
setEditorFontZoomLevel: vi.fn(),
|
||||
setRateLimitsFromPush: vi.fn(),
|
||||
setSshConnectionState: vi.fn(),
|
||||
setSshTargetLabels: vi.fn(),
|
||||
enqueueSshCredentialRequest: vi.fn(),
|
||||
removeSshCredentialRequest: vi.fn(),
|
||||
settings: { terminalFontSize: 13 },
|
||||
activeBrowserTabId: 'workspace-1',
|
||||
activeBrowserTabIdByWorktree: { 'wt-1': 'workspace-1' },
|
||||
browserTabsByWorktree: {
|
||||
'wt-1': [{ id: 'workspace-1' }]
|
||||
},
|
||||
browserPagesByWorkspace: {
|
||||
'workspace-1': [
|
||||
{ id: 'page-1', workspaceId: 'workspace-1' },
|
||||
{ id: 'page-2', workspaceId: 'workspace-1' }
|
||||
]
|
||||
},
|
||||
closeBrowserTab,
|
||||
closeBrowserPage
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
||||
vi.doMock('@/lib/ui-zoom', () => ({
|
||||
applyUIZoom: vi.fn()
|
||||
}))
|
||||
vi.doMock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorktree: vi.fn(),
|
||||
ensureWorktreeHasInitialTerminal: vi.fn()
|
||||
}))
|
||||
vi.doMock('@/components/sidebar/visible-worktrees', () => ({
|
||||
getVisibleWorktreeIds: () => []
|
||||
}))
|
||||
vi.doMock('@/lib/editor-font-zoom', () => ({
|
||||
nextEditorFontZoomLevel: vi.fn(() => 0),
|
||||
computeEditorFontSize: vi.fn(() => 13)
|
||||
}))
|
||||
vi.doMock('@/components/settings/SettingsConstants', () => ({
|
||||
zoomLevelToPercent: vi.fn(() => 100),
|
||||
ZOOM_MIN: -3,
|
||||
ZOOM_MAX: 3
|
||||
}))
|
||||
vi.doMock('@/lib/zoom-events', () => ({
|
||||
dispatchZoomLevelChanged: vi.fn()
|
||||
}))
|
||||
|
||||
vi.stubGlobal('window', {
|
||||
dispatchEvent: vi.fn(),
|
||||
api: {
|
||||
repos: { onChanged: () => () => {} },
|
||||
worktrees: { onChanged: () => () => {} },
|
||||
ui: {
|
||||
onOpenSettings: () => () => {},
|
||||
onToggleLeftSidebar: () => () => {},
|
||||
onToggleRightSidebar: () => () => {},
|
||||
onToggleWorktreePalette: () => () => {},
|
||||
onOpenQuickOpen: () => () => {},
|
||||
onJumpToWorktreeIndex: () => () => {},
|
||||
onActivateWorktree: () => () => {},
|
||||
onNewBrowserTab: () => () => {},
|
||||
onRequestTabCreate: () => () => {},
|
||||
replyTabCreate: () => {},
|
||||
onRequestTabClose: (
|
||||
listener: (data: {
|
||||
requestId: string
|
||||
tabId: string | null
|
||||
worktreeId?: string
|
||||
}) => void
|
||||
) => {
|
||||
tabCloseListenerRef.current = listener
|
||||
return () => {}
|
||||
},
|
||||
replyTabClose,
|
||||
onNewTerminalTab: () => () => {},
|
||||
onCloseActiveTab: () => () => {},
|
||||
onSwitchTab: () => () => {},
|
||||
onToggleStatusBar: () => () => {},
|
||||
onFullscreenChanged: () => () => {},
|
||||
onTerminalZoom: () => () => {},
|
||||
getZoomLevel: () => 0,
|
||||
set: vi.fn()
|
||||
},
|
||||
updater: {
|
||||
getStatus: () => Promise.resolve({ state: 'idle' }),
|
||||
onStatus: () => () => {},
|
||||
onClearDismissal: () => () => {}
|
||||
},
|
||||
browser: {
|
||||
onGuestLoadFailed: () => () => {},
|
||||
onOpenLinkInOrcaTab: () => () => {},
|
||||
onNavigationUpdate: () => () => {},
|
||||
onActivateView: () => () => {}
|
||||
},
|
||||
rateLimits: {
|
||||
get: () => Promise.resolve({ limits: {}, lastUpdatedAt: Date.now() }),
|
||||
onUpdate: () => () => {}
|
||||
},
|
||||
ssh: {
|
||||
listTargets: () => Promise.resolve([]),
|
||||
getState: () => Promise.resolve(null),
|
||||
onStateChanged: () => () => {},
|
||||
onCredentialRequest: () => () => {},
|
||||
onCredentialResolved: () => () => {}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
const { useIpcEvents } = await import('./useIpcEvents')
|
||||
useIpcEvents()
|
||||
|
||||
tabCloseListenerRef.current?.({
|
||||
requestId: 'req-2',
|
||||
tabId: 'page-2'
|
||||
})
|
||||
|
||||
expect(closeBrowserPage).toHaveBeenCalledWith('page-2')
|
||||
expect(closeBrowserTab).not.toHaveBeenCalled()
|
||||
expect(replyTabClose).toHaveBeenCalledWith({ requestId: 'req-2' })
|
||||
})
|
||||
|
||||
it('rejects explicit unknown browser page ids instead of reporting success', async () => {
|
||||
const closeBrowserTab = vi.fn()
|
||||
const closeBrowserPage = vi.fn()
|
||||
const replyTabClose = vi.fn()
|
||||
const tabCloseListenerRef: {
|
||||
current:
|
||||
| ((data: { requestId: string; tabId: string | null; worktreeId?: string }) => void)
|
||||
| null
|
||||
} = {
|
||||
current: null
|
||||
}
|
||||
|
||||
vi.doMock('react', async () => {
|
||||
const actual = await vi.importActual<typeof ReactModule>('react')
|
||||
return {
|
||||
...actual,
|
||||
useEffect: (effect: () => void | (() => void)) => {
|
||||
effect()
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
vi.doMock('../store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => ({
|
||||
setUpdateStatus: vi.fn(),
|
||||
fetchRepos: vi.fn(),
|
||||
fetchWorktrees: vi.fn(),
|
||||
setActiveView: vi.fn(),
|
||||
activeModal: null,
|
||||
closeModal: vi.fn(),
|
||||
openModal: vi.fn(),
|
||||
activeWorktreeId: 'wt-1',
|
||||
activeView: 'terminal',
|
||||
setActiveRepo: vi.fn(),
|
||||
setActiveWorktree: vi.fn(),
|
||||
revealWorktreeInSidebar: vi.fn(),
|
||||
setIsFullScreen: vi.fn(),
|
||||
updateBrowserTabPageState: vi.fn(),
|
||||
activeTabType: 'browser',
|
||||
editorFontZoomLevel: 0,
|
||||
setEditorFontZoomLevel: vi.fn(),
|
||||
setRateLimitsFromPush: vi.fn(),
|
||||
setSshConnectionState: vi.fn(),
|
||||
setSshTargetLabels: vi.fn(),
|
||||
enqueueSshCredentialRequest: vi.fn(),
|
||||
removeSshCredentialRequest: vi.fn(),
|
||||
settings: { terminalFontSize: 13 },
|
||||
activeBrowserTabId: 'workspace-1',
|
||||
activeBrowserTabIdByWorktree: { 'wt-1': 'workspace-1' },
|
||||
browserTabsByWorktree: {
|
||||
'wt-1': [{ id: 'workspace-1' }]
|
||||
},
|
||||
browserPagesByWorkspace: {
|
||||
'workspace-1': [{ id: 'page-1', workspaceId: 'workspace-1' }]
|
||||
},
|
||||
closeBrowserTab,
|
||||
closeBrowserPage
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
||||
vi.doMock('@/lib/ui-zoom', () => ({
|
||||
applyUIZoom: vi.fn()
|
||||
}))
|
||||
vi.doMock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorktree: vi.fn(),
|
||||
ensureWorktreeHasInitialTerminal: vi.fn()
|
||||
}))
|
||||
vi.doMock('@/components/sidebar/visible-worktrees', () => ({
|
||||
getVisibleWorktreeIds: () => []
|
||||
}))
|
||||
vi.doMock('@/lib/editor-font-zoom', () => ({
|
||||
nextEditorFontZoomLevel: vi.fn(() => 0),
|
||||
computeEditorFontSize: vi.fn(() => 13)
|
||||
}))
|
||||
vi.doMock('@/components/settings/SettingsConstants', () => ({
|
||||
zoomLevelToPercent: vi.fn(() => 100),
|
||||
ZOOM_MIN: -3,
|
||||
ZOOM_MAX: 3
|
||||
}))
|
||||
vi.doMock('@/lib/zoom-events', () => ({
|
||||
dispatchZoomLevelChanged: vi.fn()
|
||||
}))
|
||||
|
||||
vi.stubGlobal('window', {
|
||||
dispatchEvent: vi.fn(),
|
||||
api: {
|
||||
repos: { onChanged: () => () => {} },
|
||||
worktrees: { onChanged: () => () => {} },
|
||||
ui: {
|
||||
onOpenSettings: () => () => {},
|
||||
onToggleLeftSidebar: () => () => {},
|
||||
onToggleRightSidebar: () => () => {},
|
||||
onToggleWorktreePalette: () => () => {},
|
||||
onOpenQuickOpen: () => () => {},
|
||||
onJumpToWorktreeIndex: () => () => {},
|
||||
onActivateWorktree: () => () => {},
|
||||
onNewBrowserTab: () => () => {},
|
||||
onRequestTabCreate: () => () => {},
|
||||
replyTabCreate: () => {},
|
||||
onRequestTabClose: (
|
||||
listener: (data: {
|
||||
requestId: string
|
||||
tabId: string | null
|
||||
worktreeId?: string
|
||||
}) => void
|
||||
) => {
|
||||
tabCloseListenerRef.current = listener
|
||||
return () => {}
|
||||
},
|
||||
replyTabClose,
|
||||
onNewTerminalTab: () => () => {},
|
||||
onCloseActiveTab: () => () => {},
|
||||
onSwitchTab: () => () => {},
|
||||
onToggleStatusBar: () => () => {},
|
||||
onFullscreenChanged: () => () => {},
|
||||
onTerminalZoom: () => () => {},
|
||||
getZoomLevel: () => 0,
|
||||
set: vi.fn()
|
||||
},
|
||||
updater: {
|
||||
getStatus: () => Promise.resolve({ state: 'idle' }),
|
||||
onStatus: () => () => {},
|
||||
onClearDismissal: () => () => {}
|
||||
},
|
||||
browser: {
|
||||
onGuestLoadFailed: () => () => {},
|
||||
onOpenLinkInOrcaTab: () => () => {},
|
||||
onNavigationUpdate: () => () => {},
|
||||
onActivateView: () => () => {}
|
||||
},
|
||||
rateLimits: {
|
||||
get: () => Promise.resolve({ limits: {}, lastUpdatedAt: Date.now() }),
|
||||
onUpdate: () => () => {}
|
||||
},
|
||||
ssh: {
|
||||
listTargets: () => Promise.resolve([]),
|
||||
getState: () => Promise.resolve(null),
|
||||
onStateChanged: () => () => {},
|
||||
onCredentialRequest: () => () => {},
|
||||
onCredentialResolved: () => () => {}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
const { useIpcEvents } = await import('./useIpcEvents')
|
||||
useIpcEvents()
|
||||
|
||||
tabCloseListenerRef.current?.({
|
||||
requestId: 'req-3',
|
||||
tabId: 'missing-page'
|
||||
})
|
||||
|
||||
expect(closeBrowserPage).not.toHaveBeenCalled()
|
||||
expect(closeBrowserTab).not.toHaveBeenCalled()
|
||||
expect(replyTabClose).toHaveBeenCalledWith({
|
||||
requestId: 'req-3',
|
||||
error: 'Browser tab missing-page not found'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('useIpcEvents shortcut hint clearing', () => {
|
||||
|
|
|
|||
|
|
@ -241,8 +241,9 @@ export function useIpcEvents(): void {
|
|||
window.api.ui.onRequestTabClose((data) => {
|
||||
try {
|
||||
const store = useAppStore.getState()
|
||||
const explicitTargetId = data.tabId ?? null
|
||||
let tabToClose =
|
||||
data.tabId ??
|
||||
explicitTargetId ??
|
||||
(data.worktreeId
|
||||
? (store.activeBrowserTabIdByWorktree?.[data.worktreeId] ?? null)
|
||||
: store.activeBrowserTabId)
|
||||
|
|
@ -255,7 +256,9 @@ export function useIpcEvents(): void {
|
|||
}
|
||||
// Why: the bridge stores tabs keyed by browserPageId (which is the page
|
||||
// ID from registerGuest), but closeBrowserTab expects a workspace ID. If
|
||||
// tabToClose is a page ID, resolve it to its owning workspace ID.
|
||||
// tabToClose is a page ID, close only that page unless it is the
|
||||
// last page in its workspace. The CLI's `tab close --page` contract
|
||||
// targets one browser page, not the entire workspace tab.
|
||||
const isWorkspaceId = Object.values(store.browserTabsByWorktree)
|
||||
.flat()
|
||||
.some((ws) => ws.id === tabToClose)
|
||||
|
|
@ -264,9 +267,23 @@ export function useIpcEvents(): void {
|
|||
([, pages]) => pages.some((p) => p.id === tabToClose)
|
||||
)
|
||||
if (owningWorkspace) {
|
||||
tabToClose = owningWorkspace[0]
|
||||
const [workspaceId, pages] = owningWorkspace
|
||||
if (pages.length <= 1) {
|
||||
store.closeBrowserTab(workspaceId)
|
||||
} else {
|
||||
store.closeBrowserPage(tabToClose)
|
||||
}
|
||||
window.api.ui.replyTabClose({ requestId: data.requestId })
|
||||
return
|
||||
}
|
||||
}
|
||||
if (explicitTargetId) {
|
||||
window.api.ui.replyTabClose({
|
||||
requestId: data.requestId,
|
||||
error: `Browser tab ${explicitTargetId} not found`
|
||||
})
|
||||
return
|
||||
}
|
||||
store.closeBrowserTab(tabToClose)
|
||||
window.api.ui.replyTabClose({ requestId: data.requestId })
|
||||
} catch (err) {
|
||||
|
|
|
|||
|
|
@ -385,6 +385,7 @@ export type BrowserTabCloseResult = {
|
|||
export type BrowserErrorCode =
|
||||
| 'browser_no_tab'
|
||||
| 'browser_tab_not_found'
|
||||
| 'browser_tab_closed'
|
||||
| 'browser_stale_ref'
|
||||
| 'browser_ref_not_found'
|
||||
| 'browser_navigation_failed'
|
||||
|
|
|
|||
Loading…
Reference in a new issue