From 6b101ff23b4151806d3241cfa7cd0a2457eed566 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 21 Oct 2026 04:16:44 +0800 Subject: [PATCH 1/2] fix(errors): preserve caller abort reasons at admission (#5671) * fix(errors): preserve caller abort reasons at admission Keep the opaque caller reason on reverse, pcap, and SQLite cancellation, and leave a recognized physical digest or command failure in place when a late abort is also observed. Fixes #5670 Co-authored-by: morluto * fix(reverse): keep a foreign digest ABORT_ERR as an adapter failure Treat ABORT_ERR as cancellation on the command boundary, where that code already meant an aborted provider process. A digest or stat rejection with the same code stays a physical adapter failure unless the caller signal is aborted. Cover aborted command EACCES, ETIMEDOUT, and ABORT_ERR with and without captured output. Co-authored-by: morluto --------- Co-authored-by: Cursor Agent Co-authored-by: morluto --- .../pcap/PcapAnalysisService.test.ts | 28 +++ src/application/pcap/PcapAnalysisService.ts | 6 +- .../reverse/ReverseEngineeringService.ts | 68 +++++- .../sqlite/SqliteDatabaseService.test.ts | 22 ++ .../sqlite/SqliteDatabaseService.ts | 6 +- .../reverseEngineeringAdmission.test.ts | 204 ++++++++++++++++++ 6 files changed, 324 insertions(+), 10 deletions(-) create mode 100644 src/application/pcap/PcapAnalysisService.test.ts diff --git a/src/application/pcap/PcapAnalysisService.test.ts b/src/application/pcap/PcapAnalysisService.test.ts new file mode 100644 index 0000000000..ec7e64763f --- /dev/null +++ b/src/application/pcap/PcapAnalysisService.test.ts @@ -0,0 +1,28 @@ +import { expect, it } from "vitest"; + +import { PcapAnalysisService } from "./PcapAnalysisService.js"; + +it("preserves the exact caller reason when pcap admission is already aborted", async () => { + const reason = Object.freeze({ requested: "stop" }); + const controller = new AbortController(); + controller.abort(reason); + let calls = 0; + const service = new PcapAnalysisService({ + inspectAvailability: () => Promise.reject(new Error("unused")), + close: () => Promise.resolve(), + execute: () => { + calls += 1; + return Promise.reject(new Error("provider must not run")); + }, + }); + const result = await service.execute( + "inspect_pcap_client", + {}, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(calls).toBe(0); +}); diff --git a/src/application/pcap/PcapAnalysisService.ts b/src/application/pcap/PcapAnalysisService.ts index 23b968e582..cf2c8a94c3 100644 --- a/src/application/pcap/PcapAnalysisService.ts +++ b/src/application/pcap/PcapAnalysisService.ts @@ -24,7 +24,11 @@ export class PcapAnalysisService { options?: ExecutionOptions, ): Promise> { if (options?.signal?.aborted === true) - return err(new AnalysisCancelledError(operation)); + return err( + new AnalysisCancelledError(operation, { + cause: options.signal.reason, + }), + ); const request = pcapRequestSchema.safeParse({ operation, input }); if (!request.success) return err(new AnalysisInputError(operation, { cause: request.error })); diff --git a/src/application/reverse/ReverseEngineeringService.ts b/src/application/reverse/ReverseEngineeringService.ts index 0979313365..b082ac9d14 100644 --- a/src/application/reverse/ReverseEngineeringService.ts +++ b/src/application/reverse/ReverseEngineeringService.ts @@ -250,7 +250,9 @@ export class ReverseEngineeringService { signal?: AbortSignal, ): Promise> { if (signal?.aborted) - return Promise.resolve(err(new AnalysisCancelledError(operation))); + return Promise.resolve( + err(new AnalysisCancelledError(operation, { cause: signal.reason })), + ); if (this.#closing) return Promise.resolve( err( @@ -311,7 +313,10 @@ export class ReverseEngineeringService { signal?: AbortSignal, ): Promise> { const { executable: command, arguments: args } = profile; - if (signal?.aborted) return err(new AnalysisCancelledError(operation)); + if (signal?.aborted) + return err( + new AnalysisCancelledError(operation, { cause: signal.reason }), + ); const cleanup = await this.retryCleanup(operation); if (!cleanup.ok) return cleanup; const absolutePath = @@ -322,8 +327,12 @@ export class ReverseEngineeringService { digest = await this.#digest(absolutePath, signal, this.#stat); } catch (cause) { const code = commandFailureCode(cause); - if (signal?.aborted) - return err(new AnalysisCancelledError(operation, { cause })); + if (preflightCancelled(signal, cause, code)) + return err( + new AnalysisCancelledError(operation, { + cause: callerCancellationCause(signal, cause), + }), + ); if (code === "EACCES" || code === "EPERM") { const deniedPath = absolutePath === undefined ? command : absolutePath; return err( @@ -464,9 +473,9 @@ export class ReverseEngineeringService { code === "ENOENT" || code === "ERR_PROVIDER_LAUNCH") ) { - if (signal?.aborted || code === "ABORT_ERR") + if (commandCancelled(signal, cause, code)) earlyFailure = new AnalysisCancelledError(operation, { - cause, + cause: callerCancellationCause(signal, cause), ...ownedCleanupOptions(cause), }); else if (code === "ETIMEDOUT") @@ -520,9 +529,9 @@ export class ReverseEngineeringService { : null, signal: capture.signal, }; - if (signal?.aborted || code === "ABORT_ERR") + if (commandCancelled(signal, cause, code)) failure = new AnalysisCancelledError(operation, { - cause, + cause: callerCancellationCause(signal, cause), capturedOutput, ...ownedCleanupOptions(cause), }); @@ -605,6 +614,7 @@ export class ReverseEngineeringService { } if (signal?.aborted && failure === undefined) { failure = new AnalysisCancelledError(operation, { + cause: signal.reason, capturedOutput: { stdout, stderr, @@ -972,6 +982,48 @@ const digestFile = async ( }); }; +const PHYSICAL_FAILURE_CODES = new Set([ + "EACCES", + "EPERM", + "ETIMEDOUT", + "ENOENT", + "ENOTDIR", + "EINVAL", + "ELOOP", + "ENAMETOOLONG", + "E2BIG", + "EBUSY", + "ERR_PROVIDER_LAUNCH", +]); + +/** + * A late abort does not replace a recognized physical failure. Other rejections + * observed while the caller signal is aborted remain cancellation. A bare + * `ABORT_ERR` is not cancellation until the command boundary, which already + * used that code for an aborted provider process. + */ +const preflightCancelled = ( + signal: AbortSignal | undefined, + cause: unknown, + code: string | number | undefined, +): boolean => { + if (cause instanceof AnalysisCancelledError) return true; + if (signal?.aborted !== true) return false; + if (cause === signal.reason) return true; + return typeof code !== "string" || !PHYSICAL_FAILURE_CODES.has(code); +}; + +const commandCancelled = ( + signal: AbortSignal | undefined, + cause: unknown, + code: string | number | undefined, +): boolean => code === "ABORT_ERR" || preflightCancelled(signal, cause, code); + +const callerCancellationCause = ( + signal: AbortSignal | undefined, + cause: unknown, +): unknown => (signal?.aborted === true ? signal.reason : cause); + const commandFailureCode = (cause: unknown): string | number | undefined => { if (typeof cause !== "object" || cause === null) return undefined; const code: unknown = Reflect.get(cause, "code"); diff --git a/src/application/sqlite/SqliteDatabaseService.test.ts b/src/application/sqlite/SqliteDatabaseService.test.ts index 543cd0b027..4fbd6e5172 100644 --- a/src/application/sqlite/SqliteDatabaseService.test.ts +++ b/src/application/sqlite/SqliteDatabaseService.test.ts @@ -145,3 +145,25 @@ it("preserves completed observed evidence when cancellation arrives after provid value: { normalized_result: value }, }); }); + +it("preserves the exact caller reason when inspection is already aborted", async () => { + const reason = Object.freeze({ requested: "stop" }); + const controller = new AbortController(); + controller.abort(reason); + let calls = 0; + const service = new SqliteDatabaseService({ + inspect: () => { + calls += 1; + return Promise.reject(new Error("provider must not run")); + }, + }); + const result = await service.inspect( + { path: "/selected.db" }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(calls).toBe(0); +}); diff --git a/src/application/sqlite/SqliteDatabaseService.ts b/src/application/sqlite/SqliteDatabaseService.ts index 7f8aaeb73e..b6b47a1a2f 100644 --- a/src/application/sqlite/SqliteDatabaseService.ts +++ b/src/application/sqlite/SqliteDatabaseService.ts @@ -71,7 +71,11 @@ export class SqliteDatabaseService { options?: ExecutionOptions, ): Promise> { if (options?.signal?.aborted) - return err(new AnalysisCancelledError(OPERATION)); + return err( + new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }), + ); const input = inspectSqliteDatabaseInputSchema.safeParse(rawInput); if (!input.success) return err( diff --git a/tests/composition/reverseEngineeringAdmission.test.ts b/tests/composition/reverseEngineeringAdmission.test.ts index 1b92f587a1..8440827b34 100644 --- a/tests/composition/reverseEngineeringAdmission.test.ts +++ b/tests/composition/reverseEngineeringAdmission.test.ts @@ -101,3 +101,207 @@ it("bounds concurrent reverse-engineering provider calls", async () => { await rm(directory, { recursive: true, force: true }); } }); + +it("preserves the exact caller reason when objdump admission is already aborted", async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-reverse-preabort-")); + const path = join(directory, "fixture.bin"); + await writeFile(path, "fixture"); + const reason = Object.freeze({ requested: "stop" }); + const controller = new AbortController(); + controller.abort(reason); + let runs = 0; + const service = new TestReverseEngineeringService({ + run: async () => { + runs += 1; + return { stdout: "", stderr: "" }; + }, + }); + try { + const result = await service.inspectWithObjdump( + { path, operation: "symbols" }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(runs).toBe(0); + } finally { + await service.close(); + await rm(directory, { recursive: true, force: true }); + } +}); + +it("preserves the caller reason when cancellation arrives after provider output", async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-reverse-postabort-")); + const path = join(directory, "fixture.bin"); + await writeFile(path, "fixture"); + const reason = Object.freeze({ requested: "stop-after-output" }); + const controller = new AbortController(); + const service = new TestReverseEngineeringService({ + run: async () => { + controller.abort(reason); + return { stdout: "symbols", stderr: "" }; + }, + }); + try { + const result = await service.inspectWithObjdump( + { path, operation: "symbols" }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(result.error.partialObservation).toMatchObject({ + raw_result: { stdout: "symbols" }, + }); + } finally { + await service.close(); + await rm(directory, { recursive: true, force: true }); + } +}); + +it("keeps a physical digest denial primary when the caller aborts during that failure", async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-reverse-digest-deny-")); + const path = join(directory, "fixture.bin"); + await writeFile(path, "fixture"); + const reason = Object.freeze({ requested: "stop-during-denial" }); + const controller = new AbortController(); + const denied = Object.assign(new Error("permission denied"), { + code: "EACCES", + }); + let runs = 0; + const service = new TestReverseEngineeringService({ + digest: async () => { + controller.abort(reason); + throw denied; + }, + run: async () => { + runs += 1; + return { stdout: "", stderr: "" }; + }, + }); + try { + const result = await service.inspectWithObjdump( + { path, operation: "symbols" }, + { signal: controller.signal }, + ); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisAccessDeniedError" }, + }); + if (result.ok) return; + expect(result.error.cause).toBe(denied); + expect(runs).toBe(0); + } finally { + await service.close(); + await rm(directory, { recursive: true, force: true }); + } +}); + +it("keeps an unrelated digest ABORT_ERR as an adapter failure", async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-reverse-digest-abort-")); + const path = join(directory, "fixture.bin"); + await writeFile(path, "fixture"); + const denied = Object.assign(new Error("digest aborted elsewhere"), { + code: "ABORT_ERR", + }); + const service = new TestReverseEngineeringService({ + digest: async () => { + throw denied; + }, + }); + try { + const result = await service.inspectWithObjdump({ + path, + operation: "symbols", + }); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "ProviderAdapterError" }, + }); + if (result.ok) return; + expect(result.error.cause).toBe(denied); + } finally { + await service.close(); + await rm(directory, { recursive: true, force: true }); + } +}); + +it.each([ + { + captured: true, + code: "EACCES", + tag: "AnalysisAccessDeniedError", + }, + { + captured: true, + code: "ETIMEDOUT", + tag: "AnalysisTimeoutError", + }, + { + captured: true, + code: "ABORT_ERR", + tag: "AnalysisCancelledError", + }, + { + captured: false, + code: "EACCES", + tag: "AnalysisAccessDeniedError", + }, + { + captured: false, + code: "ETIMEDOUT", + tag: "AnalysisTimeoutError", + }, + { + captured: false, + code: "ABORT_ERR", + tag: "AnalysisCancelledError", + }, +] as const)( + "classifies a $code command failure with captured output $captured while the caller is aborted", + async ({ captured, code, tag }) => { + const directory = await mkdtemp(join(tmpdir(), "rea-reverse-command-abort-")); + const path = join(directory, "fixture.bin"); + await writeFile(path, "fixture"); + const reason = Object.freeze({ requested: `stop-${code}` }); + const controller = new AbortController(); + const failure = Object.assign(new Error("provider failure"), { + code, + ...(captured + ? { stdout: "partial stdout", stderr: "partial stderr" } + : {}), + }); + const service = new TestReverseEngineeringService({ + run: async () => { + controller.abort(reason); + throw failure; + }, + }); + try { + const result = await service.inspectWithObjdump( + { path, operation: "symbols" }, + { signal: controller.signal }, + ); + expect(result).toMatchObject({ ok: false, error: { _tag: tag } }); + if (result.ok) return; + expect(result.error.cause).toBe( + code === "ABORT_ERR" ? reason : failure, + ); + if (captured && code === "ABORT_ERR") { + expect(result.error.capturedOutput).toMatchObject({ + stdout: "partial stdout", + stderr: "partial stderr", + }); + expect(result.error.partialObservation).toMatchObject({ + raw_result: { stdout: "partial stdout" }, + }); + } + } finally { + await service.close(); + await rm(directory, { recursive: true, force: true }); + } + }, +); From de4579e993bde2935a955166f7556d10ebaa3d68 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 21 Oct 2026 04:17:46 +0800 Subject: [PATCH 2/2] fix(observation): keep incomplete cancellation observations (#5644) * fix(observation): keep incomplete cancellation observations Service projection treated a retained observation that failed the completed schema as a malformed completed result. Cancellation became unreadable_output and the partial was dropped. Incomplete observations now stay on the owner error, including the exact abort reason. * fix(observation): drop non-JSON incomplete observations Keep a retained partial only when projectAnalysisError can serialize it. Clear a non-JSON partial and record an AnalysisOutputError as a secondary failure so cancellation and timeout stay the owner error. Co-authored-by: morluto * fix(observation): validate incomplete partials without copying jsonValueSchema.safeParse cloned a JSON-safe partial that projection copies again. Use jsonValueValidationIssue so the gate only checks. Co-authored-by: morluto * test(ida): expect non-retryable unresolved procedure lookups Main stops treating invalid_request as retryable. An unchanged missing procedure stays invalid_request and is not marked retryable. Co-authored-by: morluto * test(observation): share incomplete-observation assertions The new non-JSON cases repeated the same owner-error checks and pushed duplication over the 3.5% limit. One expectation helper keeps those checks without a second copy. Co-authored-by: morluto * test(observation): keep the assertion helper out of the package The helper imports Vitest. Rename it to a fixture so the production build excludes it and dist does not import a dev dependency. Co-authored-by: morluto * fix(observation): clear non-JSON partials on cleanup primaries Projection can return the cleanup primary after retry. Clearing only the wrapper left that primary's partial in place, so projectAnalysisError threw. The gate now clears every non-JSON partial in the chain. Co-authored-by: morluto * fix(observation): freeze retained incomplete observations A JSON partial stayed a live provider object, so a later mutation could make projection throw. The gate now keeps one frozen snapshot and still drops a non-JSON partial without copying it. Co-authored-by: morluto * refactor(observation): share web runtime partial retention Observe and inspect used the same promote-or-keep path. One helper owns that sequence so the two operations cannot drift. Co-authored-by: morluto * fix(observation): snapshot a shared partial once A cleanup wrapper and its primary can hold the same partial object. The gate now reuses the first frozen snapshot for that object. Co-authored-by: morluto * fix(observation): parse a kept partial once jsonValueSchema.safeParse validates and copies only a JSON partial. The gate freezes that copy and no longer walks the same value first. Co-authored-by: morluto * test(observation): share repeated observation fixtures The incomplete-observation tests repeated the same provider setup and an empty Electron coverage tail, which pushed duplication over 3.5%. One coverage fixture and shared assertions bring the tree back under the threshold. Co-authored-by: morluto --------- Co-authored-by: Cursor Agent Co-authored-by: morluto --- .../BrowserObservationService.test.ts | 185 ++++++++++++++++-- src/application/BrowserObservationService.ts | 94 ++++----- .../BrowserScenarioCaptureService.test.ts | 144 ++++++++++---- .../BrowserScenarioCaptureService.ts | 24 +-- .../ProviderOperationExecution.test.ts | 95 ++++++++- src/application/ProviderOperationExecution.ts | 53 +++++ src/application/WebRuntimeService.test.ts | 135 ++++++++++++- src/application/WebRuntimeService.ts | 112 +++++------ .../ElectronObservationService.test.ts | 98 ++++++---- .../javascript/ElectronObservationService.ts | 53 ++--- ...avaScriptRuntimeObservationService.test.ts | 65 ++++-- .../JavaScriptRuntimeObservationService.ts | 25 +-- .../ownerObservationExpectation.fixture.ts | 72 +++++++ .../electronPageInspection.fixture.ts | 34 ++++ tests/boundary/mcp/electronMcp.test.ts | 30 +-- 15 files changed, 915 insertions(+), 304 deletions(-) create mode 100644 src/application/ownerObservationExpectation.fixture.ts create mode 100644 src/domain/javascript/electronPageInspection.fixture.ts diff --git a/src/application/BrowserObservationService.test.ts b/src/application/BrowserObservationService.test.ts index d2cdde1c27..b6e33a0e23 100644 --- a/src/application/BrowserObservationService.test.ts +++ b/src/application/BrowserObservationService.test.ts @@ -3,9 +3,19 @@ import type { BrowserObservationPort } from "./BrowserObservationPort.js"; import { listBrowserTargets } from "./BrowserObservationService.js"; import type { BrowserTargetList } from "../domain/browserObservationSchemas.js"; import { parseEvidence } from "../domain/evidence.js"; -import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisOutputError, +} from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { + expectDroppedPartial, + expectKeptPartial, + expectOutputError, + expectOwnerError, +} from "./ownerObservationExpectation.fixture.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; -import { ok } from "../domain/result.js"; +import { err, ok } from "../domain/result.js"; import { passiveNetworkCapture } from "../../tests/fixtures/passiveNetworkCapture.js"; import { inspectWebPage } from "./BrowserObservationService.js"; import { analyzeWebBundleInputSchema } from "../domain/webBundleAnalysis.js"; @@ -71,11 +81,7 @@ it("does not let late cancellation hide invalid completed browser output", async provider(async () => ok(malformed)), input, ); - expect(output).toMatchObject({ - ok: false, - error: { _tag: "AnalysisOutputError" }, - }); - if (!output.ok) expect(output.error).toBeInstanceOf(AnalysisOutputError); + expectOutputError(output); const controller = new AbortController(); const late = await listBrowserTargets( @@ -86,11 +92,7 @@ it("does not let late cancellation hide invalid completed browser output", async input, { signal: controller.signal }, ); - expect(late).toMatchObject({ - ok: false, - error: { _tag: "AnalysisOutputError" }, - }); - if (!late.ok) expect(late.error).toBeInstanceOf(AnalysisOutputError); + expectOutputError(late); }); it("converts a raw page inspection retained after browser cleanup failure", async () => { @@ -215,3 +217,162 @@ it("preserves a prior caller reason without admitting browser work", async () => expect(result.error._tag).toBe("AnalysisCancelledError"); expect(result.error.cause).toBe(reason); }); + +it("keeps page cancellation when the retained inspection is incomplete", async () => { + const reason = { requested: "stop during page inspection" }; + const partial = { + target: { target_id: "page" }, + network: {}, + capture_window: {}, + }; + const failure = new AnalysisCancelledError("inspect_web_page", { + cause: reason, + partialObservation: partial as never, + }); + const browser = { + ...provider(async () => ok(report)), + inspectPage: async () => err(failure), + } as unknown as BrowserObservationPort; + const pageInput = inspectWebPageInputSchema.parse({ + cdp_endpoint: input.cdp_endpoint.recorded, + allowed_origins: [], + target_id: "page", + observation_ms: 0, + include_script_sources: false, + }); + const result = await inspectWebPage(browser, pageInput); + expectKeptPartial(expectOwnerError(result, failure, reason), partial); +}); + +it("keeps bundle cancellation and an earlier failure when retained facts are incomplete", async () => { + const reason = { requested: "stop during bundle analysis" }; + const partial = { observations: [], capture: { status: "partial" } }; + const cancelled = new AnalysisCancelledError("analyze_web_bundle", { + cause: reason, + partialObservation: partial as never, + }); + const timedOut = new BrowserObservationError( + "analyze_web_bundle", + "timeout", + { partialObservation: partial as never }, + ); + const selected = analyzeWebBundleInputSchema.parse({ + cdp_endpoint: input.cdp_endpoint.recorded, + target_id: "page", + observation_ms: 0, + }); + for (const failure of [cancelled, timedOut]) { + const browser = { + ...provider(async () => ok(report)), + analyzeBundle: async () => err(failure), + } as unknown as BrowserObservationPort; + const result = await analyzeWebBundle(browser, selected); + expectKeptPartial( + expectOwnerError(result, failure, failure.cause), + partial, + failure === timedOut ? "provider_timeout" : "cancelled", + ); + } + expect(projectAnalysisError(cancelled).code).toBe("cancelled"); +}); + +const pageInput = () => + inspectWebPageInputSchema.parse({ + cdp_endpoint: input.cdp_endpoint.recorded, + allowed_origins: [], + target_id: "page", + observation_ms: 0, + include_script_sources: false, + }); + +const bundleInput = () => + analyzeWebBundleInputSchema.parse({ + cdp_endpoint: input.cdp_endpoint.recorded, + target_id: "page", + observation_ms: 0, + }); + +const browserReturning = ( + failure: BrowserObservationError | AnalysisCancelledError, +) => + ({ + ...provider(async () => ok(report)), + inspectPage: async () => err(failure), + analyzeBundle: async () => err(failure), + }) as unknown as BrowserObservationPort; + +it.each([ + { + name: "bundle bytes", + operation: "analyze_web_bundle" as const, + partial: { observations: [], capture: { bytes: Number.NaN } }, + }, + { + name: "bundle observations", + operation: "analyze_web_bundle" as const, + partial: { observations: [undefined], capture: {} }, + }, + { + name: "page requests", + operation: "inspect_web_page" as const, + partial: { + target: { target_id: "page" }, + network: { requests: Number.NaN }, + capture_window: {}, + }, + }, +])( + "drops a non-JSON $name partial and keeps cancellation primary", + async ({ operation, partial }) => { + const reason = { requested: `stop ${operation}` }; + const failure = new AnalysisCancelledError(operation, { + cause: reason, + partialObservation: partial as never, + }); + const result = + operation === "inspect_web_page" + ? await inspectWebPage(browserReturning(failure), pageInput()) + : await analyzeWebBundle(browserReturning(failure), bundleInput()); + const error = expectOwnerError(result, failure, reason); + expectDroppedPartial( + error, + "Browser provider retained a non-JSON incomplete observation.", + ); + expect(projectAnalysisError(error).details).not.toHaveProperty( + "partial_observation", + ); + }, +); + +it.each([ + { + name: "schema-shaped page", + partial: { + target: { target_id: "page" }, + network: { requests: Number.NaN }, + capture_window: {}, + }, + }, + { + name: "shapeless page", + partial: { requests: Number.NaN }, + }, +])( + "drops a non-JSON $name partial and keeps timeout primary", + async ({ partial }) => { + const failure = new BrowserObservationError("inspect_web_page", "timeout", { + partialObservation: partial as never, + }); + const error = expectOwnerError( + await inspectWebPage(browserReturning(failure), pageInput()), + failure, + undefined, + ); + expectDroppedPartial( + error, + "Browser provider retained a non-JSON incomplete observation.", + "provider_timeout", + ); + expect(projectAnalysisError(error).category).toBe("timeout"); + }, +); diff --git a/src/application/BrowserObservationService.ts b/src/application/BrowserObservationService.ts index 6251e9d4dd..1e32b4ead4 100644 --- a/src/application/BrowserObservationService.ts +++ b/src/application/BrowserObservationService.ts @@ -25,9 +25,11 @@ import type { ExecutionOptions } from "./AnalysisProvider.js"; import type { BrowserObservationPort } from "./BrowserObservationPort.js"; import { createBrowserEvidence } from "./BrowserEvidence.js"; import { jsonValueSchema } from "../domain/jsonValue.js"; -import { completeProviderEvidence } from "./ProviderOperationExecution.js"; +import { + completeProviderEvidence, + keepIncompleteObservation, +} from "./ProviderOperationExecution.js"; import { webPageInspectionSchema } from "../domain/browserObservationSchemas.js"; -import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; /** List browser targets within the endpoint and origin scope in the request. */ export const listBrowserTargets = async ( @@ -275,7 +277,10 @@ const browserEvidence = ( ); }; -/** Retain the completed capture or bundle facts under their actual operation. */ +const NON_JSON_BROWSER_OBSERVATION = + "Browser provider retained a non-JSON incomplete observation."; + +/** Promote completed page or bundle facts; keep only a JSON-safe incomplete observation. */ const retainBrowserObservation = ( error: AnalysisError, input: InspectWebPageInput | AnalyzeWebBundleInput, @@ -291,59 +296,42 @@ const retainBrowserObservation = ( "capture" in partial ) { const bundle = webBundleAnalysisSchema.safeParse(partial); - if (!bundle.success) { - const malformed = new AnalysisOutputError( - errorOperation, - "Browser provider retained malformed completed bundle facts.", - { cause: bundle.error }, - ); - return err( - error.cleanupIncomplete - ? analysisErrorWithCleanupFailure( - malformed, - error, - errorOperation, - ).bindCleanupDependencies([malformed, error]) - : malformed, + if (bundle.success) { + error.partialObservation = createBrowserEvidence( + "analyze_web_bundle", + input, + bundle.data, + provider.identity(), ); + return err(error); } - error.partialObservation = createBrowserEvidence( - "analyze_web_bundle", - input, - bundle.data, - provider.identity(), - ); - return err(error); - } - if ( - partial === undefined || - "evidence_id" in partial || - !("target" in partial) || - !("network" in partial) || - !("capture_window" in partial) - ) - return err(error); - const inspection = webPageInspectionSchema.safeParse(partial); - if (!inspection.success) { - if (error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - errorOperation, - "Browser provider retained a malformed completed page inspection.", - { cause: inspection.error }, - ), + } else if ( + partial !== undefined && + !("evidence_id" in partial) && + "target" in partial && + "network" in partial && + "capture_window" in partial + ) { + const inspection = webPageInspectionSchema.safeParse(partial); + if (inspection.success) { + const pageInput: InspectWebPageInput = + "fetch_source_maps" in input + ? (({ fetch_source_maps: _fetchSourceMaps, ...page }) => page)(input) + : input; + error.partialObservation = createBrowserEvidence( + "inspect_web_page", + pageInput, + inspection.data, + provider.identity(), ); - return err(error); + return err(error); + } } - const pageInput: InspectWebPageInput = - "fetch_source_maps" in input - ? (({ fetch_source_maps: _fetchSourceMaps, ...page }) => page)(input) - : input; - error.partialObservation = createBrowserEvidence( - "inspect_web_page", - pageInput, - inspection.data, - provider.identity(), + return err( + keepIncompleteObservation( + error, + errorOperation, + NON_JSON_BROWSER_OBSERVATION, + ), ); - return err(error); }; diff --git a/src/application/BrowserScenarioCaptureService.test.ts b/src/application/BrowserScenarioCaptureService.test.ts index df187682a4..34a3c588cb 100644 --- a/src/application/BrowserScenarioCaptureService.test.ts +++ b/src/application/BrowserScenarioCaptureService.test.ts @@ -1,13 +1,26 @@ import { expect, it } from "vitest"; -import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { + AnalysisCancelledError, + AnalysisOutputError, +} from "../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { parseEvidence } from "../domain/evidence.js"; import { browserScenarioInputSchema } from "../domain/browserScenario.js"; -import { err } from "../domain/result.js"; +import type { + BrowserScenarioCapture, + BrowserScenarioPartialObservation, +} from "../domain/browserScenarioCapture.js"; +import { err, ok } from "../domain/result.js"; import { scriptScenarioFixture } from "../../tests/fixtures/webScriptCapture.js"; import type { BrowserScenarioCapturePort } from "./BrowserScenarioCapturePort.js"; import { captureBrowserScenario } from "./BrowserScenarioCaptureService.js"; +import { + expectDroppedPartial, + expectKeptPartial, + expectOwnerError, +} from "./ownerObservationExpectation.fixture.js"; const scenario = browserScenarioInputSchema.parse({ browser: { mode: "launch", executable_path: "/fixture/chrome" }, @@ -15,6 +28,34 @@ const scenario = browserScenarioInputSchema.parse({ actions: [{ step_id: "settle", action: "wait_for_timeout", duration_ms: 1 }], }); const capture = scriptScenarioFixture(); +const scenarioIdentity = { + id: "scenario-fixture", + name: "Scenario fixture", + version: "1", +}; +const runScenario = ( + failure: AnalysisError, + id = { id: "fixture", name: "fixture", version: "1" }, +) => + captureBrowserScenario( + { + identity: () => id, + captureScenario: async () => err(failure), + }, + scenario, + ); +const expectRetainedCapture = ( + result: Awaited>, + failure: AnalysisError, +) => { + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected the retained capture"); + expect(result.error).toBe(failure); + expect(parseEvidence(result.error.partialObservation)).toMatchObject({ + operation: "capture_browser_scenario", + normalized_result: capture, + }); +}; it("converts a retained raw scenario capture into partial Evidence", async () => { const failure = new AnalysisCancelledError("capture_browser_scenario", { @@ -23,23 +64,7 @@ it("converts a retained raw scenario capture into partial Evidence", async () => capture, }, }); - const provider: BrowserScenarioCapturePort = { - identity: () => ({ - id: "scenario-fixture", - name: "Scenario fixture", - version: "1", - }), - captureScenario: async () => err(failure), - }; - const result = await captureBrowserScenario(provider, scenario); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.error).toBe(failure); - expect(parseEvidence(result.error.partialObservation)).toMatchObject({ - operation: "capture_browser_scenario", - normalized_result: capture, - }); - } + expectRetainedCapture(await runScenario(failure, scenarioIdentity), failure); }); it("converts a raw scenario capture retained after cleanup failure", async () => { @@ -57,24 +82,9 @@ it("converts a raw scenario capture retained after cleanup failure", async () => }, }, ); - const provider: BrowserScenarioCapturePort = { - identity: () => ({ - id: "scenario-fixture", - name: "Scenario fixture", - version: "1", - }), - captureScenario: async () => err(failure), - }; - const result = await captureBrowserScenario(provider, scenario); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.error).toBe(failure); - expect(result.error.cleanupIncomplete).toBe(true); - expect(parseEvidence(result.error.partialObservation)).toMatchObject({ - operation: "capture_browser_scenario", - normalized_result: capture, - }); - } + const result = await runScenario(failure, scenarioIdentity); + expectRetainedCapture(result, failure); + if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true); }); it.each([ @@ -103,3 +113,63 @@ it.each([ expect(calls).toBe(0); }, ); + +it("keeps startup cancellation and its incomplete capture", async () => { + const reason = { requested: "stop during initial navigation" }; + const completed = scriptScenarioFixture(); + const { + completeness: _completeness, + steps: _steps, + browser, + ...retained + } = completed; + const partial: BrowserScenarioPartialObservation = { + kind: "browser-scenario-observation", + capture: { + ...retained, + browser: { ...browser, cleanup: "incomplete" }, + steps: [], + }, + }; + const failure = new AnalysisCancelledError("capture_browser_scenario", { + cause: reason, + partialObservation: partial, + }); + const result = await runScenario(failure); + expectKeptPartial(expectOwnerError(result, failure, reason), partial); +}); + +it.each([{ steps: [undefined] }, { started_at: Number.NaN }])( + "drops a non-JSON incomplete scenario capture and keeps cancellation primary", + async (capturePartial) => { + const reason = { requested: "stop during initial navigation" }; + const partial: BrowserScenarioPartialObservation = { + kind: "browser-scenario-observation", + capture: capturePartial as never, + }; + const failure = new AnalysisCancelledError("capture_browser_scenario", { + cause: reason, + partialObservation: partial, + }); + const result = await runScenario(failure); + expectDroppedPartial( + expectOwnerError(result, failure, reason), + "Browser scenario provider retained a non-JSON incomplete observation.", + ); + }, +); + +it("rejects a successful capture that is not a completed scenario", async () => { + const provider: BrowserScenarioCapturePort = { + identity: () => ({ id: "fixture", name: "fixture", version: "1" }), + captureScenario: async () => ok({} as BrowserScenarioCapture), + }; + const result = await captureBrowserScenario(provider, scenario); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected a completed-capture refusal"); + expect(result.error).toBeInstanceOf(AnalysisOutputError); + expect(result.error.message).toContain( + "returned a malformed completed capture", + ); + expect(result.error.partialObservation).toBeUndefined(); +}); diff --git a/src/application/BrowserScenarioCaptureService.ts b/src/application/BrowserScenarioCaptureService.ts index 0a5bc26019..24e8c0ca4f 100644 --- a/src/application/BrowserScenarioCaptureService.ts +++ b/src/application/BrowserScenarioCaptureService.ts @@ -11,7 +11,10 @@ import type { Evidence } from "../domain/evidence.js"; import type { BrowserScenario } from "../domain/browserScenario.js"; import { err, type Result } from "../domain/result.js"; import { browserScenarioCaptureSchema } from "../domain/browserScenarioCapture.js"; -import { completeProviderEvidence } from "./ProviderOperationExecution.js"; +import { + completeProviderEvidence, + keepIncompleteObservation, +} from "./ProviderOperationExecution.js"; const OPERATION = "capture_browser_scenario" as const; const PROVIDER_ID = "rea-playwright-browser-scenario"; @@ -43,6 +46,8 @@ export const captureBrowserScenario = async ( partial.kind === "browser-scenario-observation" ) { const report = browserScenarioCaptureSchema.safeParse(partial.capture); + // Only a completed capture becomes Evidence. A JSON-safe incomplete + // observation stays on the owner error, including typed cancellation. if ( report.success && report.data.steps.every(({ status }) => status === "completed") @@ -52,19 +57,16 @@ export const captureBrowserScenario = async ( report.data, provider.identity(), ); - } else if (!report.success) { - if (captured.error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - OPERATION, - "Browser scenario provider retained a malformed completed capture.", - { cause: report.error }, - ), - ); return captured; } } - return captured; + return err( + keepIncompleteObservation( + captured.error, + OPERATION, + "Browser scenario provider retained a non-JSON incomplete observation.", + ), + ); } const report = browserScenarioCaptureSchema.safeParse(captured.value); if (!report.success) diff --git a/src/application/ProviderOperationExecution.test.ts b/src/application/ProviderOperationExecution.test.ts index 7729cb42b7..f3a36e1b5a 100644 --- a/src/application/ProviderOperationExecution.test.ts +++ b/src/application/ProviderOperationExecution.test.ts @@ -1,8 +1,18 @@ import { expect, it, vi } from "vitest"; import { z } from "zod"; -import { AnalysisInputError } from "../domain/analysisErrorCore.js"; -import { executeProviderOperation } from "./ProviderOperationExecution.js"; +import { + AnalysisCancelledError, + AnalysisInputError, + AnalysisOutputError, +} from "../domain/analysisErrorCore.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { + executeProviderOperation, + keepIncompleteObservation, +} from "./ProviderOperationExecution.js"; it("projects request-schema issues without exposing the request envelope", async () => { const execute = vi.fn(); @@ -87,3 +97,84 @@ it("preserves operation correction outside the input envelope", async () => { ]); expect(execute).not.toHaveBeenCalled(); }); + +const cancelledScenario = (partial: unknown) => + new AnalysisCancelledError("capture_browser_scenario", { + cause: { requested: "stop during capture" }, + partialObservation: partial as never, + }); + +const wrapScenarioCleanup = (primary: AnalysisCancelledError) => + analysisErrorWithCleanupFailure( + primary, + new ProviderCleanupError("fixture", ["browser"], {}), + "capture_browser_scenario", + ); + +it("clears a non-JSON partial from the cleanup primary projection returns after retry", () => { + const partial = { steps: [] as unknown[] }; + const primary = cancelledScenario(partial); + const composed = wrapScenarioCleanup(primary); + partial.steps.push(undefined); + const reason = + "Browser scenario provider retained a non-JSON incomplete observation."; + const retained = keepIncompleteObservation( + composed, + "capture_browser_scenario", + reason, + ); + + expect(retained).toBe(composed); + expect(composed.partialObservation).toBeUndefined(); + expect(primary.partialObservation).toBeUndefined(); + expect(composed.secondaryFailures).toMatchObject([ + { _tag: "AnalysisOutputError", reason }, + ]); + expect(primary.secondaryFailures[0]).toBeInstanceOf(AnalysisOutputError); + expect(projectAnalysisError(composed).code).toBe("cleanup_incomplete"); + + composed.resolveCleanup(); + expect(projectAnalysisError(composed)).toMatchObject({ + code: "cancelled", + category: "cancelled", + }); + expect(projectAnalysisError(composed).details).not.toHaveProperty( + "partial_observation", + ); +}); + +it("freezes one shared snapshot for a cleanup wrapper and its primary", () => { + const partial = { steps: ["kept"] }; + const primary = cancelledScenario(partial); + const composed = wrapScenarioCleanup(primary); + keepIncompleteObservation( + composed, + "capture_browser_scenario", + "unused because the partial is JSON", + ); + + expect(composed.partialObservation).toBe(primary.partialObservation); + expect(composed.partialObservation).toEqual({ steps: ["kept"] }); + expect(Object.isFrozen(composed.partialObservation)).toBe(true); + expect(composed.secondaryFailures).toHaveLength(0); + expect(primary.secondaryFailures).toHaveLength(0); +}); + +it("freezes a retained partial so a later mutation cannot break projection", () => { + const partial = { steps: [] as unknown[] }; + const primary = cancelledScenario(partial); + keepIncompleteObservation( + primary, + "capture_browser_scenario", + "unused because the partial is JSON", + ); + partial.steps.push(undefined); + + expect(primary.partialObservation).toEqual({ steps: [] }); + expect(Object.isFrozen(primary.partialObservation)).toBe(true); + expect(primary.secondaryFailures).toHaveLength(0); + expect(projectAnalysisError(primary)).toMatchObject({ + code: "cancelled", + details: { partial_observation: { steps: [] } }, + }); +}); diff --git a/src/application/ProviderOperationExecution.ts b/src/application/ProviderOperationExecution.ts index 7a48e9ae88..38012d0b5b 100644 --- a/src/application/ProviderOperationExecution.ts +++ b/src/application/ProviderOperationExecution.ts @@ -2,12 +2,19 @@ import { z } from "zod"; import type { AnalysisError, AnalysisErrorOptions, + AnalysisPartialObservation, } from "../domain/analysisErrorBase.js"; import { AnalysisCancelledError, AnalysisInputError, + AnalysisOutputError, } from "../domain/analysisErrorCore.js"; import type { Evidence } from "../domain/evidence.js"; +import { + freezeJsonSnapshot, + isImmutableJsonSnapshot, +} from "../domain/immutableJson.js"; +import { jsonValueSchema } from "../domain/jsonValue.js"; import { projectInputIssues } from "../domain/inputIssueProjection.js"; import { err, ok, type Result } from "../domain/result.js"; import { @@ -92,3 +99,49 @@ export const completeProviderEvidence = ( ); return ok(evidence); }; + +/** + * Retain an incomplete observation when projection can serialize it. + * A JSON partial is replaced by one frozen snapshot, so a later mutation + * cannot make projection throw and projection does not copy it again. + * A partial that is not JSON is cleared from the returned error and from + * every cleanup primary projection can select later. Each cleared error + * records an output failure, leaving cancellation or timeout primary. + */ +export const keepIncompleteObservation = ( + error: AnalysisError, + operation: string, + reason: string, +): AnalysisError => { + const seen = new Set(); + // One source object can sit on a wrapper and its cleanup primary. + const judged = new Map(); + let current: AnalysisError | undefined = error; + while (current !== undefined && !seen.has(current)) { + seen.add(current); + const partial = current.partialObservation; + if ( + partial !== undefined && + !isImmutableJsonSnapshot(partial) && + typeof partial === "object" + ) { + // safeParse clones only after validation succeeds. Freeze that copy once. + let snapshot = judged.get(partial); + if (!judged.has(partial)) { + const parsed = jsonValueSchema.safeParse(partial); + snapshot = parsed.success + ? (freezeJsonSnapshot(parsed.data) as AnalysisPartialObservation) + : undefined; + judged.set(partial, snapshot); + } + if (snapshot === undefined) { + current.partialObservation = undefined; + current.retainSecondaryFailure( + new AnalysisOutputError(operation, reason), + ); + } else current.partialObservation = snapshot; + } + current = current.cleanupPrimary; + } + return error; +}; diff --git a/src/application/WebRuntimeService.test.ts b/src/application/WebRuntimeService.test.ts index 7e7c651486..f28be157d9 100644 --- a/src/application/WebRuntimeService.test.ts +++ b/src/application/WebRuntimeService.test.ts @@ -6,10 +6,20 @@ import { webListenersFixture, } from "./WebRuntimeService.fixture.js"; import { err, ok } from "../domain/result.js"; -import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisOutputError, + AnalysisTimeoutError, +} from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { + expectDroppedPartial, + expectKeptPartial, + expectOwnerError, +} from "./ownerObservationExpectation.fixture.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { parseEvidence } from "../domain/evidence.js"; -import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; it("rejects malformed input and prior cancellation without calling a provider", async () => { const provider = recordingWebRuntimePort(); @@ -83,11 +93,7 @@ it("converts raw execution retained by provider cleanup failure into partial Evi { operation: "observe_web_execution", partialObservation: execution }, ); const result = await new WebRuntimeService({ - identity: () => ({ - id: "web-runtime", - name: "Runtime fixture", - version: "1", - }), + identity: runtimeIdentity, observeExecution: async () => err(failure), inspectEventListeners: async () => err(failure), }).observe(args); @@ -161,3 +167,118 @@ it("retains completed execution Evidence on late abort and keeps invalid output }); if (!invalid.ok) expect(invalid.error).toBeInstanceOf(AnalysisOutputError); }); + +const runtimeIdentity = () => ({ + id: "web-runtime", + name: "Runtime fixture", + version: "1", +}); + +const callRuntime = ( + operation: "observe" | "inspect", + failure: AnalysisError, +) => { + const service = new WebRuntimeService({ + identity: runtimeIdentity, + observeExecution: async () => err(failure), + inspectEventListeners: async () => err(failure), + }); + return operation === "observe" + ? service.observe({ + cdp_endpoint: "http://127.0.0.1:9222", + target_id: "selected-page", + observation_ms: 5, + }) + : service.inspect({ + cdp_endpoint: "http://127.0.0.1:9222", + target_id: "selected-page", + selector: "#selected", + }); +}; + +it.each([ + { + operation: "observe" as const, + partial: { + instrumentation: { cleanup: "unconfirmed" }, + window: { requested_ms: 5 }, + }, + }, + { + operation: "inspect" as const, + partial: { + selected_node: { selector: "#selected" }, + listeners: [], + }, + }, +])( + "keeps $operation cancellation when the retained observation is incomplete", + async ({ operation, partial }) => { + const reason = { requested: `stop incomplete ${operation}` }; + const failure = new AnalysisCancelledError( + operation === "observe" + ? "observe_web_execution" + : "inspect_web_event_listeners", + { cause: reason, partialObservation: partial as never }, + ); + expectKeptPartial( + expectOwnerError(await callRuntime(operation, failure), failure, reason), + partial, + ); + }, +); + +it.each([ + { + operation: "observe" as const, + partial: { + instrumentation: { cleanup: "unconfirmed" }, + window: { requested_ms: Number.NaN }, + }, + reason: "Provider retained a non-JSON incomplete execution observation.", + }, + { + operation: "inspect" as const, + partial: { + selected_node: { selector: "#selected" }, + listeners: [undefined], + }, + reason: + "Provider retained a non-JSON incomplete event-listener observation.", + }, +])( + "drops a non-JSON $operation partial and keeps cancellation primary", + async ({ operation, partial, reason: outputReason }) => { + const reason = { requested: `stop unsafe ${operation}` }; + const failure = new AnalysisCancelledError( + operation === "observe" + ? "observe_web_execution" + : "inspect_web_event_listeners", + { cause: reason, partialObservation: partial as never }, + ); + expectDroppedPartial( + expectOwnerError(await callRuntime(operation, failure), failure, reason), + outputReason, + ); + }, +); + +it("drops a non-JSON execution partial and keeps timeout primary", async () => { + const failure = new AnalysisTimeoutError("observe_web_execution", 5, { + partialObservation: { + instrumentation: { samples: [undefined] }, + window: { requested_ms: 5 }, + } as never, + }); + const error = expectOwnerError( + await callRuntime("observe", failure), + failure, + undefined, + ); + expectDroppedPartial( + error, + "Provider retained a non-JSON incomplete execution observation.", + "provider_timeout", + ); + expect(projectAnalysisError(error).category).toBe("timeout"); +}); diff --git a/src/application/WebRuntimeService.ts b/src/application/WebRuntimeService.ts index 38d2934545..627ada4906 100644 --- a/src/application/WebRuntimeService.ts +++ b/src/application/WebRuntimeService.ts @@ -18,7 +18,10 @@ import { inspectWebEventListenersInputSchema, webEventListenersSchema, } from "../domain/webEventListeners.js"; -import { completeProviderEvidence } from "./ProviderOperationExecution.js"; +import { + completeProviderEvidence, + keepIncompleteObservation, +} from "./ProviderOperationExecution.js"; /** Shared public workflows keep instrumentation and listener inspection independently selectable. */ export class WebRuntimeService { @@ -38,35 +41,19 @@ export class WebRuntimeService { ); if (!parsed.ok) return parsed; const result = await this.provider.observeExecution(parsed.value, options); - if (!result.ok) { - const partial = result.error.partialObservation; - if ( - partial !== undefined && - !("evidence_id" in partial) && - "instrumentation" in partial && - "window" in partial - ) { - const verifiedPartial = webExecutionSchema.safeParse(partial); - if (verifiedPartial.success) { - const evidence = this.evidence( - operation, - parsed.value, - verifiedPartial.data, - ); - if (evidence.ok) result.error.partialObservation = evidence.value; - else if (result.error instanceof AnalysisCancelledError) - return evidence; - } else if (result.error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - operation, - "Provider retained a malformed completed execution observation.", - { cause: verifiedPartial.error }, - ), - ); - } - return result; - } + if (!result.ok) + return this.retainRuntimePartial( + result.error, + operation, + "Provider retained a non-JSON incomplete execution observation.", + (partial) => { + if (!("instrumentation" in partial) || !("window" in partial)) + return undefined; + const verified = webExecutionSchema.safeParse(partial); + if (!verified.success) return undefined; + return this.evidence(operation, parsed.value, verified.data); + }, + ); const verified = webExecutionSchema.safeParse(result.value); if (!verified.success) return err(new AnalysisOutputError(operation, verified.error.message)); @@ -100,35 +87,19 @@ export class WebRuntimeService { parsed.value, options, ); - if (!result.ok) { - const partial = result.error.partialObservation; - if ( - partial !== undefined && - !("evidence_id" in partial) && - "selected_node" in partial && - "listeners" in partial - ) { - const verifiedPartial = webEventListenersSchema.safeParse(partial); - if (verifiedPartial.success) { - const evidence = this.evidence( - operation, - parsed.value, - verifiedPartial.data, - ); - if (evidence.ok) result.error.partialObservation = evidence.value; - else if (result.error instanceof AnalysisCancelledError) - return evidence; - } else if (result.error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - operation, - "Provider retained malformed completed event-listener data.", - { cause: verifiedPartial.error }, - ), - ); - } - return result; - } + if (!result.ok) + return this.retainRuntimePartial( + result.error, + operation, + "Provider retained a non-JSON incomplete event-listener observation.", + (partial) => { + if (!("selected_node" in partial) || !("listeners" in partial)) + return undefined; + const verified = webEventListenersSchema.safeParse(partial); + if (!verified.success) return undefined; + return this.evidence(operation, parsed.value, verified.data); + }, + ); const verified = webEventListenersSchema.safeParse(result.value); if (!verified.success) return err(new AnalysisOutputError(operation, verified.error.message)); @@ -145,6 +116,29 @@ export class WebRuntimeService { return this.evidence(operation, parsed.value, verified.data, options); } + /** Promote a schema-valid partial, or keep a JSON snapshot of an incomplete one. */ + private retainRuntimePartial( + error: AnalysisError, + operation: string, + nonJsonReason: string, + promote: ( + partial: NonNullable, + ) => Result | undefined, + ): Result { + const partial = error.partialObservation; + if (partial !== undefined && !("evidence_id" in partial)) { + const evidence = promote(partial); + if (evidence !== undefined) { + if (evidence.ok) { + error.partialObservation = evidence.value; + return err(error); + } + if (error instanceof AnalysisCancelledError) return evidence; + } + } + return err(keepIncompleteObservation(error, operation, nonJsonReason)); + } + private evidence( operation: string, input: diff --git a/src/application/javascript/ElectronObservationService.test.ts b/src/application/javascript/ElectronObservationService.test.ts index 0d055e1f09..01e9e8c0e2 100644 --- a/src/application/javascript/ElectronObservationService.test.ts +++ b/src/application/javascript/ElectronObservationService.test.ts @@ -1,6 +1,5 @@ import { expect, it } from "vitest"; -import { classifyBrowserCompleteness } from "../../domain/browserCompleteness.js"; import { AnalysisCancelledError, AnalysisOutputError, @@ -12,14 +11,27 @@ import { electronTargetListSchema, inspectElectronPageInputSchema, } from "../../domain/javascript/electronObservation.js"; +import { emptyElectronPageCoverage } from "../../domain/javascript/electronPageInspection.fixture.js"; +import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { err, ok } from "../../domain/result.js"; import type { ElectronObservationPort } from "./ElectronObservationPort.js"; import { inspectElectronPage, listElectronTargets, } from "./ElectronObservationService.js"; +import { + expectDroppedPartial, + expectKeptPartial, + expectOwnerError, +} from "../ownerObservationExpectation.fixture.js"; const identity = { id: "electron-fixture", name: "Fixture", version: "1" }; +const pageProvider = (failure: AnalysisError): ElectronObservationPort => ({ + identity: () => identity, + listTargets: async () => + err(new AnalysisCancelledError("list_electron_targets")), + inspectPage: async () => err(failure), +}); const input = inspectElectronPageInputSchema.parse({ cdp_endpoint: "http://127.0.0.1:9222", target_id: "electron-page", @@ -40,33 +52,7 @@ const inspection = electronPageInspectionSchema.parse({ file_path: "/app/index.html", attached: false, }, - capture_window: { - started_at: "2026-01-01T00:00:00.000Z", - ended_at: "2026-01-01T00:00:00.000Z", - observation_ms: 0, - }, - completeness: classifyBrowserCompleteness({ - policyFilteredSections: new Set(), - attachLimitedSections: new Set(), - truncatedSections: new Set(), - unavailableSections: new Set(), - excluded: [], - droppedEvents: { - scripts: 0, - network_requests: 0, - console_events: 0, - websocket_connections: 0, - websocket_frames: 0, - webmcp_tools: 0, - timeline_events: 0, - }, - }), - frames: [], - dom: { total_nodes: 0, nodes: [] }, - scripts: { total: 0, items: [] }, - resources: [], - workers: [], - limitations: [], + ...emptyElectronPageCoverage(), }); it("converts a retained raw Electron inspection into partial Evidence", async () => { @@ -77,13 +63,7 @@ it("converts a retained raw Electron inspection into partial Evidence", async () cleanup: "transport-closed", }, }); - const provider: ElectronObservationPort = { - identity: () => identity, - listTargets: async () => - err(new AnalysisCancelledError("list_electron_targets")), - inspectPage: async () => err(failure), - }; - const result = await inspectElectronPage(provider, input); + const result = await inspectElectronPage(pageProvider(failure), input); expect(result.ok).toBe(false); if (!result.ok) { expect(result.error).toBe(failure); @@ -116,13 +96,7 @@ it("converts a raw inspection retained by provider cleanup failure", async () => }, }, ); - const provider: ElectronObservationPort = { - identity: () => identity, - listTargets: async () => - err(new AnalysisCancelledError("list_electron_targets")), - inspectPage: async () => err(failure), - }; - const result = await inspectElectronPage(provider, input); + const result = await inspectElectronPage(pageProvider(failure), input); expect(result.ok).toBe(false); if (!result.ok) { expect(result.error).toBe(failure); @@ -244,3 +218,43 @@ it.each(["list", "inspect"] as const)( expect(providerCalls).toBe(0); }, ); + +it("keeps cancellation when the retained Electron inspection is incomplete", async () => { + const reason = { requested: "stop before page inspection completed" }; + const partial = { + kind: "electron-page-observation", + inspection: { target_id: "electron-page" }, + cleanup: "incomplete", + }; + const failure = new AnalysisCancelledError("inspect_electron_page", { + cause: reason, + partialObservation: partial as never, + }); + const result = await inspectElectronPage(pageProvider(failure), input); + expectKeptPartial(expectOwnerError(result, failure, reason), partial); +}); + +it.each([ + { inspection: { requests: Number.NaN }, cleanup: "incomplete" as const }, + { + inspection: { scripts: [undefined] }, + cleanup: "transport-closed" as const, + }, +])( + "drops a non-JSON Electron inspection and keeps cancellation primary", + async (unsafe) => { + const reason = { requested: "stop before page inspection completed" }; + const failure = new AnalysisCancelledError("inspect_electron_page", { + cause: reason, + partialObservation: { + kind: "electron-page-observation", + ...unsafe, + } as never, + }); + const result = await inspectElectronPage(pageProvider(failure), input); + expectDroppedPartial( + expectOwnerError(result, failure, reason), + "Electron provider retained a non-JSON incomplete observation.", + ); + }, +); diff --git a/src/application/javascript/ElectronObservationService.ts b/src/application/javascript/ElectronObservationService.ts index 67a0776cb4..ead41e418f 100644 --- a/src/application/javascript/ElectronObservationService.ts +++ b/src/application/javascript/ElectronObservationService.ts @@ -18,7 +18,10 @@ import { electronPageInspectionSchema, electronTargetListSchema, } from "../../domain/javascript/electronObservation.js"; -import { completeProviderEvidence } from "../ProviderOperationExecution.js"; +import { + completeProviderEvidence, + keepIncompleteObservation, +} from "../ProviderOperationExecution.js"; /** List local file pages exposed by the explicitly selected endpoint. */ export const listElectronTargets = async ( @@ -79,34 +82,32 @@ export const inspectElectronPage = async ( partial.kind === "electron-page-observation" ) { const report = electronPageInspectionSchema.safeParse(partial.inspection); - if (!report.success) { - if (result.error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - "inspect_electron_page", - "Electron provider retained a malformed completed page inspection.", - { cause: report.error }, - ), - ); + if (report.success) { + const inspection = { + ...report.data, + limitations: [ + ...report.data.limitations, + partial.cleanup === "transport-closed" + ? "REA closed its CDP transport; this does not imply target shutdown." + : "CDP transport cleanup remains incomplete.", + ], + } satisfies ElectronPageInspection; + result.error.partialObservation = createElectronEvidence( + "inspect_electron_page", + input, + inspection, + ready.value.identity(), + ); return result; } - const inspection = { - ...report.data, - limitations: [ - ...report.data.limitations, - partial.cleanup === "transport-closed" - ? "REA closed its CDP transport; this does not imply target shutdown." - : "CDP transport cleanup remains incomplete.", - ], - } satisfies ElectronPageInspection; - result.error.partialObservation = createElectronEvidence( - "inspect_electron_page", - input, - inspection, - ready.value.identity(), - ); } - return result; + return err( + keepIncompleteObservation( + result.error, + "inspect_electron_page", + "Electron provider retained a non-JSON incomplete observation.", + ), + ); } const report = electronPageInspectionSchema.safeParse(result.value); if (!report.success) diff --git a/src/application/javascript/JavaScriptRuntimeObservationService.test.ts b/src/application/javascript/JavaScriptRuntimeObservationService.test.ts index b9351db166..6ef97ff4c2 100644 --- a/src/application/javascript/JavaScriptRuntimeObservationService.test.ts +++ b/src/application/javascript/JavaScriptRuntimeObservationService.test.ts @@ -14,6 +14,35 @@ import { listJavaScriptRuntimeTargets, observeJavaScriptRuntime, } from "./JavaScriptRuntimeObservationService.js"; +import { + expectDroppedPartial, + expectKeptPartial, + expectOutputError, + expectOwnerError, + expectPreemptedCancellation, +} from "../ownerObservationExpectation.fixture.js"; + +const observeCancelled = async (partial: unknown) => { + const reason = { requested: "stop before runtime observation completed" }; + const failure = new AnalysisCancelledError("observe_javascript_runtime", { + cause: reason, + partialObservation: partial as never, + }); + const result = await observeJavaScriptRuntime( + { + identity: () => ({ id: "test", name: "Test", version: "1" }), + listTargets: async () => + err(new AnalysisCancelledError("list_javascript_runtime_targets")), + observe: async () => err(failure), + }, + { + inspector_endpoint: "http://127.0.0.1:9229", + target_id: "target", + observation_ms: 0, + }, + ); + return { result, failure, reason }; +}; it("preserves failed Inspector cleanup when cancellation occurs during observation", async () => { const controller = new AbortController(); @@ -126,11 +155,7 @@ it("does not let late cancellation hide an invalid completed Inspector report", }, { inspector_endpoint: "http://127.0.0.1:9229" }, ); - expect(output).toMatchObject({ - ok: false, - error: { _tag: "AnalysisOutputError" }, - }); - if (!output.ok) expect(output.error).toBeInstanceOf(AnalysisOutputError); + expectOutputError(output); const controller = new AbortController(); const late = await listJavaScriptRuntimeTargets( @@ -147,11 +172,7 @@ it("does not let late cancellation hide an invalid completed Inspector report", { inspector_endpoint: "http://127.0.0.1:9229" }, { signal: controller.signal }, ); - expect(late).toMatchObject({ - ok: false, - error: { _tag: "AnalysisOutputError" }, - }); - if (!late.ok) expect(late.error).toBeInstanceOf(AnalysisOutputError); + expectOutputError(late); }); it("converts a raw runtime observation retained by cleanup failure", async () => { @@ -309,11 +330,23 @@ it.each(["list", "observe"] as const)( { signal: controller.signal }, ); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.error).toBeInstanceOf(AnalysisCancelledError); - expect(result.error.cause).toBe(reason); - } - expect(providerCalls).toBe(0); + expectPreemptedCancellation(result, reason, providerCalls); + }, +); + +it("keeps cancellation when the retained runtime observation is incomplete", async () => { + const partial = { runtime: "partial", scripts: [] }; + const { result, failure, reason } = await observeCancelled(partial); + expectKeptPartial(expectOwnerError(result, failure, reason), partial); +}); + +it.each([{ scripts: [undefined] }, { elapsed_ms: Number.NaN }])( + "drops a non-JSON runtime partial and keeps cancellation primary", + async (partial) => { + const { result, failure, reason } = await observeCancelled(partial); + expectDroppedPartial( + expectOwnerError(result, failure, reason), + "Inspector retained a non-JSON incomplete observation.", + ); }, ); diff --git a/src/application/javascript/JavaScriptRuntimeObservationService.ts b/src/application/javascript/JavaScriptRuntimeObservationService.ts index 398995f139..0eedaff93b 100644 --- a/src/application/javascript/JavaScriptRuntimeObservationService.ts +++ b/src/application/javascript/JavaScriptRuntimeObservationService.ts @@ -13,7 +13,10 @@ import type { ObserveJavaScriptRuntimeInput, } from "../../domain/javascript/javascriptRuntimeObservation.js"; import { err, ok, type Result } from "../../domain/result.js"; -import { completeProviderEvidence } from "../ProviderOperationExecution.js"; +import { + completeProviderEvidence, + keepIncompleteObservation, +} from "../ProviderOperationExecution.js"; import { javascriptRuntimeObservationSchema, javascriptRuntimeTargetListSchema, @@ -76,15 +79,7 @@ export const observeJavaScriptRuntime = async ( !("kind" in partial) ) { const report = javascriptRuntimeObservationSchema.safeParse(partial); - if (!report.success && result.error instanceof AnalysisCancelledError) - return err( - new AnalysisOutputError( - "observe_javascript_runtime", - "Inspector retained a malformed completed runtime observation.", - { cause: report.error }, - ), - ); - if (report.success) + if (report.success) { result.error.partialObservation = createJavaScriptRuntimeObservationEvidence( "observe_javascript_runtime", @@ -92,8 +87,16 @@ export const observeJavaScriptRuntime = async ( report.data, ready.value.identity(), ); + return result; + } } - return result; + return err( + keepIncompleteObservation( + result.error, + "observe_javascript_runtime", + "Inspector retained a non-JSON incomplete observation.", + ), + ); } const report = javascriptRuntimeObservationSchema.safeParse(result.value); if (!report.success) diff --git a/src/application/ownerObservationExpectation.fixture.ts b/src/application/ownerObservationExpectation.fixture.ts new file mode 100644 index 0000000000..609edb0f48 --- /dev/null +++ b/src/application/ownerObservationExpectation.fixture.ts @@ -0,0 +1,72 @@ +import { expect } from "vitest"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { + AnalysisCancelledError, + AnalysisOutputError, +} from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import type { Result } from "../domain/result.js"; + +/** Assert the service returned the same owner error and its cause. */ +export const expectOwnerError = ( + result: Result, + failure: AnalysisError, + cause: unknown, +): AnalysisError => { + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected the owner failure"); + expect(result.error).toBe(failure); + expect(result.error.cause).toBe(cause); + return result.error; +}; + +/** A JSON-safe incomplete observation stays on the owner error. */ +export const expectKeptPartial = ( + error: AnalysisError, + partial: unknown, + code: "cancelled" | "provider_timeout" = "cancelled", +): void => { + expect(error.partialObservation).toEqual(partial); + expect(error.secondaryFailures).toHaveLength(0); + expect(projectAnalysisError(error)).toMatchObject({ + code, + details: { partial_observation: partial }, + }); +}; + +/** A completed value failed output validation, including after a late abort. */ +export const expectOutputError = ( + result: Result, +): void => { + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisOutputError" }, + }); + if (!result.ok) expect(result.error).toBeInstanceOf(AnalysisOutputError); +}; + +/** Cancellation won before the provider ran. */ +export const expectPreemptedCancellation = ( + result: Result, + cause: unknown, + providerCalls: number, +): void => { + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(cause); + expect(providerCalls).toBe(0); +}; + +/** A non-JSON partial is cleared and recorded as a secondary output failure. */ +export const expectDroppedPartial = ( + error: AnalysisError, + reason: string, + code: "cancelled" | "provider_timeout" = "cancelled", +): void => { + expect(error.partialObservation).toBeUndefined(); + expect(error.secondaryFailures).toMatchObject([ + { _tag: "AnalysisOutputError", reason }, + ]); + expect(projectAnalysisError(error).code).toBe(code); +}; diff --git a/src/domain/javascript/electronPageInspection.fixture.ts b/src/domain/javascript/electronPageInspection.fixture.ts new file mode 100644 index 0000000000..e5fe9d0dcb --- /dev/null +++ b/src/domain/javascript/electronPageInspection.fixture.ts @@ -0,0 +1,34 @@ +import { classifyBrowserCompleteness } from "../browserCompleteness.js"; + +/** Empty coverage tail shared by Electron page fixtures. */ +export const emptyElectronPageCoverage = (observationMs = 0) => ({ + capture_window: { + started_at: "2026-01-01T00:00:00.000Z", + ended_at: new Date( + Date.parse("2026-01-01T00:00:00.000Z") + observationMs, + ).toISOString(), + observation_ms: observationMs, + }, + completeness: classifyBrowserCompleteness({ + policyFilteredSections: new Set(), + attachLimitedSections: new Set(), + truncatedSections: new Set(), + unavailableSections: new Set(), + excluded: [], + droppedEvents: { + scripts: 0, + network_requests: 0, + console_events: 0, + websocket_connections: 0, + websocket_frames: 0, + webmcp_tools: 0, + timeline_events: 0, + }, + }), + frames: [], + dom: { total_nodes: 0, nodes: [] }, + scripts: { total: 0, items: [] }, + resources: [], + workers: [], + limitations: [], +}); diff --git a/tests/boundary/mcp/electronMcp.test.ts b/tests/boundary/mcp/electronMcp.test.ts index 9842407fa0..1d6a8fb318 100644 --- a/tests/boundary/mcp/electronMcp.test.ts +++ b/tests/boundary/mcp/electronMcp.test.ts @@ -29,7 +29,7 @@ import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js"; import { toolAvailabilityInventory } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; -import { classifyBrowserCompleteness } from "../../../src/domain/browserCompleteness.js"; +import { emptyElectronPageCoverage } from "../../../src/domain/javascript/electronPageInspection.fixture.js"; import type { ElectronPageInspection } from "../../../src/domain/javascript/electronObservation.js"; import { err } from "../../../src/domain/result.js"; @@ -115,33 +115,7 @@ it("records completed Electron facts after final detach as server Evidence", asy file_path: "/tmp/index.html", attached: true, }, - capture_window: { - started_at: "2026-01-01T00:00:00.000Z", - ended_at: "2026-01-01T00:00:00.001Z", - observation_ms: 1, - }, - completeness: classifyBrowserCompleteness({ - policyFilteredSections: new Set(), - attachLimitedSections: new Set(), - truncatedSections: new Set(), - unavailableSections: new Set(), - excluded: [], - droppedEvents: { - scripts: 0, - network_requests: 0, - console_events: 0, - websocket_connections: 0, - websocket_frames: 0, - webmcp_tools: 0, - timeline_events: 0, - }, - }), - frames: [], - dom: { total_nodes: 0, nodes: [] }, - scripts: { total: 0, items: [] }, - resources: [], - workers: [], - limitations: [], + ...emptyElectronPageCoverage(1), }; const provider: ElectronObservationPort = { identity: () => ({