From 2bcd7e6d13f0a8118d6619718254255e84f75d86 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:42:32 +0800 Subject: [PATCH] refactor(artifacts): centralize completed file outcomes Project completed values and cancellation from the descriptor owner once across artifact consumers. Preserve completed void outcomes and format-specific validation and cleanup policies. Keep later aborts from replacing captured APK and SQLite close failures, and retain the exact APK cancellation reason. Exercise real descriptors, public workflows and cleanup retries. --- src/apkSignature/ApkSignatureProvider.ts | 2 +- src/apkSignature/apkSignatureFailures.ts | 16 ++-- .../binaryDiagnostics/ByteEntropyService.ts | 24 +++--- src/artifacts/ArtifactFileOperation.ts | 26 ++++++ src/artifacts/DesktopApplicationReader.ts | 10 +-- src/browser/WebBundleAnalysisWorker.ts | 34 ++------ src/cutter/CutterBridgeClient.ts | 8 +- src/electron/ElectronFuseFailures.ts | 6 +- src/electron/ElectronFuseProvider.ts | 22 +++--- src/firmware/FirmwareFailures.ts | 6 +- src/ghidra/GhidraInstallationProperties.ts | 10 +-- src/gpu/GpuCodeObjectsProvider.ts | 17 +--- src/il2cpp/Il2cppMetadataProvider.ts | 18 ++--- src/model/ModelArtifactProvider.ts | 17 +--- src/sqlite/SqliteDatabaseFailures.ts | 5 +- src/sqlite/SqliteDatabaseWorkerExecution.ts | 11 +-- .../filesystem/stableArtifact.test.ts | 42 +++++++++- .../sqliteDatabaseDescriptorCleanup.test.ts | 65 +++++++++++++++ .../sqlite/sqliteDatabaseFailures.test.ts | 21 +++++ .../apkSignatureLifecycle.test.ts | 79 +++++++++++++++++++ 20 files changed, 291 insertions(+), 148 deletions(-) diff --git a/src/apkSignature/ApkSignatureProvider.ts b/src/apkSignature/ApkSignatureProvider.ts index 288e07c81c..66863f93c3 100644 --- a/src/apkSignature/ApkSignatureProvider.ts +++ b/src/apkSignature/ApkSignatureProvider.ts @@ -397,7 +397,7 @@ export class ApkSignatureProvider implements ApkSignaturePort { } }); } catch (cause: unknown) { - return err(apkSignatureFailure(cause, input.path, options?.signal)); + return err(apkSignatureFailure(cause, input.path)); } } diff --git a/src/apkSignature/apkSignatureFailures.ts b/src/apkSignature/apkSignatureFailures.ts index 1867972e16..0466b5b9c7 100644 --- a/src/apkSignature/apkSignatureFailures.ts +++ b/src/apkSignature/apkSignatureFailures.ts @@ -1,6 +1,7 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { - ArtifactFileCleanupFailure, + artifactFilePrimaryFailure, + artifactFileCancellationCause, bindArtifactFileCleanup, } from "../artifacts/ArtifactFileOperation.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; @@ -124,15 +125,8 @@ export const apkOutputBudgetError = (projectedBytes: number): AnalysisError => export const apkSignatureFailure = ( cause: unknown, path: string, - signal?: AbortSignal, ): AnalysisError => { - const primary = - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "failed" - ? primaryFailure(cause.fileCleanupFailure.outcome.cause, path) - : cause instanceof ArtifactFileCleanupFailure && signal?.aborted - ? new AnalysisCancelledError(OPERATION, { cause: signal.reason }) - : primaryFailure(cause, path); + const primary = primaryFailure(artifactFilePrimaryFailure(cause), path); if (!(cause instanceof ArtifactReaderFailure) || cause.cleanup === undefined) return primary; const cleanup = new ProviderCleanupError( @@ -187,7 +181,9 @@ const artifactReaderFailure = ( cause, }); case "cancelled": - return new AnalysisCancelledError(OPERATION, { cause: cause.cause }); + return new AnalysisCancelledError(OPERATION, { + cause: artifactFileCancellationCause(cause), + }); case "limit": return new AnalysisResourceConstraintError( OPERATION, diff --git a/src/application/binaryDiagnostics/ByteEntropyService.ts b/src/application/binaryDiagnostics/ByteEntropyService.ts index 405c421f2a..b3f0311c0c 100644 --- a/src/application/binaryDiagnostics/ByteEntropyService.ts +++ b/src/application/binaryDiagnostics/ByteEntropyService.ts @@ -29,7 +29,7 @@ import { withArtifactFileHandle, bindArtifactFileCleanup, ArtifactFileCleanupFailure, - ArtifactFileCancelled, + artifactFileCompletion, assertArtifactReadActive, artifactFileCancellationCause, } from "../../artifacts/ArtifactFileOperation.js"; @@ -294,6 +294,7 @@ export class ByteEntropyService { options?.signal, ); } catch (cause) { + const completion = artifactFileCompletion(cause); if (cause instanceof ArtifactFileCleanupFailure) { const outcome = cause.fileCleanupFailure.outcome; const cleanup = new AnalysisOutputError( @@ -302,9 +303,9 @@ export class ByteEntropyService { { cause, cleanup: cause.cleanup, - ...(outcome.kind === "completed" - ? { partialObservation: completedObservation(outcome.value) } - : {}), + ...(completion === undefined + ? {} + : { partialObservation: completedObservation(completion.value) }), }, ); bindArtifactFileCleanup(cause, cleanup); @@ -320,8 +321,8 @@ export class ByteEntropyService { bindArtifactFileCleanup(cause, cancelled); return err(cancelled); } - const cancellation = cause.fileCleanupFailure.completionCancellation; - if (cancellation !== undefined && outcome.kind === "completed") { + const cancellation = completion?.cancellation; + if (cancellation !== undefined) { const cancelled = new AnalysisCancelledError(OPERATION, { cause: cancellation.reason, partialObservation: cleanup.partialObservation, @@ -346,14 +347,9 @@ export class ByteEntropyService { if (cause.reason === "cancelled") { const cancelled = new AnalysisCancelledError(OPERATION, { cause: artifactFileCancellationCause(cause), - ...(cause instanceof ArtifactFileCancelled && - cause.fileCancellation.outcome.kind === "completed" - ? { - partialObservation: completedObservation( - cause.fileCancellation.outcome.value, - ), - } - : {}), + ...(completion === undefined + ? {} + : { partialObservation: completedObservation(completion.value) }), }); bindArtifactFileCleanup(cause, cancelled); return err(cancelled); diff --git a/src/artifacts/ArtifactFileOperation.ts b/src/artifacts/ArtifactFileOperation.ts index e208aeeb58..1dda61099c 100644 --- a/src/artifacts/ArtifactFileOperation.ts +++ b/src/artifacts/ArtifactFileOperation.ts @@ -142,6 +142,32 @@ export class ArtifactFileCancelled< } } +/** Completion facts recorded by the file owner, independent of later caller state. */ +export const artifactFileCompletion = ( + cause: unknown, +): + | { + readonly kind: "completed"; + readonly value: unknown; + readonly cancellation: { readonly reason: unknown } | undefined; + } + | undefined => { + if (cause instanceof ArtifactFileCancelled) + return { + ...cause.fileCancellation.outcome, + cancellation: { reason: cause.fileCancellation.reason }, + }; + if ( + cause instanceof ArtifactFileCleanupFailure && + cause.fileCleanupFailure.outcome.kind === "completed" + ) + return { + ...cause.fileCleanupFailure.outcome, + cancellation: cause.fileCleanupFailure.completionCancellation, + }; + return undefined; +}; + /** Bind a translated failure to its exact descriptor leases, including failed retries. */ export const bindArtifactFileCleanup = ( cause: unknown, diff --git a/src/artifacts/DesktopApplicationReader.ts b/src/artifacts/DesktopApplicationReader.ts index 2aa26bbea0..a04f3561f8 100644 --- a/src/artifacts/DesktopApplicationReader.ts +++ b/src/artifacts/DesktopApplicationReader.ts @@ -7,10 +7,7 @@ import { type DirectoryArtifactEntry, } from "./DirectoryArtifactReader.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; -import { - ArtifactFileCancelled, - ArtifactFileCleanupFailure, -} from "./ArtifactFileOperation.js"; +import { artifactFileCompletion } from "./ArtifactFileOperation.js"; import { ArtifactResourceScope, type ArtifactResourceOwner, @@ -330,10 +327,7 @@ const scanDirectory = async ( row.read_status = "unavailable"; row.issues.push(cause.message); partial = true; - const completedRead = - cause instanceof ArtifactFileCancelled || - (cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed"); + const completedRead = artifactFileCompletion(cause) !== undefined; if (completedRead) row.read_status = probeIssueCount === 0 ? "examined" : "partial"; if ( diff --git a/src/browser/WebBundleAnalysisWorker.ts b/src/browser/WebBundleAnalysisWorker.ts index eaa638174f..38efaac391 100644 --- a/src/browser/WebBundleAnalysisWorker.ts +++ b/src/browser/WebBundleAnalysisWorker.ts @@ -11,7 +11,7 @@ import { AnalysisCleanupLeaseGroup, } from "../artifacts/AnalysisCleanupLease.js"; import { - ArtifactFileCancelled, + artifactFileCompletion, ArtifactFileCleanupFailure, artifactFileCancellationCause, withArtifactFileHandle, @@ -393,7 +393,8 @@ export class WebBundleAnalysisWorker { completed = facts; result = ok(facts); } catch (cause) { - const snapshot = this.#completedSnapshot(cause); + const completion = artifactFileCompletion(cause); + const snapshot = stableSnapshotValue(completion?.value); if (snapshot !== undefined && resultDescriptor !== undefined) { try { completed = this.#validateCompletedResult( @@ -406,7 +407,7 @@ export class WebBundleAnalysisWorker { // The physical read completed, but only validated facts are retained. } } - const cancellationCause = this.#completedReadCancellation(cause); + const cancellationCause = completion?.cancellation; const interrupted = deadline?.interruption === "timeout" ? scriptTimeout(timeoutMs, cause) @@ -428,8 +429,7 @@ export class WebBundleAnalysisWorker { if (observedFailure !== undefined && interrupted !== undefined) observedFailure.retainSecondaryFailure(interrupted); const failure = - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" + cause instanceof ArtifactFileCleanupFailure && completion !== undefined ? this.#cleanupFailure(cause, interrupted) : interrupted !== undefined && observedFailure !== undefined ? observedFailure @@ -786,30 +786,6 @@ export class WebBundleAnalysisWorker { return this.#frameSignal.waitForOutputClose(supervisor, deadline.signal); } - #completedSnapshot(cause: unknown): StableArtifactSnapshot | undefined { - if (cause instanceof ArtifactFileCancelled) - return stableSnapshotValue(cause.fileCancellation.outcome.value); - if ( - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" - ) - return stableSnapshotValue(cause.fileCleanupFailure.outcome.value); - return undefined; - } - - #completedReadCancellation( - cause: unknown, - ): { readonly reason: unknown } | undefined { - if (cause instanceof ArtifactFileCancelled) - return { reason: artifactFileCancellationCause(cause) }; - if ( - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" - ) - return cause.fileCleanupFailure.completionCancellation; - return undefined; - } - #validateCompletedResult( stored: StableArtifactSnapshot, descriptor: WebBundleResultDescriptor, diff --git a/src/cutter/CutterBridgeClient.ts b/src/cutter/CutterBridgeClient.ts index 7968656ed7..8f090d997d 100644 --- a/src/cutter/CutterBridgeClient.ts +++ b/src/cutter/CutterBridgeClient.ts @@ -5,6 +5,7 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { withArtifactRegularFile, ArtifactFileCleanupFailure, + artifactFileCompletion, bindArtifactFileCleanup, artifactFileCancellationCause, assertArtifactReadActive, @@ -1224,14 +1225,13 @@ const discoveryFailure = ( cause instanceof ArtifactFileCleanupFailure ? cause.fileCleanupFailure.outcome : undefined; + const cancellation = artifactFileCompletion(cause)?.cancellation; const primary = outcome?.kind === "failed" ? discoveryFailure(outcome.cause, operation) - : outcome?.kind === "completed" && - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.completionCancellation !== undefined + : cancellation !== undefined ? new AnalysisCancelledError(operation, { - cause: cause.fileCleanupFailure.completionCancellation.reason, + cause: cancellation.reason, }) : undefined; if (primary === undefined && outcome?.kind !== "failed") return cleanup; diff --git a/src/electron/ElectronFuseFailures.ts b/src/electron/ElectronFuseFailures.ts index 40d88a8dcd..64714b3cc4 100644 --- a/src/electron/ElectronFuseFailures.ts +++ b/src/electron/ElectronFuseFailures.ts @@ -1,6 +1,7 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { ArtifactFileCleanupFailure, + artifactFileCompletion, artifactFileCancellationCause, bindArtifactFileCleanup, } from "../artifacts/ArtifactFileOperation.js"; @@ -56,13 +57,14 @@ export const electronFuseFailure = ( bindArtifactFileCleanup(cause, cleanup); if (cause instanceof ArtifactFileCleanupFailure) { const outcome = cause.fileCleanupFailure.outcome; + const cancellation = artifactFileCompletion(cause)?.cancellation; const primary = outcome.kind === "failed" ? electronFuseFailure(outcome.cause, path, operation) - : cause.fileCleanupFailure.completionCancellation === undefined + : cancellation === undefined ? undefined : new AnalysisCancelledError(operation, { - cause: cause.fileCleanupFailure.completionCancellation.reason, + cause: cancellation.reason, }); if (primary !== undefined) return analysisErrorWithCleanupFailure( diff --git a/src/electron/ElectronFuseProvider.ts b/src/electron/ElectronFuseProvider.ts index 6094065241..a21d5cfbdc 100644 --- a/src/electron/ElectronFuseProvider.ts +++ b/src/electron/ElectronFuseProvider.ts @@ -4,8 +4,7 @@ import { createElectronFuseEvidence } from "../domain/electron/electronFuseEvide import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { assertArtifactReadActive, - ArtifactFileCancelled, - ArtifactFileCleanupFailure, + artifactFileCompletion, } from "../artifacts/ArtifactFileOperation.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; @@ -81,19 +80,16 @@ export class ElectronFuseProvider implements ElectronFusePort { } catch (cause: unknown) { const failure = electronFuseFailure(cause, input.path); // Only this reader's completed physical outcome supplies partial fuse facts. - const outcome = - cause instanceof ArtifactFileCleanupFailure - ? (cause as ArtifactFileCleanupFailure) - .fileCleanupFailure.outcome - : cause instanceof ArtifactFileCancelled - ? (cause as ArtifactFileCancelled) - .fileCancellation.outcome - : undefined; + const completion = artifactFileCompletion(cause); const report = completed ?? - (outcome?.kind === "completed" - ? reportFor(outcome.value, input.path, resolvedPath) - : undefined); + (completion === undefined + ? undefined + : reportFor( + completion.value as ElectronFuseImage, + input.path, + resolvedPath, + )); if (report !== undefined) { failure.retainPartialObservation( createElectronFuseEvidence(input, report, this.identity), diff --git a/src/firmware/FirmwareFailures.ts b/src/firmware/FirmwareFailures.ts index 8ff7bfdec9..f9066251ea 100644 --- a/src/firmware/FirmwareFailures.ts +++ b/src/firmware/FirmwareFailures.ts @@ -2,6 +2,7 @@ import type { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js" import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { ArtifactFileCleanupFailure, + artifactFileCompletion, artifactFileCancellationCause, bindArtifactFileCleanup, } from "../artifacts/ArtifactFileOperation.js"; @@ -154,13 +155,14 @@ export const firmwareFailure = ( treeCleanupLease?.bindCleanupError(cleanup); if (cause instanceof ArtifactFileCleanupFailure) { const outcome = cause.fileCleanupFailure.outcome; + const cancellation = artifactFileCompletion(cause)?.cancellation; const primary = outcome.kind === "failed" ? firmwareFailure(outcome.cause, context) - : cause.fileCleanupFailure.completionCancellation !== undefined + : cancellation !== undefined ? firmwareCancellation( operation, - cause.fileCleanupFailure.completionCancellation.reason, + cancellation.reason, deadline, publication, ) diff --git a/src/ghidra/GhidraInstallationProperties.ts b/src/ghidra/GhidraInstallationProperties.ts index 28c6b5f83c..10d42d19e9 100644 --- a/src/ghidra/GhidraInstallationProperties.ts +++ b/src/ghidra/GhidraInstallationProperties.ts @@ -1,7 +1,7 @@ import { createHash } from "node:crypto"; import type { RetainedResourceScope } from "../application/AnalysisProvider.js"; import { - ArtifactFileCancelled, + artifactFileCompletion, ArtifactFileCleanupFailure, artifactFilePrimaryFailure, withArtifactRegularFile, @@ -64,13 +64,7 @@ export const readGhidraInstallationProperties = async ( (physical.code === "ENOENT" || physical.code === "ENOTDIR") ) return undefined; - const completed = - cause instanceof ArtifactFileCancelled - ? cause.fileCancellation.outcome.value - : cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" - ? cause.fileCleanupFailure.outcome.value - : undefined; + const completed = artifactFileCompletion(cause)?.value; const diagnostics = { path, stage: "installation_properties", diff --git a/src/gpu/GpuCodeObjectsProvider.ts b/src/gpu/GpuCodeObjectsProvider.ts index 99e18570d8..a56418b4bc 100644 --- a/src/gpu/GpuCodeObjectsProvider.ts +++ b/src/gpu/GpuCodeObjectsProvider.ts @@ -31,8 +31,7 @@ import { bindArtifactFileCleanup, artifactFilePrimaryFailure, artifactFileCancellationCause, - ArtifactFileCancelled, - ArtifactFileCleanupFailure, + artifactFileCompletion, } from "../artifacts/ArtifactFileOperation.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { @@ -146,10 +145,6 @@ export class GpuCodeObjectsProvider implements GpuCodeObjectsPort { error = new AnalysisCancelledError(OPERATION, { cause: artifactFileCancellationCause(primary), }); - else if (primary instanceof ArtifactFileCancelled) - error = new AnalysisCancelledError(OPERATION, { - cause: primary.fileCancellation.reason, - }); else if ( primary instanceof ArtifactReaderFailure && primary.reason === "integrity" @@ -209,16 +204,10 @@ export class GpuCodeObjectsProvider implements GpuCodeObjectsPort { ); error = analysisErrorWithCleanupFailure(error, cleanup, OPERATION); } - const completed = - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" - ? cause.fileCleanupFailure.outcome.value - : cause instanceof ArtifactFileCancelled - ? cause.fileCancellation.outcome.value - : undefined; + const completed = artifactFileCompletion(cause); if (completed !== undefined) { // The descriptor owner retains observations even when its final close fails. - const report = gpuCodeObjectsSchema.safeParse(completed); + const report = gpuCodeObjectsSchema.safeParse(completed.value); if (report.success) error.retainPartialObservation( createGpuCodeObjectsEvidence(report.data, this.identity, input), diff --git a/src/il2cpp/Il2cppMetadataProvider.ts b/src/il2cpp/Il2cppMetadataProvider.ts index 1b7668a905..9017ae3e2a 100644 --- a/src/il2cpp/Il2cppMetadataProvider.ts +++ b/src/il2cpp/Il2cppMetadataProvider.ts @@ -10,9 +10,8 @@ import { import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { - ArtifactFileCleanupFailure, artifactFilePrimaryFailure, - ArtifactFileCancelled, + artifactFileCompletion, bindArtifactFileCleanup, artifactFileCancellationCause, assertArtifactReadActive, @@ -593,16 +592,11 @@ export class Il2cppMetadataProvider implements Il2cppMetadataPort { observe(sourceArtifact(path, snapshot)); return snapshot; } catch (cause) { - const outcome = - cause instanceof ArtifactFileCleanupFailure - ? (cause as ArtifactFileCleanupFailure) - .fileCleanupFailure.outcome - : cause instanceof ArtifactFileCancelled - ? (cause as ArtifactFileCancelled) - .fileCancellation.outcome - : undefined; - if (outcome?.kind === "completed") - observe(sourceArtifact(path, outcome.value)); + const completion = artifactFileCompletion(cause); + if (completion !== undefined) + observe( + sourceArtifact(path, completion.value as StableArtifactSnapshot), + ); throw cause; } } diff --git a/src/model/ModelArtifactProvider.ts b/src/model/ModelArtifactProvider.ts index c29f102a67..69ac27fe5b 100644 --- a/src/model/ModelArtifactProvider.ts +++ b/src/model/ModelArtifactProvider.ts @@ -32,8 +32,7 @@ import { bindArtifactFileCleanup, artifactFilePrimaryFailure, artifactFileCancellationCause, - ArtifactFileCancelled, - ArtifactFileCleanupFailure, + artifactFileCompletion, } from "../artifacts/ArtifactFileOperation.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { ModelHeaderReader } from "./ModelHeaderReader.js"; @@ -158,10 +157,6 @@ export class ModelArtifactProvider implements ModelArtifactPort { error = new AnalysisCancelledError(OPERATION, { cause: artifactFileCancellationCause(primary), }); - else if (primary instanceof ArtifactFileCancelled) - error = new AnalysisCancelledError(OPERATION, { - cause: primary.fileCancellation.reason, - }); else if ( primary instanceof ArtifactReaderFailure && primary.reason === "integrity" @@ -221,16 +216,10 @@ export class ModelArtifactProvider implements ModelArtifactPort { ); error = analysisErrorWithCleanupFailure(error, cleanup, OPERATION); } - const completed = - cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" - ? cause.fileCleanupFailure.outcome.value - : cause instanceof ArtifactFileCancelled - ? cause.fileCancellation.outcome.value - : undefined; + const completed = artifactFileCompletion(cause); if (completed !== undefined) { // The descriptor owner retains observations even when its final close fails. - const report = modelArtifactSchema.safeParse(completed); + const report = modelArtifactSchema.safeParse(completed.value); if (report.success) error.retainPartialObservation( createModelArtifactEvidence(report.data, this.identity, input), diff --git a/src/sqlite/SqliteDatabaseFailures.ts b/src/sqlite/SqliteDatabaseFailures.ts index 140f53c294..fe70ff238a 100644 --- a/src/sqlite/SqliteDatabaseFailures.ts +++ b/src/sqlite/SqliteDatabaseFailures.ts @@ -120,10 +120,7 @@ export const sqliteDatabaseFailure = ( undefined, workerTimeoutMs, ); - else if (signal?.aborted) - previous = new AnalysisCancelledError(operation, { - cause: signal.reason, - }); + const error = createSqliteCleanupError( cause.cleanup.resources, { diff --git a/src/sqlite/SqliteDatabaseWorkerExecution.ts b/src/sqlite/SqliteDatabaseWorkerExecution.ts index acabd5666a..450d13a1c4 100644 --- a/src/sqlite/SqliteDatabaseWorkerExecution.ts +++ b/src/sqlite/SqliteDatabaseWorkerExecution.ts @@ -1,6 +1,5 @@ import { bindArtifactFileCleanup, - ArtifactFileCleanupFailure, artifactFileCancellationCause, } from "../artifacts/ArtifactFileOperation.js"; import { randomUUID } from "node:crypto"; @@ -219,17 +218,11 @@ export const executeSqliteDatabaseWorker = async (request: { }; if ( cause instanceof ArtifactReaderFailure && - (cause.reason === "cancelled" || - (cause instanceof ArtifactFileCleanupFailure && - cause.fileCleanupFailure.outcome.kind === "completed" && - request.signal?.aborted)) + cause.reason === "cancelled" ) { const error = new AnalysisCancelledError(operation, { ...options, - cause: - cause.reason === "cancelled" - ? artifactFileCancellationCause(cause) - : request.signal?.reason, + cause: artifactFileCancellationCause(cause), }); bindArtifactFileCleanup(cause, error); throw error; diff --git a/tests/boundary/filesystem/stableArtifact.test.ts b/tests/boundary/filesystem/stableArtifact.test.ts index 97b0b57180..0ce9fcd64f 100644 --- a/tests/boundary/filesystem/stableArtifact.test.ts +++ b/tests/boundary/filesystem/stableArtifact.test.ts @@ -11,6 +11,7 @@ import { createHash } from "node:crypto"; import { expect, it, onTestFinished } from "vitest"; import { readStableArtifact } from "../../../src/artifacts/readStableArtifact.js"; import { + artifactFileCompletion, ArtifactFileCleanupFailure, ArtifactFileCancelled, withArtifactFileHandle, @@ -136,12 +137,13 @@ it.each(["scoped", "transferred"])( throw new Error("Missing descriptor owner"); expect(failure.fileCleanupFailure.owner.handle).toBe(file.handle); expect(failure.fileCleanupFailure.cleanupCause).toBe(file.closeFailure); - expect(failure.fileCleanupFailure.outcome).toEqual({ + expect(artifactFileCompletion(failure)).toEqual({ kind: "completed", value: { bytes: file.bytes, sha256: createHash("sha256").update(file.bytes).digest("hex"), }, + cancellation: undefined, }); expect(file.handle.fd).toBeGreaterThanOrEqual(0); expect(failure.cleanup?.resources).toEqual([file.path]); @@ -175,6 +177,7 @@ it.each([new Error("primary read failure"), undefined, null])( cause: primary, }); expect(failure.fileCleanupFailure.cleanupCause).toBe(file.closeFailure); + expect(artifactFileCompletion(failure)).toBeUndefined(); file.release(); await scope.close(); expect(file.handle.fd).toBe(-1); @@ -198,7 +201,11 @@ it("retains physical read facts and the caller's reason when cancellation arrive if (!(failure instanceof ArtifactFileCancelled)) throw new Error("Missing late cancellation"); expect(failure.cause).toBe(reason); - expect(failure.fileCancellation.outcome.value.bytes).toEqual(file.bytes); + expect(artifactFileCompletion(failure)).toMatchObject({ + kind: "completed", + value: { bytes: file.bytes }, + cancellation: { reason }, + }); expect(file.handle.fd).toBe(-1); }); @@ -218,9 +225,10 @@ it("retains a completed read and failed cleanup when cancellation arrives during ).catch((cause: unknown) => cause); if (!(failure instanceof ArtifactFileCleanupFailure)) throw new Error("Missing cleanup ownership"); - expect(failure.fileCleanupFailure.outcome).toMatchObject({ + expect(artifactFileCompletion(failure)).toMatchObject({ kind: "completed", value: { bytes: file.bytes }, + cancellation: { reason }, }); expect(controller.signal.reason).toBe(reason); file.handle.close = file.nativeClose; @@ -283,7 +291,7 @@ it("does not turn a retention callback abort into completed-file cancellation", kind: "completed", value: file.bytes, }); - expect(failure.fileCleanupFailure.completionCancellation).toBeUndefined(); + expect(artifactFileCompletion(failure)?.cancellation).toBeUndefined(); expect(failure.reason).not.toBe("cancelled"); file.release(); await failure.cleanupLease.release(); @@ -293,3 +301,29 @@ it("does not turn a retention callback abort into completed-file cancellation", await scope.close(); } }); + +it("keeps a completed void operation distinct from a failed operation and arbitrary error payloads", async () => { + const file = await heldDescriptor(); + const scope = new ArtifactResourceScope(); + try { + const failure: unknown = await withArtifactFileHandle( + file.path, + file.handle, + scope, + undefined, + async () => undefined, + ).catch((cause: unknown) => cause); + expect(artifactFileCompletion(failure)).toEqual({ + kind: "completed", + value: undefined, + cancellation: undefined, + }); + expect(artifactFileCompletion({ cause: failure })).toBeUndefined(); + file.release(); + await scope.close(); + expect(file.handle.fd).toBe(-1); + } finally { + file.release(); + await scope.close(); + } +}); diff --git a/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts b/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts index 97fd11aabd..8b1c824240 100644 --- a/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts +++ b/tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts @@ -6,9 +6,12 @@ import { fileURLToPath } from "node:url"; import { expect, it, onTestFinished, vi } from "vitest"; import { SqliteDatabaseService } from "../../../src/application/sqlite/SqliteDatabaseService.js"; +import { ArtifactFileCleanupFailure } from "../../../src/artifacts/ArtifactFileOperation.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { registerSqliteCommands } from "../../../src/cli/sqliteCommands.js"; import { createSqliteDatabaseService } from "../../../src/composition/sqlite.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { AnalysisOutputError } from "../../../src/domain/analysisErrorCore.js"; import { silentLogger } from "../../../src/logger.js"; import { SqliteDatabaseProvider } from "../../../src/sqlite/SqliteDatabaseProvider.js"; import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; @@ -327,6 +330,68 @@ it("retains reply-read cancellation and retries the failed reply descriptor clos await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); }); +it("keeps a completed reply close failure primary when abort follows lease retention", async () => { + const selected = await fixture("reply"); + const controller = new AbortController(); + const reason = { requested: "abort after reply close failure capture" }; + let retainedReplyLease: string | undefined; + const retain = ArtifactResourceScope.prototype.retain; + const retainSpy = vi + .spyOn(ArtifactResourceScope.prototype, "retain") + .mockImplementation(function (this: ArtifactResourceScope, owner) { + retain.call(this, owner); + if (owner.kind === "lease" && owner.resource === descriptorCleanup.path) { + retainedReplyLease = owner.resource; + controller.abort(reason); + } + }); + + try { + const result = await selected.provider.inspect( + { path: selected.database.path }, + { signal: controller.signal }, + ); + expect(controller.signal.aborted).toBe(true); + expect(controller.signal.reason).toBe(reason); + expect(retainedReplyLease).toBe(descriptorCleanup.path); + expect(result.ok).toBe(false); + if (result.ok) return; + + const primary = + result.error instanceof AnalysisOutputError + ? result.error + : result.error.cleanupPrimary; + expect(primary).toBeInstanceOf(AnalysisOutputError); + if (!(primary instanceof AnalysisOutputError)) return; + expect(primary).not.toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(primary.cause).toBeInstanceOf(ArtifactFileCleanupFailure); + expect(primary.cause).toMatchObject({ + fileCleanupFailure: { + path: descriptorCleanup.path, + outcome: { kind: "completed" }, + completionCancellation: undefined, + }, + }); + expect(primary.cleanupIncomplete).toBe(true); + expect(primary.cleanupResources).toContain(descriptorCleanup.path); + + const handle = descriptorCleanup.handles[0]; + if (handle === undefined) + throw new Error("Expected owned reply descriptor"); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(descriptorCleanup.attempts).toBe(1); + + descriptorCleanup.denied = false; + await selected.provider.close(); + expect(descriptorCleanup.attempts).toBe(2); + expect(handle.fd).toBe(-1); + expect(primary.cleanupIncomplete).toBe(false); + expect(primary).toBeInstanceOf(AnalysisOutputError); + } finally { + retainSpy.mockRestore(); + } +}); + it("retries source cleanup before admitting another snapshot", async () => { const selected = await fixture("database"); await selected.provider.inspect({ path: selected.database.path }); diff --git a/tests/boundary/sqlite/sqliteDatabaseFailures.test.ts b/tests/boundary/sqlite/sqliteDatabaseFailures.test.ts index cf41ca2c1f..69d78aaa9d 100644 --- a/tests/boundary/sqlite/sqliteDatabaseFailures.test.ts +++ b/tests/boundary/sqlite/sqliteDatabaseFailures.test.ts @@ -1,3 +1,4 @@ +import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js"; import { expect, it } from "vitest"; import { OwnedCommandFailure } from "../../../src/process/OwnedCommand.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; @@ -104,3 +105,23 @@ it("says diagnostic capture has no runtime override", () => { "worker_timeout_ms does not raise it", ); }); + +it("does not infer cancellation from a later signal when cleanup carries no cancellation fact", () => { + const controller = new AbortController(); + const cause = new ArtifactReaderFailure("io", "cleanup remains owned", { + cleanup: { reason: "close failed", resources: ["/owned/reply.json"] }, + }); + controller.abort(new Error("later caller cancellation")); + const failure = sqliteDatabaseFailure( + cause, + "/selected.db", + "worker", + controller.signal, + ); + expect(failure.cleanupIncomplete).toBe(true); + expect(failure.cleanupPrimary).toBeUndefined(); + expect(projectAnalysisError(failure)).toMatchObject({ + code: "cleanup_incomplete", + details: { resources: ["/owned/reply.json"] }, + }); +}); diff --git a/tests/process-global/apkSignatureLifecycle.test.ts b/tests/process-global/apkSignatureLifecycle.test.ts index 63d074e2a1..bca360c835 100644 --- a/tests/process-global/apkSignatureLifecycle.test.ts +++ b/tests/process-global/apkSignatureLifecycle.test.ts @@ -8,6 +8,9 @@ import { vi } from "vitest"; const fault = vi.hoisted(() => ({ path: "", failClose: "never" as "never" | "once" | "always", + abortAtClose: undefined as + | { controller: AbortController; reason: unknown } + | undefined, abortAtStat: undefined as | { controller: AbortController; reason: unknown } | undefined, @@ -40,6 +43,9 @@ vi.mock("node:fs/promises", async (importOriginal) => { handle.close = async () => { if (fault.failClose !== "never") { if (fault.failClose === "once") fault.failClose = "never"; + const abort = fault.abortAtClose; + fault.abortAtClose = undefined; + abort?.controller.abort(abort.reason); throw new Error("controlled descriptor close failure"); } return close(); @@ -50,6 +56,7 @@ vi.mock("node:fs/promises", async (importOriginal) => { }); import { ApkSignatureProvider } from "../../src/apkSignature/ApkSignatureProvider.js"; +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; import { createCli } from "../../src/cli.js"; import { CLI_COMMANDS } from "../../src/cliCommandNames.js"; import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; @@ -74,6 +81,7 @@ beforeAll(async () => { afterEach(() => { fault.path = ""; fault.failClose = "never"; + fault.abortAtClose = undefined; fault.abortAtStat = undefined; fault.pauseAtStat = undefined; process.exitCode = exitCode; @@ -169,6 +177,77 @@ it("reports a completed read whose close failed, and a closed scope, as executio expect(projectAnalysisError(closed.error).code).toBe("execution_failure"); }); +it("keeps a post-capture abort from reclassifying a completed close failure", async () => { + const provider = new ApkSignatureProvider(); + const controller = new AbortController(); + const reason = { requested: "abort after close failure was captured" }; + const retain = ArtifactResourceScope.prototype.retain; + const retainSpy = vi + .spyOn(ArtifactResourceScope.prototype, "retain") + .mockImplementation(function (this: ArtifactResourceScope, owner) { + retain.call(this, owner); + if (owner.kind === "lease" && owner.resource === path) + controller.abort(reason); + }); + fault.path = path; + fault.failClose = "once"; + try { + const result = await provider.inspect( + { path }, + { signal: controller.signal }, + ); + expect(controller.signal.aborted).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toMatchObject({ cleanupIncomplete: true }); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [path], + diagnostics: { primary_error: { code: "execution_failure" } }, + }, + }); + + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + retainSpy.mockRestore(); + await provider.close(); + } +}); + +it("preserves cancellation observed during a completed read's failed close", async () => { + const provider = new ApkSignatureProvider(); + const controller = new AbortController(); + const reason = { requested: "abort during descriptor close" }; + fault.path = path; + fault.failClose = "once"; + fault.abortAtClose = { controller, reason }; + try { + const result = await provider.inspect( + { path }, + { signal: controller.signal }, + ); + expect(controller.signal.aborted).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toMatchObject({ cleanupIncomplete: true }); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [path], + diagnostics: { primary_error: { code: "cancelled" } }, + }, + }); + expect(result.error.cleanupPrimary?.cause).toBe(reason); + + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + await provider.close(); + } +}); + it("projects a provider close that still fails through the public CLI", async () => { fault.path = path; fault.failClose = "always";