diff --git a/ts/packages/agents/browser/src/agent/discovery/actionHandler.mts b/ts/packages/agents/browser/src/agent/discovery/actionHandler.mts index 8713a85f0..14fbf22c8 100644 --- a/ts/packages/agents/browser/src/agent/discovery/actionHandler.mts +++ b/ts/packages/agents/browser/src/agent/discovery/actionHandler.mts @@ -705,12 +705,10 @@ async function handleCreateWebFlowFromRecording( async function handleGetWebFlowsForDomain( action: GetWebFlowsForDomain, - ctx: DiscoveryActionHandlerContext, + sessionContext: SessionContext, + entities: EntityCollector, ): Promise { - const webFlowStore = ctx.sessionContext.agentContext.webFlowStore; - if (!webFlowStore) { - throw new Error("WebFlowStore not available"); - } + const webFlowStore = await getWebFlowStore(sessionContext); const flows = await webFlowStore.listForDomainWithDetails( action.parameters.domain, @@ -718,7 +716,7 @@ async function handleGetWebFlowsForDomain( return { displayText: `Found ${flows.length} actions`, - entities: ctx.entities.getEntities(), + entities: entities.getEntities(), data: { actions: flows.map((f) => ({ name: f.name, @@ -734,18 +732,16 @@ async function handleGetWebFlowsForDomain( async function handleGetAllWebFlows( action: GetAllWebFlows, - ctx: DiscoveryActionHandlerContext, + sessionContext: SessionContext, + entities: EntityCollector, ): Promise { - const webFlowStore = ctx.sessionContext.agentContext.webFlowStore; - if (!webFlowStore) { - throw new Error("WebFlowStore not available"); - } + const webFlowStore = await getWebFlowStore(sessionContext); const flows = await webFlowStore.listAllWithDetails(); return { displayText: `Found ${flows.length} actions`, - entities: ctx.entities.getEntities(), + entities: entities.getEntities(), data: { actions: flows.map((f) => ({ name: f.name, @@ -761,25 +757,23 @@ async function handleGetAllWebFlows( async function handleDeleteWebFlow( action: DeleteWebFlow, - ctx: DiscoveryActionHandlerContext, + sessionContext: SessionContext, + entities: EntityCollector, ): Promise { - const webFlowStore = ctx.sessionContext.agentContext.webFlowStore; - if (!webFlowStore) { - throw new Error("WebFlowStore not available"); - } + const webFlowStore = await getWebFlowStore(sessionContext); const deleted = await webFlowStore.delete(action.parameters.name); if (deleted) { debug(`Deleted webFlow: ${action.parameters.name}`); - sendWebFlowRefreshToClient(ctx.sessionContext); + sendWebFlowRefreshToClient(sessionContext); } return { displayText: deleted ? "Action deleted successfully" : "Action not found", - entities: ctx.entities.getEntities(), + entities: entities.getEntities(), data: { success: deleted, name: action.parameters.name, @@ -1351,6 +1345,54 @@ export async function handleSchemaDiscoveryAction( progressCallback?: (message: string) => void, actionIO?: ProgressActionIO, ) { + // Create entity collector up front; some handlers below need it before + // we know whether a full discovery context (browser + LLM) is required. + const entityCollector = new EntityCollector(); + + // Store-only operations don't need a connected browser session or the + // discovery LLM translator. Dispatch them first so the macros library + // (and similar pure-store queries) can succeed even when no browser + // page is active. This mirrors how the chat-side `listWebFlows` + // handler in webFlows/actionHandler.mts operates. + switch (action.actionName) { + case "getAllWebFlows": { + const r = await handleGetAllWebFlows( + action, + context, + entityCollector, + ); + return { + displayText: r.displayText, + data: r.data, + entities: r.entities, + }; + } + case "getWebFlowsForDomain": { + const r = await handleGetWebFlowsForDomain( + action, + context, + entityCollector, + ); + return { + displayText: r.displayText, + data: r.data, + entities: r.entities, + }; + } + case "deleteWebFlow": { + const r = await handleDeleteWebFlow( + action, + context, + entityCollector, + ); + return { + displayText: r.displayText, + data: r.data, + entities: r.entities, + }; + } + } + if (!context.agentContext.browserControl) { throw new Error("No connection to browser session."); } @@ -1358,8 +1400,6 @@ export async function handleSchemaDiscoveryAction( const browser: BrowserControl = context.agentContext.browserControl; const agent = await createDiscoveryPageTranslator("GPT_5_2"); - // Create entity collector and action context - const entityCollector = new EntityCollector(); const discoveryContext: DiscoveryActionHandlerContext = { browser, agent, @@ -1400,15 +1440,6 @@ export async function handleSchemaDiscoveryAction( discoveryContext, ); break; - case "getWebFlowsForDomain": - result = await handleGetWebFlowsForDomain(action, discoveryContext); - break; - case "getAllWebFlows": - result = await handleGetAllWebFlows(action, discoveryContext); - break; - case "deleteWebFlow": - result = await handleDeleteWebFlow(action, discoveryContext); - break; case "inferActions": result = await handleInferActions(action, discoveryContext); break; diff --git a/ts/packages/agents/browser/src/extension/views/macroUtilities.ts b/ts/packages/agents/browser/src/extension/views/macroUtilities.ts index e660ed889..ee80b9596 100644 --- a/ts/packages/agents/browser/src/extension/views/macroUtilities.ts +++ b/ts/packages/agents/browser/src/extension/views/macroUtilities.ts @@ -37,7 +37,24 @@ export async function getAllWebFlows(): Promise { const response = await sendToServiceWorker({ type: "getAllWebFlows", }); - return response?.actions || response || []; + // The service worker returns an error envelope ({ success: false, error }) + // when the agent RPC fails (e.g. transport disconnect, no browser session). + // Treat that as a hard failure so the caller can surface it to the user + // rather than silently rendering an empty / broken macro list. + if ( + response && + typeof response === "object" && + response.success === false + ) { + throw new Error(response.error || "Failed to fetch macros"); + } + if (Array.isArray(response?.actions)) { + return response.actions; + } + if (Array.isArray(response)) { + return response; + } + throw new Error("Unexpected getAllWebFlows response"); } export async function deleteWebFlow( diff --git a/ts/packages/agents/browser/src/extension/views/macrosLibrary.ts b/ts/packages/agents/browser/src/extension/views/macrosLibrary.ts index 385c7a906..8c84974a4 100644 --- a/ts/packages/agents/browser/src/extension/views/macrosLibrary.ts +++ b/ts/packages/agents/browser/src/extension/views/macrosLibrary.ts @@ -194,7 +194,10 @@ class MacroIndexApp { } catch (error) { console.error("Error loading macros:", error); this.state.error = "Failed to load macros. Please try again."; - const container = document.getElementById("actionsContainer")!; + // Reuse the container captured above. The previous code referenced + // a non-existent "actionsContainer" element, which made the catch + // handler throw a second exception ("Cannot set properties of null") + // and left the loading spinner displayed forever. showErrorState(container, this.state.error); } finally { this.state.loading = false; diff --git a/ts/packages/agents/browser/test/views/macroUtilities.test.ts b/ts/packages/agents/browser/test/views/macroUtilities.test.ts new file mode 100644 index 000000000..11027da74 --- /dev/null +++ b/ts/packages/agents/browser/test/views/macroUtilities.test.ts @@ -0,0 +1,84 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { getAllWebFlows } from "../../src/extension/views/macroUtilities"; + +// Helper: configure the (already-mocked) chrome.runtime.sendMessage to invoke +// its callback with the supplied response. The real sendToServiceWorker helper +// in macroUtilities wraps this callback in a Promise. +function mockSendMessageResponse(response: unknown): void { + (chrome.runtime.sendMessage as jest.Mock).mockImplementation( + (_message: unknown, cb: (r: unknown) => void) => { + cb(response); + }, + ); +} + +describe("getAllWebFlows response parsing", () => { + beforeEach(() => { + (chrome.runtime.sendMessage as jest.Mock).mockReset(); + }); + + it("returns the actions array on a normal success response", async () => { + const macros = [ + { name: "buyProduct", description: "Buy a product" }, + { name: "addToCart", description: "Add to cart" }, + ]; + mockSendMessageResponse({ actions: macros }); + + await expect(getAllWebFlows()).resolves.toEqual(macros); + }); + + it("returns an empty array when actions is empty", async () => { + mockSendMessageResponse({ actions: [] }); + + await expect(getAllWebFlows()).resolves.toEqual([]); + }); + + it("throws with the agent-supplied error on a {success:false} envelope", async () => { + // This is the regression case: the service worker returns this envelope + // when the agent RPC throws (e.g. "No connection to browser session.", + // transport disconnect, bfcache port close). Previous behavior was to + // pass the envelope through as if it were the macros array, causing + // the caller's .forEach to crash and leave the loading spinner forever. + mockSendMessageResponse({ + success: false, + error: "No connection to browser session.", + }); + + await expect(getAllWebFlows()).rejects.toThrow( + "No connection to browser session.", + ); + }); + + it("throws a generic message when an error envelope omits the error string", async () => { + mockSendMessageResponse({ success: false }); + + await expect(getAllWebFlows()).rejects.toThrow( + "Failed to fetch macros", + ); + }); + + it("accepts a direct array response (legacy shape)", async () => { + const macros = [{ name: "search" }]; + mockSendMessageResponse(macros); + + await expect(getAllWebFlows()).resolves.toEqual(macros); + }); + + it("throws on an unexpected response shape", async () => { + mockSendMessageResponse({ foo: "bar" }); + + await expect(getAllWebFlows()).rejects.toThrow( + "Unexpected getAllWebFlows response", + ); + }); + + it("throws on a null response", async () => { + mockSendMessageResponse(null); + + await expect(getAllWebFlows()).rejects.toThrow( + "Unexpected getAllWebFlows response", + ); + }); +});