Skip to content

fix(onboard): use effectivePort in dashboard wait and guard forward port collision #2086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ericksoa merged 5 commits into from
Apr 20, 2026
Merged

fix(onboard): use effectivePort in dashboard wait and guard forward port collision #2086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2e8310
fix(#2063): use agent forwardPort in dashboard wait and guard ensureD…
Dongni-Yang Apr 20, 2026
44c6324
merge: resolve conflict with upstream argv-array refactor in dashboar…
Dongni-Yang Apr 20, 2026
bcaa37f
fix(onboard): address CodeRabbit review — line-level forward collisio…
Dongni-Yang Apr 20, 2026
cf1c597
fix(onboard): fix forward list parser column indices and update test …
Dongni-Yang Apr 20, 2026
2ef0638
test(onboard): add coverage for forward port collision guard
ericksoa Apr 20, 2026
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
24 changes: 23 additions & 1 deletion src/lib/onboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3496,7 +3496,7 @@ async function createSandbox(
console.log(" Waiting for NemoClaw dashboard to become ready...");
for (let i = 0; i < 15; i++) {
const readyMatch = runCaptureOpenshell(
["sandbox", "exec", sandboxName, "curl", "-sf", `http://localhost:${CONTROL_UI_PORT}/`],
["sandbox", "exec", sandboxName, "curl", "-sf", `http://localhost:${effectivePort}/`],
{ ignoreError: true },
);
if (readyMatch) {
Expand Down Expand Up @@ -5558,6 +5558,28 @@ const { resolveDashboardForwardTarget, buildControlUiUrls } = dashboard;
function ensureDashboardForward(sandboxName, chatUiUrl = `http://127.0.0.1:${CONTROL_UI_PORT}`) {
const portToStop = getDashboardForwardPort(chatUiUrl);
const forwardTarget = getDashboardForwardTarget(chatUiUrl);
// Detect port already claimed by a different sandbox and fail fast with an
// actionable message rather than silently stealing that sandbox's forward.
// (Same sandbox is always allowed — covers reconnect and resume paths.)
const existingForwards = runCaptureOpenshell(["forward", "list"], { ignoreError: true });
// Parse line-by-line to avoid false positives from substring matches.
// openshell forward list columns: SANDBOX BIND PORT PID STATUS
// Port is at column index 2; sandbox name is at column index 0.
const portLine = existingForwards
?.split("\n")
.map((l) => l.trim())
.find((l) => {
const parts = l.split(/\s+/);
return parts[2] === portToStop;
});
const portOwner = portLine ? (portLine.split(/\s+/)[0] ?? null) : null;
if (portOwner !== null && portOwner !== sandboxName) {
throw new Error(
`Port ${portToStop} is already forwarded for sandbox '${portOwner}'. ` +
`Set CHAT_UI_URL to a different local port (e.g. http://127.0.0.1:18790) ` +
`before onboarding a second sandbox.`,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
runOpenshell(["forward", "stop", portToStop], { ignoreError: true });
// Use stdio "ignore" to prevent spawnSync from waiting on inherited pipe fds.
// The --background flag forks a child that inherits stdout/stderr; if those are
Expand Down
115 changes: 103 additions & 12 deletions test/onboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2418,7 +2418,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -2492,6 +2492,97 @@ const { createSandbox } = require(${onboardPath});
);
});

it("rejects sandbox creation when the dashboard port is already forwarded for a different sandbox", async () => {
const repoRoot = path.join(import.meta.dirname, "..");
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-forward-collision-"));
const fakeBin = path.join(tmpDir, "bin");
const scriptPath = path.join(tmpDir, "forward-collision-check.js");
const onboardPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "onboard.js"));
const runnerPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "runner.js"));
const registryPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "registry.js"));
const preflightPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "preflight.js"));
const credentialsPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "credentials.js"));

fs.mkdirSync(fakeBin, { recursive: true });
fs.writeFileSync(path.join(fakeBin, "openshell"), "#!/usr/bin/env bash\nexit 0\n", {
mode: 0o755,
});

