fix(pcap): report workspace allocation failures by their system cause (#6229)

* fix(pcap): report workspace allocation failures by their system cause Pcap workspace allocation and snapshot writes mapped every filesystem error, including ENOTDIR and EACCES from mkdtemp, to a file-size resource constraint with null limits and worker file-size advice. Flutter's owned snapshot storage had the same errno-blind mapping, while TShark and sigrok carried duplicate copies of the correct split. Move the private-workspace failure projection into one shared owner: ENOSPC/EDQUOT/EFBIG stay file-size resource constraints with their system code and path; descriptor exhaustion and every other failure become provider workspace failures carrying stage, workspace path, failing path, system code and cause-specific recovery. Fixes #6123 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(flutter): locate owned storage failures by their workspace mapOwnedIoFailure always passed a null workspace path, so a descriptor write or read failure without its own errno path lost every location. Thread the owned root through: allocation keeps null (no workspace yet), snapshot failures use the allocated root, and package-reader system errors propagate to the request owner, which maps them against the snapshot root instead of mapping early without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-25 22:08:11 -04:00 · 2026-10-21 21:19:18 -07:00
parent ec0e663cca
commit b4f9f0deac
8 changed files with 430 additions and 79 deletions
@@ -60,6 +60,7 @@ import {
  OwnedCommandFailure,
 } from "../process/OwnedCommand.js";
 import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
+import { privateWorkspaceFailure } from "../process/privateWorkspaceFailure.js";

 const OPERATION = "inspect_packet_capture";
 export const TSHARK_PROVIDER_IDENTITY = {
@@ -498,32 +499,13 @@ export class TsharkCaptureProvider implements PacketCapturePort {
        effectiveLimits,
        operation,
      );
-    if (phase === "workspace") {
-      const code =
-        cause instanceof Error &&
-        "code" in cause &&
-        typeof cause.code === "string"
-          ? cause.code
-          : null;
-      if (code === "ENOSPC" || code === "EDQUOT" || code === "EFBIG")
-        return new AnalysisResourceConstraintError(
-          operation,
-          "file-size",
-          `Private analysis workspace failed (${code}): ${reason}`,
-          { workspace_path: workspacePath, system_code: code },
-          { cause },
-        );
-      return new ProviderAdapterError(TSHARK_PROVIDER_IDENTITY.id, operation, {
+    if (phase === "workspace")
+      return privateWorkspaceFailure(
+        TSHARK_PROVIDER_IDENTITY.id,
+        operation,
        cause,
-        userMessage: `Cannot prepare the private analysis workspace: ${reason}`,
-        diagnostics: {
-          stage: "workspace",
-          workspace_path: workspacePath,
-          system_code: code,
-          reason,
-        },
-      });
-    }
+        workspacePath,
+      );
    if (phase === "integrity") {
      const detail = {
        cause,
@@ -57,6 +57,7 @@ import {
  readSnapshotHeader,
 } from "./DartElf.js";
 import { readElfBuildIds } from "./ElfBuildId.js";
+import { privateWorkspaceFailure } from "../process/privateWorkspaceFailure.js";

 /** Stable identity for the Flutter build-identification provider. */
 export const FLUTTER_PROVIDER_IDENTITY = {
@@ -221,10 +222,15 @@ const inputFailureMessage = (path: string, cause: unknown): string => {
  return `The APK could not be read as a package: ${path}: ${message}`;
 };

-/** Provider-owned storage failures must never be attributed to caller input. */
+/**
+ * Provider-owned storage failures must never be attributed to caller input.
+ * `workspacePath` is the owned root once allocated, so a failure without its
+ * own errno path still names the workspace it occurred in.
+ */
 const mapOwnedIoFailure = (
  operation: string,
  cause: unknown,
+  workspacePath: string | null,
 ): AnalysisError => {
  if (cause instanceof AnalysisError) return cause;
  const cancellation = regularFileCancellation(cause);
@@ -239,24 +245,27 @@ const mapOwnedIoFailure = (
    "code" in cause &&
    typeof cause.code === "string"
  )
-    return new AnalysisResourceConstraintError(
+    return privateWorkspaceFailure(
+      FLUTTER_PROVIDER_IDENTITY.id,
      operation,
-      "file-size",
-      `Provider-owned Flutter snapshot storage failed (${cause.code}): ${cause.message}`,
-      null,
-      { cause },
+      cause,
+      workspacePath,
    );
  return new AnalysisProtocolError(operation, { cause });
 };

-/** ZIP format failures describe caller bytes; its descriptor is provider-owned. */
+/**
+ * ZIP format failures describe caller bytes. A system error comes from the
+ * provider-owned snapshot descriptor, so it propagates unchanged to the
+ * snapshot owner, which maps it against the owned workspace.
+ */
 const mapPackageFailure = (
  operation: string,
  path: string,
  cause: unknown,
-): AnalysisError =>
+): unknown =>
  cause instanceof Error && "code" in cause && typeof cause.code === "string"
-    ? mapOwnedIoFailure(operation, cause)
+    ? cause
    : mapInputFailure(operation, path, cause);

 interface FlutterLibrary {
@@ -382,7 +391,8 @@ export class FlutterBuildProvider {
    try {
      root = await mkdtemp(join(tmpdir(), "rea-flutter-"));
    } catch (cause) {
-      throw mapOwnedIoFailure(operation, cause);
+      // No workspace exists yet; the errno path names the attempted template.
+      throw mapOwnedIoFailure(operation, cause, null);
    }
    this.ownedRoots.set(root, new Set());
    const snapshotPath = join(root, "input.apk");
@@ -482,7 +492,7 @@ export class FlutterBuildProvider {
      await this.discardRoot(root);
      if (signal?.aborted === true)
        throw new AnalysisCancelledError(operation, { cause });
-      throw mapOwnedIoFailure(operation, cause);
+      throw mapOwnedIoFailure(operation, cause, root);
    }
  }

@@ -630,7 +640,9 @@ export class FlutterBuildProvider {
              libraries,
            );
    } catch (cause) {
-      result = err(mapOwnedIoFailure(request.operation, cause));
+      result = err(
+        mapOwnedIoFailure(request.operation, cause, snapshot?.root ?? null),
+      );
    } finally {
      if (snapshot !== undefined) await this.discardRoot(snapshot.root);
    }
@@ -22,6 +22,7 @@ import {
 } from "../filesystem/RegularFile.js";
 import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
 import { PrivateRuntimeRootUnavailableError } from "../process/PrivateRuntimeRootUnavailableError.js";
+import { privateWorkspaceFailure } from "../process/privateWorkspaceFailure.js";

 const OPERATION = "inspect_pcap_capture";
 const MAX_CAPTURE_BYTES = 2 * 1024 ** 3;
@@ -53,13 +54,7 @@ export class PcapSnapshot {
          cause.message,
          { cause, retryable: cause.retryable },
        );
-      throw new AnalysisResourceConstraintError(
-        OPERATION,
-        "file-size",
-        "Cannot allocate the owned capture workspace",
-        null,
-        { cause },
-      );
+      throw privateWorkspaceFailure("pcap", OPERATION, cause, null);
    }
  }

@@ -237,13 +232,7 @@ export class PcapSnapshot {
          cause: cancellation.reason,
        });
      if (cause instanceof AnalysisError) throw cause;
-      throw new AnalysisResourceConstraintError(
-        OPERATION,
-        "file-size",
-        "Owned capture snapshot I/O failed",
-        null,
-        { cause },
-      );
+      throw privateWorkspaceFailure("pcap", OPERATION, primary, this.root.path);
    }
  }

