diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index f278fa53d2e..a29fc49a964 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -12,7 +12,7 @@ // src/lib/actions/**; keep this file limited to argv normalization, // public route translation, suggestions, and registry-aware sandbox-name checks. const { ROOT, validateName } = require("../runner"); -const { CLI_NAME } = require("./branding"); +const { CLI_NAME, CLI_DISPLAY_NAME } = require("./branding"); const { help } = require("../actions/root-help"); const { runOclifArgv, runOclifCommandById } = require("./oclif-runner"); const { @@ -22,6 +22,7 @@ const { sandboxActionTokensForDispatch, } = require("./command-registry"); +import { type ForeignGatewaySandbox, findForeignGatewaySandbox } from "../state/gateway-registry"; import { migrateLegacyPortState } from "../state/legacy-port-migration"; import { type NormalizedArgv, @@ -277,6 +278,41 @@ function printGlobalStatusScopeHint(sandboxName: string, args: readonly string[] process.exit(2); } +/** Name every gateway that owns a sandbox the selected gateway's registry cannot see (#10656). */ +function printForeignGatewaySandbox( + sandboxName: string, + action: string, + owner: ForeignGatewaySandbox, +): never { + const { owners } = owner; + console.error( + owners.length === 1 + ? ` Sandbox '${sandboxName}' is registered on a different ${CLI_DISPLAY_NAME} gateway.` + : ` Sandbox '${sandboxName}' is registered on ${String(owners.length)} other ${CLI_DISPLAY_NAME} gateways.`, + ); + console.error(""); + for (const entry of owners) { + console.error(` Owning gateway: ${entry.gatewayName} (port ${String(entry.gatewayPort)})`); + console.error(` Owning registry: ${entry.registryFile}`); + } + console.error( + ` Selected gateway: ${owner.selectedGatewayName} (port ${String(owner.selectedGatewayPort)})`, + ); + console.error(""); + console.error(` Every command reads the state root that NEMOCLAW_GATEWAY_PORT selects.`); + console.error( + owners.length === 1 + ? " Rerun against the owning gateway:" + : " Rerun against the gateway you meant:", + ); + for (const entry of owners) { + console.error( + ` NEMOCLAW_GATEWAY_PORT=${String(entry.gatewayPort)} ${CLI_NAME} ${sandboxName} ${action}`, + ); + } + process.exit(1); +} + /** * Report the sandbox-first grammar for a first token that names a sandbox action. * @@ -331,6 +367,12 @@ async function recoverRequestedSandboxIfNeeded( await registryRecovery().recoverRegistryEntries({ requestedSandboxName: sandboxName }); if (registry().getSandbox(sandboxName)) return; + // Registry reads are pinned to the selected gateway's state root, so another + // gateway's sandbox reads as absent and the block below would claim it does + // not exist. Name the owner instead; recovery stays gateway-scoped (#7105). + const foreignOwner = findForeignGatewaySandbox(sandboxName); + if (foreignOwner) printForeignGatewaySandbox(sandboxName, action, foreignOwner); + // Recovery runs first so a live sandbox named after an action stays reachable // through the name-first grammar. A token that recovery cannot resolve is a // scope error, not a missing sandbox. diff --git a/src/lib/state/gateway-registry.test.ts b/src/lib/state/gateway-registry.test.ts index 2dde76a4959..328cccd9cb8 100644 --- a/src/lib/state/gateway-registry.test.ts +++ b/src/lib/state/gateway-registry.test.ts @@ -7,7 +7,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { listHostGatewayRegistryEntries } from "./gateway-registry"; +import { findForeignGatewaySandbox, listHostGatewayRegistryEntries } from "./gateway-registry"; describe("host gateway registry index", () => { it.runIf(process.platform !== "win32")( @@ -124,3 +124,103 @@ describe("host gateway registry index", () => { } }); }); + +describe("foreign gateway sandbox lookup (#10656)", () => { + function twoGatewayHome(siblingRegistry: string): string { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-foreign-gateway-")); + const selected = path.join(home, ".nemoclaw"); + const sibling = path.join(selected, "gateways", "8990"); + fs.mkdirSync(sibling, { recursive: true }); + fs.writeFileSync( + path.join(selected, "sandboxes.json"), + JSON.stringify({ + defaultSandbox: "healthy-b", + sandboxes: { "healthy-b": { name: "healthy-b" } }, + }), + ); + fs.writeFileSync(path.join(sibling, "sandboxes.json"), siblingRegistry); + return home; + } + + const ownedBySibling = JSON.stringify({ + defaultSandbox: "failure-a", + sandboxes: { + "failure-a": { name: "failure-a", gatewayName: "nemoclaw-8990", gatewayPort: 8990 }, + }, + }); + + it("names the gateway that owns a sandbox the selected root cannot see", () => { + const home = twoGatewayHome(ownedBySibling); + try { + expect(findForeignGatewaySandbox("failure-a", 8080, home)).toEqual({ + owners: [ + { + gatewayName: "nemoclaw-8990", + gatewayPort: 8990, + registryFile: path.join(home, ".nemoclaw", "gateways", "8990", "sandboxes.json"), + }, + ], + selectedGatewayName: "nemoclaw", + selectedGatewayPort: 8080, + }); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("reports no owner for a sandbox the selected gateway already holds", () => { + const home = twoGatewayHome(ownedBySibling); + try { + expect(findForeignGatewaySandbox("healthy-b", 8080, home)).toBeNull(); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + // Nothing enforces host-wide sandbox-name uniqueness, so a lookup that named + // only the lowest-numbered port would send the rerun command to an arbitrary + // gateway. + it("reports every gateway that claims the same sandbox name", () => { + const home = twoGatewayHome(ownedBySibling); + const second = path.join(home, ".nemoclaw", "gateways", "9000"); + fs.mkdirSync(second, { recursive: true }); + fs.writeFileSync( + path.join(second, "sandboxes.json"), + JSON.stringify({ + defaultSandbox: "failure-a", + sandboxes: { + "failure-a": { name: "failure-a", gatewayName: "nemoclaw-9000", gatewayPort: 9000 }, + }, + }), + ); + try { + expect(findForeignGatewaySandbox("failure-a", 8080, home)?.owners).toEqual([ + { + gatewayName: "nemoclaw-8990", + gatewayPort: 8990, + registryFile: path.join(home, ".nemoclaw", "gateways", "8990", "sandboxes.json"), + }, + { + gatewayName: "nemoclaw-9000", + gatewayPort: 9000, + registryFile: path.join(home, ".nemoclaw", "gateways", "9000", "sandboxes.json"), + }, + ]); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + // listHostGatewayRegistryEntries fails closed for its allocating callers. This + // caller is a diagnostic on an already-failing command, so a malformed sibling + // must degrade to "no owner" and let the caller exit cleanly, never throw. + it("degrades to no owner when a sibling registry is unreadable", () => { + const home = twoGatewayHome("{ not json"); + try { + expect(() => listHostGatewayRegistryEntries(home)).toThrow(); + expect(findForeignGatewaySandbox("failure-a", 8080, home)).toBeNull(); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/src/lib/state/gateway-registry.ts b/src/lib/state/gateway-registry.ts index 1d86f67c39a..607303f914c 100644 --- a/src/lib/state/gateway-registry.ts +++ b/src/lib/state/gateway-registry.ts @@ -6,10 +6,10 @@ import path from "node:path"; import { isErrnoException } from "../core/errno"; import { isObjectRecord } from "../core/json-types"; -import { DEFAULT_GATEWAY_PORT } from "../core/ports"; +import { DEFAULT_GATEWAY_PORT, GATEWAY_PORT } from "../core/ports"; import { NAME_MAX_LENGTH, NAME_VALID_PATTERN } from "../name-validation"; import { resolveGatewayName, resolveGatewayPortFromName } from "../onboard/gateway-binding"; -import { GATEWAYS_SUBDIR, nemoclawStateRoot } from "./state-root"; +import { GATEWAYS_SUBDIR, nemoclawStateRoot, resolveHome } from "./state-root"; export { GATEWAYS_SUBDIR } from "./state-root"; export { @@ -252,3 +252,63 @@ export function listHostGatewayRegistryEntries(home: string): HostGatewayRegistr } return result; } + +export interface ForeignGatewayOwner { + gatewayName: string; + gatewayPort: number; + registryFile: string; +} + +export interface ForeignGatewaySandbox { + /** Every other gateway root claiming the name, ascending by port. */ + owners: ForeignGatewayOwner[]; + selectedGatewayName: string; + selectedGatewayPort: number; +} + +/** + * Locate a sandbox registered under a gateway state root other than the selected + * one. + * + * Registry reads are pinned to the selected gateway's root, so a second + * gateway's sandbox reads as absent. Recovery stays gateway-scoped on purpose + * (#7105), so this only reports the owner and never adopts its row (#10656). + * + * Returns null when no other root claims the name, and also when any host + * registry is unreadable. `listHostGatewayRegistryEntries` fails closed on + * malformed JSON, a bad row, a symlinked root or a port mismatch, which is + * right for its allocating callers but wrong here: the sole caller is an + * already-failing diagnostic that must still exit cleanly rather than throw. + * That matches the read-only carve-out `safeListRegistryEntries` established. + */ +export function findForeignGatewaySandbox( + sandboxName: string, + selectedGatewayPort: number = GATEWAY_PORT, + home: string = resolveHome(), +): ForeignGatewaySandbox | null { + let hostEntries: HostGatewayRegistryEntry[]; + try { + hostEntries = listHostGatewayRegistryEntries(home); + } catch { + return null; + } + // Nothing enforces host-wide sandbox-name uniqueness, so two gateway roots can + // each register the name. Report every claimant: naming one would send a + // rerun command to an arbitrary gateway, chosen only by port order. + const owners = hostEntries + .filter( + (candidate) => + candidate.entry.name === sandboxName && candidate.gatewayPort !== selectedGatewayPort, + ) + .map((candidate) => ({ + gatewayName: resolveGatewayName(candidate.gatewayPort), + gatewayPort: candidate.gatewayPort, + registryFile: candidate.registryFile, + })); + if (owners.length === 0) return null; + return { + owners, + selectedGatewayName: resolveGatewayName(selectedGatewayPort), + selectedGatewayPort, + }; +} diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 808f972f635..70769fd59eb 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -948,3 +948,93 @@ describe("CLI dispatch", () => { ); }); }); + +describe("multi-gateway sandbox ownership (#10656)", () => { + const owner = { + owners: [ + { + gatewayName: "nemoclaw-8990", + gatewayPort: 8990, + registryFile: "/home/qa/.nemoclaw/gateways/8990/sandboxes.json", + }, + ], + selectedGatewayName: "nemoclaw", + selectedGatewayPort: 8080, + }; + + it.each(["status", "recover"])( + "names the owning gateway instead of denying the sandbox for %s", + async (action) => { + await withDirectPublicDispatch( + async ({ dispatchCli, exitSpy, stderr }) => { + await expect(dispatchCli(["failure-a", action])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain( + "Sandbox 'failure-a' is registered on a different NemoClaw gateway.", + ); + expect(output).toContain("Owning gateway: nemoclaw-8990 (port 8990)"); + expect(output).toContain( + "Owning registry: /home/qa/.nemoclaw/gateways/8990/sandboxes.json", + ); + expect(output).toContain("Selected gateway: nemoclaw (port 8080)"); + expect(output).toContain(`NEMOCLAW_GATEWAY_PORT=8990 nemoclaw failure-a ${action}`); + expect(output).not.toContain("does not exist"); + expect(output).not.toContain("Registered sandboxes:"); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + { sandboxNames: ["healthy-b"], foreignGatewaySandbox: owner }, + ); + }, + ); + + it("names every gateway when more than one claims the sandbox", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, exitSpy, stderr }) => { + await expect(dispatchCli(["failure-a", "status"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("Sandbox 'failure-a' is registered on 2 other NemoClaw gateways."); + expect(output).toContain("Owning gateway: nemoclaw-8990 (port 8990)"); + expect(output).toContain("Owning gateway: nemoclaw-9000 (port 9000)"); + expect(output).toContain("NEMOCLAW_GATEWAY_PORT=8990 nemoclaw failure-a status"); + expect(output).toContain("NEMOCLAW_GATEWAY_PORT=9000 nemoclaw failure-a status"); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + { + sandboxNames: ["healthy-b"], + foreignGatewaySandbox: { + owners: [ + { + gatewayName: "nemoclaw-8990", + gatewayPort: 8990, + registryFile: "/home/qa/.nemoclaw/gateways/8990/sandboxes.json", + }, + { + gatewayName: "nemoclaw-9000", + gatewayPort: 9000, + registryFile: "/home/qa/.nemoclaw/gateways/9000/sandboxes.json", + }, + ], + selectedGatewayName: "nemoclaw", + selectedGatewayPort: 8080, + }, + }, + ); + }); + + it("keeps the unknown-sandbox message when no other gateway owns the name", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, exitSpy, stderr }) => { + await expect(dispatchCli(["failure-a", "status"])).rejects.toThrow("process.exit:1"); + + const output = stderr.join("\n"); + expect(output).toContain("Sandbox 'failure-a' does not exist."); + expect(output).toContain("Registered sandboxes: healthy-b"); + expect(output).not.toContain("Owning gateway:"); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + { sandboxNames: ["healthy-b"], foreignGatewaySandbox: null }, + ); + }); +}); diff --git a/test/support/public-dispatch-test-harness.ts b/test/support/public-dispatch-test-harness.ts index af1ff6500e3..e2cb930bbbd 100644 --- a/test/support/public-dispatch-test-harness.ts +++ b/test/support/public-dispatch-test-harness.ts @@ -3,6 +3,8 @@ import { vi } from "vitest"; +import type { ForeignGatewaySandbox } from "../../src/lib/state/gateway-registry.js"; + type SandboxStub = { name: string; pendingRouteReservation?: true; createdAt?: string }; export type DirectPublicDispatchHarness = { @@ -30,6 +32,12 @@ type DirectPublicDispatchOptions = { connectFlags?: readonly string[]; /** Error injected by the pre-dispatch legacy-state migration seam. */ migrationError?: Error; + /** + * Owning-gateway row the host-wide lookup reports for a foreign sandbox + * (#10656). Stubbed for every case so the real lookup never reads the + * developer's own `$HOME`. + */ + foreignGatewaySandbox?: ForeignGatewaySandbox | null; }; const requireCache = require.cache as Record; @@ -64,12 +72,14 @@ export async function withDirectPublicDispatch( const legacyPortMigrationPath = require.resolve("../../src/lib/state/legacy-port-migration.js"); const registryRecoveryPath = require.resolve("../../src/lib/registry-recovery-action.js"); const runnerPath = require.resolve("../../src/lib/runner.js"); + const gatewayRegistryPath = require.resolve("../../src/lib/state/gateway-registry.js"); const priorPublicDispatch = requireCache[publicDispatchPath]; const priorOclifRunner = requireCache[oclifRunnerPath]; const priorSandboxConnect = requireCache[sandboxConnectPath]; const priorRegistry = requireCache[registryPath]; const priorLegacyPortMigration = requireCache[legacyPortMigrationPath]; const priorRegistryRecovery = requireCache[registryRecoveryPath]; + const priorGatewayRegistry = requireCache[gatewayRegistryPath]; const priorRunner = requireCache[runnerPath]; const priorDockerHost = process.env.DOCKER_HOST; const pendingSandboxNames = new Set(options.pendingSandboxNames ?? []); @@ -138,6 +148,9 @@ export async function withDirectPublicDispatch( }); cacheModule(legacyPortMigrationPath, { migrateLegacyPortState }); cacheModule(registryRecoveryPath, { recoverRegistryEntries }); + cacheModule(gatewayRegistryPath, { + findForeignGatewaySandbox: vi.fn(() => options.foreignGatewaySandbox ?? null), + }); cacheModule(oclifRunnerPath, { runOclifArgv, runOclifCommandById }); const connectFlags = new Set(options.connectFlags ?? []); cacheModule(sandboxConnectPath, { @@ -177,6 +190,7 @@ export async function withDirectPublicDispatch( restoreCache(registryPath, priorRegistry); restoreCache(legacyPortMigrationPath, priorLegacyPortMigration); restoreCache(registryRecoveryPath, priorRegistryRecovery); + restoreCache(gatewayRegistryPath, priorGatewayRegistry); restoreCache(runnerPath, priorRunner); if (priorDockerHost === undefined) { delete process.env.DOCKER_HOST;