From be75ed5e4b4bfb146af497f8fa5d172a1ea19462 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 11 Oct 2026 05:16:42 +0800 Subject: [PATCH] fix(publication)!: retain cleanup owners across export workflows Require an ArtifactResourceScope for web script exports and share its lifetime with CLI or MCP shutdown. Retain setup-failed output trees in firmware and Wakaru providers. Preserve completed publications when descriptor cleanup fails, returning the full result after a successful retry or as a partial observation while cleanup remains unresolved. --- src/application/WebScriptExportService.ts | 15 +- src/browser/assets/PublishWebScripts.ts | 157 +++++++----- src/cli/webScriptCommands.ts | 41 ++- src/firmware/FirmwareProvider.ts | 30 ++- src/firmware/FirmwarePublication.ts | 26 +- src/javascript/recovery/WakaruProvider.ts | 78 ++++-- src/server/createServer.ts | 14 +- src/server/registerElectronTools.ts | 4 +- src/server/registerWebScriptTool.ts | 4 +- .../browser/cdpScriptMetadata.test.ts | 4 + .../filesystem/webModuleArtifacts.test.ts | 14 +- .../filesystem/webScriptExport.test.ts | 239 ++++++++++++++---- .../firmware/firmwareIntegration.test.ts | 179 ++++++++++++- .../wakaruIntegration.test.ts | 158 +++++++++++- .../mcp/webScriptSelectionMcp.test.ts | 4 + 15 files changed, 801 insertions(+), 166 deletions(-) diff --git a/src/application/WebScriptExportService.ts b/src/application/WebScriptExportService.ts index 3117cb69a8..f996bd36e0 100644 --- a/src/application/WebScriptExportService.ts +++ b/src/application/WebScriptExportService.ts @@ -4,6 +4,7 @@ import { isAbsolute } from "node:path"; import { z } from "zod"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { publishWebScripts } from "../browser/assets/PublishWebScripts.js"; import { selectScriptCapture } from "../browser/assets/ScriptCaptureAdapters.js"; import { @@ -37,11 +38,12 @@ const OPERATION = "export_web_scripts"; /** Export one local capture through the shared CLI/MCP application workflow. */ export const exportWebScripts = async ( rawInput: unknown, + resources: ArtifactResourceScope, options: ExecutionOptions = {}, ): Promise> => { const input = exportWebScriptsInputSchema.safeParse(rawInput); return input.success - ? exportWebScriptsValidated(input.data, options) + ? exportWebScriptsValidated(input.data, resources, options) : err( analysisInputErrorFromIssues(OPERATION, input.error.issues, rawInput), ); @@ -50,6 +52,7 @@ export const exportWebScripts = async ( /** Publish input already parsed by a named adapter contract. */ export const exportWebScriptsValidated = async ( input: ExportWebScriptsInput, + resources: ArtifactResourceScope, options: ExecutionOptions = {}, ): Promise> => { if (!isAbsolute(input.capture_path) || !isAbsolute(input.output_directory)) @@ -72,12 +75,10 @@ export const exportWebScriptsValidated = async ( if (!loaded.ok) return loaded; options.signal?.throwIfAborted(); const sha256 = createHash("sha256").update(bytes).digest("hex"); - const result = await publishWebScripts( - input, - loaded.value, - sha256, - options.signal, - ); + const result = await publishWebScripts(input, loaded.value, sha256, { + resources, + ...(options.signal === undefined ? {} : { signal: options.signal }), + }); return ok( createEvidence( { path: input.capture_path, sha256, format: "file" }, diff --git a/src/browser/assets/PublishWebScripts.ts b/src/browser/assets/PublishWebScripts.ts index 52439494e4..fb64aa0466 100644 --- a/src/browser/assets/PublishWebScripts.ts +++ b/src/browser/assets/PublishWebScripts.ts @@ -3,8 +3,10 @@ import { join } from "node:path"; import { Readable } from "node:stream"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { DESTINATION_CASE_COLLISION_PREFIX } from "../../artifacts/ArtifactPaths.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../../artifacts/SafeOutputTreeCreationFailure.js"; import { webScriptExportManifestSchema, webScriptExportResultSchema, @@ -27,78 +29,105 @@ const LIMITATIONS = [ "Captured response bytes are browser-decoded bytes; Debugger sources are the exposed text encoded as UTF-8. Redacted bytes are exported exactly as retained, and may no longer parse as JavaScript.", ]; +interface PublishWebScriptsOptions { + readonly resources: ArtifactResourceScope; + readonly signal?: AbortSignal; +} + /** Exclusively publish verified bytes and a manifest, rolling back partial work. */ export const publishWebScripts = async ( input: ExportWebScriptsInput, capture: SelectedScriptCapture & { readonly sourceEvidenceId: string | null }, captureSha256: string, - signal?: AbortSignal, -): Promise => { - const planned = planWebScriptExport(capture.scripts); - const tree = await SafeOutputTree.create(input.output_directory); - try { - signal?.throwIfAborted(); - const records: ExportedWebScript[] = []; - for (const [index, item] of planned.entries()) { - signal?.throwIfAborted(); - records.push(await publishPlannedScript(tree, item, index, signal)); - } - const manifest = webScriptExportManifestSchema.parse({ - capture_path: input.capture_path, - capture_sha256: captureSha256, - capture_kind: capture.kind, - source_evidence_id: capture.sourceEvidenceId, - capture_completeness: capture.completeness, - output_directory: tree.outputRoot, - analysis_input: records.some( - ({ content }) => content.state === "exported", - ) - ? { input_path: join(tree.outputRoot, "files"), format: "directory" } - : null, - scripts: records, - limitations: [ - ...capture.limitations, - ...LIMITATIONS, - ...(records.some(({ content }) => content.state === "exported") - ? [] - : [ - "No script bytes were exportable. Include script sources in inspect_web_page or select response_body in capture_browser_scenario, then capture again.", - ]), - ], - }); - const bytes = Buffer.from(`${JSON.stringify(manifest, null, 2)}\n`); - const digest = createHash("sha256").update(bytes).digest("hex"); - const result = webScriptExportResultSchema.parse({ - ...manifest, - manifest: { - path: join(tree.outputRoot, "manifest.json"), - sha256: digest, - bytes: bytes.length, - }, - }); - await tree.write( - "manifest.json", - Readable.from([bytes]), - { sha256: digest, bytes: bytes.byteLength }, - signal, - ); - signal?.throwIfAborted(); - await tree.commit(); - return result; - } catch (cause: unknown) { + options: PublishWebScriptsOptions, +): Promise => + options.resources.run(async () => { + const planned = planWebScriptExport(capture.scripts); + let tree: SafeOutputTree | undefined; + let result: WebScriptExportResult | undefined; try { - await tree.rollback(); - } catch (cleanupCause: unknown) { - throw new WebScriptExportError( - "io", - tree.outputRoot, - `Publication failed (${message(cause)}); rollback failed (${message(cleanupCause)}). Remove the residual output directory before retrying.`, - [tree.outputRoot], + tree = await SafeOutputTree.create(input.output_directory); + options.signal?.throwIfAborted(); + const records: ExportedWebScript[] = []; + for (const [index, item] of planned.entries()) { + options.signal?.throwIfAborted(); + records.push( + await publishPlannedScript(tree, item, index, options.signal), + ); + } + const manifest = webScriptExportManifestSchema.parse({ + capture_path: input.capture_path, + capture_sha256: captureSha256, + capture_kind: capture.kind, + source_evidence_id: capture.sourceEvidenceId, + capture_completeness: capture.completeness, + output_directory: tree.outputRoot, + analysis_input: records.some( + ({ content }) => content.state === "exported", + ) + ? { input_path: join(tree.outputRoot, "files"), format: "directory" } + : null, + scripts: records, + limitations: [ + ...capture.limitations, + ...LIMITATIONS, + ...(records.some(({ content }) => content.state === "exported") + ? [] + : [ + "No script bytes were exportable. Include script sources in inspect_web_page or select response_body in capture_browser_scenario, then capture again.", + ]), + ], + }); + const bytes = Buffer.from(`${JSON.stringify(manifest, null, 2)}\n`); + const digest = createHash("sha256").update(bytes).digest("hex"); + result = webScriptExportResultSchema.parse({ + ...manifest, + manifest: { + path: join(tree.outputRoot, "manifest.json"), + sha256: digest, + bytes: bytes.length, + }, + }); + await tree.write( + "manifest.json", + Readable.from([bytes]), + { sha256: digest, bytes: bytes.byteLength }, + options.signal, ); + options.signal?.throwIfAborted(); + await tree.commit(); + return result; + } catch (cause: unknown) { + if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree; + if (tree !== undefined) { + const cleanup = await options.resources.release({ + kind: "output-tree", + resource: tree.outputRoot, + tree, + }); + if (cleanup.kind === "failed") { + const cleanupCause = cleanup.cause; + const diagnostic = tree.published + ? `Publication completed, but descriptor cleanup failed (${message(cleanupCause)}). The published output remains available.` + : `Publication failed (${message(cause)}); rollback failed (${message(cleanupCause)}). Remove the residual output directory before retrying.`; + const error = new WebScriptExportError( + cause instanceof ArtifactReaderFailure ? cause.reason : "io", + tree.outputRoot, + diagnostic, + [tree.outputRoot], + ); + if (tree.published && result !== undefined) + error.retainPartialObservation({ + kind: "web-script-export", + result, + }); + throw error; + } + if (tree.published && result !== undefined) return result; + } + throw cause; } - throw cause; - } -}; + }); const publishPlannedScript = async ( tree: SafeOutputTree, diff --git a/src/cli/webScriptCommands.ts b/src/cli/webScriptCommands.ts index 34ae34970e..476789ae9a 100644 --- a/src/cli/webScriptCommands.ts +++ b/src/cli/webScriptCommands.ts @@ -1,9 +1,13 @@ import { Cli, z } from "incur"; import { exportWebScripts } from "../application/WebScriptExportService.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { logCliCommand } from "../cliLogging.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { Logger } from "pino"; /** Register the local captured-script export CLI workflow. */ @@ -26,10 +30,39 @@ export const registerWebScriptCommands = ( }), run: ({ args }) => logCliCommand(logger, CLI_COMMANDS.exportWebScripts, async () => { - const result = await exportWebScripts({ - capture_path: args.capturePath, - output_directory: args.outputDirectory, - }); + const resources = new ArtifactResourceScope(); + const result = await exportWebScripts( + { + capture_path: args.capturePath, + output_directory: args.outputDirectory, + }, + resources, + ); + try { + await resources.close(); + } catch (cause: unknown) { + const cleanup = ArtifactReaderFailure.cleanupObservation( + cause, + "Web script export resources", + ); + const failure = new ProviderCleanupError( + "web-script-export", + cleanup.resources, + { reason: cleanup.reason }, + { cause, operation: "export_web_scripts" }, + ); + const error = result.ok + ? failure + : analysisErrorWithCleanupFailure( + result.error, + failure, + "export_web_scripts", + ); + return { + error: "Script export failed", + ...projectAnalysisError(error), + }; + } return result.ok ? result.value : { diff --git a/src/firmware/FirmwareProvider.ts b/src/firmware/FirmwareProvider.ts index 6f21f22887..31fac4df1f 100644 --- a/src/firmware/FirmwareProvider.ts +++ b/src/firmware/FirmwareProvider.ts @@ -25,6 +25,7 @@ import { jsonValueSchema } from "../domain/jsonValue.js"; import { err, ok } from "../domain/result.js"; import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import type { ProviderProcessSupervisor } from "../process/ProviderProcess.js"; +import type { SafeOutputTree } from "../artifacts/SafeOutputTree.js"; import { publishFirmwareExtraction } from "./FirmwarePublication.js"; import { FIRMWARE_LIMITS, @@ -48,6 +49,7 @@ import { type Outcome = Awaited>; interface PendingFirmwareCleanup { supervisor?: ProviderProcessSupervisor; + tree?: SafeOutputTree; readonly root: PrivateRuntimeRoot; readonly engine: string; readonly operation: string; @@ -183,9 +185,24 @@ export class FirmwareProvider implements FirmwareAnalysisPort { ); continue; } + try { + if (pending.tree !== undefined) { + await pending.tree.rollback(); + delete pending.tree; + } + } catch (cause: unknown) { + failures.push( + new ProviderCleanupError( + pending.engine, + [path, pending.tree?.outputRoot ?? path], + { reason: cause instanceof Error ? cause.message : String(cause) }, + { operation: pending.operation, cause }, + ), + ); + } try { await pending.root.close(); - this.#pendingCleanup.delete(path); + if (pending.tree === undefined) this.#pendingCleanup.delete(path); } catch (cause: unknown) { // A verified stopped process cannot use this root. Keep ownership for // later removal without blocking a new operation's disjoint workspace. @@ -366,6 +383,17 @@ export class FirmwareProvider implements FirmwareAnalysisPort { selection: target.selection, engine, signal, + retainCleanup: (tree) => { + const pending: PendingFirmwareCleanup = this.#pendingCleanup.get( + ownedRoot.path, + ) ?? { + root: ownedRoot, + engine: engineName, + operation: request.operation, + }; + pending.tree = tree; + this.#pendingCleanup.set(ownedRoot.path, pending); + }, }); raw = { report, execution: processRun, version_execution: versionRun }; } diff --git a/src/firmware/FirmwarePublication.ts b/src/firmware/FirmwarePublication.ts index 47f9c3a5b5..bc53084b1b 100644 --- a/src/firmware/FirmwarePublication.ts +++ b/src/firmware/FirmwarePublication.ts @@ -2,8 +2,10 @@ import { createReadStream } from "node:fs"; import { join } from "node:path"; import type { z } from "zod"; import { SafeOutputTree } from "../artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../artifacts/SafeOutputTreeCreationFailure.js"; import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { jsonValueSchema } from "../domain/jsonValue.js"; import { firmwareResultSchemas } from "../domain/firmware/firmwareAnalysis.js"; import { hashFirmwareFile, type FirmwareEntry } from "./FirmwareFiles.js"; import type { normalizeUnblobReport } from "./FirmwareReports.js"; @@ -16,8 +18,12 @@ export const publishFirmwareExtraction = async (context: { selection: { offset: number; length: number; sha256: string }; engine: z.infer["engine"]; signal: AbortSignal; + retainCleanup: (tree: SafeOutputTree) => void; }) => { let tree: SafeOutputTree | undefined; + let result: + | z.infer + | undefined; try { const materialized = new Set( context.entries @@ -75,7 +81,7 @@ export const publishFirmwareExtraction = async (context: { original_file_range: null, }); } - const result = firmwareResultSchemas.extract_firmware.parse({ + result = firmwareResultSchemas.extract_firmware.parse({ engine: context.engine, selection: context.selection, output_directory: tree.outputRoot, @@ -93,9 +99,18 @@ export const publishFirmwareExtraction = async (context: { await tree.commit(); return result; } catch (cause: unknown) { + if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree; try { await tree?.rollback(); } catch (cleanupCause: unknown) { + if (tree !== undefined) context.retainCleanup(tree); + const publishedResult = + tree?.published === true && result !== undefined + ? { + kind: "firmware" as const, + result: jsonValueSchema.parse(result), + } + : undefined; throw new ProviderCleanupError( "unblob", [context.outputDirectory], @@ -107,9 +122,16 @@ export const publishFirmwareExtraction = async (context: { previous_error: cause instanceof Error ? cause.message : String(cause), }, - { cause: cleanupCause }, + { + cause: cleanupCause, + operation: "extract_firmware", + ...(publishedResult === undefined + ? {} + : { partialObservation: publishedResult }), + }, ); } + if (tree?.published === true && result !== undefined) return result; throw cause; } }; diff --git a/src/javascript/recovery/WakaruProvider.ts b/src/javascript/recovery/WakaruProvider.ts index 8f3181f2cb..008f480968 100644 --- a/src/javascript/recovery/WakaruProvider.ts +++ b/src/javascript/recovery/WakaruProvider.ts @@ -7,9 +7,13 @@ import { type ExecutionOptions, } from "../../application/AnalysisProvider.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../../artifacts/SafeOutputTreeCreationFailure.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; -import type { JavaScriptRecoveryInput } from "../../domain/javascript/javascriptRecovery.js"; +import { + javascriptRecoveryResultSchema, + type JavaScriptRecoveryInput, +} from "../../domain/javascript/javascriptRecovery.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; import { err, ok, type Result } from "../../domain/result.js"; @@ -94,7 +98,7 @@ export class WakaruProvider implements JavaScriptRecoveryPort { if (pending !== undefined) return err(pending); let root: string | undefined; let tree: SafeOutputTree | undefined; - let failure: unknown; + let failed: { readonly cause: unknown } | undefined; let execution: AnalysisExecution | undefined; const deadline = Date.now() + RECOVERY_LIMITS.timeoutMs; try { @@ -110,15 +114,16 @@ export class WakaruProvider implements JavaScriptRecoveryPort { join(inputs, "bundle.js"), options?.signal, ); - tree = await SafeOutputTree.create(input.output_directory).catch( - (cause: unknown) => { - throw recoveryInputFailure( - "output_directory", - `Cannot create new recovery directory ${input.output_directory}: ${recoveryFailureMessage(cause)}`, - cause, - ); - }, - ); + try { + tree = await SafeOutputTree.create(input.output_directory); + } catch (cause: unknown) { + if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree; + throw recoveryInputFailure( + "output_directory", + `Cannot create new recovery directory ${input.output_directory}: ${recoveryFailureMessage(cause)}`, + cause, + ); + } execution = await prepareWakaruExecution({ root, tree, @@ -136,10 +141,12 @@ export class WakaruProvider implements JavaScriptRecoveryPort { checkRecoveryDeadline(deadline, options?.signal); await tree.commit(); } catch (cause: unknown) { - failure = cause; + failed = { cause }; } - if (failure !== undefined) { + if (failed !== undefined) { + const failure = failed.cause; const residuals: string[] = []; + const cleanupFailures: { resource: string; reason: string }[] = []; const cleanupOwner = failure instanceof WakaruCleanupFailure ? failure.cleanupOwner @@ -147,10 +154,14 @@ export class WakaruProvider implements JavaScriptRecoveryPort { let failedTree: SafeOutputTree | undefined; try { await tree?.rollback(); - } catch { + } catch (cause: unknown) { if (tree !== undefined) { failedTree = tree; residuals.push(tree.outputRoot); + cleanupFailures.push({ + resource: tree.outputRoot, + reason: recoveryFailureMessage(cause), + }); } } if (cleanupOwner !== undefined) @@ -160,8 +171,12 @@ export class WakaruProvider implements JavaScriptRecoveryPort { try { await rm(failedRoot, { recursive: true, force: true }); root = undefined; - } catch { + } catch (cause: unknown) { residuals.push(failedRoot); + cleanupFailures.push({ + resource: failedRoot, + reason: recoveryFailureMessage(cause), + }); } } if ( @@ -179,10 +194,37 @@ export class WakaruProvider implements JavaScriptRecoveryPort { } if (residuals.length > 0) return err( - new ProviderCleanupError("wakaru", residuals, { - previous_error: recoveryFailureMessage(failure), - }), + new ProviderCleanupError( + "wakaru", + residuals, + { + ...(failure instanceof WakaruCleanupFailure + ? failure.diagnostics + : {}), + previous_error: + failure instanceof WakaruCleanupFailure + ? (failure.diagnostics?.previous_error ?? null) + : recoveryFailureMessage(failure), + cleanup_failures: cleanupFailures, + }, + { + operation: OPERATION, + cause: failure, + ...(tree?.published && execution !== undefined + ? { + partialObservation: { + kind: "javascript-recovery" as const, + result: javascriptRecoveryResultSchema.parse( + execution.result, + ), + }, + } + : {}), + }, + ), ); + if (tree?.published === true && execution !== undefined) + return ok(execution); return err( failure instanceof AnalysisError ? failure diff --git a/src/server/createServer.ts b/src/server/createServer.ts index 3b55a49272..ff1a338351 100644 --- a/src/server/createServer.ts +++ b/src/server/createServer.ts @@ -211,7 +211,7 @@ export const createServer = ( options.webSourceLocation ?? createWebSourceLocationService(environment); const javascriptRecovery = options.javascriptRecovery ?? createJavaScriptRecoveryProvider(environment); - const javascriptApplicationResources = new ArtifactResourceScope(); + const artifactResources = new ArtifactResourceScope(); const firmwareAnalysis = new FirmwareAnalysisService( options.firmwareAnalysis ?? createFirmwareAnalysisProvider(environment), ); @@ -248,7 +248,7 @@ export const createServer = ( evmInterface.close(), webSourceLocation.close(), javascriptRecovery.close?.() ?? Promise.resolve(), - javascriptApplicationResources.close(), + artifactResources.close(), firmwareAnalysis.close(), options.browserScenarioCapture?.close?.() ?? Promise.resolve(), ]) @@ -289,7 +289,7 @@ export const createServer = ( recordEvidence, recordEvidenceWithUnknown, withAdmittedAnalysis: analysisAdmission, - javascriptApplicationResources, + artifactResources, }; registerBinaryAnalysisTools(toolContext); const previousOnclose = server.server.onclose; @@ -501,7 +501,7 @@ const createSessionRecorders = ( }); interface ServerToolContext extends ReturnType { - readonly javascriptApplicationResources: ArtifactResourceScope; + readonly artifactResources: ArtifactResourceScope; readonly server: EvidenceMcpServer; readonly session: BinarySessionPort | undefined; readonly options: CreateServerOptions; @@ -572,10 +572,10 @@ const registerObservationTools = ({ recordEvidence, recordEvidenceWithUnknown, withAdmittedAnalysis, - javascriptApplicationResources, + artifactResources, }: ServerToolContext): void => { const common = { logger, recordEvidence, withAdmittedAnalysis }; - registerWebScriptTool(server, common); + registerWebScriptTool(server, { ...common, artifactResources }); registerBrowserTools(server, { ...common, browser: options.browserObservation, @@ -588,7 +588,7 @@ const registerObservationTools = ({ }); registerElectronTools(server, { ...common, - javascriptApplicationResources, + artifactResources, evidenceById, electron: options.electronObservation, electronActive: options.electronActiveObservation, diff --git a/src/server/registerElectronTools.ts b/src/server/registerElectronTools.ts index 167d41eac7..a0478f310e 100644 --- a/src/server/registerElectronTools.ts +++ b/src/server/registerElectronTools.ts @@ -35,7 +35,7 @@ import type { WithAdmittedAnalysis } from "./analysisAdmission.js"; import { runAdmittedToolOperation } from "./admittedToolOperation.js"; interface ElectronToolRegistration { - readonly javascriptApplicationResources: ArtifactResourceScope; + readonly artifactResources: ArtifactResourceScope; readonly logger: Logger; readonly observationLoadFailure?: OptionalProviderLoadFailure | undefined; readonly activeLoadFailure?: OptionalProviderLoadFailure | undefined; @@ -106,7 +106,7 @@ export const registerElectronTools = ( async ({ detail, ...request }, { signal, progress }) => { const analyzed = await analyzeJavaScriptApplicationValidated( request, - options.javascriptApplicationResources, + options.artifactResources, { signal, progress }, ); return detail === "summary" && analyzed.ok diff --git a/src/server/registerWebScriptTool.ts b/src/server/registerWebScriptTool.ts index 37a4bc8353..05427e73e5 100644 --- a/src/server/registerWebScriptTool.ts +++ b/src/server/registerWebScriptTool.ts @@ -8,6 +8,7 @@ import { logToolExecution } from "./toolLogging.js"; import { toolRegistrationOptions } from "./toolRegistrationOptions.js"; import type { WithAdmittedAnalysis } from "./analysisAdmission.js"; import { runAdmittedToolOperation } from "./admittedToolOperation.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; /** Register local script export without requiring a live browser provider. */ export const registerWebScriptTool = ( @@ -15,6 +16,7 @@ export const registerWebScriptTool = ( options: { readonly logger: Logger; readonly recordEvidence: EvidenceWriter["recordEvidence"] | undefined; + readonly artifactResources: ArtifactResourceScope; readonly withAdmittedAnalysis?: WithAdmittedAnalysis; }, ): void => { @@ -33,7 +35,7 @@ export const registerWebScriptTool = ( options.logger, contract.name, () => - exportWebScriptsValidated(input, { + exportWebScriptsValidated(input, options.artifactResources, { signal: context.mcpReq.signal, }), ); diff --git a/tests/boundary/browser/cdpScriptMetadata.test.ts b/tests/boundary/browser/cdpScriptMetadata.test.ts index 37b1eb13f0..41d89fba41 100644 --- a/tests/boundary/browser/cdpScriptMetadata.test.ts +++ b/tests/boundary/browser/cdpScriptMetadata.test.ts @@ -9,6 +9,7 @@ import { createJavaScriptRuntimeObservationEvidence } from "../../../src/applica import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; import { CdpElectronProvider } from "../../../src/browser/CdpElectronProvider.js"; import { publishWebScripts } from "../../../src/browser/assets/PublishWebScripts.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { selectScriptCapture } from "../../../src/browser/assets/ScriptCaptureAdapters.js"; import { inspectWebPageInputSchema } from "../../../src/domain/browserObservation.js"; import { inspectElectronPageInputSchema } from "../../../src/domain/javascript/electronObservation.js"; @@ -69,6 +70,8 @@ describe.each(["browser", "electron"] as const)( "$name through capture and consumers", async ({ reported, expected }) => { const root = await fixtureRoot(); + const artifactResources = new ArtifactResourceScope(); + resources.push(() => artifactResources.close()); const browser = await startFakeCdpBrowser({ ...(kind === "electron" ? { electronFileUrl: pathToFileURL(join(root, "index.html")).href } @@ -129,6 +132,7 @@ describe.each(["browser", "electron"] as const)( }, selectScriptCapture(result.value), createHash("sha256").update(captureBytes).digest("hex"), + { resources: artifactResources }, ); expect(exported.scripts[0]?.source).toMatchObject({ is_module: expected.is_module, diff --git a/tests/boundary/filesystem/webModuleArtifacts.test.ts b/tests/boundary/filesystem/webModuleArtifacts.test.ts index 62b23432a0..549d038313 100644 --- a/tests/boundary/filesystem/webModuleArtifacts.test.ts +++ b/tests/boundary/filesystem/webModuleArtifacts.test.ts @@ -10,6 +10,7 @@ import { import { dirname, join } from "node:path"; import { describe, expect, it, onTestFinished } from "vitest"; import { exportWebScripts } from "../../../src/application/WebScriptExportService.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { LocalWebModuleArtifacts } from "../../../src/browser/modules/WebModuleArtifacts.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { webScriptExportResultSchema } from "../../../src/domain/webScriptExport.js"; @@ -19,16 +20,21 @@ import { scriptScenarioFixture } from "../../fixtures/webScriptCapture.js"; const fixture = async (bytes = Buffer.from('import "./dep.js";')) => { const root = await createTestTempDirectory("rea-module-artifacts-"); const capturePath = join(root, "capture.json"); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); await writeFile( capturePath, JSON.stringify( scriptScenarioFixture([{ url: "https://app.test/main.js", bytes }]), ), ); - const response = await exportWebScripts({ - capture_path: capturePath, - output_directory: join(root, "export"), - }); + const response = await exportWebScripts( + { + capture_path: capturePath, + output_directory: join(root, "export"), + }, + resources, + ); if (!response.ok) throw response.error; const result = webScriptExportResultSchema.parse( response.value.normalized_result, diff --git a/tests/boundary/filesystem/webScriptExport.test.ts b/tests/boundary/filesystem/webScriptExport.test.ts index 87f05c545b..c7fbda94e2 100644 --- a/tests/boundary/filesystem/webScriptExport.test.ts +++ b/tests/boundary/filesystem/webScriptExport.test.ts @@ -12,11 +12,14 @@ import { import { join } from "node:path"; import { promisify } from "node:util"; -import { describe, expect, it, onTestFinished } from "vitest"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; import { analyzeJavaScriptApplication } from "../../support/javascriptApplicationScope.js"; import { exportWebScripts } from "../../../src/application/WebScriptExportService.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { publishWebScripts } from "../../../src/browser/assets/PublishWebScripts.js"; +import { SafeOutputTree } from "../../../src/artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../../../src/artifacts/SafeOutputTreeCreationFailure.js"; import { selectScriptCapture } from "../../../src/browser/assets/ScriptCaptureAdapters.js"; import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; @@ -30,12 +33,14 @@ import { const setup = async (capture: unknown = scriptCaptureEvidenceFixture()) => { const root = await createTestTempDirectory("rea-web-script-export-"); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); const input = { capture_path: join(root, "capture.json"), output_directory: join(root, "export"), }; await writeFile(input.capture_path, JSON.stringify(capture)); - return { root, input }; + return { root, input, resources }; }; describe("captured script publication boundary", () => { @@ -52,8 +57,8 @@ describe("captured script publication boundary", () => { bytes: Buffer.from("export const marker = 'source-owned';\n"), }, ]); - const { input } = await setup(capture); - const exported = await exportWebScripts(input); + const { input, resources } = await setup(capture); + const exported = await exportWebScripts(input, resources); if (!exported.ok) throw exported.error; const result = webScriptExportResultSchema.parse( exported.value.normalized_result, @@ -106,20 +111,20 @@ describe("captured script publication boundary", () => { }); it("links authenticated input Evidence and preserves binary and empty source bytes", async () => { - const { input } = await setup(); - const result = await exportWebScripts(input); + const { input, resources } = await setup(); + const result = await exportWebScripts(input, resources); if (!result.ok) throw result.error; expect(result.value.evidence_links).toEqual([ scriptCaptureEvidenceFixture().evidence_id, ]); const binary = Buffer.from([255, 0, 254, 1]); - const { input: binaryInput } = await setup( + const { input: binaryInput, resources: binaryResources } = await setup( scriptScenarioFixture([ { url: "https://fixture.test/binary.js", bytes: binary }, { url: "https://fixture.test/empty.js", bytes: Buffer.alloc(0) }, ]), ); - const binaryExport = await exportWebScripts(binaryInput); + const binaryExport = await exportWebScripts(binaryInput, binaryResources); if (!binaryExport.ok) throw binaryExport.error; const parsed = webScriptExportResultSchema.parse( binaryExport.value.normalized_result, @@ -149,8 +154,8 @@ describe("captured script publication boundary", () => { ({ kind }) => kind !== "network-content", ); capture.events.retained = capture.events.items.length; - const { input } = await setup(capture); - const result = await exportWebScripts(input); + const { input, resources } = await setup(capture); + const result = await exportWebScripts(input, resources); if (!result.ok) throw result.error; const parsed = webScriptExportResultSchema.parse( result.value.normalized_result, @@ -163,12 +168,14 @@ describe("captured script publication boundary", () => { describe("captured script publication failures and cleanup", () => { it("preserves adapter-reported limitations without strengthening their string contract", async () => { - const { input } = await setup(); + const { input, resources } = await setup(); const capture = { ...selectScriptCapture(scriptScenarioFixture()), limitations: ["", "producer-reported limitation"], }; - const result = await publishWebScripts(input, capture, "a".repeat(64)); + const result = await publishWebScripts(input, capture, "a".repeat(64), { + resources, + }); expect(result.limitations.slice(0, 2)).toEqual(capture.limitations); }); @@ -179,9 +186,9 @@ describe("captured script publication failures and cleanup", () => { ])( "rejects malformed capture bytes before creating output", async (bytes) => { - const { input } = await setup(); + const { input, resources } = await setup(); await writeFile(input.capture_path, bytes); - const result = await exportWebScripts(input); + const result = await exportWebScripts(input, resources); if (result.ok) throw new Error("Expected invalid input"); expect(result.error._tag).toBe("AnalysisInputError"); await expect(access(input.output_directory)).rejects.toMatchObject({ @@ -191,7 +198,7 @@ describe("captured script publication failures and cleanup", () => { ); it("preserves an existing destination and refuses output through a symlink", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const marker = join(root, "marker.js"); await writeFile(marker, "keep"); await symlink( @@ -199,14 +206,14 @@ describe("captured script publication failures and cleanup", () => { input.output_directory, process.platform === "win32" ? "junction" : "dir", ); - const result = await exportWebScripts(input); + const result = await exportWebScripts(input, resources); if (result.ok) throw new Error("Expected exclusive output failure"); expect(result.error.userMessage).toContain("already exists"); expect(await readFile(marker, "utf8")).toBe("keep"); }); it("rolls back only the new owned output when durable byte verification fails", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const capture = selectScriptCapture( scriptScenarioFixture([ { url: "https://fixture.test/a.js", bytes: Buffer.from("first") }, @@ -223,16 +230,132 @@ describe("captured script publication failures and cleanup", () => { ].filter((value) => value !== undefined), }; await expect( - publishWebScripts(input, broken, "a".repeat(64)), + publishWebScripts(input, broken, "a".repeat(64), { resources }), ).rejects.toMatchObject({ reason: "integrity" }); expect(await readdir(root)).toEqual(["capture.json"]); }); +}); +describe("captured script output owner retry", () => { + it("retains a setup-failed output tree in the caller resource scope", async () => { + const { input, resources } = await setup(); + const capture = selectScriptCapture(scriptScenarioFixture()); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + let cleanupAttempts = 0; + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const rollbackTree = tree.rollback.bind(tree); + vi.spyOn(tree, "rollback").mockImplementation(async () => { + cleanupAttempts += 1; + if (cleanupAttempts === 1) + throw new Error("injected first cleanup failure"); + return rollbackTree(); + }); + throw new SafeOutputTreeCreationFailure( + new Error("injected post-acquisition setup failure"), + tree, + ); + }); + try { + await expect( + publishWebScripts(input, capture, "a".repeat(64), { resources }), + ).rejects.toMatchObject({ cleanupIncomplete: true }); + await expect(access(input.output_directory)).resolves.toBeUndefined(); + } finally { + create.mockRestore(); + } + await resources.close(); + expect(cleanupAttempts).toBe(2); + await expect(access(input.output_directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + + 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") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const commit = tree.commit.bind(tree); + vi.spyOn(tree, "commit").mockImplementation(async () => { + await commit(); + throw new Error("injected descriptor close failure"); + }); + return tree; + }); + try { + const result = await exportWebScripts(input, resources); + if (!result.ok) throw result.error; + const published = webScriptExportResultSchema.parse( + result.value.normalized_result, + ); + expect(published.manifest.path).toBe( + join(input.output_directory, "manifest.json"), + ); + await expect(access(published.manifest.path)).resolves.toBeUndefined(); + } finally { + create.mockRestore(); + } + }); + + it("retains published output and its result while cleanup remains unresolved", async () => { + const { input, resources } = await setup(); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + let cleanupAttempts = 0; + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const commit = tree.commit.bind(tree); + vi.spyOn(tree, "commit").mockImplementation(async () => { + await commit(); + throw new Error("injected descriptor close failure"); + }); + const rollback = tree.rollback.bind(tree); + vi.spyOn(tree, "rollback").mockImplementation(async () => { + cleanupAttempts += 1; + if (cleanupAttempts === 1) + throw new Error("injected cleanup refusal"); + return rollback(); + }); + return tree; + }); + try { + const result = await exportWebScripts(input, resources); + if (result.ok) throw new Error("Expected unresolved cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.userMessage).not.toContain("Remove the residual"); + expect(result.error.partialObservation).toMatchObject({ + kind: "web-script-export", + result: { + output_directory: input.output_directory, + manifest: { path: join(input.output_directory, "manifest.json") }, + }, + }); + await expect( + access(join(input.output_directory, "manifest.json")), + ).resolves.toBeUndefined(); + await resources.close(); + expect(cleanupAttempts).toBe(2); + } finally { + create.mockRestore(); + } + }); +}); + +describe("captured script publication failures and cleanup", () => { it("cancels before work and rolls back cancellation after output creation", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const controller = new AbortController(); controller.abort(); - const result = await exportWebScripts(input, { signal: controller.signal }); + const result = await exportWebScripts(input, resources, { + signal: controller.signal, + }); if (result.ok) throw new Error("Expected cancellation"); expect(result.error._tag).toBe("AnalysisCancelledError"); await expect( @@ -240,18 +363,21 @@ describe("captured script publication failures and cleanup", () => { input, selectScriptCapture(scriptScenarioFixture()), "a".repeat(64), - controller.signal, + { resources, signal: controller.signal }, ), ).rejects.toBeDefined(); expect(await readdir(root)).toEqual(["capture.json"]); }); it("identifies a missing capture file and rejected host path syntax", async () => { - const { input } = await setup(); - const missing = await exportWebScripts({ - ...input, - capture_path: `${input.capture_path}.missing`, - }); + const { input, resources } = await setup(); + const missing = await exportWebScripts( + { + ...input, + capture_path: `${input.capture_path}.missing`, + }, + resources, + ); if (missing.ok) throw new Error("Expected unavailable input"); expect(projectAnalysisError(missing.error)).toMatchObject({ code: "invalid_request", @@ -265,10 +391,13 @@ describe("captured script publication failures and cleanup", () => { ], }, }); - const relative = await exportWebScripts({ - ...input, - capture_path: "capture.json", - }); + const relative = await exportWebScripts( + { + ...input, + capture_path: "capture.json", + }, + resources, + ); if (relative.ok) throw new Error("Expected host path error"); expect(relative.error._tag).toBe("AnalysisInputError"); }); @@ -276,11 +405,14 @@ describe("captured script publication failures and cleanup", () => { describe("captured script selection failures", () => { it("accepts a capture symlink to a regular file and preserves its selected path", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const selected = join(root, "selected-capture.json"); await symlink(input.capture_path, selected, "file"); - const result = await exportWebScripts({ ...input, capture_path: selected }); + const result = await exportWebScripts( + { ...input, capture_path: selected }, + resources, + ); if (!result.ok) throw result.error; expect(result.value.subject?.local_path).toBe(selected); expect( @@ -301,7 +433,7 @@ describe("captured script selection failures", () => { .each(["a named pipe", "a symlink to a named pipe"])( "rejects %s without waiting for a writer or creating output", async (kind) => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const fifoPath = join(root, "capture.pipe"); await promisify(execFile)("mkfifo", [fifoPath]); const selected = @@ -311,7 +443,7 @@ describe("captured script selection failures", () => { if (selected !== fifoPath) await symlink(fifoPath, selected, "file"); const outcome = await readWithoutFifoWriter(fifoPath, () => - exportWebScripts({ ...input, capture_path: selected }), + exportWebScripts({ ...input, capture_path: selected }, resources), ); expect(outcome.state).toBe("completed"); if (outcome.state !== "completed") @@ -342,11 +474,14 @@ describe("captured script selection failures", () => { it.skipIf(process.platform === "win32")( "rejects a character device as a capture selection before parsing bytes", async () => { - const { input } = await setup(); - const result = await exportWebScripts({ - ...input, - capture_path: "/dev/null", - }); + const { input, resources } = await setup(); + const result = await exportWebScripts( + { + ...input, + capture_path: "/dev/null", + }, + resources, + ); if (result.ok) throw new Error("Expected invalid capture selection"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "invalid_request", @@ -372,17 +507,16 @@ describe("captured script selection failures", () => { it.skipIf(process.platform === "win32")( "honors a pre-aborted capture export before opening a named pipe", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const fifoPath = join(root, "capture.pipe"); await promisify(execFile)("mkfifo", [fifoPath]); const controller = new AbortController(); controller.abort(); const outcome = await readWithoutFifoWriter(fifoPath, () => - exportWebScripts( - { ...input, capture_path: fifoPath }, - { signal: controller.signal }, - ), + exportWebScripts({ ...input, capture_path: fifoPath }, resources, { + signal: controller.signal, + }), ); expect(outcome.state).toBe("completed"); if (outcome.state !== "completed") @@ -398,14 +532,17 @@ describe("captured script selection failures", () => { describe("captured script path and permission failures", () => { it("reports a directory selected as the capture as invalid input", async () => { - const { root, input } = await setup(); + const { root, input, resources } = await setup(); const directory = join(root, "captures"); await mkdir(directory); - const result = await exportWebScripts({ - ...input, - capture_path: directory, - }); + const result = await exportWebScripts( + { + ...input, + capture_path: directory, + }, + resources, + ); if (result.ok) throw new Error("Expected invalid input"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "invalid_request", @@ -427,11 +564,11 @@ describe("captured script path and permission failures", () => { it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( "reports an unreadable capture as a host access denial", async () => { - const { input } = await setup(); + const { input, resources } = await setup(); await chmod(input.capture_path, 0o000); onTestFinished(() => chmod(input.capture_path, 0o600)); - const result = await exportWebScripts(input); + const result = await exportWebScripts(input, resources); if (result.ok) throw new Error("Expected access denial"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "access_denied", diff --git a/tests/boundary/firmware/firmwareIntegration.test.ts b/tests/boundary/firmware/firmwareIntegration.test.ts index 4cce5a73db..8fee7e83c9 100644 --- a/tests/boundary/firmware/firmwareIntegration.test.ts +++ b/tests/boundary/firmware/firmwareIntegration.test.ts @@ -1,6 +1,6 @@ import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server"; import { access, mkdir, readFile, writeFile } from "node:fs/promises"; -import { expect, it as test } from "vitest"; +import { expect, it as test, vi } from "vitest"; import { firmwareFixture, assertFirmwareCleanup, @@ -9,8 +9,45 @@ import { firmwareResultSchemas } from "../../../src/domain/firmware/firmwareAnal import { FirmwareAnalysisService } from "../../../src/application/firmware/FirmwareAnalysisService.js"; import { FirmwareProvider } from "../../../src/firmware/FirmwareProvider.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { toolContract } from "../../../src/contracts/toolContracts.js"; import { ToolResultDelivery } from "../../../src/server/toolResult.js"; +import { SafeOutputTree } from "../../../src/artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../../../src/artifacts/SafeOutputTreeCreationFailure.js"; + +const directoryCloseFailure = vi.hoisted(() => ({ + mode: "off" as "off" | "transient" | "persistent", + attempts: 0, +})); + +vi.mock("node:fs/promises", async (importOriginal) => { + const filesystem = await importOriginal(); + return { + ...filesystem, + open: async (...args: Parameters) => { + const handle = await filesystem.open(...args); + if ( + directoryCloseFailure.mode !== "off" && + (await handle.stat()).isDirectory() + ) { + const close = handle.close.bind(handle); + let attempted = false; + handle.close = async () => { + directoryCloseFailure.attempts += 1; + if ( + directoryCloseFailure.mode === "persistent" || + (directoryCloseFailure.mode === "transient" && !attempted) + ) { + attempted = true; + throw new Error("injected published directory close failure"); + } + await close(); + }; + } + return handle; + }, + }; +}); const delivery = new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE); @@ -111,6 +148,146 @@ it("extracts a selected interval with verified file identity and parent-byte lin await assertFirmwareCleanup(fixture.launches); }); +it("retains a setup-failed publication tree for provider cleanup retry", async () => { + const fixture = await firmwareFixture(); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + let cleanupAttempts = 0; + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const rollbackTree = tree.rollback.bind(tree); + vi.spyOn(tree, "rollback").mockImplementation(async () => { + cleanupAttempts += 1; + if (cleanupAttempts === 1) + throw new Error("injected first cleanup failure"); + return rollbackTree(); + }); + throw new SafeOutputTreeCreationFailure( + new Error("injected post-acquisition setup failure"), + tree, + ); + }); + try { + const result = await fixture.service.execute("extract_firmware", { + path: fixture.path, + output_directory: fixture.output, + range: { offset: 2, length: 4 }, + }); + expect(result).toMatchObject({ + ok: false, + error: { + cleanupIncomplete: true, + diagnostics: { + previous_error: expect.stringContaining( + "injected post-acquisition setup failure", + ), + }, + }, + }); + await expect(access(fixture.output)).resolves.toBeUndefined(); + } finally { + create.mockRestore(); + } + await fixture.close(); + expect(cleanupAttempts).toBe(2); + await expect(access(fixture.output)).rejects.toMatchObject({ + code: "ENOENT", + }); +}); + +it("returns firmware output after a transient published-directory close failure", async () => { + const fixture = await firmwareFixture(); + const closeFailure = injectPublishedDirectoryCloseFailure(false); + try { + const result = await fixture.service.execute("extract_firmware", { + path: fixture.path, + output_directory: fixture.output, + range: { offset: 2, length: 4 }, + }); + if (!result.ok) throw result.error; + const normalized = firmwareResultSchemas.extract_firmware.parse( + result.value.normalized_result, + ); + expect(normalized.output_directory).toBe(fixture.output); + expect(await readFile(normalized.files[0]?.path ?? "", "utf8")).toBe( + "firmware=true\n", + ); + expect(closeFailure.attempts()).toBeGreaterThanOrEqual(4); + } finally { + closeFailure.restore(); + } +}); + +it("retains the complete firmware result when published output cleanup stays uncertain", async () => { + const fixture = await firmwareFixture(); + const closeFailure = injectPublishedDirectoryCloseFailure(true); + try { + const result = await fixture.service.execute("extract_firmware", { + path: fixture.path, + output_directory: fixture.output, + range: { offset: 2, length: 4 }, + }); + expect(result).toMatchObject({ + ok: false, + error: { + cleanupIncomplete: true, + partialObservation: { + kind: "firmware", + result: { + output_directory: fixture.output, + files: [ + expect.objectContaining({ + relative_path: "rootfs/config", + size: 14, + }), + ], + }, + }, + }, + }); + if (result.ok) throw new Error("Expected unresolved descriptor cleanup"); + expect(result.error).toBeInstanceOf(ProviderCleanupError); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + partial_observation: { + kind: "firmware", + result: { output_directory: fixture.output }, + }, + }, + }); + expect(await readFile(`${fixture.output}/rootfs/config`, "utf8")).toBe( + "firmware=true\n", + ); + expect(closeFailure.attempts()).toBeGreaterThanOrEqual(4); + } finally { + closeFailure.restore(); + } + await fixture.close(); + expect(await readFile(`${fixture.output}/rootfs/config`, "utf8")).toBe( + "firmware=true\n", + ); +}); + +const injectPublishedDirectoryCloseFailure = (persistent: boolean) => { + directoryCloseFailure.attempts = 0; + const commit = SafeOutputTree.prototype.commit; + const spy = vi + .spyOn(SafeOutputTree.prototype, "commit") + .mockImplementation(async function (this: SafeOutputTree) { + directoryCloseFailure.mode = persistent ? "persistent" : "transient"; + await commit.call(this); + }); + return { + attempts: () => directoryCloseFailure.attempts, + restore: () => { + directoryCloseFailure.mode = "off"; + spy.mockRestore(); + }, + }; +}; + it.each(["depth", "dependency", "link"])( "reports partial extraction for %s without losing inline results", async (mode) => { diff --git a/tests/boundary/javascript-recovery/wakaruIntegration.test.ts b/tests/boundary/javascript-recovery/wakaruIntegration.test.ts index a46009f3bf..40f27b332c 100644 --- a/tests/boundary/javascript-recovery/wakaruIntegration.test.ts +++ b/tests/boundary/javascript-recovery/wakaruIntegration.test.ts @@ -1,6 +1,8 @@ import { access, mkdir, readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { SafeOutputTree } from "../../../src/artifacts/SafeOutputTree.js"; +import { SafeOutputTreeCreationFailure } from "../../../src/artifacts/SafeOutputTreeCreationFailure.js"; import { javascriptRecoveryResultSchema } from "../../../src/domain/javascript/javascriptRecovery.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; @@ -73,9 +75,10 @@ describe.skipIf(process.platform !== "linux" || process.arch !== "x64")( }); if (result.ok) throw new Error("expected boundary failure"); const error = projectAnalysisError(result.error); - expect(["execution_failure", "unreadable_output"]).toContain( - error.code, - ); + expect( + ["execution_failure", "unreadable_output"], + JSON.stringify(error), + ).toContain(error.code); expect(error.details).toBeDefined(); await expect(access(fixture.output)).rejects.toMatchObject({ code: "ENOENT", @@ -220,6 +223,9 @@ describe.skipIf(process.platform !== "linux" || process.arch !== "x64")( expect(projectAnalysisError(result.error).code).toBe( "cleanup_incomplete", ); + expect(result.error).toMatchObject({ + diagnostics: { reason: "injected cleanup refusal" }, + }); const launch = fixture.launches[0]; if (launch?.cwd === undefined) throw new Error("missing process ownership"); @@ -246,3 +252,147 @@ describe.skipIf(process.platform !== "linux" || process.arch !== "x64")( }); }, ); + +describe.skipIf(process.platform !== "linux" || process.arch !== "x64")( + "Wakaru publication ownership", + () => { + it("retains a setup-failed publication tree for provider cleanup retry", async () => { + const fixture = await recoveryFixture(); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + let cleanupAttempts = 0; + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const rollbackTree = tree.rollback.bind(tree); + vi.spyOn(tree, "rollback").mockImplementation(async () => { + cleanupAttempts += 1; + if (cleanupAttempts === 1) + throw new Error("injected first cleanup failure"); + return rollbackTree(); + }); + throw new SafeOutputTreeCreationFailure( + new Error("injected post-acquisition setup failure"), + tree, + ); + }); + try { + const result = await fixture.service.recover({ + path: fixture.path, + output_directory: fixture.output, + }); + if (result.ok) throw new Error("expected setup failure"); + expect(projectAnalysisError(result.error).code).toBe( + "cleanup_incomplete", + ); + expect(result.error).toMatchObject({ + diagnostics: { + cleanup_failures: [ + { + resource: fixture.output, + reason: "injected first cleanup failure", + }, + ], + }, + }); + await expect(access(fixture.output)).resolves.toBeUndefined(); + } finally { + create.mockRestore(); + } + if (fixture.provider.close === undefined) + throw new Error("Expected Wakaru cleanup capability"); + await fixture.provider.close(); + expect(cleanupAttempts).toBe(2); + await expect(access(fixture.output)).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + + it("returns completed recovery when retry closes a descriptor after commit", async () => { + const fixture = await recoveryFixture(); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const commit = tree.commit.bind(tree); + vi.spyOn(tree, "commit").mockImplementation(async () => { + await commit(); + throw new Error("injected descriptor close failure"); + }); + return tree; + }); + try { + const result = await fixture.service.recover({ + path: fixture.path, + output_directory: fixture.output, + }); + if (!result.ok) throw result.error; + const published = javascriptRecoveryResultSchema.parse( + result.value.normalized_result, + ); + await expect(access(published.manifest.path)).resolves.toBeUndefined(); + expect(published.source.published_copy.path).toContain(fixture.output); + } finally { + create.mockRestore(); + } + await assertRecoveryCleanup(fixture.launches); + }); + + it("retains published recovery and its result while cleanup remains unresolved", async () => { + const fixture = await recoveryFixture(); + const createTree = SafeOutputTree.create.bind(SafeOutputTree); + let cleanupAttempts = 0; + const create = vi + .spyOn(SafeOutputTree, "create") + .mockImplementationOnce(async (outputRoot, platform) => { + const tree = await createTree(outputRoot, platform); + const commit = tree.commit.bind(tree); + vi.spyOn(tree, "commit").mockImplementation(async () => { + await commit(); + throw new Error("injected descriptor close failure"); + }); + const rollback = tree.rollback.bind(tree); + vi.spyOn(tree, "rollback").mockImplementation(async () => { + cleanupAttempts += 1; + if (cleanupAttempts === 1) + throw new Error("injected cleanup refusal"); + return rollback(); + }); + return tree; + }); + try { + const result = await fixture.service.recover({ + path: fixture.path, + output_directory: fixture.output, + }); + if (result.ok) throw new Error("Expected unresolved cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.partialObservation).toMatchObject({ + kind: "javascript-recovery", + result: { + source: { + published_copy: { + path: expect.stringContaining(fixture.output), + }, + }, + manifest: { path: expect.stringContaining(fixture.output) }, + }, + }); + await expect( + access(join(fixture.output, "manifest.json")), + ).resolves.toBeUndefined(); + } finally { + create.mockRestore(); + } + if (fixture.provider.close === undefined) + throw new Error("Expected Wakaru cleanup capability"); + await fixture.provider.close(); + expect(cleanupAttempts).toBe(2); + await expect( + access(join(fixture.output, "manifest.json")), + ).resolves.toBeUndefined(); + await assertRecoveryCleanup(fixture.launches); + }); + }, +); diff --git a/tests/boundary/mcp/webScriptSelectionMcp.test.ts b/tests/boundary/mcp/webScriptSelectionMcp.test.ts index 062718fc01..381c1dcb25 100644 --- a/tests/boundary/mcp/webScriptSelectionMcp.test.ts +++ b/tests/boundary/mcp/webScriptSelectionMcp.test.ts @@ -7,6 +7,7 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server"; import { expect, it, onTestFinished } from "vitest"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { silentLogger } from "../../../src/logger.js"; import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { registerWebScriptTool } from "../../../src/server/registerWebScriptTool.js"; @@ -29,14 +30,17 @@ it.skipIf(process.platform === "win32")( undefined, new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE), ); + const artifactResources = new ArtifactResourceScope(); registerWebScriptTool(server, { logger: silentLogger, recordEvidence: undefined, + artifactResources: artifactResources, }); const client = new Client({ name: "script-selection-test", version: "1" }); onTestFinished(async () => { await client.close(); await server.close(); + await artifactResources.close(); }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();