From 2732d538e8ea77f5eed910f59bfc7a03e8fdcf9e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:40:14 +0800 Subject: [PATCH] fix(session): own target admission descriptors through finalization --- src/application/BinaryTargetResolver.ts | 523 ++++++++++++------ src/application/DirectAnalysis.ts | 22 +- src/application/binary/BinarySession.ts | 40 +- src/application/binary/BinarySessionOpen.ts | 3 + .../binary/BinarySessionRecords.ts | 41 +- .../filesystem/binaryTargetCleanup.test.ts | 244 ++++++++ .../binaryTargetAdmissionCancellation.test.ts | 35 +- 7 files changed, 695 insertions(+), 213 deletions(-) create mode 100644 tests/boundary/filesystem/binaryTargetCleanup.test.ts diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index c1c361715e..2ed5a57966 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -1,4 +1,19 @@ -import { regularFileCancellation } from "../filesystem/OwnedRegularFile.js"; +import { + regularFileCancellation, + RegularFileCleanupFailure, + RegularFileCancelled, + withOwnedRegularFile, + OwnedRegularFileOperationFailure, +} from "../filesystem/OwnedRegularFile.js"; +import { + bindRegularFileCleanup, + RetainedRegularFileCleanupFailure, + retryRegularFileCleanup, +} from "./RegularFileRead.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { jsonValueSchema } from "../domain/jsonValue.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { constants, type Stats } from "node:fs"; import { createHash } from "node:crypto"; import { @@ -11,10 +26,7 @@ import { import { extname } from "node:path"; import { BinaryTargetError } from "../domain/configurationErrors.js"; -import { - openRegularFile, - sameRegularFileState, -} from "../filesystem/RegularFile.js"; +import { sameRegularFileState } from "../filesystem/RegularFile.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -81,6 +93,7 @@ export interface BinaryTargetResolverOptions { readonly architecture?: BinaryArchitecture; readonly entryPoint?: string; readonly signal?: AbortSignal; + readonly resources?: ArtifactResourceScope; } /** @@ -96,13 +109,12 @@ export const parseBinaryTarget = async ( Result > => { const cwd = options.cwd ?? process.cwd(); - const hostArchitecture = options.hostArchitecture ?? process.arch; - const { targetKind, formatHint, signal, architecture, entryPoint } = options; + const { signal } = options; const candidate = absoluteCallerPath(input, { cwd }); // Failures before canonicalization concern the selected path itself; later // failures concern the canonical path opened with symbolic links rejected. let stage: TargetFilesystemStage = "resolve"; - try { + const resolve = async (): Promise => { throwIfTargetResolutionCancelled(signal); await access(candidate, constants.R_OK); throwIfTargetResolutionCancelled(signal); @@ -117,165 +129,344 @@ export const parseBinaryTarget = async ( const { executable: path, infoPlist } = resolved.value; const bundle = infoPlist === undefined ? {} : { bundleInfoPlist: infoPlist }; - const handle = await openRegularFile(path, { - symlinks: "reject", - signal, - }); - try { - throwIfTargetResolutionCancelled(signal); - const metadata = await handle.stat(); - throwIfTargetResolutionCancelled(signal); - if (formatHint === "raw-binary") { - if (targetKind !== undefined && targetKind !== "executable") - return err( - new BinaryTargetError( - path, - "raw-binary format requires an executable target kind", - { constraint: "load_selection_rejected" }, - ), + return withOwnedRegularFile( + path, + { + symlinks: "reject", + ...(signal === undefined ? {} : { signal }), + }, + async (handle) => { + try { + return await readOpenedBinaryTarget( + handle, + { path, sourcePath, ...bundle }, + options, ); - const admitted = admitRawBinary({ - architecture, - entryPoint, - fileBytes: metadata.size, - }); - if (!admitted.ok) - return err( - new BinaryTargetError(path, admitted.error.reason, { - constraint: admitted.error.constraint, - }), - ); - return ok({ - path, - sourcePath, - ...bundle, - sha256: await sha256Handle(handle, path, metadata, signal), - kind: "executable", - format: "raw-binary", - architecture: admitted.value.architecture, - availableArchitectures: [admitted.value.architecture], - entryPoint: admitted.value.entryPoint, - entryPointSource: admitted.value.entryPointSource, - }); - } - const pairing = rawBinarySelectionError({ - ...(formatHint === undefined ? {} : { format: formatHint }), - architecture, - entryPoint, - }); - if (pairing !== undefined) + } catch (cause: unknown) { + throw new OwnedRegularFileOperationFailure(cause); + } + }, + ); + }; + try { + const resolved = await (options.resources === undefined + ? resolve() + : options.resources.run(resolve)); + return finalizedTargetResolution(resolved, signal); + } catch (cause: unknown) { + // A caller-selected abort value never transfers file ownership. + if (signal?.aborted === true && cause === signal.reason) + return err( + new AnalysisCancelledError("resolve_binary_target", { cause }), + ); + if (cause instanceof RegularFileCleanupFailure) { + const fileFailure = cause as RegularFileCleanupFailure; + if (options.resources === undefined) return err( - new BinaryTargetError(path, pairing, { - constraint: "load_selection_rejected", - }), + targetFileCleanupError( + new RetainedRegularFileCleanupFailure(fileFailure), + candidate, + stage, + ), ); - if (formatHint === "dos-com") { - if (targetKind !== undefined && targetKind !== "executable") - return err( - new BinaryTargetError( - path, - "DOS COM format requires an executable target kind", - { constraint: "load_selection_rejected" }, - ), - ); - const length = validateDosComLength(metadata.size); - if (!length.ok) - return err( - new BinaryTargetError(path, length.error, { - constraint: "file_format_rejected", - }), - ); - return ok({ - path, - sourcePath, - ...bundle, - sha256: await sha256Handle(handle, path, metadata, signal), - kind: "executable", - format: "dos-com", - architecture: "x86", - availableArchitectures: ["x86"], - }); - } - // Probe WASM content before suffix-only database routing; an explicit - // database selection keeps the caller's chosen interpretation. - const artifactFormat = - targetKind === "database" - ? undefined - : await detectArtifactFormat(path, handle, signal); - if ( - targetKind === "database" || - (targetKind === undefined && path.toLowerCase().endsWith(".hop")) - ) - return ok({ - path, - sourcePath, - sha256: await sha256Handle(handle, path, metadata, signal), - kind: "database", - format: "analysis-database", - }); - if (artifactFormat !== undefined) { - const identity = { - path, - sourcePath, - sha256: await sha256Handle(handle, path, metadata, signal), - }; - return isArchiveFormat(artifactFormat) - ? ok({ ...identity, kind: "archive", format: artifactFormat }) - : ok({ ...identity, kind: "artifact", format: artifactFormat }); - } - const detected = await readExecutableMetadata( - handle, - hostArchitecture, + const retried = await retryRegularFileCleanup( + fileFailure, + options.resources, signal, ); - if (!detected.ok) { - // Only bytes no other format claims may lead a located ZIP, so an - // executable with an appended archive keeps its executable route. - if ( - !(await claimsLeadingExecutableHeader( - handle, - metadata.size, - signal, - )) && - (await locateTrailingZipArchive(metadata.size, (position, length) => - readHandleRange(handle, position, length, signal), - )) !== undefined - ) - return ok({ - path, - sourcePath, - sha256: await sha256Handle(handle, path, metadata, signal), - kind: "archive", - format: zipRootPackageFormat(path), - }); - const reason = - typeof detected.error === "string" - ? detected.error - : detected.error.reason; - const diagnostics = - typeof detected.error === "string" - ? undefined - : detected.error.diagnostics; - return err( - new BinaryTargetError(path, reason, { - constraint: "file_format_rejected", - ...(diagnostics === undefined ? {} : { diagnostics }), - }), + if (retried.failure !== undefined) + return err(targetFileCleanupError(retried.failure, candidate, stage)); + if (retried.outcome.kind === "failed") + return targetResolutionFailure( + fileFailure.operationFailure ?? retried.outcome.cause, + candidate, + stage, + signal, ); - } + return finalizedTargetResolution(retried.outcome.value, signal); + } + if (cause instanceof RegularFileCancelled) + return finalizedTargetResolution( + cause.outcome.value as TargetResolution, + signal, + ); + if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) { + const error = new BinaryTargetError( + candidate, + "A previous target file could not be closed", + { + cause, + cleanup: cause.cleanup, + partialObservation: cause.partialObservation, + }, + ); + bindRegularFileCleanup(cause, error); + return err(error); + } + return targetResolutionFailure(cause, candidate, stage, signal); + } +}; + +type TargetResolution = Result< + BinaryTarget, + BinaryTargetError | AnalysisCancelledError +>; + +/** Classify and hash the one descriptor admitted by the lifetime owner. */ +const readOpenedBinaryTarget = async ( + handle: FileHandle, + selected: { + readonly path: string; + readonly sourcePath: string; + readonly bundleInfoPlist?: string; + }, + options: BinaryTargetResolverOptions, +): Promise => { + const { path, sourcePath, bundleInfoPlist } = selected; + const bundle = bundleInfoPlist === undefined ? {} : { bundleInfoPlist }; + const hostArchitecture = options.hostArchitecture ?? process.arch; + const { targetKind, formatHint, signal, architecture, entryPoint } = options; + throwIfTargetResolutionCancelled(signal); + const metadata = await handle.stat(); + throwIfTargetResolutionCancelled(signal); + if (formatHint === "raw-binary") { + if (targetKind !== undefined && targetKind !== "executable") + return err( + new BinaryTargetError( + path, + "raw-binary format requires an executable target kind", + { constraint: "load_selection_rejected" }, + ), + ); + const admitted = admitRawBinary({ + architecture, + entryPoint, + fileBytes: metadata.size, + }); + if (!admitted.ok) + return err( + new BinaryTargetError(path, admitted.error.reason, { + constraint: admitted.error.constraint, + }), + ); + return ok({ + path, + sourcePath, + ...bundle, + sha256: await sha256Handle(handle, path, metadata, signal), + kind: "executable", + format: "raw-binary", + architecture: admitted.value.architecture, + availableArchitectures: [admitted.value.architecture], + entryPoint: admitted.value.entryPoint, + entryPointSource: admitted.value.entryPointSource, + }); + } + const pairing = rawBinarySelectionError({ + ...(formatHint === undefined ? {} : { format: formatHint }), + architecture, + entryPoint, + }); + if (pairing !== undefined) + return err( + new BinaryTargetError(path, pairing, { + constraint: "load_selection_rejected", + }), + ); + if (formatHint === "dos-com") { + if (targetKind !== undefined && targetKind !== "executable") + return err( + new BinaryTargetError( + path, + "DOS COM format requires an executable target kind", + { constraint: "load_selection_rejected" }, + ), + ); + const length = validateDosComLength(metadata.size); + if (!length.ok) + return err( + new BinaryTargetError(path, length.error, { + constraint: "file_format_rejected", + }), + ); + return ok({ + path, + sourcePath, + ...bundle, + sha256: await sha256Handle(handle, path, metadata, signal), + kind: "executable", + format: "dos-com", + architecture: "x86", + availableArchitectures: ["x86"], + }); + } + // Probe WASM content before suffix-only database routing; an explicit + // database selection keeps the caller's chosen interpretation. + const artifactFormat = + targetKind === "database" + ? undefined + : await detectArtifactFormat(path, handle, signal); + if ( + targetKind === "database" || + (targetKind === undefined && path.toLowerCase().endsWith(".hop")) + ) + return ok({ + path, + sourcePath, + sha256: await sha256Handle(handle, path, metadata, signal), + kind: "database", + format: "analysis-database", + }); + if (artifactFormat !== undefined) { + const identity = { + path, + sourcePath, + sha256: await sha256Handle(handle, path, metadata, signal), + }; + return isArchiveFormat(artifactFormat) + ? ok({ ...identity, kind: "archive", format: artifactFormat }) + : ok({ ...identity, kind: "artifact", format: artifactFormat }); + } + const detected = await readExecutableMetadata( + handle, + hostArchitecture, + signal, + ); + if (!detected.ok) { + // Only bytes no other format claims may lead a located ZIP, so an + // executable with an appended archive keeps its executable route. + if ( + !(await claimsLeadingExecutableHeader(handle, metadata.size, signal)) && + (await locateTrailingZipArchive(metadata.size, (position, length) => + readHandleRange(handle, position, length, signal), + )) !== undefined + ) return ok({ path, sourcePath, - ...bundle, sha256: await sha256Handle(handle, path, metadata, signal), - kind: "executable", - ...detected.value, + kind: "archive", + format: zipRootPackageFormat(path), }); - } finally { - await handle.close(); - } - } catch (cause: unknown) { - if (cause instanceof AnalysisCancelledError) return err(cause); + const reason = + typeof detected.error === "string" + ? detected.error + : detected.error.reason; + const diagnostics = + typeof detected.error === "string" + ? undefined + : detected.error.diagnostics; + return err( + new BinaryTargetError(path, reason, { + constraint: "file_format_rejected", + ...(diagnostics === undefined ? {} : { diagnostics }), + }), + ); + } + return ok({ + path, + sourcePath, + ...bundle, + sha256: await sha256Handle(handle, path, metadata, signal), + kind: "executable", + ...detected.value, + }); +}; + +/** Preserve an earlier admission refusal over an interruption during final close. */ +const finalizedTargetResolution = ( + result: TargetResolution, + signal?: AbortSignal, +): TargetResolution => { + if (!result.ok || signal?.aborted !== true) return result; + return err( + new AnalysisCancelledError("resolve_binary_target", { + cause: signal.reason, + diagnostics: { completed_target: jsonValueSchema.parse(result.value) }, + }), + ); +}; + +/** Descriptor cleanup retains the physical result and binds its exact release lease. */ +const targetFileCleanupError = ( + failure: RetainedRegularFileCleanupFailure, + candidate: string, + stage: TargetFilesystemStage, +): BinaryTargetError => { + const cancellation = regularFileCancellation(failure); + const completed = + failure.outcome.kind === "completed" ? failure.outcome.value : undefined; + const completedTarget = + completed?.ok === true ? jsonValueSchema.parse(completed.value) : undefined; + let primary: BinaryTargetError | AnalysisCancelledError | undefined; + if (failure.outcome.kind === "failed") { + const physical = failure.outcome.cause; + primary = + physical instanceof BinaryTargetError || + physical instanceof AnalysisCancelledError + ? physical + : cancellation !== undefined + ? new AnalysisCancelledError("resolve_binary_target", { + cause: cancellation.reason, + }) + : (classifyTargetFilesystemFailure(candidate, physical, stage) ?? + new BinaryTargetError( + failure.path, + "Target read failed before file cleanup", + { cause: physical }, + )); + } else if (completed?.ok === false) primary = completed.error; + else if (cancellation !== undefined) + primary = new AnalysisCancelledError("resolve_binary_target", { + cause: cancellation.reason, + ...(completedTarget === undefined + ? {} + : { diagnostics: { completed_target: completedTarget } }), + }); + const error = new BinaryTargetError( + failure.path, + "The target file handle could not be closed", + { + cause: failure, + cleanupPrimary: primary, + cleanup: { + reason: + failure.cleanupCause instanceof Error + ? failure.cleanupCause.message + : String(failure.cleanupCause), + resources: [failure.path], + }, + diagnostics: { + ...(primary === undefined + ? {} + : { primary_error: projectAnalysisError(primary) }), + ...(completedTarget === undefined + ? {} + : { completed_target: completedTarget }), + }, + }, + ); + bindRegularFileCleanup(failure, error); + return error; +}; + +const targetResolutionFailure = ( + cause: unknown, + candidate: string, + stage: TargetFilesystemStage, + signal?: AbortSignal, +): TargetResolution => { + // Physical rejection values do not grant ownership of foreign descriptors. + const physical = + cause instanceof OwnedRegularFileOperationFailure + ? cause.operationCause + : cause; + if ( + physical instanceof AnalysisCancelledError || + physical instanceof BinaryTargetError + ) + return err(physical); + if (!(cause instanceof OwnedRegularFileOperationFailure)) { const cancellation = regularFileCancellation(cause); if (cancellation !== undefined) return err( @@ -283,16 +474,14 @@ export const parseBinaryTarget = async ( cause: cancellation.reason, }), ); - if (signal?.aborted === true && cause === signal.reason) - return err( - new AnalysisCancelledError("resolve_binary_target", { cause }), - ); - if (cause instanceof BinaryTargetError) return err(cause); - if (!(cause instanceof Error)) throw cause; - const failure = classifyTargetFilesystemFailure(candidate, cause, stage); - if (failure === undefined) throw cause; - return err(failure); } + if (signal?.aborted === true && physical === signal.reason) + return err( + new AnalysisCancelledError("resolve_binary_target", { cause: physical }), + ); + const failure = classifyTargetFilesystemFailure(candidate, physical, stage); + if (failure === undefined) throw physical; + return err(failure); }; /** @@ -359,7 +548,9 @@ const targetFailureReason = ( const throwIfTargetResolutionCancelled = (signal?: AbortSignal): void => { if (signal?.aborted === true) - throw new AnalysisCancelledError("resolve_binary_target"); + throw new AnalysisCancelledError("resolve_binary_target", { + cause: signal.reason, + }); }; const detectArtifactFormat = async ( diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index 166ebd140b..80c0cf9935 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -28,12 +28,8 @@ import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { access, stat } from "node:fs/promises"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; -import { readAnalysisSnapshot } from "./binary/AnalysisSnapshotFiles.js"; import { requiresNativeSourceAdmissionBeforeReplay } from "./binary/AnalysisSnapshotCache.js"; -import { - classifyBinaryTargetFilesystemFailure, - parseBinaryTarget, -} from "./BinaryTargetResolver.js"; +import { classifyBinaryTargetFilesystemFailure } from "./BinaryTargetResolver.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { snapshotEvidenceForQuery, @@ -341,6 +337,7 @@ const runAnalysis = async ( ); } const prepared = await prepareSnapshot({ + session, path, snapshotPath, signal, @@ -482,7 +479,7 @@ const runAnalysis = async ( const recovered = written.error instanceof EvidenceFileError && written.error.published !== undefined && - (await session.closeWriteResources()).ok; + (await session.closeFileResources()).ok; if (!recovered) return err(written.error.retainPartialObservation(evidence)); } @@ -653,6 +650,10 @@ const withProcessCancellation = async ( }; const prepareSnapshot = async (options: { + readonly session: Pick< + BinarySession, + "readAnalysisSnapshot" | "resolveTarget" + >; readonly path: string; readonly formatHint?: ExecutableFormatHint; readonly architecture?: BinaryArchitecture; @@ -665,12 +666,15 @@ const prepareSnapshot = async (options: { AnalysisError > > => { - const { path, snapshotPath } = options; + const { path, snapshotPath, session } = options; if (snapshotPath === undefined || !(await fileExists(snapshotPath))) return ok({}); - const loaded = await readAnalysisSnapshot(snapshotPath, options.signal); + const loaded = await session.readAnalysisSnapshot( + snapshotPath, + options.signal, + ); if (!loaded.ok) return loaded; - const target = await parseBinaryTarget(path, { + const target = await session.resolveTarget(path, { signal: options.signal, ...(options.formatHint === undefined ? {} diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index 0cc232ac69..340606c27b 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -53,6 +53,10 @@ import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { profilelessNativeDispatchProfile } from "./ProfilelessNativeWorkflowSnapshot.js"; import type { AnalysisSnapshot } from "../../domain/analysisSnapshot.js"; import { AnalysisSnapshotMismatchError } from "../../domain/evidenceErrors.js"; +import { + parseBinaryTarget, + type BinaryTargetResolverOptions, +} from "../BinaryTargetResolver.js"; import { resolveAppBundleResourceRoot, type ResolvedAppBundleResourceRoot, @@ -422,6 +426,17 @@ export class BinarySession return () => this.#availabilityListeners.delete(listener); } + /** Resolve target identity under this session's file cleanup owner. */ + resolveTarget( + path: string, + options: Omit = {}, + ) { + return parseBinaryTarget(path, { + ...options, + resources: this.fileResources, + }); + } + /** Resolve a target and provider profile without creating a provider client. */ previewTarget( target: BinaryTarget, @@ -464,6 +479,7 @@ export class BinarySession return this.#open( (admitted) => resolveSessionOpen({ + resources: this.fileResources, router: this.#providerRouter, current: this.#active, path, @@ -850,7 +866,7 @@ export class BinarySession }); if (clearRecords) this.clearSessionRecords(); this.#clearRuntimeAvailability(); - const fileResources = await this.closeWriteResources(); + const fileResources = await this.closeFileResources(); if (!fileResources.ok) onCleanupFailure?.(fileResources.error); const candidateResources = await this.#providerRouter.closeOwnedResources(); if (!candidateResources.ok) onCleanupFailure?.(candidateResources.error); @@ -865,20 +881,24 @@ export class BinarySession ]; let resourceFailure = resourceFailures[0]; for (const next of resourceFailures.slice(1)) - if (resourceFailure !== undefined) + if (resourceFailure !== undefined) { + const previous = resourceFailure; resourceFailure = analysisErrorWithCleanupFailure( - resourceFailure, + previous, next, "close_binary", ); - if (!closed.ok && resourceFailure !== undefined) - return err( - analysisErrorWithCleanupFailure( - closed.error, - resourceFailure, - "close_binary", - ), + resourceFailure.bindCleanupDependencies([previous, next]); + } + if (!closed.ok && resourceFailure !== undefined) { + const combined = analysisErrorWithCleanupFailure( + closed.error, + resourceFailure, + "close_binary", ); + combined.bindCleanupDependencies([closed.error, resourceFailure]); + return err(combined); + } if (!closed.ok) return closed; return resourceFailure === undefined ? ok(null) : err(resourceFailure); } diff --git a/src/application/binary/BinarySessionOpen.ts b/src/application/binary/BinarySessionOpen.ts index 2d633b15e1..8b0252f7e7 100644 --- a/src/application/binary/BinarySessionOpen.ts +++ b/src/application/binary/BinarySessionOpen.ts @@ -9,6 +9,7 @@ import { } from "../../domain/analysisSnapshot.js"; import type { AnalysisSnapshot } from "../../domain/analysisSnapshot.js"; import { parseBinaryTarget } from "../BinaryTargetResolver.js"; +import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import type { BinaryTarget } from "../../domain/binaryTargetTypes.js"; import { EvidenceIntegrityError, @@ -56,6 +57,7 @@ export interface ResolvedSessionOpen { } interface ResolveSessionOpenInput { + readonly resources: ArtifactResourceScope; readonly router: SessionProviderRouter; readonly current: CurrentOpenBinding | undefined; readonly path: string; @@ -83,6 +85,7 @@ export const resolveSessionOpen = async ( ): Promise> => { const { path, options } = input; const parsed = await parseBinaryTarget(path, { + resources: input.resources, ...(options.targetKind === undefined ? {} : { targetKind: options.targetKind }), diff --git a/src/application/binary/BinarySessionRecords.ts b/src/application/binary/BinarySessionRecords.ts index e447d05277..5942a164f1 100644 --- a/src/application/binary/BinarySessionRecords.ts +++ b/src/application/binary/BinarySessionRecords.ts @@ -33,7 +33,12 @@ import { AnalysisSnapshotCache } from "./AnalysisSnapshotCache.js"; import { InvestigationRecords } from "../investigation/InvestigationRecords.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; -import { writeAnalysisSnapshot as writeSnapshotFile } from "./AnalysisSnapshotFiles.js"; +import { bindRegularFileCleanup } from "../RegularFileRead.js"; +import { + writeAnalysisSnapshot as writeSnapshotFile, + readAnalysisSnapshot as readSnapshotFile, +} from "./AnalysisSnapshotFiles.js"; +import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; import { workflowAnalysisProfile } from "../WorkflowAnalysisProfile.js"; import { hasOnlyProfilelessNativeDispatchEntries, @@ -63,10 +68,10 @@ export interface WorkflowSnapshotRecordInput { /** Own binary snapshots and coordinate the composed investigation records. */ export abstract class BinarySessionRecords { readonly #records: InvestigationRecords; - readonly #writeResources = new ArtifactResourceScope(); + protected readonly fileResources = new ArtifactResourceScope(); protected get hasPendingFileResources(): boolean { - return this.#writeResources.hasPending; + return this.fileResources.hasPending; } readonly #snapshot = new AnalysisSnapshotCache(); #snapshotInvalidated = false; @@ -112,6 +117,11 @@ export abstract class BinarySessionRecords { return this.#records.immutableEvidenceBundle(); } + /** Read one snapshot while retaining failed descriptors under this session. */ + readAnalysisSnapshot(path: string, signal?: AbortSignal) { + return readSnapshotFile(path, signal, this.fileResources); + } + /** Persist one provider-neutral snapshot under this session's resource owner. */ writeAnalysisSnapshot( snapshot: AnalysisSnapshot, @@ -120,28 +130,27 @@ export abstract class BinarySessionRecords { ): Promise< Result<{ readonly path: string; readonly bytes: number }, AnalysisError> > { - return writeSnapshotFile(snapshot, path, overwrite, this.#writeResources); + return writeSnapshotFile(snapshot, path, overwrite, this.fileResources); } - /** Retry and release staged snapshot files at the owning session shutdown. */ - async closeWriteResources(): Promise> { + /** Retry retained target reads and snapshot files at the owning session shutdown. */ + async closeFileResources(): Promise> { try { - await this.#writeResources.retryCleanup(); + await this.fileResources.retryCleanup(); return ok(null); } catch (cause: unknown) { const observation = ArtifactReaderFailure.cleanupObservation( cause, - "analysis snapshot staging", + "analysis session files", ); - return err( - new EvidenceFileError("write", "io", { - cause, - cleanup: { - reason: observation.reason, - resources: observation.resources, - }, - }), + const error = new ProviderCleanupError( + "rea", + observation.resources, + { reason: observation.reason }, + { cause, operation: "close_binary" }, ); + bindRegularFileCleanup(cause, error); + return err(error); } } diff --git a/tests/boundary/filesystem/binaryTargetCleanup.test.ts b/tests/boundary/filesystem/binaryTargetCleanup.test.ts new file mode 100644 index 0000000000..3a77c35687 --- /dev/null +++ b/tests/boundary/filesystem/binaryTargetCleanup.test.ts @@ -0,0 +1,244 @@ +import { createHash } from "node:crypto"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, vi } from "vitest"; + +import { parseBinaryTarget } from "../../../src/application/BinaryTargetResolver.js"; +import type { AnalysisOperation } from "../../../src/application/AnalysisProvider.js"; +import { RetainedRegularFileCleanupFailure } from "../../../src/application/RegularFileRead.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; +import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; +import type { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { ok, err } from "../../../src/domain/result.js"; +import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { + createCacheProvider, + createTestBinarySession, +} from "../../fixtures/binarySession.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { observed } from "../../fixtures/analysisExecution.js"; + +it.each( + [true, false].flatMap((supported) => + [true, false].map((cancel) => ({ supported, cancel })), + ), +)( + "retains target admission and one descriptor through failed close (supported=$supported, cancelled=$cancel)", + async ({ supported, cancel }) => { + const directory = await createTestTempDirectory("rea-target-close-"); + const path = join(directory, supported ? "target.hop" : "target.bin"); + const bytes = Buffer.from("target source bytes"); + await writeFile(path, bytes); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { caller: "cancel during target close" }; + const nativeClose = OwnedFileHandle.prototype.close; + let denied = true; + const owners: OwnedFileHandle[] = []; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(async function (this: OwnedFileHandle) { + owners.push(this); + if (cancel) controller.abort(reason); + if (denied) throw new Error("controlled target close denial"); + await nativeClose.call(this); + }); + try { + const result = await parseBinaryTarget(path, { + resources, + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected retained target cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cause).toBeInstanceOf( + RetainedRegularFileCleanupFailure, + ); + const failure = result.error.cause as RetainedRegularFileCleanupFailure; + expect(failure.owner).toBe(owners[0]); + if (supported) { + expect(result.error.diagnostics).toMatchObject({ + completed_target: { + path, + format: "analysis-database", + sha256: createHash("sha256").update(bytes).digest("hex"), + }, + }); + if (cancel) { + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "AnalysisCancelledError", + }); + expect(result.error.cleanupPrimary?.cause).toBe(reason); + } + } else { + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "BinaryTargetError", + constraint: "file_format_rejected", + }); + } + const blocked = await parseBinaryTarget(path, { resources }); + if (blocked.ok) + throw new Error( + "Expected unresolved cleanup to block another admission", + ); + expect(owners).toHaveLength(3); + expect(owners.every((owner) => owner === failure.owner)).toBe(true); + denied = false; + await resources.retryCleanup(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(blocked.error.cleanupIncomplete).toBe(false); + expect(failure.owner.handle.fd).toBe(-1); + if (supported && cancel) + expect(result.error.cleanupPrimary?.diagnostics).toMatchObject({ + completed_target: { path }, + }); + expect((await parseBinaryTarget(path, { resources })).ok).toBe(supported); + } finally { + denied = false; + await resources.close(); + close.mockRestore(); + } + }, +); + +it.each([true, false])( + "keeps target completion or refusal when cancellation arrives during confirmed close (supported=%s)", + async (supported) => { + const directory = await createTestTempDirectory("rea-target-final-cancel-"); + const path = join(directory, supported ? "target.hop" : "target.bin"); + await writeFile(path, "target source bytes"); + const controller = new AbortController(); + const reason = "caller stopped after reading"; + const nativeClose = OwnedFileHandle.prototype.close; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(async function (this: OwnedFileHandle) { + await nativeClose.call(this); + controller.abort(reason); + }); + try { + const result = await parseBinaryTarget(path, { + signal: controller.signal, + }); + if (result.ok) throw new Error("Expected refusal or cancellation"); + expect(result.error.cleanupIncomplete).toBe(false); + if (supported) { + expect(result.error).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + diagnostics: { completed_target: { path } }, + }); + } else { + expect(result.error).toMatchObject({ + _tag: "BinaryTargetError", + constraint: "file_format_rejected", + }); + } + } finally { + close.mockRestore(); + } + }, +); + +it("retries target and provider owners independently through the same MCP connection", async () => { + const directory = await createTestTempDirectory("rea-target-mcp-close-"); + const first = join(directory, "first.hop"); + const second = join(directory, "second.hop"); + await writeFile(first, "first database bytes"); + await writeFile(second, "second database bytes"); + const provider = createCacheProvider([]); + const providerFailure = new ProviderCleanupError( + "fixture", + ["provider-owned-resource"], + { reason: "controlled provider close denial" }, + ); + let providerAllowed = false; + const execute = vi.fn(async (operation: AnalysisOperation) => { + if (operation === "health") return observed(null); + throw new Error("No analysis command should run"); + }); + provider.createClient = vi.fn(() => ({ + execute, + close: async () => { + if (!providerAllowed) return err(providerFailure); + providerFailure.resolveCleanup(); + return ok(null); + }, + })); + const session = createTestBinarySession(provider); + const opened = await session.open(first); + if (!opened.ok) throw opened.error; + const harness = await createApplicationMcpHarness({ session }); + const closeErrors: AnalysisError[] = []; + const sessionClose = session.close.bind(session); + vi.spyOn(session, "close").mockImplementation(async (...args) => { + const result = await sessionClose(...args); + if (!result.ok) closeErrors.push(result.error); + return result; + }); + const nativeClose = OwnedFileHandle.prototype.close; + let fileAllowed = false; + const owners: OwnedFileHandle[] = []; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(async function (this: OwnedFileHandle) { + owners.push(this); + if (!fileAllowed) throw new Error("controlled target close denial"); + await nativeClose.call(this); + }); + try { + const refused = await harness.client.callTool({ + name: "open_binary", + arguments: { path: second }, + }); + expect(parseMcpToolError(refused).error).toMatchObject({ + code: "cleanup_incomplete", + details: { diagnostics: { completed_target: { path: second } } }, + }); + expect(provider.createClient).toHaveBeenCalledTimes(1); + expect(execute).toHaveBeenCalledTimes(1); + expect(execute.mock.calls[0]?.[0]).toBe("health"); + expect(await session.status()).toMatchObject({ cleanup_pending: true }); + await harness.client.ping(); + const firstClose = await harness.client.callTool({ + name: "close_binary", + arguments: {}, + }); + expect(parseMcpToolError(firstClose).error.code).toBe("cleanup_incomplete"); + const aggregate = closeErrors[0]; + if (aggregate === undefined) + throw new Error("Expected mixed cleanup aggregate"); + expect(aggregate.cleanupResources).toEqual( + expect.arrayContaining([second, "provider-owned-resource"]), + ); + fileAllowed = true; + const secondClose = await harness.client.callTool({ + name: "close_binary", + arguments: {}, + }); + expect(parseMcpToolError(secondClose).error.code).toBe( + "cleanup_incomplete", + ); + expect(aggregate.cleanupIncomplete).toBe(true); + expect(aggregate.cleanupResources).toEqual(["provider-owned-resource"]); + const completedFileCalls = owners.length; + providerAllowed = true; + const released = await harness.client.callTool({ + name: "close_binary", + arguments: {}, + }); + expect(released.isError).not.toBe(true); + expect(owners).toHaveLength(completedFileCalls); + expect(owners.every((owner) => owner === owners[0])).toBe(true); + expect(owners[0]?.handle.fd).toBe(-1); + expect(aggregate.cleanupIncomplete).toBe(false); + expect(await session.status()).toMatchObject({ cleanup_pending: false }); + } finally { + fileAllowed = true; + providerAllowed = true; + close.mockRestore(); + await harness.close(); + vi.restoreAllMocks(); + } +}); diff --git a/tests/process-global/binaryTargetAdmissionCancellation.test.ts b/tests/process-global/binaryTargetAdmissionCancellation.test.ts index a8d2814289..6347346f2b 100644 --- a/tests/process-global/binaryTargetAdmissionCancellation.test.ts +++ b/tests/process-global/binaryTargetAdmissionCancellation.test.ts @@ -51,10 +51,12 @@ vi.mock("node:fs/promises", async (importOriginal) => { import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js"; import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; +import { RegularFileOperationCancelled } from "../../src/filesystem/RegularFile.js"; import { - RegularFileAdmissionFailure, - RegularFileOperationCancelled, -} from "../../src/filesystem/RegularFile.js"; + RetainedRegularFileCleanupFailure, + retryRegularFileCleanup, +} from "../../src/application/RegularFileRead.js"; +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; import { createApplicationMcpHarness } from "../fixtures/applicationMcpHarness.js"; import { createTestBinarySession } from "../fixtures/binarySession.js"; import { advertisedMcpToolSchemas } from "../fixtures/advertisedMcpTool.js"; @@ -153,20 +155,29 @@ it("retains the cleanup failure and its file owner when cancelled admission cann const { path, controller } = await fixture(1, reason); const cleanup = new Error("descriptor close failed"); admission.closeFailure = cleanup; - const failure = await parseBinaryTarget(path, { + const result = await parseBinaryTarget(path, { signal: controller.signal, - }).catch((cause: unknown) => cause); - expect(failure).toBeInstanceOf(RegularFileAdmissionFailure); - if (!(failure instanceof RegularFileAdmissionFailure)) throw failure; - expect(failure.cause).toBeInstanceOf(RegularFileOperationCancelled); - if (!(failure.cause instanceof RegularFileOperationCancelled)) - throw failure.cause; - expect(failure.cause.reason).toBe(reason); + }); + if (result.ok) throw new Error("Expected typed admission cleanup failure"); + expect(result.error.cleanupIncomplete).toBe(true); + const failure = result.error.cause; + expect(failure).toBeInstanceOf(RetainedRegularFileCleanupFailure); + if (!(failure instanceof RetainedRegularFileCleanupFailure)) throw failure; + expect(failure.outcome.kind).toBe("failed"); + if (failure.outcome.kind !== "failed") + throw new Error("Expected failed admission"); + expect(failure.outcome.cause).toBeInstanceOf(RegularFileOperationCancelled); + if (!(failure.outcome.cause instanceof RegularFileOperationCancelled)) + throw failure.outcome.cause; + expect(failure.outcome.cause.reason).toBe(reason); expect(failure.cleanupCause).toBe(cleanup); expect(failure.owner.handle).toBe(admission.handles[0]); expect(failure.owner.handle.fd).toBeGreaterThanOrEqual(0); - await failure.owner.close(); + const resources = new ArtifactResourceScope(); + await retryRegularFileCleanup(failure, resources); + await resources.close(); expect(failure.owner.handle.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); }); it("settles SDK cancellation at the admission boundary and reuses the same MCP connection", async () => {