From 991a2406060ae200ee5db260e486e68c1f5bb4bd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:47:15 +0800 Subject: [PATCH] fix(cli): preserve script export evidence through shared finalization --- src/cli/webScriptCommands.ts | 60 +++++--------- .../cli/webScriptSelectionCli.test.ts | 80 ++++++++++++++++++- 2 files changed, 95 insertions(+), 45 deletions(-) diff --git a/src/cli/webScriptCommands.ts b/src/cli/webScriptCommands.ts index 9655f74cf8..17303ebb43 100644 --- a/src/cli/webScriptCommands.ts +++ b/src/cli/webScriptCommands.ts @@ -1,15 +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"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; +import { WEB_SCRIPT_EXPORT_PROVIDER } from "../domain/investigationProviders.js"; /** Register the local captured-script export CLI workflow. */ export const registerWebScriptCommands = ( @@ -33,45 +31,25 @@ export const registerWebScriptCommands = ( withCommandCancellation((signal) => logCliCommand(logger, CLI_COMMANDS.exportWebScripts, async () => { const resources = new ArtifactResourceScope(); - const result = await exportWebScripts( - { - capture_path: args.capturePath, - output_directory: args.outputDirectory, - }, + const result = await finishAnalysisCommand( + () => + exportWebScripts( + { + capture_path: args.capturePath, + output_directory: args.outputDirectory, + }, + resources, + { signal }, + ), resources, - { signal }, + WEB_SCRIPT_EXPORT_PROVIDER.id, + "export_web_scripts", + signal, + "Web script export 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 - : { - error: "Script export failed", - ...projectAnalysisError(result.error), - }; + return "evidence_id" in result + ? result + : { error: "Script export failed", ...result }; }), ), }); diff --git a/tests/boundary/cli/webScriptSelectionCli.test.ts b/tests/boundary/cli/webScriptSelectionCli.test.ts index 48abd4830d..73150c72bb 100644 --- a/tests/boundary/cli/webScriptSelectionCli.test.ts +++ b/tests/boundary/cli/webScriptSelectionCli.test.ts @@ -8,12 +8,16 @@ import { registerWebScriptCommands } from "../../../src/cli/webScriptCommands.js import * as webScriptExport from "../../../src/application/WebScriptExportService.js"; import { CLI_COMMANDS } from "../../../src/cliCommandNames.js"; import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; -import { err } from "../../../src/domain/result.js"; +import { err, ok } from "../../../src/domain/result.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; -import { scriptScenarioFixture } from "../../fixtures/webScriptCapture.js"; +import { + scriptCaptureEvidenceFixture, + scriptScenarioFixture, +} from "../../fixtures/webScriptCapture.js"; import { cliTest } from "../../support/cli/cliFixture.js"; -it("forwards an interrupt into the export workflow and reports cancellation", async () => { +const registeredExportCommand = () => { const commands = new Map }>(); const cli = { command: ( @@ -25,6 +29,11 @@ it("forwards an interrupt into the export workflow and reports cancellation", as }, } as unknown as Parameters[0]; registerWebScriptCommands(cli, pino({ enabled: false })); + return commands.get(CLI_COMMANDS.exportWebScripts)!; +}; + +it("forwards an interrupt into the export workflow and reports cancellation", async () => { + const command = registeredExportCommand(); const exported = vi .spyOn(webScriptExport, "exportWebScripts") .mockImplementation(async (_input, _resources, options) => { @@ -37,7 +46,7 @@ it("forwards an interrupt into the export workflow and reports cancellation", as }); const previousExitCode = process.exitCode; try { - const output = await commands.get(CLI_COMMANDS.exportWebScripts)?.run({ + const output = await command.run({ args: { capturePath: "/unused/capture.json", outputDirectory: "/unused/out", @@ -52,6 +61,69 @@ it("forwards an interrupt into the export workflow and reports cancellation", as } }); +it.each(["cleanup failure", "cancellation during cleanup"] as const)( + "retains exported Evidence on %s", + async (outcome) => { + const command = registeredExportCommand(); + const evidence = scriptCaptureEvidenceFixture(); + const exported = vi + .spyOn(webScriptExport, "exportWebScripts") + .mockResolvedValue(ok(evidence)); + const close = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockImplementation(async () => { + if (outcome === "cleanup failure") throw new Error("close denied"); + process.emit("SIGINT"); + }); + const previousExitCode = process.exitCode; + try { + const output = await command.run({ + args: { + capturePath: "/unused/capture.json", + outputDirectory: "/unused/out", + }, + }); + expect(close).toHaveBeenCalledOnce(); + expect(output).toMatchObject({ + error: "Script export failed", + code: + outcome === "cleanup failure" ? "cleanup_incomplete" : "cancelled", + details: { partial_observation: evidence }, + }); + } finally { + exported.mockRestore(); + close.mockRestore(); + process.exitCode = previousExitCode; + } + }, +); + +it("closes export resources when the operation throws", async () => { + const command = registeredExportCommand(); + const exported = vi + .spyOn(webScriptExport, "exportWebScripts") + .mockRejectedValue(new Error("unexpected export failure")); + const close = vi + .spyOn(ArtifactResourceScope.prototype, "close") + .mockResolvedValue(); + try { + const output = await command.run({ + args: { + capturePath: "/unused/capture.json", + outputDirectory: "/unused/out", + }, + }); + expect(close).toHaveBeenCalledOnce(); + expect(output).toMatchObject({ + error: "Script export failed", + details: { operation: "export_web_scripts" }, + }); + } finally { + exported.mockRestore(); + close.mockRestore(); + } +}); + cliTest.skipIf(process.platform === "win32")( "exits with a capture_path invalid_request for a named pipe before publishing output", async ({ cli, processes }) => {