diff --git a/docs/reference/commands.md b/docs/reference/commands.md index e17a956777..741961df18 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -64,7 +64,7 @@ The wizard creates an OpenShell gateway, registers inference providers, builds t Use this command for new installs and for recreating a sandbox after changes to policy or configuration. ```console -$ nemoclaw onboard [--non-interactive] [--resume] [--recreate-sandbox] [--from ] [--agent ] [--dangerously-skip-permissions] [--yes-i-accept-third-party-software] +$ nemoclaw onboard [--non-interactive] [--resume] [--recreate-sandbox] [--from ] [--name ] [--agent ] [--dangerously-skip-permissions] [--yes-i-accept-third-party-software] ``` :::{warning} @@ -170,14 +170,28 @@ $ nemoclaw onboard --from path/to/Dockerfile The file can have any name; if it is not already named `Dockerfile`, onboard copies it to `Dockerfile` inside the staged build context automatically. All NemoClaw build arguments (`NEMOCLAW_MODEL`, `NEMOCLAW_PROVIDER_KEY`, `NEMOCLAW_INFERENCE_BASE_URL`, etc.) are injected as `ARG` overrides at build time, so declare them in your Dockerfile if you need to reference them. -In non-interactive mode, the path can also be supplied via the `NEMOCLAW_FROM_DOCKERFILE` environment variable: +In non-interactive mode, the path can also be supplied via the `NEMOCLAW_FROM_DOCKERFILE` environment variable. +You must also supply a sandbox name via `--name ` or `NEMOCLAW_SANDBOX_NAME` so a `--from` build cannot silently clobber the default `my-assistant` sandbox. ```console -$ NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_FROM_DOCKERFILE=path/to/Dockerfile nemoclaw onboard +$ NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_FROM_DOCKERFILE=path/to/Dockerfile NEMOCLAW_SANDBOX_NAME=my-build nemoclaw onboard ``` If a `--resume` is attempted with a different `--from` path than the original session, onboarding exits with a conflict error rather than silently building from the wrong image. +#### `--name ` + +Set the sandbox name without going through the interactive prompt. +The same RFC 1123 and reserved-name rules that the wizard enforces apply here too — names that match a NemoClaw CLI command (`status`, `list`, `debug`, etc.) are rejected up front. + +```console +$ nemoclaw onboard --non-interactive --name my-build --from path/to/Dockerfile +``` + +The flag wins over `NEMOCLAW_SANDBOX_NAME`. +When prompting is impossible (no TTY or `--non-interactive`), the env var is also honoured so existing CI scripts keep working. +Combining `--from ` with non-interactive onboarding requires one of `--name` or `NEMOCLAW_SANDBOX_NAME`; otherwise onboarding exits rather than silently defaulting to `my-assistant` and clobbering the default sandbox. + #### `--dangerously-skip-permissions` :::{warning} diff --git a/src/lib/onboard-command.test.ts b/src/lib/onboard-command.test.ts index dc22d3d596..394eb36953 100644 --- a/src/lib/onboard-command.test.ts +++ b/src/lib/onboard-command.test.ts @@ -40,6 +40,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: true, agent: null, dangerouslySkipPermissions: false, @@ -65,6 +66,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: true, agent: null, dangerouslySkipPermissions: false, @@ -89,6 +91,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: false, agent: null, dangerouslySkipPermissions: false, @@ -112,6 +115,7 @@ describe("onboard command", () => { expect(runOnboard).not.toHaveBeenCalled(); expect(lines.join("\n")).toContain("Usage: nemoclaw onboard"); expect(lines.join("\n")).toContain("--from "); + expect(lines.join("\n")).toContain("--name "); expect(lines.join("\n")).toContain("--agent "); expect(lines.join("\n")).toContain("--dangerously-skip-permissions"); }); @@ -138,6 +142,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: dockerfilePath, + sandboxName: null, acceptThirdPartySoftware: false, agent: null, dangerouslySkipPermissions: false, @@ -163,6 +168,7 @@ describe("onboard command", () => { fresh: true, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: false, agent: null, dangerouslySkipPermissions: false, @@ -187,6 +193,70 @@ describe("onboard command", () => { expect(errors.join("\n")).toContain("--resume and --fresh are mutually exclusive"); }); + it("parses --name ", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-name-parse-")); + const dockerfilePath = path.join(tmpDir, "Custom.Dockerfile"); + fs.writeFileSync(dockerfilePath, "FROM scratch\n"); + + expect( + parseOnboardArgs( + ["--non-interactive", "--from", dockerfilePath, "--name", "second-assistant"], + "--yes-i-accept-third-party-software", + "NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE", + { + env: {}, + error: () => {}, + exit: exitWithCode, + }, + ), + ).toEqual({ + nonInteractive: true, + resume: false, + fresh: false, + recreateSandbox: false, + fromDockerfile: dockerfilePath, + sandboxName: "second-assistant", + acceptThirdPartySoftware: false, + agent: null, + dangerouslySkipPermissions: false, + controlUiPort: null, + }); + }); + + it("exits when --name is missing its sandbox value", () => { + const errors: string[] = []; + expect(() => + parseOnboardArgs( + ["--name"], + "--yes-i-accept-third-party-software", + "NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE", + { + env: {}, + error: (message = "") => errors.push(message), + exit: exitWithPrefixedCode, + }, + ), + ).toThrow("exit:1"); + expect(errors.join("\n")).toContain("--name requires a sandbox name"); + }); + + it("exits when --name is followed by another flag instead of a value", () => { + const errors: string[] = []; + expect(() => + parseOnboardArgs( + ["--name", "--resume"], + "--yes-i-accept-third-party-software", + "NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE", + { + env: {}, + error: (message = "") => errors.push(message), + exit: exitWithPrefixedCode, + }, + ), + ).toThrow("exit:1"); + expect(errors.join("\n")).toContain("--name requires a sandbox name"); + }); + it("exits when --from is missing its Dockerfile path", () => { expect(() => parseOnboardArgs( @@ -260,6 +330,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: false, agent: "openclaw", dangerouslySkipPermissions: true, @@ -394,6 +465,7 @@ describe("onboard command", () => { fresh: false, recreateSandbox: false, fromDockerfile: null, + sandboxName: null, acceptThirdPartySoftware: false, agent: null, dangerouslySkipPermissions: false, diff --git a/src/lib/onboard-command.ts b/src/lib/onboard-command.ts index 18822f89c6..4e17d80e5a 100644 --- a/src/lib/onboard-command.ts +++ b/src/lib/onboard-command.ts @@ -10,6 +10,7 @@ export interface OnboardCommandOptions { fresh: boolean; recreateSandbox: boolean; fromDockerfile: string | null; + sandboxName: string | null; acceptThirdPartySoftware: boolean; agent: string | null; dangerouslySkipPermissions: boolean; @@ -42,7 +43,7 @@ const ONBOARD_BASE_ARGS = [ function onboardUsageLines(noticeAcceptFlag: string): string[] { return [ - ` Usage: nemoclaw onboard [--non-interactive] [--resume | --fresh] [--recreate-sandbox] [--from ] [--agent ] [--control-ui-port ] [--dangerously-skip-permissions] [${noticeAcceptFlag}]`, + ` Usage: nemoclaw onboard [--non-interactive] [--resume | --fresh] [--recreate-sandbox] [--from ] [--name ] [--agent ] [--control-ui-port ] [--dangerously-skip-permissions] [${noticeAcceptFlag}]`, "", ]; } @@ -81,6 +82,19 @@ export function parseOnboardArgs( parsedArgs.splice(fromIdx, 2); } + let sandboxName: string | null = null; + const nameIdx = parsedArgs.indexOf("--name"); + if (nameIdx !== -1) { + const nameValue = parsedArgs[nameIdx + 1]; + if (typeof nameValue !== "string" || nameValue.length === 0 || nameValue.startsWith("--")) { + error(" --name requires a sandbox name"); + printOnboardUsage(error, noticeAcceptFlag); + exit(1); + } + sandboxName = nameValue; + parsedArgs.splice(nameIdx, 2); + } + let agent: string | null = null; const agentIdx = parsedArgs.indexOf("--agent"); if (agentIdx !== -1) { @@ -141,6 +155,7 @@ export function parseOnboardArgs( fresh, recreateSandbox: parsedArgs.includes("--recreate-sandbox"), fromDockerfile, + sandboxName, acceptThirdPartySoftware: parsedArgs.includes(noticeAcceptFlag) || String(deps.env[noticeAcceptEnv] || "") === "1", agent, diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 037879ba6c..817d0b7845 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -291,6 +291,7 @@ type OnboardOptions = { resume?: boolean; fresh?: boolean; fromDockerfile?: string | null; + sandboxName?: string | null; acceptThirdPartySoftware?: boolean; agent?: string | null; controlUiPort?: number | null; @@ -1868,15 +1869,27 @@ const { prepareOllamaModel, } = require("./onboard-ollama-proxy"); -function getRequestedSandboxNameHint(): string | null { - const raw = process.env.NEMOCLAW_SANDBOX_NAME; +function getRequestedSandboxNameHint(opts: { sandboxName?: string | null } = {}): string | null { + const raw = + typeof opts.sandboxName === "string" && opts.sandboxName.length > 0 + ? opts.sandboxName + : process.env.NEMOCLAW_SANDBOX_NAME; if (typeof raw !== "string") return null; const normalized = raw.trim().toLowerCase(); return normalized || null; } -function getResumeSandboxConflict(session: Session | null) { - const requestedSandboxName = getRequestedSandboxNameHint(); +function getResumeSandboxConflict( + session: Session | null, + opts: { sandboxName?: string | null } = {}, +) { + // Use opts.sandboxName as the sole source — the caller has already + // resolved it (--name first, NEMOCLAW_SANDBOX_NAME only when prompting + // is impossible). Falling back to the env var here would fire spurious + // conflicts for interactive resume runs whose shell happens to export + // NEMOCLAW_SANDBOX_NAME but which never actually consult it. + const raw = typeof opts.sandboxName === "string" ? opts.sandboxName.trim().toLowerCase() : ""; + const requestedSandboxName = raw || null; if (!requestedSandboxName || !session?.sandboxName) { return null; } @@ -1896,12 +1909,17 @@ function getRequestedModelHint(nonInteractive = isNonInteractive()) { function getResumeConfigConflicts( session: Session | null, - opts: { nonInteractive?: boolean; fromDockerfile?: string | null; agent?: string | null } = {}, + opts: { + nonInteractive?: boolean; + fromDockerfile?: string | null; + sandboxName?: string | null; + agent?: string | null; + } = {}, ) { const conflicts = []; const nonInteractive = opts.nonInteractive ?? isNonInteractive(); - const sandboxConflict = getResumeSandboxConflict(session); + const sandboxConflict = getResumeSandboxConflict(session, { sandboxName: opts.sandboxName }); if (sandboxConflict) { conflicts.push({ field: "sandbox", @@ -3190,6 +3208,25 @@ async function recoverGatewayRuntime() { // ── Step 3: Sandbox ────────────────────────────────────────────── +// Names that collide with global CLI commands. A sandbox named 'status' +// makes 'nemoclaw status connect' route to the global status command +// instead of the sandbox, so reject these wherever a sandbox name enters +// the system (interactive prompt, --name flag, NEMOCLAW_SANDBOX_NAME). +const RESERVED_SANDBOX_NAMES = new Set([ + "onboard", + "list", + "deploy", + "setup", + "setup-spark", + "start", + "stop", + "status", + "debug", + "uninstall", + "credentials", + "help", +]); + async function promptValidatedSandboxName() { const MAX_ATTEMPTS = 3; for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) { @@ -3202,24 +3239,7 @@ async function promptValidatedSandboxName() { try { const validatedSandboxName = validateName(sandboxName, "sandbox name"); - // Reject names that collide with global CLI commands. - // A sandbox named 'status' makes 'nemoclaw status connect' route to - // the global status command instead of the sandbox. - const RESERVED_NAMES = new Set([ - "onboard", - "list", - "deploy", - "setup", - "setup-spark", - "start", - "stop", - "status", - "debug", - "uninstall", - "credentials", - "help", - ]); - if (RESERVED_NAMES.has(sandboxName)) { + if (RESERVED_SANDBOX_NAMES.has(sandboxName)) { console.error(` Reserved name: '${sandboxName}' is a NemoClaw CLI command.`); console.error(" Choose a different name to avoid routing conflicts."); if (isNonInteractive()) { @@ -6916,6 +6936,60 @@ async function onboard(opts: OnboardOptions = {}): Promise { const requestedFromDockerfile = opts.fromDockerfile || (isNonInteractive() ? process.env.NEMOCLAW_FROM_DOCKERFILE || null : null); + // Resolve the explicit sandbox name early so both validation and the + // --from guard work off the same source. --name always counts; the env + // var only counts when we cannot prompt (otherwise interactive runs would + // bypass the prompt UX), since the existing prompt path already seeds + // from the env var via promptOrDefault when --non-interactive is set. + // Cover both --non-interactive and missing-TTY runs (CI scripts, piped + // stdin) — the issue's test plan asks for both. + const stdinIsTty = Boolean(process.stdin && process.stdin.isTTY); + const stdoutIsTty = Boolean(process.stdout && process.stdout.isTTY); + const cannotPrompt = isNonInteractive() || !stdinIsTty || !stdoutIsTty; + let requestedSandboxName: string | null = + typeof opts.sandboxName === "string" && opts.sandboxName.length > 0 + ? opts.sandboxName + : null; + let requestedSandboxSource: "--name" | "NEMOCLAW_SANDBOX_NAME" | null = requestedSandboxName + ? "--name" + : null; + if (!requestedSandboxName && cannotPrompt) { + const envName = process.env.NEMOCLAW_SANDBOX_NAME; + if (typeof envName === "string" && envName.trim().length > 0) { + requestedSandboxName = envName.trim(); + requestedSandboxSource = "NEMOCLAW_SANDBOX_NAME"; + } + } + if (requestedSandboxName) { + try { + const validated = validateName(requestedSandboxName, "sandbox name"); + if (RESERVED_SANDBOX_NAMES.has(validated)) { + console.error(` Reserved name: '${validated}' is a NemoClaw CLI command.`); + console.error( + ` Choose a different sandbox name (passed via ${requestedSandboxSource}) to avoid routing conflicts.`, + ); + process.exit(1); + } + requestedSandboxName = validated; + } catch (error) { + console.error(` ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } + } + // The downstream prompt path silently defaults to 'my-assistant' when no + // input arrives. With --from in play that would clobber the default + // sandbox, so refuse to proceed unless the caller has supplied a name + // out-of-band. Cover both --non-interactive and missing-TTY runs (CI + // scripts, piped stdin) — the issue's test plan asks for both. The resume + // case is handled separately after session load (see below) because its + // recorded sandboxName may already satisfy the requirement. + if (cannotPrompt && !resume && requestedFromDockerfile && !requestedSandboxName) { + console.error( + " --from requires --name (or NEMOCLAW_SANDBOX_NAME) when running without a TTY or with --non-interactive.", + ); + console.error(" A sandbox name cannot be prompted for in this context."); + process.exit(1); + } const noticeAccepted = await ensureUsageNoticeConsent({ nonInteractive: isNonInteractive(), acceptedByFlag: opts.acceptThirdPartySoftware === true, @@ -7019,6 +7093,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { const resumeConflicts = getResumeConfigConflicts(session, { nonInteractive: isNonInteractive(), fromDockerfile: requestedFromDockerfile, + sandboxName: requestedSandboxName, agent: opts.agent || null, }); if (resumeConflicts.length > 0) { @@ -7079,6 +7154,28 @@ async function onboard(opts: OnboardOptions = {}): Promise { ); } + // Backstop for the resume path: a session may exist (so the early guard + // skipped because resume === true) but never have recorded a sandboxName + // — sandbox creation could have failed before that step ran. Without a + // --name or env-var seed, the downstream prompt path would fall back to + // 'my-assistant' under no TTY, exactly the silent-default the early + // guard is meant to prevent. + if ( + resume && + cannotPrompt && + fromDockerfile && + !requestedSandboxName && + !session?.sandboxName + ) { + console.error( + " --from requires --name (or NEMOCLAW_SANDBOX_NAME) when running without a TTY or with --non-interactive.", + ); + console.error( + " The resumed session has no recorded sandbox name, so one cannot be inferred.", + ); + process.exit(1); + } + let completed = false; process.once("exit", (code) => { if (!completed && code !== 0) { @@ -7165,7 +7262,14 @@ async function onboard(opts: OnboardOptions = {}): Promise { onboardSession.markStepComplete("gateway"); } - let sandboxName = session?.sandboxName || null; + let sandboxName = session?.sandboxName || requestedSandboxName || null; + if (sandboxName && RESERVED_SANDBOX_NAMES.has(sandboxName)) { + console.error( + ` Reserved name in resumed session: '${sandboxName}' is a NemoClaw CLI command.`, + ); + console.error(" Start a fresh onboard with --name to choose a different name."); + process.exit(1); + } let model = session?.model || null; let provider = session?.provider || null; let endpointUrl = session?.endpointUrl || null; diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 2d7c6997b3..1f533452da 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -1591,15 +1591,56 @@ startGateway(null).catch(() => {}); } }); + it("prefers the explicit --name option over NEMOCLAW_SANDBOX_NAME", () => { + const previous = process.env.NEMOCLAW_SANDBOX_NAME; + process.env.NEMOCLAW_SANDBOX_NAME = "from-env"; + try { + expect(getRequestedSandboxNameHint({ sandboxName: "From-Flag" })).toBe("from-flag"); + } finally { + if (previous === undefined) { + delete process.env.NEMOCLAW_SANDBOX_NAME; + } else { + process.env.NEMOCLAW_SANDBOX_NAME = previous; + } + } + }); + + it("detects resume conflicts when --name does not match the recorded sandbox", () => { + expect( + getResumeConfigConflicts( + { sandboxName: "my-assistant" }, + { sandboxName: "second-assistant" }, + ), + ).toEqual([ + { + field: "sandbox", + requested: "second-assistant", + recorded: "my-assistant", + }, + ]); + }); + it("detects resume conflicts when a different sandbox is requested", () => { + expect( + getResumeSandboxConflict({ sandboxName: "my-assistant" }, { sandboxName: "other-sandbox" }), + ).toEqual({ + requestedSandboxName: "other-sandbox", + recordedSandboxName: "my-assistant", + }); + expect( + getResumeSandboxConflict({ sandboxName: "other-sandbox" }, { sandboxName: "other-sandbox" }), + ).toBe(null); + }); + + it("does not fire a resume conflict from NEMOCLAW_SANDBOX_NAME alone", () => { + // Interactive resume runs never consult the env var (sandbox creation + // is already complete in the session, so promptOrDefault is skipped). + // Reading it here would surface a spurious conflict whenever a user + // happens to export NEMOCLAW_SANDBOX_NAME in their shell rc. const previous = process.env.NEMOCLAW_SANDBOX_NAME; process.env.NEMOCLAW_SANDBOX_NAME = "other-sandbox"; try { - expect(getResumeSandboxConflict({ sandboxName: "my-assistant" })).toEqual({ - requestedSandboxName: "other-sandbox", - recordedSandboxName: "my-assistant", - }); - expect(getResumeSandboxConflict({ sandboxName: "other-sandbox" })).toBe(null); + expect(getResumeSandboxConflict({ sandboxName: "my-assistant" })).toBe(null); } finally { if (previous === undefined) { delete process.env.NEMOCLAW_SANDBOX_NAME; @@ -2637,6 +2678,18 @@ const { setupInference } = require(${onboardPath}); ); }); + it("re-checks RESERVED_SANDBOX_NAMES against a resumed session's sandboxName", () => { + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"), + "utf-8", + ); + + assert.match( + source, + /let sandboxName = session\?\.sandboxName \|\| requestedSandboxName \|\| null;\s*if \(sandboxName && RESERVED_SANDBOX_NAMES\.has\(sandboxName\)\) \{[\s\S]*?process\.exit\(1\);\s*\}/, + ); + }); + it("delegates sandbox-create progress streaming to the extracted helper module", () => { const onboardSource = fs.readFileSync( path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"),