From 0e0af455f29d470e5e47bbee73442fe35d9276a0 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:53:08 +0800 Subject: [PATCH] fix(reference): preserve physical failures across late cancellation (#10072) * fix(reference): preserve physical failures across late cancellation * fix(reference): retain late cancellation alongside read failures * fix(reference): preserve metadata errors through cancellation * fix(reference): retain directory failure identity * fix(reference): preserve parallel physical and cancellation outcomes * fix(reference): preserve optional Git metadata absence --- src/application/ReferenceSourceImport.ts | 32 +- .../ReferenceSourceImportPolicy.ts | 24 +- .../ReferenceSourceImportTypes.test.ts | 17 + src/application/ReferenceSourceImportTypes.ts | 35 +- src/application/ReferenceSourceVcsAdapter.ts | 51 ++- src/reference/ReferenceSourceReader.ts | 18 +- src/reference/ReferenceSourceReaderEntries.ts | 98 +++-- src/reference/ReferenceSourceReaderFile.ts | 13 +- src/reference/ReferenceSourceReaderTypes.ts | 3 + .../ReferenceSourceReaderValidate.ts | 13 +- .../referenceSourceCleanupOwnership.test.ts | 364 ++++++++++++------ .../referenceSourceMetadataCleanup.test.ts | 275 ++++++++++++- .../filesystem/referenceSourceReader.test.ts | 37 ++ 13 files changed, 781 insertions(+), 199 deletions(-) diff --git a/src/application/ReferenceSourceImport.ts b/src/application/ReferenceSourceImport.ts index 9ff57563ec..4229125b03 100644 --- a/src/application/ReferenceSourceImport.ts +++ b/src/application/ReferenceSourceImport.ts @@ -263,33 +263,43 @@ export const importReferenceSource = async ( }; const combineReadFailures = ( - primary: ReferenceSourceReaderError, - other: ReferenceSourceReaderError | undefined, + inventory: ReferenceSourceReaderError, + metadata: ReferenceSourceReaderError | undefined, partial: ReferenceSourceRead | undefined, ): ReferenceSourceImportError => { - const code = - primary.code === "cancelled" || other?.code === "cancelled" - ? "cancelled" - : primary.code; + const primary = + inventory.code === "cancelled" && + metadata !== undefined && + metadata.code !== "cancelled" + ? metadata + : inventory; + const other = primary === inventory ? metadata : inventory; const cleanup = mergeCleanup(primary.cleanup, other?.cleanup); const message = - other === undefined + other === undefined || other.code === "cancelled" ? primary.message - : `${primary.message}; additional reference metadata failure: ${other.message}`; + : `${primary.message}; additional reference failure: ${other.message}`; const cause = - other === undefined + other === undefined || other.code === "cancelled" ? primary.cause : new AggregateError( [primary, other], "Reference source inventory and metadata both failed", { cause: primary }, ); + const secondaryCancellation = + primary.secondaryCancellation ?? + other?.secondaryCancellation ?? + (other?.code === "cancelled" && primary.code !== "cancelled" + ? { cause: other.cause } + : undefined); return { - ...failure(code, message), - ...(primary.remediationAction === undefined || code !== primary.code + ...failure(primary.code, message), + ...(primary.remediationAction === undefined ? {} : { remediationAction: primary.remediationAction }), ...(cleanup === undefined ? {} : { cleanup }), + ...(secondaryCancellation === undefined ? {} : { secondaryCancellation }), ...(partial === undefined ? {} : { partial }), ...(cause === undefined ? {} : { cause }), }; diff --git a/src/application/ReferenceSourceImportPolicy.ts b/src/application/ReferenceSourceImportPolicy.ts index 222356c934..f8531c4fa8 100644 --- a/src/application/ReferenceSourceImportPolicy.ts +++ b/src/application/ReferenceSourceImportPolicy.ts @@ -106,10 +106,10 @@ const resolveRoot = async ( }); return ok(canonicalRoot); } catch (cause: unknown) { - if (isAborted(signal)) + if (signal?.aborted === true && cause === signal.reason) return err({ ...failure("cancelled", "Reference source import cancelled"), - cause: signal?.reason, + cause: signal.reason, }); const rootFailure = rootFilesystemFailure( cause, @@ -122,6 +122,9 @@ const resolveRoot = async ( ? {} : { remediationAction: rootFailure.remediationAction }), cause, + ...(signal?.aborted === true + ? { secondaryCancellation: { cause: signal.reason } } + : {}), }); } }; @@ -182,6 +185,12 @@ const readPolicyFile = async ( ), cleanup: retry.cleanup, cause: primaryCause, + ...(signal !== undefined && + isAborted(signal) && + !isIgnorePolicyCancellation(primary) && + operationCancellation === undefined + ? { secondaryCancellation: { cause: signal.reason } } + : {}), }); } if (retry.outcome.kind === "failed") { @@ -202,6 +211,9 @@ const readPolicyFile = async ( `Reference source policy could not be read at ${policyPath}: ${errorMessage(causeValue)}`, ), cause: causeValue, + ...(signal?.aborted === true && !isIgnorePolicyCancellation(cause) + ? { secondaryCancellation: { cause: signal.reason } } + : {}), }); } }; @@ -253,7 +265,10 @@ const readIgnoreCase = async ( ); } catch (cause: unknown) { if (readFailure !== undefined) return err(readFailure); - if (isAborted(signal) || isIgnorePolicyCancellation(cause)) + if ( + isIgnorePolicyCancellation(cause) || + (signal?.aborted === true && cause === signal.reason) + ) return err({ ...failure("cancelled", "Reference source import cancelled"), cause: @@ -265,6 +280,9 @@ const readIgnoreCase = async ( `Reference source Git policy could not be read: ${errorMessage(cause)}`, ), cause, + ...(signal?.aborted === true + ? { secondaryCancellation: { cause: signal.reason } } + : {}), }); } }; diff --git a/src/application/ReferenceSourceImportTypes.test.ts b/src/application/ReferenceSourceImportTypes.test.ts index 76775e77bd..ea7bc4dcfd 100644 --- a/src/application/ReferenceSourceImportTypes.test.ts +++ b/src/application/ReferenceSourceImportTypes.test.ts @@ -38,6 +38,23 @@ describe("reference-source import error projection", () => { } }); + it("projects a later cancellation without replacing a physical failure", () => { + const reason = { caller: "stop after I/O failure" }; + const projected = projectReferenceSourceImportError({ + tag: "reference-source-import", + code: "io", + message: "directory read failed", + secondaryCancellation: { cause: reason }, + }); + expect(projected.category).toBe("execution_failure"); + expect(projected.details?.secondary_failures).toEqual([ + expect.objectContaining({ category: "cancelled" }), + ]); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe( + true, + ); + }); + it("preserves cancellation, partial facts, and cleanup diagnostics", () => { const bytes = Buffer.from("export const observed = true;\n"); const partial: ReferenceSourceRead = { diff --git a/src/application/ReferenceSourceImportTypes.ts b/src/application/ReferenceSourceImportTypes.ts index 488e419a68..6f8d022d00 100644 --- a/src/application/ReferenceSourceImportTypes.ts +++ b/src/application/ReferenceSourceImportTypes.ts @@ -4,6 +4,7 @@ import { AnalysisInputError, AnalysisOutputError, } from "../domain/analysisErrorCore.js"; +import { analysisErrorWithCancellation } from "../domain/analysisErrorCancellation.js"; import { projectAnalysisError, type AnalysisErrorProjection, @@ -25,6 +26,7 @@ export interface ReferenceSourceImportError { readonly cleanup?: AnalysisCleanupObservation; readonly partial?: ReferenceSourceRead; readonly cause?: unknown; + readonly secondaryCancellation?: { readonly cause: unknown }; } const IMPORT_FAILURE_GUIDANCE = { @@ -71,14 +73,16 @@ export const referenceSourceImportAnalysisError = ( }), }, }; + let primary: AnalysisError; switch (error.code) { case "cancelled": - return new AnalysisCancelledError("import_reference_source", { + primary = new AnalysisCancelledError("import_reference_source", { ...options, userMessage: message, }); + break; case "invalid-root": - return new AnalysisInputError( + primary = new AnalysisInputError( "import_reference_source", { ...options, @@ -89,21 +93,36 @@ export const referenceSourceImportAnalysisError = ( }, [{ path: ["root"], reason: "invalid_value", message: error.message }], ); + break; case "unsupported": - return new AnalysisCapabilityUnavailableError( + primary = new AnalysisCapabilityUnavailableError( "filesystem", "import_reference_source", error.message, { ...options, userMessage: message, remediationAction: guidance }, ); + break; case "io": case "parse": - return new AnalysisOutputError("import_reference_source", error.message, { - ...options, - userMessage: message, - remediationAction: guidance, - }); + primary = new AnalysisOutputError( + "import_reference_source", + error.message, + { + ...options, + userMessage: message, + remediationAction: guidance, + }, + ); + break; } + return error.secondaryCancellation === undefined + ? primary + : analysisErrorWithCancellation( + primary, + new AnalysisCancelledError("import_reference_source", { + cause: error.secondaryCancellation.cause, + }), + ); }; /** Canonical CLI/MCP projection, retaining source facts under diagnostic details. */ diff --git a/src/application/ReferenceSourceVcsAdapter.ts b/src/application/ReferenceSourceVcsAdapter.ts index 904828e288..17ea31210c 100644 --- a/src/application/ReferenceSourceVcsAdapter.ts +++ b/src/application/ReferenceSourceVcsAdapter.ts @@ -4,8 +4,15 @@ import { locateReferenceSourceGitDirectory } from "./ReferenceSourceGitDirectory import { err, ok, type Result } from "../domain/result.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; -import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js"; -import { readRegularFile, retryRegularFileCleanup } from "./RegularFileRead.js"; +import { + RegularFileCleanupFailure, + regularFileCancellation, +} from "../filesystem/OwnedRegularFile.js"; +import { + readRegularFile, + RegularFileReadFailure, + retryRegularFileCleanup, +} from "./RegularFileRead.js"; import { referenceSourceResourceFailure } from "../reference/ReferenceSourceReaderErrors.js"; import type { ReferenceSourceReaderError } from "../reference/ReferenceSourceReaderTypes.js"; @@ -39,7 +46,12 @@ export const readReferenceSourceVcs = async ( }); } catch (cause: unknown) { if (!(cause instanceof ArtifactReaderFailure)) throw cause; - return err(referenceSourceResourceFailure(cause)); + return err({ + ...referenceSourceResourceFailure(cause), + ...(cause.reason !== "cancelled" && signal?.aborted === true + ? { secondaryCancellation: { cause: signal.reason } } + : {}), + }); } }; @@ -63,7 +75,25 @@ const resolveSourceVcs = async ( return await readRegularFile(path, { signal }); } catch (cause: unknown) { if (signal?.aborted === true && cause === signal.reason) throw cause; - if (!(cause instanceof RegularFileCleanupFailure)) throw cause; + if (!(cause instanceof RegularFileCleanupFailure)) { + if ( + signal?.aborted === true && + regularFileCancellation(cause) === undefined + ) { + const physical = + cause instanceof RegularFileReadFailure + ? cause.operationCause + : cause; + // The Git directory owner interprets absence of optional metadata. + if (errorCode(physical) === "ENOENT") throw physical; + throw new ArtifactReaderFailure( + "io", + `Git metadata could not be read safely: ${errorMessage(physical)}`, + { cause: physical }, + ); + } + throw cause; + } const retry = await retryRegularFileCleanup(cause, resources, signal); if ( retry.outcome.kind === "failed" && @@ -81,7 +111,10 @@ const resolveSourceVcs = async ( ? retry.outcome.cause : cause.cleanupCause; throw new ArtifactReaderFailure( - isAborted(signal) ? "cancelled" : "io", + regularFileCancellation(cause) !== undefined || + (signal?.aborted === true && primary === signal.reason) + ? "cancelled" + : "io", `Git metadata could not be read safely: ${errorMessage(primary)}`, { cause: primary, @@ -97,7 +130,11 @@ const resolveSourceVcs = async ( failure.cleanup === undefined ? [] : [failure.cleanup], ); throw new ArtifactReaderFailure( - isAborted(signal) ? "cancelled" : "io", + (primary instanceof ArtifactReaderFailure && + primary.reason === "cancelled") || + (signal?.aborted === true && primary === signal.reason) + ? "cancelled" + : "io", `Git metadata could not be read safely: ${errorMessage(primary)}`, { cause: new AggregateError( @@ -155,6 +192,8 @@ const resolveSourceVcs = async ( } catch (cause: unknown) { if (cause instanceof ArtifactReaderFailure) { cleanupFailures.push(cause); + if (cause.reason !== "cancelled") + failedRead ??= { cause: cause.cause ?? cause }; } else if (errorCode(cause) !== "ENOENT") failedRead ??= { cause }; throw cause; } diff --git a/src/reference/ReferenceSourceReader.ts b/src/reference/ReferenceSourceReader.ts index 01d18a3357..bc7d83f8dd 100644 --- a/src/reference/ReferenceSourceReader.ts +++ b/src/reference/ReferenceSourceReader.ts @@ -33,12 +33,23 @@ export const readReferenceSource = async ( resources: ArtifactResourceScope, options: ReferenceSourceReaderOptions = {}, ): Promise> => { + let result: ReferenceSourceResult; try { - return await resources.run(() => readSource(root, resources, options)); + result = await resources.run(() => readSource(root, resources, options)); } catch (cause: unknown) { if (!(cause instanceof ArtifactReaderFailure)) throw cause; - return err(referenceSourceResourceFailure(cause)); + result = err(referenceSourceResourceFailure(cause)); } + if ( + !result.ok && + result.error.code !== "cancelled" && + options.signal?.aborted + ) + return err({ + ...result.error, + secondaryCancellation: { cause: options.signal.reason }, + }); + return result; }; const readSource = async ( @@ -99,6 +110,9 @@ const readSource = async ( tag: "reference-source-reader", code: "io", message: state.rootFailure.message, + ...(state.rootFailureCause === undefined + ? {} + : { cause: state.rootFailureCause.value }), partial: read, }); return traversal.ok ? ok(read) : err({ ...traversal.error, partial: read }); diff --git a/src/reference/ReferenceSourceReaderEntries.ts b/src/reference/ReferenceSourceReaderEntries.ts index 38f7bd67a5..39dc66fa76 100644 --- a/src/reference/ReferenceSourceReaderEntries.ts +++ b/src/reference/ReferenceSourceReaderEntries.ts @@ -69,8 +69,6 @@ export const traverseDirectory = async ( current.path, state.signal, ); - if (isAborted(state.signal)) - return { ok: false, error: cancelled(state.signal) }; if (!before.ok) { recordDirectoryFailure( state, @@ -79,10 +77,26 @@ export const traverseDirectory = async ( before.message, before.localDiagnostic, ); - return { ok: true, value: undefined }; + return isAborted(state.signal) + ? err(cancelled(state.signal)) + : ok(undefined); } + if (isAborted(state.signal)) return err(cancelled(state.signal)); const directories: PendingDirectory[] = []; let readFailure: FilesystemFailureDetail | undefined; + let readFailureCause: { readonly value: unknown } | undefined; + const recordReadFailure = ( + detail: FilesystemFailureDetail, + ): FilesystemFailureDetail => { + recordDirectoryFailure( + state, + current.path, + "io", + detail.message, + detail.localDiagnostic, + ); + return detail; + }; let unrepresentableNameCount = 0; let firstUnrepresentableNameHex: string | undefined; let handle: Dir | undefined; @@ -94,7 +108,9 @@ export const traverseDirectory = async ( encoding: "buffer" as BufferEncoding, }); } catch (cause: unknown) { - readFailure = directoryReadFailure(cause); + readFailureCause = { value: cause }; + if (current.path === state.root) state.rootFailureCause = readFailureCause; + readFailure = recordReadFailure(directoryReadFailure(cause)); } if (handle !== undefined) { const owner = new OwnedDirectoryHandle(handle); @@ -115,14 +131,17 @@ export const traverseDirectory = async ( try { child = await handle.read(); } catch (cause: unknown) { - readFailure = directoryReadFailure(cause); + readFailureCause = { value: cause }; + if (current.path === state.root) + state.rootFailureCause = readFailureCause; + readFailure = recordReadFailure(directoryReadFailure(cause)); break; } if (child === null) break; if (!Buffer.isBuffer(child.name)) { - readFailure = { + readFailure = recordReadFailure({ message: "Directory entry name was not returned as raw bytes", - }; + }); break; } const decoded = decodeDirectoryEntryName(child.name); @@ -141,28 +160,28 @@ export const traverseDirectory = async ( }); } if (cleanup.kind === "failed") { - if (result.ok && !isAborted(state.signal) && readFailure !== undefined) - recordDirectoryFailure( - state, - current.path, - "io", - readFailure.message, - readFailure.localDiagnostic, - ); const observation = ArtifactReaderFailure.cleanupObservation( cleanup.cause, current.path, ); const previous = result.ok ? undefined : result.error.cleanup; const primary = result.ok - ? isAborted(state.signal) - ? cancelled(state.signal) - : failure( - "io", - readFailure?.localDiagnostic ?? - readFailure?.message ?? + ? readFailure !== undefined + ? { + ...failure( + "io", + readFailure.localDiagnostic ?? readFailure.message, + ), + ...(readFailureCause === undefined + ? {} + : { cause: readFailureCause.value }), + } + : isAborted(state.signal) + ? cancelled(state.signal) + : failure( + "io", `Reference directory cleanup failed: ${current.path}`, - ) + ) : result.error; return err({ ...primary, @@ -180,27 +199,24 @@ export const traverseDirectory = async ( } if (!result.ok) return result; } - if (isAborted(state.signal)) - return { ok: false, error: cancelled(state.signal) }; if (readFailure !== undefined) { - recordDirectoryFailure( - state, - current.path, - "io", - readFailure.message, - readFailure.localDiagnostic, - ); scheduleObservedDirectories(state, directories); - return { ok: true, value: undefined }; + return isAborted(state.signal) + ? err({ + ...failure("io", readFailure.localDiagnostic ?? readFailure.message), + ...(readFailureCause === undefined + ? {} + : { cause: readFailureCause.value }), + }) + : ok(undefined); } + if (isAborted(state.signal)) return err(cancelled(state.signal)); const after = await validateDirectory( state.root, state.rootIdentity, current.path, state.signal, ); - if (isAborted(state.signal)) - return { ok: false, error: cancelled(state.signal) }; if (!after.ok || !sameFile(before.stats, after.stats)) { recordDirectoryFailure( state, @@ -210,8 +226,11 @@ export const traverseDirectory = async ( after.ok ? undefined : after.localDiagnostic, ); scheduleObservedDirectories(state, directories); - return { ok: true, value: undefined }; + return isAborted(state.signal) + ? err(cancelled(state.signal)) + : ok(undefined); } + if (isAborted(state.signal)) return err(cancelled(state.signal)); if (unrepresentableNameCount > 0) { recordDirectoryFailure( state, @@ -272,8 +291,6 @@ const processEntry = async ( const absolute = join(current.path, name); const path = pathFromRoot(state.root, absolute); const metadata = await readMetadata(absolute); - if (isAborted(state.signal)) - return { ok: false, error: cancelled(state.signal) }; if (!metadata.ok) { const excluded = applyExclusion(state.shouldExclude, path, "unknown"); if (!excluded.ok) @@ -295,8 +312,11 @@ const processEntry = async ( metadata.localDiagnostic, ), ); - return { ok: true, value: undefined }; + return isAborted(state.signal) + ? err(cancelled(state.signal)) + : ok(undefined); } + if (isAborted(state.signal)) return err(cancelled(state.signal)); const kind = metadata.value.isSymbolicLink() ? "symlink" : metadata.value.isDirectory() @@ -400,6 +420,7 @@ export const readStableSymlinkTarget = async ( if (!before.isSymbolicLink() || !sameFile(expected, before)) return changedSymlink(path); const observed = await readTarget(); + if (observed.status === "failed") return observed; if (isAborted(signal)) return cancelledSymlink(path); const after = await bigLstat(absolute); if (isAborted(signal)) return cancelledSymlink(path); @@ -407,7 +428,6 @@ export const readStableSymlinkTarget = async ( return changedSymlink(path); return observed; } catch (cause: unknown) { - if (isAborted(signal)) return cancelledSymlink(path); const code = cause instanceof Error ? Reflect.get(cause, "code") : undefined; if (code === "ENOENT" || code === "ENOTDIR") return changedSymlink(path); diff --git a/src/reference/ReferenceSourceReaderFile.ts b/src/reference/ReferenceSourceReaderFile.ts index c4d8def722..1f9f853281 100644 --- a/src/reference/ReferenceSourceReaderFile.ts +++ b/src/reference/ReferenceSourceReaderFile.ts @@ -173,7 +173,16 @@ const finalizeFileRead = async ( dirname(absolute), request.signal, ); - if (!parentAfter.ok || !sameFile(parentBefore.stats, parentAfter.stats)) + if (!parentAfter.ok) + return entryFailure( + path, + "file", + parentAfter.code, + parentAfter.message, + total, + parentAfter.localDiagnostic, + ); + if (!sameFile(parentBefore.stats, parentAfter.stats)) return entryFailure( path, "file", @@ -279,7 +288,7 @@ const readStableFileEntry = async ( retain(cause.owner); cause = cause.cause; } - if (regularFileCancellation(cause) !== undefined || isAborted(signal)) + if (regularFileCancellation(cause) !== undefined) return entryFailure( path, "file", diff --git a/src/reference/ReferenceSourceReaderTypes.ts b/src/reference/ReferenceSourceReaderTypes.ts index c7422db3ea..2f0f90e942 100644 --- a/src/reference/ReferenceSourceReaderTypes.ts +++ b/src/reference/ReferenceSourceReaderTypes.ts @@ -88,6 +88,8 @@ export interface ReferenceSourceReaderError { readonly message: string; readonly remediationAction?: string; readonly cleanup?: AnalysisCleanupObservation; + /** Exact caller cancellation observed after a distinct failure. */ + readonly secondaryCancellation?: { readonly cause: unknown }; /** Captured entries before failure; this does not establish complete tree coverage. */ readonly partial?: ReferenceSourceRead; readonly cause?: unknown; @@ -117,6 +119,7 @@ export type TraversalState = { }>; readonly pending: PendingDirectory[]; rootFailure?: ReferenceSourceRootFailure; + rootFailureCause?: { readonly value: unknown }; bytesRead: number; }; diff --git a/src/reference/ReferenceSourceReaderValidate.ts b/src/reference/ReferenceSourceReaderValidate.ts index f69fb75081..f2ca0c5da1 100644 --- a/src/reference/ReferenceSourceReaderValidate.ts +++ b/src/reference/ReferenceSourceReaderValidate.ts @@ -45,6 +45,12 @@ export const validateDirectory = async ( readonly localDiagnostic?: string; } > => { + if (isAborted(signal)) + return { + ok: false, + code: "cancelled", + message: "Reference source traversal cancelled", + }; try { const rootNow = await bigLstat(root); if (!sameFile(rootIdentity, rootNow)) @@ -71,12 +77,6 @@ export const validateDirectory = async ( }; return { ok: true, stats }; } catch (cause: unknown) { - if (isAborted(signal)) - return { - ok: false, - code: "cancelled", - message: "Reference source traversal cancelled", - }; const detail = filesystemFailureDetail( cause, "Directory identity could not be verified", @@ -121,7 +121,6 @@ export const prepareRoot = async ( ); return ok({ canonicalRoot, rootIdentity: canonicalMetadata }); } catch (cause: unknown) { - if (isAborted(signal)) return err(cancelled(signal)); const rootFailure = rootFilesystemFailure( cause, "Reference source root could not be resolved", diff --git a/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts b/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts index ea8a7e196e..ec0e0d0182 100644 --- a/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts +++ b/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts @@ -1,5 +1,6 @@ import { mkdir, + lstat as fsLstat, open as fsOpen, opendir as fsOpendir, writeFile, @@ -12,6 +13,7 @@ import { afterEach, beforeEach, expect, it, onTestFinished, vi } from "vitest"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { importReferenceSource } from "../../../src/application/ReferenceSourceImport.js"; +import { projectReferenceSourceImportError } from "../../../src/application/ReferenceSourceImportTypes.js"; import { readReferenceSource } from "../../../src/reference/ReferenceSourceReader.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { err, ok } from "../../../src/domain/result.js"; @@ -21,11 +23,13 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...actual, open: vi.fn(actual.open), + lstat: vi.fn(actual.lstat), opendir: vi.fn(actual.opendir), }; }); const openMock = vi.mocked(fsOpen); +const lstatMock = vi.mocked(fsLstat); const opendirMock = vi.mocked(fsOpendir); let actual: typeof import("node:fs/promises"); @@ -40,14 +44,77 @@ beforeEach(async () => { "node:fs/promises", ); openMock.mockImplementation((...args) => actual.open(...args)); + lstatMock.mockImplementation((...args) => actual.lstat(...args)); opendirMock.mockImplementation((...args) => actual.opendir(...args)); }); afterEach(() => { openMock.mockReset(); + lstatMock.mockReset(); opendirMock.mockReset(); }); +it.each([ + "root-admission", + "directory-validation", + "entry-metadata", + "file-finalization", +] as const)( + "retains physical metadata failure during cancellation at %s", + async (phase) => { + const root = await createTestTempDirectory("rea-reference-metadata-abort-"); + const file = join(root, "source.ts"); + if (phase === "entry-metadata" || phase === "file-finalization") + await writeFile(file, "observed"); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); + const controller = new AbortController(); + const physical = Object.assign(new Error("authored metadata EIO"), { + code: "EIO", + }); + let armed = phase === "root-admission"; + if (phase === "file-finalization") + openMock.mockImplementation(async (...args) => { + const handle = await actual.open(...args); + const read = handle.read.bind(handle); + vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => { + const result = await read(...readArgs); + if (result.bytesRead === 0) armed = true; + return result; + }); + return handle; + }); + lstatMock.mockImplementation(async (...args) => { + if (!armed || (phase === "entry-metadata" && String(args[0]) !== file)) + return actual.lstat(...args); + controller.abort("during failed metadata lookup"); + throw physical; + }); + const result = await readReferenceSource(root, resources, { + signal: controller.signal, + beforeDirectory: async () => { + if (phase !== "file-finalization") armed = true; + return ok(undefined); + }, + }); + if (result.ok) throw new Error("Metadata failure was ignored"); + if (phase === "root-admission") { + expect(result.error).toMatchObject({ code: "io", cause: physical }); + } else if (phase === "directory-validation") { + expect(result.error.code).toBe("cancelled"); + expect(result.error.partial?.rootFailure).toMatchObject({ + code: "io", + message: expect.stringContaining("EIO"), + }); + } else { + expect(result.error.code).toBe("cancelled"); + expect(result.error.partial?.entries).toContainEqual( + expect.objectContaining({ path: "source.ts", code: "io" }), + ); + } + }, +); + it.each([ ["refusal", false], ["refusal", true], @@ -129,64 +196,75 @@ it.each([ }, ); -it("preserves admission EIO and retains an invalidated failed-close owner", async () => { - const root = await createTestTempDirectory("rea-reference-admission-owner-"); - const file = join(root, "source.ts"); - await writeFile(file, "export const answer = 42;\n"); - const resources = new ArtifactResourceScope(); - onTestFinished(() => resources.close().catch(() => undefined)); - const statFailure = Object.assign(new Error("injected admission EIO"), { - code: "EIO", - }); - const closeFailure = new Error("native close rejected after invalidation"); - let closeCalls = 0; - - openMock.mockImplementation(async (...args) => { - const handle = await actual.open(...args); - vi.spyOn(handle, "stat").mockRejectedValueOnce(statFailure); - const close = handle.close.bind(handle); - vi.spyOn(handle, "close").mockImplementation(async () => { - closeCalls += 1; - await close(); - throw closeFailure; +it.each([false, true])( + "preserves admission EIO and retains its failed-close owner (cancelled=%s)", + async (cancelled) => { + const root = await createTestTempDirectory( + "rea-reference-admission-owner-", + ); + const file = join(root, "source.ts"); + await writeFile(file, "export const answer = 42;\n"); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close().catch(() => undefined)); + const statFailure = Object.assign(new Error("injected admission EIO"), { + code: "EIO", }); - return handle; - }); + const closeFailure = new Error("native close rejected after invalidation"); + const controller = new AbortController(); + let closeCalls = 0; - const result = await readReferenceSource(root, resources); - expect(result).toMatchObject({ - ok: false, - error: { - tag: "reference-source-reader", - code: "io", - message: expect.stringContaining("EIO"), + openMock.mockImplementation(async (...args) => { + const handle = await actual.open(...args); + vi.spyOn(handle, "stat").mockImplementationOnce(async () => { + if (cancelled) controller.abort("during failed admission"); + throw statFailure; + }); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeCalls += 1; + await close(); + throw closeFailure; + }); + return handle; + }); + + const result = await readReferenceSource(root, resources, { + signal: controller.signal, + }); + expect(result).toMatchObject({ + ok: false, + error: { + tag: "reference-source-reader", + code: "io", + message: expect.stringContaining("EIO"), + cleanup: { + resources: [file], + reason: expect.stringContaining(closeFailure.message), + }, + partial: { + entries: [ + { + status: "failed", + kind: "file", + path: "source.ts", + code: "io", + message: expect.stringContaining("EIO"), + }, + ], + }, + }, + }); + expect(closeCalls).toBe(1); + + await expect(resources.close()).rejects.toMatchObject({ cleanup: { resources: [file], reason: expect.stringContaining(closeFailure.message), }, - partial: { - entries: [ - { - status: "failed", - kind: "file", - path: "source.ts", - code: "io", - message: expect.stringContaining("EIO"), - }, - ], - }, - }, - }); - expect(closeCalls).toBe(1); - - await expect(resources.close()).rejects.toMatchObject({ - cleanup: { - resources: [file], - reason: expect.stringContaining(closeFailure.message), - }, - }); - expect(closeCalls).toBe(1); -}); + }); + expect(closeCalls).toBe(1); + }, +); const assertSourceCleanupRetry = async (enclosingGit: boolean) => { const workspace = await createTestTempDirectory("rea-reference-file-owner-"); @@ -285,77 +363,127 @@ it.each([false, true])( assertSourceCleanupRetry, ); -it("preserves a directory read failure when closing that directory also fails", async () => { - const root = await createTestTempDirectory( - "rea-reference-directory-read-owner-", - ); - const file = join(root, "source.ts"); - const bytes = "observed before directory EIO\n"; - await writeFile(file, bytes); - const resources = new ArtifactResourceScope(); - onTestFinished(() => resources.close().catch(() => undefined)); - const readFailure = Object.assign(new Error("injected directory EIO"), { - code: "EIO", - }); - const closeFailure = new Error("directory close rejected after invalidation"); - let readCalls = 0; - let closeCalls = 0; - - opendirMock.mockImplementation(async (...args) => { - const directory = await actual.opendir(...args); - const operations: DirectoryOperations = directory; - const read = operations.read.bind(directory); - vi.spyOn(operations, "read").mockImplementation(async () => { - if (readCalls++ > 0) throw readFailure; - return read(); +it.each([ + [false, false, "reader"], + [false, true, "reader"], + [true, false, "reader"], + [true, true, "reader"], + [true, false, "import"], + [true, true, "import"], +] as const)( + "preserves directory EIO through close (late cancellation=%s, close fails=%s, surface=%s)", + async (lateCancellation, closeFails, surface) => { + const root = await createTestTempDirectory( + "rea-reference-directory-read-owner-", + ); + const file = join(root, "source.ts"); + const bytes = "observed before directory EIO\n"; + await writeFile(file, bytes); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close().catch(() => undefined)); + const readFailure = Object.assign(new Error("injected directory EIO"), { + code: "EIO", }); - const close = operations.close.bind(directory); - vi.spyOn(operations, "close").mockImplementation(async () => { - closeCalls += 1; - await close(); - throw closeFailure; - }); - return directory; - }); + const closeFailure = new Error( + "directory close rejected after invalidation", + ); + const controller = new AbortController(); + const reason = { selected: "after directory read failed" }; + let readCalls = 0; + let closeCalls = 0; - const result = await readReferenceSource(root, resources); - if (result.ok) throw new Error("Directory and cleanup failures were ignored"); - expect(result).toMatchObject({ - ok: false, - error: { + opendirMock.mockImplementation(async (...args) => { + const directory = await actual.opendir(...args); + const operations: DirectoryOperations = directory; + const read = operations.read.bind(directory); + vi.spyOn(operations, "read").mockImplementation(async () => { + if (readCalls++ > 0) throw readFailure; + return read(); + }); + const close = operations.close.bind(directory); + vi.spyOn(operations, "close").mockImplementation(async () => { + closeCalls += 1; + if (lateCancellation) controller.abort(reason); + await close(); + if (closeFails) throw closeFailure; + }); + return directory; + }); + + const result = + surface === "reader" + ? await readReferenceSource(root, resources, { + signal: controller.signal, + }) + : await importReferenceSource( + { + root, + caller: "reference-cancellation-test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + if (result.ok) + throw new Error("Directory and cleanup failures were ignored"); + expect(result).toMatchObject({ + ok: false, + error: { + code: "io", + message: expect.stringContaining(readFailure.message), + ...(closeFails + ? { + cleanup: { + resources: [root], + reason: expect.stringContaining(closeFailure.message), + }, + } + : {}), + partial: { + bytesRead: Buffer.byteLength(bytes), + }, + }, + }); + if (lateCancellation) + expect(result.error.secondaryCancellation?.cause).toBe(reason); + expect(result.error.cause).toBe(readFailure); + if ( + surface === "import" && + result.error.tag === "reference-source-import" + ) { + const projected = projectReferenceSourceImportError(result.error); + expect(projected.details?.secondary_failures).toEqual([ + expect.objectContaining({ category: "cancelled" }), + ]); + } + expect(result.error.partial?.entries).toContainEqual( + expect.objectContaining({ + status: "read", + kind: "file", + path: "source.ts", + bytes: Buffer.from(bytes), + }), + ); + expect(result.error.partial?.rootFailure).toEqual({ code: "io", message: expect.stringContaining(readFailure.message), - cleanup: { - resources: [root], - reason: expect.stringContaining(closeFailure.message), - }, - partial: { - bytesRead: Buffer.byteLength(bytes), - }, - }, - }); - expect(result.error.partial?.entries).toContainEqual( - expect.objectContaining({ - status: "read", - kind: "file", - path: "source.ts", - bytes: Buffer.from(bytes), - }), - ); - expect(result.error.partial?.rootFailure).toEqual({ - code: "io", - message: expect.stringContaining(readFailure.message), - }); - expect(closeCalls).toBe(1); + }); + expect(closeCalls).toBe(1); - await expect(resources.close()).rejects.toMatchObject({ - cleanup: { - resources: [root], - reason: expect.stringContaining(closeFailure.message), - }, - }); - expect(closeCalls).toBe(1); -}); + if (closeFails) + await expect(resources.close()).rejects.toMatchObject({ + cleanup: { + resources: [root], + reason: expect.stringContaining(closeFailure.message), + }, + }); + else { + expect(result.error.cleanup).toBeUndefined(); + await resources.close(); + } + expect(closeCalls).toBe(1); + }, +); it("keeps cancelled traversal typed when native directory close rejects after invalidation", async () => { const root = await createTestTempDirectory("rea-reference-directory-owner-"); diff --git a/tests/boundary/filesystem/referenceSourceMetadataCleanup.test.ts b/tests/boundary/filesystem/referenceSourceMetadataCleanup.test.ts index 8f9131e52c..3613f52d62 100644 --- a/tests/boundary/filesystem/referenceSourceMetadataCleanup.test.ts +++ b/tests/boundary/filesystem/referenceSourceMetadataCleanup.test.ts @@ -1,4 +1,10 @@ -import { mkdir, open as fsOpen, writeFile } from "node:fs/promises"; +import { + mkdir, + open as fsOpen, + opendir as fsOpendir, + stat as fsStat, + writeFile, +} from "node:fs/promises"; import { join, resolve } from "node:path"; import { afterEach, beforeEach, expect, it, onTestFinished, vi } from "vitest"; @@ -11,10 +17,17 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, open: vi.fn(actual.open) }; + return { + ...actual, + open: vi.fn(actual.open), + stat: vi.fn(actual.stat), + opendir: vi.fn(actual.opendir), + }; }); const openMock = vi.mocked(fsOpen); +const opendirMock = vi.mocked(fsOpendir); +const statMock = vi.mocked(fsStat); let actual: typeof import("node:fs/promises"); beforeEach(async () => { @@ -23,9 +36,15 @@ beforeEach(async () => { "node:fs/promises", ); openMock.mockImplementation((...args) => actual.open(...args)); + opendirMock.mockImplementation((...args) => actual.opendir(...args)); + statMock.mockImplementation((...args) => actual.stat(...args)); }); -afterEach(() => openMock.mockReset()); +afterEach(() => { + openMock.mockReset(); + opendirMock.mockReset(); + statMock.mockReset(); +}); const failCloseUntilAllowed = ( target: string, @@ -315,3 +334,253 @@ it("keeps a later packed-ref read error primary over earlier loose-ref cleanup", Array.from({ length: looseCloseCalls.size }, () => 3), ); }); + +it("preserves a physical root admission failure when cancellation arrives during stat", async () => { + const root = await createTestTempDirectory("rea-reference-root-eio-"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { source: "caller" }; + const physical = Object.assign(new Error("root stat failed"), { + code: "EIO", + }); + statMock.mockImplementation(async (...args) => { + if (String(args[0]) !== root) return actual.stat(...args); + controller.abort(reason); + throw physical; + }); + try { + const result = await importReferenceSource( + { + root, + caller: "test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + expect(result).toMatchObject({ + ok: false, + error: { + code: "io", + cause: physical, + secondaryCancellation: { cause: reason }, + }, + }); + } finally { + await resources.close(); + } +}); + +it.each([false, true])( + "retains a physical Git metadata read failure and late cancellation through cleanup (close fails: %s)", + async (closeFails) => { + const root = await createTestTempDirectory("rea-reference-vcs-eio-"); + const gitdir = join(root, ".git"); + await mkdir(gitdir); + const head = join(gitdir, "HEAD"); + await writeFile(head, "ref: refs/heads/main\n"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { source: "caller" }; + const physical = Object.assign(new Error("metadata read failed"), { + code: "EIO", + }); + let allowClose = false; + openMock.mockImplementation(async (...args) => { + const handle = await actual.open(...args); + if (resolve(String(args[0])) !== resolve(head)) return handle; + vi.spyOn(handle, "read").mockImplementation(async () => { + throw physical; + }); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + controller.abort(reason); + if (closeFails && !allowClose) throw new Error("close failed"); + await close(); + }); + return handle; + }); + try { + const result = await readReferenceSourceVcs( + root, + resources, + controller.signal, + ); + expect(result).toMatchObject({ + ok: false, + error: { + code: "io", + secondaryCancellation: { cause: reason }, + ...(closeFails ? { cleanup: { resources: [head] } } : {}), + }, + }); + } finally { + allowClose = true; + await resources.close(); + } + }, +); + +it.each([false, true])( + "preserves a policy read failure and later cancellation (close fails: %s)", + async (closeFails) => { + const root = await createTestTempDirectory("rea-reference-policy-eio-"); + const policyPath = join(root, ".gitignore"); + await writeFile(policyPath, "ignored\n"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { source: "caller" }; + const physical = Object.assign(new Error("policy read failed"), { + code: "EIO", + }); + let allowClose = false; + openMock.mockImplementation(async (...args) => { + const handle = await actual.open(...args); + if (resolve(String(args[0])) !== resolve(policyPath)) return handle; + vi.spyOn(handle, "read").mockImplementation(async () => { + throw physical; + }); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + controller.abort(reason); + if (closeFails && !allowClose) throw new Error("close failed"); + await close(); + }); + return handle; + }); + try { + const result = await importReferenceSource( + { + root, + caller: "test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + expect(result).toMatchObject({ + ok: false, + error: { code: "io", secondaryCancellation: { cause: reason } }, + }); + if (result.ok) throw new Error("Policy failure was ignored"); + expect(result.error.message).toContain("policy read failed"); + expect(resources.cleanupResources).toEqual( + closeFails ? [policyPath] : [], + ); + } finally { + allowClose = true; + await resources.close(); + } + }, +); + +it("preserves physical Git failure and exact cancellation across parallel traversal", async () => { + const root = await createTestTempDirectory("rea-reference-parallel-abort-"); + const gitdir = join(root, ".git"); + await mkdir(gitdir); + const head = join(gitdir, "HEAD"); + await writeFile(head, "ref: refs/heads/main\n"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { caller: "cancel while Git metadata fails" }; + const physical = Object.assign(new Error("HEAD read EIO"), { code: "EIO" }); + let entered!: () => void; + let release!: () => void; + const opendirEntered = new Promise((resolve) => { + entered = resolve; + }); + const gate = new Promise((resolve) => { + release = resolve; + }); + opendirMock.mockImplementation(async (...args) => { + entered(); + await gate; + return actual.opendir(...args); + }); + openMock.mockImplementation(async (...args) => { + const handle = await actual.open(...args); + if (resolve(String(args[0])) === resolve(head)) + vi.spyOn(handle, "read").mockImplementation(async () => { + await opendirEntered; + controller.abort(reason); + release(); + throw physical; + }); + return handle; + }); + try { + const result = await importReferenceSource( + { + root, + caller: "test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + if (result.ok) throw new Error("Expected failure"); + expect(result.error.code).toBe("io"); + expect(result.error.secondaryCancellation?.cause).toBe(reason); + expect(((result.error.cause as Error).cause as Error).cause).toBe(physical); + } finally { + release(); + await resources.close(); + } +}); + +it.each(["metadata", "import"] as const)( + "preserves optional commondir absence when its probe coincides with cancellation (%s)", + async (surface) => { + const root = await createTestTempDirectory("rea-reference-optional-git-"); + const gitdir = join(root, ".git"); + await mkdir(gitdir); + await writeFile( + join(gitdir, "HEAD"), + "0123456789abcdef0123456789abcdef01234567\n", + ); + const commondir = join(gitdir, "commondir"); + const resources = new ArtifactResourceScope(); + const controller = new AbortController(); + const reason = { caller: "stop after optional metadata absence" }; + let probed = false; + openMock.mockImplementation(async (...args) => { + try { + return await actual.open(...args); + } catch (cause: unknown) { + if (resolve(String(args[0])) === resolve(commondir)) { + probed = true; + controller.abort(reason); + } + throw cause; + } + }); + try { + const result = + surface === "metadata" + ? await readReferenceSourceVcs(root, resources, controller.signal) + : await importReferenceSource( + { + root, + caller: "test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + expect(probed).toBe(true); + expect(controller.signal.reason).toBe(reason); + if (surface === "metadata") + expect(result).toEqual({ + ok: true, + value: { kind: "unknown", head: null, dirty: null }, + }); + else { + if (result.ok) throw new Error("Expected cancelled import"); + expect(result.error.code).toBe("cancelled"); + expect(result.error.cause).toBe(reason); + } + } finally { + await resources.close(); + } + }, +); diff --git a/tests/boundary/filesystem/referenceSourceReader.test.ts b/tests/boundary/filesystem/referenceSourceReader.test.ts index 2a457e44d9..be46d79487 100644 --- a/tests/boundary/filesystem/referenceSourceReader.test.ts +++ b/tests/boundary/filesystem/referenceSourceReader.test.ts @@ -10,6 +10,43 @@ import { readStableSymlinkTarget } from "../../../src/reference/ReferenceSourceR import { readReferenceSource } from "../../support/referenceSourceResourceScope.js"; import { ok } from "../../../src/domain/result.js"; +it.each(["rejected", "returned"] as const)( + "retains a symlink's physical failure when cancellation accompanies its %s outcome", + async (outcome) => { + const root = await createTestTempDirectory("rea-reference-link-failure-"); + const link = join(root, "alias"); + await symlink("target", link); + const expected = await lstat(link, { bigint: true }); + const controller = new AbortController(); + const physical = Object.assign(new Error("authored link EIO"), { + code: "EIO", + }); + const result = await readStableSymlinkTarget( + link, + "alias", + expected, + controller.signal, + async () => { + controller.abort("during link failure"); + if (outcome === "rejected") throw physical; + return { + status: "failed", + kind: "symlink", + path: "alias", + code: "io", + message: "authored link EIO", + }; + }, + ); + expect(result).toMatchObject({ + status: "failed", + kind: "symlink", + path: "alias", + code: "io", + }); + }, +); + it("retains the caller reason and observed entries when traversal is cancelled", async () => { const root = await createTestTempDirectory("rea-reference-cancel-"); await writeFile(join(root, "observed.js"), "observed");