fix(messaging): preserve forward-list failures during cleanup

src/lib/actions/sandbox/messaging-host-forward-lifecycle.test.ts

@@ -0,0 +1,106 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => ({
+  captureOpenshell: vi.fn(),
+  runOpenshell: vi.fn(() => ({ status: 0 })),
+  runDetachedForwardStartWithRetries: vi.fn(),
+}));
+
+vi.mock("../../adapters/openshell/runtime", () => ({
+  captureOpenshell: mocks.captureOpenshell,
+  getOpenshellBinary: vi.fn(() => "/usr/bin/openshell"),
+  runOpenshell: mocks.runOpenshell,
+}));
+
+vi.mock("../../core/wait", () => ({ sleepSeconds: vi.fn() }));
+
+vi.mock("../../onboard/forward-start", () => ({
+  buildDetachedForwardStartSpawn: vi.fn(() => vi.fn()),
+  buildForwardStartProgressLogger: vi.fn(() => vi.fn()),
+  runDetachedForwardStartWithRetries: mocks.runDetachedForwardStartWithRetries,
+}));
+
+import type { SandboxMessagingPlan } from "../../messaging/manifest";
+import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward-lifecycle";
+
+function makePlan(): SandboxMessagingPlan {
+  return {
+    schemaVersion: 1,
+    sandboxName: "alpha",
+    agent: "openclaw",
+    workflow: "rebuild",
+    channels: [
+      {
+        channelId: "teams",
+        displayName: "Microsoft Teams",
+        authMode: "token-paste",
+        active: true,
+        selected: true,
+        configured: true,
+        disabled: false,
+        inputs: [],
+        hooks: [],
+        hostForward: {
+          channelId: "teams",
+          port: 3978,
+          label: "Microsoft Teams webhook",
+        },
+      },
+    ],
+    disabledChannels: [],
+    credentialBindings: [],
+    networkPolicy: { presets: [], entries: [] },
+    agentRender: [],
+    buildSteps: [],
+    stateUpdates: [],
+    healthChecks: [],
+  };
+}
+
+describe("ensureMessagingHostForwardAfterRebuild", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.runDetachedForwardStartWithRetries.mockReturnValue({ ok: true, diagnostic: "" });
+  });
+
+  it("does not verify startup when OpenShell cannot list forwards (#8522)", () => {
+    mocks.captureOpenshell.mockReturnValue({ status: 1, output: "" });
+    mocks.runDetachedForwardStartWithRetries.mockImplementation(
+      (_runDetachedSpawn, fetchForwardList: () => string) => {
+        expect(() => fetchForwardList()).toThrow("OpenShell forward list failed");
+        return { ok: false, diagnostic: "ownership query failed" };
+      },
+    );
+
+    const result = ensureMessagingHostForwardAfterRebuild("alpha", makePlan());
+
+    expect(result).toBe(false);
+    expect(mocks.captureOpenshell).toHaveBeenNthCalledWith(1, ["forward", "list"], {
+      ignoreError: true,
+    });
+    expect(mocks.captureOpenshell).toHaveBeenNthCalledWith(2, ["forward", "list"], {
+      ignoreError: true,
+      timeout: 15_000,
+    });
+    expect(mocks.captureOpenshell).toHaveBeenNthCalledWith(3, ["forward", "list"], {
+      ignoreError: true,
+      timeout: 15_000,
+    });
+    expect(mocks.runOpenshell).not.toHaveBeenCalled();
+  });
+
+  it("runs the sandbox-scoped stop when OpenShell returns an empty forward list (#8522)", () => {
+    mocks.captureOpenshell.mockReturnValue({ status: 0, output: "" });
+
+    const result = ensureMessagingHostForwardAfterRebuild("alpha", makePlan());
+
+    expect(result).toBe(true);
+    expect(mocks.runOpenshell).toHaveBeenCalledWith(["forward", "stop", "3978", "alpha"], {
+      ignoreError: true,
+      suppressOutput: true,
+    });
+  });
+});

