fix(reference): preserve physical failures across late cancellation (#10101)

* fix(reference): preserve physical root admission failures

* fix(reference): retain physical Git read origins during cancellation

* fix(reference): preserve Git directory lookup failures

src/application/ReferenceSourceGitDirectory.ts

@@ -9,6 +9,13 @@ export interface ReferenceSourceGitDirectory {
   readonly commonDirectory: string;
 }
 
+/** A cancellation observed by the Git directory locator itself. */
+export class ReferenceSourceGitDirectoryCancelled extends Error {
+  constructor(readonly reason: unknown) {
+    super("Git directory lookup cancelled", { cause: reason });
+  }
+}
+
 /** Locate the nearest repository without running Git or following config includes. */
 export const locateReferenceSourceGitDirectory = async (
   root: string,
@@ -17,7 +24,8 @@ export const locateReferenceSourceGitDirectory = async (
 ): Promise<ReferenceSourceGitDirectory | undefined> => {
   let repository = root;
   for (;;) {
-    signal?.throwIfAborted();
+    if (signal?.aborted === true)
+      throw new ReferenceSourceGitDirectoryCancelled(signal.reason);
     const dotgit = join(repository, ".git");
     let metadata;
     try {

src/application/ReferenceSourceImportPolicy.ts

@@ -1,7 +1,10 @@
 import { lstat, realpath, stat } from "node:fs/promises";
 import fs from "node:fs";
 import { join } from "node:path";
-import { locateReferenceSourceGitDirectory } from "./ReferenceSourceGitDirectory.js";
+import {
+  locateReferenceSourceGitDirectory,
+  ReferenceSourceGitDirectoryCancelled,
+} from "./ReferenceSourceGitDirectory.js";
 import type { ReferenceSourceReaderError } from "../reference/ReferenceSourceReaderTypes.js";
 
 import ignore from "ignore";
@@ -106,11 +109,6 @@ const resolveRoot = async (
       });
     return ok(canonicalRoot);
   } catch (cause: unknown) {
-    if (signal?.aborted === true && cause === signal.reason)
-      return err({
-        ...failure("cancelled", "Reference source import cancelled"),
-        cause: signal.reason,
-      });
     const rootFailure = rootFilesystemFailure(
       cause,
       "Reference source root could not be resolved",
@@ -267,12 +265,14 @@ const readIgnoreCase = async (
     if (readFailure !== undefined) return err(readFailure);
     if (
       isIgnorePolicyCancellation(cause) ||
-      (signal?.aborted === true && cause === signal.reason)
+      cause instanceof ReferenceSourceGitDirectoryCancelled
     )
       return err({
         ...failure("cancelled", "Reference source import cancelled"),
         cause:
-          signal?.aborted === true ? signal.reason : ignorePolicyCause(cause),
+          cause instanceof ReferenceSourceGitDirectoryCancelled
+            ? cause.reason
+            : ignorePolicyCause(cause),
       });
     return err({
       ...failure(

src/application/ReferenceSourceImportTypes.test.ts

@@ -38,14 +38,20 @@ describe("reference-source import error projection", () => {
     }
   });
 
-  it("projects a later cancellation without replacing a physical failure", () => {
+  it("retains a physical failure and exact later cancellation before projection", () => {
     const reason = { caller: "stop after I/O failure" };
-    const projected = projectReferenceSourceImportError({
+    const physical = new Error("directory read failed");
+    const typed = referenceSourceImportAnalysisError({
       tag: "reference-source-import",
       code: "io",
       message: "directory read failed",
+      cause: physical,
       secondaryCancellation: { cause: reason },
     });
+    expect(typed.cause).toBe(physical);
+    expect(typed.secondaryFailures).toHaveLength(1);
+    expect(typed.secondaryFailures[0]?.cause).toBe(reason);
+    const projected = projectAnalysisError(typed);
     expect(projected.category).toBe("execution_failure");
     expect(projected.details?.secondary_failures).toEqual([
       expect.objectContaining({ category: "cancelled" }),

src/application/ReferenceSourceVcsAdapter.ts

@@ -1,6 +1,9 @@
 import fs from "node:fs";
 import { join } from "node:path";
-import { locateReferenceSourceGitDirectory } from "./ReferenceSourceGitDirectory.js";
+import {
+  locateReferenceSourceGitDirectory,
+  ReferenceSourceGitDirectoryCancelled,
+} from "./ReferenceSourceGitDirectory.js";
 import { err, ok, type Result } from "../domain/result.js";
 import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
 import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
@@ -72,9 +75,19 @@ const resolveSourceVcs = async (
   const cleanupFailures: ArtifactReaderFailure[] = [];
   const readMetadataBytes = async (path: string): Promise<Buffer> => {
     try {
-      return await readRegularFile(path, { signal });
+      return await readRegularFile(path, {
+        signal,
+        cancellationFailure: (reason) =>
+          new ArtifactReaderFailure(
+            "cancelled",
+            "Git metadata read cancelled",
+            {
+              cause: reason,
+            },
+          ),
+      });
     } catch (cause: unknown) {
-      if (signal?.aborted === true && cause === signal.reason) throw cause;
+      if (cause instanceof ArtifactReaderFailure) throw cause;
       if (!(cause instanceof RegularFileCleanupFailure)) {
         if (
           signal?.aborted === true &&
@@ -92,11 +105,15 @@ const resolveSourceVcs = async (
             { cause: physical },
           );
         }
-        throw cause;
+        throw cause instanceof RegularFileReadFailure
+          ? cause.operationCause
+          : cause;
       }
+      const cancellation = regularFileCancellation(cause);
       const retry = await retryRegularFileCleanup(cause, resources, signal);
       if (
         retry.outcome.kind === "failed" &&
+        cancellation === undefined &&
         errorCode(retry.outcome.cause) !== "ENOENT"
       )
         failedRead ??= { cause: retry.outcome.cause };
@@ -111,13 +128,10 @@ const resolveSourceVcs = async (
           ? retry.outcome.cause
           : cause.cleanupCause;
       throw new ArtifactReaderFailure(
-        regularFileCancellation(cause) !== undefined ||
-          (signal?.aborted === true && primary === signal.reason)
-          ? "cancelled"
-          : "io",
+        cancellation === undefined ? "io" : "cancelled",
         `Git metadata could not be read safely: ${errorMessage(primary)}`,
         {
-          cause: primary,
+          cause: cancellation === undefined ? primary : cancellation.reason,
           ...(retry.cleanup === undefined ? {} : { cleanup: retry.cleanup }),
         },
       );
@@ -130,9 +144,7 @@ const resolveSourceVcs = async (
       failure.cleanup === undefined ? [] : [failure.cleanup],
     );
     throw new ArtifactReaderFailure(
-      (primary instanceof ArtifactReaderFailure &&
-        primary.reason === "cancelled") ||
-        (signal?.aborted === true && primary === signal.reason)
+      primary instanceof ArtifactReaderFailure && primary.reason === "cancelled"
         ? "cancelled"
         : "io",
       `Git metadata could not be read safely: ${errorMessage(primary)}`,
@@ -161,9 +173,9 @@ const resolveSourceVcs = async (
     ...fs,
     promises: {
       ...fs.promises,
-      // Let isomorphic-git continue from a failed loose ref to packed-refs,
-      // then report the original read failure after resolution. This preserves
-      // both observations when packed metadata also fails cleanup.
+      // Let isomorphic-git finish probing refs, then report original failures.
+      // A later resolved ref must not discard an earlier physical read or
+      // retained cleanup failure.
       readFile: async (
         path: string | undefined,
         options:
@@ -209,22 +221,38 @@ const resolveSourceVcs = async (
       head = await resolveRef({ fs: referenceFs, dir: root, ...options });
     } catch (cause: unknown) {
       if (cleanupFailures.length > 0)
-        throwCleanupFailures(failedRead?.cause ?? cause);
+        throwCleanupFailures(failedRead?.cause ?? cleanupFailures[0] ?? cause);
       if (failedRead !== undefined) throw failedRead.cause;
       throw cause;
     }
     if (cleanupFailures.length > 0)
-      throwCleanupFailures(failedRead?.cause ?? cleanupFailures[0]?.cause);
+      throwCleanupFailures(failedRead?.cause ?? cleanupFailures[0]);
     signal?.throwIfAborted();
     if (failedRead !== undefined) throw failedRead.cause;
     return head;
   };
-  const locations = await locateReferenceSourceGitDirectory(
-    root,
-    readMetadataText,
-    signal,
-  );
+  let locations;
+  try {
+    locations = await locateReferenceSourceGitDirectory(
+      root,
+      readMetadataText,
+      signal,
+    );
+  } catch (cause: unknown) {
+    if (
+      signal?.aborted === true &&
+      !(cause instanceof ReferenceSourceGitDirectoryCancelled) &&
+      !(cause instanceof ArtifactReaderFailure)
+    )
+      throw new ArtifactReaderFailure(
+        "io",
+        `Git directory could not be resolved: ${errorMessage(cause)}`,
+        { cause },
+      );
+    throw cause;
+  }
   if (locations === undefined) return { kind: "none", head: null, dirty: null };
+  if (isAborted(signal)) return unknownVcs();
   const { gitDirectory, commonDirectory } = locations;
   if (gitDirectory === commonDirectory) {
     const head = await readRef({ gitdir: gitDirectory, ref: "HEAD" });

tests/boundary/filesystem/referenceSourceMetadataCleanup.test.ts

@@ -1,11 +1,15 @@
 import {
   mkdir,
+  lstat as fsLstat,
   open as fsOpen,
   opendir as fsOpendir,
+  realpath as fsRealpath,
   stat as fsStat,
   writeFile,
+  type FileHandle,
 } from "node:fs/promises";
 import { join, resolve } from "node:path";
+import { constants } from "node:fs";
 
 import { afterEach, beforeEach, expect, it, onTestFinished, vi } from "vitest";
 
@@ -19,15 +23,19 @@ vi.mock("node:fs/promises", async (importOriginal) => {
   const actual = await importOriginal<typeof import("node:fs/promises")>();
   return {
     ...actual,
+    lstat: vi.fn(actual.lstat),
     open: vi.fn(actual.open),
     stat: vi.fn(actual.stat),
+    realpath: vi.fn(actual.realpath),
     opendir: vi.fn(actual.opendir),
   };
 });
 
+const lstatMock = vi.mocked(fsLstat);
 const openMock = vi.mocked(fsOpen);
 const opendirMock = vi.mocked(fsOpendir);
 const statMock = vi.mocked(fsStat);
+const realpathMock = vi.mocked(fsRealpath);
 let actual: typeof import("node:fs/promises");
 
 beforeEach(async () => {
@@ -35,15 +43,19 @@ beforeEach(async () => {
     await vi.importActual<typeof import("node:fs/promises")>(
       "node:fs/promises",
     );
+  lstatMock.mockImplementation((...args) => actual.lstat(...args));
   openMock.mockImplementation((...args) => actual.open(...args));
   opendirMock.mockImplementation((...args) => actual.opendir(...args));
   statMock.mockImplementation((...args) => actual.stat(...args));
+  realpathMock.mockImplementation((...args) => actual.realpath(...args));
 });
 
 afterEach(() => {
+  lstatMock.mockReset();
   openMock.mockReset();
   opendirMock.mockReset();
   statMock.mockReset();
+  realpathMock.mockReset();
 });
 
 const failCloseUntilAllowed = (
@@ -335,45 +347,65 @@ it("keeps a later packed-ref read error primary over earlier loose-ref cleanup",
   );
 });
 
-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 },
-      },
+it.each([
+  ["stat", false],
+  ["stat", true],
+  ["realpath", false],
+  ["realpath", true],
+] as const)(
+  "preserves a physical root %s failure when cancellation arrives (same reason: %s)",
+  async (stage, sameReason) => {
+    const root = await createTestTempDirectory("rea-reference-root-eio-");
+    const resources = new ArtifactResourceScope();
+    const controller = new AbortController();
+    const physical = Object.assign(new Error(`root ${stage} failed`), {
+      code: "EIO",
     });
-  } finally {
-    await resources.close();
-  }
-});
+    const reason = sameReason ? physical : { source: "caller" };
+    const reject = (): never => {
+      controller.abort(reason);
+      throw physical;
+    };
+    if (stage === "stat") {
+      statMock.mockImplementation(async (...args) => {
+        if (String(args[0]) === root) return reject();
+        return actual.stat(...args);
+      });
+    } else {
+      realpathMock.mockImplementation(async (...args) => {
+        if (String(args[0]) === root) return reject();
+        return actual.realpath(...args);
+      });
+    }
+    try {
+      const result = await importReferenceSource(
+        {
+          root,
+          caller: "test",
+          policy: { secretPatterns: [] },
+          signal: controller.signal,
+        },
+        resources,
+      );
+      if (result.ok)
+        throw new Error("Expected physical root admission failure");
+      expect(result.error.code).toBe("io");
+      expect(result.error.cause).toBe(physical);
+      expect(result.error.secondaryCancellation?.cause).toBe(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) => {
+it.each([
+  { closeFails: false, sameReason: false },
+  { closeFails: false, sameReason: true },
+  { closeFails: true, sameReason: false },
+  { closeFails: true, sameReason: true },
+])(
+  "retains physical Git read failure through late cancellation (close fails: $closeFails, same reason: $sameReason)",
+  async ({ closeFails, sameReason }) => {
     const root = await createTestTempDirectory("rea-reference-vcs-eio-");
     const gitdir = join(root, ".git");
     await mkdir(gitdir);
@@ -382,19 +414,34 @@ it.each([false, true])(
     const resources = new ArtifactResourceScope();
     const controller = new AbortController();
     const reason = { source: "caller" };
-    const physical = Object.assign(new Error("metadata read failed"), {
-      code: "EIO",
-    });
+    let physical: unknown;
+    const handles: FileHandle[] = [];
     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;
+      if (resolve(String(args[0])) !== resolve(head))
+        return actual.open(...args);
+      const flags = args[1];
+      if (typeof flags !== "number")
+        throw new Error("Expected numeric regular-file admission flags");
+      // Preserve admission flags; neither O_TRUNC nor a write is used.
+      const handle = await actual.open(
+        args[0],
+        flags | constants.O_WRONLY,
+        args[2],
+      );
+      handles.push(handle);
+      const read = handle.read.bind(handle);
+      vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => {
+        try {
+          return await read(...readArgs);
+        } catch (cause: unknown) {
+          physical = cause;
+          throw cause;
+        }
       });
       const close = handle.close.bind(handle);
       vi.spyOn(handle, "close").mockImplementation(async () => {
-        controller.abort(reason);
+        controller.abort(sameReason ? physical : reason);
         if (closeFails && !allowClose) throw new Error("close failed");
         await close();
       });
@@ -406,14 +453,37 @@ it.each([false, true])(
         resources,
         controller.signal,
       );
+      expect(physical).toMatchObject({ code: "EBADF" });
+      const cancellation = sameReason ? physical : reason;
+      expect(controller.signal.reason).toBe(cancellation);
       expect(result).toMatchObject({
         ok: false,
         error: {
           code: "io",
-          secondaryCancellation: { cause: reason },
+          secondaryCancellation: { cause: cancellation },
           ...(closeFails ? { cleanup: { resources: [head] } } : {}),
         },
       });
+      if (result.ok) throw new Error("Physical metadata failure was ignored");
+      expect(result.error.secondaryCancellation?.cause).toBe(cancellation);
+      expect(resources.cleanupResources).toEqual(closeFails ? [head] : []);
+      let retained: unknown = result.error.cause;
+      while (
+        retained !== physical &&
+        retained instanceof Error &&
+        retained.cause !== undefined
+      )
+        retained = retained.cause;
+      expect(retained).toBe(physical);
+      expect(
+        handles.every((handle) =>
+          closeFails ? handle.fd >= 0 : handle.fd === -1,
+        ),
+      ).toBe(true);
+      allowClose = true;
+      await resources.close();
+      expect(handles.every((handle) => handle.fd === -1)).toBe(true);
+      expect(resources.hasPending).toBe(false);
     } finally {
       allowClose = true;
       await resources.close();
@@ -584,3 +654,124 @@ it.each(["metadata", "import"] as const)(
     }
   },
 );
+
+it.each([
+  ["lstat", false],
+  ["lstat", true],
+  ["stat", false],
+  ["stat", true],
+  ["realpath", false],
+  ["realpath", true],
+] as const)(
+  "retains physical git directory %s with late abort (same reason: %s)",
+  async (stage, sameReason) => {
+    const root = await createTestTempDirectory("rea-git-physical-");
+    await mkdir(join(root, ".git"));
+    const resources = new ArtifactResourceScope();
+    const controller = new AbortController();
+    const physical = Object.assign(new Error("git lookup failed"), {
+      code: "EIO",
+    });
+    const reason = sameReason ? physical : { caller: "stopped" };
+    const reject = (): never => {
+      controller.abort(reason);
+      throw physical;
+    };
+    if (stage === "lstat")
+      lstatMock.mockImplementation(async (...args) => {
+        if (resolve(String(args[0])) === resolve(root, ".git")) return reject();
+        return actual.lstat(...args);
+      });
+    else if (stage === "stat")
+      statMock.mockImplementation(async (...args) => {
+        if (resolve(String(args[0])) === resolve(root, ".git")) return reject();
+        return actual.stat(...args);
+      });
+    else
+      realpathMock.mockImplementation(async (...args) => {
+        if (resolve(String(args[0])) === resolve(root, ".git")) return reject();
+        return actual.realpath(...args);
+      });
+    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("preserves real cancellation between Git directory lookup checkpoints", async () => {
+  const root = await createTestTempDirectory("rea-git-checkpoint-");
+  const resources = new ArtifactResourceScope();
+  const controller = new AbortController();
+  const reason = { caller: "stopped" };
+  lstatMock.mockImplementation(async (...args) => {
+    if (resolve(String(args[0])) === resolve(root, ".git")) {
+      controller.abort(reason);
+      throw Object.assign(new Error("absent"), { code: "ENOENT" });
+    }
+    return actual.lstat(...args);
+  });
+  try {
+    const result = await importReferenceSource(
+      {
+        root,
+        caller: "test",
+        policy: { secretPatterns: [] },
+        signal: controller.signal,
+      },
+      resources,
+    );
+    expect(result).toMatchObject({
+      ok: false,
+      error: { code: "cancelled", cause: reason },
+    });
+  } finally {
+    await resources.close();
+  }
+});
+
+it("retains physical Git discovery lstat failure with late abort through VCS", async () => {
+  const root = await createTestTempDirectory("rea-git-vcs-physical-");
+  const resources = new ArtifactResourceScope();
+  const controller = new AbortController();
+  const physical = Object.assign(new Error("git discovery failed"), {
+    code: "EIO",
+  });
+  lstatMock.mockImplementation(async (...args) => {
+    if (resolve(String(args[0])) === resolve(root, ".git")) {
+      controller.abort(physical);
+      throw physical;
+    }
+    return actual.lstat(...args);
+  });
+  try {
+    const result = await readReferenceSourceVcs(
+      root,
+      resources,
+      controller.signal,
+    );
+    expect(result).toMatchObject({
+      ok: false,
+      error: { code: "io", secondaryCancellation: { cause: physical } },
+    });
+  } finally {
+    await resources.close();
+  }
+});