From 087813fb889496bfcd1a8bc32c4ec225d22eb388 Mon Sep 17 00:00:00 2001 From: Alex Gorevski Date: Wed, 21 Oct 2026 13:15:16 -0700 Subject: [PATCH] fix(cancellation): preserve caller abort reasons through late finalization (#6143) * fix(cancellation): preserve caller abort reasons through late finalization Caller aborts that arrived during final provider, cleanup, or protocol handling were dropped or replaced by I/O, cleanup, or success results. Each affected path now samples the caller signal after its owner has settled and returns AnalysisCancelledError carrying the exact signal.reason, keeping any completed partial Evidence or cleanup state. - Web script export: keep the exact reason for an interrupted output write, and honor a cancellation fact recorded before a retried capture close succeeds. - Native value tracing and Hopper regex search: check the signal after the final provider reply or regex worker settles. - Apple asset catalog: stop assetutil with SIGTERM so cancellation settles only after the helper exits. - CDP observation: end an armed window on a fatal protocol failure. - ADB bugreport: prefer cancellation over the staging-removal output error when the caller aborted. - JavaScript recovery: check the signal after provider success. - export-web-scripts CLI: run under command cancellation and forward the signal into the export workflow. - Objdump and Rizin CLI: report cancellation after cleanup completes. - Binary session: keep cancellation primary when previous-client retirement fails after abort. Fixes morluto/rea#6042 Fixes morluto/rea#6043 Fixes morluto/rea#6048 Fixes morluto/rea#6049 Fixes morluto/rea#6057 Fixes morluto/rea#6060 Fixes morluto/rea#6078 Fixes morluto/rea#6080 Fixes morluto/rea#6108 Fixes morluto/rea#6111 Fixes morluto/rea#6115 * fix(cancellation): keep caller reasons through export rollback and commit - Rollback failure after a caller abort now returns AnalysisCancelledError with the exact signal.reason, attaching the rollback failure as cleanup and retaining any published result as partial observation. - A capture close retry that succeeds after abort returns cancellation instead of the stale cleanup diagnostic. - An abort that lands during the final output commit returns cancellation carrying the completed manifest as partial observation. - Apple assetutil stop test: abort and await the run in finally so a timed-out wait does not leave the child running, and skip on Windows where the JavaScript SIGTERM handler cannot run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b247e9-c793-4b8f-979a-807c27112cf5 * fix(cancellation): bind export cancellations to cleanup and caller reasons - A rollback-failure cancellation is bound to the retained cleanup lease, so a later successful close clears cleanup_incomplete instead of reporting a residual directory that no longer exists. - mapExportFailure wraps the caller's own AnalysisCancelledError reason under export_web_scripts rather than returning it as the export's error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b247e9-c793-4b8f-979a-807c27112cf5 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> Copilot-Session: 35b247e9-c793-4b8f-979a-807c27112cf5 --- src/adb/AdbProvider.ts | 10 +- src/application/WebScriptExportService.ts | 34 +++- src/application/binary/BinarySession.ts | 10 + .../JavaScriptRecoveryService.test.ts | 26 +++ .../javascript/JavaScriptRecoveryService.ts | 12 +- .../native/NativeValueTrace.test.ts | 19 ++ src/application/native/NativeValueTrace.ts | 6 + .../apple/AppleAssetCatalogAnalysis.test.ts | 49 ++++- .../apple/AppleAssetCatalogAnalysis.ts | 1 + src/browser/CdpObservationSession.ts | 8 + src/browser/assets/PublishWebScripts.ts | 27 ++- src/cli/reverseEngineeringCommands.ts | 27 ++- src/cli/webScriptCommands.ts | 80 ++++---- src/hopper/HopperRegexSearch.test.ts | 43 +++- src/hopper/HopperRegexSearch.ts | 4 + .../browser/cdpObservationWindowScope.test.ts | 21 ++ .../cli/reverseEngineeringCommands.test.ts | 14 ++ .../cli/webScriptSelectionCli.test.ts | 47 ++++- .../filesystem/webScriptExport.test.ts | 189 ++++++++++++++++++ .../adb/adbBugreportPublication.test.ts | 21 ++ .../binarySessionCleanupRetry.test.ts | 24 +++ 21 files changed, 611 insertions(+), 61 deletions(-) diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index 1f042d685e..fbbf43fcec 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -3216,10 +3216,12 @@ export class AdbProvider { await removeOwnedOutputDirectory(stagingDirectory, stagingIdentity); } catch (cause) { const primary = collectedResult.ok - ? new AnalysisOutputError( - operation, - "The archive was published, but its staging directory could not be removed", - ) + ? signal?.aborted === true + ? new AnalysisCancelledError(operation, { cause: signal.reason }) + : new AnalysisOutputError( + operation, + "The archive was published, but its staging directory could not be removed", + ) : collectedResult.error; if (retainedObservation !== undefined) primary.retainPartialObservation(retainedObservation); diff --git a/src/application/WebScriptExportService.ts b/src/application/WebScriptExportService.ts index c53c698021..df9201166a 100644 --- a/src/application/WebScriptExportService.ts +++ b/src/application/WebScriptExportService.ts @@ -38,6 +38,7 @@ import { NonRegularFileReadError, RegularFileChangedError, } from "../filesystem/RegularFile.js"; +import { regularFileCancellation } from "../filesystem/OwnedRegularFile.js"; const OPERATION = "export_web_scripts"; @@ -101,6 +102,13 @@ export const exportWebScriptsValidated = async ( resources, ...(options.signal === undefined ? {} : { signal: options.signal }), }); + if (options.signal?.aborted === true) + return err( + new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + partialObservation: { kind: "web-script-export", result }, + }), + ); return ok( createEvidence( { path: input.capture_path, sha256, format: "file" }, @@ -170,6 +178,8 @@ const mapExportFailure = ( signal: AbortSignal | undefined, ): AnalysisError => { if (cause instanceof WebScriptExportError) return cause; + if (cause instanceof AnalysisCancelledError && cause !== signal?.reason) + return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) return analysisErrorWithCleanupFailure( new WebScriptExportError( @@ -187,7 +197,12 @@ const mapExportFailure = ( OPERATION, ); if (signal?.aborted === true) - return new AnalysisCancelledError(OPERATION, { cause }); + return new AnalysisCancelledError(OPERATION, { + cause: + cause instanceof ArtifactReaderFailure && cause.cause === signal.reason + ? signal.reason + : cause, + }); if (cause instanceof ArtifactReaderFailure) return new WebScriptExportError( cause.reason, @@ -245,8 +260,23 @@ const readCapture = async ( incomplete: retry.cleanup !== undefined, cause: cause.cleanupCause, }; - if (retry.outcome.kind === "completed") + if (retry.outcome.kind === "completed") { + const cancellation = + signal?.aborted === true + ? { reason: signal.reason } + : regularFileCancellation(cause); + if (cancellation !== undefined) { + const primary = new AnalysisCancelledError(OPERATION, { + cause: cancellation.reason, + }); + return err( + retry.cleanup === undefined + ? primary + : incompleteCaptureCleanup(primary, path, cleanup), + ); + } return ok({ bytes: retry.outcome.value as Buffer, cleanup }); + } const primary = captureReadFailure( path, retry.outcome.cause, diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index ed8e64fbfa..3ea582b4d1 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -589,6 +589,16 @@ export class BinarySession // Preserve Evidence and Unknown history for export and recovery. this.clearSnapshot(); this.#clearRuntimeAvailability(); + if (isAborted(options.signal)) + return err( + analysisErrorWithCleanupFailure( + new AnalysisCancelledError("open_binary", { + cause: options.signal?.reason, + }), + closed.error, + "open_binary", + ), + ); return closed; } } diff --git a/src/application/javascript/JavaScriptRecoveryService.test.ts b/src/application/javascript/JavaScriptRecoveryService.test.ts index d9d1be9361..76fe66c04c 100644 --- a/src/application/javascript/JavaScriptRecoveryService.test.ts +++ b/src/application/javascript/JavaScriptRecoveryService.test.ts @@ -176,3 +176,29 @@ describe("JavaScript recovery workflow", () => { }); }); }); + +it("returns cancellation with the validated Evidence when the caller aborts after provider success", async () => { + const controller = new AbortController(); + const reason = { request: "stop recovery" }; + const service = new JavaScriptRecoveryService({ + recover: async () => { + controller.abort(reason); + return ok( + createAnalysisExecution(recoveryResult(), providerIdentity, { + subject: { + path: artifact.path, + sha256: artifact.sha256, + format: "javascript" as const, + }, + }), + ); + }, + }); + const result = await service.recover(input, { signal: controller.signal }); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(result.error.partialObservation).toMatchObject({ + operation: "recover_javascript_sources", + }); +}); diff --git a/src/application/javascript/JavaScriptRecoveryService.ts b/src/application/javascript/JavaScriptRecoveryService.ts index 4a5b282159..6eb906401d 100644 --- a/src/application/javascript/JavaScriptRecoveryService.ts +++ b/src/application/javascript/JavaScriptRecoveryService.ts @@ -13,8 +13,9 @@ import { } from "../../domain/javascript/javascriptRecovery.js"; import { jsonObjectSchema, jsonValueSchema } from "../../domain/jsonValue.js"; import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; -import { err, ok, type Result } from "../../domain/result.js"; +import { err, type Result } from "../../domain/result.js"; import type { ExecutionOptions } from "../AnalysisProvider.js"; +import { completeProviderEvidence } from "../ProviderOperationExecution.js"; import type { JavaScriptRecoveryPort } from "./JavaScriptRecoveryPort.js"; const OPERATION = "recover_javascript_sources"; @@ -107,8 +108,10 @@ export class JavaScriptRecoveryService { "Recovery source or engine identity disagrees with its Evidence binding", ), ); - return ok( - createEvidence(execution.value.subject, execution.value.provider, { + const evidence = createEvidence( + execution.value.subject, + execution.value.provider, + { operation: OPERATION, parameters: jsonObjectSchema.parse(parsed.data), result: jsonValueSchema.parse(result.data), @@ -117,7 +120,8 @@ export class JavaScriptRecoveryService { limitations: execution.value.limitations, confidence: "derived", authority: "shipped-artifact", - }), + }, ); + return completeProviderEvidence(evidence, options?.signal); } } diff --git a/src/application/native/NativeValueTrace.test.ts b/src/application/native/NativeValueTrace.test.ts index 51702723c9..6e73f761a4 100644 --- a/src/application/native/NativeValueTrace.test.ts +++ b/src/application/native/NativeValueTrace.test.ts @@ -698,6 +698,25 @@ describe("bounded native value dependency composition", () => { error: { _tag: "AnalysisOutputError" }, }); }); + + it("returns typed cancellation when the caller aborts with the final provider reply", async () => { + const controller = new AbortController(); + const reason = { request: "stop trace" }; + const result = await traceNativeValues( + { + execute: async (operation, parameters, options) => { + const value = await analysis.execute(operation, parameters, options); + if (parameters.procedure === "0x2000") controller.abort(reason); + return value; + }, + }, + { procedure: "0x1000" }, + controller.signal, + ); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + }); }); describe("native value trace selector origin", () => { diff --git a/src/application/native/NativeValueTrace.ts b/src/application/native/NativeValueTrace.ts index 54f92de248..87da312736 100644 --- a/src/application/native/NativeValueTrace.ts +++ b/src/application/native/NativeValueTrace.ts @@ -446,5 +446,11 @@ export const traceNativeValues = async ( "Graph output exceeds 33554432 bytes (32 MiB). This release has no runtime override. Lower max_nodes, max_edges, or the page limit.", ), ); + if (signal?.aborted) + return err( + new AnalysisCancelledError("trace_native_values", { + cause: signal.reason, + }), + ); return ok(result); }; diff --git a/src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts b/src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts index 1528df7fe9..b3f446ac80 100644 --- a/src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts +++ b/src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts @@ -1,10 +1,11 @@ import { createHash } from "node:crypto"; -import { mkdir, symlink, writeFile } from "node:fs/promises"; +import { access, mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; +import * as processOutput from "../../process/ExecFileOutput.js"; import { encodeNibArchiveFixture } from "./NibArchive.fixture.js"; import type { analyzeInterfaceBuilderBundle } from "./InterfaceBuilderAnalysis.js"; import { ArtifactReaderFailure } from "../ArtifactReader.js"; @@ -330,4 +331,50 @@ describe("Apple asset catalog and Interface Builder concurrency", () => { finishCatalog("[]"); await expect(analysis).rejects.toThrow("interface builder failed"); }); + + it.skipIf(process.platform === "win32")( + "settles a cancelled assetutil run only after its process exits", + async () => { + const bundle = await catalogBundle(); + const work = await createTestTempDirectory("rea-assetutil-stop-"); + const ready = join(work, "ready"); + const exited = join(work, "exited"); + const script = `const fs=require("node:fs");process.on("SIGTERM",()=>{setTimeout(()=>{fs.writeFileSync(process.argv[2],"exited");process.exit(0)},500)});fs.writeFileSync(process.argv[1],"ready");setInterval(()=>{},1000);`; + const platform = Object.getOwnPropertyDescriptor(process, "platform")!; + const realExecFileOutput = processOutput.execFileOutput; + Object.defineProperty(process, "platform", { + ...platform, + value: "darwin", + }); + const spy = vi + .spyOn(processOutput, "execFileOutput") + .mockImplementation((_command, _args, options) => + realExecFileOutput( + process.execPath, + ["-e", script, ready, exited], + options, + ), + ); + const controller = new AbortController(); + let settled: Promise | undefined; + try { + settled = analyzeAppleAssetCatalogs({ + environment: {}, + bundlePath: bundle, + targetSha256: "a".repeat(64), + signal: controller.signal, + analyzeInterfaceBuilder: async () => emptyGraph, + }).catch((cause: unknown) => cause); + await vi.waitFor(() => expect(access(ready)).resolves.toBeUndefined()); + controller.abort("stop asset inspection"); + expect(await settled).toMatchObject({ reason: "cancelled" }); + await expect(access(exited)).resolves.toBeUndefined(); + } finally { + controller.abort("stop asset inspection"); + await settled; + spy.mockRestore(); + Object.defineProperty(process, "platform", platform); + } + }, + ); }); diff --git a/src/artifacts/apple/AppleAssetCatalogAnalysis.ts b/src/artifacts/apple/AppleAssetCatalogAnalysis.ts index 202c169ea2..46e72ebd6f 100644 --- a/src/artifacts/apple/AppleAssetCatalogAnalysis.ts +++ b/src/artifacts/apple/AppleAssetCatalogAnalysis.ts @@ -274,6 +274,7 @@ const runAssetUtil = async ( env: environment, timeout: 60_000, maxBuffer: MAX_CATALOG_BYTES, + stopSignal: "SIGTERM", ...(signal === undefined ? {} : { signal }), }) ).stdout; diff --git a/src/browser/CdpObservationSession.ts b/src/browser/CdpObservationSession.ts index ccb2d6b1ad..ed74a5c7f7 100644 --- a/src/browser/CdpObservationSession.ts +++ b/src/browser/CdpObservationSession.ts @@ -123,6 +123,7 @@ export const observeCdpSession = async ( let endReason = await waitForWindow( context.input.observation_ms, capture, + context.connection, context.signal, ); if (endReason === "window_elapsed") @@ -593,18 +594,21 @@ const performanceTimestamp = (value: unknown): number => { const waitForWindow = async ( durationMs: number, capture: TimelineCapture, + connection: CdpConnection, signal?: AbortSignal, ): Promise => await new Promise((resolve, reject) => { const timerController = new AbortController(); let settled = false; let removeEnd = (): void => {}; + let removeProtocolFailure = (): void => {}; const settle = (complete: () => void): void => { if (settled) return; settled = true; timerController.abort(); signal?.removeEventListener("abort", onAbort); removeEnd(); + removeProtocolFailure(); complete(); }; const finish = (reason: EndReason): void => settle(() => resolve(reason)); @@ -619,6 +623,10 @@ const waitForWindow = async ( signal?.addEventListener("abort", onAbort, { once: true }); if (signal?.aborted === true) onAbort(); if (!settled) removeEnd = capture.onEnd(finish); + if (!settled) + removeProtocolFailure = connection.onProtocolFailure((error) => + settle(() => reject(error)), + ); if (settled) return; void delayWithCancellation( durationMs, diff --git a/src/browser/assets/PublishWebScripts.ts b/src/browser/assets/PublishWebScripts.ts index ba886f00e8..afad3965f9 100644 --- a/src/browser/assets/PublishWebScripts.ts +++ b/src/browser/assets/PublishWebScripts.ts @@ -8,6 +8,7 @@ import { DESTINATION_CASE_COLLISION_PREFIX } from "../../artifacts/ArtifactPaths import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; import { SafeOutputTreeCreationFailure } from "../../artifacts/SafeOutputTreeCreationFailure.js"; import { PLAYWRIGHT_RESPONSE_BODY_PROVENANCE_LIMITATION } from "../../domain/browserNetworkEvidence.js"; +import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; import { webScriptExportManifestSchema, webScriptExportResultSchema, @@ -30,6 +31,8 @@ const LIMITATIONS = [ "Scenario response bytes preserve the output of Playwright's Response.body() API; any source-body provenance caveat from the capture is retained in this manifest. Debugger sources are the exposed text encoded as UTF-8. Redacted bytes are exported exactly as retained, and may no longer parse as JavaScript.", ]; +const OPERATION = "export_web_scripts"; + interface PublishWebScriptsOptions { readonly resources: ArtifactResourceScope; readonly signal?: AbortSignal; @@ -128,19 +131,33 @@ export const publishWebScripts = async ( const diagnostic = tree.published ? `Publication completed, but descriptor cleanup failed (${message(cleanupCause)}). The published output remains available.` : `Publication failed (${message(cause)}); rollback failed (${message(cleanupCause)}). Remove the residual output directory before retrying.`; + const partialObservation = + tree.published && result !== undefined + ? ({ kind: "web-script-export", result } as const) + : undefined; + if (options.signal?.aborted === true) { + const cancelled = new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + cleanup: { reason: diagnostic, resources: [tree.outputRoot] }, + ...(partialObservation === undefined + ? {} + : { partialObservation }), + }); + cleanup.lease.bindCleanupError(cancelled); + throw cancelled; + } const error = new WebScriptExportError( cause instanceof ArtifactReaderFailure ? cause.reason : "io", tree.outputRoot, diagnostic, { residualPaths: [tree.outputRoot] }, ); - if (tree.published && result !== undefined) - error.retainPartialObservation({ - kind: "web-script-export", - result, - }); + if (partialObservation !== undefined) + error.retainPartialObservation(partialObservation); + cleanup.lease.bindCleanupError(error); throw error; } + if (tree.published && result !== undefined) return result; } throw cause; diff --git a/src/cli/reverseEngineeringCommands.ts b/src/cli/reverseEngineeringCommands.ts index 561e801caa..d1086c40e1 100644 --- a/src/cli/reverseEngineeringCommands.ts +++ b/src/cli/reverseEngineeringCommands.ts @@ -4,6 +4,7 @@ import type { Logger } from "pino"; import { createReverseEngineeringService } from "../composition/reverseEngineering.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { + AnalysisCancelledError, AnalysisInputError, AnalysisOutputError, } from "../domain/analysisErrorCore.js"; @@ -33,6 +34,7 @@ import { const executeWithCleanup = async ( service: ReverseEngineeringService, + signal: AbortSignal, execute: () => Promise>, ): Promise> => { let result: Result; @@ -66,11 +68,26 @@ const executeWithCleanup = async ( } if (!result.ok && hadIncompleteCleanup && !result.error.cleanupIncomplete) { const evidence = cleanupOnlyCompleteEvidence(result.error); - if (evidence !== undefined) return { ok: true, value: evidence }; + if (evidence !== undefined) + return cancelledAfterCleanup({ ok: true, value: evidence }, signal); } - return result; + return cancelledAfterCleanup(result, signal); }; +/** Late cancellation observed during finalization keeps the completed Evidence as partial data. */ +const cancelledAfterCleanup = ( + result: Result, + signal: AbortSignal, +): Result => + result.ok && signal.aborted + ? err( + new AnalysisCancelledError(result.value.operation, { + cause: signal.reason, + partialObservation: result.value, + }), + ) + : result; + const cleanupOnlyCompleteEvidence = ( error: AnalysisError, ): Evidence | undefined => { @@ -152,7 +169,7 @@ export const registerReverseEngineeringCommands = ( }; const inputError = cliEvidenceDeliveryInputError(delivery); if (inputError !== undefined) return projectAnalysisError(inputError); - const result = await executeWithCleanup(service, () => + const result = await executeWithCleanup(service, signal, () => service.inspectWithObjdump( { ...args, @@ -196,7 +213,7 @@ export const registerReverseEngineeringCommands = ( const inputError = cliEvidenceDeliveryInputError(delivery); if (inputError !== undefined) return projectAnalysisError(inputError); - const result = await executeWithCleanup(service, () => + const result = await executeWithCleanup(service, signal, () => service.inspectWithObjdump( { operation: "architectures", @@ -229,7 +246,7 @@ export const registerReverseEngineeringCommands = ( run: ({ args, options }) => withCommandCancellation((signal) => logCliCommand(logger, CLI_COMMANDS.executeRizinCommand, async () => { - const result = await executeWithCleanup(service, () => + const result = await executeWithCleanup(service, signal, () => service.executeRizinCommand( { ...args, diff --git a/src/cli/webScriptCommands.ts b/src/cli/webScriptCommands.ts index 476789ae9a..9655f74cf8 100644 --- a/src/cli/webScriptCommands.ts +++ b/src/cli/webScriptCommands.ts @@ -9,6 +9,7 @@ import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup. import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { Logger } from "pino"; +import { withCommandCancellation } from "./commandCancellation.js"; /** Register the local captured-script export CLI workflow. */ export const registerWebScriptCommands = ( @@ -29,46 +30,49 @@ export const registerWebScriptCommands = ( ), }), run: ({ args }) => - logCliCommand(logger, CLI_COMMANDS.exportWebScripts, async () => { - const resources = new ArtifactResourceScope(); - const result = await exportWebScripts( - { - capture_path: args.capturePath, - output_directory: args.outputDirectory, - }, - resources, - ); - try { - await resources.close(); - } catch (cause: unknown) { - const cleanup = ArtifactReaderFailure.cleanupObservation( - cause, - "Web script export resources", + withCommandCancellation((signal) => + logCliCommand(logger, CLI_COMMANDS.exportWebScripts, async () => { + const resources = new ArtifactResourceScope(); + const result = await exportWebScripts( + { + capture_path: args.capturePath, + output_directory: args.outputDirectory, + }, + resources, + { signal }, ); - const failure = new ProviderCleanupError( - "web-script-export", - cleanup.resources, - { reason: cleanup.reason }, - { cause, operation: "export_web_scripts" }, - ); - const error = result.ok - ? failure - : analysisErrorWithCleanupFailure( - result.error, - failure, - "export_web_scripts", - ); - return { - error: "Script export failed", - ...projectAnalysisError(error), - }; - } - return result.ok - ? result.value - : { + try { + await resources.close(); + } catch (cause: unknown) { + const cleanup = ArtifactReaderFailure.cleanupObservation( + cause, + "Web script export resources", + ); + const failure = new ProviderCleanupError( + "web-script-export", + cleanup.resources, + { reason: cleanup.reason }, + { cause, operation: "export_web_scripts" }, + ); + const error = result.ok + ? failure + : analysisErrorWithCleanupFailure( + result.error, + failure, + "export_web_scripts", + ); + return { error: "Script export failed", - ...projectAnalysisError(result.error), + ...projectAnalysisError(error), }; - }), + } + return result.ok + ? result.value + : { + error: "Script export failed", + ...projectAnalysisError(result.error), + }; + }), + ), }); }; diff --git a/src/hopper/HopperRegexSearch.test.ts b/src/hopper/HopperRegexSearch.test.ts index a4d07c1eca..2a3b4c6ad6 100644 --- a/src/hopper/HopperRegexSearch.test.ts +++ b/src/hopper/HopperRegexSearch.test.ts @@ -1,4 +1,4 @@ -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; import { HopperRegexSearch } from "./HopperRegexSearch.js"; import { err, ok } from "../domain/result.js"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; @@ -9,6 +9,24 @@ import { } from "../domain/hopperErrors.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +const regexMatch = vi.hoisted(() => ({ + afterSettle: undefined as (() => void) | undefined, +})); +vi.mock("../process/RegexMatcher.js", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + matchRegexValues: async ( + request: Parameters[0], + ) => { + const result = await actual.matchRegexValues(request); + regexMatch.afterSettle?.(); + return result; + }, + }; +}); + it.each( (["search_strings", "search_procedures"] as const).flatMap((operation) => (["remote", "capability", "timeout", "process"] as const).map((family) => ({ @@ -196,3 +214,26 @@ it("preserves an inventory failure when cancellation arrives after the physical await search.close(); } }); + +it("returns cancellation when the caller aborts after the regex worker settles", async () => { + const controller = new AbortController(); + const reason = { request: "stop search" }; + regexMatch.afterSettle = () => controller.abort(reason); + const search = new HopperRegexSearch({ + callTool: async () => + ok([{ address: "0x1000", value: "fixture" }] as never), + }); + try { + const result = await search.execute( + "search_strings", + { pattern: "fixture", mode: "regex", case_sensitive: false }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + } finally { + regexMatch.afterSettle = undefined; + await search.close(); + } +}); diff --git a/src/hopper/HopperRegexSearch.ts b/src/hopper/HopperRegexSearch.ts index c9e867416f..fa55c79b4c 100644 --- a/src/hopper/HopperRegexSearch.ts +++ b/src/hopper/HopperRegexSearch.ts @@ -136,6 +136,10 @@ export class HopperRegexSearch { }), }); if (!matched.ok) return matched; + if (options.signal?.aborted) + return err( + new AnalysisCancelledError(operation, { cause: options.signal.reason }), + ); const selected = new Set(matched.value); return ok(parsed.data.filter((_, index) => selected.has(index))); } diff --git a/tests/boundary/browser/cdpObservationWindowScope.test.ts b/tests/boundary/browser/cdpObservationWindowScope.test.ts index 8331adc17b..c07188c7a6 100644 --- a/tests/boundary/browser/cdpObservationWindowScope.test.ts +++ b/tests/boundary/browser/cdpObservationWindowScope.test.ts @@ -374,3 +374,24 @@ it("represents absent timeline timestamps as unknown and preserves an observed z ]), ); }); + +it("ends an armed observation at a fatal protocol failure without waiting out its window", async () => { + const browser = await startFakeCdpBrowser({ + malformedEventOnMethod: "Page.setLifecycleEventsEnabled", + }); + trackBrowser(browser); + const started = performance.now(); + const result = await new CdpBrowserProvider().observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 10_000, + }), + ); + expect(performance.now() - started).toBeLessThan(5_000); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "BrowserObservationError", reason: "protocol_error" }, + }); +}, 15_000); diff --git a/tests/boundary/cli/reverseEngineeringCommands.test.ts b/tests/boundary/cli/reverseEngineeringCommands.test.ts index c6af908a37..aa4448fe6b 100644 --- a/tests/boundary/cli/reverseEngineeringCommands.test.ts +++ b/tests/boundary/cli/reverseEngineeringCommands.test.ts @@ -369,6 +369,20 @@ it("does not project cancellation cleanup as incomplete after a successful retry }); }); +it("returns cancellation with the completed observation when the caller interrupts service close", async () => { + const service = { + inspectWithObjdump: vi.fn(async () => ok(observation("complete"))), + close: vi.fn(async () => { + process.emit("SIGINT"); + }), + }; + + const result = await runObjdump(service); + + expect(result).toMatchObject({ code: "cancelled" }); + expect(process.exitCode).toBe(130); +}); + it("parses the Rizin inferior TTY as an option and passes it to the manager", async () => { const commands = new Map< string, diff --git a/tests/boundary/cli/webScriptSelectionCli.test.ts b/tests/boundary/cli/webScriptSelectionCli.test.ts index 416c81ac1e..e2d60e093d 100644 --- a/tests/boundary/cli/webScriptSelectionCli.test.ts +++ b/tests/boundary/cli/webScriptSelectionCli.test.ts @@ -1,11 +1,56 @@ import { access } from "node:fs/promises"; import { join } from "node:path"; -import { expect } from "vitest"; +import pino from "pino"; +import { expect, it, vi } from "vitest"; +import { registerWebScriptCommands } from "../../../src/cli/webScriptCommands.js"; +import * as webScriptExport from "../../../src/application/WebScriptExportService.js"; +import { CLI_COMMANDS } from "../../../src/cliCommandNames.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; +import { err } from "../../../src/domain/result.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; +it("forwards an interrupt into the export workflow and reports cancellation", async () => { + const commands = new Map }>(); + const cli = { + command: ( + name: string, + definition: { run(input: unknown): Promise }, + ) => { + commands.set(name, definition); + return cli; + }, + } as unknown as Parameters[0]; + registerWebScriptCommands(cli, pino({ enabled: false })); + const exported = vi + .spyOn(webScriptExport, "exportWebScripts") + .mockImplementation(async (_input, _resources, options) => { + process.emit("SIGINT"); + return err( + new AnalysisCancelledError("export_web_scripts", { + cause: options?.signal?.reason, + }), + ); + }); + const previousExitCode = process.exitCode; + try { + const output = await commands.get(CLI_COMMANDS.exportWebScripts)?.run({ + args: { + capturePath: "/unused/capture.json", + outputDirectory: "/unused/out", + }, + }); + expect(exported.mock.calls[0]?.[2]?.signal?.aborted).toBe(true); + expect(output).toMatchObject({ code: "cancelled" }); + expect(process.exitCode).toBe(130); + } finally { + exported.mockRestore(); + process.exitCode = previousExitCode; + } +}); + cliTest.skipIf(process.platform === "win32")( "exits with a capture_path invalid_request for a named pipe before publishing output", async ({ cli, processes }) => { diff --git a/tests/boundary/filesystem/webScriptExport.test.ts b/tests/boundary/filesystem/webScriptExport.test.ts index 998bbccf2d..dc39a5672f 100644 --- a/tests/boundary/filesystem/webScriptExport.test.ts +++ b/tests/boundary/filesystem/webScriptExport.test.ts @@ -25,6 +25,7 @@ import { import { analyzeJavaScriptApplication } from "../../support/javascriptApplicationScope.js"; import { exportWebScripts } from "../../../src/application/WebScriptExportService.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { publishWebScripts } from "../../../src/browser/assets/PublishWebScripts.js"; import { SafeOutputTree } from "../../../src/artifacts/SafeOutputTree.js"; @@ -468,6 +469,194 @@ describe("captured script publication failures and cleanup", () => { expect(await readdir(root)).toEqual(["capture.json"]); }); + it("keeps the exact caller reason when cancellation interrupts the first output write", async () => { + const { input, resources } = await setup( + scriptScenarioFixture([ + { + url: "https://fixture.test/first.js", + bytes: Buffer.from("export const a = 1;"), + }, + { + url: "https://fixture.test/second.js", + bytes: Buffer.from("export const b = 2;"), + }, + ]), + ); + const controller = new AbortController(); + const reason = { request: "stop export" }; + const create = SafeOutputTree.create.bind(SafeOutputTree); + const createSpy = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (...args) => { + const tree = await create(...args); + const write = tree.write.bind(tree); + vi.spyOn(tree, "write").mockImplementationOnce(async (...writeArgs) => { + controller.abort(reason); + return write(...writeArgs); + }); + return tree; + }); + try { + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(resources.hasPending).toBe(false); + } finally { + createSpy.mockRestore(); + } + }); +}); + +describe("captured script cancellation finalization", () => { + it("keeps the caller reason when rollback also fails after an interrupted write", async () => { + const { input, resources } = await setup( + scriptScenarioFixture([ + { + url: "https://fixture.test/first.js", + bytes: Buffer.from("export const a = 1;"), + }, + ]), + ); + const controller = new AbortController(); + const reason = { request: "stop export" }; + const create = SafeOutputTree.create.bind(SafeOutputTree); + const createSpy = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (...args) => { + const tree = await create(...args); + vi.spyOn(tree, "write").mockImplementationOnce(async () => { + controller.abort(reason); + throw new Error("injected write failure"); + }); + vi.spyOn(tree, "rollback").mockRejectedValueOnce( + new Error("injected rollback failure"), + ); + return tree; + }); + try { + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(result.error.cleanupIncomplete).toBe(true); + } finally { + createSpy.mockRestore(); + await resources.close(); + } + }); + + it("wraps a caller-owned cancellation reason under the export operation", async () => { + const { input, resources } = await setup( + scriptScenarioFixture([ + { + url: "https://fixture.test/first.js", + bytes: Buffer.from("export const a = 1;"), + }, + ]), + ); + const controller = new AbortController(); + const reason = new AnalysisCancelledError("upstream_workflow", { + cause: { request: "stop upstream" }, + }); + const create = SafeOutputTree.create.bind(SafeOutputTree); + const createSpy = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (...args) => { + const tree = await create(...args); + const write = tree.write.bind(tree); + vi.spyOn(tree, "write").mockImplementationOnce(async (...writeArgs) => { + controller.abort(reason); + return write(...writeArgs); + }); + return tree; + }); + try { + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error).not.toBe(reason); + if (!(result.error instanceof AnalysisCancelledError)) + throw new Error("Expected AnalysisCancelledError"); + expect(result.error.operation).toBe("export_web_scripts"); + expect(result.error.cause).toBe(reason); + } finally { + createSpy.mockRestore(); + await resources.close(); + } + }); +}); + +describe("captured script final commit cancellation", () => { + it("reports cancellation with the completed output when abort lands during final commit", async () => { + const { input, resources } = await setup(); + const controller = new AbortController(); + const reason = { request: "stop export" }; + const create = SafeOutputTree.create.bind(SafeOutputTree); + const createSpy = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (...args) => { + const tree = await create(...args); + const commit = tree.commit.bind(tree); + vi.spyOn(tree, "commit").mockImplementation(async (...commitArgs) => { + await commit(...commitArgs); + controller.abort(reason); + }); + return tree; + }); + try { + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(result.error.partialObservation).toMatchObject({ + kind: "web-script-export", + result: { + output_directory: input.output_directory, + manifest: { path: join(input.output_directory, "manifest.json") }, + }, + }); + } finally { + createSpy.mockRestore(); + } + }); + + it("returns cancellation with the caller reason when a retried capture close recovers", async () => { + const { input, resources } = await setup(); + const controller = new AbortController(); + const reason = { request: "stop export" }; + let closeCalls = 0; + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if (String(args[0]) !== input.capture_path) return handle; + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeCalls += 1; + if (closeCalls === 1) { + controller.abort(reason); + throw new Error("injected capture close failure"); + } + await close(); + }); + return handle; + }); + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancellation"); + expect(closeCalls).toBe(2); + expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(result.error.cause).toBe(reason); + expect(resources.hasPending).toBe(false); + }); + it("identifies a missing capture file and rejected host path syntax", async () => { const { input, resources } = await setup(); const missing = await exportWebScripts( diff --git a/tests/boundary/providers/adb/adbBugreportPublication.test.ts b/tests/boundary/providers/adb/adbBugreportPublication.test.ts index cef6cc25f1..4abb1be624 100644 --- a/tests/boundary/providers/adb/adbBugreportPublication.test.ts +++ b/tests/boundary/providers/adb/adbBugreportPublication.test.ts @@ -382,3 +382,24 @@ test("preserves the primary protocol failure when staging cleanup also fails", a await f.provider.close(); expect(await readdir(f.root)).toEqual([]); }); + +test("reports caller cancellation when staging removal fails after the archive is published", async () => { + const controller = new AbortController(); + const reason = { request: "stop bugreport" }; + const f = await fixture(); + vi.mocked(rm).mockImplementationOnce(async () => { + controller.abort(reason); + throw new Error("cleanup denied"); + }); + const result = await f.provider.execute(f.request, { + signal: controller.signal, + }); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect(result.error.cleanupPrimary?.cause).toBe(reason); + await f.provider.close(); + expect(projectAnalysisError(result.error).code).toBe("cancelled"); +}); diff --git a/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts b/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts index ce52603da1..519a8f7e43 100644 --- a/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts +++ b/tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts @@ -17,6 +17,7 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; const retryFixture = async ( execute: AnalysisClient["execute"] = () => Promise.resolve(ok(null)), + onClose?: () => void, ) => { const parent = await createTestTempDirectory("rea-session-cleanup-retry-"); const runtime = join(parent, "owned-runtime"); @@ -42,6 +43,7 @@ const retryFixture = async ( requests: [], }), close: async (options) => { + onClose?.(); if (!allowed) return err(cleanupError); await rm(runtime, { recursive: true, force: true }); if (options?.retainDocument !== true) await rm(document, { force: true }); @@ -221,6 +223,28 @@ it("retains cancelled startup cleanup and the cancellation reason", async () => }); }); +it("keeps cancellation as the primary when previous-client retirement fails after abort", async () => { + const controller = new AbortController(); + const reason = { phase: "binary-retirement" }; + const fixture = await retryFixture(undefined, () => controller.abort(reason)); + expect((await fixture.session.open(fixture.targets[0])).ok).toBe(true); + const result = await fixture.session.open(fixture.targets[1], { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected cancelled retirement"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect(result.error.cleanupPrimary?.cause).toBe(reason); + expect(await fixture.session.status()).toMatchObject({ + open: false, + cleanup_pending: true, + }); + fixture.allowCleanup(); + expect((await fixture.session.close()).ok).toBe(true); +}); + it("preserves caller-selected document retention when retrying cleanup", async () => { const fixture = await retryFixture(); expect((await fixture.session.open(fixture.targets[0])).ok).toBe(true);