From a9dc83ccbced6fc300094ddcf9b9b41ab7ee0b15 Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:27:13 +0100 Subject: [PATCH 1/3] fix(browser): refuse an act or evaluate that reuses another tab's observation Observation revisions count per tab, so two tabs can share them. An observation from one tab could then pass the currency check on another and run its refs there. Bind act and evaluate observations to the tab they were taken from. --- .../__tests__/browserSessionDriver.test.ts | 22 ++++++++++++++++--- .../services/browserSessionDriver.ts | 14 ++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/web/src/features/preview/automation/services/__tests__/browserSessionDriver.test.ts b/apps/web/src/features/preview/automation/services/__tests__/browserSessionDriver.test.ts index 52df9e839..68e480496 100644 --- a/apps/web/src/features/preview/automation/services/__tests__/browserSessionDriver.test.ts +++ b/apps/web/src/features/preview/automation/services/__tests__/browserSessionDriver.test.ts @@ -415,7 +415,7 @@ describe("BrowserSessionDriver", () => { const inspect = await driver.execute({ connection: { connectionGeneration: 1, capabilityRevision: 1 }, target: { threadId: "thread", tabId: "tab", targetGeneration: 1 }, - request: { contractVersion: 1, requestId: "inspect", sequence: 1, operation: "inspect", expectedControlEpoch: 0 }, + request: { contractVersion: 1, workspaceId: "workspace", requestId: "inspect", sequence: 1, operation: "inspect", expectedControlEpoch: 0 }, } as unknown as BrowserAutomationHostDispatch, new AbortController().signal); const observationRef = (inspect as { result?: { observationRef?: string } }).result?.observationRef; expect(observationRef).toEqual(expect.any(String)); @@ -536,7 +536,7 @@ describe("BrowserSessionDriver", () => { const inspect = await driver.execute({ connection: { connectionGeneration: 1, capabilityRevision: 1 }, target: { threadId: "thread", tabId: "tab", targetGeneration: 1 }, - request: { contractVersion: 1, requestId: "inspect-issued", sequence: 1, operation: "inspect", expectedControlEpoch: 0 }, + request: { contractVersion: 1, workspaceId: "workspace", requestId: "inspect-issued", sequence: 1, operation: "inspect", expectedControlEpoch: 0 }, } as unknown as BrowserAutomationHostDispatch, new AbortController().signal); expect(inspect).toMatchObject({ ok: true, result: { observationRef: "driver-issued" } }); await expect(driver.execute(actDispatch("driver-issued", [{ operation: "click", target: { cssSelector: "#save" } }]), new AbortController().signal)) @@ -544,6 +544,22 @@ describe("BrowserSessionDriver", () => { expect(execute).toHaveBeenCalledTimes(2); }); + it("refuses an act or evaluate that reuses another tab's observation", async () => { + const execute = vi.fn(async (dispatch: BrowserAutomationHostDispatch) => ( + { ok: true, result: { operation: dispatch.request.operation } } as unknown as BrowserAutomationResponse + )); + const driver = new BrowserSessionDriver({ web: { execute }, electron: { execute }, isElectron: () => true, supportedActOperations: ["click"] }); + const userTab = { ...inspectDispatch("read-user-tab"), target: { threadId: "thread", tabId: "user-tab", targetGeneration: 1, windowId: 1 } } as BrowserAutomationHostDispatch; + const read = await driver.execute(userTab, new AbortController().signal); + const observationRef = (read as { result?: { observationRef?: string } }).result?.observationRef; + + await expect(driver.execute(actDispatch(observationRef!, [{ operation: "click", target: { cssSelector: "#save" } }]), new AbortController().signal)) + .resolves.toMatchObject({ ok: false, error: { code: "STALE_TARGET_GENERATION" } }); + await expect(driver.execute(evaluateDispatch(observationRef!), new AbortController().signal)) + .resolves.toMatchObject({ ok: false, error: { code: "STALE_TARGET_GENERATION" } }); + expect(execute.mock.calls.map(([dispatch]) => dispatch.request.operation)).toEqual(["inspect"]); + }); + it("rejects unsupported steps before the first adapter effect", async () => { const execute = vi.fn().mockResolvedValue({ ok: true, result: { operation: "click" } } as unknown as BrowserAutomationResponse); const driver = new BrowserSessionDriver({ web: { execute }, electron: { execute }, isElectron: () => false, supportedActOperations: ["click"] }); @@ -596,7 +612,7 @@ describe("BrowserSessionDriver", () => { return { ok: true, result: { operation: dispatch.request.operation } } as unknown as BrowserAutomationResponse; }); const driver = new BrowserSessionDriver({ web: { execute }, electron: { execute }, isElectron: () => false, getControlRevision: () => controlRevision, supportedActOperations: ["click", "type"] }); - const inspect = await driver.execute({ connection: { connectionGeneration: 1, capabilityRevision: 1 }, target: { threadId: "thread", tabId: "tab", targetGeneration: 1 }, request: { contractVersion: 1, requestId: "inspect", sequence: 1, operation: "inspect", expectedControlEpoch: 0 } } as unknown as BrowserAutomationHostDispatch, new AbortController().signal); + const inspect = await driver.execute({ connection: { connectionGeneration: 1, capabilityRevision: 1 }, target: { threadId: "thread", tabId: "tab", targetGeneration: 1 }, request: { contractVersion: 1, workspaceId: "workspace", requestId: "inspect", sequence: 1, operation: "inspect", expectedControlEpoch: 0 } } as unknown as BrowserAutomationHostDispatch, new AbortController().signal); const observationRef = (inspect as { result: { observationRef: string } }).result.observationRef; const result = await driver.execute(actDispatch(observationRef, [{ operation: "click", target: { cssSelector: "#save" } }, { operation: "type", text: "secret" }]), new AbortController().signal); expect(result).toMatchObject({ ok: true, result: { outcome: "interrupted", effect: "partial", receipts: [{ index: 0, status: "applied" }, { index: 1, status: "interrupted" }] } }); diff --git a/apps/web/src/features/preview/automation/services/browserSessionDriver.ts b/apps/web/src/features/preview/automation/services/browserSessionDriver.ts index cc0432dfe..bba89db0a 100644 --- a/apps/web/src/features/preview/automation/services/browserSessionDriver.ts +++ b/apps/web/src/features/preview/automation/services/browserSessionDriver.ts @@ -610,7 +610,8 @@ export class BrowserSessionDriver { capabilityRevision: number, ): boolean { const current = this.currentObservationBinding(dispatch, capabilityRevision); - return observation.hostRevision === current.hostRevision && + return this.observedDispatchTarget(dispatch, observation) && + observation.hostRevision === current.hostRevision && observation.documentRevision === current.documentRevision && observation.controlRevision === current.controlRevision && observation.capabilityRevision === current.capabilityRevision && @@ -859,12 +860,21 @@ export class BrowserSessionDriver { capabilityRevision: number, ): boolean { const current = this.currentObservationBinding(dispatch, capabilityRevision); - return observation.hostRevision === current.hostRevision && + return this.observedDispatchTarget(dispatch, observation) && + observation.hostRevision === current.hostRevision && observation.documentRevision === current.documentRevision && observation.controlRevision === current.controlRevision && observation.capabilityRevision === current.capabilityRevision; } + /** + * Revisions count per tab, so two tabs can share them. An inspect can read a + * tab other than the one effects go to, so its refs must not reach another tab. + */ + private observedDispatchTarget(dispatch: BrowserAutomationHostDispatch, observation: ObservationRecord): boolean { + return observation.targetKey === observationTargetKey(dispatch.request.workspaceId, dispatch.target.threadId, dispatch.target.tabId); + } + private evaluatePreEffectInterruption( dispatch: BrowserAutomationHostDispatch, observation: ObservationRecord, From 6bf49c0709c88c7419db5adffcfc44322eb3b878 Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:27:13 +0100 Subject: [PATCH 2/3] feat(browser): let the agent read the user's own Browser tab browser_inspect takes an optional tabId to read any tab in the thread, including one the user holds. A named read never claims control and never moves the agent's current tab. The inspect tab listing now shows who holds each tab, taken from the holder the broker enforces. Readiness and guidance reflect a user hold anywhere in the thread, since that hold blocks every effect. Closes #2219 --- .../execution/__tests__/broker.test.ts | 2 +- .../__tests__/broker.user-tab.test.ts | 267 ++++++++++++++++++ .../browser-automation/execution/broker.ts | 49 +++- .../__tests__/browser-toolkit.test.ts | 9 + .../toolkits/browser-tool-schemas.ts | 8 +- .../src/models/browser-automation.ts | 2 + 6 files changed, 324 insertions(+), 13 deletions(-) create mode 100644 apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts diff --git a/apps/server/src/features/browser-automation/execution/__tests__/broker.test.ts b/apps/server/src/features/browser-automation/execution/__tests__/broker.test.ts index e4a5efdf6..fdb2dff06 100644 --- a/apps/server/src/features/browser-automation/execution/__tests__/broker.test.ts +++ b/apps/server/src/features/browser-automation/execution/__tests__/broker.test.ts @@ -284,7 +284,7 @@ describe("BrowserAutomationBroker", () => { result: { controlEpoch: 3, readiness: { ready: false, state: "human-control" }, - guidance: "The user has control of the Browser. Do not act until browser_inspect reports ready; the user hands control back.", + guidance: "The user has control of the Browser. browser_inspect can still read any tab. Do not act until browser_inspect reports ready; the user hands control back.", }, }); broker.disconnect(hostSocket); diff --git a/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts b/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts new file mode 100644 index 000000000..5033b6384 --- /dev/null +++ b/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts @@ -0,0 +1,267 @@ +import { describe, expect, it } from "vitest"; +import { + BROWSER_AUTOMATION_CONTRACT_VERSION, + BROWSER_AUTOMATION_OPERATIONS, + type BrowserAutomationHostDispatchTarget, + type BrowserAutomationHostRegistration, + type BrowserAutomationRequest, +} from "@mcode/contracts"; +import type { WebSocket } from "ws"; +import { BrowserAutomationBroker } from "../broker.js"; +import type { BrowserAutomationCredentialClaims } from "../../access/credential-registry.js"; + +const HOST_ID = "user-tab-host"; +const USER_TAB = "user-tab"; +const AGENT_TAB = "agent-tab"; + +type Controller = NonNullable; + +interface Delivery { + readonly dispatch: { readonly request: BrowserAutomationRequest; readonly target: BrowserAutomationHostDispatchTarget }; +} + +function registration(): BrowserAutomationHostRegistration { + return { + contractVersion: BROWSER_AUTOMATION_CONTRACT_VERSION, + hostId: HOST_ID, + runtime: "electron", + desktopInstanceId: `desktop-${HOST_ID}`, + worktreeIdentity: "worktree-a", + workspaceIds: ["workspace-a"], + executorDescriptor: { + runtime: "electron", + operations: [...BROWSER_AUTOMATION_OPERATIONS], + constraints: { maxTabs: 32, maxSnapshotChars: 20_000, maxDiagnostics: 200 }, + capabilityRevision: 1, + }, + capabilities: BROWSER_AUTOMATION_OPERATIONS.map((operation) => ({ operation, available: true })), + maxPendingRequests: 4, + connectedAt: 1, + }; +} + +const scope: BrowserAutomationCredentialClaims = { + credentialId: "credential-thread-a", + providerId: "cursor", + providerSessionId: "provider-thread-a", + mcodeSessionId: "mcode-thread-a", + threadId: "thread-a", + workspaceId: "workspace-a", + worktreeIdentity: "worktree-a", + permissionCapability: "interact", + allowedOperations: ["open", "inspect", "act", "tabs"], + issuedAt: 1, + expiresAt: 100_000, +}; + +function target( + generation: number, + tabId: string, + overrides: Partial = {}, +): BrowserAutomationHostDispatchTarget { + return { + desktopInstanceId: `desktop-${HOST_ID}`, + windowId: 1, + connectionGeneration: generation, + threadId: "thread-a", + tabId, + targetGeneration: 1, + active: false, + focused: false, + lastUsedAt: 1, + ...overrides, + }; +} + +function userHold(epoch: number): Controller { + return { tabId: USER_TAB, controller: "human", controlEpoch: epoch }; +} + +/** A Browser with a user-opened tab and an agent tab in one thread, reachable through one host. */ +function browser(options: { readonly userController?: Controller; readonly focusedTab?: string; readonly agentTab?: boolean } = {}) { + const deliveries: Delivery[] = []; + const broker = new BrowserAutomationBroker({ + now: () => 10, + send: (_socket, channel, data) => { + if (channel === "browserAutomation.request") deliveries.push(data as Delivery); + return true; + }, + }); + const hostSocket = { name: HOST_ID } as unknown as WebSocket; + const generation = broker.registerHost(hostSocket, registration(), { + desktopInstanceId: `desktop-${HOST_ID}`, + worktreeIdentity: "worktree-a", + allowedWorkspaceIds: ["workspace-a"], + }).generation; + const focusedTab = options.focusedTab ?? USER_TAB; + const publish = (userController: Controller | undefined): void => { + broker.updateTargets(hostSocket, HOST_ID, generation, [ + target(generation, USER_TAB, { + active: true, + focused: focusedTab === USER_TAB, + ...(userController ? { controller: userController } : {}), + }), + ...(options.agentTab === false ? [] : [target(generation, AGENT_TAB, { focused: focusedTab === AGENT_TAB, lastUsedAt: 2 })]), + ]); + }; + publish(options.userController); + let sequence = 0; + const send = (operation: BrowserAutomationRequest["operation"], args: Record, expectedControlEpoch = 0) => { + sequence += 1; + const request = { + contractVersion: BROWSER_AUTOMATION_CONTRACT_VERSION, + workspaceId: scope.workspaceId, + threadId: scope.threadId, + providerSessionId: scope.providerSessionId, + providerInstanceId: scope.mcodeSessionId, + requestId: `request-${sequence}`, + sequence, + deadline: 50_000, + expectedControlEpoch, + operation, + args, + } as BrowserAutomationRequest; + return broker.execute(scope, request); + }; + /** Answers the latest delivery the way a host answers an inspect of its target. */ + const answerInspect = (controlEpoch = 0): Delivery => { + const delivery = deliveries.at(-1)!; + const inspected = delivery.dispatch.target; + broker.respond(hostSocket, HOST_ID, generation, { + contractVersion: BROWSER_AUTOMATION_CONTRACT_VERSION, + requestId: delivery.dispatch.request.requestId, + sequence: delivery.dispatch.request.sequence, + ok: true, + result: { + operation: "inspect", + controlEpoch, + // Hosts list the thread's tabs without their holder. + tabs: [target(generation, USER_TAB, { active: true }), target(generation, AGENT_TAB, { lastUsedAt: 2 })], + observationRef: `observation-${delivery.dispatch.request.sequence}`, + snapshot: snapshotOf(inspected.tabId), + }, + }); + return delivery; + }; + return { broker, deliveries, publish, send, answerInspect }; +} + +function snapshotOf(tabId: string) { + const none = { truncated: false as const }; + return { + url: `https://example.test/${tabId}`, + title: `Page in ${tabId}`, + loading: false, + visibleText: `Content of ${tabId}`, + visibleTextTruncation: none, + elements: [], + elementsTruncation: none, + accessibility: [], + accessibilityTruncation: none, + console: [], + consoleTruncation: none, + network: [], + networkTruncation: none, + actions: [], + actionsTruncation: none, + }; +} + +const INSPECT = { includeScreenshot: false, includeDiagnostics: false }; + +describe("BrowserAutomationBroker reading the user's own tab", () => { + it("reads the user's tab when the agent has no tab of its own", async () => { + const { send, answerInspect } = browser({ agentTab: false }); + + const pending = send("inspect", INSPECT); + const delivery = answerInspect(); + + expect(delivery.dispatch.target.tabId).toBe(USER_TAB); + await expect(pending).resolves.toMatchObject({ + ok: true, + result: { snapshot: { url: `https://example.test/${USER_TAB}`, title: `Page in ${USER_TAB}`, visibleText: `Content of ${USER_TAB}` } }, + }); + }); + + it("reads a named tab the user holds without moving the agent off its own tab", async () => { + const { send, answerInspect } = browser({ focusedTab: AGENT_TAB, userController: userHold(2) }); + const sticky = send("inspect", INSPECT); + expect(answerInspect(2).dispatch.target.tabId).toBe(AGENT_TAB); + await sticky; + + const named = send("inspect", { ...INSPECT, tabId: USER_TAB }); + expect(answerInspect(2).dispatch.target.tabId).toBe(USER_TAB); + await expect(named).resolves.toMatchObject({ + ok: true, + result: { target: { tabId: USER_TAB }, snapshot: { visibleText: `Content of ${USER_TAB}` } }, + }); + + const next = send("inspect", INSPECT); + expect(answerInspect(2).dispatch.target.tabId).toBe(AGENT_TAB); + await next; + }); + + it("keeps the user's hold after the agent reads the held tab", async () => { + const { send, answerInspect } = browser({ userController: userHold(2) }); + + const first = send("inspect", { ...INSPECT, tabId: USER_TAB }); + answerInspect(2); + await first; + const second = send("inspect", { ...INSPECT, tabId: USER_TAB }); + answerInspect(2); + + await expect(second).resolves.toMatchObject({ + ok: true, + result: { readiness: { ready: false, state: "human-control" }, tabs: expect.arrayContaining([expect.objectContaining({ tabId: USER_TAB, controller: userHold(2) })]) }, + }); + }); + + it("refuses a named tab that is not in the thread", async () => { + const { send, deliveries } = browser(); + + await expect(send("inspect", { ...INSPECT, tabId: "elsewhere" })).resolves.toMatchObject({ + ok: false, + error: { code: "TAB_UNAVAILABLE", recovery: "inspect" }, + }); + expect(deliveries).toHaveLength(0); + }); + + it("lists every thread tab with the holder the broker enforces", async () => { + const { send, answerInspect } = browser({ userController: userHold(4) }); + + const pending = send("inspect", { ...INSPECT, tabId: AGENT_TAB }); + answerInspect(4); + + const response = await pending; + expect(response.ok && response.result.operation === "inspect" && response.result.tabs.map((tab) => [tab.tabId, tab.controller?.controller])) + .toEqual([[AGENT_TAB, undefined], [USER_TAB, "human"]]); + }); + + it("reports the Browser as not ready while the user holds another tab in the thread", async () => { + const { send, answerInspect } = browser({ userController: userHold(1) }); + + const pending = send("inspect", { ...INSPECT, tabId: AGENT_TAB }); + answerInspect(1); + + await expect(pending).resolves.toMatchObject({ + ok: true, + result: { readiness: { ready: false, state: "human-control" } }, + }); + }); + + it("refuses an action on the user's held tab and admits it after Hand back", async () => { + const { send, publish, deliveries } = browser({ agentTab: false, userController: userHold(1) }); + const act = { idempotencyKey: "act-key", observationRef: "observation", deadlineMs: 1_000, steps: [{ operation: "press", key: "A", modifiers: [] }] }; + + await expect(send("act", act, 1)).resolves.toMatchObject({ + ok: false, + error: { code: "HUMAN_INTERRUPTED", recovery: "yield_to_user" }, + }); + expect(deliveries).toHaveLength(0); + + publish({ tabId: USER_TAB, controller: "none", controlEpoch: 2 }); + void send("act", { ...act, idempotencyKey: "act-after-hand-back" }, 2); + + expect(deliveries.map(({ dispatch }) => [dispatch.request.operation, dispatch.target.tabId])).toEqual([["act", USER_TAB]]); + }); +}); diff --git a/apps/server/src/features/browser-automation/execution/broker.ts b/apps/server/src/features/browser-automation/execution/broker.ts index eed573f2b..f55f35e89 100644 --- a/apps/server/src/features/browser-automation/execution/broker.ts +++ b/apps/server/src/features/browser-automation/execution/broker.ts @@ -202,6 +202,12 @@ function failure( }; } +/** A named tab outside the thread is a bad reference, so the agent re-reads the tab list instead of reopening. */ +function unlistedTabFailure(request: BrowserAutomationRequest): BrowserAutomationResponse { + const response = failure(request, "TAB_UNAVAILABLE", "That tab is not open in this thread; browser_inspect lists the thread's tabs", false); + return response.ok ? response : { ...response, error: { ...response.error, recovery: "inspect" } }; +} + function assignmentKeyParts( providerId: string, providerSessionId: string, @@ -429,9 +435,17 @@ function inspectCapabilities( : capabilities.filter((operation) => operation !== "act"); } -function inspectReadiness(target: BrowserAutomationHostDispatchTarget | undefined): { ready: boolean; state: "ready" | "target-unavailable" | "human-control"; reason?: string } { +/** The thread tab the user holds, if any. ADR-0029 makes the hold thread-wide, so it blocks effects on every tab. */ +function userHeldTarget(host: RegisteredHost, threadId: string): BrowserAutomationHostDispatchTarget | undefined { + return [...host.targets.values()].find((target) => target.threadId === threadId && target.controller?.controller === "human"); +} + +function inspectReadiness( + target: BrowserAutomationHostDispatchTarget | undefined, + userHeld: BrowserAutomationHostDispatchTarget | undefined, +): { ready: boolean; state: "ready" | "target-unavailable" | "human-control"; reason?: string } { if (!target) return { ready: false, state: "target-unavailable", reason: "Visible browser target is unavailable" }; - if (target.controller?.controller === "human") return { ready: false, state: "human-control", reason: "Visible Preview is under human control" }; + if (userHeld) return { ready: false, state: "human-control", reason: "Visible Preview is under human control" }; return { ready: true, state: "ready" }; } @@ -451,19 +465,28 @@ function boundedInspectSnapshot( }; } +/** + * Hosts list a thread's tabs without their holder, so each listed tab takes the + * controller the broker admits effects against. + */ function inspectTabs( responseTabs: BrowserInspectResult["tabs"], target: BrowserAutomationHostDispatchTarget | undefined, + host: RegisteredHost, ): BrowserInspectResult["tabs"] { - return (target ? [target, ...responseTabs.filter((tab) => tab.tabId !== target.tabId)] : responseTabs) as BrowserInspectResult["tabs"]; + const tabs = target ? [target, ...responseTabs.filter((tab) => tab.tabId !== target.tabId)] : responseTabs; + return tabs.map(({ controller: _reported, ...tab }) => { + const controller = host.targets.get(targetKey(tab))?.controller; + return controller ? { ...tab, controller } : tab; + }); } function inspectGuidance( descriptor: RegisteredHost["registration"]["executorDescriptor"], - target: BrowserAutomationHostDispatchTarget | undefined, + userHeld: BrowserAutomationHostDispatchTarget | undefined, capabilities: BrowserAutomationPublicOperation[], ): string { - if (target?.controller?.controller === "human") return "The user has control of the Browser. Do not act until browser_inspect reports ready; the user hands control back."; + if (userHeld) return "The user has control of the Browser. browser_inspect can still read any tab. Do not act until browser_inspect reports ready; the user hands control back."; return `Visible browser executor (${descriptor.runtime}) supports: ${capabilities.join(", ") || "none"}.`; } @@ -478,19 +501,20 @@ function shapeNegotiatedResponse( const observationRef = inspectObservationReference(response.result); const capabilities = inspectCapabilities(negotiated, observationRef); const diagnostics = response.result.diagnostics?.slice(0, descriptor.constraints.maxDiagnostics); + const userHeld = userHeldTarget(pending.host, pending.request.threadId); return { ...response, result: { ...response.result, - readiness: inspectReadiness(pending.target), + readiness: inspectReadiness(pending.target, userHeld), target: pending.target && { threadId: pending.target.threadId, tabId: pending.target.tabId, targetGeneration: pending.target.targetGeneration, sticky: true }, - tabs: inspectTabs(response.result.tabs, pending.target).slice(0, descriptor.constraints.maxTabs), + tabs: inspectTabs(response.result.tabs, pending.target, pending.host).slice(0, descriptor.constraints.maxTabs), snapshot: boundedInspectSnapshot(response.result.snapshot, descriptor.constraints.maxSnapshotChars), ...(diagnostics ? { diagnostics } : {}), ...(observationRef ? { observationRef } : !negotiated.includes("act") ? { observationRef: NodeCrypto.randomUUID() } : {}), capabilities, capabilityRevision: descriptor.capabilityRevision, - guidance: inspectGuidance(descriptor, pending.target, capabilities).slice(0, 4_000), + guidance: inspectGuidance(descriptor, userHeld, capabilities).slice(0, 4_000), }, }; } @@ -1235,6 +1259,8 @@ export class BrowserAutomationBroker { ): RoutingCandidate | undefined { if (request.operation === "open" && request.args.idempotencyKey !== undefined) return undefined; if (request.operation === "tabs") return this.resolveTabsAssignment(claims, request); + // A named read leaves the sticky assignment alone: reading a tab never selects it. + if (request.operation === "inspect" && request.args.tabId) return this.resolveExplicitTarget(claims, request, request.args.tabId); return this.resolveAssignment(claims, request); } @@ -1247,6 +1273,9 @@ export class BrowserAutomationBroker { const host = this.resolveBootstrapHost(claims, request); if (host) return this.executeBootstrap(host, claims.providerId, request); } + if (request.operation === "inspect" && request.args.tabId && this.resolveSelectedHost(claims)) { + return finishImmediately(unlistedTabFailure(request)); + } const code = this.resolveSelectedHost(claims) ? "TAB_UNAVAILABLE" : "HOST_UNAVAILABLE"; const message = code === "TAB_UNAVAILABLE" ? "No authorized visible browser tab is available" @@ -1681,9 +1710,7 @@ export class BrowserAutomationBroker { } private targetDeliveryError(pending: PendingRequest): BrowserAutomationResponse | null { - if (pending.request.operation !== "inspect" && [...pending.host.targets.values()].some((target) => - target.threadId === pending.request.threadId && target.controller?.controller === "human" - )) { + if (pending.request.operation !== "inspect" && userHeldTarget(pending.host, pending.request.threadId)) { return failure(pending.request, "HUMAN_INTERRUPTED", "The user has control of the Browser until Hand back", true); } if (!pending.target) return null; diff --git a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts index dfc435367..03a6a184b 100644 --- a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts +++ b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts @@ -342,6 +342,15 @@ describe("BrowserToolkit", () => { }); }); + it("reads a named tab by passing its tabId to the broker", async () => { + const page = fakeBrowser(); + const { call, requestAt } = setup({ execute: page.execute }); + + await call("browser_inspect", { tabId: "user-tab" }); + + expect(requestAt(0)).toMatchObject({ operation: "inspect", args: { tabId: "user-tab" } }); + }); + it("rejects caller-supplied scope, internal fields, and invalid argument shapes before the broker", async () => { const page = fakeBrowser(); const { call, execute } = setup({ execute: page.execute }); diff --git a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts index 68b06c799..81db3e2e7 100644 --- a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts +++ b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts @@ -81,7 +81,11 @@ const inputSchemaByOperation: Record = "Returns a snapshot whose elements each carry a ref such as s3e12. Target browser_act with these refs. Each inspection replaces the previous refs.", "Inspect before the first action and again whenever an action says to. Tool discovery does not prove readiness.", "Request includeScreenshot only for visual evidence or when the user asks. Verify every success claim against current page evidence.", + "tabs lists every Browser tab in this thread, including tabs the user opened, with who holds each. Pass a tabId to read that tab, even while the user holds it; reading never takes control.", + "browser_act targets the current tab, not one read by tabId: to act on another tab, browser_tabs select it while nobody holds the Browser.", "Use only mcode MCP browser tools; Browser-only tasks need no shell commands or local skills/files.", ].join(" "), act: [ diff --git a/packages/contracts/src/models/browser-automation.ts b/packages/contracts/src/models/browser-automation.ts index c3ad4072d..191e2127b 100644 --- a/packages/contracts/src/models/browser-automation.ts +++ b/packages/contracts/src/models/browser-automation.ts @@ -993,6 +993,8 @@ export const BrowserAutomationRequestSchema = lazySchema(() => requestVariant("inspect", z.object({ includeScreenshot: z.boolean().default(false), includeDiagnostics: z.boolean().default(false), + /** Reads this thread tab instead of the session's current one, without selecting it. */ + tabId: idSchema.optional(), }).strict()), requestVariant("status", emptyArgs), requestVariant( From d3ab9cc75c4cfa50bd033818a30aefe7d7543b03 Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:34:19 +0100 Subject: [PATCH 3/3] fix(browser): report tab holders consistently and keep busy tabs apart from missing ones The Electron host marks every tab in a held thread as held by the user, while the web host marks only the tab the user took. browser_inspect now lists each tab as { tabId, active, focused, holder } and reports a user hold on any tab for every tab, which matches the thread-wide hold the broker enforces. The listing also drops routing fields and control epochs the model never uses. A named read of a listed tab on a busy host now fails as retryable instead of claiming the tab is not open. --- .../__tests__/broker.user-tab.test.ts | 13 ++++++++- .../browser-automation/execution/broker.ts | 13 ++++++--- .../__tests__/browser-toolkit.test.ts | 29 +++++++++++++++++++ .../mcode-mcp/toolkits/browser-tool-refs.ts | 22 ++++++++++++-- .../toolkits/browser-tool-schemas.ts | 2 +- 5 files changed, 70 insertions(+), 9 deletions(-) diff --git a/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts b/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts index 5033b6384..3d019d5df 100644 --- a/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts +++ b/apps/server/src/features/browser-automation/execution/__tests__/broker.user-tab.test.ts @@ -226,7 +226,18 @@ describe("BrowserAutomationBroker reading the user's own tab", () => { expect(deliveries).toHaveLength(0); }); - it("lists every thread tab with the holder the broker enforces", async () => { + it("treats a listed tab on a busy host as unavailable for now, not as missing", async () => { + const { send, deliveries } = browser(); + for (let index = 0; index < registration().maxPendingRequests; index += 1) void send("inspect", INSPECT); + + await expect(send("inspect", { ...INSPECT, tabId: USER_TAB })).resolves.toMatchObject({ + ok: false, + error: { code: "TAB_UNAVAILABLE", retryable: true }, + }); + expect(deliveries).toHaveLength(registration().maxPendingRequests); + }); + + it("lists every thread tab with the holder its host published", async () => { const { send, answerInspect } = browser({ userController: userHold(4) }); const pending = send("inspect", { ...INSPECT, tabId: AGENT_TAB }); diff --git a/apps/server/src/features/browser-automation/execution/broker.ts b/apps/server/src/features/browser-automation/execution/broker.ts index f55f35e89..bf1e5c5c4 100644 --- a/apps/server/src/features/browser-automation/execution/broker.ts +++ b/apps/server/src/features/browser-automation/execution/broker.ts @@ -183,8 +183,8 @@ function failure( code: BrowserAutomationErrorCode, message: string, retryable: boolean, + recovery: ReturnType = browserV2Recovery(code), ): BrowserAutomationResponse { - const recovery = browserV2Recovery(code); return { contractVersion: BROWSER_AUTOMATION_CONTRACT_VERSION, requestId: request.requestId, @@ -204,8 +204,7 @@ function failure( /** A named tab outside the thread is a bad reference, so the agent re-reads the tab list instead of reopening. */ function unlistedTabFailure(request: BrowserAutomationRequest): BrowserAutomationResponse { - const response = failure(request, "TAB_UNAVAILABLE", "That tab is not open in this thread; browser_inspect lists the thread's tabs", false); - return response.ok ? response : { ...response, error: { ...response.error, recovery: "inspect" } }; + return failure(request, "TAB_UNAVAILABLE", "That tab is not open in this thread; browser_inspect lists the thread's tabs", false, "inspect"); } function assignmentKeyParts( @@ -1273,7 +1272,7 @@ export class BrowserAutomationBroker { const host = this.resolveBootstrapHost(claims, request); if (host) return this.executeBootstrap(host, claims.providerId, request); } - if (request.operation === "inspect" && request.args.tabId && this.resolveSelectedHost(claims)) { + if (request.operation === "inspect" && request.args.tabId && !this.threadListsTab(claims, request.args.tabId) && this.resolveSelectedHost(claims)) { return finishImmediately(unlistedTabFailure(request)); } const code = this.resolveSelectedHost(claims) ? "TAB_UNAVAILABLE" : "HOST_UNAVAILABLE"; @@ -1651,6 +1650,12 @@ export class BrowserAutomationBroker { return candidates[0]; } + /** Whether any host in scope lists the tab, busy or not, so a busy host is not reported as a missing tab. */ + private threadListsTab(claims: BrowserAutomationCredentialClaims, tabId: string): boolean { + return [...this.hostsBySocket.values()].some((host) => this.matchesScope(host, claims) && + [...host.targets.values()].some((target) => target.threadId === claims.threadId && target.tabId === tabId)); + } + private canAccept( host: RegisteredHost, claims: BrowserAutomationCredentialClaims, diff --git a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts index 03a6a184b..01cfb536c 100644 --- a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts +++ b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts @@ -351,6 +351,35 @@ describe("BrowserToolkit", () => { expect(requestAt(0)).toMatchObject({ operation: "inspect", args: { tabId: "user-tab" } }); }); + it("lists each tab's holder, counting a user hold on any tab as holding the whole thread", async () => { + const { call, execute } = setup(); + const tab = (tabId: string, controller?: "human" | "agent") => ({ + desktopInstanceId: "desktop", + windowId: 1, + connectionGeneration: 1, + threadId: "thread-a", + tabId, + targetGeneration: 1, + active: tabId === "user-tab", + focused: false, + lastUsedAt: 1, + ...(controller ? { controller: { tabId, controller, controlEpoch: 3 } } : {}), + }); + const listing = (tabs: ReturnType[]) => execute.mockImplementationOnce(async (_claims, input) => responseFor(input, { operation: "inspect", controlEpoch: 3, tabs })); + + listing([tab("agent-tab", "agent"), tab("user-tab")]); + expect(JSON.parse(textOf(await call("browser_inspect"))).tabs).toEqual([ + { tabId: "agent-tab", active: false, focused: false, holder: "agent" }, + { tabId: "user-tab", active: true, focused: false, holder: "none" }, + ]); + + listing([tab("agent-tab"), tab("user-tab", "human")]); + expect(JSON.parse(textOf(await call("browser_inspect"))).tabs).toEqual([ + { tabId: "agent-tab", active: false, focused: false, holder: "user" }, + { tabId: "user-tab", active: true, focused: false, holder: "user" }, + ]); + }); + it("rejects caller-supplied scope, internal fields, and invalid argument shapes before the broker", async () => { const page = fakeBrowser(); const { call, execute } = setup({ execute: page.execute }); diff --git a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-refs.ts b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-refs.ts index 402536350..59ab2e797 100644 --- a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-refs.ts +++ b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-refs.ts @@ -202,14 +202,30 @@ export function advanceBrowserToolLedger( return { controlEpoch: ledger.controlEpoch }; } +/** + * Lists tabs without routing internals or control epochs. ADR-0029 makes a user + * hold thread-wide, and hosts differ on whether they mark every thread tab, so a + * user hold on any tab is reported on all of them. + */ +function projectTabs(tabs: Result<"inspect">["tabs"]): Array<{ tabId: string; active: boolean; focused: boolean; holder: "user" | "agent" | "none" }> { + const userHolds = tabs.some((tab) => tab.controller?.controller === "human"); + return tabs.map(({ tabId, active, focused, controller }) => ({ + tabId, + active, + focused, + holder: userHolds ? "user" : controller?.controller === "agent" ? "agent" : "none", + })); +} + function projectInspect(result: Result<"inspect">, snapshotId: number | undefined): Record { - const { observationRef: _observationRef, controlEpoch: _controlEpoch, snapshot, ...rest } = result; - if (!snapshot) return rest; + const { observationRef: _observationRef, controlEpoch: _controlEpoch, snapshot, tabs, ...rest } = result; + const listed = { ...rest, tabs: projectTabs(tabs) }; + if (!snapshot) return listed; const elements = snapshot.elements.map(({ semanticId: _semanticId, ...element }, index) => ({ ref: elementRef(snapshotId ?? 0, index), ...element, })); - return { ...rest, snapshot: { ...snapshot, elements } }; + return { ...listed, snapshot: { ...snapshot, elements } }; } /** Removes internal bookkeeping from a Browser result and swaps semantic ids for refs. */ diff --git a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts index 81db3e2e7..bf6d8e6cf 100644 --- a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts +++ b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts @@ -121,7 +121,7 @@ const descriptionByOperation: Record = "Inspect before the first action and again whenever an action says to. Tool discovery does not prove readiness.", "Request includeScreenshot only for visual evidence or when the user asks. Verify every success claim against current page evidence.", "tabs lists every Browser tab in this thread, including tabs the user opened, with who holds each. Pass a tabId to read that tab, even while the user holds it; reading never takes control.", - "browser_act targets the current tab, not one read by tabId: to act on another tab, browser_tabs select it while nobody holds the Browser.", + "browser_act targets the current tab, not one read by tabId, and refuses refs from another tab. To act on a tab read by tabId, browser_tabs select it while nobody holds the Browser, then inspect again.", "Use only mcode MCP browser tools; Browser-only tasks need no shell commands or local skills/files.", ].join(" "), act: [