diff --git a/docs/tool-design.md b/docs/tool-design.md index ea54ba4e3b..26e5e40098 100644 --- a/docs/tool-design.md +++ b/docs/tool-design.md @@ -151,6 +151,11 @@ providers need not expose identical state machines. waits for owned cleanup and retains its failure. Provider closure is distinct from caller cancellation. Retained cleanup capabilities block replacement; derive readiness from the current startup's completion and live connection. +- Session `cleanup_pending` includes retained blockers from external mutation + owners until those owners confirm release. Closing the analysis session does + not release resources owned by an external debugger. Reading a selected + analysis snapshot is part of `open_binary`; cancellation keeps that operation + and the caller's reason through the shared JSON reader. - For cancellable observations, carry cancellation through result consumption and awaited cleanup. Before returning success, check the signal after the last awaited finalization step. diff --git a/src/application/JsonFiles.ts b/src/application/JsonFiles.ts index 8476eff44f..a96b06ee69 100644 --- a/src/application/JsonFiles.ts +++ b/src/application/JsonFiles.ts @@ -103,6 +103,7 @@ export const readJsonObjectMembersFile = ( scans: readonly ReadonlyMap[], signal?: AbortSignal, resources?: ArtifactResourceScope, + operation = "read_evidence_file", ): Promise< Result< void, @@ -112,14 +113,9 @@ export const readJsonObjectMembersFile = ( readJsonFileInput( path, (selected) => - readJsonObjectFile( - selected, - "read_evidence_file", - scans, - signal, - resources, - ), + readJsonObjectFile(selected, operation, scans, signal, resources), signal, + operation, ); type JsonFileReadFailure = diff --git a/src/application/binary/AnalysisSnapshotFiles.ts b/src/application/binary/AnalysisSnapshotFiles.ts index 4ff1b9709e..af1842af35 100644 --- a/src/application/binary/AnalysisSnapshotFiles.ts +++ b/src/application/binary/AnalysisSnapshotFiles.ts @@ -19,6 +19,7 @@ import type { JsonObjectMemberReader } from "../JsonInputFile.js"; import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; type SnapshotFailure = EvidenceFileError | EvidenceIntegrityError; +const READ_OPERATION = "open_binary"; /** Read and validate an analysis snapshot from the caller's path. */ export const readAnalysisSnapshot = async ( @@ -63,6 +64,7 @@ export const readAnalysisSnapshot = async ( ], signal, resources, + READ_OPERATION, ); if (!loaded.ok) return loaded; signal?.throwIfAborted(); @@ -77,7 +79,9 @@ export const readAnalysisSnapshot = async ( return ok(snapshot); } catch (cause: unknown) { if (signal?.aborted === true) - return err(new AnalysisCancelledError("read_evidence_file", { cause })); + return err( + new AnalysisCancelledError(READ_OPERATION, { cause: signal.reason }), + ); return err( new EvidenceIntegrityError(snapshotValidationMessage(cause), { cause }), ); diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index 340606c27b..9992e87d58 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -1033,6 +1033,7 @@ export class BinarySession requestActivitySnapshots: client?.requestActivitySnapshots?.() ?? [], providerOperationHealth: client?.operationHealthSnapshot?.() ?? null, cleanupPending: + this.#externalMutationCleanupFailure?.cleanupIncomplete === true || this.#pendingCleanup !== undefined || this.#pendingBundleResourceCleanup.length > 0 || this.hasPendingFileResources || diff --git a/tests/boundary/filesystem/analysisSnapshot.test.ts b/tests/boundary/filesystem/analysisSnapshot.test.ts index 11701e519c..407751b8c2 100644 --- a/tests/boundary/filesystem/analysisSnapshot.test.ts +++ b/tests/boundary/filesystem/analysisSnapshot.test.ts @@ -276,6 +276,7 @@ describe("analysis snapshot member decoding", () => { `${" ".repeat(9 * 1024 * 1024)}${JSON.stringify(snapshot)}`, ); const controller = new AbortController(); + const reason = { request: "cancel snapshot import" }; const reads = await holdFirstFileRead(path); onTestFinished(() => { controller.abort(); @@ -283,12 +284,16 @@ describe("analysis snapshot member decoding", () => { }); const pending = readAnalysisSnapshot(path, controller.signal); await reads.firstRead; - controller.abort(); + controller.abort(reason); reads.release(); try { expect(await pending).toMatchObject({ ok: false, - error: { _tag: "AnalysisCancelledError" }, + error: { + _tag: "AnalysisCancelledError", + operation: "open_binary", + cause: reason, + }, }); expect(reads.readCount).toBe(1); } finally { diff --git a/tests/composition/analysis-sessions/binarySession.test.ts b/tests/composition/analysis-sessions/binarySession.test.ts index 0e4ab9d80e..c0dfe6cced 100644 --- a/tests/composition/analysis-sessions/binarySession.test.ts +++ b/tests/composition/analysis-sessions/binarySession.test.ts @@ -11,6 +11,7 @@ import { createAnalysisProfile } from "../../../src/domain/analysisProfile.js"; import { HopperStartError } from "../../../src/domain/hopperErrors.js"; import { ProviderAdapterError } from "../../../src/domain/providerAdapterError.js"; import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { AnalysisCleanupLease } from "../../../src/artifacts/AnalysisCleanupLease.js"; import { err, ok as resultOk } from "../../../src/domain/result.js"; import { createEvidenceBundle } from "../../../src/domain/evidenceBundle.js"; import { createEvidence } from "../../../src/domain/evidence.js"; @@ -262,6 +263,41 @@ it("leaves analysis unbound when a command returns before its writer finishes", await session.close(); }); +it("reports an external cleanup blocker until its exact owner confirms release", async () => { + const [target] = await createBinarySessionTargets(); + const session = createTestBinarySession(createCacheProvider([])); + const failure = new ProviderCleanupError( + "external-debugger", + ["external-run"], + {}, + ); + let released = false; + const lease = new AnalysisCleanupLease(async () => { + released = true; + }, ["external-run"]); + lease.bindCleanupError(failure); + try { + expect(await session.status()).toMatchObject({ cleanup_pending: false }); + await session.withAdmittedExternalMutation( + target, + undefined, + async () => err(failure), + (_value, error) => error, + ); + expect(await session.status()).toMatchObject({ cleanup_pending: true }); + expect(await session.open(target)).toEqual(err(failure)); + await session.close(); + expect(released).toBe(false); + expect(await session.status()).toMatchObject({ cleanup_pending: true }); + await lease.release(); + expect(await session.status()).toMatchObject({ cleanup_pending: false }); + expect((await session.open(target)).ok).toBe(true); + } finally { + await lease.release(); + await session.close(); + } +}); + it("retires analysis when an external mutation uses a hard link", async () => { const [target] = await createBinarySessionTargets(); const alias = join(dirname(target), "hard-link-alias.hop");