From 5d43d3c7958f6fc4fffb7d08a61354df6d7aee09 Mon Sep 17 00:00:00 2001 From: Revant Patel Date: Tue, 21 Apr 2026 09:51:48 -0700 Subject: [PATCH] fix(onboard): recreate same-name sandbox on provider/model drift --- .../references/commands.md | 6 + docs/reference/commands.md | 6 + src/lib/onboard.ts | 185 +++++++++++++++--- test/onboard.test.ts | 108 +++++++++- 4 files changed, 270 insertions(+), 35 deletions(-) diff --git a/.agents/skills/nemoclaw-user-reference/references/commands.md b/.agents/skills/nemoclaw-user-reference/references/commands.md index 2539c47571a..a7f96a3ba83 100644 --- a/.agents/skills/nemoclaw-user-reference/references/commands.md +++ b/.agents/skills/nemoclaw-user-reference/references/commands.md @@ -110,6 +110,7 @@ The wizard prompts for a sandbox name. Names must follow RFC 1123 subdomain rules: lowercase alphanumeric characters and hyphens only, and must start and end with an alphanumeric character. Uppercase letters are automatically lowercased. Names that match global CLI commands (`status`, `list`, `debug`, etc.) are rejected to avoid routing conflicts. +Use `--agent ` to target a specific installed agent profile during onboarding. If you enable Slack during onboarding, the wizard collects both the Bot Token (`SLACK_BOT_TOKEN`) and the App-Level Token (`SLACK_APP_TOKEN`). Socket Mode requires both tokens. @@ -120,6 +121,11 @@ NemoClaw bakes those values into the sandbox image as Discord guild workspace co If you leave the Discord User ID blank, the guild config omits the user allowlist and any member of the configured server can message the bot. Guild responses remain mention-gated by default unless you opt into all-message replies. +If you run onboarding again with the same sandbox name and choose a different inference provider or model, NemoClaw detects the drift and recreates the sandbox so the running OpenClaw UI matches your selection. +In interactive mode, the wizard asks for confirmation before delete and recreate. +In non-interactive mode, NemoClaw recreates automatically when the stored selection is readable and differs; if NemoClaw cannot read the stored selection, NemoClaw reuses by default. +Set `NEMOCLAW_RECREATE_SANDBOX=1` to force recreation even when no drift is detected. + Before creating the gateway, the wizard runs preflight checks. It verifies that Docker is reachable, warns on untested runtimes such as Podman, and prints host remediation guidance when prerequisites are missing. The preflight also enforces the OpenShell version range declared in the blueprint (`min_openshell_version` and `max_openshell_version`). diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 9acb8c7d5c6..6c9a4cb1f71 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -132,6 +132,7 @@ The wizard prompts for a sandbox name. Names must follow RFC 1123 subdomain rules: lowercase alphanumeric characters and hyphens only, and must start and end with an alphanumeric character. Uppercase letters are automatically lowercased. Names that match global CLI commands (`status`, `list`, `debug`, etc.) are rejected to avoid routing conflicts. +Use `--agent ` to target a specific installed agent profile during onboarding. If you enable Slack during onboarding, the wizard collects both the Bot Token (`SLACK_BOT_TOKEN`) and the App-Level Token (`SLACK_APP_TOKEN`). Socket Mode requires both tokens. @@ -142,6 +143,11 @@ NemoClaw bakes those values into the sandbox image as Discord guild workspace co If you leave the Discord User ID blank, the guild config omits the user allowlist and any member of the configured server can message the bot. Guild responses remain mention-gated by default unless you opt into all-message replies. +If you run onboarding again with the same sandbox name and choose a different inference provider or model, NemoClaw detects the drift and recreates the sandbox so the running OpenClaw UI matches your selection. +In interactive mode, the wizard asks for confirmation before delete and recreate. +In non-interactive mode, NemoClaw recreates automatically when the stored selection is readable and differs; if NemoClaw cannot read the stored selection, NemoClaw reuses by default. +Set `NEMOCLAW_RECREATE_SANDBOX=1` to force recreation even when no drift is detected. + Before creating the gateway, the wizard runs preflight checks. It verifies that Docker is reachable, warns on untested runtimes such as Podman, and prints host remediation guidance when prerequisites are missing. The preflight also enforces the OpenShell version range declared in the blueprint (`min_openshell_version` and `max_openshell_version`). diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index fbc3413a655..7ea7f428304 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -964,6 +964,112 @@ function pruneStaleSandboxEntry(sandboxName) { return liveExists; } +function findSelectionConfigPath(dir) { + if (!dir || !fs.existsSync(dir)) return null; + const entries = fs.readdirSync(dir, { withFileTypes: true }); + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + const found = findSelectionConfigPath(fullPath); + if (found) return found; + continue; + } + if (entry.name === "config.json") { + return fullPath; + } + } + return null; +} + +function readSandboxSelectionConfig(sandboxName) { + if (!sandboxName) return null; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-selection-")); + try { + const result = runOpenshell( + ["sandbox", "download", sandboxName, "/sandbox/.nemoclaw/config.json", `${tmpDir}${path.sep}`], + { ignoreError: true, stdio: ["ignore", "ignore", "ignore"] }, + ); + if (result.status !== 0) return null; + const configPath = findSelectionConfigPath(tmpDir); + if (!configPath) return null; + try { + const parsed = JSON.parse(fs.readFileSync(configPath, "utf-8")); + return parsed && typeof parsed === "object" ? parsed : null; + } catch { + return null; + } + } catch { + return null; + } finally { + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch { + // ignore cleanup errors + } + } +} + +function getSelectionDrift(sandboxName, requestedProvider, requestedModel) { + const existing = readSandboxSelectionConfig(sandboxName); + if (!existing) { + return { + changed: true, + providerChanged: false, + modelChanged: false, + existingProvider: null, + existingModel: null, + unknown: true, + }; + } + + const existingProvider = typeof existing.provider === "string" ? existing.provider : null; + const existingModel = typeof existing.model === "string" ? existing.model : null; + if (!existingProvider || !existingModel) { + return { + changed: true, + providerChanged: false, + modelChanged: false, + existingProvider, + existingModel, + unknown: true, + }; + } + + const providerChanged = Boolean( + existingProvider && requestedProvider && existingProvider !== requestedProvider, + ); + const modelChanged = Boolean(existingModel && requestedModel && existingModel !== requestedModel); + + return { + changed: providerChanged || modelChanged, + providerChanged, + modelChanged, + existingProvider, + existingModel, + unknown: false, + }; +} + +async function confirmRecreateForSelectionDrift(sandboxName, drift, requestedProvider, requestedModel) { + const currentProvider = drift.existingProvider || "unknown"; + const currentModel = drift.existingModel || "unknown"; + const nextProvider = requestedProvider || "unknown"; + const nextModel = requestedModel || "unknown"; + + console.log(` Sandbox '${sandboxName}' exists but requested inference selection changed.`); + console.log(` Current: provider=${currentProvider} model=${currentModel}`); + console.log(` Requested: provider=${nextProvider} model=${nextModel}`); + console.log(" Recreating the sandbox is required to apply this change to the running OpenClaw UI."); + + if (isNonInteractive()) { + note(" [non-interactive] Recreating sandbox due to provider/model drift."); + return true; + } + + const answer = await prompt(` Recreate sandbox '${sandboxName}' now? [y/N]: `); + return isAffirmativeAnswer(answer); +} + function buildSandboxConfigSyncScript(selectionConfig) { // openclaw.json is immutable (root:root 444, Landlock read-only) — never // write to it at runtime. Model routing is handled by the host-side @@ -3285,6 +3391,8 @@ async function createSandbox( const needsProviderMigration = hasMessagingTokens && messagingTokenDefs.some(({ name, token }) => token && !providerExistsInGateway(name)); + const selectionDrift = getSelectionDrift(sandboxName, provider, model); + const confirmedSelectionDrift = selectionDrift.changed && !selectionDrift.unknown; // Detect whether any messaging credential has been rotated since the // sandbox was created. Provider credentials are resolved once at sandbox @@ -3296,37 +3404,60 @@ async function createSandbox( if (!isRecreateSandbox() && !needsProviderMigration && !credentialRotation.changed) { if (isNonInteractive()) { if (existingSandboxState === "ready") { - // Upsert messaging providers even on reuse so credential changes take - // effect without requiring a full sandbox recreation. - upsertMessagingProviders(messagingTokenDefs); - note(` [non-interactive] Sandbox '${sandboxName}' exists and is ready — reusing it`); - note(" Pass --recreate-sandbox or set NEMOCLAW_RECREATE_SANDBOX=1 to force recreation."); - ensureDashboardForward(sandboxName, chatUiUrl); - return sandboxName; + if (confirmedSelectionDrift) { + note(" [non-interactive] Recreating sandbox due to provider/model drift."); + } else { + // Upsert messaging providers even on reuse so credential changes take + // effect without requiring a full sandbox recreation. + upsertMessagingProviders(messagingTokenDefs); + if (selectionDrift.unknown) { + note( + " [non-interactive] Existing provider/model selection is unreadable; reusing sandbox.", + ); + note( + " [non-interactive] Set NEMOCLAW_RECREATE_SANDBOX=1 (or --recreate-sandbox) to force recreation.", + ); + } else { + note(` [non-interactive] Sandbox '${sandboxName}' exists and is ready — reusing it`); + note( + " Pass --recreate-sandbox or set NEMOCLAW_RECREATE_SANDBOX=1 to force recreation.", + ); + } + ensureDashboardForward(sandboxName, chatUiUrl); + return sandboxName; + } + } else { + console.error(` Sandbox '${sandboxName}' already exists but is not ready.`); + console.error(" Pass --recreate-sandbox or set NEMOCLAW_RECREATE_SANDBOX=1 to overwrite."); + process.exit(1); } - console.error(` Sandbox '${sandboxName}' already exists but is not ready.`); - console.error(" Pass --recreate-sandbox or set NEMOCLAW_RECREATE_SANDBOX=1 to overwrite."); - process.exit(1); - } - - if (existingSandboxState === "ready") { - console.log(` Sandbox '${sandboxName}' already exists.`); - console.log(" Choosing 'n' will delete the existing sandbox and create a new one."); - const answer = await promptOrDefault(" Reuse existing sandbox? [Y/n]: ", null, "y"); - const normalizedAnswer = answer.trim().toLowerCase(); - if (normalizedAnswer !== "n" && normalizedAnswer !== "no") { - upsertMessagingProviders(messagingTokenDefs); - ensureDashboardForward(sandboxName, chatUiUrl); - return sandboxName; + } else if (existingSandboxState === "ready") { + if (confirmedSelectionDrift) { + const confirmed = await confirmRecreateForSelectionDrift( + sandboxName, + selectionDrift, + provider, + model, + ); + if (!confirmed) { + console.error(" Aborted. Existing sandbox left unchanged."); + process.exit(1); + } + } else { + console.log(` Sandbox '${sandboxName}' already exists.`); + console.log(" Choosing 'n' will delete the existing sandbox and create a new one."); + const answer = await promptOrDefault(" Reuse existing sandbox? [Y/n]: ", null, "y"); + const normalizedAnswer = answer.trim().toLowerCase(); + if (normalizedAnswer !== "n" && normalizedAnswer !== "no") { + upsertMessagingProviders(messagingTokenDefs); + ensureDashboardForward(sandboxName, chatUiUrl); + return sandboxName; + } } } else { console.log(` Sandbox '${sandboxName}' exists but is not ready.`); console.log(" Selecting 'n' will abort onboarding."); - const answer = await promptOrDefault( - " Delete it and create a new one? [Y/n]: ", - null, - "y", - ); + const answer = await promptOrDefault(" Delete it and create a new one? [Y/n]: ", null, "y"); const normalizedAnswer = answer.trim().toLowerCase(); if (normalizedAnswer === "n" || normalizedAnswer === "no") { console.log(" Aborting onboarding."); @@ -3380,6 +3511,8 @@ async function createSandbox( if (needsProviderMigration) { console.log(` Sandbox '${sandboxName}' exists but messaging providers are not attached.`); console.log(" Recreating to ensure credentials flow through the provider pipeline."); + } else if (confirmedSelectionDrift) { + note(` Sandbox '${sandboxName}' exists — recreating to apply model/provider change.`); } else if (credentialRotation.changed) { // Message already printed above during backup. } else if (existingSandboxState === "ready") { diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 0cced605456..767e0370ea6 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2227,6 +2227,10 @@ const runner = require(${runnerPath}); const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); const registry = require(${registryPath}); const credentials = require(${credentialsPath}); +const childProcess = require("node:child_process"); +const { EventEmitter } = require("node:events"); +const fs = require("node:fs"); +const path = require("node:path"); const commands = []; runner.run = (command, opts = {}) => { @@ -3324,12 +3328,36 @@ const runner = require(${runnerPath}); const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); const registry = require(${registryPath}); const credentials = require(${credentialsPath}); +const childProcess = require("node:child_process"); +const { EventEmitter } = require("node:events"); +const fs = require("node:fs"); +const path = require("node:path"); const commands = []; runner.run = (command, opts = {}) => { + const commandString = Array.isArray(command) ? command.join(" ") : String(command); + if (_n(command).includes("sandbox download")) { + const parts = commandString.match(/'([^']*)'/g) || []; + const downloadDir = Array.isArray(command) + ? String(command[command.length - 1] || "") + : parts.length + ? parts[parts.length - 1].slice(1, -1) + : null; + if (downloadDir) { + fs.mkdirSync(downloadDir, { recursive: true }); + fs.writeFileSync( + path.join(downloadDir, "config.json"), + JSON.stringify({ provider: "nvidia-prod", model: "gpt-5.4" }), + ); + } + } commands.push({ command: _n(command), env: opts.env || null }); return { status: 0 }; }; +runner.runFile = (file, args = [], opts = {}) => { + commands.push({ type: "runFile", command: _n([file, ...args]), file, args, env: opts.env || null }); + return { status: 0 }; +}; runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return "my-assistant"; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; @@ -3341,6 +3369,18 @@ registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false }); // Mock prompt to return "y" (reuse) credentials.prompt = async () => "y"; +childProcess.spawn = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + commands.push({ command: args[1]?.[1] || String(args[0]), env: args[2]?.env || null }); + process.nextTick(() => { + child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); + child.emit("close", 0); + }); + return child; +}; + const { createSandbox } = require(${onboardPath}); (async () => { @@ -3395,7 +3435,7 @@ const { createSandbox } = require(${onboardPath}); ); it( - "interactive mode deletes and recreates sandbox when user declines reuse", + "interactive mode deletes and recreates sandbox when user confirms drift recreate", { timeout: 60_000 }, async () => { const repoRoot = path.join(import.meta.dirname, ".."); @@ -3421,12 +3461,34 @@ const registry = require(${registryPath}); const credentials = require(${credentialsPath}); const childProcess = require("node:child_process"); const { EventEmitter } = require("node:events"); +const fs = require("node:fs"); +const path = require("node:path"); const commands = []; runner.run = (command, opts = {}) => { + const commandString = Array.isArray(command) ? command.join(" ") : String(command); + if (_n(command).includes("sandbox download")) { + const parts = commandString.match(/'([^']*)'/g) || []; + const downloadDir = Array.isArray(command) + ? String(command[command.length - 1] || "") + : parts.length + ? parts[parts.length - 1].slice(1, -1) + : null; + if (downloadDir) { + fs.mkdirSync(downloadDir, { recursive: true }); + fs.writeFileSync( + path.join(downloadDir, "config.json"), + JSON.stringify({ provider: "openai-prod", model: "gpt-4o" }), + ); + } + } commands.push({ command: _n(command), env: opts.env || null }); return { status: 0 }; }; +runner.runFile = (file, args = [], opts = {}) => { + commands.push({ type: "runFile", command: _n([file, ...args]), file, args, env: opts.env || null }); + return { status: 0 }; +}; runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return "my-assistant"; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; @@ -3441,8 +3503,8 @@ registry.removeSandbox = () => true; const preflight = require(${JSON.stringify(path.join(repoRoot, "dist", "lib", "preflight.js"))}); preflight.checkPortAvailable = async () => ({ ok: true }); -// Mock prompt to return "n" (decline reuse) -credentials.prompt = async () => "n"; +// Mock prompt to return "y" (confirm recreate) +credentials.prompt = async () => "y"; childProcess.spawn = (...args) => { const child = new EventEmitter(); @@ -3494,16 +3556,16 @@ const { createSandbox } = require(${onboardPath}); const payload = JSON.parse(payloadLine); assert.ok( - payload.commands.some((entry) => entry.command.includes("sandbox delete")), - "should delete existing sandbox when user declines reuse", + payload.commands.some((entry) => /sandbox.*delete/.test(String(entry.command))), + "should delete existing sandbox when user confirms recreate", ); assert.ok( - payload.commands.some((entry) => entry.command.includes("sandbox create")), - "should create a new sandbox when user declines reuse", + payload.commands.some((entry) => /sandbox.*create/.test(String(entry.command))), + "should create a new sandbox when user confirms recreate", ); assert.ok( - result.stdout.includes("already exists"), - "should show 'already exists' message before prompting", + result.stdout.includes("requested inference selection changed"), + "should show drift warning before prompting", ); }, ); @@ -3635,6 +3697,34 @@ const { createSandbox } = require(${onboardPath}); assert.ok(result.stdout.includes("not ready"), "should mention sandbox is not ready"); }, ); + it("detects provider/model drift and avoids silent reuse", () => { + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"), + "utf-8", + ); + assert.match(source, /const selectionDrift = getSelectionDrift\(sandboxName, provider, model\);/); + assert.match( + source, + /const confirmedSelectionDrift = selectionDrift\.changed && !selectionDrift\.unknown;/, + ); + assert.match(source, /unknown:\s*true/); + assert.match(source, /if \(confirmedSelectionDrift\)/); + assert.match(source, /Recreating sandbox due to provider\/model drift/); + assert.match( + source, + /Sandbox '\$\{sandboxName\}' exists — recreating to apply model\/provider change\./, + ); + }); + + it("prompts before destructive recreate when drift is detected in interactive mode", () => { + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"), + "utf-8", + ); + assert.match(source, /async function confirmRecreateForSelectionDrift/); + assert.match(source, /Recreate sandbox '\$\{sandboxName\}' now\? \[y\/N\]:/); + assert.match(source, /Aborted\. Existing sandbox left unchanged\./); + }); it("upsertProvider creates a new provider and returns ok on success", () => { const repoRoot = path.join(import.meta.dirname, "..");