From 26520bd29095bd3a57b4919aeba41392d673d4ff Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 17:44:13 +0800 Subject: [PATCH] refactor(providers): finalize apktool and shared artifact resources through runOwned (#10653) * refactor(providers): finalize apktool and shared artifact resources through runOwned runOwned moves to the provider layer so provider sites can use it without depending on application workflows. It gains a completed-value projection (partial) for values that are not themselves partial observations, an optional signal, and cleanupLeases that bind a close failure to the exact leases whose confirmed release resolves it. ArtifactAnalysisResources.finish is removed; TShark and sigrok finalize through runOwned with the shared release as close. Apktool's hand-written release, cancellation and lease-group composition is replaced by runOwned. A failed primary now keeps a cancellation observed during final cleanup as a secondary failure, as the documented precedence requires. A counted architecture guard refuses new hand-written completed-result abort reclassification outside runOwned. * refactor(adb): finalize runtime-root work through runOwned (#10660) * refactor(adb): finalize runtime-root work through runOwned The ADB runtime-root finalizer re-derived the precedence by hand and kept its own cancellation-with-cleanup combiner. It now runs through runOwned: the work body maps snapshot failures, close releases the root lease and binds its own cleanup error, and partial projects the completed execution. A cancellation whose cleanup fails is now the shared cleanup composite with the cancellation as its primary, as runOwned produces for every other owner. * refactor(providers): keep completed values as diagnostics in runOwned and migrate four finalizers (#10691) * refactor(providers): keep completed values as diagnostics in runOwned and migrate signature, Ghidra, bundle and LLDB finalization * test(providers): run the lifecycle scenarios over every runOwned adopter shape (#10707) One table of testing.md lifecycle rows runs over partial, diagnostics, publishing and lease adopters, replacing the hand-written precedence cases it covers. * chore(lint): move runOwned's classifier baseline entry with the file --- docs/development/testing.md | 6 + docs/development/tool-design.md | 2 +- .../provider-failure-classifier-baseline.json | 2 +- .../verify/architecture-guard-baselines.json | 34 +- scripts/verify/architecture-guards.mjs | 10 + src/adb/AdbProvider.ts | 215 ++++------ src/apktool/ApktoolProvider.ts | 60 ++- src/application/EvidenceBundleCommands.ts | 99 +++-- src/captures/TsharkCaptureProvider.ts | 27 +- src/cli/finishAnalysisCommand.ts | 3 +- src/ghidra/GhidraProvider.ts | 27 +- src/native/LldbCallTracer.ts | 210 +++++----- src/native/SignatureTargetBinding.test.ts | 7 + src/native/SignatureTargetBinding.ts | 213 +++++----- src/process/ArtifactAnalysisResources.test.ts | 48 ++- src/process/ArtifactAnalysisResources.ts | 54 --- src/providers/runOwned.lifecycle.test.ts | 373 ++++++++++++++++++ .../runOwned.test.ts | 185 +++------ src/{application => providers}/runOwned.ts | 116 +++++- src/signals/SigrokRecordingProvider.ts | 27 +- .../adb/adbInputSnapshotOwnership.test.ts | 13 +- .../adbScreenPublicationCancellation.test.ts | 25 +- .../bundleComparisonFailures.test.ts | 19 + 23 files changed, 1066 insertions(+), 709 deletions(-) create mode 100644 src/providers/runOwned.lifecycle.test.ts rename src/{application => providers}/runOwned.test.ts (54%) rename src/{application => providers}/runOwned.ts (70%) diff --git a/docs/development/testing.md b/docs/development/testing.md index 73f72ce106..ed776b2423 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -786,6 +786,12 @@ controlled production seam for interleavings that cannot be reproduced reliably. | Cleanup owners recover after an error was returned | Retain the original child and aggregate errors and project them after each owner releases. A successful peer stays released; the aggregate remains incomplete until all owners release. Preserve the first cause even when rejection is `undefined`. | | Target exits without an acknowledgement, or transport disappears while it stays alive | Positive owned exit proof resolves target lifetime; other resources still need cleanup. Lost transport alone does not prove exit. Verify unrelated/attached targets survive and same-connection recovery works when supported. | +Owners that finalize through `runOwned` inherit the precedence rows above from +`src/providers/runOwned.lifecycle.test.ts`, which runs each scenario over a +table of adopter shapes (partial or diagnostics record, publication, lease). +Add a shape to that table instead of repeating those cases per provider; a +provider's own tests cover how its work and close reach `runOwned`. + Shared process-supervision controls separate leader exit from output closure. Cover output during held or failed cleanup, bounded retention and drain timeout, retry, and borrowed stream authority. The POSIX inherited-stderr control uses a diff --git a/docs/development/tool-design.md b/docs/development/tool-design.md index fb9a0673d5..7731a7b2ed 100644 --- a/docs/development/tool-design.md +++ b/docs/development/tool-design.md @@ -482,7 +482,7 @@ failed request's physical, cancellation and cleanup facts. confirmed release. CLI adapters should reuse [`finishAnalysisCommand`](https://github.com/morluto/rea/blob/main/src/cli/finishAnalysisCommand.ts) where its contract applies rather than duplicate close/finally handling. It projects - [`runOwned`](https://github.com/morluto/rea/blob/main/src/application/runOwned.ts), + [`runOwned`](https://github.com/morluto/rea/blob/main/src/providers/runOwned.ts), which owns this precedence for owned work: a recorded failure stays primary, cancellation before commit keeps the completed value as a partial observation, cancellation after commit keeps the value, and unconfirmed diff --git a/scripts/lint/rules/provider-failure-classifier-baseline.json b/scripts/lint/rules/provider-failure-classifier-baseline.json index 836027cc18..22299cd33e 100644 --- a/scripts/lint/rules/provider-failure-classifier-baseline.json +++ b/scripts/lint/rules/provider-failure-classifier-baseline.json @@ -20,7 +20,6 @@ "src/application/managed/ManagedMemberComparisonService.ts": 1, "src/application/ntfs/NtfsJournalService.ts": 1, "src/application/process/ProcessCli.ts": 1, - "src/application/runOwned.ts": 1, "src/artifacts/ArtifactOperationFailure.ts": 1, "src/artifacts/ReleaseExposureReader.ts": 1, "src/artifacts/StableArtifactFailure.ts": 2, @@ -75,6 +74,7 @@ "src/process/privateWorkspaceFailure.ts": 1, "src/providers/AnalysisClientFailure.ts": 1, "src/providers/OwnedCommandCleanup.ts": 2, + "src/providers/runOwned.ts": 1, "src/rust/RustBinaryProvider.ts": 1, "src/signals/SigrokRecordingProvider.ts": 1, "src/sqlite/SqliteDatabaseFailures.ts": 1, diff --git a/scripts/verify/architecture-guard-baselines.json b/scripts/verify/architecture-guard-baselines.json index 6d3dbfcb60..1cbf2d6e86 100644 --- a/scripts/verify/architecture-guard-baselines.json +++ b/scripts/verify/architecture-guard-baselines.json @@ -11,7 +11,7 @@ "src/application/Doctor.ts": 4, "src/application/EnhancedLiteralTracing.ts": 1, "src/application/EnhancedTools.ts": 2, - "src/application/EvidenceBundleCommands.ts": 3, + "src/application/EvidenceBundleCommands.ts": 1, "src/application/EvidenceBundleFiles.ts": 3, "src/application/EvidenceInputResolver.ts": 2, "src/application/JsonFiles.ts": 8, @@ -121,7 +121,7 @@ "src/javascript/sourceMaps/SourceMapCommand.ts": 4, "src/javascript/sourceMaps/SourceMapDecoder.ts": 3, "src/native/AppleDispatchMetadata.ts": 3, - "src/native/LldbCallTracer.ts": 4, + "src/native/LldbCallTracer.ts": 2, "src/native/NativeCallObservation.ts": 2, "src/native/NativeCallTarget.ts": 2, "src/native/NativeCommandAnalysisError.ts": 1, @@ -129,7 +129,6 @@ "src/native/NativePlistSource.ts": 2, "src/native/NativeUiInterruptionControl.ts": 1, "src/native/NativeUiObservation.ts": 1, - "src/native/SignatureTargetBinding.ts": 5, "src/native/pe/PeBinaryFailure.ts": 1, "src/native/pwndbg/PwndbgCoreContext.ts": 3, "src/native/pwntools/PwntoolsDecoder.ts": 8, @@ -381,7 +380,7 @@ "src/android/JadxJavaProbe.ts": 1, "src/android/JadxProvider.ts": 2, "src/apktool/ApktoolCommandCleanup.ts": 1, - "src/apktool/ApktoolProvider.ts": 3, + "src/apktool/ApktoolProvider.ts": 2, "src/application/AnalysisContextQueries.ts": 1, "src/application/DirectAnalysisCleanup.ts": 2, "src/application/Doctor.ts": 1, @@ -414,7 +413,6 @@ "src/application/native/NativeUiActionTrace.ts": 1, "src/application/process/ProcessCli.ts": 1, "src/application/releaseExposure/ReleaseExposureService.ts": 1, - "src/application/runOwned.ts": 1, "src/artifacts/apple/AppleAssetCatalogAnalysis.ts": 2, "src/artifacts/apple/KeyedArchiveReader.ts": 1, "src/browser/BrowserScenarioPartialObservation.ts": 1, @@ -471,7 +469,7 @@ "src/kuna/KunaProjectReader.ts": 2, "src/kuna/KunaSessionClient.ts": 1, "src/kuna/KunaTargetCommands.ts": 2, - "src/native/LldbCallTracer.ts": 6, + "src/native/LldbCallTracer.ts": 4, "src/native/NativeCommandAnalysisError.ts": 1, "src/native/NativeMacOSProvider.ts": 5, "src/native/NativeUiObservation.ts": 1, @@ -480,7 +478,7 @@ "src/native/pwntools/PwntoolsDecoder.ts": 6, "src/pcap/PcapProvider.ts": 7, "src/pcap/PcapSnapshot.ts": 1, - "src/process/ArtifactAnalysisResources.ts": 4, + "src/process/ArtifactAnalysisResources.ts": 3, "src/process/DarwinProcessRunTokenReader.ts": 2, "src/process/capture/FilesystemSnapshot.ts": 1, "src/process/capture/ProcessCaptureError.ts": 3, @@ -489,6 +487,7 @@ "src/providers/AnalysisClientCreation.ts": 3, "src/providers/AnalysisClientFailure.ts": 1, "src/providers/OwnedCommandCleanup.ts": 1, + "src/providers/runOwned.ts": 1, "src/rizin/RizinDebugSessionManager.ts": 1, "src/server/registerCloseLifecycleTool.ts": 1, "src/server/registerResetInvestigationTool.ts": 1, @@ -645,5 +644,24 @@ "src/native/MachoArchitectureSelection.ts": 2, "src/native/NativeMachoInspection.ts": 1 }, - "abort-without-reason": {} + "abort-without-reason": {}, + "completed-abort-reclassification": { + "src/application/EvidenceInputResolver.ts": 1, + "src/application/binary/BinarySession.ts": 1, + "src/application/desktop/DesktopApplicationState.ts": 1, + "src/application/frida/FridaInstrumentationService.ts": 1, + "src/browser/CdpBrowserProvider.ts": 1, + "src/browser/CdpElectronProvider.ts": 1, + "src/browser/PlaywrightBrowserScenarioProvider.ts": 1, + "src/browser/execution/CdpWebRuntimeProvider.ts": 1, + "src/browser/history/HistoricalCaptureDecoder.ts": 1, + "src/cliJsonInput.ts": 1, + "src/evm/EvmoleInterfaceProvider.ts": 1, + "src/ghidra/GhidraClient.ts": 1, + "src/hopper/HopperClient.ts": 2, + "src/hopper/HopperMachOImage.ts": 1, + "src/native/pwntools/PwntoolsDecoder.ts": 1, + "src/reference/ReferenceSourceReaderEntries.ts": 1, + "src/sqlite/SqliteDatabaseProvider.ts": 1 + } } diff --git a/scripts/verify/architecture-guards.mjs b/scripts/verify/architecture-guards.mjs index b450dff5ba..f1ce0540cb 100644 --- a/scripts/verify/architecture-guards.mjs +++ b/scripts/verify/architecture-guards.mjs @@ -330,6 +330,16 @@ checkCounted( ["src/pcap/TsharkJsonPackets.ts"], ); +// Turning a completed result into a cancellation after cleanup is the +// finalization precedence that runOwned owns (src/providers/runOwned.ts). +checkCounted( + "completed result reclassified by a hand-read abort signal (finalize through runOwned in src/providers/runOwned)", + "completed-abort-reclassification", + files, + /\.ok\s*&&\s*(?:isAborted\(|[\w.?]*signal\??\.aborted)/gu, + ["src/providers/runOwned.ts"], +); + // An abort reason names who ended the work: a typed `CallerAbort` at the CLI, // MCP and HTTP adapters, or an `OwnerCancellation` when REA closes an owner. // A bare `.abort()` leaves an anonymous platform AbortError as the cause. diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index 4404b34bec..57bc04e99f 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -74,6 +74,7 @@ import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup. import { analysisErrorWithCancellation } from "../domain/analysisErrorCancellation.js"; import { readPngHeaderDimensions } from "../domain/pngHeader.js"; import { err, ok, type Result } from "../domain/result.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { jsonObjectSchema } from "../domain/jsonValue.js"; import { @@ -616,35 +617,6 @@ const publishScreenExclusively = async ( throw writeFailure.cause; }; -const cancellationWithCleanupFailure = ( - primary: AnalysisCancelledError, - cleanup: AnalysisError, - resources: readonly string[], -): AnalysisCancelledError => - cancelledBy( - primary.operation, - carriedReason( - new AggregateError( - [primary, cleanup], - "ADB cancellation cleanup failed", - { cause: primary }, - ), - ), - { - diagnostics: primary.diagnostics, - ...(primary.capturedOutput === undefined - ? {} - : { capturedOutput: primary.capturedOutput }), - cleanup: { - reason: [primary.cleanup?.reason, cleanup.message] - .filter((reason): reason is string => reason !== undefined) - .join("; "), - resources: [...new Set([...primary.cleanupResources, ...resources])], - }, - partialObservation: primary.partialObservation, - }, - ); - /** Publish an ADB-produced archive without replacing an existing caller file. */ const publishBugreport = async ( source: string, @@ -1059,124 +1031,81 @@ export class AdbProvider { ); } - let result: Result | undefined; - let primary: AnalysisError | undefined; const children = new ArtifactResourceScope(); - try { - result = await work(root, children); - } catch (cause: unknown) { - if (cause instanceof AdbInputSnapshotFailure) { - primary = - cause.primaryCause instanceof AnalysisError - ? cause.primaryCause - : new AnalysisOutputError( - operation, - `The caller input could not be staged for adb: ${cause.sourcePath}`, - { cause: cause.primaryCause ?? cause }, - ); - // Snapshot handle cleanup may recover when the runtime-root lease is - // drained below. Keep that historical failure linked to the typed - // primary error while the lease owns and clears live cleanup state. - primary.retainSecondaryFailure( - new AnalysisOutputError( - operation, - `ADB input snapshot cleanup previously failed for ${cause.sourcePath}`, - { cause }, - ), - {}, - ); - } else { - primary = - cause instanceof AnalysisError - ? cause - : new AnalysisProtocolError(`ADB ${operation} failed`, { cause }); - } - } - const rootLease = runtimeRootLease(root, children); - const released = await this.#resources.release({ - kind: "lease", - resource: root.path, - lease: rootLease, - }); - if (signal?.aborted === true) { - const failure = - primary ?? - (result !== undefined && !result.ok ? result.error : undefined); - if (failure !== undefined) - primary = analysisErrorWithCancellation( - failure, - cancelledBy(operation, signal), - ); - } - if (primary === undefined && result?.ok && signal?.aborted === true) { - const completed = result.value; - primary = cancelledBy(operation, signal, { - partialObservation: createEvidence( - completed.subject ?? undefined, - completed.provider, - { - operation, - parameters, - result: completed.result, - rawResult: completed.rawResult, - limitations: completed.limitations, - locations: completed.locations, - confidence: "observed", + return finalizedResult( + await runOwned( + async () => { + try { + return await work(root, children); + } catch (cause: unknown) { + if (!(cause instanceof AdbInputSnapshotFailure)) + return err( + cause instanceof AnalysisError + ? cause + : new AnalysisProtocolError(`ADB ${operation} failed`, { + cause, + }), + ); + const primary = + cause.primaryCause instanceof AnalysisError + ? cause.primaryCause + : new AnalysisOutputError( + operation, + `The caller input could not be staged for adb: ${cause.sourcePath}`, + { cause: cause.primaryCause ?? cause }, + ); + // Snapshot handle cleanup may recover when the runtime-root lease + // is drained during close. Keep that historical failure linked to + // the typed primary error while the lease owns live cleanup state. + return err( + primary.retainSecondaryFailure( + new AnalysisOutputError( + operation, + `ADB input snapshot cleanup previously failed for ${cause.sourcePath}`, + { cause }, + ), + {}, + ), + ); + } + }, + { + providerId: ADB_PROVIDER_IDENTITY.id, + operation, + signal, + close: async () => { + const released = await this.#resources.release({ + kind: "lease", + resource: root.path, + lease: rootLease, + }); + if (released.kind === "released") return; + const cleanup = this.#cleanupError( + operation, + parameters, + [released.cause], + [ + root.path, + ...(released.cause instanceof ArtifactReaderFailure && + released.cause.cleanup !== undefined + ? released.cause.cleanup.resources + : []), + ], + ); + rootLease.bindCleanupError(cleanup); + throw cleanup; }, - ), - }); - } - if (released.kind === "failed") { - const cleanupResources = [ - root.path, - ...(released.cause instanceof ArtifactReaderFailure && - released.cause.cleanup !== undefined - ? released.cause.cleanup.resources - : []), - ]; - const cleanup = this.#cleanupError( - operation, - parameters, - [released.cause], - cleanupResources, - result?.ok ? result.value : undefined, - ); - const returnCleanupError = (error: AnalysisError) => { - rootLease.bindCleanupError(error); - return err(error); - }; - if (primary instanceof AnalysisCancelledError) { - return returnCleanupError( - cancellationWithCleanupFailure(primary, cleanup, cleanupResources), - ); - } - if (primary !== undefined) - return returnCleanupError( - analysisErrorWithCleanupFailure(primary, cleanup, operation), - ); - if (result !== undefined && !result.ok) { - if (result.error instanceof AnalysisCancelledError) { - return returnCleanupError( - cancellationWithCleanupFailure( - result.error, - cleanup, - cleanupResources, + partial: (completed) => + createAnalysisExecutionEvidence( + operation, + parameters, + completed, + "observed", ), - ); - } - return returnCleanupError( - analysisErrorWithCleanupFailure(result.error, cleanup, operation), - ); - } - return returnCleanupError(cleanup); - } - if (primary !== undefined) return err(primary); - if (result === undefined) - return err( - new AnalysisProtocolError(`ADB ${operation} returned no result`), - ); - return result; + }, + ), + ); } #retainOutputDirectoryCleanup( diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index 03c6e6c65c..de09c98862 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -58,6 +58,7 @@ import { import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js"; import { err, ok, type Result } from "../domain/result.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { @@ -779,44 +780,31 @@ export class ApktoolProvider { }); result = err(retained); } - const released = await this.#artifacts.release(owner); - const observation = - result.ok && (released.kind === "failed" || signal?.aborted === true) - ? createAnalysisExecutionEvidence( + const completed = result; + return finalizedResult( + await runOwned(async () => completed, { + providerId: "apktool", + operation, + signal, + close: async () => { + const released = await this.#artifacts.release(owner); + if (released.kind === "failed") + throw new ProviderCleanupError( + "apktool", + [root], + {}, + { operation, cause: released.cause }, + ); + }, + cleanupLeases: [lease], + partial: (execution) => + createAnalysisExecutionEvidence( operation, input, - result.value, + execution, "observed", - ) - : undefined; - if (result.ok && signal?.aborted) - result = err( - cancelledBy(operation, signal, { partialObservation: observation }), - ); - if (released.kind === "failed") { - const cleanup = new ProviderCleanupError( - "apktool", - [root], - {}, - { - operation, - cause: released.cause, - ...(observation === undefined - ? {} - : { partialObservation: observation }), - }, - ); - new AnalysisCleanupLeaseGroup(cleanup, [lease]); - return err( - result.ok - ? cleanup - : analysisErrorWithCleanupFailure( - result.error, - cleanup, - operation, - ).bindCleanupDependencies([result.error, cleanup]), - ); - } - return result; + ), + }), + ); } } diff --git a/src/application/EvidenceBundleCommands.ts b/src/application/EvidenceBundleCommands.ts index 02b2ea2e81..d2a060f89e 100644 --- a/src/application/EvidenceBundleCommands.ts +++ b/src/application/EvidenceBundleCommands.ts @@ -39,6 +39,11 @@ import { import { inspectEvidenceBundle } from "./investigation/InspectEvidenceBundle.js"; import type { EvidenceRecordFilters } from "../domain/evidenceRecordSummary.js"; import { inspectAnalysisViewValidated } from "./analysisView/AnalysisViewService.js"; +import { + finalizedResult, + runOwned, + type RunOwnedOptions, +} from "../providers/runOwned.js"; /** Validate a standalone bundle and report its complete record/revision counts. */ export const importEvidenceBundleCommand = async ( @@ -85,21 +90,25 @@ export const exportEvidenceBundleCommand = async ( signal, ); if (!loaded.ok) return loaded; - const written = await persistEvidenceBundleCommand( - loaded.value, - outputPath, - overwrite, - signal, - "export_evidence_bundle", + const written = await finalizeBundleWork( + () => + persistEvidenceBundleCommand( + loaded.value, + outputPath, + overwrite, + signal, + "export_evidence_bundle", + ), + { + providerId: "rea", + operation: "export_evidence_bundle", + signal, + // Deliver interrupts queued during publication cleanup before deciding + // whether a written bundle survives cancellation. + close: dispatchQueuedInterrupts, + diagnostics: (receipt) => ({ evidence_bundle: { ...receipt } }), + }, ); - await dispatchQueuedInterrupts(); - if (written.ok && signal?.aborted) - return err( - new AnalysisCancelledError("export_evidence_bundle", { - cause: signal.reason, - diagnostics: { evidence_bundle: { ...written.value } }, - }), - ); return projectWrite(loaded.value, written); }; @@ -346,21 +355,49 @@ const withBundleReadResources = async ( signal?: AbortSignal, ): Promise> => { const resources = new ArtifactResourceScope(); - let result: Result; - try { - result = await read(resources); - } finally { - // Read errors own their retained leases and observe resolution or remaining - // failure in place. A retry must not replace the original read failure. - await resources.close().catch(() => undefined); - await dispatchQueuedInterrupts(); - } - return result.ok && signal?.aborted - ? err( - new AnalysisCancelledError(operation, { - cause: signal.reason, - diagnostics: { completed_result: result.value }, - }), - ) - : result; + let readFailed = false; + return finalizeBundleWork(() => read(resources), { + providerId: "rea", + operation, + signal, + inspectPrimary: (primary) => { + readFailed = !primary.ok; + }, + close: async () => { + try { + await resources.close(); + } catch (cause: unknown) { + // A failed read already owns its retained leases and observes their + // resolution in place; a retry must not replace that failure. Only + // a completed read leaves a close failure without another owner. + if (!readFailed) throw cause; + } finally { + await dispatchQueuedInterrupts(); + } + }, + fallbackResource: `${operation} bundle read resources`, + diagnostics: (value) => ({ completed_result: value }), + }); +}; + +/** + * Finalize bundle work through runOwned. An unexpected throw is a defect the + * adapters report as such, so it is rethrown after cleanup instead of being + * typed as a provider failure. + */ +const finalizeBundleWork = async ( + work: () => Promise>, + options: RunOwnedOptions, +): Promise> => { + let defect: { readonly cause: unknown } | undefined; + const finalized = await runOwned(async () => { + try { + return await work(); + } catch (cause: unknown) { + defect = { cause }; + throw cause; + } + }, options); + if (defect !== undefined) throw defect.cause; + return finalizedResult(finalized); }; diff --git a/src/captures/TsharkCaptureProvider.ts b/src/captures/TsharkCaptureProvider.ts index b9e19dd089..7604768bb6 100644 --- a/src/captures/TsharkCaptureProvider.ts +++ b/src/captures/TsharkCaptureProvider.ts @@ -64,6 +64,7 @@ import { packetCaptureSchema, } from "../domain/captures/packetCapture.js"; import { err, ok, type Result } from "../domain/result.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import { runOwnedCommand, OwnedCommandFailure, @@ -477,17 +478,21 @@ export class TsharkCaptureProvider implements PacketCapturePort { ), ); } - return this.#resources.finish( - result, - (execution) => - createAnalysisExecutionEvidence( - operation, - input, - execution, - "observed", - ), - operation, - options?.signal, + const completed = result; + return finalizedResult( + await runOwned(async () => completed, { + providerId: this.#resources.providerId, + operation, + signal: options?.signal, + close: () => this.#resources.release(), + partial: (execution) => + createAnalysisExecutionEvidence( + operation, + input, + execution, + "observed", + ), + }), ); } #failure( diff --git a/src/cli/finishAnalysisCommand.ts b/src/cli/finishAnalysisCommand.ts index 9dbf71c68d..faa9a37f7d 100644 --- a/src/cli/finishAnalysisCommand.ts +++ b/src/cli/finishAnalysisCommand.ts @@ -2,7 +2,7 @@ import { finalizedResult, runOwned, type RecoveredCompletion, -} from "../application/runOwned.js"; +} from "../providers/runOwned.js"; import type { AnalysisError, AnalysisPartialObservation, @@ -53,6 +53,7 @@ export const finishAnalysisCommand = async < operation: canonicalOperation, signal, ...(fallbackResource === undefined ? {} : { fallbackResource }), + partial: (value) => value, close: async () => { try { await service.close(); diff --git a/src/ghidra/GhidraProvider.ts b/src/ghidra/GhidraProvider.ts index d4e26b4192..d37f2a77aa 100644 --- a/src/ghidra/GhidraProvider.ts +++ b/src/ghidra/GhidraProvider.ts @@ -18,6 +18,7 @@ import { resolveGhidraProject } from "./GhidraProjectSnapshot.js"; import { DeferredAnalysisClient } from "../providers/DeferredAnalysisClient.js"; import type { ProviderInspectionScope } from "../application/ProviderInspectionScope.js"; import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import { SharedStartupAttempt } from "../process/SharedStartupAttempt.js"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; @@ -383,27 +384,31 @@ export class GhidraProvider implements AnalysisProviderCandidate { ): Promise> { try { return await this.#runInspectionWork(async (signal) => { - const result = await this.#resolveProfile(target, { + const resolved = await this.#resolveProfile(target, { ...options, signal, }); - if (!result.ok && result.error instanceof AnalysisCancelledError) + if (!resolved.ok && resolved.error instanceof AnalysisCancelledError) return err( analysisClientFailureForOperation( - result.error, + resolved.error, "ghidra", "open_binary", ), ); - if (result.ok && signal.aborted) - return err( - cancelledBy("open_binary", signal, { - diagnostics: { - completed_profile_digest: result.value.profile?.digest ?? null, - }, + // Resolution owns no resource of its own; runOwned still decides + // whether a completed profile survives a cancellation. + return finalizedResult( + await runOwned(async () => resolved, { + providerId: "ghidra", + operation: "open_binary", + signal, + close: async () => undefined, + diagnostics: (resolution) => ({ + completed_profile_digest: resolution.profile?.digest ?? null, }), - ); - return result; + }), + ); }, options?.signal); } catch (cause: unknown) { return err( diff --git a/src/native/LldbCallTracer.ts b/src/native/LldbCallTracer.ts index af7dbefb51..308635c17c 100644 --- a/src/native/LldbCallTracer.ts +++ b/src/native/LldbCallTracer.ts @@ -63,6 +63,7 @@ import { LldbReadbackFiles } from "./LldbReadbackFiles.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { resolvedAnalysisError } from "../domain/analysisErrorBase.js"; +import { runOwned } from "../providers/runOwned.js"; import { readLldbObservationJournal, readBoundedPrefix, @@ -270,128 +271,105 @@ export class LldbCallTracer implements NativeCallTracer { processStopped: true, targetStopped: true, }; - let result: Result; - try { - result = await this.#run(lldb.value, owner, request, signal); - } catch (cause: unknown) { - const partial = await partialNativeObservation({ - request, - files: owner.files, - pidPath: paths.pid, - stdoutPath: paths.stdoutCapture, - stderrPath: paths.stderrCapture, - observationPath: paths.observations, - version: null, - reason: "tracer-failure", - }); - if ( - !owner.processStopped || - !owner.targetStopped || - owner.files.hasPending - ) - this.#pendingCleanup = owner; - const failure = new ProviderAdapterError(PROVIDER, OPERATION, { - cause, - diagnostics: { reason: errorReason(cause) }, - partialObservation: partial.observation, - ...(this.#pendingCleanup === owner - ? { - cleanup: { - reason: "LLDB trace failed while owned resources remain", - resources: lldbCleanupResources(owner), - }, - } - : {}), - }); - result = err(retainPartialReadFailures(failure, partial.readFailures)); - } - if (result.ok && signal?.aborted) - result = err( - new AnalysisCancelledError(OPERATION, { - cause: signal.reason, - partialObservation: partialObservationFromTrace( + let primaryFailure: AnalysisError | undefined; + const finalized = await runOwned( + async (): Promise> => { + try { + return await this.#run(lldb.value, owner, request, signal); + } catch (cause: unknown) { + const partial = await partialNativeObservation({ request, - result.value, - "cancelled", - ), - }), - ); - if (owner.files.hasPending) this.#pendingCleanup = owner; - if (this.#pendingCleanup === owner) { - if (!result.ok && result.error.cleanupIncomplete) { - if (owner.files.hasPending) { - const files = lldbCleanupFailure( - owner, - "LLDB readback descriptor cleanup remains incomplete", - owner.files.cleanupCause, - ); - result.error.retainSecondaryFailure(files, { - readback_cleanup: projectAnalysisError(files), + files: owner.files, + pidPath: paths.pid, + stdoutPath: paths.stdoutCapture, + stderrPath: paths.stderrCapture, + observationPath: paths.observations, + version: null, + reason: "tracer-failure", }); + if ( + !owner.processStopped || + !owner.targetStopped || + owner.files.hasPending + ) + this.#pendingCleanup = owner; + const failure = new ProviderAdapterError(PROVIDER, OPERATION, { + cause, + diagnostics: { reason: errorReason(cause) }, + partialObservation: partial.observation, + ...(this.#pendingCleanup === owner + ? { + cleanup: { + reason: "LLDB trace failed while owned resources remain", + resources: lldbCleanupResources(owner), + }, + } + : {}), + }); + return err(retainPartialReadFailures(failure, partial.readFailures)); } - return err(bindLldbCleanup(result.error, owner)); - } - const cleanup = lldbCleanupFailure( - owner, - "LLDB readback descriptor cleanup remains incomplete", - owner.files.cleanupCause, - result.ok - ? partialObservationFromTrace(request, result.value) - : undefined, - ); - return result.ok - ? err(cleanup) - : err( - analysisErrorWithCleanupFailure( - result.error, - cleanup, - OPERATION, - ).bindCleanupDependencies([result.error, cleanup]), - ); - } - let rootCleanup: Result; - try { - await releaseLldbRuntime(owner); - rootCleanup = ok(undefined); - } catch (cause: unknown) { - rootCleanup = err(cause); - this.#pendingCleanup = owner; - } - if (result.ok && signal?.aborted) - result = err( - new AnalysisCancelledError(OPERATION, { - cause: signal.reason, - partialObservation: partialObservationFromTrace( + }, + { + providerId: PROVIDER, + operation: OPERATION, + signal, + inspectPrimary: (primary) => { + if (!primary.ok) primaryFailure = primary.error; + }, + close: async () => { + if (owner.files.hasPending) this.#pendingCleanup = owner; + if (this.#pendingCleanup === owner) { + // A primary that already reports this cleanup keeps owning it; + // its unreleased readback files are a secondary failure. + if (primaryFailure?.cleanupIncomplete === true) { + if (owner.files.hasPending) { + const files = lldbCleanupFailure( + owner, + "LLDB readback descriptor cleanup remains incomplete", + owner.files.cleanupCause, + ); + primaryFailure.retainSecondaryFailure(files, { + readback_cleanup: projectAnalysisError(files), + }); + } + bindLldbCleanup(primaryFailure, owner); + return; + } + // The runtime root stays retained while its process or files do. + throw lldbCleanupFailure( + owner, + "LLDB readback descriptor cleanup remains incomplete", + owner.files.cleanupCause, + ); + } + try { + await releaseLldbRuntime(owner); + } catch (cause: unknown) { + this.#pendingCleanup = owner; + if (primaryFailure?.cleanupIncomplete === true) + bindLldbCleanup(primaryFailure, owner); + throw lldbCleanupFailure(owner, errorReason(cause), cause); + } + }, + // A cancellation keeps the trace as cancelled; a retained cleanup + // keeps it as the completed trace it was. + partial: (trace, replacedBy) => + partialObservationFromTrace( request, - result.value, - "cancelled", + trace, + replacedBy === "cancellation" ? "cancelled" : "cleanup-failure", ), - }), + }, + ); + if (finalized.kind === "completed") return ok(finalized.value); + if ( + finalized.kind !== "completed-cleanup-retained" && + finalized.cleanup.kind === "released" + ) + return err( + resolvedAnalysisError(bindLldbCleanup(finalized.error, owner)), ); - if (!rootCleanup.ok) { - const cleanup = lldbCleanupFailure( - owner, - errorReason(rootCleanup.error), - rootCleanup.error, - result.ok - ? partialObservationFromTrace(request, result.value) - : undefined, - ); - if (!result.ok && result.error.cleanupIncomplete) - bindLldbCleanup(result.error, owner); - return result.ok - ? err(cleanup) - : err( - analysisErrorWithCleanupFailure( - result.error, - cleanup, - OPERATION, - ).bindCleanupDependencies([result.error, cleanup]), - ); - } - return result.ok - ? result - : err(resolvedAnalysisError(bindLldbCleanup(result.error, owner))); + return err(finalized.error); } /** Retry retained LLDB cleanup without abandoning its process or runtime owner. */ diff --git a/src/native/SignatureTargetBinding.test.ts b/src/native/SignatureTargetBinding.test.ts index 1a9958db02..32f6e8ee8a 100644 --- a/src/native/SignatureTargetBinding.test.ts +++ b/src/native/SignatureTargetBinding.test.ts @@ -698,6 +698,13 @@ for (const ownerIndex of [0, 1]) expect(resolvedAnalysisError(result.error)).toMatchObject({ _tag: "AnalysisCancelledError", cause: reason, + // The completed binding is not an observation; it stays as diagnostics. + diagnostics: { + snapshot_binding: { + identity: expect.any(String), + snapshot_path: expect.stringContaining("target.bin"), + }, + }, }); }); diff --git a/src/native/SignatureTargetBinding.ts b/src/native/SignatureTargetBinding.ts index f23710b1eb..a63e419df1 100644 --- a/src/native/SignatureTargetBinding.ts +++ b/src/native/SignatureTargetBinding.ts @@ -13,8 +13,10 @@ import { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisAccessDeniedError, AnalysisArtifactChangedError, - AnalysisCancelledError, AnalysisOutputError, + cancelledBy, + carriedReason, + isAborted, AnalysisResourceConstraintError, } from "../domain/analysisErrorCore.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; @@ -31,6 +33,7 @@ import { AnalysisCleanupLeaseGroup, } from "../artifacts/AnalysisCleanupLease.js"; import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import type { ArtifactResourceOwner } from "../artifacts/ArtifactResourceScope.js"; import { NATIVE_MACOS_PROVIDER_IDENTITY } from "./NativeMacosProviderIdentity.js"; @@ -62,9 +65,7 @@ const signatureReadFailure = ( cause: unknown, ): AnalysisError => { if (cause instanceof RegularFileOperationCancelled) - return new AnalysisCancelledError("inspect_signature", { - cause: cause.reason, - }); + return cancelledBy("inspect_signature", carriedReason(cause.reason)); if (cause instanceof AnalysisError) return cause; const code = cause instanceof Error && "code" in cause ? cause.code : undefined; @@ -120,12 +121,10 @@ export const withSignatureTarget = async ( ? signatureReadFailure(target, cause) : snapshotFailure(target, "create", cause); return err( - signal?.aborted + isAborted(signal) ? analysisErrorWithCancellation( cause === signal.reason ? undefined : failure, - new AnalysisCancelledError("inspect_signature", { - cause: signal.reason, - }), + cancelledBy("inspect_signature", signal), ) : failure, ); @@ -146,63 +145,62 @@ export const withSignatureTarget = async ( ), resource: root.path, }; - let result: Result; - try { - const binding = await bindSignatureTarget( - target, - join(root.path, basename(target.path)), - children, - signal, - ); - if (!binding.ok) result = binding; - else { + let completed: InspectSignature | undefined; + const finalized = await runOwned( + async (): Promise> => { try { - result = await inspect(binding.value); - } catch (cause: unknown) { - result = err( - new AnalysisOutputError( - "inspect_signature", - "Native output parsing failed", - { cause }, - ), + const binding = await bindSignatureTarget( + target, + join(root.path, basename(target.path)), + children, + signal, ); + if (!binding.ok) return binding; + try { + return await inspect(binding.value); + } catch (cause: unknown) { + return err( + new AnalysisOutputError( + "inspect_signature", + "Native output parsing failed", + { cause }, + ), + ); + } + } catch (cause: unknown) { + return err(signatureReadFailure(target, cause)); } - } - } catch (cause: unknown) { - result = err(signatureReadFailure(target, cause)); - } - const cleanup = await resources.release(owner); - if (signal?.aborted) - result = err( - analysisErrorWithCancellation( - result.ok ? undefined : result.error, - new AnalysisCancelledError("inspect_signature", { - cause: signal.reason, - ...(result.ok ? { partialObservation: result.value } : {}), - }), - ), - ); - if (cleanup.kind === "failed") { - const failure = signatureCleanupFailure( - target, - [root.path], - cleanup.cause, - "cleanup", - result.ok ? result.value : undefined, - ); - new AnalysisCleanupLeaseGroup(failure, [owner.lease]); - return combineCleanupFailure(result, failure); - } - return result; + }, + { + providerId: NATIVE_MACOS_PROVIDER_IDENTITY.id, + operation: "inspect_signature", + signal, + inspectPrimary: (primary) => { + if (primary.ok) completed = primary.value; + }, + close: async () => { + const cleanup = await resources.release(owner); + if (cleanup.kind === "failed") + throw signatureCleanupFailure( + target, + [root.path], + cleanup.cause, + "cleanup", + completed, + ); + }, + cleanupLeases: [owner.lease], + partial: (inspected) => inspected, + }, + ); + return finalizedResult(finalized); }); } catch (cause: unknown) { const primary = signatureReadFailure(target, cause); - const outcome = signal?.aborted + const outcome = isAborted(signal) ? analysisErrorWithCancellation( cause === signal.reason ? undefined : primary, - new AnalysisCancelledError("inspect_signature", { - cause: signal.reason, - }), + cancelledBy("inspect_signature", signal), ) : primary; const retained = @@ -217,27 +215,18 @@ export const withSignatureTarget = async ( "retry", ); bindArtifactFileCleanup(cause, failure); - return combineCleanupFailure(err(outcome), failure); + return err( + analysisErrorWithCleanupFailure( + outcome, + failure, + "inspect_signature", + ).bindCleanupDependencies([outcome, failure]), + ); } return err(outcome); } }; -/** Each composed error follows the exact primary and secondary cleanup owners. */ -const combineCleanupFailure = ( - primary: Result, - cleanup: AnalysisError, -): Result => - primary.ok - ? err(cleanup) - : err( - analysisErrorWithCleanupFailure( - primary.error, - cleanup, - "inspect_signature", - ).bindCleanupDependencies([primary.error, cleanup]), - ); - const signatureCleanupFailure = ( target: BinaryTarget, resources: readonly string[], @@ -434,47 +423,47 @@ const bindSignatureTarget = async ( return err(signatureReadFailure(target, cause)); } }; - let result = await perform(); - const cleanupFailures: { - readonly owner: SignatureOwner; - readonly cause: unknown; - }[] = []; - for (const owner of [snapshotOwner, sourceOwner]) { - if (owner === undefined) continue; - const cleanup = await resources.release(owner); - if (cleanup.kind === "failed") - cleanupFailures.push({ owner, cause: cleanup.cause }); - } - if (result.ok && signal?.aborted) - result = err( - new AnalysisCancelledError("inspect_signature", { - cause: signal.reason, - diagnostics: { - snapshot_binding: { - identity: result.value.identity, - snapshot_path: result.value.snapshotPath, - }, + return finalizedResult( + await runOwned(perform, { + providerId: NATIVE_MACOS_PROVIDER_IDENTITY.id, + operation: "inspect_signature", + signal, + close: async () => { + const failed: SignatureOwner[] = []; + const causes: unknown[] = []; + for (const owner of [snapshotOwner, sourceOwner]) { + if (owner === undefined) continue; + const cleanup = await resources.release(owner); + if (cleanup.kind === "failed") { + failed.push(owner); + causes.push(cleanup.cause); + } + } + if (failed.length === 0) return; + const cleanup = signatureCleanupFailure( + target, + failed.map((owner) => owner.resource), + new AggregateError( + causes, + "Signature snapshot file handles could not be closed", + ), + "close", + ); + // Only the leases that failed hold resources; they alone resolve it. + new AnalysisCleanupLeaseGroup( + cleanup, + failed.map((owner) => owner.lease), + ); + throw cleanup; + }, + diagnostics: (binding) => ({ + snapshot_binding: { + identity: binding.identity, + snapshot_path: binding.snapshotPath, }, }), - ); - if (cleanupFailures.length > 0) { - const cause = new AggregateError( - cleanupFailures.map(({ cause: failure }) => failure), - "Signature snapshot file handles could not be closed", - ); - const cleanup = signatureCleanupFailure( - target, - cleanupFailures.map(({ owner }) => owner.resource), - cause, - "close", - ); - new AnalysisCleanupLeaseGroup( - cleanup, - cleanupFailures.map(({ owner }) => owner.lease), - ); - result = combineCleanupFailure(result, cleanup); - } - return result; + }), + ); }; /** Reject target drift across signature command captures. */ diff --git a/src/process/ArtifactAnalysisResources.test.ts b/src/process/ArtifactAnalysisResources.test.ts index 8d029f3e59..ce5b42d7fa 100644 --- a/src/process/ArtifactAnalysisResources.test.ts +++ b/src/process/ArtifactAnalysisResources.test.ts @@ -27,6 +27,30 @@ import { err, ok } from "../domain/result.js"; import { ArtifactAnalysisResources } from "./ArtifactAnalysisResources.js"; import { readLinuxResourceLimits } from "./LinuxResourceLimits.js"; import { ProviderProcessSupervisor } from "./ProviderProcess.js"; +import type { + AnalysisError, + AnalysisPartialObservation, +} from "../domain/analysisErrorBase.js"; +import type { Result } from "../domain/result.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; + +/** Providers finalize these resources through runOwned with their release as the close. */ +const finish = async ( + resources: ArtifactAnalysisResources, + result: Result, + partial: (value: T) => AnalysisPartialObservation, + operation = resources.operation, + signal?: AbortSignal, +): Promise> => + finalizedResult( + await runOwned(async () => result, { + providerId: resources.providerId, + operation, + signal, + close: () => resources.release(), + partial, + }), + ); it.runIf(process.platform !== "win32")( "offers a resolved-file retry from the shared selected-artifact boundary", @@ -155,7 +179,8 @@ it("projects the specific reason and member evidence for retained cleanup", asyn ); resources.retain({ path: "runtime-root", close: async () => {} }, owner); - const result = await resources.finish( + const result = await finish( + resources, err(new ProviderAdapterError("fixture-provider", "fixture-operation")), () => { throw new Error("unexpected success observation"); @@ -237,7 +262,7 @@ it("preserves the primary error and resolves cleanup status after retry", async }, ); - const result = await resources.finish(err(primary), () => { + const result = await finish(resources, err(primary), () => { throw new Error("unexpected success observation"); }); @@ -277,7 +302,8 @@ it("resolves each returned cleanup error with its own retained root", async () = if (closesA === 1) throw new Error("root A close failure"); }, }); - const first = await resources.finish( + const first = await finish( + resources, err(new AnalysisOutputError("fixture-operation", "primary failure")), () => { throw new Error("unexpected success observation"); @@ -302,7 +328,7 @@ it("resolves each returned cleanup error with its own retained root", async () = confidence: "observed", }, ); - const second = await resources.finish(ok("completed"), () => partial); + const second = await finish(resources, ok("completed"), () => partial); if (second.ok) throw new Error("expected root B cleanup failure"); expect(second.error.partialObservation).toEqual(partial); @@ -428,7 +454,7 @@ it("keeps a runtime root until its descriptor cleanup owner is released", async }, }); - const outer = await resources.finish(first, () => { + const outer = await finish(resources, first, () => { throw new Error("unexpected success observation"); }); @@ -807,7 +833,7 @@ it("preserves typed caller cancellation through failed workspace cleanup and ret }, }); try { - const result = await resources.finish(err(primary), () => { + const result = await finish(resources, err(primary), () => { throw new Error("Must not produce success Evidence"); }); if (result.ok) throw new Error("Expected retained workspace"); @@ -931,7 +957,8 @@ it.each([false, true])( }, }); try { - const result = await resources.finish( + const result = await finish( + resources, err(primary), () => { throw new Error("Failed work must not produce success Evidence"); @@ -943,6 +970,13 @@ it.each([false, true])( expect(failsClose ? result.error.cleanupPrimary : result.error).toBe( primary, ); + // The recorded failure stays primary and keeps the late cancellation. + expect(primary.secondaryFailures).toEqual([ + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: { request: "late cancellation" }, + }), + ]); denied = false; await resources.close(); expect(result.error.cleanupIncomplete).toBe(false); diff --git a/src/process/ArtifactAnalysisResources.ts b/src/process/ArtifactAnalysisResources.ts index 0b29ab2994..c448d6dbbc 100644 --- a/src/process/ArtifactAnalysisResources.ts +++ b/src/process/ArtifactAnalysisResources.ts @@ -14,7 +14,6 @@ import { stableArtifactInputRecovery } from "../artifacts/StableArtifactFailure. import { AnalysisError, type AnalysisCapturedOutput, - type AnalysisPartialObservation, } from "../domain/analysisErrorBase.js"; import { AnalysisAccessDeniedError, @@ -351,12 +350,6 @@ export class ArtifactAnalysisResources { : ownedCommandResources(ownership, root.path); } - get #pendingResources(): readonly string[] { - return [...this.#pending.keys()].flatMap((root) => - this.#resourcesFor(root), - ); - } - async release(): Promise { const failures: ProviderCleanupError[] = []; for (const [root, retained] of this.#pending) { @@ -567,53 +560,6 @@ export class ArtifactAnalysisResources { }); } - async finish( - result: Result, - observation: (value: T) => AnalysisPartialObservation, - operation = this.operation, - signal?: AbortSignal, - ): Promise> { - try { - await this.release(); - if (result.ok && signal?.aborted) - return err( - cancelledBy(operation, signal, { - partialObservation: observation(result.value), - }), - ); - return result; - } catch (cause: unknown) { - const partialObservation = result.ok - ? observation(result.value) - : result.error.partialObservation; - const primary = !result.ok - ? result.error - : signal?.aborted - ? cancelledBy(operation, signal, { partialObservation }) - : undefined; - const cleanup = - cause instanceof AnalysisError - ? cause - : new ProviderCleanupError( - this.providerId, - this.#pendingResources, - { reason: String(cause) }, - { operation, cause }, - ); - const failure = - primary === undefined - ? cleanup - : analysisErrorWithCleanupFailure( - primary, - cleanup, - operation, - ).bindCleanupDependencies([primary, cleanup]); - if (partialObservation !== undefined) - failure.retainPartialObservation(partialObservation); - return err(failure); - } - } - close(): Promise { this.#closed = true; const work = this.#tail.then(() => this.#closeResources()); diff --git a/src/providers/runOwned.lifecycle.test.ts b/src/providers/runOwned.lifecycle.test.ts new file mode 100644 index 0000000000..e64db54bdc --- /dev/null +++ b/src/providers/runOwned.lifecycle.test.ts @@ -0,0 +1,373 @@ +import { describe, expect, it } from "vitest"; + +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { + primaryAnalysisError, + type AnalysisError, +} from "../domain/analysisErrorBase.js"; +import { + AnalysisCancelledError, + AnalysisProtocolError, +} from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { err, ok, type Result } from "../domain/result.js"; +import { + finalizedResult, + runOwned, + type CompletedValueRecord, + type CompletedValueReplacement, + type Finalized, +} from "./runOwned.js"; + +/** + * The lifecycle scenarios of docs/development/testing.md#lifecycle-regressions + * that `runOwned` owns, run over every adopter shape below. A provider that + * finalizes through `runOwned` inherits these semantics; its own tests cover + * only how its work and close reach this primitive. + */ + +interface Completed { + readonly count: number; +} + +const completed: Completed = { count: 3 }; +const reason = { caller: "stop" }; + +/** How an adopter declares the value an error must keep, and how it publishes. */ +interface AdopterFixture { + readonly name: string; + readonly record: CompletedValueRecord; + /** Asserts that an error replacing the completed value kept it. */ + readonly kept: ( + error: AnalysisError, + replacedBy: CompletedValueReplacement, + ) => void; + readonly publishes: boolean; + /** Close releases an exact lease instead of throwing a typed producer error. */ + readonly lease: boolean; +} + +const partialRecord: CompletedValueRecord = { + partial: (value, replacedBy) => ({ + kind: "retained-evidence", + evidence_id: `counted-${String(value.count)}-${replacedBy}`, + }), +}; +const keptPartial = ( + error: AnalysisError, + replacedBy: CompletedValueReplacement, +) => + expect(error.partialObservation).toEqual({ + kind: "retained-evidence", + evidence_id: `counted-3-${replacedBy}`, + }); +const diagnosticsRecord: CompletedValueRecord = { + diagnostics: (value, replacedBy) => ({ + completed_count: value.count, + replaced_by: replacedBy, + }), +}; +const keptDiagnostics = ( + error: AnalysisError, + replacedBy: CompletedValueReplacement, +) => { + expect(error.partialObservation).toBeUndefined(); + expect(error.diagnostics).toMatchObject({ + completed_count: 3, + replaced_by: replacedBy, + }); +}; + +const adopters: readonly AdopterFixture[] = [ + { + // apktool, ADB, tshark and sigrok one-shot readers. + name: "one-shot reader keeping a partial observation through a lease", + record: partialRecord, + kept: keptPartial, + publishes: false, + lease: true, + }, + { + // Ghidra profile, signature binding and LLDB call tracing. + name: "one-shot reader keeping a non-observation value as diagnostics", + record: diagnosticsRecord, + kept: keptDiagnostics, + publishes: false, + lease: false, + }, + { + name: "publishing writer keeping a partial observation", + record: partialRecord, + kept: keptPartial, + publishes: true, + lease: false, + }, + { + // Evidence bundle export. + name: "publishing writer keeping diagnostics through a lease", + record: diagnosticsRecord, + kept: keptDiagnostics, + publishes: true, + lease: true, + }, +]; + +interface ScenarioControl { + /** The work's own result; `abort` cancels after the workload exits. */ + readonly work: "completes" | "fails" | "throws" | "completes-then-abort"; + /** Cancellation arrives during final cleanup. */ + readonly abortDuringClose?: true; + readonly cleanupFails?: true; + readonly publicationFails?: true; +} + +interface ScenarioRun { + readonly finalized: Finalized; + readonly commits: readonly Completed[]; + readonly primary: AnalysisProtocolError; + /** Lets the cleanup owner confirm release, as a retry would. */ + readonly recoverCleanup: () => Promise; +} + +const run = async ( + adopter: AdopterFixture, + control: ScenarioControl, +): Promise => { + const controller = new AbortController(); + const primary = new AnalysisProtocolError("fixture failure"); + const commits: Completed[] = []; + let releaseAttempts = 0; + const lease = new AnalysisCleanupLease(async () => { + releaseAttempts += 1; + if (control.cleanupFails === true && releaseAttempts === 1) + throw new Error("root removal denied"); + }, ["/owned/root"]); + const typedCleanup = new ProviderCleanupError("fixture", ["/owned/root"], { + reason: "still owned", + }); + const work = async (): Promise> => { + switch (control.work) { + case "completes": + return ok(completed); + case "completes-then-abort": + controller.abort(reason); + return ok(completed); + case "fails": + return err(primary); + case "throws": + throw new Error("unexpected"); + } + }; + const finalized = await runOwned(work, { + providerId: "fixture", + operation: "fixture_operation", + signal: controller.signal, + ...adopter.record, + ...(adopter.lease ? { cleanupLeases: [lease] } : {}), + close: async () => { + if (control.abortDuringClose === true) controller.abort(reason); + if (adopter.lease) await lease.release(); + else if (control.cleanupFails === true) throw typedCleanup; + }, + ...(adopter.publishes + ? { + commit: async (value: Completed) => { + commits.push(value); + if (control.publicationFails === true) + throw new Error("publication failed"); + }, + } + : {}), + }); + return { + finalized, + commits, + primary, + recoverCleanup: async () => { + if (adopter.lease) await lease.release(); + else typedCleanup.resolveCleanup(); + }, + }; +}; + +const failure = (finalized: Finalized): AnalysisError => { + const result = finalizedResult(finalized); + if (result.ok) throw new Error(`expected an error, got ${finalized.kind}`); + return result.error; +}; + +const expectCancelledBy = (error: AnalysisError) => { + const cancellation = primaryAnalysisError(error); + expect(cancellation).toBeInstanceOf(AnalysisCancelledError); + expect(cancellation.cause).toBe(reason); +}; + +interface LifecycleScenario { + /** The docs/development/testing.md lifecycle row this scenario covers. */ + readonly row: string; + readonly name: string; + readonly control: ScenarioControl; + readonly applies?: (adopter: AdopterFixture) => boolean; + readonly expect: (adopter: AdopterFixture, run: ScenarioRun) => Promise; +} + +const scenarios: readonly LifecycleScenario[] = [ + { + row: "Cancellation arrives after workload exit (non-aborted control)", + name: "completes when cleanup is released and no cancellation arrives", + control: { work: "completes" }, + expect: async (adopter, { finalized, commits }) => { + expect(finalized).toEqual({ kind: "completed", value: completed }); + expect(commits).toEqual(adopter.publishes ? [completed] : []); + }, + }, + { + row: "Cancellation arrives after workload exit, during result reads or temporary-root removal", + name: "cancellation after workload exit, before the commit cutoff", + control: { work: "completes-then-abort" }, + expect: async (adopter, { finalized, commits }) => { + expect(commits).toEqual([]); + expect(finalized).toMatchObject({ + kind: "cancelled", + cleanup: { kind: "released" }, + }); + const error = failure(finalized); + expectCancelledBy(error); + adopter.kept(error, "cancellation"); + }, + }, + { + row: "Cancellation arrives after workload exit, during result reads or temporary-root removal", + name: "cancellation during final cleanup", + control: { work: "completes", abortDuringClose: true }, + expect: async (adopter, { finalized, commits }) => { + if (adopter.publishes) { + // Publication precedes close, so the receipt stands. + expect(commits).toEqual([completed]); + expect(finalized).toEqual({ + kind: "completed", + value: completed, + lateCancellation: reason, + }); + return; + } + expect(finalized).toMatchObject({ + kind: "cancelled", + cleanup: { kind: "released" }, + }); + const error = failure(finalized); + expectCancelledBy(error); + adopter.kept(error, "cancellation"); + }, + }, + { + row: "Cleanup fails after success or primary failure", + name: "cleanup fails after success, then its owner confirms release", + control: { work: "completes", cleanupFails: true }, + expect: async (adopter, { finalized, commits, recoverCleanup }) => { + expect(commits).toEqual(adopter.publishes ? [completed] : []); + expect(finalized.kind).toBe("completed-cleanup-retained"); + const error = failure(finalized); + // The documented one-shot contract: an error that keeps the value. + expect(projectAnalysisError(error).code).toBe("cleanup_incomplete"); + expect(error.cleanupIncomplete).toBe(true); + expect(error.cleanupResources).toEqual(["/owned/root"]); + adopter.kept(error, "failure"); + await recoverCleanup(); + expect(error.cleanupIncomplete).toBe(false); + }, + }, + { + row: "Cleanup fails after success or primary failure", + name: "cleanup fails after a primary failure, then its owner confirms release", + control: { work: "fails", cleanupFails: true }, + expect: async ( + _adopter, + { finalized, commits, primary, recoverCleanup }, + ) => { + expect(commits).toEqual([]); + expect(finalized).toMatchObject({ + kind: "failed", + cleanup: { kind: "retained" }, + }); + const error = failure(finalized); + expect(primaryAnalysisError(error)).toBe(primary); + expect(error.cleanupIncomplete).toBe(true); + await recoverCleanup(); + expect(error.cleanupIncomplete).toBe(false); + }, + }, + { + row: "Cancellation arrives after workload exit, during result reads or temporary-root removal", + name: "a recorded failure keeps precedence over cancellation during final cleanup", + control: { work: "fails", abortDuringClose: true }, + expect: async (_adopter, { finalized, primary }) => { + expect(finalized).toMatchObject({ + kind: "failed", + cleanup: { kind: "released" }, + }); + expect(primaryAnalysisError(failure(finalized))).toBe(primary); + expect(primary.secondaryFailures).toEqual([ + expect.objectContaining({ + _tag: "AnalysisCancelledError", + cause: reason, + }), + ]); + }, + }, + { + row: "Cancellation arrives after workload exit, during result reads or temporary-root removal", + name: "cancellation during final cleanup that also fails", + control: { work: "completes", abortDuringClose: true, cleanupFails: true }, + expect: async (adopter, { finalized, recoverCleanup }) => { + const error = failure(finalized); + expect(error.cleanupIncomplete).toBe(true); + if (adopter.publishes) { + expect(finalized.kind).toBe("completed-cleanup-retained"); + adopter.kept(error, "failure"); + } else { + expect(finalized).toMatchObject({ + kind: "cancelled", + cleanup: { kind: "retained" }, + }); + expectCancelledBy(error); + } + await recoverCleanup(); + expect(error.cleanupIncomplete).toBe(false); + }, + }, + { + row: "Cancellation arrives after workload exit (publication outcome after commit)", + name: "publication fails and keeps the completed value", + control: { work: "completes", publicationFails: true }, + applies: (adopter) => adopter.publishes, + expect: async (adopter, { finalized, commits }) => { + expect(commits).toEqual([completed]); + expect(finalized).toMatchObject({ + kind: "failed", + cleanup: { kind: "released" }, + }); + adopter.kept(failure(finalized), "failure"); + }, + }, + { + row: "Launch returns an error (thrown work)", + name: "types a thrown work failure as a provider adapter failure", + control: { work: "throws" }, + expect: async (_adopter, { finalized, commits }) => { + expect(commits).toEqual([]); + expect(finalized.kind).toBe("failed"); + expect(failure(finalized)._tag).toBe("ProviderAdapterError"); + }, + }, +]; + +describe.each(adopters)("runOwned lifecycle: $name", (adopter) => { + it.each(scenarios.filter((scenario) => scenario.applies?.(adopter) ?? true))( + "$name", + async (scenario) => { + await scenario.expect(adopter, await run(adopter, scenario.control)); + }, + ); +}); diff --git a/src/application/runOwned.test.ts b/src/providers/runOwned.test.ts similarity index 54% rename from src/application/runOwned.test.ts rename to src/providers/runOwned.test.ts index 7b2ee3389e..9abae9a9fe 100644 --- a/src/application/runOwned.test.ts +++ b/src/providers/runOwned.test.ts @@ -9,6 +9,8 @@ import { type AnalysisError, type AnalysisPartialObservation, } from "../domain/analysisErrorBase.js"; +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -19,147 +21,75 @@ const value: AnalysisPartialObservation = { evidence_id: "evidence-1", }; const reason = { caller: "stop" }; -const completedWork = async (): Promise< - Result -> => ok(value); -/** A close that aborts the caller's signal, so cancellation lands during final cleanup. */ -const closeThenAbort = - (controller: AbortController, failure?: unknown) => async () => { - controller.abort(reason); - if (failure !== undefined) throw failure; - }; +// Scenarios shared by every adopter live in runOwned.lifecycle.test.ts; these +// cases cover producer-supplied errors, reader leases and recovered completions. const options = (controller: AbortController, close: () => Promise) => ({ providerId: "fixture", operation: "fixture_operation", signal: controller.signal, close, + partial: (completed: AnalysisPartialObservation) => completed, }); -describe("runOwned precedence", () => { - it("completes when cleanup is released and no cancellation arrives", async () => { - const controller = new AbortController(); - const finalized = await runOwned( - completedWork, - options(controller, async () => undefined), - ); - expect(finalized).toEqual({ kind: "completed", value }); - }); +const counted = async (): Promise< + Result<{ readonly count: number }, AnalysisError> +> => ok({ count: 3 }); +const project = (completed: { readonly count: number }) => ({ + kind: "retained-evidence" as const, + evidence_id: `counted-${String(completed.count)}`, +}); - it("cancels a completed value when cancellation arrives during final cleanup before commit", async () => { - const controller = new AbortController(); - const finalized = await runOwned( - completedWork, - options(controller, closeThenAbort(controller)), - ); - expect(finalized.kind).toBe("cancelled"); - const result = finalizedResult(finalized); - if (result.ok) throw new Error("expected cancellation"); - expect(result.error).toBeInstanceOf(AnalysisCancelledError); - expect(result.error.cause).toBe(reason); - expect(result.error.partialObservation).toBe(value); - }); - - it("keeps a committed value when cancellation arrives after the commit", async () => { - const controller = new AbortController(); - const commits: unknown[] = []; - const finalized = await runOwned(completedWork, { - ...options(controller, closeThenAbort(controller)), - commit: async (committed) => { - commits.push(committed); - }, - }); - expect(commits).toEqual([value]); - expect(finalized).toEqual({ - kind: "completed", - value, - lateCancellation: reason, - }); - }); - - it("does not commit after cancellation and reports it before commit", async () => { - const controller = new AbortController(); - controller.abort(reason); - let committed = false; - const finalized = await runOwned(completedWork, { - ...options(controller, async () => undefined), - commit: async () => { - committed = true; - }, - }); - expect(committed).toBe(false); - expect(finalized.kind).toBe("cancelled"); - }); - - it("keeps a recorded failure primary when cancellation arrives during final cleanup", async () => { - const controller = new AbortController(); - const failure = new AnalysisProtocolError("fixture failure"); - const finalized = await runOwned( - async () => err(failure), - options(controller, closeThenAbort(controller)), - ); - expect(finalized.kind).toBe("failed"); - const result = finalizedResult(finalized); - if (result.ok) throw new Error("expected failure"); - expect(primaryAnalysisError(result.error)).toBe(failure); - expect(failure.secondaryFailures).toEqual([ - expect.objectContaining({ - _tag: "AnalysisCancelledError", - cause: reason, +describe("runOwned close-failure lease binding", () => { + it("binds an artifact reader close failure to the leases it reports", async () => { + let attempts = 0; + const lease = new AnalysisCleanupLease(async () => { + attempts += 1; + if (attempts === 1) throw new Error("close failed"); + }, ["/reader/handle"]); + await lease.release().catch(() => undefined); + const finalized = await runOwned(counted, { + ...options(new AbortController(), async () => { + throw new ArtifactReaderFailure("io", "reader close failed", { + cleanup: { reason: "handle retained", resources: ["/reader/handle"] }, + cleanupLeases: [lease], + }); }), - ]); - }); - - it("reports a completed value with unconfirmed cleanup as cleanup_incomplete keeping the value", async () => { - const controller = new AbortController(); - const finalized = await runOwned( - completedWork, - options(controller, async () => { - throw new Error("close failed"); - }), - ); + partial: project, + }); expect(finalized.kind).toBe("completed-cleanup-retained"); const result = finalizedResult(finalized); if (result.ok) throw new Error("expected cleanup failure"); - expect(result.error).toBeInstanceOf(ProviderCleanupError); expect(result.error.cleanupIncomplete).toBe(true); - expect(result.error.partialObservation).toBe(value); + await lease.release(); + expect(result.error.cleanupIncomplete).toBe(false); }); - it("keeps the primary failure's identity when cleanup also fails", async () => { + it("binds a typed close failure to the leases while the failed work stays primary", async () => { const controller = new AbortController(); const failure = new AnalysisProtocolError("fixture failure"); - const cleanup = new ProviderCleanupError("fixture", ["fixture-run"], { - reason: "still running", + const typed = new ProviderCleanupError("fixture", ["/producer/root"], { + reason: "still owned", }); - const finalized = await runOwned( - async () => err(failure), - options(controller, async () => { - throw cleanup; - }), + const lease = new AnalysisCleanupLease( + async () => undefined, + ["/owned/root"], ); - expect(finalized).toMatchObject({ - kind: "failed", - cleanup: { kind: "retained", failure: cleanup }, + const finalized = await runOwned(async () => err(failure), { + ...options(controller, async () => { + throw typed; + }), + partial: project, + cleanupLeases: [lease], }); const result = finalizedResult(finalized); if (result.ok) throw new Error("expected failure"); expect(primaryAnalysisError(result.error)).toBe(failure); - expect(result.error.cleanupIncomplete).toBe(true); - }); - - it("types a thrown work failure as a provider adapter failure", async () => { - const controller = new AbortController(); - const finalized = await runOwned( - async () => { - throw new Error("unexpected"); - }, - options(controller, async () => undefined), - ); - const result = finalizedResult(finalized); - if (result.ok) throw new Error("expected failure"); - expect(result.error._tag).toBe("ProviderAdapterError"); + expect(result.error.cleanupResources).toEqual(["/owned/root"]); + await lease.release(); + expect(typed.cleanupIncomplete).toBe(false); + expect(result.error.cleanupIncomplete).toBe(false); }); }); @@ -205,17 +135,6 @@ it("classifies a producer composite whose primary is a cancellation as cancelled }); }); -it("keeps a failure with a later cancellation classified as failed", async () => { - const controller = new AbortController(); - const failure = new AnalysisProtocolError("fixture failure"); - const finalized = await runOwned( - async () => err(failure), - options(controller, closeThenAbort(controller)), - ); - expect(finalized.kind).toBe("failed"); - expect(failure.secondaryFailures).toHaveLength(1); -}); - it("keeps producer cleanup retained when close does not resolve it", async () => { const controller = new AbortController(); const failure = new ProviderCleanupError("fixture", ["owned-process"], { @@ -232,18 +151,6 @@ it("keeps producer cleanup retained when close does not resolve it", async () => }); }); -it("retains completed work when publication fails", async () => { - const finalized = await runOwned(completedWork, { - ...options(new AbortController(), async () => undefined), - commit: async () => { - throw new Error("publication failed"); - }, - }); - const result = finalizedResult(finalized); - if (result.ok) throw new Error("expected publication failure"); - expect(result.error.partialObservation).toBe(value); -}); - it("publishes recovered completion after cleanup resolves", async () => { const controller = new AbortController(); const failure = new ProviderCleanupError("fixture", ["owned-process"], { diff --git a/src/application/runOwned.ts b/src/providers/runOwned.ts similarity index 70% rename from src/application/runOwned.ts rename to src/providers/runOwned.ts index 518cd62bd5..b2e6ec2934 100644 --- a/src/application/runOwned.ts +++ b/src/providers/runOwned.ts @@ -1,3 +1,7 @@ +import { + AnalysisCleanupLeaseGroup, + type AnalysisCleanupLease, +} from "../artifacts/AnalysisCleanupLease.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { AnalysisError, @@ -25,11 +29,11 @@ export type CleanupOutcome = * Work, owner cleanup and cancellation composed by one precedence rule: * - a recorded failure stays primary; a later cancellation is attached to it; * - cancellation observed before commit turns a completed value into a - * cancellation that keeps the value as its partial observation; + * cancellation that keeps the value (see `CompletedValueRecord`); * - cancellation observed after commit keeps the completed value; * - unconfirmed cleanup never replaces the primary. A completed value whose * cleanup is unconfirmed is a `cleanup_incomplete` error that keeps the - * value as its partial observation (the documented one-shot contract). + * value (the documented one-shot contract). */ export type Finalized = | { @@ -61,14 +65,53 @@ export interface RecoveredCompletion { readonly cancellationDiagnostics?: Readonly>; } -export interface RunOwnedOptions { +/** The error that replaces a completed value and must keep it. */ +export type CompletedValueReplacement = "cancellation" | "failure"; + +/** + * How an error that replaces a completed value keeps it: as its partial + * observation, or as diagnostics when the value is not an observation a + * caller can recover. The replacement lets a producer keep a different + * projection for a cancellation than for a cleanup or publication failure. + */ +export type CompletedValueRecord = + | { + readonly partial: ( + value: Value, + replacedBy: CompletedValueReplacement, + ) => AnalysisPartialObservation; + readonly diagnostics?: never; + } + | { + readonly diagnostics: ( + value: Value, + replacedBy: CompletedValueReplacement, + ) => Readonly>; + readonly partial?: never; + }; + +type KeptCompletion = + | { readonly partialObservation: AnalysisPartialObservation } + | { readonly diagnostics: Readonly> }; + +export type RunOwnedOptions = RunOwnedBaseOptions & + CompletedValueRecord; + +interface RunOwnedBaseOptions { readonly providerId: string; readonly operation: string; - readonly signal: AbortSignal; + readonly signal?: AbortSignal | undefined; /** Releases everything the work acquired; awaited before the signal is read. */ readonly close: () => Promise; /** Names the owned resources when a non-REA close failure names none. */ readonly fallbackResource?: string; + /** + * The exact leases whose confirmed release resolves a close failure; they + * also attribute its resources. Omit them when close throws an error its + * producer already bound, or an artifact reader failure that names its own + * retained leases. + */ + readonly cleanupLeases?: readonly AnalysisCleanupLease[]; /** * Publishes a completed value. Cancellation observed before it runs cancels * the work; cancellation observed after it keeps the committed value. @@ -87,10 +130,27 @@ export interface RunOwnedOptions { ) => RecoveredCompletion | undefined; } +const keepCompleted = ( + record: CompletedValueRecord, + value: Value, + replacedBy: CompletedValueReplacement, +): KeptCompletion => + record.partial === undefined + ? { diagnostics: record.diagnostics(value, replacedBy) } + : { partialObservation: record.partial(value, replacedBy) }; + +const retainCompleted = ( + error: AnalysisError, + kept: KeptCompletion, +): AnalysisError => + "partialObservation" in kept + ? error.retainPartialObservation(kept.partialObservation) + : error.retainDiagnostics(kept.diagnostics); + const closeFailure = ( cause: unknown, options: Pick< - RunOwnedOptions, + RunOwnedBaseOptions, "providerId" | "operation" | "fallbackResource" >, ): AnalysisError => { @@ -114,7 +174,7 @@ const closeFailure = ( }; /** Run owned work, close its owner, and apply the finalization precedence once. */ -export const runOwned = async ( +export const runOwned = async ( work: () => Promise>, options: RunOwnedOptions, ): Promise> => { @@ -141,19 +201,20 @@ export const runOwned = async ( committed = true; return { ok: true, value }; } catch (cause) { + const kept = keepCompleted(options, value, "failure"); return { ok: false, error: cause instanceof AnalysisError - ? cause.retainPartialObservation(value) + ? retainCompleted(cause, kept) : new ProviderAdapterError(options.providerId, options.operation, { cause, - partialObservation: value, + ...kept, }), }; } }; - if (primary.ok && options.commit !== undefined && !options.signal.aborted) + if (primary.ok && options.commit !== undefined && !isAborted(options.signal)) primary = await commit(primary.value); options.inspectPrimary?.(primary); const hadIncompleteCleanup = !primary.ok && primary.error.cleanupIncomplete; @@ -163,7 +224,16 @@ export const runOwned = async ( // they can confirm it, so its status is read again afterwards. await options.close(); } catch (cause) { - cleanup = { kind: "retained", failure: closeFailure(cause, options) }; + const failure = closeFailure(cause, options); + const leases = + options.cleanupLeases ?? + // A reader failure's leases are authoritative only for the cleanup it + // reports. + (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined + ? cause.cleanupLeases + : undefined); + if (leases !== undefined) new AnalysisCleanupLeaseGroup(failure, leases); + cleanup = { kind: "retained", failure }; } const signal = options.signal; let outcome: @@ -197,7 +267,7 @@ export const runOwned = async ( outcome.ok && !primary.ok && options.commit !== undefined && - !signal.aborted + !isAborted(signal) ) { const publication = await commit(outcome.value); if (!publication.ok) { @@ -232,14 +302,21 @@ export const runOwned = async ( }; if (!committed) { cancelled = true; + const kept = keepCompleted(options, outcome.value, "cancellation"); outcome = { ok: false, - error: cancelledBy(options.operation, signal, { - partialObservation: outcome.value, - ...(outcome.diagnostics === undefined - ? {} - : { diagnostics: outcome.diagnostics }), - }), + error: cancelledBy( + options.operation, + signal, + "partialObservation" in kept + ? { + partialObservation: kept.partialObservation, + ...(outcome.diagnostics === undefined + ? {} + : { diagnostics: outcome.diagnostics }), + } + : { diagnostics: { ...outcome.diagnostics, ...kept.diagnostics } }, + ), }; } } @@ -248,7 +325,10 @@ export const runOwned = async ( return { kind: "completed-cleanup-retained", value: outcome.value, - error: cleanup.failure.retainPartialObservation(outcome.value), + error: retainCompleted( + cleanup.failure, + keepCompleted(options, outcome.value, "failure"), + ), }; return { kind: cancelled ? "cancelled" : "failed", diff --git a/src/signals/SigrokRecordingProvider.ts b/src/signals/SigrokRecordingProvider.ts index c3215513e7..582bdcbd1e 100644 --- a/src/signals/SigrokRecordingProvider.ts +++ b/src/signals/SigrokRecordingProvider.ts @@ -66,6 +66,7 @@ import { signalRecordingSchema, } from "../domain/signals/signalRecording.js"; import { err, ok, type Result } from "../domain/result.js"; +import { finalizedResult, runOwned } from "../providers/runOwned.js"; import { runOwnedCommand, OwnedCommandFailure, @@ -451,17 +452,21 @@ export class SigrokRecordingProvider implements SignalRecordingPort { ), ); } - return this.#resources.finish( - result, - (execution) => - createAnalysisExecutionEvidence( - operation, - input, - execution, - "observed", - ), - operation, - options?.signal, + const completed = result; + return finalizedResult( + await runOwned(async () => completed, { + providerId: this.#resources.providerId, + operation, + signal: options?.signal, + close: () => this.#resources.release(), + partial: (execution) => + createAnalysisExecutionEvidence( + operation, + input, + execution, + "observed", + ), + }), ); } #failure( diff --git a/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts b/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts index d716d7ce2e..174bb04f0b 100644 --- a/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts +++ b/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts @@ -9,6 +9,7 @@ import { execFileOutput, } from "../../../../src/process/ExecFileOutput.js"; import { parseEvidence } from "../../../../src/domain/evidence.js"; +import { primaryAnalysisError } from "../../../../src/domain/analysisErrorBase.js"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { PrivateRuntimeRoot } from "../../../../src/process/PrivateRuntimeRoot.js"; import { AdbInputSnapshotFailure } from "../../../../src/adb/AdbInputSnapshot.js"; @@ -544,7 +545,9 @@ test.each([false, true])( throw new Error("expected retained package digest cleanup failure"); expect(result.error.cleanupIncomplete).toBe(!cancelDuringClose); if (cancelDuringClose) { - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cause).toBe(reason); expect(result.error.cleanupResources).toEqual([]); } @@ -649,7 +652,9 @@ test.each([false, true])( signal: controller.signal, }); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); if (failCleanup) { expect(result.error.cause).toBeInstanceOf(AggregateError); const causes = (result.error.cause as AggregateError).errors as [ @@ -713,7 +718,9 @@ test("cancels after the input snapshot copy and before contacting the device", a signal: controller.signal, }); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(execFileOutput).toHaveBeenCalledTimes(1); expect(execFileBytes).not.toHaveBeenCalled(); expect(await readFile(f.source, "utf8")).toBe("immutable payload"); diff --git a/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts b/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts index a1dfaa07ce..8f084cf5ae 100644 --- a/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts +++ b/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts @@ -5,6 +5,7 @@ import { afterEach, expect, onTestFinished, test, vi } from "vitest"; import { AdbProvider } from "../../../../src/adb/AdbProvider.js"; import { parseEvidence } from "../../../../src/domain/evidence.js"; +import { primaryAnalysisError } from "../../../../src/domain/analysisErrorBase.js"; import { execFileBytes, execFileOutput, @@ -73,7 +74,9 @@ test("does not publish a screen capture cancelled before the commit point", asyn { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cause).toBe(abortReason); await expect(readFile(join(output, "screen.png"))).rejects.toMatchObject({ code: "ENOENT", @@ -106,7 +109,9 @@ test("retains a committed screen capture receipt when cancellation arrives after { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cause).toBe(abortReason); const evidence = parseEvidence(result.error.partialObservation); expect(evidence).toMatchObject({ @@ -262,7 +267,9 @@ test("retains the committed receipt and cleanup observation when cancellation ra { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cleanupIncomplete).toBe(true); expect(result.error.cause).toBeInstanceOf(AggregateError); const causes = (result.error.cause as AggregateError).errors as [ @@ -354,7 +361,9 @@ test("cancellation after a failed hard link but before exclusive creation does n { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cause).toBe(abortReason); await expect(readFile(join(output, "screen.png"))).rejects.toMatchObject({ code: "ENOENT", @@ -392,7 +401,9 @@ test("retains a receipt when cancellation arrives after exclusive output creatio { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cause).toBe(abortReason); expect(parseEvidence(result.error.partialObservation)).toMatchObject({ operation: "capture_adb_screen", @@ -718,7 +729,9 @@ test("retains incomplete cleanup on a cancelled capture while keeping cancellati { signal: controller.signal }, ); if (result.ok) throw new Error("expected cancellation"); - expect(result.error._tag).toBe("AnalysisCancelledError"); + expect(primaryAnalysisError(result.error)._tag).toBe( + "AnalysisCancelledError", + ); expect(result.error.cleanupIncomplete).toBe(true); expect(result.error.cleanupResources).toContain(runtime.path); await provider.close(); diff --git a/tests/process-global/bundleComparisonFailures.test.ts b/tests/process-global/bundleComparisonFailures.test.ts index 2542508220..81d85e34ce 100644 --- a/tests/process-global/bundleComparisonFailures.test.ts +++ b/tests/process-global/bundleComparisonFailures.test.ts @@ -565,3 +565,22 @@ it.each(["before-commit", "after-commit", "cleanup-retry"])( } }, ); + +it("reports an unconfirmed read cleanup after a completed comparison as cleanup_incomplete", async () => { + const directory = await createTestTempDirectory("rea-bundle-close-failure-"); + const { path, bundle } = await validBundleFile(directory); + const close = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockRejectedValueOnce(new Error("injected scope close failure")); + onTestFinished(() => close.mockRestore()); + const result = await compareEvidenceBundlesCommand({ + leftPath: path, + rightPath: path, + }); + if (result.ok) throw new Error("Expected an unconfirmed cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + // The completed comparison stays with the cleanup failure. + expect(result.error.diagnostics).toMatchObject({ + completed_result: bundleComparison.compareBundles(bundle, bundle), + }); +});