From 7285e15839a4fecf9940683699b781a60ccc4775 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 06:29:27 +0800 Subject: [PATCH] fix(workflows): retain staged output ownership and cancellation semantics Give atomic Evidence and snapshot writes an explicit resource owner, preserve primary failures and completed publication receipts, and retry retained staging cleanup at the owning lifecycle. Verify inode identity before publishing or deleting; unknown path disappearance remains a cleanup failure. Keep snapshot admission reusable when a session closes and reopens. Consolidate JavaScript source-scope classification so cancellation cannot fabricate failures for unselected files. Classify configuration failures as invalid input. Remove stale assertions and reuse shared test cleanup ownership. Validation: 266 focused tests before final ownership tightening, 108 affected tests afterward, static and documentation checks, actual invalid-configuration CLI behavior, and isolated test temporary-directory hygiene. Refs #1968 --- docs/mcp-contracts.md | 5 + src/application/DirectAnalysis.ts | 22 +- src/application/EvidenceBundleCommands.ts | 33 ++- src/application/EvidenceBundleFiles.ts | 3 + src/application/JsonFiles.ts | 139 ++++++++- .../binary/AnalysisSnapshotFiles.ts | 4 +- src/application/binary/BinarySession.ts | 14 +- .../binary/BinarySessionRecords.ts | 38 +++ .../javascript/JavaScriptArtifactAnalysis.ts | 61 ++-- src/artifacts/ArtifactResourceScope.ts | 15 +- src/config.test.ts | 3 + src/domain/analysisErrorPresentation.ts | 6 + src/domain/analysisErrorProjection.test.ts | 32 ++ src/domain/analysisErrorProjection.ts | 7 + src/domain/evidenceErrors.ts | 14 +- src/filesystem/OwnedStagedOutput.ts | 169 +++++++++++ src/server/createServer.ts | 1 + src/server/registerSessionRecordTools.ts | 4 + src/server/registerSessionTools.ts | 5 +- .../filesystem/analysisSnapshot.test.ts | 15 +- .../directAnalysisPermissions.test.ts | 8 +- .../directAnalysisSnapshotReplay.test.ts | 92 ++++-- .../filesystem/evidenceBundleFiles.test.ts | 75 +++-- .../javascriptApplicationService.test.ts | 73 ++++- .../filesystem/jsonOutputOwnership.test.ts | 273 ++++++++++++++++++ tests/support/javascriptApplicationScope.ts | 3 +- 26 files changed, 988 insertions(+), 126 deletions(-) create mode 100644 src/filesystem/OwnedStagedOutput.ts create mode 100644 tests/boundary/filesystem/jsonOutputOwnership.test.ts diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index cd1a75316c..b8d8d6fb82 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -165,6 +165,11 @@ Provider calls receive the request cancellation signal. Artifact traversal, hashing, version comparisons, Hopper requests, and process capture check the same signal. Cancellation is distinct from timeout. A cleanup failure uses `cleanup_incomplete` and lists only the owned resource kinds that remain. +An atomic Evidence bundle or snapshot write can publish its complete output +before staging cleanup fails. In that case, `details.published: true`, +`details.published_path`, and `details.bytes` retain the publication receipt. +Use that receipt to distinguish completed publication from a write that never +committed. Staging resources remain owned until cleanup is verified. Native call tracing and process capture retain available observations in `details.partial_observation` on failure, including when cleanup succeeds. The observation reports its partial coverage; cleanup details describe host diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index cc0e358bb8..d5300ff5e3 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -15,14 +15,14 @@ import { isManagedToolName, type ManagedToolName, } from "../contracts/managed/managedToolContracts.js"; -import { EvidenceIntegrityError } from "../domain/evidenceErrors.js"; +import { + EvidenceFileError, + EvidenceIntegrityError, +} from "../domain/evidenceErrors.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { access } from "node:fs/promises"; -import { - readAnalysisSnapshot, - writeAnalysisSnapshot, -} from "./binary/AnalysisSnapshotFiles.js"; +import { readAnalysisSnapshot } from "./binary/AnalysisSnapshotFiles.js"; import { parseBinaryTarget } from "./BinaryTargetResolver.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { @@ -334,13 +334,19 @@ const runAnalysis = async ( const snapshot = session.exportAnalysisSnapshot(); if (!snapshot.ok) return cliError(snapshot.error.retainPartialObservation(evidence)); - const written = await writeAnalysisSnapshot( + const written = await session.writeAnalysisSnapshot( snapshot.value, snapshotPath, true, ); - if (!written.ok) - return cliError(written.error.retainPartialObservation(evidence)); + if (!written.ok) { + const recovered = + written.error instanceof EvidenceFileError && + written.error.published !== undefined && + (await session.closeWriteResources()).ok; + if (!recovered) + return cliError(written.error.retainPartialObservation(evidence)); + } } return output; }, diff --git a/src/application/EvidenceBundleCommands.ts b/src/application/EvidenceBundleCommands.ts index 7b91c05c37..a72d17cb53 100644 --- a/src/application/EvidenceBundleCommands.ts +++ b/src/application/EvidenceBundleCommands.ts @@ -1,10 +1,14 @@ -import { EvidenceIntegrityError } from "../domain/evidenceErrors.js"; +import { + EvidenceFileError, + EvidenceIntegrityError, +} from "../domain/evidenceErrors.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { EvidenceBundle } from "../domain/evidenceBundle.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { jsonValueSchema } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; import { compareBundles } from "../domain/bundleComparison.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { readEvidenceBundle, writeEvidenceBundle, @@ -31,10 +35,33 @@ export const exportEvidenceBundleCommand = async ( ): Promise> => { const loaded = await readEvidenceBundle(sourcePath); if (!loaded.ok) return loaded; - return projectWrite( + const resources = new ArtifactResourceScope(); + const written = await writeEvidenceBundle( loaded.value, - await writeEvidenceBundle(loaded.value, outputPath, overwrite), + outputPath, + overwrite, + resources, ); + if (written.ok) return projectWrite(loaded.value, written); + + let cleanupSucceeded = false; + try { + await resources.close(); + cleanupSucceeded = true; + } catch { + // The original error cause retains the same staged-output owner for diagnostics. + } + if ( + cleanupSucceeded && + written.error instanceof EvidenceFileError && + written.error.published + ) { + return projectWrite(loaded.value, { + ok: true, + value: written.error.published, + }); + } + return written; }; /** Compare two validated canonical Evidence bundles without session state. */ diff --git a/src/application/EvidenceBundleFiles.ts b/src/application/EvidenceBundleFiles.ts index dfc63b8844..a9ff27990d 100644 --- a/src/application/EvidenceBundleFiles.ts +++ b/src/application/EvidenceBundleFiles.ts @@ -21,6 +21,7 @@ import { } from "../domain/jsonSerialization.js"; import { readJsonFile, writeTextParts } from "./JsonFiles.js"; import { RegularFileCleanupFailure } from "./RegularFileRead.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; type EvidenceReadFailure = | EvidenceFileError @@ -105,6 +106,7 @@ export const writeEvidenceBundle = async ( bundle: EvidenceBundle, path: string, overwrite: boolean, + resources: ArtifactResourceScope, signal?: AbortSignal, ): Promise< Result< @@ -128,6 +130,7 @@ export const writeEvidenceBundle = async ( bufferedJsonParts(canonicalJsonParts(checked)), path, overwrite, + resources, signal === undefined ? undefined : { signal, operation: "export_evidence_bundle" }, diff --git a/src/application/JsonFiles.ts b/src/application/JsonFiles.ts index 1eb9a76507..76a05f9cb9 100644 --- a/src/application/JsonFiles.ts +++ b/src/application/JsonFiles.ts @@ -5,12 +5,14 @@ import { open, realpath, rename, - rm, type FileHandle, } from "node:fs/promises"; import { basename, dirname, resolve } from "node:path"; -import { EvidenceFileError } from "../domain/evidenceErrors.js"; +import { + EvidenceFileError, + type PublishedEvidenceFile, +} from "../domain/evidenceErrors.js"; import { AnalysisCancelledError, AnalysisResourceConstraintError, @@ -22,6 +24,10 @@ import { type JsonObjectMemberReader, } from "./JsonInputFile.js"; import { NonRegularFileReadError } from "../filesystem/RegularFile.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; /** Request control and its owning operation for an interruptible atomic write. */ export interface TextWriteCancellation { @@ -119,28 +125,57 @@ export const writeTextFile = async ( encoded: string, path: string, overwrite: boolean, + resources: ArtifactResourceScope, ): Promise> => - writeTextParts([encoded], path, overwrite); + writeTextParts([encoded], path, overwrite, resources); /** Publish streamed text atomically unless cancellation wins before publication. */ export function writeTextParts( parts: Iterable, path: string, overwrite: boolean, + resources: ArtifactResourceScope, ): Promise>; -/** Pass request control to stop streamed generation before atomic publication. */ export function writeTextParts( parts: Iterable, path: string, overwrite: boolean, + resources: ArtifactResourceScope, cancellation: TextWriteCancellation | undefined, ): Promise; -export async function writeTextParts( +export function writeTextParts( parts: Iterable, path: string, overwrite: boolean, + resources: ArtifactResourceScope, cancellation?: TextWriteCancellation, ): Promise { + return resources + .run(() => + writeTextPartsOwned(parts, path, overwrite, resources, cancellation), + ) + .catch((cause: unknown) => { + const cleanup = + cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined + ? cause.cleanup + : undefined; + return err( + new EvidenceFileError("write", "io", { + cause, + path: resolve(path), + ...(cleanup === undefined ? {} : { cleanup }), + }), + ); + }); +} + +const writeTextPartsOwned = async ( + parts: Iterable, + path: string, + overwrite: boolean, + resources: ArtifactResourceScope, + cancellation?: TextWriteCancellation, +): Promise => { const requestedPath = resolve(path); try { assertWriteActive(cancellation); @@ -165,10 +200,40 @@ export async function writeTextParts( destination, parts, overwrite, + resources, cancellation, ); return ok({ path: requestedPath, bytes }); } catch (cause: unknown) { + if (cause instanceof StagedOutputCleanupFailure) { + const cleanup = { + reason: failureMessage(cause.cleanupCause), + resources: [cause.owner.directory], + }; + if (cause.outcome.ok) + return err( + new EvidenceFileError("write", "io", { + cause, + path: requestedPath, + cleanup, + published: { ...cause.outcome.value, path: requestedPath }, + }), + ); + if (cause.outcome.error instanceof AnalysisCancelledError) + return err( + new AnalysisCancelledError(cause.outcome.error.operation, { + cause, + cleanup, + }), + ); + return err( + new EvidenceFileError("write", missingOrIo(cause.outcome.error), { + cause, + path: requestedPath, + cleanup, + }), + ); + } if ( cancellation?.signal.aborted === true && cause instanceof AnalysisCancelledError && @@ -189,7 +254,7 @@ export async function writeTextParts( }), ); } -} +}; /** A missing path or parent is a selection error, not a permission failure. */ const missingOrIo = (cause: unknown): "missing" | "io" => { @@ -201,33 +266,75 @@ const publishFile = async ( destination: string, parts: Iterable, overwrite: boolean, + resources: ArtifactResourceScope, cancellation?: TextWriteCancellation, ): Promise => { const stagingDirectory = await mkdtemp( resolve(dirname(destination), ".rea-write-"), ); + const owner = await OwnedStagedOutput.create(stagingDirectory); + let outcome: Result; + const staged = resolve(stagingDirectory, "content"); try { - const staged = resolve(stagingDirectory, "content"); const file = await open(staged, "wx", 0o600); - let bytes = 0; + await owner.setFile(new OwnedFileHandle(file)); + let written: Result; try { - bytes = await writeStagedParts(file, parts, cancellation); + const bytes = await writeStagedParts(file, parts, cancellation); assertWriteActive(cancellation); await file.sync(); - } finally { - await file.close(); + written = ok(bytes); + } catch (cause: unknown) { + written = err(cause); } - // Once submitted, the atomic publication can win a later cancellation race. + try { + await owner.closeFile(); + } catch (cause: unknown) { + if (written.ok) written = err(cause); + } + if (!written.ok) throw written.error; + // Once submitted, atomic publication can win a later cancellation race. assertWriteActive(cancellation); - if (overwrite) await rename(staged, destination); + await owner.assertReadyForPublication(staged); + assertWriteActive(cancellation); + if (overwrite) { + await rename(staged, destination); + owner.recordPublishedRename(); + } // A hard link publishes complete bytes atomically and cannot replace an existing name. else await link(staged, destination); - return bytes; - } finally { - await rm(stagingDirectory, { recursive: true, force: true }); + outcome = ok({ path: destination, bytes: written.value }); + } catch (cause: unknown) { + outcome = err(cause); } + + // One cleanup phase owns both the open descriptor (if any) and its staging directory. + const released = await resources.release({ + kind: "staged-output", + resource: owner.directory, + output: owner, + }); + if (released.kind === "failed") + throw new StagedOutputCleanupFailure(outcome, released.cause, owner); + if (!outcome.ok) throw outcome.error; + return outcome.value.bytes; }; +class StagedOutputCleanupFailure extends Error { + constructor( + readonly outcome: Result, + readonly cleanupCause: unknown, + readonly owner: OwnedStagedOutput, + ) { + super("Staged output cleanup failed", { + cause: outcome.ok ? cleanupCause : outcome.error, + }); + } +} + +const failureMessage = (cause: unknown): string => + cause instanceof Error ? cause.message || cause.name : String(cause); + const writeStagedParts = async ( file: FileHandle, parts: Iterable, diff --git a/src/application/binary/AnalysisSnapshotFiles.ts b/src/application/binary/AnalysisSnapshotFiles.ts index 064aae7056..174b58bfcc 100644 --- a/src/application/binary/AnalysisSnapshotFiles.ts +++ b/src/application/binary/AnalysisSnapshotFiles.ts @@ -16,6 +16,7 @@ import type { AnalysisResourceConstraintError } from "../../domain/analysisError import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; import { readJsonObjectMembersFile, writeTextFile } from "../JsonFiles.js"; import type { JsonObjectMemberReader } from "../JsonInputFile.js"; +import type { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; type SnapshotFailure = EvidenceFileError | EvidenceIntegrityError; @@ -118,6 +119,7 @@ export const writeAnalysisSnapshot = async ( snapshot: AnalysisSnapshot, path: string, overwrite: boolean, + resources: ArtifactResourceScope, ): Promise< Result<{ readonly path: string; readonly bytes: number }, SnapshotFailure> > => { @@ -129,7 +131,7 @@ export const writeAnalysisSnapshot = async ( new EvidenceIntegrityError(snapshotValidationMessage(cause), { cause }), ); } - return writeTextFile(encoded, path, overwrite); + return writeTextFile(encoded, path, overwrite, resources); }; const snapshotValidationMessage = (cause: unknown): string => diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index 30239874ee..d5841aa938 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -24,7 +24,6 @@ import type { BinarySessionPort, SavedAnalysisSnapshot, } from "./BinarySessionPort.js"; -import { writeAnalysisSnapshot } from "./AnalysisSnapshotFiles.js"; import { SessionProviderRouter, type SessionProviderRoute, @@ -376,7 +375,7 @@ export class BinarySession await this.#drainCalls(admittedThrough); const snapshot = this.exportAnalysisSnapshot(); if (!snapshot.ok) return snapshot; - const written = await writeAnalysisSnapshot( + const written = await this.writeAnalysisSnapshot( snapshot.value, path, overwrite, @@ -421,7 +420,16 @@ export class BinarySession ); this.clearSessionRecords(); this.#clearRuntimeAvailability(); - return closed; + const fileResources = await this.closeWriteResources(); + if (!closed.ok && !fileResources.ok) + return err( + analysisErrorWithCleanupFailure( + closed.error, + fileResources.error, + "close_binary", + ), + ); + return closed.ok ? fileResources : closed; } async #retireClient( diff --git a/src/application/binary/BinarySessionRecords.ts b/src/application/binary/BinarySessionRecords.ts index 1f94dcdfd8..182d5892fc 100644 --- a/src/application/binary/BinarySessionRecords.ts +++ b/src/application/binary/BinarySessionRecords.ts @@ -6,6 +6,7 @@ import type { EvidenceBundle } from "../../domain/evidenceBundle.js"; import type { JsonValue } from "../../domain/jsonValue.js"; import { evidenceBundleForTarget } from "../../domain/evidenceBundle.js"; import { + EvidenceFileError, EvidenceIntegrityError, AnalysisSnapshotMismatchError, } from "../../domain/evidenceErrors.js"; @@ -24,6 +25,9 @@ import type { } from "../AnalysisProvider.js"; import { AnalysisSnapshotCache } from "./AnalysisSnapshotCache.js"; import { InvestigationRecords } from "../investigation/InvestigationRecords.js"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; +import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; +import { writeAnalysisSnapshot as writeSnapshotFile } from "./AnalysisSnapshotFiles.js"; const SNAPSHOT_MUTATION_RECOVERY = "Export session observations through export_evidence_bundle if needed. Close the active target without saving a snapshot, then reopen it before importing or saving a snapshot."; @@ -45,6 +49,7 @@ export interface WorkflowSnapshotRecordInput { /** Own binary snapshots and coordinate the composed investigation records. */ export abstract class BinarySessionRecords { readonly #records: InvestigationRecords; + readonly #writeResources = new ArtifactResourceScope(); readonly #snapshot = new AnalysisSnapshotCache(); #snapshotInvalidated = false; readonly #snapshotListeners = new Set<() => void | Promise>(); @@ -89,6 +94,39 @@ export abstract class BinarySessionRecords { return this.#records.evidenceBundleForSerialization(); } + /** Persist one provider-neutral snapshot under this session's resource owner. */ + writeAnalysisSnapshot( + snapshot: AnalysisSnapshot, + path: string, + overwrite: boolean, + ): Promise< + Result<{ readonly path: string; readonly bytes: number }, AnalysisError> + > { + return writeSnapshotFile(snapshot, path, overwrite, this.#writeResources); + } + + /** Retry and release staged snapshot files at the owning session shutdown. */ + async closeWriteResources(): Promise> { + try { + await this.#writeResources.retryCleanup(); + return ok(null); + } catch (cause: unknown) { + const observation = ArtifactReaderFailure.cleanupObservation( + cause, + "analysis snapshot staging", + ); + return err( + new EvidenceFileError("write", "io", { + cause, + cleanup: { + reason: observation.reason, + resources: observation.resources, + }, + }), + ); + } + } + importEvidenceBundle( bundle: unknown, ): Result { diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.ts b/src/application/javascript/JavaScriptArtifactAnalysis.ts index 81714ada04..e0c3b23cb0 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.ts @@ -146,22 +146,17 @@ export const analyzeAndAdoptJavaScriptArtifactFiles = async ( for (const file of fileSet.files) { try { await setImmediate(); - if (signal?.aborted === true) - throw new AnalysisCancelledError("analyze_javascript_application"); - if ( - !file.text.included && - file.text.reason === "outside-analysis-scope" - ) { - state.files.push({ file, javascript: null, semantic: null }); - if (file.kind === "javascript") - projection.recordUnavailableFile( - file, - "Source was not analyzed because it is outside the caller-selected source_paths.", - null, - "incomplete-module", + if (classifyOutsideAnalysisScope(file, state, projection)) { + if (signal?.aborted === true) { + interruption = new AnalysisCancelledError( + "analyze_javascript_application", ); + break; + } continue; } + if (signal?.aborted === true) + throw new AnalysisCancelledError("analyze_javascript_application"); await beforeFile(file, selectedFileIndex++, selectedFileCount); if (file.kind !== "javascript" || !file.text.included) { const steps = analyzeArtifactFileSteps(file, { @@ -287,20 +282,7 @@ export const analyzeAndAdoptJavaScriptArtifactFiles = async ( }; for (const file of fileSet.files) { if (analyzedPaths.has(file.path)) continue; - if ( - !file.text.included && - file.text.reason === "outside-analysis-scope" - ) { - state.files.push({ file, javascript: null, semantic: null }); - if (file.kind === "javascript") - projection.recordUnavailableFile( - file, - "Source was not analyzed because it is outside the caller-selected source_paths.", - null, - "incomplete-module", - ); - continue; - } + if (classifyOutsideAnalysisScope(file, state, projection)) continue; state.files.push({ file, javascript: null, @@ -370,10 +352,7 @@ function* analyzeArtifactFileSteps< context: ArtifactAnalysisContext, ): Generator { const { state } = context; - if (!file.text.included && file.text.reason === "outside-analysis-scope") { - state.files.push({ file, javascript: null, semantic: null }); - return; - } + if (classifyOutsideAnalysisScope(file, state)) return; addStructuredObservations(file, state); if (file.kind === "html" && file.text.included) state.htmlScripts.push(...parseHtmlScripts(file.path, file.text.value)); @@ -421,6 +400,26 @@ function* analyzeArtifactFileSteps< if (analysis.parse_status === "failed") state.parseFailures += 1; } +const classifyOutsideAnalysisScope = < + SemanticIr extends JavaScriptModuleSemanticIr, +>( + file: JavaScriptArtifactFile, + state: MutableArtifactAnalysis, + projection?: JavaScriptSemanticGraphProjection, +): boolean => { + if (file.text.included || file.text.reason !== "outside-analysis-scope") + return false; + state.files.push({ file, javascript: null, semantic: null }); + if (file.kind === "javascript") + projection?.recordUnavailableFile( + file, + "Source was not analyzed because it is outside the caller-selected source_paths.", + null, + "incomplete-module", + ); + return true; +}; + const addSourceMap = ( file: JavaScriptArtifactFile, context: ArtifactAnalysisContext, diff --git a/src/artifacts/ArtifactResourceScope.ts b/src/artifacts/ArtifactResourceScope.ts index 1f06ed2544..59fdcba367 100644 --- a/src/artifacts/ArtifactResourceScope.ts +++ b/src/artifacts/ArtifactResourceScope.ts @@ -1,6 +1,7 @@ import type { JavaScriptSourceAnalysisPort } from "../domain/javascript/javascriptSourceAnalysis.js"; import type { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import type { OwnedDirectoryHandle } from "../filesystem/OwnedDirectoryHandle.js"; +import type { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import type { ArtifactReader } from "./ArtifactReader.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; @@ -32,6 +33,11 @@ export type ArtifactResourceOwner = readonly kind: "output-tree"; readonly resource: string; readonly tree: SafeOutputTree; + } + | { + readonly kind: "staged-output"; + readonly resource: string; + readonly output: OwnedStagedOutput; }; export type ArtifactCleanupAttempt = @@ -60,7 +66,7 @@ export class ArtifactResourceScope { "Artifact resource scope is closed", ); if (this.#pending.size > 0) { - await this.#retryCleanup(); + await this.retryCleanup(); continue; } const running = Promise.resolve().then(operation); @@ -86,7 +92,7 @@ export class ArtifactResourceScope { } /** Retry retained owners without closing resource admission. */ - #retryCleanup(): Promise { + retryCleanup(): Promise { if (this.#draining !== undefined) return this.#draining; this.#draining = this.#drainAndRetry().finally(() => { this.#draining = undefined; @@ -97,7 +103,7 @@ export class ArtifactResourceScope { /** Stop admission, wait for active operations, and retry every retained owner. */ close(): Promise { this.#closed = true; - return this.#retryCleanup(); + return this.retryCleanup(); } async #drainAndRetry(): Promise { @@ -152,5 +158,8 @@ const cleanup = async (owner: ArtifactResourceOwner): Promise => { return; case "output-tree": await owner.tree.rollback(); + return; + case "staged-output": + await owner.output.close(); } }; diff --git a/src/config.test.ts b/src/config.test.ts index 68a145e206..eb784802fa 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -193,6 +193,7 @@ describe("configuration failure diagnostics", () => { if (result.ok) throw new Error("Expected invalid startup deadline"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "configuration_invalid", + category: "invalid_input", details: { settings: [ { @@ -223,6 +224,7 @@ describe("configuration failure diagnostics", () => { if (result.ok) throw new Error("Expected invalid startup deadline"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "configuration_invalid", + category: "invalid_input", details: { settings: [ { @@ -248,6 +250,7 @@ describe("configuration failure diagnostics", () => { const constraint = `${setting} must be absolute`; expect(projectAnalysisError(result.error)).toMatchObject({ code: "configuration_invalid", + category: "invalid_input", message: `REA configuration is invalid: ${constraint}.`, remediation: { action: expect.stringContaining(`Correct ${setting} `) }, details: { settings: [{ setting, constraint }] }, diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index fa159394d3..0c433e7891 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -33,6 +33,8 @@ import type { AnalysisErrorProjection } from "./analysisErrorProjection.js"; export const analysisErrorRemediationAction = ( error: AnalysisError, ): string => { + if (error instanceof EvidenceFileError && error.published !== undefined) + return "Use the published file at details.published_path and resolve the reported staging cleanup failure."; if (error instanceof AnalysisSnapshotMismatchError) return "Run analysis without this snapshot, then save a fresh snapshot using the intended artifact, provider, and analysis profile."; if (error instanceof AnalysisUnsupportedTargetError) @@ -156,6 +158,7 @@ const artifactErrorCategory = ( const STATIC_ERROR_CATEGORIES: Readonly< Partial> > = { + ConfigurationError: "invalid_input", AnalysisInputError: "invalid_input", AnalysisAccessDeniedError: "unavailable", AnalysisArtifactChangedError: "integrity_mismatch", @@ -368,7 +371,10 @@ const artifactMessage = ({ const evidenceFileMessage = ({ operation, reason, + published, }: EvidenceFileError): string => { + if (published !== undefined) + return "The output file was published, but staging cleanup remains incomplete."; if (reason === "missing") return operation === "read" ? "Evidence file does not exist at the selected path. Check the path and try again." diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index eb1de87005..12597fc945 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -25,6 +25,38 @@ import { ProviderSelectionError } from "./providerSelectionError.js"; import { analysisErrorProjectionSchema } from "../contracts/errorSchemas.js"; import { nativeCallPartialObservationSchema } from "./native/nativeCallPartialObservation.js"; +it("projects a published file receipt alongside retained cleanup failure", () => { + const projected = projectAnalysisError( + new EvidenceFileError("write", "io", { + path: "/tmp/evidence.json", + published: { path: "/tmp/evidence.json", bytes: 42 }, + cleanup: { + reason: "staging directory is still owned", + resources: ["/tmp/.rea-write-abc"], + }, + cause: new Error("cleanup incomplete"), + }), + ); + expect(projected).toMatchObject({ + code: "cleanup_incomplete", + message: + "The output file was published, but staging cleanup remains incomplete.", + remediation: { + action: + "Use the published file at details.published_path and resolve the reported staging cleanup failure.", + }, + details: { + operation: "write", + path: "/tmp/evidence.json", + published: true, + published_path: "/tmp/evidence.json", + bytes: 42, + resources: ["/tmp/.rea-write-abc"], + }, + }); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe(true); +}); + const nativeCallPartialObservation = nativeCallPartialObservationSchema.parse({ kind: "native-call-observation", target: { diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 2ab2a6c5cc..803d73b4e9 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -309,6 +309,13 @@ const artifactStateErrorDetails = ( operation: error.operation, reason: error.reason, ...(error.path === undefined ? {} : { path: error.path }), + ...(error.published === undefined + ? {} + : { + published: true, + bytes: error.published.bytes, + published_path: error.published.path, + }), }; return undefined; }; diff --git a/src/domain/evidenceErrors.ts b/src/domain/evidenceErrors.ts index 37fe1127c7..364cbe79db 100644 --- a/src/domain/evidenceErrors.ts +++ b/src/domain/evidenceErrors.ts @@ -3,6 +3,12 @@ import { type AnalysisErrorOptions, } from "./analysisErrorBase.js"; +/** Receipt retained when publication succeeded but staging cleanup did not. */ +export interface PublishedEvidenceFile { + readonly path: string; + readonly bytes: number; +} + /** Evidence identity, schema, or bundle manifests failed integrity checks. */ export class EvidenceIntegrityError extends AnalysisError { readonly _tag = "EvidenceIntegrityError"; @@ -42,13 +48,19 @@ export class EvidenceFileError extends AnalysisError { /** Caller-selected path that failed, resolved against the working directory. */ readonly path: string | undefined; + /** Present only when a write committed before cleanup failed. */ + readonly published: PublishedEvidenceFile | undefined; constructor( readonly operation: "read" | "write", readonly reason: "not-file" | "exists" | "invalid-json" | "missing" | "io", - options?: ErrorOptions & { readonly path?: string }, + options?: AnalysisErrorOptions & { + readonly path?: string; + readonly published?: PublishedEvidenceFile; + }, ) { super(`Evidence bundle ${operation} failed: ${reason}`, options); this.path = options?.path; + this.published = options?.published; } } diff --git a/src/filesystem/OwnedStagedOutput.ts b/src/filesystem/OwnedStagedOutput.ts new file mode 100644 index 0000000000..2a1cfd9f3d --- /dev/null +++ b/src/filesystem/OwnedStagedOutput.ts @@ -0,0 +1,169 @@ +import { lstat, readdir, rmdir, unlink } from "node:fs/promises"; +import type { Stats } from "node:fs"; + +import { OwnedFileHandle } from "./OwnedFileHandle.js"; + +interface Identity { + readonly dev: number; + readonly ino: number; +} + +const identityOf = (stats: Stats): Identity => ({ + dev: stats.dev, + ino: stats.ino, +}); + +const sameIdentity = (stats: Stats, identity: Identity): boolean => + stats.dev === identity.dev && stats.ino === identity.ino; + +/** One atomically staged file and directory retained through verified cleanup. */ +export class OwnedStagedOutput { + #file: OwnedFileHandle | undefined; + #fileIdentity: Identity | undefined; + #fileRemovedByOwner = false; + #publishedRename = false; + #closeAttempt: Promise | undefined; + #closed = false; + #identityFailure: unknown; + + private constructor( + readonly directory: string, + private readonly directoryIdentity: Identity | undefined, + ) {} + + static async create(directory: string): Promise { + try { + const stats = await lstat(directory); + if (!stats.isDirectory() || stats.isSymbolicLink()) + return new OwnedStagedOutput(directory, undefined); + return new OwnedStagedOutput(directory, identityOf(stats)); + } catch (cause: unknown) { + // Keep an owner that refuses deletion when creation identity is unknown. + const owner = new OwnedStagedOutput(directory, undefined); + owner.#identityFailure = cause; + return owner; + } + } + + async setFile(file: OwnedFileHandle): Promise { + if (this.#file !== undefined) + throw new Error("Staged output already owns a file handle"); + this.#file = file; + try { + this.#fileIdentity = identityOf(await file.handle.stat()); + } catch (cause: unknown) { + // The handle remains owned; cleanup will close it and refuse pathname removal. + this.#identityFailure = cause; + } + } + + /** Verify that the path still names the exact closed file before publication. */ + async assertReadyForPublication(content: string): Promise { + await this.#verifiedDirectory(); + if (this.#fileIdentity === undefined) + throw new Error("Cannot verify staged file identity", { + cause: this.#identityFailure, + }); + const stagedFile = await lstat(content); + if ( + !stagedFile.isFile() || + stagedFile.isSymbolicLink() || + !sameIdentity(stagedFile, this.#fileIdentity) + ) + throw new Error(`Staged file identity changed: ${content}`); + } + + closeFile(): Promise { + return this.#file?.close() ?? Promise.resolve(); + } + + /** Record that the staged file moved to the published destination by rename. */ + recordPublishedRename(): void { + this.#publishedRename = true; + } + + /** Close the owned descriptor and remove only the exact staged entries. */ + close(): Promise { + if (this.#closed) return Promise.resolve(); + if (this.#closeAttempt !== undefined) return this.#closeAttempt; + const attempt = Promise.resolve().then(() => this.#cleanup()); + this.#closeAttempt = attempt; + void attempt.then( + () => { + this.#closed = true; + this.#closeAttempt = undefined; + }, + () => { + this.#closeAttempt = undefined; + }, + ); + return attempt; + } + + async #cleanup(): Promise { + await this.closeFile(); + await this.#verifiedDirectory(); + + const content = `${this.directory}/content`; + let stagedFile: Stats | undefined; + try { + stagedFile = await lstat(content); + } catch (cause: unknown) { + if ( + errorCode(cause) === "ENOENT" && + (this.#file === undefined || + this.#fileRemovedByOwner || + this.#publishedRename) + ) { + stagedFile = undefined; + } else if (errorCode(cause) === "ENOENT") { + throw new Error(`Owned staged file disappeared: ${content}`, { + cause, + }); + } else { + throw cause; + } + } + if (stagedFile !== undefined) { + if ( + !stagedFile.isFile() || + stagedFile.isSymbolicLink() || + this.#fileIdentity === undefined || + !sameIdentity(stagedFile, this.#fileIdentity) + ) + throw new Error(`Staged file identity changed: ${content}`); + await unlink(content); + this.#fileRemovedByOwner = true; + } + + const remaining = await readdir(this.directory); + if (remaining.length > 0) + throw new Error( + `Unexpected staging entries remain in ${this.directory}: ${remaining.join(", ")}`, + ); + await this.#verifiedDirectory(); + await rmdir(this.directory); + } + + async #verifiedDirectory(): Promise { + if (this.directoryIdentity === undefined) + throw new Error( + `Cannot verify staging directory ownership: ${this.directory}`, + { + cause: this.#identityFailure, + }, + ); + const directory = await lstat(this.directory); + if ( + !directory.isDirectory() || + directory.isSymbolicLink() || + !sameIdentity(directory, this.directoryIdentity) + ) + throw new Error(`Staging directory identity changed: ${this.directory}`); + } +} + +const errorCode = (cause: unknown): unknown => + typeof cause === "object" && cause !== null && "code" in cause + ? cause.code + : undefined; diff --git a/src/server/createServer.ts b/src/server/createServer.ts index 8318a2f600..e7b6a880c8 100644 --- a/src/server/createServer.ts +++ b/src/server/createServer.ts @@ -348,6 +348,7 @@ export const createServer = ( if (session !== undefined) { registerSessionTools(server, session, toolLogger, { ...selectedOptions, + artifactResources, ...(availability === undefined ? {} : { availabilityPolicy: availability.policy }), diff --git a/src/server/registerSessionRecordTools.ts b/src/server/registerSessionRecordTools.ts index a1558fed46..5854aac9d8 100644 --- a/src/server/registerSessionRecordTools.ts +++ b/src/server/registerSessionRecordTools.ts @@ -7,6 +7,7 @@ import { } from "../application/EvidenceBundleFiles.js"; import { toolContract } from "../contracts/toolContracts.js"; import type { EvidenceBundle } from "../domain/evidenceBundle.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { err, ok } from "../domain/result.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { inspectEvidenceBundle } from "../application/investigation/InspectEvidenceBundle.js"; @@ -15,6 +16,7 @@ import { toolRegistrationOptions } from "./toolRegistrationOptions.js"; interface EvidenceToolRegistration { readonly server: EvidenceMcpServer; readonly session: BinarySessionPort; + readonly resources: ArtifactResourceScope; readonly exportContract: ReturnType< typeof toolContract<"export_evidence_bundle"> >; @@ -38,6 +40,7 @@ export const registerEvidenceTools = ( const registerExportEvidenceTool = ({ server, session, + resources, exportContract, }: EvidenceToolRegistration): void => { server.registerTool( @@ -49,6 +52,7 @@ const registerExportEvidenceTool = ({ bundle, input.path, input.overwrite, + resources, context.mcpReq.signal, ); return written.ok diff --git a/src/server/registerSessionTools.ts b/src/server/registerSessionTools.ts index 1c4b89a54c..4db8031ce7 100644 --- a/src/server/registerSessionTools.ts +++ b/src/server/registerSessionTools.ts @@ -1,4 +1,5 @@ import type { EvidenceMcpServer } from "./EvidenceMcpServer.js"; +import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { getNavigationContext, @@ -231,6 +232,7 @@ const registerOpenLifecycleTool = ({ /** Register MCP-only target lifecycle operations on a long-lived session. */ export interface SessionToolOptions { + readonly artifactResources: ArtifactResourceScope; readonly startedAt?: string; readonly availabilityPolicy?: () => AvailabilityPolicy; readonly androidAnalysisAvailability?: ( @@ -286,7 +288,7 @@ export const registerSessionTools = ( server: EvidenceMcpServer, session: BinarySessionPort, logger: Logger, - options: SessionToolOptions = {}, + options: SessionToolOptions, ): void => { const openContract = toolContract("open_binary"); const closeContract = toolContract("close_binary"); @@ -339,6 +341,7 @@ export const registerSessionTools = ( registerEvidenceTools({ server, session, + resources: options.artifactResources, exportContract, importContract, snapshotContract, diff --git a/tests/boundary/filesystem/analysisSnapshot.test.ts b/tests/boundary/filesystem/analysisSnapshot.test.ts index 84cd14e035..a235fb4e6b 100644 --- a/tests/boundary/filesystem/analysisSnapshot.test.ts +++ b/tests/boundary/filesystem/analysisSnapshot.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { withArtifactResourceScope } from "../../fixtures/artifactInventory.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js"; @@ -76,11 +77,14 @@ describe("analysis snapshots: persistence", () => { const directory = await createTestTempDirectory("rea-snapshot-"); const path = join(directory, "analysis.json"); const snapshot = snapshotFixture(); - const written = await writeAnalysisSnapshot(snapshot, path, false); - expect(written.ok).toBe(true); + const written = await withArtifactResourceScope((resources) => + writeAnalysisSnapshot(snapshot, path, false, resources), + ); + if (!written.ok) throw written.error; expect((await readFile(path, "utf8")).endsWith("\n")).toBe(true); expect((await stat(path)).mode & 0o777).toBe(0o600); - expect((await readAnalysisSnapshot(path)).ok).toBe(true); + const roundTrip = await readAnalysisSnapshot(path); + if (!roundTrip.ok) throw roundTrip.error; const altered: unknown = JSON.parse(await readFile(path, "utf8")); if ( @@ -102,8 +106,9 @@ describe("analysis snapshots: persistence", () => { Reflect.set(first.parameters, "address", "0x2000"); await writeFile(path, JSON.stringify(altered)); const loaded = await readAnalysisSnapshot(path); - expect(loaded.ok).toBe(false); - if (!loaded.ok) expect(loaded.error._tag).toBe("EvidenceIntegrityError"); + if (loaded.ok) + throw new Error("Expected changed snapshot identity rejection"); + expect(loaded.error._tag).toBe("EvidenceIntegrityError"); expect(() => parseAnalysisSnapshot(altered)).toThrow(/identifier/u); }); }); diff --git a/tests/boundary/filesystem/directAnalysisPermissions.test.ts b/tests/boundary/filesystem/directAnalysisPermissions.test.ts index e3faf63c4c..26e308c44a 100644 --- a/tests/boundary/filesystem/directAnalysisPermissions.test.ts +++ b/tests/boundary/filesystem/directAnalysisPermissions.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { withArtifactResourceScope } from "../../fixtures/artifactInventory.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { writeAnalysisSnapshot } from "../../../src/application/binary/AnalysisSnapshotFiles.js"; @@ -84,9 +85,10 @@ describe("direct analysis snapshot files", () => { workflow_entries: [], evidence_bundle: createEvidenceBundle([evidence]), }; - expect( - (await writeAnalysisSnapshot(snapshot, snapshotPath, false)).ok, - ).toBe(true); + const written = await withArtifactResourceScope((resources) => + writeAnalysisSnapshot(snapshot, snapshotPath, false, resources), + ); + if (!written.ok) throw written.error; vi.stubEnv("HOPPER_LAUNCHER_PATH", launcherPath); await expect( diff --git a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts index 6827340eb3..6687b0ef71 100644 --- a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts +++ b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts @@ -2,7 +2,7 @@ import { parseConfig } from "../../../src/config/parseConfig.js"; import { writeFile } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { z } from "zod"; import { runDirectAnalysis } from "../../../src/application/DirectAnalysis.js"; @@ -33,11 +33,13 @@ import { createAnalysisSnapshotWorkflowEntry, type AnalysisSnapshot, } from "../../../src/domain/analysisSnapshot.js"; +import { withArtifactResourceScope } from "../../fixtures/artifactInventory.js"; import { createTestBinarySession, createTestProviderRouter, } from "../../fixtures/binarySession.js"; import { BinarySession } from "../../../src/application/binary/BinarySession.js"; +import { OwnedStagedOutput } from "../../../src/filesystem/OwnedStagedOutput.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { CAPABILITIES as GHIDRA_CAPABILITIES, @@ -342,6 +344,58 @@ const withEarlierHistoricalEvidence = ( }; }; +describe("snapshot output cleanup lifecycle", () => { + it("returns completed analysis after retrying cleanup of a published snapshot", async () => { + const fixture = await createSnapshotReplayFixture( + "rea-snapshot-cleanup-retry-", + ); + const cleanup = vi + .spyOn(OwnedStagedOutput.prototype, "close") + .mockRejectedValueOnce(new Error("transient staging cleanup failure")); + try { + const output = await runDirectAnalysis( + fixture.dependencies(fixture.createProvider()), + fixture.path, + "binary_overview", + {}, + { snapshotPath: fixture.snapshotPath }, + ); + expect(parseEvidence(output).operation).toBe("binary_overview"); + const saved = await readAnalysisSnapshot(fixture.snapshotPath); + if (!saved.ok) throw saved.error; + expect(saved.value.workflow_entries).toHaveLength(1); + expect(cleanup).toHaveBeenCalledTimes(2); + } finally { + cleanup.mockRestore(); + } + }); + + it("saves snapshots after closing and reopening the same session", async () => { + const fixture = await createSnapshotReplayFixture("rea-snapshot-reopen-"); + const session = createTestBinarySession(fixture.createProvider()); + try { + for (const index of [1, 2]) { + const opened = await session.open(fixture.path); + if (!opened.ok) throw opened.error; + const analyzed = await session.execute("list_segments", {}); + if (!analyzed.ok) throw analyzed.error; + const path = join( + dirname(fixture.snapshotPath), + `snapshot-${index}.json`, + ); + const saved = await session.closeWithSnapshot(path, false); + if (!saved.ok) throw saved.error; + const loaded = await readAnalysisSnapshot(path); + if (!loaded.ok) throw loaded.error; + expect(loaded.value.target.sha256).toBe(opened.value.sha256); + } + } finally { + const closed = await session.close(); + if (!closed.ok) throw closed.error; + } + }); +}); + describe("direct analysis composed snapshot replay", () => { it.each([ { @@ -530,15 +584,15 @@ describe("binary overview snapshot replay", () => { ); if (current === undefined) throw new Error("composed Evidence was not saved"); - expect( - ( - await writeAnalysisSnapshot( - withEarlierHistoricalEvidence(loaded.value, fixture.path, current), - fixture.snapshotPath, - true, - ) - ).ok, - ).toBe(true); + const written = await withArtifactResourceScope((resources) => + writeAnalysisSnapshot( + withEarlierHistoricalEvidence(loaded.value, fixture.path, current), + fixture.snapshotPath, + true, + resources, + ), + ); + if (!written.ok) throw written.error; const second = await runDirectAnalysis( dependencies, @@ -735,15 +789,15 @@ describe("workflow snapshot profile and cancellation binding", () => { ); if (current === undefined) throw new Error("composed Evidence was not saved"); - expect( - ( - await writeAnalysisSnapshot( - withAlternateWorkflowProfile(loaded.value, fixture.path, current), - fixture.snapshotPath, - true, - ) - ).ok, - ).toBe(true); + const written = await withArtifactResourceScope((resources) => + writeAnalysisSnapshot( + withAlternateWorkflowProfile(loaded.value, fixture.path, current), + fixture.snapshotPath, + true, + resources, + ), + ); + if (!written.ok) throw written.error; const refreshed = await runDirectAnalysis( dependencies, diff --git a/tests/boundary/filesystem/evidenceBundleFiles.test.ts b/tests/boundary/filesystem/evidenceBundleFiles.test.ts index 267d381cef..d93afe620d 100644 --- a/tests/boundary/filesystem/evidenceBundleFiles.test.ts +++ b/tests/boundary/filesystem/evidenceBundleFiles.test.ts @@ -11,6 +11,7 @@ import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { withArtifactResourceScope } from "../../fixtures/artifactInventory.js"; import { readEvidenceBundle, @@ -99,15 +100,10 @@ describe("evidence bundle export cancellation", () => { yield "unused"; } expect( - await writeTextParts( - parts(), - join(root, "missing", "bundle.json"), - false, - { - signal: controller.signal, - operation: "export_evidence_bundle", - }, - ), + await writeParts(parts(), join(root, "missing", "bundle.json"), false, { + signal: controller.signal, + operation: "export_evidence_bundle", + }), ).toMatchObject({ ok: false, error: { @@ -118,7 +114,7 @@ describe("evidence bundle export cancellation", () => { expect(advanced).toBe(false); expect(await readdir(root)).toEqual([]); expect( - await writeEvidenceBundle( + await writeBundle( bundle(), join(root, "bundle.json"), false, @@ -149,7 +145,7 @@ describe("evidence bundle export cancellation", () => { } } expect( - await writeTextParts(parts(), path, overwrite, { + await writeParts(parts(), path, overwrite, { signal: controller.signal, operation: "export_evidence_bundle", }), @@ -164,9 +160,9 @@ describe("evidence bundle export cancellation", () => { expect(continued).toBe(false); expect(await readdir(root)).toEqual(overwrite ? ["bundle.json"] : []); if (overwrite) expect(await readFile(path, "utf8")).toBe("original"); - expect( - await writeEvidenceBundle(bundle(), path, overwrite), - ).toMatchObject({ ok: true }); + expect(await writeBundle(bundle(), path, overwrite)).toMatchObject({ + ok: true, + }); expect(await readEvidenceBundle(path)).toEqual({ ok: true, value: bundle(), @@ -184,7 +180,7 @@ describe("evidence bundle export cancellation", () => { controller.abort(); } expect( - await writeTextParts(parts(), path, true, { + await writeParts(parts(), path, true, { signal: controller.signal, operation: "export_evidence_bundle", }), @@ -194,6 +190,29 @@ describe("evidence bundle export cancellation", () => { }); }); +const writeBundle = ( + value: ReturnType, + path: string, + overwrite: boolean, + signal?: AbortSignal, +) => + withArtifactResourceScope((resources) => + writeEvidenceBundle(value, path, overwrite, resources, signal), + ); + +const writeParts = ( + parts: Iterable, + path: string, + overwrite: boolean, + cancellation?: { + readonly signal: AbortSignal; + readonly operation: string; + }, +) => + withArtifactResourceScope((resources) => + writeTextParts(parts, path, overwrite, resources, cancellation), + ); + describe("evidence bundle publication", () => { it.each([false, true])( "never publishes a partial streamed file (overwrite: %s)", @@ -205,9 +224,7 @@ describe("evidence bundle publication", () => { yield "partial"; throw new Error("Source stream failed"); } - expect( - await writeTextParts(brokenParts(), path, overwrite), - ).toMatchObject({ + expect(await writeParts(brokenParts(), path, overwrite)).toMatchObject({ ok: false, error: { _tag: "EvidenceFileError", reason: "io" }, }); @@ -220,7 +237,7 @@ describe("evidence bundle publication", () => { const root = await createTestTempDirectory("rea-stream-bytes-"); const path = join(root, "bundle.json"); const parts = ["雪", "😀", "\n"]; - expect(await writeTextParts(parts, path, false)).toEqual({ + expect(await writeParts(parts, path, false)).toEqual({ ok: true, value: { path, bytes: Buffer.byteLength(parts.join("")) }, }); @@ -233,7 +250,7 @@ describe("evidence bundle publication", () => { const path = join(root, "bundle.json"); const candidates = [bundle(), bundle(false)]; const results = await Promise.all( - candidates.map((value) => writeEvidenceBundle(value, path, false)), + candidates.map((value) => writeBundle(value, path, false)), ); expect(results.filter(({ ok }) => ok)).toHaveLength(1); expect(results.filter(({ ok }) => !ok)).toMatchObject([ @@ -253,7 +270,7 @@ describe("evidence bundle publication", () => { await mkdir(directory); const path = join(directory, "bundle.json"); const value = bundle(); - expect(await writeEvidenceBundle(value, path, false)).toMatchObject({ + expect(await writeBundle(value, path, false)).toMatchObject({ ok: true, }); expect(await readEvidenceBundle(path)).toEqual({ @@ -308,7 +325,7 @@ describe("evidence bundle filesystem adapter", () => { const directory = await createTestTempDirectory("rea-evidence-"); const path = join(directory, "bundle.json"); const evidenceBundle = bundle(); - const first = await writeEvidenceBundle(evidenceBundle, path, false); + const first = await writeBundle(evidenceBundle, path, false); expect(first).toMatchObject({ ok: true, value: { path } }); expect(await readFile(path, "utf8")).toBe( serializeEvidenceBundle(evidenceBundle), @@ -329,15 +346,13 @@ describe("evidence bundle filesystem adapter", () => { summary: { records_unchanged: 1 }, }, }); - expect( - await writeEvidenceBundle(evidenceBundle, path, false), - ).toMatchObject({ + expect(await writeBundle(evidenceBundle, path, false)).toMatchObject({ ok: false, error: { _tag: "EvidenceFileError", reason: "exists" }, }); - expect(await writeEvidenceBundle(evidenceBundle, path, true)).toMatchObject( - { ok: true }, - ); + expect(await writeBundle(evidenceBundle, path, true)).toMatchObject({ + ok: true, + }); }); it("reads the caller-selected symlink target and refuses to replace a symlink", async () => { @@ -352,7 +367,7 @@ describe("evidence bundle filesystem adapter", () => { ok: true, value: bundle(), }); - expect(await writeEvidenceBundle(bundle(), link, true)).toMatchObject({ + expect(await writeBundle(bundle(), link, true)).toMatchObject({ ok: false, error: { _tag: "EvidenceFileError", reason: "not-file" }, }); @@ -373,7 +388,7 @@ describe("evidence bundle filesystem adapter", () => { }); const orphan = join(directory, "absent", "bundle.json"); - const written = await writeEvidenceBundle(bundle(), orphan, false); + const written = await writeBundle(bundle(), orphan, false); expect(written).toMatchObject({ ok: false, error: { _tag: "EvidenceFileError", reason: "missing", path: orphan }, diff --git a/tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts b/tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts index 59525b439b..4dbf49ae0e 100644 --- a/tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts +++ b/tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts @@ -11,7 +11,78 @@ import { ArtifactReaderFailure } from "../../../../src/artifacts/ArtifactReader. import { scanCanonicalArtifactInventory } from "../../../fixtures/artifactInventory.js"; import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; -import { analyzeJavaScriptApplication } from "../../../support/javascriptApplicationScope.js"; +import { AnalysisCancelledError } from "../../../../src/domain/analysisErrorCore.js"; +import { + analyzeJavaScriptApplication, + readJavaScriptArtifactFiles, +} from "../../../support/javascriptApplicationScope.js"; +import { createJavaScriptArtifactReader } from "../../../../src/artifacts/javascript/JavaScriptArtifactReader.js"; +import { createJavaScriptSourceAnalysis } from "../../../../src/artifacts/javascript/JavaScriptSourceAnalysis.js"; +import { analyzeAndAdoptJavaScriptArtifactFiles } from "../../../../src/application/javascript/JavaScriptArtifactAnalysis.js"; +import { createJavaScriptSemanticGraphProjection } from "../../../../src/application/javascript/JavaScriptSemanticGraphBuilder.js"; + +describe("JavaScript source selection cancellation boundary", () => { + it("keeps unselected files outside scope when the real worker cancels after the last selected file", async () => { + const root = await createTestTempDirectory( + "rea-js-selected-last-cancellation-", + ); + await writeJavaScriptSourceSelectionFixture(root); + const snapshot = await scanCanonicalArtifactInventory(root, {}); + const reader = createJavaScriptArtifactReader(root, "directory"); + const controller = new AbortController(); + const sourcePaths = ["main.mjs"]; + try { + const files = await readJavaScriptArtifactFiles( + reader, + snapshot, + controller.signal, + sourcePaths, + ); + const worker = createJavaScriptSourceAnalysis( + { heapMb: 512, timeoutMs: 30_000 }, + 1, + ); + let outcome: Awaited< + ReturnType + >; + try { + const realWorker = worker.analyze.bind(worker); + outcome = await analyzeAndAdoptJavaScriptArtifactFiles( + files, + { + analyze: async (...args) => { + const result = await realWorker(...args); + controller.abort(); + return result; + }, + }, + createJavaScriptSemanticGraphProjection(), + async () => {}, + controller.signal, + ); + } finally { + await worker.close(); + } + expect(outcome.interruption).toBeInstanceOf(AnalysisCancelledError); + const unselected = outcome.analysis.files.filter( + ({ file }) => + !file.text.included && file.text.reason === "outside-analysis-scope", + ); + expect(unselected).toHaveLength(5); + expect( + unselected.every( + ({ analysis_failure }) => analysis_failure === undefined, + ), + ).toBe(true); + expect( + outcome.analysis.files.find(({ file }) => file.path === "main.mjs") + ?.javascript, + ).toMatchObject({ parse_status: "complete" }); + } finally { + await reader.close(); + } + }); +}); describe("Selected JavaScript source failure diagnostics", () => { it("analyzes selected files in first-request order and reports only selected progress", async () => { diff --git a/tests/boundary/filesystem/jsonOutputOwnership.test.ts b/tests/boundary/filesystem/jsonOutputOwnership.test.ts new file mode 100644 index 0000000000..daac89b9d4 --- /dev/null +++ b/tests/boundary/filesystem/jsonOutputOwnership.test.ts @@ -0,0 +1,273 @@ +import { readdirSync, renameSync, writeFileSync } from "node:fs"; +import { mkdir, open, rm } from "node:fs/promises"; +import * as fs from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, vi } from "vitest"; + +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; +import { EvidenceFileError } from "../../../src/domain/evidenceErrors.js"; +import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; +import { OwnedStagedOutput } from "../../../src/filesystem/OwnedStagedOutput.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { writeTextParts } from "../../../src/application/JsonFiles.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("reports committed output when post-publication staging cleanup remains owned", async () => { + const root = await createTestTempDirectory("rea-json-write-"); + const resources = new ArtifactResourceScope(); + try { + const result = await writeTextParts( + stagingPollutingParts(root), + join(root, "bundle.json"), + false, + resources, + ); + if (result.ok) throw new Error("Expected staged cleanup failure"); + const cleanupResource = result.error.cleanup?.resources[0]; + if (cleanupResource === undefined) + throw new Error("Expected retained staging resource"); + expect(result.error).toBeInstanceOf(EvidenceFileError); + expect(result.error.published).toMatchObject({ + path: join(root, "bundle.json"), + bytes: 16, + }); + expect(cleanupResource).toContain(".rea-write-"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { published: true, published_path: join(root, "bundle.json") }, + }); + expect(await fs.readFile(join(root, "bundle.json"), "utf8")).toBe( + "canonical output", + ); + await fs.rm(join(cleanupResource, "unexpected")); + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("preserves cancellation when staging cleanup also fails", async () => { + const root = await createTestTempDirectory("rea-json-cancel-"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + try { + function* cancelledParts(): Generator { + const directories = requireStages(root); + if (directories.length !== 1) + throw new Error("Expected one staging directory"); + writeFileSync(join(root, directories[0]!, "unexpected"), "retain this"); + controller.abort(); + yield "must not publish"; + } + const result = await writeTextParts( + cancelledParts(), + join(root, "cancelled.json"), + false, + resources, + { signal: controller.signal, operation: "export_evidence_bundle" }, + ); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cause).toMatchObject({ + cause: expect.any(AnalysisCancelledError), + owner: { directory: expect.stringContaining(".rea-write-") }, + }); + await expect(fs.access(join(root, "cancelled.json"))).rejects.toThrow(); + const stage = requireStages(root)[0]; + if (stage !== undefined) await fs.rm(join(root, stage, "unexpected")); + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("refuses to publish a staged path replaced after its descriptor opened", async () => { + const root = await createTestTempDirectory("rea-staged-swap-"); + const resources = new ArtifactResourceScope(); + const destination = join(root, "swapped.json"); + try { + const result = await writeTextParts( + replacingStagedParts(root), + destination, + false, + resources, + ); + if (result.ok) throw new Error("Expected staged identity rejection"); + expect( + await fs.readFile(join(root, requireStages(root)[0]!, "content"), "utf8"), + ).toBe("foreign bytes"); + await expect(fs.access(destination)).rejects.toThrow(); + const stage = requireStages(root)[0]; + if (stage !== undefined) { + const directory = join(root, stage); + await fs.rm(join(directory, "content")); + await fs.rename(join(directory, "original"), join(directory, "content")); + } + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("does not invent a staged owner when the supplied scope is closed", async () => { + const root = await createTestTempDirectory("rea-closed-output-scope-"); + const resources = new ArtifactResourceScope(); + await resources.close(); + const destination = join(root, "closed.json"); + const result = await writeTextParts( + ["unused"], + destination, + false, + resources, + ); + if (result.ok) throw new Error("Expected closed-scope admission failure"); + expect(result.error).toBeInstanceOf(EvidenceFileError); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + expect(await fs.readdir(root)).toEqual([]); +}); + +function stagingPollutingParts(root: string): Iterable { + return { + *[Symbol.iterator](): Generator { + const directories = readdirSync(root); + const stage = directories.find((entry) => + entry.startsWith(".rea-write-"), + ); + if (stage === undefined) throw new Error("Expected staging directory"); + writeFileSync(join(root, stage, "unexpected"), "retain this"); + yield "canonical output"; + }, + }; +} + +function replacingStagedParts(root: string): Iterable { + return { + *[Symbol.iterator](): Generator { + const stage = requireStages(root)[0]; + if (stage === undefined) throw new Error("Expected staging directory"); + const content = join(root, stage, "content"); + renameSync(content, join(root, stage, "original")); + writeFileSync(content, "foreign bytes"); + yield "owned bytes"; + }, + }; +} + +const requireStages = (root: string): string[] => + readdirSync(root).filter((entry) => entry.startsWith(".rea-write-")); + +it("retains a failed file close owner and retries before removing its stage", async () => { + const root = await createTestTempDirectory("rea-staged-close-"); + const directory = join(root, ".rea-write-owned"); + await mkdir(directory); + const path = join(directory, "content"); + const handle = await open(path, "wx", 0o600); + const owner = await OwnedStagedOutput.create(directory); + let closeAttempts = 0; + const actualClose = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeAttempts += 1; + if (closeAttempts === 1) throw new Error("injected close failure"); + await actualClose(); + }); + await owner.setFile(new OwnedFileHandle(handle)); + const resources = new ArtifactResourceScope(); + const stagedOwner = { + kind: "staged-output" as const, + resource: directory, + output: owner, + }; + try { + expect(await resources.release(stagedOwner)).toMatchObject({ + kind: "failed", + }); + await resources.close(); + expect(closeAttempts).toBe(2); + await expect(fs.access(directory)).rejects.toThrow(); + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("refuses to remove unexpected files from an owned staging directory", async () => { + const root = await createTestTempDirectory("rea-staged-replaced-"); + const directory = join(root, ".rea-write-owned"); + await mkdir(directory); + const owner = await OwnedStagedOutput.create(directory); + const unexpected = join(directory, "unexpected"); + writeFileSync(unexpected, "unowned"); + try { + await expect(owner.close()).rejects.toThrow(/Unexpected staging entries/u); + expect(await fs.readFile(unexpected, "utf8")).toBe("unowned"); + } finally { + await fs.rm(unexpected, { force: true }); + await owner.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("retains cleanup when its staging directory was renamed", async () => { + const root = await createTestTempDirectory("rea-staged-renamed-dir-"); + const directory = join(root, ".rea-write-owned"); + const moved = join(root, ".rea-write-moved"); + await mkdir(directory); + const path = join(directory, "content"); + const handle = await open(path, "wx", 0o600); + const owner = await OwnedStagedOutput.create(directory); + await owner.setFile(new OwnedFileHandle(handle)); + const resources = new ArtifactResourceScope(); + const stagedOwner = { + kind: "staged-output" as const, + resource: directory, + output: owner, + }; + try { + writeFileSync(join(directory, "unexpected"), "retain this"); + expect(await resources.release(stagedOwner)).toMatchObject({ + kind: "failed", + }); + await fs.rename(directory, moved); + await expect(resources.close()).rejects.toThrow(); + await fs.rename(moved, directory); + await rm(join(directory, "unexpected")); + await expect(resources.close()).resolves.toBeUndefined(); + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); + +it("retains cleanup when owned staged content was renamed", async () => { + const root = await createTestTempDirectory("rea-staged-renamed-file-"); + const directory = join(root, ".rea-write-owned"); + const content = join(directory, "content"); + const moved = join(directory, "moved-content"); + await mkdir(directory); + const handle = await open(content, "wx", 0o600); + const owner = await OwnedStagedOutput.create(directory); + await owner.setFile(new OwnedFileHandle(handle)); + const resources = new ArtifactResourceScope(); + const stagedOwner = { + kind: "staged-output" as const, + resource: directory, + output: owner, + }; + try { + await fs.rename(content, moved); + expect(await resources.release(stagedOwner)).toMatchObject({ + kind: "failed", + }); + await expect(resources.close()).rejects.toThrow( + /cleanup remains incomplete/u, + ); + await fs.rename(moved, content); + await expect(resources.close()).resolves.toBeUndefined(); + } finally { + await resources.close(); + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/tests/support/javascriptApplicationScope.ts b/tests/support/javascriptApplicationScope.ts index c9c920f02b..7f5d51b80f 100644 --- a/tests/support/javascriptApplicationScope.ts +++ b/tests/support/javascriptApplicationScope.ts @@ -12,9 +12,10 @@ export const readJavaScriptArtifactFiles = ( reader: ArtifactReader, snapshot: ArtifactInventorySnapshot, signal?: AbortSignal, + sourcePaths?: readonly string[], ) => withArtifactResourceScope((scope) => - readOwned(reader, snapshot, scope, signal), + readOwned(reader, snapshot, scope, signal, sourcePaths), ); /** Run a JavaScript analysis with an awaited, test-owned artifact scope. */