fix(cli): resolve sandbox container by longest-owner and validate dashboardPort

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

Author: laitingsheng

src/lib/actions/sandbox/gateway-failure-classifier.ts

@@ -6,6 +6,7 @@ import net from "node:net";
 import { dockerInfo } from "../../adapters/docker/info";
 import { dockerCapture } from "../../adapters/docker/run";
 import { GATEWAY_PORT } from "../../core/ports";
+import * as registry from "../../state/registry";
 
 const DEFAULT_CONTAINER = "openshell-cluster-nemoclaw";
 const DOCKER_TIMEOUT_MS = 3000;
@@ -44,6 +45,7 @@ export type SandboxContainerFailureResult = {
 export type SandboxContainerFailureRunners = {
   listAllContainerNames: () => string;
   listRunningContainerNames: () => string;
+  listSandboxNames: () => string[];
   portProbe: (port: number) => Promise<boolean>;
 };
 
@@ -172,29 +174,71 @@ function defaultListRunningContainerNames(): string {
   });
 }
 
+function defaultListSandboxNames(): string[] {
+  try {
+    return registry.listSandboxes().sandboxes.map((entry) => entry.name);
+  } catch {
+    return [];
+  }
+}
+
 const defaultSandboxContainerRunners: SandboxContainerFailureRunners = {
   listAllContainerNames: defaultListAllContainerNames,
   listRunningContainerNames: defaultListRunningContainerNames,
+  listSandboxNames: defaultListSandboxNames,
   portProbe: defaultPortProbe,
 };
 
