fix(bundler): keep the failure that caused the ownership error

extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts

@@ -6,10 +6,15 @@
  */
 
 import { assertEquals, assertExists, assertRejects, assertStringIncludes } from "@std/assert";
-import { describe, it } from "@std/testing/bdd";
+import { afterEach, describe, it } from "#veryfront/testing/bdd.ts";
 import { createRequire } from "node:module";
 
-import { EsbuildBundler, isLiveEsbuildServiceProcess } from "./esbuild-bundler.ts";
+import {
+  __recordOwnershipErrorForTests,
+  __resetOwnershipErrorForTests,
+  EsbuildBundler,
+  isLiveEsbuildServiceProcess,
+} from "./esbuild-bundler.ts";
 import { rebuildContextWithSignal } from "./context-build-lifecycle.ts";
 
 const childProcess = createRequire(import.meta.url)("node:child_process") as {
@@ -811,6 +816,74 @@ describe("EsbuildBundler.bundle", () => {
   });
 });
 
+describe("ownership error cause", () => {
+  afterEach(() => {
+    __resetOwnershipErrorForTests();
+  });
+
+  it("adopts the underlying failure when the latch was set before it surfaced", () => {
+    // The latch is created before the operation settles, so it starts without a
+    // cause. Discarding the cause that arrives afterwards is what made every
+    // real esbuild failure surface as a lifecycle problem instead.
+    __resetOwnershipErrorForTests();
+    __recordOwnershipErrorForTests();
+    const error = __recordOwnershipErrorForTests(new Error("spawn ENOENT esbuild"));
+
+    assertStringIncludes(error.message, "module-wide adapter");
+    assertStringIncludes(error.message, "spawn ENOENT esbuild");
+    assertEquals((error.cause as Error).message, "spawn ENOENT esbuild");
+  });
+
+  it("reports a cause supplied on the first record", () => {
+    __resetOwnershipErrorForTests();
+    const error = __recordOwnershipErrorForTests(new Error("binary missing"));
+
+    assertStringIncludes(error.message, "binary missing");
+    assertEquals((error.cause as Error).message, "binary missing");
+  });
+
+  it("keeps the first cause rather than overwriting it", () => {
+    __resetOwnershipErrorForTests();
+    __recordOwnershipErrorForTests(new Error("first failure"));
+    const error = __recordOwnershipErrorForTests(new Error("second failure"));
+
+    assertEquals((error.cause as Error).message, "first failure");
+    assertStringIncludes(error.message, "first failure");
+  });
+
+  it("keeps the filesystem layout out of the reported cause", () => {
+    // A compiled runtime resolves esbuild under a temp directory, and spawn
+    // errors quote that path. The message is logged, so it must carry the
+    // failure without the machine's layout.
+    __resetOwnershipErrorForTests();
+    const error = __recordOwnershipErrorForTests(
+      new Error("spawn /tmp/veryfront-esbuild-0.28.1-c3fd/esbuild ENOENT"),
+    );
+
+    assertStringIncludes(error.message, "spawn esbuild ENOENT");
+    assertEquals(error.message.includes("/tmp/"), false);
+  });
+
+  it("keeps a stack out of the reported cause", () => {
+    __resetOwnershipErrorForTests();
+    const error = __recordOwnershipErrorForTests(
+      new Error("boom\n    at Object.create (file:///tmp/deno-compile/src/errors/types.ts:111:14)"),
+    );
+
+    assertStringIncludes(error.message, "boom");
+    assertEquals(error.message.includes("types.ts"), false);
+    assertEquals(error.message.includes("    at "), false);
+  });
+
+  it("stays usable when there is no underlying failure", () => {
+    __resetOwnershipErrorForTests();
+    const error = __recordOwnershipErrorForTests();
+
+    assertStringIncludes(error.message, "module-wide adapter");
+    assertEquals(error.cause, undefined);
+  });
+});
+
 describe("EsbuildBundler unsupported lifecycle ownership", () => {
   it("rejects shutdown after a raw service generation replaces the managed one", async () => {
     const observation = observeEsbuildServices();

extensions/ext-bundler-esbuild/src/esbuild-bundler.ts

@@ -63,11 +63,60 @@ let activeOperationsIdle: Promise<void> = Promise.resolve();
 let resolveActiveOperationsIdle: (() => void) | null = null;
 const operationScopes = new AsyncLocalStorage<OperationScope>();
 
+const OWNERSHIP_ERROR_MESSAGE =
+  "[ext-bundler-esbuild] Cannot own an esbuild service started outside the module-wide adapter; restart the process and use only the Bundler contract";
+
+const MAX_CAUSE_DETAIL_LENGTH = 200;
+/** Absolute POSIX and Windows paths, reduced to a basename below. */
+const ABSOLUTE_PATH_PATTERN = /(?:[A-Za-z]:)?(?:\/|\\\\)[^\s"']*/g;
+
+/**
+ * Reduce a cause to a single redacted line.
+ *
+ * The message is logged, so it must not carry a machine's filesystem layout:
+ * a compiled runtime resolves esbuild under a temp directory, and spawn errors
+ * quote that path verbatim. Keeping only the basename preserves what the reader
+ * needs -- which binary or module failed -- without the surrounding layout. The
+ * first line only, so a stack never reaches the message, and bounded so a large
+ * esbuild diagnostic cannot dominate the log line.
+ */
+function describeCause(cause: unknown): string {
+  if (cause === undefined) return "";
+  const raw = cause instanceof Error ? cause.message : String(cause);
+  const firstLine = raw.split("\n", 1)[0] ?? "";
+  const withoutPaths = firstLine.replace(ABSOLUTE_PATH_PATTERN, (match) => {
+    const parts = match.split(/[\/\\]/).filter(Boolean);
+    return parts.length > 0 ? parts[parts.length - 1]! : match;
+  });
+  const detail = withoutPaths.trim().slice(0, MAX_CAUSE_DETAIL_LENGTH);
+  return detail ? ` (underlying failure: ${detail})` : "";
+}
+
+/**
+ * Latch the ownership failure, keeping whatever really went wrong.
+ *
+ * The latch is permanent, so the first call decides the error every later
+ * operation sees. That call is often made before the operation settles, when no
+ * cause is known yet; adopting the cause afterwards is what keeps the real
+ * failure visible instead of reporting a lifecycle problem that may not be the
+ * actual one. The cause is folded into the message because callers log
+ * `error.message` and would otherwise never print the chain.
+ */
 function recordOwnershipError(cause?: unknown): Error {
-  const message =
-    "[ext-bundler-esbuild] Cannot own an esbuild service started outside the module-wide adapter; restart the process and use only the Bundler contract";
-  esbuildOwnershipError ??= new Error(message, cause === undefined ? undefined : { cause });
-  return esbuildOwnershipError;
+  const existing = esbuildOwnershipError;
+  if (!existing) {
+    esbuildOwnershipError = new Error(
+      `${OWNERSHIP_ERROR_MESSAGE}${describeCause(cause)}`,
+      cause === undefined ? undefined : { cause },
+    );
+    return esbuildOwnershipError;
+  }
+
+  if (cause !== undefined && existing.cause === undefined) {
+    existing.cause = cause;
+    existing.message = `${OWNERSHIP_ERROR_MESSAGE}${describeCause(cause)}`;
+  }
+  return existing;
 }
 
 async function getEsbuild(): Promise<EsbuildModule> {
@@ -220,6 +269,16 @@ function isEsbuildServiceSpawn(spawnArgs: unknown[]): boolean {
  * Native Node represents an active child with `null` exit fields. Some
  * compatible runtimes leave those fields undefined until the child exits.
  */
+/** The ownership latch is module-wide; tests must clear it between cases. */
+export function __resetOwnershipErrorForTests(): void {
+  esbuildOwnershipError = null;
+}
+
+/** Exercise the latch without starting a real esbuild service. */
+export function __recordOwnershipErrorForTests(cause?: unknown): Error {
+  return recordOwnershipError(cause);
+}
+
 export function isLiveEsbuildServiceProcess(
   child: Pick<ChildProcess, "killed" | "exitCode" | "signalCode">,
 ): boolean {
@@ -270,10 +329,15 @@ function invokeEsbuild<T extends Promise<unknown>>(operation: () => T): T {
 
   const ownedService = capturedService ?? esbuildService;
   if (!ownedService || !isLiveService(ownedService)) {
-    const ownershipError = recordOwnershipError();
+    // Latch synchronously so a concurrent operation cannot pass the admission
+    // check in runBundlerOperation and drive esbuild while ownership is already
+    // known to be invalid. The rejection handler still supplies the cause: the
+    // latch is created without one here, and recordOwnershipError adopts the
+    // first cause offered afterwards.
+    recordOwnershipError();
     return result.then(
       () => {
-        throw ownershipError;
+        throw recordOwnershipError();
       },
       (cause) => {
         throw recordOwnershipError(cause);