fix(sandbox): catch McpBridgeError in destroy MCP bridge path (#8103)

src/lib/actions/sandbox/destroy-execution.ts

@@ -20,6 +20,7 @@ import type { SandboxEntry } from "../../state/registry";
 import type { DestroyRunOpenshell } from "./destroy-gateway";
 import {
   finalizeMcpBridgesAfterSandboxDelete,
+  McpBridgeError,
   type McpDestroyPreparation,
   prepareMcpBridgesForAbsentSandboxDestroy,
   prepareMcpBridgesForDestroy,
@@ -228,7 +229,7 @@ async function finalizeMcpDestroy(
   try {
     await finalizeMcpBridgesAfterSandboxDelete(sandboxName, preparation, { force });
   } catch (error) {
-    const detail = error instanceof Error ? error.message : String(error);
+    const detail = redactDestroyError(error);
     console.error(
       `  Sandbox '${sandboxName}' is gone, but authenticated MCP provider cleanup is incomplete: ${detail}`,
     );
@@ -269,12 +270,22 @@ export async function executeSandboxDestroy({
         };
       }
     }
-    const mcpPreparation = await prepareMcpDestroy(
-      sandboxName,
-      sandbox,
-      sandboxConfirmedAbsent,
-      force,
-    );
+    let mcpPreparation: McpDestroyPreparation;
+    try {
+      mcpPreparation = await prepareMcpDestroy(sandboxName, sandbox, sandboxConfirmedAbsent, force);
+    } catch (error) {
+      if (error instanceof McpBridgeError) {
+        return {
+          ok: false as const,
+          deleteOutput: redactDestroyError(error),
+          exitCode: error.exitCode,
+          gatewayUnreachable: false,
+          mcpOwnershipRequiresGateway: false,
+          shieldsRelockRequiresGateway: false,
+        };
+      }
+      throw error;
+    }
     // Prepared-only/incomplete adds have no external resources and are safely
     // discarded during preparation. Remaining entries are the durable exact
     // provider ownership manifest and must survive an unconfirmed delete.
@@ -329,7 +340,21 @@ export async function executeSandboxDestroy({
     // stale timer state cannot target a same-name replacement.
     cleanupShieldsArtifacts(sandboxName);
     if (!forcedLocalCleanup) {
-      await finalizeMcpDestroy(sandboxName, mcpPreparation, force);
+      try {
+        await finalizeMcpDestroy(sandboxName, mcpPreparation, force);
+      } catch (error) {
+        if (error instanceof McpBridgeError) {
+          return {
+            ok: false as const,
+            deleteOutput: redactDestroyError(error),
+            exitCode: error.exitCode,
+            gatewayUnreachable: false,
+            mcpOwnershipRequiresGateway: false,
+            shieldsRelockRequiresGateway: false,
+          };
+        }
+        throw error;
+      }
     }
     return {
       ok: true as const,

src/lib/actions/sandbox/destroy-flow.test.ts

@@ -12,7 +12,9 @@ import {
   expectFailedHardeningStillDeletes,
   expectFailedMcpFinalizePreservesRegistry,
   expectFailedMcpRestorePreservesDestroyFailure,
+  expectMcpFinalizeBridgeErrorReturnsFailure,
   expectMcpFinalizeAfterDelete,
+  expectMcpPrepareBridgeErrorAborts,
   expectMcpRestoreAfterDeleteFailure,
   expectShieldsUpRefusalBeforeMutation,
   expectStrictSandboxPresenceClassification,
@@ -371,4 +373,54 @@ describe("destroySandbox flow", () => {
 
     expectAbsentSandboxMcpFinalize(harness);
   });
+
+  it("exits with code 1 when MCP bridge prepare throws McpBridgeError, gateway down (#8103)", async () => {
+    const harness = createDestroyHarness({
+      mcpServers: ["github"],
+      prepareMcpBridgeError: "Could not inspect OpenShell provider: gateway unreachable",
+    });
+
+    await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)");
+
+    expectMcpPrepareBridgeErrorAborts(harness);
+  });
+
+  it("redacts MCP bridge finalize errors after sandbox deletion (#8103)", async () => {
+    const secretMarker = "destroy-secret-marker";
+    const harness = createDestroyHarness({
+      mcpServers: ["github"],
+      finalizeMcpBridgeError: `Could not inspect OpenShell provider: OPENAI_API_KEY=${secretMarker}`,
+    });
+
+    await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)");
+
+    expectMcpFinalizeBridgeErrorReturnsFailure(harness, secretMarker);
+  });
+
+  it("retires retained MCP state when destroy retries after finalization failure (#8103)", async () => {
+    const harness = createDestroyHarness({
+      mcpServers: ["github"],
+      finalizeMcpBridgeError: "Could not inspect OpenShell provider: gateway unreachable",
+    });
+
+    await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)");
+
+    harness.setSandboxPresent(false);
+    harness.finalizeMcpBridgesAfterSandboxDeleteSpy.mockResolvedValue(undefined);
+
+    await expect(
+      harness.destroySandbox("alpha", { yes: true, cleanupGateway: true }),
+    ).resolves.toBeUndefined();
+
+    expect(harness.prepareMcpBridgesForAbsentSandboxDestroySpy).toHaveBeenCalledWith("alpha", {
+      force: false,
+    });
+    expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).toHaveBeenCalledTimes(2);
+    expect(harness.removeSandboxSpy).toHaveBeenCalledWith("alpha");
+    expect(harness.updateSessionSpy).toHaveBeenCalledOnce();
+    expect(harness.cleanupGatewaySpy).toHaveBeenCalledWith(
+      "nemoclaw-19080",
+      harness.runOpenshellSpy,
+    );
+  });
 });

test/helpers/destroy-flow-test-assertions.ts

@@ -219,6 +219,36 @@ export function expectFailedMcpFinalizePreservesRegistry(harness: DestroyHarness
   expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled();
 }
 
+export function expectMcpPrepareBridgeErrorAborts(harness: DestroyHarness): void {
+  expect(harness.prepareMcpBridgesForDestroySpy).toHaveBeenCalled();
+  // No delete should happen when MCP prepare itself throws McpBridgeError.
+  expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith(
+    expect.arrayContaining(["sandbox", "delete"]),
+    expect.anything(),
+  );
+  expect(harness.removeSandboxSpy).not.toHaveBeenCalled();
+}
+
+export function expectMcpFinalizeBridgeErrorReturnsFailure(
+  harness: DestroyHarness,
+  secretMarker: string,
+): void {
+  expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).toHaveBeenCalled();
+  const deleteCall = harness.runOpenshellSpy.mock.calls.findIndex(
+    (call) => Array.isArray(call[0]) && call[0].join(" ") === "sandbox delete alpha",
+  );
+  expect(deleteCall).toBeGreaterThanOrEqual(0);
+  expect(
+    harness.finalizeMcpBridgesAfterSandboxDeleteSpy.mock.invocationCallOrder.at(-1),
+  ).toBeGreaterThan(harness.runOpenshellSpy.mock.invocationCallOrder[deleteCall]);
+  const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n");
+  expect(errorOutput).not.toContain(secretMarker);
+  expect(errorOutput).toContain("<REDACTED>");
+  // Registry must not be cleaned up when post-delete MCP finalize throws McpBridgeError.
+  expect(harness.removeSandboxSpy).not.toHaveBeenCalled();
+  expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled();
+}
+
 export function expectAbsentSandboxMcpFinalize(harness: DestroyHarness): void {
   expect(harness.prepareMcpBridgesForDestroySpy).not.toHaveBeenCalled();
   expect(harness.prepareMcpBridgesForAbsentSandboxDestroySpy).toHaveBeenCalledWith("alpha", {

test/helpers/destroy-flow-test-harness.ts

@@ -35,6 +35,7 @@ export type DestroyHarness = {
   restoreMcpBridgesAfterDestroyAbortSpy: MockInstance;
   runOpenshellSpy: MockInstance;
   selectGatewaySpy: MockInstance;
+  setSandboxPresent: (present: boolean) => void;
   shieldsDownSpy: MockInstance;
   stopAllSpy: MockInstance;
   stopNimByNameSpy: MockInstance;
@@ -50,12 +51,14 @@ type DestroyHarnessOptions = {
   deleteStatus?: number;
   dockerPsOutput?: string;
   endpointUrl?: string;
+  finalizeMcpBridgeError?: string;
   finalizeMcpError?: string;
   imageTag?: string | null;
   liveListOutput?: string;
   mcpAddState?: "prepared";
   mcpServers?: string[];
   openshellDriver?: string;
+  prepareMcpBridgeError?: string;
   promptResponses?: string[];
   registeredSandboxCount?: number;
   restoreMcpError?: string;
@@ -110,6 +113,7 @@ export function loadDestroySandboxPresenceClassifier(): DestroySandboxPresenceCl
 export function createDestroyHarness(options: DestroyHarnessOptions = {}): DestroyHarness {
   resetDestroyModuleCache();
   const events: string[] = [];
+  let sandboxPresent = options.sandboxPresent !== false;
 
   const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined);
   const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
@@ -199,7 +203,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr
         gatewayPinsAtSandboxList.push(process.env.OPENSHELL_GATEWAY);
         return {
           status: 0,
-          stdout: sandboxListJson(options.sandboxPresent === false ? [] : ["alpha"]),
+          stdout: sandboxListJson(sandboxPresent ? ["alpha"] : []),
           stderr: "",
         };
       case "sandbox:delete":
@@ -294,9 +298,14 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr
     destroyAlreadyPending: false,
   };
   const gatewayPinsAtMcpPrepare: Array<string | undefined> = [];
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const { McpBridgeError } = mcpBridge as any;
   const prepareMcpBridgesForDestroySpy = vi
     .spyOn(mcpBridge, "prepareMcpBridgesForDestroy")
     .mockImplementation(async () => {
+      if (options.prepareMcpBridgeError !== undefined) {
+        throw new McpBridgeError(options.prepareMcpBridgeError);
+      }
       gatewayPinsAtMcpPrepare.push(process.env.OPENSHELL_GATEWAY);
       return mcpPreparation;
     });
@@ -316,11 +325,14 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr
     });
   const finalizeMcpBridgesAfterSandboxDeleteSpy = vi
     .spyOn(mcpBridge, "finalizeMcpBridgesAfterSandboxDelete")
-    .mockImplementation(() =>
-      options.finalizeMcpError
+    .mockImplementation(() => {
+      if (options.finalizeMcpBridgeError !== undefined) {
+        return Promise.reject(new McpBridgeError(options.finalizeMcpBridgeError));
+      }
+      return options.finalizeMcpError
         ? Promise.reject(new Error(options.finalizeMcpError))
-        : Promise.resolve(),
-    );
+        : Promise.resolve();
+    });
 
   logSpy.mockClear();
 
@@ -346,6 +358,9 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr
     restoreMcpBridgesAfterDestroyAbortSpy,
     runOpenshellSpy,
     selectGatewaySpy,
+    setSandboxPresent: (present: boolean) => {
+      sandboxPresent = present;
+    },
     shieldsDownSpy,
     stopAllSpy,
     stopNimByNameSpy,