-function sandboxContainerNamePrefix(sandboxName: string): string {
-  return `openshell-${sandboxName}`;
-}
-
+// OpenShell names sandbox containers either as `openshell-<sandbox>` (no
+// suffix) or `openshell-<sandbox>-<id>`, where `<id>` is appended by
+// openshell at runtime. Two prefix collisions are possible:
+//
+//   1. A sandbox name can be a prefix of another sandbox name
+//      (`my` vs `my-assistant`).
+//   2. Even with a hyphen-free `<id>`, a sandbox name can be a prefix
+//      of another sandbox name whose own suffix is hyphen-free
+//      (`my-assistant` vs `my-assistant-prod`).
+//
+// Resolve each candidate to the LONGEST registered sandbox name it could
+// belong to and only accept candidates that resolve to the sandbox we are
+// looking up. The exact-name form is preferred before suffixed forms so
+// that an `openshell-<sandbox>` container always wins over an unrelated
+// `openshell-<sandbox>-<runtime-id>` co-tenant. Mirrors the resolver in
+// `docker-health.ts`.
 function findSandboxContainerName(
   names: string,
   sandboxName: string,
+  registeredSandboxNames: readonly string[],
 ): string | null {
-  const prefix = sandboxContainerNamePrefix(sandboxName);
-  for (const line of names.split("\n")) {
-    const trimmed = line.trim();
-    if (!trimmed) continue;
-    if (trimmed === prefix || trimmed.startsWith(`${prefix}-`)) return trimmed;
+  const ourPrefix = `openshell-${sandboxName}-`;
+  const ourExact = `openshell-${sandboxName}`;
+  const known = new Set<string>(registeredSandboxNames);
+  known.add(sandboxName);
+  const candidates = names
+    .split("\n")
+    .map((line) => line.trim())
+    .filter((line) => line === ourExact || line.startsWith(ourPrefix));
+  if (candidates.includes(ourExact)) return ourExact;
+  const knownArr = [...known];
+  for (const candidate of candidates) {
+    const stripped = candidate.replace(/^openshell-/, "");
+    const owner = knownArr
+      .filter((name) => stripped === name || stripped.startsWith(`${name}-`))
+      .sort((a, b) => b.length - a.length)[0];
+    if (owner === sandboxName) return candidate;
   }
   return null;
 }
 
+function isValidDashboardPort(port: number | null | undefined): port is number {
+  return (
+    typeof port === "number" &&
+    Number.isInteger(port) &&
+    port >= 1 &&
+    port <= 65535
+  );
+}
+
 export async function classifySandboxContainerFailure(
   sandboxName: string,
   opts: {
@@ -203,18 +247,24 @@ export async function classifySandboxContainerFailure(
   } = {},
 ): Promise<SandboxContainerFailureResult | null> {
   const runners = opts.runners ?? defaultSandboxContainerRunners;
+  const registeredSandboxNames = runners.listSandboxNames();
   const running = findSandboxContainerName(
     runners.listRunningContainerNames(),
     sandboxName,
+    registeredSandboxNames,
   );
   if (running) return null;
   const present = findSandboxContainerName(
     runners.listAllContainerNames(),
     sandboxName,
+    registeredSandboxNames,
   );
   if (!present) return null;
-  const dashboardPort = opts.dashboardPort ?? null;
-  if (dashboardPort && (await runners.portProbe(dashboardPort))) {
+  const dashboardPort = opts.dashboardPort;
+  if (
+    isValidDashboardPort(dashboardPort) &&
+    (await runners.portProbe(dashboardPort))
+  ) {
     return {
       layer: "sandbox_dashboard_port_conflict",
       detail: `Sandbox container '${present}' is stopped and dashboard port ${dashboardPort} is held by another process.`,

src/lib/actions/sandbox/status.ts

@@ -293,11 +293,11 @@ export async function showSandboxStatus(sandboxName: string): Promise<void> {
     console.log(getLayerHeader("docker_unreachable"));
     process.exitCode = 1;
   }
-  // #4515: when the per-sandbox container is stopped (and optionally its
-  // dashboard port is held by a foreign listener) the host-side Inference
-  // probe still hits the remote provider directly and falsely shows healthy.
-  // Probe the sandbox container upfront so the failure layer is the first
-  // user-visible signal and the misleading Inference line is suppressed.
+  // When the per-sandbox container is stopped (and optionally its dashboard
+  // port is held by a foreign listener) the host-side Inference probe still
+  // hits the remote provider directly and falsely shows healthy. Probe the
+  // sandbox container upfront so the failure layer is the first user-visible
+  // signal and the misleading Inference line is suppressed.
   const sandboxFailure = dockerUnreachable
     ? null
     : await classifySandboxContainerFailureForStatus(sandboxEntryEarly);

test/gateway-failure-classifier.test.ts

@@ -145,6 +145,7 @@ function makeSandboxRunners(
   return {
     listAllContainerNames: () => "",
     listRunningContainerNames: () => "",
+    listSandboxNames: () => [],
     portProbe: async () => false,
     ...overrides,
   };
@@ -243,7 +244,7 @@ describe("classifySandboxContainerFailure", () => {
     expect(portProbeCalled).toBe(false);
   });
 
-  it("matches the exact prefix and the uuid-suffixed shape but not unrelated containers", async () => {
+  it("matches the exact prefix and accepts uuid-suffixed shapes that resolve back to the queried sandbox", async () => {
     const exactResult = await classifySandboxContainerFailure("my-assistant", {
       runners: makeSandboxRunners({
         listAllContainerNames: () => "openshell-my-assistant\n",
@@ -252,14 +253,17 @@ describe("classifySandboxContainerFailure", () => {
     expect(exactResult?.layer).toBe("sandbox_container_stopped");
     expect(exactResult?.detail).toContain("openshell-my-assistant");
 
-    const otherSandbox = await classifySandboxContainerFailure("my-assistant", {
+    // `openshell-my-assistant-7616dcb1` belongs to `my-assistant` because no
+    // other registered sandbox name claims it via the longest-owner rule.
+    const uuidResult = await classifySandboxContainerFailure("my-assistant", {
       runners: makeSandboxRunners({
         listAllContainerNames: () =>
-          "openshell-my-assistant-evil\nopenshell-different-sandbox-abc",
+          "openshell-my-assistant-7616dcb1\nopenshell-different-sandbox-abc",
+        listSandboxNames: () => ["my-assistant", "different-sandbox"],
       }),
     });
-    expect(otherSandbox?.layer).toBe("sandbox_container_stopped");
-    expect(otherSandbox?.detail).toContain("openshell-my-assistant-evil");
+    expect(uuidResult?.layer).toBe("sandbox_container_stopped");
+    expect(uuidResult?.detail).toContain("openshell-my-assistant-7616dcb1");
 
     const unrelated = await classifySandboxContainerFailure("my-assistant", {
       runners: makeSandboxRunners({
@@ -269,4 +273,80 @@ describe("classifySandboxContainerFailure", () => {
     });
     expect(unrelated).toBeNull();
   });
+
+  it("rejects a longer registered sandbox's container even when the literal prefix matches the queried name", async () => {
+    // `openshell-my-assistant-prod-7616dcb1` resolves to the longer
+    // `my-assistant-prod` sandbox via the longest-owner rule; the query for
+    // `my-assistant` must not consume it. Mirrors the docker-health.ts
+    // resolver and prevents the prefix-collision bug.
+    const collision = await classifySandboxContainerFailure("my-assistant", {
+      runners: makeSandboxRunners({
+        listAllContainerNames: () =>
+          "openshell-my-assistant-prod-7616dcb1\nopenshell-cluster-nemoclaw",
+        listSandboxNames: () => ["my-assistant", "my-assistant-prod"],
+      }),
+    });
+    expect(collision).toBeNull();
+  });
+
+  it("matches an `openshell-<name>` exact container even when a co-tenant `openshell-<name>-<id>` exists in the same listing", async () => {
+    const result = await classifySandboxContainerFailure("my-assistant", {
+      runners: makeSandboxRunners({
+        listAllContainerNames: () =>
+          "openshell-my-assistant-7616dcb1\nopenshell-my-assistant",
+        listSandboxNames: () => ["my-assistant"],
+      }),
+    });
+    expect(result?.layer).toBe("sandbox_container_stopped");
+    expect(result?.detail).toContain("openshell-my-assistant");
+    expect(result?.detail).not.toContain("openshell-my-assistant-7616dcb1");
+  });
+
+  it("ignores an out-of-range dashboardPort and falls back to sandbox_container_stopped", async () => {
+    let portProbeCalled = false;
+    const result = await classifySandboxContainerFailure("my-assistant", {
+      dashboardPort: 70000,
+      runners: makeSandboxRunners({
+        listAllContainerNames: () => "openshell-my-assistant-7616dcb1\n",
+        portProbe: async () => {
+          portProbeCalled = true;
+          return true;
+        },
+      }),
+    });
+    expect(result?.layer).toBe("sandbox_container_stopped");
+    expect(portProbeCalled).toBe(false);
+  });
+
+  it("ignores a non-integer dashboardPort and falls back to sandbox_container_stopped", async () => {
+    let portProbeCalled = false;
+    const result = await classifySandboxContainerFailure("my-assistant", {
+      dashboardPort: 18789.5 as unknown as number,
+      runners: makeSandboxRunners({
+        listAllContainerNames: () => "openshell-my-assistant-7616dcb1\n",
+        portProbe: async () => {
+          portProbeCalled = true;
+          return true;
+        },
+      }),
+    });
+    expect(result?.layer).toBe("sandbox_container_stopped");
+    expect(portProbeCalled).toBe(false);
+  });
+
+  it("ignores a zero dashboardPort and falls back to sandbox_container_stopped", async () => {
+    let portProbeCalled = false;
+    const result = await classifySandboxContainerFailure("my-assistant", {
+      dashboardPort: 0,
+      runners: makeSandboxRunners({
+        listAllContainerNames: () => "openshell-my-assistant-7616dcb1\n",
+        portProbe: async () => {
+          portProbeCalled = true;
+          return true;
+        },
+      }),
+    });
+    expect(result?.layer).toBe("sandbox_container_stopped");
+    expect(portProbeCalled).toBe(false);
+  });
 });