@@ -0,0 +1,56 @@
+import { expect, it } from "vitest";
+
+import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
+import { privateWorkspaceFailure } from "./privateWorkspaceFailure.js";
+
+const failure = (code: string | undefined, path?: string) =>
+  Object.assign(new Error(`${code ?? "plain"} failure`), {
+    ...(code === undefined ? {} : { code }),
+    ...(path === undefined ? {} : { path }),
+  });
+
+it("keeps EFBIG as a file-size refusal with the worker file-size recovery", () => {
+  const projection = projectAnalysisError(
+    privateWorkspaceFailure("p", "op", failure("EFBIG", "/w/f"), "/w"),
+  );
+  expect(projection).toMatchObject({
+    code: "resource_constraint",
+    details: {
+      resource: "file-size",
+      reported_limits: {
+        workspace_path: "/w",
+        path: "/w/f",
+        system_code: "EFBIG",
+      },
+    },
+    remediation: {
+      action: expect.stringContaining("worker file-size") as unknown,
+    },
+  });
+});
+
+it.each([
+  ["errno without a path", failure("EIO"), "EIO", "/w"],
+  ["error without errno", failure(undefined), null, "/w"],
+  ["non-Error value", "unexpected", null, "/w"],
+])(
+  "falls back to the workspace coordinate for %s",
+  (_name, cause, code, path) => {
+    const projection = projectAnalysisError(
+      privateWorkspaceFailure("p", "op", cause, "/w"),
+    );
+    expect(projection).toMatchObject({
+      code: "execution_failure",
+      details: {
+        provider_id: "p",
+        diagnostics: {
+          stage: "workspace",
+          workspace_path: "/w",
+          path,
+          system_code: code,
+        },
+      },
+    });
+    expect(projection.message).not.toContain("()");
+  },
+);
@@ -0,0 +1,77 @@
+import type { AnalysisError } from "../domain/analysisErrorBase.js";
+import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js";
+import { ProviderAdapterError } from "../domain/providerAdapterError.js";
+
+/** Host storage exhaustion; only these errors establish a storage constraint. */
+const STORAGE_EXHAUSTION_CODES: ReadonlySet<string> = new Set([
+  "ENOSPC",
+  "EDQUOT",
+]);
+
+/** Host or process descriptor exhaustion; the workspace location is usable. */
+const DESCRIPTOR_EXHAUSTION_CODES: ReadonlySet<string> = new Set([
+  "EMFILE",
+  "ENFILE",
+]);
+
+const systemCode = (cause: unknown): string | null =>
+  cause instanceof Error && "code" in cause && typeof cause.code === "string"
+    ? cause.code
+    : null;
+
+const systemPath = (cause: unknown): string | null =>
+  cause instanceof Error && "path" in cause && typeof cause.path === "string"
+    ? cause.path
+    : null;
+
+/**
+ * Project a filesystem failure while allocating or writing a provider-owned
+ * private analysis workspace.
+ *
+ * Storage exhaustion and file-size refusal are resource constraints. Every
+ * other failure, such as a denied, read-only, missing or non-directory
+ * temporary parent, is an unusable workspace location: it keeps its system
+ * code and failing path instead of being reported as an exceeded allowance.
+ */
+export const privateWorkspaceFailure = (
+  providerId: string,
+  operation: string,
+  cause: unknown,
+  workspacePath: string | null,
+): AnalysisError => {
+  const code = systemCode(cause);
+  const path = systemPath(cause) ?? workspacePath;
+  const reason = cause instanceof Error ? cause.message : String(cause);
+  if (code !== null && (STORAGE_EXHAUSTION_CODES.has(code) || code === "EFBIG"))
+    return new AnalysisResourceConstraintError(
+      operation,
+      "file-size",
+      `Private analysis workspace failed (${code}): ${reason}`,
+      { workspace_path: workspacePath, path, system_code: code },
+      {
+        cause,
+        // EFBIG is a file-size refusal, so the worker file-size recovery applies.
+        ...(STORAGE_EXHAUSTION_CODES.has(code)
+          ? {
+              remediationAction:
+                "Free disk space or quota on the filesystem holding details.reported_limits.path (the host temporary directory, TMPDIR), then retry.",
+            }
+          : {}),
+      },
+    );
+  return new ProviderAdapterError(providerId, operation, {
+    cause,
+    userMessage: `Cannot prepare the private analysis workspace${code === null ? "" : ` (${code})`}: ${reason}`,
+    remediationAction:
+      code !== null && DESCRIPTOR_EXHAUSTION_CODES.has(code)
+        ? "Close unused files or raise the open-file limit of the REA process, then retry."
+        : "Review details.diagnostics.system_code and details.diagnostics.path. Make the host temporary directory (TMPDIR) an existing directory this user can create files in on a writable filesystem, then retry.",
+    diagnostics: {
+      stage: "workspace",
+      workspace_path: workspacePath,
+      path,
+      system_code: code,
+      reason,
+    },
+  });
+};
@@ -63,6 +63,7 @@ import {
  OwnedCommandFailure,
 } from "../process/OwnedCommand.js";
 import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