const script = String.raw`
const runner = require(${runnerPath});
const registry = require(${registryPath});
const preflight = require(${preflightPath});
const credentials = require(${credentialsPath});
const childProcess = require("node:child_process");
const { EventEmitter } = require("node:events");

runner.run = (command, opts = {}) => {
return { status: 0 };
};
runner.runFile = (file, args = [], opts = {}) => {
return { status: 0 };
};
runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
// Port 18789 is already forwarded by a DIFFERENT sandbox (other-sandbox)
if (command.includes("'forward' 'list'")) return "other-sandbox 127.0.0.1 18789 99999 running";
return "";
};
registry.registerSandbox = () => true;
registry.removeSandbox = () => true;
preflight.checkPortAvailable = async () => ({ ok: true });
credentials.prompt = async () => "";

childProcess.spawn = (...args) => {
const child = new EventEmitter();
child.stdout = new EventEmitter();
child.stderr = new EventEmitter();
process.nextTick(() => {
child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n"));
child.emit("close", 0);
});
return child;
};

const { createSandbox } = require(${onboardPath});

(async () => {
process.env.OPENSHELL_GATEWAY = "nemoclaw";
await createSandbox(null, "gpt-5.4");
// Should not reach here — the collision guard must throw.
console.log("ERROR_NO_THROW");
process.exit(1);
})().catch((error) => {
console.log(JSON.stringify({ error: error.message }));
});
`;
fs.writeFileSync(scriptPath, script);

const result = spawnSync(process.execPath, [scriptPath], {
cwd: repoRoot,
encoding: "utf-8",
env: {
...process.env,
HOME: tmpDir,
PATH: `${fakeBin}:${process.env.PATH || ""}`,
NEMOCLAW_NON_INTERACTIVE: "1",
},
});

assert.ok(!result.stdout.includes("ERROR_NO_THROW"), "expected createSandbox to throw on port collision");
const payloadLine = result.stdout
.trim()
.split("\n")
.slice()
.reverse()
.find((line) => line.startsWith("{") && line.endsWith("}"));
assert.ok(payloadLine, `expected JSON error payload in stdout:\n${result.stdout}`);
const payload = JSON.parse(payloadLine);
assert.match(payload.error, /Port 18789 is already forwarded for sandbox 'other-sandbox'/);
});

it("binds the dashboard forward to 0.0.0.0 when CHAT_UI_URL points to a remote host", async () => {
const repoRoot = path.join(import.meta.dirname, "..");
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-remote-forward-"));
Expand Down Expand Up @@ -2529,7 +2620,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -2627,7 +2718,7 @@ runner.runCapture = (command) => {
if (normalized.includes("'sandbox' 'list'")) return "my-assistant Ready";
// Custom port: dashboard readiness curl uses 19000 (DASHBOARD_PORT from env)
if (normalized.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:19000/'")) return "ok";
if (normalized.includes("'forward' 'list'")) return "19000 -> my-assistant:19000";
if (normalized.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 19000 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -2760,7 +2851,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'provider' 'get'")) return "Provider: discord-bridge";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
if (command.includes("'sandbox' 'exec'") && command.includes("'curl'")) return "ok";
return "";
};
Expand Down Expand Up @@ -3003,7 +3094,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
// All messaging providers already exist in gateway
if (command.includes("'provider' 'get'")) return "Provider: exists";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false });
Expand Down Expand Up @@ -3293,7 +3384,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "my-assistant";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'forward' 'list'")) return "";
if (command.includes("sandbox exec") && command.includes("curl")) return "ok";
if (command.includes("'sandbox' 'exec'") && command.includes("curl")) return "ok";
return "";
};

Expand Down Expand Up @@ -3403,7 +3494,7 @@ runner.runFile = (file, args = [], opts = {}) => {
runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "my-assistant";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false });
Expand Down Expand Up @@ -4324,7 +4415,7 @@ runner.runCapture = (command) => {
return sandboxListCalls >= 2 ? "my-assistant Ready" : "my-assistant Pending";
}
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -4436,7 +4527,7 @@ runner.runFile = (file, args = [], opts = {}) => {
runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "my-assistant";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false });
Expand Down Expand Up @@ -4725,7 +4816,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -4856,7 +4947,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down Expand Up @@ -5132,7 +5223,7 @@ runner.runCapture = (command) => {
if (command.includes("'sandbox' 'get' 'my-assistant'")) return "";
if (command.includes("'sandbox' 'list'")) return "my-assistant Ready";
if (command.includes("'sandbox' 'exec' 'my-assistant' 'curl' '-sf' 'http://localhost:18789/'")) return "ok";
if (command.includes("'forward' 'list'")) return "18789 -> my-assistant:18789";
if (command.includes("'forward' 'list'")) return "my-assistant 127.0.0.1 18789 12345 running";
return "";
};
registry.registerSandbox = () => true;
Expand Down
Loading