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. */