fix(onboard): classify a failed forward list as list-failed

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

@@ -0,0 +1,80 @@
+// 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";
+
+import { captureOpenshell, runOpenshell } from "../../adapters/openshell/runtime";
+import type { SandboxMessagingPlan } from "../../messaging/manifest";
+import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward-lifecycle";
+
+vi.mock("../../adapters/openshell/runtime", () => ({
+  captureOpenshell: vi.fn(),
+  getOpenshellBinary: vi.fn(() => "openshell"),
+  runOpenshell: vi.fn(() => ({ status: 0 })),
+}));
+
+vi.mock("../../onboard/forward-start", () => ({
+  buildDetachedForwardStartSpawn: vi.fn(() => vi.fn()),
+  buildForwardStartProgressLogger: vi.fn(() => vi.fn()),
+  runDetachedForwardStartWithRetries: vi.fn(() => ({ ok: true, diagnostic: "" })),
+}));
+
+function makePlan(): SandboxMessagingPlan {
+  return {
+    schemaVersion: 1,
+    sandboxName: "demo",
+    agent: "openclaw",
+    workflow: "onboard",
+    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();
+  });
+
+  it("preserves a failed list probe and skips forward cleanup (#8522)", () => {
+    vi.mocked(captureOpenshell).mockReturnValue({ status: 1, output: "" });
+
+    const ok = ensureMessagingHostForwardAfterRebuild("demo", makePlan());
+
+    expect(ok).toBe(true);
+    expect(captureOpenshell).toHaveBeenCalledTimes(2);
+    expect(captureOpenshell).toHaveBeenNthCalledWith(
+      1,
+      ["forward", "list"],
+      expect.objectContaining({ ignoreError: true }),
+    );
+    expect(captureOpenshell).toHaveBeenNthCalledWith(
+      2,
+      ["forward", "list"],
+      expect.objectContaining({ ignoreError: true, timeout: expect.any(Number) }),
+    );
+    expect(runOpenshell).not.toHaveBeenCalled();
+  });
+});

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/onboard/agent-fixed-forward.test.ts

@@ -0,0 +1,57 @@
+// 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";
+
+import { ensureAgentFixedForward } from "./agent-fixed-forward";
+
+vi.mock("./forward-start", () => ({
+  buildDetachedForwardStartSpawn: vi.fn(() => vi.fn()),
+  buildForwardStartProgressLogger: vi.fn(() => vi.fn()),
+  runDetachedForwardStartWithRetries: vi.fn(() => ({ ok: true, diagnostic: "" })),
+}));
+
+function makeDeps(runCaptureOpenshell: () => string | null) {
+  return {
+    runOpenshell: vi.fn(() => ({ status: 0 })),
+    runCaptureOpenshell: vi.fn(runCaptureOpenshell),
+    openshellArgv: (args: string[]) => ["openshell", ...args],
+    cliName: () => "nemoclaw",
+    sleep: vi.fn(),
+  };
+}
+
+describe("ensureAgentFixedForward", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("skips the stop when the capture runner reports a failed `forward list` (#8522)", () => {
+    const deps = makeDeps(() => null);
+
+    const started = ensureAgentFixedForward(deps, "my-sandbox", 18789, "messaging webhook");
+
+    expect(started).toBe(true);
+    expect(deps.runCaptureOpenshell).toHaveBeenCalledWith(
+      ["forward", "list"],
+      expect.objectContaining({ timeout: expect.any(Number) }),
+    );
+    expect(deps.runOpenshell).not.toHaveBeenCalled();
+  });
+
+  it("runs a sandbox-scoped `forward stop` when the `forward list` is genuinely empty (#8522)", () => {
+    const deps = makeDeps(() => "SANDBOX   BIND        PORT   PID    STATUS");
+
+    const started = ensureAgentFixedForward(deps, "my-sandbox", 18789, "messaging webhook");
+
+    expect(started).toBe(true);
+    expect(deps.runCaptureOpenshell).toHaveBeenCalledWith(
+      ["forward", "list"],
+      expect.objectContaining({ timeout: expect.any(Number) }),
+    );
+    expect(deps.runOpenshell).toHaveBeenCalledWith(["forward", "stop", "18789", "my-sandbox"], {
+      ignoreError: true,
+      suppressOutput: true,
+    });
+  });
+});

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,
     );

src/lib/onboard/dashboard-forward-control.ts

@@ -32,13 +32,7 @@ export function createSandboxForwardStopper(deps: {
     if (stoppedPorts.has(portKey)) return null;
     const result = bestEffortForwardStopForSandbox(
       deps.runOpenshell,
-      (args, opts) => {
-        const output = deps.runCaptureOpenshell(args, opts);
-        if (output === null) {
-          throw new Error("Failed to list OpenShell forwards before stopping dashboard forward");
-        }
-        return output;
-      },
+      (args, opts) => deps.runCaptureOpenshell(args, opts),
       port,
       deps.sandboxName,
     );

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

@@ -41,8 +41,8 @@ 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.
+    // The helper must not suppress list failures. A runner may throw or return
+    // null, but it must not convert a failed probe to empty output.
     expect(fetch).not.toHaveBeenCalledWith(
       ["forward", "list"],
       expect.objectContaining({ ignoreError: true }),
@@ -88,13 +88,25 @@ describe("bestEffortForwardStopForSandbox", () => {
 
     const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox");
 
+    expect(fetch).toHaveBeenCalledWith(["forward", "list"], expect.anything());
     expect(outcome).toBe("list-failed");
     // Without ownership data, a port-only stop could kill another
     // sandbox's forward — better to leave the port alone and let the
     // helper's retry / next poll observe the real state.
     expect(run).not.toHaveBeenCalled();
   });
 
+  it("skips the stop entirely when `forward list` reports failure as null (owner unknown)", () => {
+    const run = vi.fn();
+    const fetch = vi.fn().mockReturnValue(null);
+
+    const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox");
+
+    expect(fetch).toHaveBeenCalledWith(["forward", "list"], expect.anything());
+    expect(outcome).toBe("list-failed");
+    expect(run).not.toHaveBeenCalled();
+  });
+
   it("ignores forwards with non-live status when deciding ownership", () => {
     // `getOccupiedPorts` filters by `isLiveForwardStatus`, so a "stopped"
     // entry on the requested port should be treated as no-entry (not as a

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,24 @@ 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 runner reports failure either by throwing or by returning null; both
+  // mean "list-failed" here. Do not pass either result to getOccupiedPorts,
+  // which parses an empty string into an empty map, so the "no-entry" branch
+  // below would still run the stop against this sandbox's own live forward
+  // without any ownership evidence. A runner that ignores the command failure
+  // itself must convert it to null, never to an empty string, which is
+  // indistinguishable from a genuinely empty forward list.
+  let listOutput: string | null = 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";