Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion src/lib/cli/public-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down
102 changes: 101 additions & 1 deletion src/lib/state/gateway-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")(
Expand Down Expand Up @@ -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 });
}
});
});
64 changes: 62 additions & 2 deletions src/lib/state/gateway-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
};
}
90 changes: 90 additions & 0 deletions test/cli/dispatch-basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);
});
});
Loading
Loading