src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts

@@ -18,7 +18,9 @@ import { parseForwardList } from "../../state/sandbox-session";
 import { classifyForwardHealthWithReachability, isLocalForwardReachable } from "./forward-health";
 
 function captureOpenShellOutput(args: string[], opts: Record<string, unknown> = {}): string | null {
-  const result = captureOpenshell(args, opts as Parameters<typeof captureOpenshell>[1]);
+  const result = captureOpenshell(args, { ...opts, ignoreError: true } as Parameters<
+    typeof captureOpenshell
+  >[1]);
   return result.status === 0 ? result.output : null;
 }
 

src/lib/actions/sandbox/rebuild-finalization.test.ts

@@ -125,6 +125,10 @@ describe("finalizeRebuildPostRestore", () => {
     expect(output).toContain("Mutable config permissions were not verified");
     expect(output).toContain("Mutable OpenClaw config hash was not refreshed");
     expect(output).toContain("Messaging webhook forward was not verified");
+    expect(output).toContain(
+      "Correct the preceding OpenShell error or port conflict. Then run `nemoclaw alpha connect`.",
+    );
+    expect(output).not.toContain("after resolving the port conflict");
     expect(output).toContain("Policy presets failed to reapply: messaging-telegram");
     expect(output).toContain("Shields were previously enabled");
     const orderedFragments = [

src/lib/actions/sandbox/rebuild-finalization.ts

@@ -143,7 +143,7 @@ export function finalizeRebuildPostRestore(
     }
     if (messagingHostForwardUnverified) {
       writeLine(
-        `    Messaging webhook forward was not verified \u2014 run \`${CLI_NAME} ${options.sandboxName} connect\` after resolving the port conflict`,
+        `    Messaging webhook forward was not verified. Correct the preceding OpenShell error or port conflict. Then run \`${CLI_NAME} ${options.sandboxName} connect\`.`,
       );
     }
     if (policyPresetRestoreIncomplete) {

src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts

@@ -185,6 +185,10 @@ describe("rebuild post-restore phase", () => {
     const output = vi.mocked(console.log).mock.calls.flat().join("\n");
     expect(args.bail).not.toHaveBeenCalled();
     expect(output).toContain("rebuilt but some post-restore steps were incomplete");
+    expect(output).toContain(
+      "Correct the preceding OpenShell error or port conflict. Then run `nemoclaw alpha connect`.",
+    );
+    expect(output).not.toContain("after resolving the port conflict");
     expect(output).toContain("Hermes API bearer token changed during rebuild");
     expect(output).toContain("nemoclaw alpha gateway-token --quiet");
   });

src/lib/actions/sandbox/rebuild-post-restore-phase.ts

@@ -326,7 +326,7 @@ export async function runRebuildPostRestorePhase(
     }
     if (messagingHostForwardUnverified) {
       console.log(
-        `    Messaging webhook forward was not verified \u2014 run \`${CLI_NAME} ${sandboxName} connect\` after resolving the port conflict`,
+        `    Messaging webhook forward was not verified. Correct the preceding OpenShell error or port conflict. Then run \`${CLI_NAME} ${sandboxName} connect\`.`,
       );
     }
     printHermesGatewayRestoreRecovery(sandboxName, hermesGatewayRestoreState);

src/lib/onboard/agent-fixed-forward.ts

@@ -29,7 +29,7 @@ export function ensureAgentFixedForward(
   const stopForwardForSandbox = (portToStop: string | number) =>
     bestEffortForwardStopForSandbox(
       deps.runOpenshell,
-      (args, opts) => (deps.runCaptureOpenshell(args, opts) ?? "") as string,
+      (args, opts) => deps.runCaptureOpenshell(args, opts),
       portToStop,
       sandboxName,
     );
@@ -39,9 +39,13 @@ export function ensureAgentFixedForward(
     buildDetachedForwardStartSpawn(
       deps.openshellArgv(["forward", "start", "--background", forwardTarget, sandboxName]),
     ),
-    () =>
-      (deps.runCaptureOpenshell(["forward", "list"], { timeout: OPENSHELL_PROBE_TIMEOUT_MS }) ??
-        "") as string,
+    () => {
+      const output = deps.runCaptureOpenshell(["forward", "list"], {
+        timeout: OPENSHELL_PROBE_TIMEOUT_MS,
+      });
+      if (output === null) throw new Error("OpenShell forward list failed");
+      return output;
+    },
     { port, sandboxName },
     () => {
       deps.sleep(1);

src/lib/onboard/forward-cleanup.test.ts

@@ -41,12 +41,6 @@ describe("bestEffortForwardStopForSandbox", () => {
       ["forward", "list"],
       expect.objectContaining({ timeout: 15_000 }),
     );
-    // Caller must NOT pass ignoreError; failures should throw so the catch
-    // branch returns "list-failed" instead of running a stop with no owner data.
-    expect(fetch).not.toHaveBeenCalledWith(
-      ["forward", "list"],
-      expect.objectContaining({ ignoreError: true }),
-    );
   });
 
   it("returns stopped and uses the sandbox-scoped forward stop form when ownership matches", () => {
@@ -69,7 +63,7 @@ describe("bestEffortForwardStopForSandbox", () => {
 
   it("returns no-entry and runs a sandbox-scoped stop when no live forward is on that port", () => {
     const run = vi.fn();
-    const fetch = vi.fn().mockReturnValue(forwardListWith([]));
+    const fetch = vi.fn().mockReturnValue("");
 
     const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox");
 
@@ -80,6 +74,16 @@ describe("bestEffortForwardStopForSandbox", () => {
     });
   });
 
+  it("returns list-failed and skips the stop when the forward-list runner returns null (#8522)", () => {
+    const run = vi.fn();
+    const fetch = vi.fn().mockReturnValue(null);
+
+    const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox");
+
+    expect(outcome).toBe("list-failed");
+    expect(run).not.toHaveBeenCalled();
+  });
+
   it("skips the stop entirely when `forward list` itself throws (owner unknown)", () => {
     const run = vi.fn();
     const fetch = vi.fn().mockImplementation(() => {

src/lib/onboard/forward-cleanup.ts

@@ -13,7 +13,7 @@ export type ForwardStopRunner = (
 export type ForwardListRunner = (
   args: string[],
   opts: { ignoreError?: boolean; timeout?: number },
-) => string;
+) => string | null;
 
 /**
  * `openshell forward stop <port>` — port-scoped, kills whatever forward is
@@ -64,19 +64,18 @@ export function bestEffortForwardStopForSandbox(
   port: string | number,
   sandboxName: string,
 ): "stopped" | "owned-other" | "no-entry" | "list-failed" {
-  // Let runCaptureOpenshell throw on failure/timeout so the catch branch
-  // returns "list-failed". With ignoreError: true the runner would swallow
-  // the error and return "", which getOccupiedPorts parses as an empty map
-  // and the "no-entry" branch below would still run the stop — exactly the
-  // collateral-damage case this helper exists to avoid.
-  let listOutput = "";
+  // A thrown error or `null` means that OpenShell did not return ownership data.
+  // Preserve either result as `list-failed`. Converting it to an empty string
+  // would enter the `no-entry` cleanup path.
+  let listOutput: string | null;
   try {
     listOutput = runCaptureOpenshell(["forward", "list"], {
       timeout: OPENSHELL_PROBE_TIMEOUT_MS,
     });
   } catch {
     return "list-failed";
   }
+  if (listOutput === null) return "list-failed";
   const owner = getOccupiedPorts(listOutput).get(String(port)) ?? null;
   if (owner && owner !== sandboxName) {
     return "owned-other";