+import { privateWorkspaceFailure } from "../process/privateWorkspaceFailure.js";

 const OPERATION = "decode_signal_recording";
 export const SIGROK_PROVIDER_IDENTITY = {
@@ -475,32 +476,13 @@ export class SigrokRecordingProvider implements SignalRecordingPort {
        effectiveLimits,
        operation,
      );
-    if (phase === "workspace") {
-      const code =
-        cause instanceof Error &&
-        "code" in cause &&
-        typeof cause.code === "string"
-          ? cause.code
-          : null;
-      if (code === "ENOSPC" || code === "EDQUOT" || code === "EFBIG")
-        return new AnalysisResourceConstraintError(
-          operation,
-          "file-size",
-          `Private analysis workspace failed (${code}): ${reason}`,
-          { workspace_path: workspacePath, system_code: code },
-          { cause },
-        );
-      return new ProviderAdapterError(SIGROK_PROVIDER_IDENTITY.id, operation, {
+    if (phase === "workspace")
+      return privateWorkspaceFailure(
+        SIGROK_PROVIDER_IDENTITY.id,
+        operation,
        cause,
-        userMessage: `Cannot prepare the private analysis workspace: ${reason}`,
-        diagnostics: {
-          stage: "workspace",
-          workspace_path: workspacePath,
-          system_code: code,
-          reason,
-        },
-      });
-    }
+        workspacePath,
+      );
    if (phase === "integrity") {
      const detail = {
        cause,
@@ -316,10 +316,33 @@ it.each([
      input: { path },
    });
    if (result.ok) throw new Error("expected owned I/O failure");
-    expect(projectAnalysisError(result.error).code).toBe("resource_constraint");
    expect(result.error.cause).toBe(cause);
-    expect(result.error.message).toContain(
-      "Provider-owned Flutter snapshot storage",
+    // Only storage exhaustion is a resource constraint; other owned-storage
+    // failures keep their system code instead of claiming a size allowance.
+    // These causes carry no errno path, so the failure is located by the owned
+    // workspace once allocated; before allocation no workspace exists.
+    const workspace =
+      stage === "root"
+        ? null
+        : (expect.stringMatching(/\/rea-flutter-[^/]+$/u) as unknown);
+    const location = { workspace_path: workspace, path: workspace };
+    expect(projectAnalysisError(result.error)).toMatchObject(
+      code === "ENOSPC"
+        ? {
+            code: "resource_constraint",
+            details: { reported_limits: { ...location, system_code: code } },
+          }
+        : {
+            code: "execution_failure",
+            details: {
+              provider_id: "flutter",
+              diagnostics: {
+                stage: "workspace",
+                ...location,
+                system_code: code,
+              },
+            },
+          },
    );
  },
 );
@@ -0,0 +1,230 @@
+import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
+import { chmod, mkdir, open, readdir, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { expect, onTestFinished, test, vi } from "vitest";
+
+import * as nativeBound from "../../../src/pcap/PcapNativeBound.js";
+import { PcapProvider } from "../../../src/pcap/PcapProvider.js";
+import {
+  PrivateRuntimeRoot,
+  type PrivateRuntimeRootOptions,
+} from "../../../src/process/PrivateRuntimeRoot.js";
+import { createServer } from "../../../src/server/createServer.js";
+import { createTestBinarySession } from "../../fixtures/binarySession.js";
+import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+// POSIX fixtures use real mkdtemp under caller-controlled temporary parents.
+const it = test.skipIf(process.platform === "win32");
+const nonRoot = process.getuid?.() !== 0;
+const stubs = join(import.meta.dirname, "../../fixtures/pcap");
+
+type Allocate = (
+  options: PrivateRuntimeRootOptions,
+  allocate: (options: PrivateRuntimeRootOptions) => Promise<PrivateRuntimeRoot>,
+) => Promise<PrivateRuntimeRoot>;
+
+/** Inspect a capture through MCP while only the pcap workspace allocation is redirected. */
+const inspectWith = async (allocate: Allocate, path: string) => {
+  const create = PrivateRuntimeRoot.create.bind(PrivateRuntimeRoot);
+  vi.spyOn(PrivateRuntimeRoot, "create").mockImplementation((options = {}) =>
+    options.prefix === "rea-pcap-"
+      ? allocate(options, create)
+      : create(options),
+  );
+  // macOS has no native capture bound; admit it so allocation is reached.
+  vi.spyOn(nativeBound, "preparePcapNativeBound").mockResolvedValue({
+    nativeBound: {},
+    limitations: ["Native resource admission simulated by this test"],
+  });
+  onTestFinished(() => {
+    vi.restoreAllMocks();
+  });
+  const provider = new PcapProvider({
+    environment: {
+      REA_TSHARK_COMMAND: join(stubs, "tshark-stub.sh"),
+      REA_CAPINFOS_COMMAND: join(stubs, "capinfos-stub.sh"),
+    },
+  });
+  const server = createServer(
+    {
+      kind: "session",
+      session: createTestBinarySession(() => {
+        throw new Error("No native binary provider");
+      }),
+    },
+    { pcapAnalysis: provider },
+  );
+  const client = new Client({ name: "pcap-workspace-test", version: "1" });
+  const [clientTransport, serverTransport] =
+    InMemoryTransport.createLinkedPair();
+  onTestFinished(async () => {
+    await client.close();
+    await server.close();
+    await provider.close();
+  });
+  await server.connect(serverTransport);
+  await client.connect(clientTransport);
+  return parseMcpToolError(
+    await client.callTool({
+      name: "inspect_pcap_capture",
+      arguments: { path },
+    }),
+  ).error;
+};
+
+const systemError = (code: string, path: string) =>
+  Object.assign(new Error(`${code}: simulated, mkdtemp '${path}'`), {
+    code,
+    path,
+  });
+
+it.each<readonly [string, string]>([
+  ["ENOTDIR", "a regular file"],
+  // Root bypasses directory permissions, so EACCES needs a non-root account.
+  ...(nonRoot ? [["EACCES", "a mode-0500 directory"] as const] : []),
+])(
+  "reports real %s allocation under %s as an unusable workspace, not a size limit",
+  async (code) => {
+    const root = await createTestTempDirectory("rea-pcap-workspace-");
+    const parent = join(root, "parent");
+    if (code === "ENOTDIR") await writeFile(parent, "not a directory");
+    else {
+      await mkdir(parent, { mode: 0o500 });
+      onTestFinished(() => chmod(parent, 0o700));
+    }
+    let cause: unknown;
+    const error = await inspectWith(
+      (options, create) =>
+        create({ ...options, parent }).catch((failure: unknown) => {
+          cause = failure;
+          throw failure;
+        }),
+      join(root, "missing.pcap"),
+    );
+    expect(cause).toMatchObject({ code });
+    expect(error).toMatchObject({
+      code: "execution_failure",
+      category: "execution_failure",
+      message: expect.stringContaining(
+        `Cannot prepare the private analysis workspace (${code})`,
+      ) as unknown,
+      remediation: {
+        action: expect.stringContaining("TMPDIR") as unknown,
+      },
+      details: {
+        provider_id: "pcap",
+        operation: "inspect_pcap_capture",
+        diagnostics: {
+          stage: "workspace",
+          workspace_path: null,
+          path: expect.stringContaining(join(parent, "rea-pcap-")) as unknown,
+          system_code: code,
+        },
+      },
+    });
+    expect(JSON.stringify(error)).not.toContain("file-size");
+  },
+);
+
+it("reaches input admission when the workspace parent is usable", async () => {
+  const root = await createTestTempDirectory("rea-pcap-workspace-");
+  const error = await inspectWith(
+    (options, create) => create({ ...options, parent: root }),
+    join(root, "missing.pcap"),
+  );
+  expect(error.code).toBe("invalid_request");
+  expect(
+    (await readdir(root)).filter((name) => name.startsWith("rea-pcap-")),
+  ).toEqual([]);
+});
+
+it.each([
+  ["ENOSPC", "resource_constraint", "Free disk space or quota"],
+  ["EDQUOT", "resource_constraint", "Free disk space or quota"],
+  ["EMFILE", "execution_failure", "open-file limit"],
+  ["ENFILE", "execution_failure", "open-file limit"],
+  ["EROFS", "execution_failure", "TMPDIR"],
+])(
+  "classifies simulated %s allocation by its storage or descriptor cause",
+  async (code, expected, recovery) => {
+    const root = await createTestTempDirectory("rea-pcap-workspace-");
+    const template = join(root, "rea-pcap-XXXXXX");
+    const error = await inspectWith(
+      () => Promise.reject(systemError(code, template)),
+      join(root, "missing.pcap"),
+    );
+    expect(error).toMatchObject({
+      code: expected,
+      remediation: { action: expect.stringContaining(recovery) as unknown },
+    });
+    if (expected === "resource_constraint")
+      expect(error.details).toMatchObject({
+        resource: "file-size",
+        reported_limits: {
+          workspace_path: null,
+          path: template,
+          system_code: code,
+        },
+      });
+    else
+      expect(error.details).toMatchObject({
+        diagnostics: { stage: "workspace", path: template, system_code: code },
+      });
+  },
+);
+
+test.skipIf(process.platform === "win32" || !nonRoot)(
+  "removes a partially prepared workspace whose contents cannot be created",
+  async () => {
+    const root = await createTestTempDirectory("rea-pcap-workspace-");
+    let workspace: string | undefined;
+    const error = await inspectWith(
+      async (options, create) => {
+        const allocated = await create({ ...options, parent: root });
+        workspace = allocated.path;
+        // The owner-only workspace exists, but its configuration directory cannot.
+        await chmod(allocated.path, 0o500);
+        return allocated;
+      },
+      join(root, "missing.pcap"),
+    );
+    expect(error).toMatchObject({
+      code: "execution_failure",
+      details: {
+        diagnostics: {
+          stage: "workspace",
+          workspace_path: workspace,
+          path: join(workspace ?? "", "configuration"),
+          system_code: "EACCES",
+        },
+      },
+    });
+    expect(
+      (await readdir(root)).filter((name) => name.startsWith("rea-pcap-")),
+    ).toEqual([]);
+  },
+);
+
+it("still reports a capture over the snapshot budget as a file-size limit", async () => {
+  const root = await createTestTempDirectory("rea-pcap-workspace-");
+  const capture = join(root, "oversized.pcap");
+  // Sparse: one byte over the 2 GiB owned-snapshot allowance, no data blocks.
+  const handle = await open(capture, "w");
+  await handle.truncate(2 * 1024 ** 3 + 1);
+  await handle.close();
+  const error = await inspectWith(
+    (options, create) => create({ ...options, parent: root }),
+    capture,
+  );
+  expect(error).toMatchObject({
+    code: "resource_constraint",
+    details: {
+      resource: "file-size",
+      reported_limits: { max_capture_bytes: 2 * 1024 ** 3 },
+    },
+  });
+  expect(
+    (await readdir(root)).filter((name) => name.startsWith("rea-pcap-")),
+  ).toEqual([]);
+});