diff --git a/src/application/WebScriptExportService.ts b/src/application/WebScriptExportService.ts index f996bd36e0..1fbb419d7c 100644 --- a/src/application/WebScriptExportService.ts +++ b/src/application/WebScriptExportService.ts @@ -15,9 +15,11 @@ import { } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { createEvidence, type Evidence } from "../domain/evidence.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js"; import { jsonValueSchema } from "../domain/jsonValue.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { err, ok, type Result } from "../domain/result.js"; import { safeParseJson } from "../domain/safeJson.js"; import { @@ -27,7 +29,11 @@ import { import { WebScriptExportError } from "../domain/webScriptExportError.js"; import type { ExecutionOptions } from "./AnalysisProvider.js"; import { WEB_SCRIPT_EXPORT_PROVIDER as PROVIDER } from "./InvestigationProviders.js"; -import { readRegularFile } from "./RegularFileRead.js"; +import { + readRegularFile, + RegularFileCleanupFailure, + retryRegularFileCleanup, +} from "./RegularFileRead.js"; import { NonRegularFileReadError, RegularFileChangedError, @@ -35,6 +41,20 @@ import { const OPERATION = "export_web_scripts"; +interface CaptureRead { + readonly bytes: Buffer; + readonly cleanup?: { + readonly reason: string; + readonly incomplete: boolean; + readonly cause: unknown; + }; +} + +interface PreparedCapture { + readonly bytes: Buffer; + readonly loaded: ReturnType; +} + /** Export one local capture through the shared CLI/MCP application workflow. */ export const exportWebScripts = async ( rawInput: unknown, @@ -68,14 +88,16 @@ export const exportWebScriptsValidated = async ( ); try { options.signal?.throwIfAborted(); - const read = await readCapture(input.capture_path, options.signal); - if (!read.ok) return read; - const bytes = read.value; - const loaded = parseCapture(bytes); - if (!loaded.ok) return loaded; + const prepared = await prepareCapture( + input.capture_path, + resources, + options.signal, + ); + if (!prepared.ok) return prepared; + const { bytes, loaded } = prepared.value; options.signal?.throwIfAborted(); const sha256 = createHash("sha256").update(bytes).digest("hex"); - const result = await publishWebScripts(input, loaded.value, sha256, { + const result = await publishWebScripts(input, loaded, sha256, { resources, ...(options.signal === undefined ? {} : { signal: options.signal }), }); @@ -97,55 +119,109 @@ export const exportWebScriptsValidated = async ( limitations: result.limitations, locations: [{ kind: "artifact-path", path: result.manifest.path }], evidenceLinks: - loaded.value.sourceEvidenceId === null - ? [] - : [loaded.value.sourceEvidenceId], + loaded.sourceEvidenceId === null ? [] : [loaded.sourceEvidenceId], }, ), ); } catch (cause: unknown) { - if (cause instanceof WebScriptExportError) return err(cause); - if (options.signal?.aborted === true) - return err(new AnalysisCancelledError(OPERATION)); - if (cause instanceof ArtifactReaderFailure) - return err( - new WebScriptExportError( - cause.reason, - input.output_directory, - cause.message, - ), - ); - if (cause instanceof z.ZodError) - return err(new AnalysisOutputError(OPERATION, cause.message, { cause })); - if ( - cause instanceof Error && - "code" in cause && - typeof cause.code === "string" - ) - return err( - new WebScriptExportError( - "io", - `${input.capture_path} → ${input.output_directory}`, - cause.message, - ), - ); + return err(mapExportFailure(cause, input, options.signal)); + } +}; + +const prepareCapture = async ( + path: string, + resources: ArtifactResourceScope, + signal: AbortSignal | undefined, +): Promise> => { + const read = await resources.run(() => readCapture(path, resources, signal)); + if (!read.ok) return read; + const cleanup = read.value.cleanup; + const loaded = parseCapture( + read.value.bytes, + cleanup === undefined || cleanup.incomplete + ? undefined + : { cause: cleanup.cause }, + ); + if (!loaded.ok) return err( - new ProviderAdapterError(PROVIDER.id, OPERATION, { - cause, - diagnostics: { - capture_path: input.capture_path, - output_directory: input.output_directory, - error_name: cause instanceof Error ? cause.name : "UnknownError", - error_message: - cause instanceof Error - ? cause.message - : typeof cause === "string" - ? cause - : "Unknown script export failure", - }, - }), + cleanup?.incomplete === true + ? incompleteCaptureCleanup(loaded.error, path, cleanup) + : loaded.error, + ); + if (cleanup !== undefined) { + const primary = new WebScriptExportError( + "io", + path, + `Capture handle close failed: ${cleanup.reason}`, + { cause: cleanup.cause }, + ); + return err( + cleanup.incomplete + ? incompleteCaptureCleanup(primary, path, cleanup) + : primary, ); } + return ok({ bytes: read.value.bytes, loaded: loaded.value }); +}; + +const mapExportFailure = ( + cause: unknown, + input: ExportWebScriptsInput, + signal: AbortSignal | undefined, +): AnalysisError => { + if (cause instanceof WebScriptExportError) return cause; + if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) + return analysisErrorWithCleanupFailure( + new WebScriptExportError( + cause.reason, + input.output_directory, + cause.message, + { cause }, + ), + new ProviderCleanupError( + PROVIDER.id, + cause.cleanup.resources, + { reason: cause.cleanup.reason }, + { cause, operation: OPERATION }, + ), + OPERATION, + ); + if (signal?.aborted === true) + return new AnalysisCancelledError(OPERATION, { cause }); + if (cause instanceof ArtifactReaderFailure) + return new WebScriptExportError( + cause.reason, + input.output_directory, + cause.message, + { cause }, + ); + if (cause instanceof z.ZodError) + return new AnalysisOutputError(OPERATION, cause.message, { cause }); + if ( + cause instanceof Error && + "code" in cause && + typeof cause.code === "string" + ) + return new WebScriptExportError( + "io", + `${input.capture_path} → ${input.output_directory}`, + cause.message, + { cause }, + ); + return new ProviderAdapterError(PROVIDER.id, OPERATION, { + cause, + diagnostics: { + capture_path: input.capture_path, + output_directory: input.output_directory, + error_name: cause instanceof Error ? cause.name : "UnknownError", + error_message: + cause instanceof Error + ? cause.message + : typeof cause === "string" + ? cause + : "Unknown script export failure", + }, + }); }; /** @@ -154,45 +230,115 @@ export const exportWebScriptsValidated = async ( */ const readCapture = async ( path: string, + resources: ArtifactResourceScope, signal: AbortSignal | undefined, -): Promise> => { +): Promise> => { try { - return ok(await readRegularFile(path, { signal })); + return ok({ bytes: await readRegularFile(path, { signal }) }); } catch (cause: unknown) { - const code = - cause instanceof Error && "code" in cause ? String(cause.code) : ""; - if (code === "EACCES" || code === "EPERM") - return err( - new AnalysisAccessDeniedError(OPERATION, path, code, { cause }), + if (cause instanceof RegularFileCleanupFailure) { + const retry = await retryRegularFileCleanup(cause, resources); + const cleanup = { + reason: retry.cleanup?.reason ?? errorMessage(cause.cleanupCause), + incomplete: retry.cleanup !== undefined, + cause: cause.cleanupCause, + }; + if (cause.outcome.kind === "completed") + return ok({ bytes: cause.outcome.value as Buffer, cleanup }); + const primary = captureReadFailure( + path, + cause.outcome.cause, + signal, + retry.cleanup === undefined ? { cause: cause.cleanupCause } : undefined, ); - if ( - cause instanceof NonRegularFileReadError || - cause instanceof RegularFileChangedError || - code === "ENOENT" || - code === "ENOTDIR" || - code === "EISDIR" || - code === "ENXIO" - ) return err( - new AnalysisInputError(OPERATION, { cause }, [ - { - path: ["capture_path"], - reason: "invalid_value", - message: - cause instanceof NonRegularFileReadError && code !== "EISDIR" - ? cause.message - : code === "EISDIR" - ? `Selected capture is a directory, not a file: ${path}` - : `Selected capture could not be read (${code}): ${path}`, - }, - ]), + retry.cleanup === undefined + ? primary + : incompleteCaptureCleanup(primary, path, cleanup), ); - throw cause; + } + return err(captureReadFailure(path, cause, signal)); } }; +const captureReadFailure = ( + path: string, + cause: unknown, + signal: AbortSignal | undefined, + closeFailure?: { readonly cause: unknown }, +): AnalysisError => { + const causeWithClose = + closeFailure === undefined + ? cause + : new AggregateError( + [cause, closeFailure.cause], + "Capture read and file-handle close both failed", + { cause }, + ); + if (signal?.aborted === true) + return new AnalysisCancelledError(OPERATION, { cause: causeWithClose }); + const code = + cause instanceof Error && "code" in cause ? String(cause.code) : ""; + if (code === "EACCES" || code === "EPERM") + return new AnalysisAccessDeniedError(OPERATION, path, code, { + cause: causeWithClose, + }); + if ( + cause instanceof NonRegularFileReadError || + cause instanceof RegularFileChangedError || + code === "ENOENT" || + code === "ENOTDIR" || + code === "EISDIR" || + code === "ENXIO" + ) + return new AnalysisInputError(OPERATION, { cause: causeWithClose }, [ + { + path: ["capture_path"], + reason: "invalid_value", + message: + cause instanceof NonRegularFileReadError && code !== "EISDIR" + ? cause.message + : code === "EISDIR" + ? `Selected capture is a directory, not a file: ${path}` + : `Selected capture could not be read (${code}): ${path}`, + }, + ]); + if (code !== "") + return new WebScriptExportError("io", path, errorMessage(cause), { + cause: causeWithClose, + }); + return new ProviderAdapterError(PROVIDER.id, OPERATION, { + cause: causeWithClose, + diagnostics: { + capture_path: path, + error_name: cause instanceof Error ? cause.name : "UnknownError", + error_message: cause instanceof Error ? cause.message : String(cause), + }, + }); +}; + +const incompleteCaptureCleanup = ( + primary: AnalysisError, + path: string, + cleanup: NonNullable, +): AnalysisError => + analysisErrorWithCleanupFailure( + primary, + new ProviderCleanupError( + PROVIDER.id, + [path], + { reason: cleanup.reason }, + { cause: cleanup.cause, operation: OPERATION }, + ), + OPERATION, + ); + +const errorMessage = (cause: unknown): string => + cause instanceof Error ? cause.message : String(cause); + const parseCapture = ( bytes: Buffer, + closeFailure?: { readonly cause: unknown }, ): Result, AnalysisError> => { try { const text = new TextDecoder("utf-8", { @@ -203,8 +349,16 @@ const parseCapture = ( if (!json.ok) throw new TypeError(json.error); return ok(selectScriptCapture(json.value)); } catch (cause: unknown) { + const causeWithClose = + closeFailure === undefined + ? cause + : new AggregateError( + [cause, closeFailure.cause], + "Capture parse and file-handle close both failed", + { cause }, + ); return err( - new AnalysisInputError(OPERATION, { cause }, [ + new AnalysisInputError(OPERATION, { cause: causeWithClose }, [ { path: ["capture_path"], reason: "invalid_format", diff --git a/src/browser/assets/PublishWebScripts.ts b/src/browser/assets/PublishWebScripts.ts index fb64aa0466..5d6bac53c0 100644 --- a/src/browser/assets/PublishWebScripts.ts +++ b/src/browser/assets/PublishWebScripts.ts @@ -114,7 +114,7 @@ export const publishWebScripts = async ( cause instanceof ArtifactReaderFailure ? cause.reason : "io", tree.outputRoot, diagnostic, - [tree.outputRoot], + { residualPaths: [tree.outputRoot] }, ); if (tree.published && result !== undefined) error.retainPartialObservation({ diff --git a/src/domain/webScriptExportError.ts b/src/domain/webScriptExportError.ts index 63270ee360..68067a3ec0 100644 --- a/src/domain/webScriptExportError.ts +++ b/src/domain/webScriptExportError.ts @@ -1,4 +1,5 @@ import { ArtifactOperationError } from "./artifactOperationError.js"; +import type { AnalysisErrorOptions } from "./analysisErrorBase.js"; /** Publication failure retaining its input/output target and cleanup state. */ export class WebScriptExportError extends ArtifactOperationError { @@ -10,11 +11,19 @@ export class WebScriptExportError extends ArtifactOperationError { reason: ArtifactOperationError["reason"], target: string, diagnostic: string, - residualPaths: readonly string[] = [], + options: AnalysisErrorOptions & { + readonly residualPaths?: readonly string[]; + } = {}, ) { - super("export_web_scripts", reason); + super("export_web_scripts", reason, undefined, undefined, options); this.userMessage = `Script export failed for ${target}: ${diagnostic}`; - this.cleanupResources = residualPaths; - this.cleanupIncomplete = residualPaths.length > 0; + this.cleanupResources = [ + ...new Set([ + ...(options.cleanup?.resources ?? []), + ...(options.residualPaths ?? []), + ]), + ]; + this.cleanupIncomplete = + options.cleanup !== undefined || (options.residualPaths?.length ?? 0) > 0; } } diff --git a/tests/boundary/filesystem/webScriptExport.test.ts b/tests/boundary/filesystem/webScriptExport.test.ts index c7fbda94e2..7dcf0f68fb 100644 --- a/tests/boundary/filesystem/webScriptExport.test.ts +++ b/tests/boundary/filesystem/webScriptExport.test.ts @@ -4,6 +4,7 @@ import { access, chmod, mkdir, + open as fsOpen, readFile, readdir, symlink, @@ -12,7 +13,15 @@ import { import { join } from "node:path"; import { promisify } from "node:util"; -import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { + afterEach, + beforeEach, + describe, + expect, + it, + onTestFinished, + vi, +} from "vitest"; import { analyzeJavaScriptApplication } from "../../support/javascriptApplicationScope.js"; import { exportWebScripts } from "../../../src/application/WebScriptExportService.js"; @@ -31,6 +40,24 @@ import { scriptScenarioFixture, } from "../../fixtures/webScriptCapture.js"; +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, open: vi.fn(actual.open) }; +}); + +const openMock = vi.mocked(fsOpen); +let actualFs: typeof import("node:fs/promises"); + +beforeEach(async () => { + actualFs = + await vi.importActual( + "node:fs/promises", + ); + openMock.mockImplementation((...args) => actualFs.open(...args)); +}); + +afterEach(() => openMock.mockReset()); + const setup = async (capture: unknown = scriptCaptureEvidenceFixture()) => { const root = await createTestTempDirectory("rea-web-script-export-"); const resources = new ArtifactResourceScope(); @@ -275,7 +302,6 @@ describe("captured script output owner retry", () => { it("returns completed output when retry closes a descriptor after commit", async () => { const { input, resources } = await setup(); - const capture = selectScriptCapture(scriptScenarioFixture()); const createTree = SafeOutputTree.create.bind(SafeOutputTree); const create = vi .spyOn(SafeOutputTree, "create") @@ -577,3 +603,208 @@ describe("captured script path and permission failures", () => { }, ); }); + +describe("capture read and close failure", () => { + it("retains a handle after capture read and close both fail", async () => { + const { input, resources } = await setup(); + const readFailure = Object.assign( + new Error("injected capture read failure"), + { code: "EIO" }, + ); + const closeFailure = new Error("injected capture close failure"); + let openedHandle: Awaited> | undefined; + let closeCalls = 0; + let allowClose = false; + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if (String(args[0]) !== input.capture_path) return handle; + openedHandle = handle; + vi.spyOn(handle, "read").mockRejectedValue(readFailure); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeCalls += 1; + if (!allowClose) throw closeFailure; + await close(); + }); + return handle; + }); + + try { + const result = await exportWebScripts(input, resources); + if (result.ok) throw new Error("Expected capture read failure"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [input.capture_path], + diagnostics: { + primary_error: { + code: "artifact_operation_failed", + details: { reason: "io" }, + }, + }, + }, + }); + if (openedHandle === undefined) + throw new Error("Expected the capture handle to be admitted"); + expect(openedHandle.fd).toBeGreaterThanOrEqual(0); + expect(closeCalls).toBe(2); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + + await expect(resources.close()).rejects.toMatchObject({ + cleanup: { + resources: [input.capture_path], + reason: expect.stringContaining(closeFailure.message), + }, + }); + expect(closeCalls).toBe(3); + allowClose = true; + await resources.close(); + expect(closeCalls).toBe(4); + expect(openedHandle.fd).toBe(-1); + } finally { + allowClose = true; + await resources.close().catch(() => undefined); + } + }); +}); + +describe("capture handle ownership", () => { + it("retains the failed handle and keeps invalid JSON primary", async () => { + const { input, resources } = await setup(); + await writeFile(input.capture_path, "{"); + const closeFailure = new Error("injected capture close failure"); + const closeCalls = new Map(); + let allowClose = false; + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if (String(args[0]) !== input.capture_path) return handle; + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeCalls.set(handle, (closeCalls.get(handle) ?? 0) + 1); + if (!allowClose) throw closeFailure; + await close(); + }); + return handle; + }); + + try { + const result = await exportWebScripts(input, resources); + if (result.ok) throw new Error("Expected invalid capture failure"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [input.capture_path], + diagnostics: { + primary_error: { + code: "invalid_request", + details: { + issues: [{ path: ["capture_path"], reason: "invalid_format" }], + }, + }, + }, + }, + }); + expect([...closeCalls.values()]).toEqual([2]); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + + await expect(resources.close()).rejects.toMatchObject({ + cleanup: { + resources: [input.capture_path], + reason: expect.stringContaining(closeFailure.message), + }, + }); + expect([...closeCalls.values()]).toEqual([3]); + + allowClose = true; + await resources.close(); + expect([...closeCalls.values()]).toEqual([4]); + } finally { + allowClose = true; + await resources.close().catch(() => undefined); + } + }); + + it("reports a recovered close failure without marking cleanup incomplete", async () => { + const { input, resources } = await setup(); + await writeFile(input.capture_path, "{"); + const closeFailure = new Error("injected one-time capture close failure"); + let closeCalls = 0; + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if (String(args[0]) !== input.capture_path) return handle; + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeCalls += 1; + if (closeCalls === 1) throw closeFailure; + await close(); + }); + return handle; + }); + + const result = await exportWebScripts(input, resources); + if (result.ok) throw new Error("Expected invalid capture failure"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "invalid_request", + details: { + issues: [{ path: ["capture_path"], reason: "invalid_format" }], + }, + }); + expect(result.error.cause).toBeInstanceOf(AggregateError); + const cause = result.error.cause; + if (!(cause instanceof AggregateError)) + throw new Error("Expected close diagnostic in the cause chain"); + expect(cause.errors.map((error) => String(error))).toContain( + closeFailure.toString(), + ); + expect(closeCalls).toBe(2); + await resources.close(); + }); + + it("makes scope close wait for an admitted capture read", async () => { + const { input, resources } = await setup(); + let notifyOpen!: () => void; + let releaseOpen!: () => void; + const opened = new Promise((resolve) => { + notifyOpen = resolve; + }); + const openGate = new Promise((resolve) => { + releaseOpen = resolve; + }); + openMock.mockImplementation(async (...args) => { + const handle = await actualFs.open(...args); + if (String(args[0]) === input.capture_path) { + notifyOpen(); + await openGate; + } + return handle; + }); + + const exporting = exportWebScripts(input, resources); + try { + await opened; + let closeFinished = false; + const closing = resources.close().then(() => { + closeFinished = true; + }); + await new Promise((resolve) => setImmediate(resolve)); + expect(closeFinished).toBe(false); + + releaseOpen(); + await closing; + const result = await exporting; + if (result.ok) + throw new Error("Expected the closed scope to refuse publication"); + expect(result.error.userMessage).toContain("resource scope is closed"); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + releaseOpen(); + await resources.close().catch(() => undefined); + } + }); +});