From 3fcba07962a1f9f23130a5210dba242f6a331a5b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 18 Oct 2026 02:55:41 +0800 Subject: [PATCH] fix(process): preserve file outcomes through exact cleanup recovery Classify canonical physical file outcomes through the provider mapper before composing cleanup. Retain unknown capabilities as unresolved and replace runtime failure maps with exact leases, including mixed cleanup and partial retry. Keep completed-read cancellation output and cleanup history. Closes #4865. Validation: 179 unique focused tests passed across 13 selected files, 7 Linux-only skips; static checks, docs checks and 1551-file discovery passed. Authored Node producers and controlled real-descriptor faults do not qualify native providers. --- src/captures/TsharkCaptureProvider.ts | 12 +- src/process/ArtifactAnalysisResources.test.ts | 369 ++++++++++++++---- src/process/ArtifactAnalysisResources.ts | 313 ++++++++------- src/signals/SigrokRecordingProvider.ts | 11 +- .../process/artifactFileOutcomes.test.ts | 179 +++++++++ 5 files changed, 664 insertions(+), 220 deletions(-) create mode 100644 tests/boundary/process/artifactFileOutcomes.test.ts diff --git a/src/captures/TsharkCaptureProvider.ts b/src/captures/TsharkCaptureProvider.ts index a42413b2cd..8acada8e5a 100644 --- a/src/captures/TsharkCaptureProvider.ts +++ b/src/captures/TsharkCaptureProvider.ts @@ -447,8 +447,18 @@ export class TsharkCaptureProvider implements PacketCapturePort { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) return this.#resources.cleanupFailure( - cause.cleanup, cause, + (primary) => + this.#failure( + primary, + phase, + path, + workspacePath, + effectiveLimits, + observedOutput, + operation, + limits, + ), observedOutput, operation, ); diff --git a/src/process/ArtifactAnalysisResources.test.ts b/src/process/ArtifactAnalysisResources.test.ts index ede5260ab8..1536c0a4ae 100644 --- a/src/process/ArtifactAnalysisResources.test.ts +++ b/src/process/ArtifactAnalysisResources.test.ts @@ -1,3 +1,10 @@ +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { ArtifactResourceRetryFailure } from "../artifacts/ArtifactResourceScope.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { withArtifactRegularFile } from "../artifacts/ArtifactFileOperation.js"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { OwnedCommandFailure } from "./OwnedCommand.js"; import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; @@ -76,17 +83,27 @@ it("projects the specific reason and member evidence for retained cleanup", asyn "process-group:4242", ], diagnostics: { - reason: "process ownership token could not be read for 1 live process", - failures: [ - { - pid: 101, - reason: "environment-unreadable", - diagnostic: "sysctl_errno_22", + cleanup_error: { + details: { + diagnostics: { + reason: + "process ownership token could not be read for 1 live process", + failures: [ + { + pid: 101, + reason: "environment-unreadable", + diagnostic: "sysctl_errno_22", + }, + ], + unverified: [ + { + pid: 900, + diagnostic: "platform_binary_environment_withheld", + }, + ], + }, }, - ], - unverified: [ - { pid: 900, diagnostic: "platform_binary_environment_withheld" }, - ], + }, }, }, }); @@ -142,7 +159,7 @@ it("preserves the primary error and resolves cleanup status after retry", async expect(result.error.partialObservation).toEqual(primary.partialObservation); expect(result.error.cleanupIncomplete).toBe(true); expect(result.error.diagnostics).toMatchObject({ - previous_error: { code: "unreadable_output" }, + primary_error: { code: "unreadable_output" }, }); await resources.release(); @@ -220,20 +237,22 @@ it("resolves an artifact cleanup error when its scope owner later closes", async const owner = { kind: "lease", resource: "fixture descriptor", - lease: { - release: async () => { - closes++; - if (closes === 1) throw new Error("descriptor remains open"); - }, - }, + lease: new AnalysisCleanupLease(async () => { + closes++; + if (closes === 1) throw new Error("descriptor remains open"); + }, ["fixture descriptor"]), } as const; expect(await scope.release(owner)).toMatchObject({ kind: "failed" }); - throw new ArtifactReaderFailure("io", "Descriptor close failed", { - cleanup: { - reason: "descriptor remains open", - resources: ["fixture descriptor"], + throw new ArtifactResourceRetryFailure( + [{ owner, lease: owner.lease }], + "Descriptor close failed", + { + cleanup: { + reason: "descriptor remains open", + resources: ["fixture descriptor"], + }, }, - }); + ); } return { bytes: Buffer.alloc(0), sha256: "0".repeat(64) }; }; @@ -276,21 +295,23 @@ it("keeps a runtime root until its descriptor cleanup owner is released", async const owner = { kind: "lease", resource: "fixture descriptor", - lease: { - release: async () => { - descriptorCloses++; - if (descriptorCloses === 1) - throw new Error("descriptor remains open"); - }, - }, + lease: new AnalysisCleanupLease(async () => { + descriptorCloses++; + if (descriptorCloses === 1) + throw new Error("descriptor remains open"); + }, ["fixture descriptor"]), } as const; expect(await scope.release(owner)).toMatchObject({ kind: "failed" }); - throw new ArtifactReaderFailure("io", "Descriptor close failed", { - cleanup: { - reason: "descriptor remains open", - resources: ["fixture descriptor"], + throw new ArtifactResourceRetryFailure( + [{ owner, lease: owner.lease }], + "Descriptor close failed", + { + cleanup: { + reason: "descriptor remains open", + resources: ["fixture descriptor"], + }, }, - }); + ); } return { bytes: Buffer.alloc(0), sha256: "0".repeat(64) }; }; @@ -418,6 +439,9 @@ it.each([false, true])( "preserves artifact cancellation and independent failed cleanup (failed close: %s)", async (failedClose) => { const reason = { request: "artifact cancellation" }; + const root = await createTestTempDirectory("rea-resource-cancel-"); + const path = join(root, "descriptor"); + await writeFile(path, "authored bytes"); let mayClose = false; const readArtifact: typeof readStableArtifact = async ( _path, @@ -426,31 +450,18 @@ it.each([false, true])( _fs, scope, ) => { - if (failedClose) { - if (scope === undefined) - throw new Error("Expected retained file scope"); - const retained = { - kind: "lease", - resource: "descriptor", - lease: { - release: async () => { - if (!mayClose) - throw new Error("injected descriptor close failure"); - }, - }, - } as const; - expect(await scope.release(retained)).toMatchObject({ kind: "failed" }); - } - throw new ArtifactReaderFailure("cancelled", "read cancelled", { + const primary = new ArtifactReaderFailure("cancelled", "read cancelled", { cause: reason, - ...(failedClose - ? { - cleanup: { - reason: "injected descriptor close failure", - resources: ["descriptor"], - }, - } - : {}), + }); + if (!failedClose) throw primary; + if (scope === undefined) throw new Error("Expected retained file scope"); + return withArtifactRegularFile(path, scope, _signal, async (file) => { + const close = file.close.bind(file); + file.close = async () => { + if (!mayClose) throw new Error("injected descriptor close failure"); + await close(); + }; + throw primary; }); }; const resources = new ArtifactAnalysisResources( @@ -569,19 +580,24 @@ it("observes cancellation after pending descriptor cleanup before admitting repl const retained = { kind: "lease", resource: "descriptor", - lease: { - release: async () => { - closes++; - if (closes === 1) throw new Error("descriptor close denied"); - cleanupStarted(); - await held; - }, - }, + lease: new AnalysisCleanupLease(async () => { + closes++; + if (closes === 1) throw new Error("descriptor close denied"); + cleanupStarted(); + await held; + }, ["descriptor"]), } as const; await scope.release(retained); - throw new ArtifactReaderFailure("io", "descriptor cleanup failed", { - cleanup: { reason: "descriptor close denied", resources: ["descriptor"] }, - }); + throw new ArtifactResourceRetryFailure( + [{ owner: retained, lease: retained.lease }], + "descriptor cleanup failed", + { + cleanup: { + reason: "descriptor close denied", + resources: ["descriptor"], + }, + }, + ); }; const resources = new ArtifactAnalysisResources( "fixture", @@ -667,3 +683,218 @@ it.each([false, true])( } }, ); + +it("keeps cleanup with no reported capability unresolved instead of clearing it from an empty scope", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + const source = new ArtifactReaderFailure("io", "unverified cleanup", { + cleanup: { + reason: "no cleanup capability reported", + resources: ["unverified-resource"], + }, + }); + const result = await resources.run(async () => { + throw source; + }); + if (result.ok) throw new Error("Expected unresolved cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual(["unverified-resource"]); + let calls = 0; + const blocked = await resources.run(async () => { + calls++; + return ok("Must not run"); + }); + expect(blocked).toMatchObject({ + ok: false, + error: { cleanupIncomplete: true }, + }); + expect(calls).toBe(0); + await expect(resources.close()).rejects.toMatchObject({ + cleanupIncomplete: true, + }); + expect(result.error.cleanupIncomplete).toBe(true); + // No OS resource was supplied. An unresolved observation never grants cleanup authority. +}); + +it("joins mixed runtime cleanup and resolves each error only through its exact lease", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + let denied = true; + let healthyCloses = 0; + const bad = { + path: "bad-root", + close: async () => { + if (denied) throw new Error("bad root denied"); + }, + }; + const good = { + path: "good-root", + close: async () => { + healthyCloses++; + }, + }; + resources.retain(bad); + resources.retain(good); + try { + const failure: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(failure).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["bad-root"], + }); + expect(healthyCloses).toBe(1); + expect(resources.pendingPaths).toEqual(["bad-root"]); + denied = false; + await resources.close(); + expect(failure).toMatchObject({ + cleanupIncomplete: false, + cleanupResources: [], + }); + expect(healthyCloses).toBe(1); + } finally { + denied = false; + await resources.close(); + } +}); + +it("retains producer output when a completed read is cancelled during failed close", async () => { + const root = await createTestTempDirectory("rea-completed-read-cancel-"); + const path = join(root, "descriptor"); + await writeFile(path, "authored bytes"); + const caller = new AbortController(); + const reason = { request: "cancel during close" }; + const scope = new ArtifactResourceScope(); + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + let mayClose = false; + const output = { + stdout: "completed producer output", + stderr: "", + truncated: false, + exit_code: 0, + }; + try { + const cause: unknown = await withArtifactRegularFile( + path, + scope, + caller.signal, + async (file) => { + const close = file.close.bind(file); + file.close = async () => { + caller.abort(reason); + if (!mayClose) throw new Error("controlled descriptor close failure"); + await close(); + }; + return await file.readFile(); + }, + ).catch((cause: unknown) => cause); + if (!(cause instanceof ArtifactReaderFailure)) + throw new Error("Expected retained artifact failure"); + const error = resources.cleanupFailure( + cause, + () => { + throw new Error("Completed read has no physical failure"); + }, + output, + ); + const primary = error.cleanupPrimary; + expect(primary).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + capturedOutput: output, + }); + expect(error.capturedOutput).toEqual(output); + expect(error.cleanupIncomplete).toBe(true); + await expect(scope.close()).rejects.toMatchObject({ + reason: "unavailable", + }); + mayClose = true; + await scope.close(); + expect(error.cleanupIncomplete).toBe(false); + expect(error.cleanupPrimary).toBe(primary); + expect(error.capturedOutput).toEqual(output); + expect(projectAnalysisError(error)).toMatchObject( + projectAnalysisError(primary!), + ); + } finally { + mayClose = true; + await scope.close(); + await resources.close(); + } +}); + +it("keeps aggregate cleanup live through partial recovery without reclosing healthy roots", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + const denied = new Set(["first", "second"]); + const closes = new Map(); + for (const path of ["first", "healthy", "second"]) + resources.retain({ + path, + close: async () => { + closes.set(path, (closes.get(path) ?? 0) + 1); + if (denied.has(path)) throw new Error(`Close denied: ${path}`); + }, + }); + try { + const original: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(original).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["first", "second"], + }); + expect(closes.get("healthy")).toBe(1); + denied.delete("first"); + const retry: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(original).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["second"], + }); + expect(retry).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["second"], + }); + expect(resources.pendingPaths).toEqual(["second"]); + denied.clear(); + await resources.close(); + for (const error of [original, retry]) + expect(error).toMatchObject({ + cleanupIncomplete: false, + cleanupResources: [], + }); + expect(closes).toEqual( + new Map([ + ["first", 2], + ["healthy", 1], + ["second", 3], + ]), + ); + } finally { + denied.clear(); + await resources.close(); + } +}); diff --git a/src/process/ArtifactAnalysisResources.ts b/src/process/ArtifactAnalysisResources.ts index ad1277fc53..c8674ebe83 100644 --- a/src/process/ArtifactAnalysisResources.ts +++ b/src/process/ArtifactAnalysisResources.ts @@ -1,6 +1,16 @@ -import { artifactFileCancellationCause } from "../artifacts/ArtifactFileOperation.js"; +import { + AnalysisCleanupLease, + AnalysisCleanupLeaseGroup, +} from "../artifacts/AnalysisCleanupLease.js"; +import { + ArtifactFileCleanupFailure, + artifactFileCancellationCause, +} from "../artifacts/ArtifactFileOperation.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; -import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { + ArtifactResourceScope, + ArtifactResourceRetryFailure, +} from "../artifacts/ArtifactResourceScope.js"; import { readStableArtifact } from "../artifacts/readStableArtifact.js"; import { AnalysisError, @@ -43,14 +53,7 @@ export class ArtifactAnalysisResources { { readonly owner: ProviderProcessSupervisor | undefined; readonly operation: string; - } - >(); - readonly #cleanupFailures = new Map< - AnalysisError, - { - readonly roots: Set; - readonly primaryFailure: AnalysisError | undefined; - readonly scopeOwner: boolean; + readonly lease: AnalysisCleanupLease; } >(); #tail: Promise = Promise.resolve(); @@ -101,7 +104,6 @@ export class ArtifactAnalysisResources { // Failed descriptor cleanup still belongs to the preceding request until // this scope has drained it and admits acquisition for the new request. this.#artifacts.operation = operation; - this.#resolveCleanupFailures(); if (signal?.aborted) return Promise.resolve( err( @@ -140,10 +142,18 @@ export class ArtifactAnalysisResources { root: ArtifactAnalysisRuntime, owner?: ProviderProcessSupervisor, ): void { + const retained = this.#pending.get(root); + const resources = () => this.#resourcesFor(root); this.#pending.set(root, { - owner, - operation: - this.#pending.get(root)?.operation ?? this.#artifacts.operation, + owner: owner ?? retained?.owner, + operation: retained?.operation ?? this.#artifacts.operation, + lease: + retained?.lease ?? + new AnalysisCleanupLease(() => this.#releaseRoot(root), { + get cleanupResources() { + return resources(); + }, + }), }); } @@ -152,7 +162,9 @@ export class ArtifactAnalysisResources { } #resourcesFor(root: ArtifactAnalysisRuntime): readonly string[] { - const ownership = this.#pending.get(root)?.owner?.launch.ownership; + const retained = this.#pending.get(root); + if (retained === undefined) return []; + const ownership = retained.owner?.launch.ownership; return ownership === undefined ? [root.path] : ownedCommandResources(ownership, root.path); @@ -165,55 +177,14 @@ export class ArtifactAnalysisResources { } async release(): Promise { + const failures: ProviderCleanupError[] = []; for (const [root, retained] of this.#pending) { try { - const { owner } = retained; - if (owner !== undefined) { - const stopped = await owner.stop(); - if (stopped.status === "incomplete") { - const cleanup = stopped.cleanupResult; - throw new ProviderCleanupError( - this.providerId, - this.#resourcesFor(root), - { - reason: stopped.reason, - ...(cleanup?.failures === undefined - ? {} - : { - failures: cleanup.failures.map((failure) => ({ - ...failure, - })), - }), - ...(cleanup?.unverified === undefined - ? {} - : { - unverified: cleanup.unverified.map((process) => ({ - ...process, - })), - }), - }, - { operation: retained.operation }, - ); - } - this.#pending.set(root, { ...retained, owner: undefined }); - } - if (this.#artifacts.scope.hasPending) - throw new ProviderCleanupError( - this.providerId, - [root.path], - { - reason: - "Artifact descriptors remain open; the private runtime root is retained until descriptor cleanup succeeds.", - }, - { operation: retained.operation }, - ); - await root.close(); - this.#pending.delete(root); - this.#resolveCleanupFailures(root); + await retained.lease.release(); } catch (cause: unknown) { const diagnostics = cause instanceof ProviderCleanupError ? cause.diagnostics : undefined; - throw new ProviderCleanupError( + const failure = new ProviderCleanupError( this.providerId, this.#resourcesFor(root), { @@ -225,16 +196,81 @@ export class ArtifactAnalysisResources { }, { operation: retained.operation, cause }, ); + new AnalysisCleanupLeaseGroup(failure, [retained.lease]); + failures.push(failure); } } + if (failures.length === 0) return; + if (failures.length === 1) throw failures[0]; + throw new ProviderCleanupError( + this.providerId, + failures.flatMap((failure) => failure.cleanupResources), + { + reason: "One or more analysis runtime resources remain owned.", + failures: failures.map(projectAnalysisError), + }, + { + operation: failures[0]?.operation ?? this.#artifacts.operation, + cause: new AggregateError( + failures, + "Analysis runtime cleanup remains incomplete", + ), + }, + ).bindCleanupDependencies(failures); + } + + async #releaseRoot(root: ArtifactAnalysisRuntime): Promise { + const retained = this.#pending.get(root); + if (retained === undefined) return; + if (retained.owner !== undefined) { + const stopped = await retained.owner.stop(); + if (stopped.status === "incomplete") { + const cleanup = stopped.cleanupResult; + throw new ProviderCleanupError( + this.providerId, + this.#resourcesFor(root), + { + reason: stopped.reason, + ...(cleanup?.failures === undefined + ? {} + : { + failures: cleanup.failures.map((failure) => ({ ...failure })), + }), + ...(cleanup?.unverified === undefined + ? {} + : { + unverified: cleanup.unverified.map((process) => ({ + ...process, + })), + }), + }, + { operation: retained.operation }, + ); + } + this.#pending.set(root, { ...retained, owner: undefined }); + } + if (this.#artifacts.scope.hasPending) + throw new ProviderCleanupError( + this.providerId, + [root.path], + { + reason: + "Artifact resources remain owned; the private runtime root is retained until their cleanup succeeds.", + }, + { operation: retained.operation }, + ); + await root.close(); + this.#pending.delete(root); } cleanupFailure( - observation: NonNullable, - cause: unknown, + cause: ArtifactReaderFailure, + classifyPrimary: (cause: unknown) => AnalysisError, capturedOutput?: AnalysisCapturedOutput, operation = this.operation, ): AnalysisError { + const observation = cause.cleanup; + if (observation === undefined) return classifyPrimary(cause); const failure = new ProviderCleanupError( this.providerId, observation.resources, @@ -246,23 +282,50 @@ export class ArtifactAnalysisResources { }, { operation, cause }, ); - this.#trackCleanupFailure(failure, [], failure, true); - if ( - cause instanceof ArtifactReaderFailure && - cause.reason === "cancelled" - ) { - const primary = new AnalysisCancelledError(operation, { + let leases = + cause instanceof ArtifactFileCleanupFailure || + cause instanceof ArtifactResourceRetryFailure + ? cause.cleanupLeases + : undefined; + if (leases === undefined) { + // An unreported capability cannot be recovered by observing an empty scope. + // Retain the unresolved source failure and refuse replacement admission. + const unresolved = new AnalysisCleanupLease(async () => { + throw cause; + }, observation.resources); + this.#artifacts.scope.retain({ + kind: "lease", + resource: observation.resources[0] ?? "Unresolved artifact cleanup", + lease: unresolved, + }); + leases = [unresolved]; + } + new AnalysisCleanupLeaseGroup(failure, leases); + let primary: AnalysisError | undefined; + if (cause instanceof ArtifactFileCleanupFailure) { + const physical = cause.fileCleanupFailure; + if (physical.outcome.kind === "failed") + primary = classifyPrimary(physical.outcome.cause); + else if (physical.completionCancellation !== undefined) + primary = new AnalysisCancelledError(operation, { + cause: physical.completionCancellation.reason, + ...(capturedOutput === undefined ? {} : { capturedOutput }), + partialObservation: cause.partialObservation, + }); + } else if (cause.reason === "cancelled") { + primary = new AnalysisCancelledError(operation, { cause: artifactFileCancellationCause(cause), ...(capturedOutput === undefined ? {} : { capturedOutput }), partialObservation: cause.partialObservation, }); - return analysisErrorWithCleanupFailure( - primary, - failure, - operation, - ).bindCleanupDependencies([primary, failure]); } - return failure; + return primary === undefined + ? failure + : analysisErrorWithCleanupFailure( + primary, + failure, + operation, + ).bindCleanupDependencies([primary, failure]); } commandFailure( @@ -343,7 +406,7 @@ export class ArtifactAnalysisResources { const partialObservation = result.ok ? observation(result.value) : result.error.partialObservation; - const primaryFailure = !result.ok + const primary = !result.ok ? result.error : signal?.aborted ? new AnalysisCancelledError(operation, { @@ -351,52 +414,26 @@ export class ArtifactAnalysisResources { partialObservation, }) : undefined; - const cleanupDiagnostics = - cause instanceof ProviderCleanupError ? cause.diagnostics : undefined; - const cleanupFailure = new ProviderCleanupError( - this.providerId, - [ - ...new Set([ - ...(!result.ok && result.error.cleanupIncomplete - ? result.error.cleanupResources - : []), - ...this.#pendingResources, - ]), - ], - { - ...cleanupDiagnostics, - reason: - cause instanceof ProviderCleanupError - ? String(cleanupDiagnostics?.reason ?? cause.message) - : String(cause), - previous_error: - primaryFailure === undefined - ? null - : projectAnalysisError(primaryFailure), - }, - { - operation, - ...(primaryFailure === undefined - ? {} - : { cleanupPrimary: primaryFailure }), - cause: - primaryFailure === undefined - ? cause - : new AggregateError( - [primaryFailure, cause], - "Analysis failed and artifact cleanup remains incomplete.", - { cause: primaryFailure }, - ), - ...(partialObservation === undefined ? {} : { partialObservation }), - }, - ); - this.#trackCleanupFailure( - cleanupFailure, - [...this.#pending.keys()], - primaryFailure?.cleanupIncomplete === true ? primaryFailure : undefined, - false, - ); - return err(cleanupFailure); + 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); } } @@ -411,7 +448,6 @@ export class ArtifactAnalysisResources { let descriptorFailure: AnalysisError | undefined; try { await this.#artifacts.scope.close(); - this.#resolveCleanupFailures(); } catch (cause: unknown) { descriptorFailure = this.#artifactFailure(cause); } @@ -423,45 +459,24 @@ export class ArtifactAnalysisResources { descriptorFailure, cause, this.#artifacts.operation, - ); + ).bindCleanupDependencies([descriptorFailure, cause]); throw cause; } if (descriptorFailure !== undefined) throw descriptorFailure; } - #resolveCleanupFailures(releasedRoot?: ArtifactAnalysisRuntime): void { - for (const [failure, binding] of this.#cleanupFailures) { - if (releasedRoot !== undefined) binding.roots.delete(releasedRoot); - if (binding.roots.size > 0) continue; - if (binding.scopeOwner && this.#artifacts.scope.hasPending) continue; - if (binding.scopeOwner) binding.primaryFailure?.resolveCleanup(); - if (binding.primaryFailure?.cleanupIncomplete === true) continue; - failure.resolveCleanup(); - this.#cleanupFailures.delete(failure); - } - } - - #trackCleanupFailure( - failure: AnalysisError, - roots: readonly ArtifactAnalysisRuntime[], - primaryFailure: AnalysisError | undefined, - scopeOwner: boolean, - ): void { - this.#cleanupFailures.set(failure, { - roots: new Set(roots), - primaryFailure, - scopeOwner, - }); - this.#resolveCleanupFailures(); - } - #artifactFailure( cause: unknown, operation = this.#artifacts.operation, ): AnalysisError { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) - return this.cleanupFailure(cause.cleanup, cause, undefined, operation); + return this.cleanupFailure( + cause, + (primary) => this.#artifactFailure(primary, operation), + undefined, + operation, + ); if (cause instanceof ArtifactReaderFailure && cause.reason === "cancelled") return new AnalysisCancelledError(operation, { cause: artifactFileCancellationCause(cause), diff --git a/src/signals/SigrokRecordingProvider.ts b/src/signals/SigrokRecordingProvider.ts index c14b20d3d7..e623e02985 100644 --- a/src/signals/SigrokRecordingProvider.ts +++ b/src/signals/SigrokRecordingProvider.ts @@ -407,8 +407,17 @@ export class SigrokRecordingProvider implements SignalRecordingPort { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) return this.#resources.cleanupFailure( - cause.cleanup, cause, + (primary) => + this.#failure( + primary, + phase, + path, + workspacePath, + effectiveLimits, + observedOutput, + operation, + ), observedOutput, operation, ); diff --git a/tests/boundary/process/artifactFileOutcomes.test.ts b/tests/boundary/process/artifactFileOutcomes.test.ts new file mode 100644 index 0000000000..d8c68779f9 --- /dev/null +++ b/tests/boundary/process/artifactFileOutcomes.test.ts @@ -0,0 +1,179 @@ +import { writeFile } from "node:fs/promises"; +import type { FileHandle } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { SigrokRecordingProvider } from "../../../src/signals/SigrokRecordingProvider.js"; +import { TsharkCaptureProvider } from "../../../src/captures/TsharkCaptureProvider.js"; +import { SignalRecordingService } from "../../../src/application/signals/SignalRecordingService.js"; +import { PacketCaptureService } from "../../../src/application/captures/PacketCaptureService.js"; +import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js"; +import { withArtifactRegularFile } from "../../../src/artifacts/ArtifactFileOperation.js"; +import { readStableArtifact } from "../../../src/artifacts/readStableArtifact.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; +import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +// Real descriptors/owned Node commands; simulated Linux limits, no native tools. +it.each( + (["signal", "capture"] as const).flatMap((kind) => + [ + { + stage: "configuration", + failure: "denied", + tag: "AnalysisCapabilityUnavailableError", + }, + { + stage: "artifact", + failure: "denied", + tag: "AnalysisAccessDeniedError", + }, + { stage: "artifact", failure: "format", tag: "AnalysisInputError" }, + { + stage: "artifact", + failure: "integrity", + tag: "AnalysisArtifactChangedError", + }, + { + stage: "artifact", + failure: "limit", + tag: "AnalysisResourceConstraintError", + }, + { + stage: "integrity", + failure: "integrity", + tag: "AnalysisArtifactChangedError", + }, + ].map((control) => ({ kind, ...control })), + ), +)( + "retains the physical file outcome and exact descriptor through retry: %j", + async ({ kind, stage, failure, tag }) => { + const root = await createTestTempDirectory("rea-file-primary-"); + const inputPath = join(root, "input"); + const command = join(root, "producer.mjs"); + await writeFile(inputPath, "authored recording"); + await writeFile( + command, + kind === "signal" + ? "if(process.argv.includes('--version')) console.log('sigrok-cli 0.7.2\\nlibsigrokdecode 0.5.3'); else console.log('annotation');" + : "if(process.argv.includes('--version')) console.log('TShark (Wireshark) 4.6.9 (fixture)'); else console.log('[]');", + { mode: 0o700 }, + ); + const physical = + failure === "denied" + ? Object.assign(new Error("controlled read denial"), { code: "EACCES" }) + : new ArtifactReaderFailure( + failure === "format" + ? "format" + : failure === "limit" + ? "limit" + : "integrity", + "controlled physical file failure", + ); + const selectedPath = stage === "artifact" ? inputPath : command; + let allowed = false; + let commandReads = 0; + let launches = 0; + let handle: FileHandle | undefined; + const caller = new AbortController(); + const Provider = + kind === "signal" ? SigrokRecordingProvider : TsharkCaptureProvider; + const provider = new Provider( + { + ...process.env, + REA_SIGROK_COMMAND: command, + REA_SIGROK_PRLIMIT_COMMAND: command, + REA_TSHARK_COMMAND: command, + REA_TSHARK_PRLIMIT_COMMAND: command, + }, + async (options) => { + launches++; + return spawnOwnedProviderProcess({ + ...options, + command: process.execPath, + arguments: [command, ...options.arguments], + expectedCommand: process.execPath, + }); + }, + () => PrivateRuntimeRoot.create({ parent: root }), + "linux", + async (...args) => { + if (args[0] === command) commandReads++; + const selected = + args[0] === selectedPath && + (stage !== "integrity" || commandReads === 3); + if (!selected) return readStableArtifact(...args); + const scope = args[4]; + if (scope === undefined) + throw new Error("Expected retained artifact scope"); + return withArtifactRegularFile( + args[0], + scope, + args[2], + async (file) => { + handle = file; + const close = file.close.bind(file); + file.close = async () => { + // A later abort must not replace the already observed physical failure. + caller.abort({ + request: "late cancellation during failed close", + }); + if (!allowed) + throw new Error("controlled descriptor close failure"); + await close(); + }; + throw physical; + }, + ); + }, + async (limits) => ({ ...limits, file_bytes: 1048576 }), + ); + try { + const result = + provider instanceof SigrokRecordingProvider + ? await new SignalRecordingService(provider).decode( + { path: inputPath, decoders: "uart" }, + { signal: caller.signal }, + ) + : await new PacketCaptureService(provider).inspect( + { path: inputPath, fields: ["frame.number"] }, + { signal: caller.signal }, + ); + if (result.ok) + throw new Error("Expected physical failure and retained descriptor"); + let primary = result.error.cleanupPrimary; + while (primary?.cleanupPrimary !== undefined) + primary = primary.cleanupPrimary; + expect(primary).toMatchObject({ _tag: tag, cause: physical }); + expect(result.error.cleanupIncomplete).toBe(true); + expect(handle?.fd).toBeGreaterThanOrEqual(0); + const failed: unknown = await provider + .close() + .catch((cause: unknown) => cause); + expect(failed).toMatchObject({ cleanupIncomplete: true }); + const launchCount = launches; + allowed = true; + await provider.close(); + expect(handle?.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + let restored = result.error.cleanupPrimary; + while (restored?.cleanupPrimary !== undefined) + restored = restored.cleanupPrimary; + expect(restored).toBe(primary); + expect(primary?.cause).toBe(physical); + expect(projectAnalysisError(result.error)).toMatchObject( + projectAnalysisError(primary!), + ); + expect(projectAnalysisError(result.error).details).toHaveProperty( + "cleanup_history", + ); + expect(failed).toMatchObject({ cleanupIncomplete: false }); + expect(launches).toBe(launchCount); + } finally { + allowed = true; + await provider.close(); + await handle?.close(); + } + }, +);