From b25d3517b68ad4352fa44e435f23c26de4bce54a Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:09:57 +0800 Subject: [PATCH] fix(browser): report cancellation after cleanup and source-map abort (#2893) * fix(browser): report cancellation after cleanup and source-map abort An interrupt during CDP cleanup could still publish the captured page, and a source-map download abort was wrapped as a provider failure. Both now return AnalysisCancelledError for the requested operation. * fix(setup): keep Devin configuration JSON-formatted The empty DSH_HOME fix also changed Devin's declared format to "devin", which is not a configuration format and stops the typecheck. Devin still writes mcp_config.json. * test(browser): keep cleanup cancellation coverage in its own suite The added interrupt cases made the existing lifecycle suite exceed the function-length lint limit. * test(ghidra): drop the duplicated path join import The inventory MCP test imported join twice, which fails lint on current main. * test(cli): type the retained skill directory pairs noUncheckedIndexedAccess treats the unlabeled pair list as an open array, so the directory passed to join was string or undefined. * fix(browser): preserve source-map cancellation results * style(test): format the partial Frida load cancellation fixture Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Cursor Agent Co-authored-by: REA Maintenance Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- ...BrowserProvider.lifecycle_failures.test.ts | 195 ++++++++++++++++++ tests/boundary/mcp/fridaLifecycleMcp.test.ts | 9 +- tests/boundary/mcp/toolInputAdmission.test.ts | 2 +- tests/fixtures/fakeCdpBrowser.ts | 5 + tests/fixtures/fakeCdpBrowserTypes.ts | 2 + 5 files changed, 208 insertions(+), 5 deletions(-) diff --git a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts index 01b1f54da6..3aa173cf2e 100644 --- a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts +++ b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts @@ -8,12 +8,207 @@ import { import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js"; import { analyzeWebBundleInputSchema } from "../../../src/domain/webBundleAnalysis.js"; import { observeWebSessionInputSchema } from "../../../src/domain/browserSession.js"; +import { discoverWebMcpToolsInputSchema } from "../../../src/domain/webMcpDiscovery.js"; +import { captureWebScreenshotInputSchema } from "../../../src/domain/webScreenshot.js"; import { startFakeCdpBrowser, type FakeCdpBrowser, } from "../../fixtures/fakeCdpBrowser.js"; import { trackBrowser } from "./cdpBrowserProvider.support.js"; +const waitFor = async (ready: () => boolean, description: string) => { + const deadline = Date.now() + 5_000; + while (!ready()) { + if (Date.now() > deadline) throw new Error(description); + await new Promise((resolve) => setTimeout(resolve, 10)); + } +}; + +describe("CdpBrowserProvider: cancellation during cleanup", () => { + it.each([ + { + name: "WebMCP discovery", + operation: "discover_webmcp_tools", + invoke: ( + provider: CdpBrowserProvider, + browser: FakeCdpBrowser, + signal: AbortSignal, + ) => + provider.discoverWebMcpTools( + discoverWebMcpToolsInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 0, + }), + { signal }, + ), + }, + { + name: "screenshot capture", + operation: "capture_web_screenshot", + invoke: ( + provider: CdpBrowserProvider, + browser: FakeCdpBrowser, + signal: AbortSignal, + ) => + provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + }), + { signal }, + ), + }, + ])( + "reports cancellation during $name cleanup", + async ({ operation, invoke }) => { + const browser = await startFakeCdpBrowser({ + hangOnMethod: "Page.disable", + webMcpTools: true, + }); + trackBrowser(browser); + const controller = new AbortController(); + const pending = invoke( + new CdpBrowserProvider(), + browser, + controller.signal, + ); + await waitFor( + () => browser.commands.some(({ method }) => method === "Page.disable"), + "page cleanup did not start", + ); + controller.abort(); + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError", operation }, + }); + }, + ); + + it("reports cancellation when an interrupt arrives during page cleanup", async () => { + const browser = await startFakeCdpBrowser({ + hangOnMethod: "Network.disable", + }); + trackBrowser(browser); + const controller = new AbortController(); + const pending = new CdpBrowserProvider().inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 0, + }), + { signal: controller.signal }, + ); + await waitFor( + () => browser.commands.some(({ method }) => method === "Network.disable"), + "page cleanup did not start", + ); + controller.abort(); + + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "inspect_web_page", + }, + }); + }); + + it("reports cancellation when an interrupt arrives during session cleanup", async () => { + const browser = await startFakeCdpBrowser({ + hangOnMethod: "Network.disable", + }); + trackBrowser(browser); + const controller = new AbortController(); + const pending = new CdpBrowserProvider().observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 1, + }), + { signal: controller.signal }, + ); + await waitFor( + () => browser.commands.some(({ method }) => method === "Network.disable"), + "session cleanup did not start", + ); + controller.abort(); + + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "observe_web_session", + }, + }); + }); +}); + +describe("CdpBrowserProvider: cancellation during source-map download", () => { + it("reports cancellation when an interrupt arrives during source-map download", async () => { + const browser = await startFakeCdpBrowser({ hangSourceMap: true }); + trackBrowser(browser); + const controller = new AbortController(); + const pending = new CdpBrowserProvider().analyzeBundle( + analyzeWebBundleInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 0, + fetch_source_maps: true, + }), + { signal: controller.signal }, + ); + await waitFor( + () => + browser.httpRequests.some(({ url }) => url.startsWith("/app.js.map")), + "source-map download did not start", + ); + controller.abort(); + + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "analyze_web_bundle", + }, + }); + }); + + it("classifies custom source-map abort reasons as cancellation", async () => { + const browser = await startFakeCdpBrowser({ hangSourceMap: true }); + trackBrowser(browser); + const controller = new AbortController(); + const pending = new CdpBrowserProvider().analyzeBundle( + analyzeWebBundleInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 0, + fetch_source_maps: true, + }), + { signal: controller.signal }, + ); + await waitFor( + () => + browser.httpRequests.some(({ url }) => url.startsWith("/app.js.map")), + "source-map download did not start", + ); + controller.abort(new Error("stopped")); + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "analyze_web_bundle", + }, + }); + }); +}); + describe("CdpBrowserProvider: cancellation and session events", () => { it("honors cancellation during the final progress callback before publishing inspection", async () => { const browser = await startFakeCdpBrowser(); diff --git a/tests/boundary/mcp/fridaLifecycleMcp.test.ts b/tests/boundary/mcp/fridaLifecycleMcp.test.ts index 4c657668eb..c46eb72d9a 100644 --- a/tests/boundary/mcp/fridaLifecycleMcp.test.ts +++ b/tests/boundary/mcp/fridaLifecycleMcp.test.ts @@ -272,11 +272,12 @@ const callPartialFridaLoadError = async ( const failure = partial === "evidence" ? new FridaScriptLoadCancelledError(script) - : new AnalysisCancelledError("load_frida_script", { - ...(partial === "non-evidence" + : new AnalysisCancelledError( + "load_frida_script", + partial === "non-evidence" ? { partialObservation: nonEvidencePartialObservation() } - : {}), - }); + : {}, + ); const provider: FridaInstrumentationPort = { async listDevices() { return unusedFridaPortMethod(); diff --git a/tests/boundary/mcp/toolInputAdmission.test.ts b/tests/boundary/mcp/toolInputAdmission.test.ts index 8284ede7c7..3629b5f7f1 100644 --- a/tests/boundary/mcp/toolInputAdmission.test.ts +++ b/tests/boundary/mcp/toolInputAdmission.test.ts @@ -59,7 +59,7 @@ describe("catalog-wide MCP input admission", () => { expect(TOOL_CONTRACTS.length).toBeGreaterThan(0); for (const contract of TOOL_CONTRACTS) { const input = { - ...(contract.examples[0]?.input ?? {}), + ...contract.examples[0]?.input, __rea_unknown_admission_probe__: true, }; expect(contract.inputSchema.safeParse(input).success).toBe(false); diff --git a/tests/fixtures/fakeCdpBrowser.ts b/tests/fixtures/fakeCdpBrowser.ts index 37e59b77b6..f06300f065 100644 --- a/tests/fixtures/fakeCdpBrowser.ts +++ b/tests/fixtures/fakeCdpBrowser.ts @@ -250,6 +250,11 @@ const serveSourceMap = ( response: ServerResponse, options: FakeOptions, ): boolean => { + if ( + options.hangSourceMap === true && + request.url?.startsWith("/app.js.map") === true + ) + return true; if (options.sourceMapBody === undefined) return false; if (request.url?.startsWith("/app.js.map") === true) { if (options.sourceMapRedirect !== undefined) diff --git a/tests/fixtures/fakeCdpBrowserTypes.ts b/tests/fixtures/fakeCdpBrowserTypes.ts index 69a3c8ec74..e00ea02c4b 100644 --- a/tests/fixtures/fakeCdpBrowserTypes.ts +++ b/tests/fixtures/fakeCdpBrowserTypes.ts @@ -95,6 +95,8 @@ export interface FakeOptions { readonly webSocketWithoutFrames?: boolean; readonly invalidBinaryWebSocketEvent?: boolean; readonly sourceMapBody?: string; + /** Accept a source-map request and leave the response open. */ + readonly hangSourceMap?: boolean; readonly sourceMapRedirect?: string; readonly sessionTimeline?: | "same_origin"