From 63507a34e88a1a390dbb4c36b569abdc9c58f6b1 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 11:26:15 +0000 Subject: [PATCH 1/8] fix(cli): make channels add atomic when policy preset is missing Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/policy-channel.ts | 18 +++---- test/channels-add-preset.test.ts | 58 ++++++++++++++++------- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index a92ea1ab30..3bf4ae0fd7 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -825,6 +825,9 @@ export async function addSandboxChannel( } persistChannelTokens(acquired); + if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { + process.exit(1); + } // Push to the gateway and update the registry NOW so that answering // "rebuild later" (or running non-interactively) does not silently // discard the change. Pre-fix this was safe because saveCredential() @@ -833,28 +836,19 @@ export async function addSandboxChannel( await applyChannelAddToGatewayAndRegistry(sandboxName, canonical, acquired); console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); - applyChannelPresetIfAvailable(sandboxName, canonical); - const rebuilt = await promptAndRebuild(sandboxName, `add '${canonical}'`); if (rebuilt) verifyChannelBridgeAfterRebuild(sandboxName, canonical); } -// Must run before promptAndRebuild — the rebuild's backup manifest only -// captures presets already applied (#3437). Without this, channel bridges -// boot without egress to their upstream API after rebuild. function applyChannelPresetIfAvailable(sandboxName: string, channelName: string): boolean { - const builtinPresets = new Set(policies.listPresets().map((p) => p.name)); - if (!builtinPresets.has(channelName)) { - return true; - } try { const applied = policies.applyPreset(sandboxName, channelName); if (!applied) { console.error( - ` ${YW}⚠${R} Channel '${channelName}' bridge registered but its policy preset failed to apply.`, + ` ${YW}⚠${R} Cannot enable channel '${channelName}': policy preset failed to apply.`, ); console.error( - ` Re-apply manually after rebuild with: ${CLI_NAME} ${sandboxName} policy-add ${channelName}`, + ` Restore the preset YAML and re-run: ${CLI_NAME} ${sandboxName} channels add ${channelName}`, ); return false; } @@ -864,7 +858,7 @@ function applyChannelPresetIfAvailable(sandboxName: string, channelName: string) const msg = err instanceof Error ? err.message : String(err); console.error(` ${YW}⚠${R} Failed to apply '${channelName}' policy preset: ${msg}`); console.error( - ` Re-apply manually after rebuild with: ${CLI_NAME} ${sandboxName} policy-add ${channelName}`, + ` Restore the preset YAML and re-run: ${CLI_NAME} ${sandboxName} channels add ${channelName}`, ); return false; } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 82d8a78175..cbca4d565e 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -337,24 +337,39 @@ process.exit = (code) => { ); }); - // Negative: when the channel name does not match any built-in preset, - // the helper short-circuits via listPresets() and applyPreset is not - // invoked at all. This guards against a future channel name that happens - // to collide with no preset (or a typo) from spamming "Cannot load preset" - // errors out of policies.applyPreset. - it("skips applyPreset when no matching built-in preset exists", () => { - const script = `${buildPreamble({ presetNamesAvailable: ["npm", "github"] })} + // Regression for #4548: a missing preset YAML (applyPreset returns false + // because loadPresetForSandbox cannot find the file) must abort the + // non-QR add flow BEFORE the registry write, BEFORE the gateway provider + // upsert, and BEFORE the rebuild prompt. Previously the channel name + // landed in messagingChannels and the sandbox was rebuilt without the + // preset, leaving the bridge advertised but not policed. + it("aborts non-QR channel before registry and rebuild when preset apply fails (issue #4548)", () => { + const script = `${buildPreamble({ applyPresetResult: false })} const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; (async () => { try { await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); - process.stdout.write("\\n__RESULT__" + JSON.stringify({ - appliedCalls: ctx.appliedCalls, - callOrder: ctx.callOrder, - }) + "\\n"); } catch (err) { - process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + exitCodes, + }) + "\\n"); })(); `; const result = runScript(script); @@ -364,16 +379,25 @@ const ctx = module.exports; const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + assert.deepEqual(payload.exitCodes, [1]); assert.deepEqual( payload.appliedCalls, + [{ sandboxName: "test-sb", presetName: "telegram" }], + `expected one failed applyPreset call; got ${JSON.stringify(payload.appliedCalls)}`, + ); + assert.deepEqual( + payload.providerCalls, + [], + `preset failure must not register host-side providers; got ${JSON.stringify(payload.providerCalls)}`, + ); + assert.deepEqual( + payload.registryUpdates, [], - `expected applyPreset NOT to be called when no built-in preset matches; got ${JSON.stringify(payload.appliedCalls)}`, + `preset failure must not register telegram in messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, ); - // Rebuild should still be triggered — channel registration succeeded, - // only the preset path was skipped. assert.ok( - payload.callOrder.includes("promptAndRebuild"), - `expected promptAndRebuild to still run; got order: ${JSON.stringify(payload.callOrder)}`, + !payload.callOrder.includes("promptAndRebuild"), + `preset failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); }); }); From 1f9359a2c7d958231f4d2c6d1981d1f4fa190e48 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 11:56:04 +0000 Subject: [PATCH 2/8] fix(cli): probe preset YAML before tokens; doc behaviour update Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 4 ++-- docs/reference/commands.mdx | 2 +- src/lib/actions/sandbox/policy-channel.ts | 11 +++++++--- test/channels-add-preset.test.ts | 23 +++++++++----------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index acb9d3aa06..c76c65b2ad 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -164,8 +164,8 @@ $ nemoclaw my-assistant channels add whatsapp It prompts for Telegram, Discord, and Slack tokens, runs an interactive host-side QR scan for WeChat, and collects nothing for WhatsApp because pairing happens in-sandbox after rebuild. It registers bridge providers with the OpenShell gateway when tokens were captured, records the channel in the sandbox registry, and asks whether to rebuild immediately. The command accepts mixed-case input such as `Telegram`, then stores and prints the canonical lowercase channel name. -If a matching built-in network policy preset exists, `channels add` applies it to the sandbox automatically before the rebuild so the bridge has egress to its upstream API. -If applying the preset fails, NemoClaw warns and tells you to re-apply manually with `nemoclaw policy-add ` after the rebuild. +`channels add` requires the matching built-in network policy preset YAML to be present; if the file is missing the command exits with a non-zero status before any token prompt, registry write, or rebuild prompt, so the sandbox is never left advertising a channel without a matching network policy. +If the file is present, `channels add` applies the preset to the sandbox automatically before the rebuild so the bridge has egress to its upstream API; if the apply itself fails, the command exits with a non-zero status without prompting for rebuild — restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. For Telegram, Discord, and Slack, `channels add` also checks the rebuilt runtime for the selected bridge and reports startup, credential, or missing-plugin warnings before returning. If you need optional channel settings such as `TELEGRAM_ALLOWED_IDS`, `TELEGRAM_REQUIRE_MENTION`, `DISCORD_SERVER_ID`, `DISCORD_USER_ID`, `DISCORD_REQUIRE_MENTION`, `SLACK_ALLOWED_USERS`, or `SLACK_ALLOWED_CHANNELS`, export them before the rebuild starts. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d1a45c5a4e..247d4e8479 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -723,7 +723,7 @@ Channels fall into three login modes: After registering the channel, NemoClaw asks whether to rebuild immediately. Running `add` for an already-configured channel simply overwrites the stored credentials where applicable — the operation is idempotent. Channel names are trimmed and lowercased before NemoClaw stores credentials, names bridge providers, or prints rebuild messages. -If a matching built-in network policy preset exists, NemoClaw applies it to the sandbox before the rebuild so the bridge has egress to its upstream API; if applying the preset fails, NemoClaw warns and tells you to re-apply manually with `nemoclaw policy-add `. +NemoClaw requires the matching built-in network policy preset YAML to be present; if the file is missing `channels add` exits with a non-zero status before any token prompt, registry write, or rebuild prompt. When the file is present, NemoClaw applies the preset to the sandbox before the rebuild so the bridge has egress to its upstream API; if the apply itself fails, the command exits with a non-zero status without prompting for rebuild — restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. ```console diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 3bf4ae0fd7..d7320412b2 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -793,6 +793,10 @@ export async function addSandboxChannel( return; } + if (policies.loadPreset(canonical) === null) { + process.exit(1); + } + // QR-paired channels that own their session inside the sandbox have no // host-side credential to acquire; register the bridge now and let the // operator complete pairing after rebuild. @@ -825,9 +829,6 @@ export async function addSandboxChannel( } persistChannelTokens(acquired); - if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { - process.exit(1); - } // Push to the gateway and update the registry NOW so that answering // "rebuild later" (or running non-interactively) does not silently // discard the change. Pre-fix this was safe because saveCredential() @@ -836,6 +837,10 @@ export async function addSandboxChannel( await applyChannelAddToGatewayAndRegistry(sandboxName, canonical, acquired); console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); + if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { + process.exit(1); + } + const rebuilt = await promptAndRebuild(sandboxName, `add '${canonical}'`); if (rebuilt) verifyChannelBridgeAfterRebuild(sandboxName, canonical); } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index cbca4d565e..7cc9bfad86 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -55,6 +55,7 @@ function buildPreamble({ sessionLoadThrows = false, sessionUpdateThrows = false, sessionMissing = false, + presetFileMissing = false, }: { presetNamesAvailable?: string[]; applyPresetResult?: boolean; @@ -65,6 +66,7 @@ function buildPreamble({ sessionLoadThrows?: boolean; sessionUpdateThrows?: boolean; sessionMissing?: boolean; + presetFileMissing?: boolean; } = {}): string { const j = (p: string) => JSON.stringify(path.join(repoRoot, "dist", "lib", p)); return String.raw` @@ -117,6 +119,7 @@ const appliedCalls = []; const removedCalls = []; const callOrder = []; policies.listPresets = () => ${JSON.stringify(presetNamesAvailable.map((name) => ({ name })))}; +policies.loadPreset = (name) => ${JSON.stringify(presetFileMissing)} ? null : "network_policies:\n " + name + ":\n egress:\n - host: example.com"; policies.applyPreset = (sandboxName, presetName) => { appliedCalls.push({ sandboxName, presetName }); callOrder.push("applyPreset:" + presetName); @@ -337,14 +340,8 @@ process.exit = (code) => { ); }); - // Regression for #4548: a missing preset YAML (applyPreset returns false - // because loadPresetForSandbox cannot find the file) must abort the - // non-QR add flow BEFORE the registry write, BEFORE the gateway provider - // upsert, and BEFORE the rebuild prompt. Previously the channel name - // landed in messagingChannels and the sandbox was rebuilt without the - // preset, leaving the bridge advertised but not policed. - it("aborts non-QR channel before registry and rebuild when preset apply fails (issue #4548)", () => { - const script = `${buildPreamble({ applyPresetResult: false })} + it("aborts non-QR channel when policy preset YAML is missing", () => { + const script = `${buildPreamble({ presetFileMissing: true })} const ctx = module.exports; const exitCodes = []; const originalExit = process.exit; @@ -382,22 +379,22 @@ process.exit = (code) => { assert.deepEqual(payload.exitCodes, [1]); assert.deepEqual( payload.appliedCalls, - [{ sandboxName: "test-sb", presetName: "telegram" }], - `expected one failed applyPreset call; got ${JSON.stringify(payload.appliedCalls)}`, + [], + `missing preset YAML must abort before applyPreset; got ${JSON.stringify(payload.appliedCalls)}`, ); assert.deepEqual( payload.providerCalls, [], - `preset failure must not register host-side providers; got ${JSON.stringify(payload.providerCalls)}`, + `missing preset YAML must not register host-side providers; got ${JSON.stringify(payload.providerCalls)}`, ); assert.deepEqual( payload.registryUpdates, [], - `preset failure must not register telegram in messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, + `missing preset YAML must not register telegram in messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, ); assert.ok( !payload.callOrder.includes("promptAndRebuild"), - `preset failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + `missing preset YAML must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); }); }); From 2f7f685579b215710cacec721745ccd06c5d1bbd Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 12:24:26 +0000 Subject: [PATCH 3/8] fix(cli): roll back gateway and registry on preset apply failure; doc style Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 7 +- docs/reference/commands.mdx | 6 +- src/lib/actions/sandbox/policy-channel.ts | 9 ++ test/channels-add-preset.test.ts | 94 +++++++++++++++++++- 4 files changed, 110 insertions(+), 6 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index c76c65b2ad..438373639c 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -164,8 +164,11 @@ $ nemoclaw my-assistant channels add whatsapp It prompts for Telegram, Discord, and Slack tokens, runs an interactive host-side QR scan for WeChat, and collects nothing for WhatsApp because pairing happens in-sandbox after rebuild. It registers bridge providers with the OpenShell gateway when tokens were captured, records the channel in the sandbox registry, and asks whether to rebuild immediately. The command accepts mixed-case input such as `Telegram`, then stores and prints the canonical lowercase channel name. -`channels add` requires the matching built-in network policy preset YAML to be present; if the file is missing the command exits with a non-zero status before any token prompt, registry write, or rebuild prompt, so the sandbox is never left advertising a channel without a matching network policy. -If the file is present, `channels add` applies the preset to the sandbox automatically before the rebuild so the bridge has egress to its upstream API; if the apply itself fails, the command exits with a non-zero status without prompting for rebuild — restore the preset YAML and re-run `nemoclaw channels add `. +`channels add` requires the matching built-in network policy preset YAML to be present. +If the file is missing, the command exits with a non-zero status before any token prompt, registry write, or rebuild prompt, so the sandbox is never left advertising a channel without a matching network policy. +If the file is present, `channels add` applies the preset to the sandbox automatically before the rebuild so the bridge has egress to its upstream API. +If the apply itself fails, the command exits with a non-zero status without prompting for rebuild. +Restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. For Telegram, Discord, and Slack, `channels add` also checks the rebuilt runtime for the selected bridge and reports startup, credential, or missing-plugin warnings before returning. If you need optional channel settings such as `TELEGRAM_ALLOWED_IDS`, `TELEGRAM_REQUIRE_MENTION`, `DISCORD_SERVER_ID`, `DISCORD_USER_ID`, `DISCORD_REQUIRE_MENTION`, `SLACK_ALLOWED_USERS`, or `SLACK_ALLOWED_CHANNELS`, export them before the rebuild starts. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 247d4e8479..d62723aefb 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -723,7 +723,11 @@ Channels fall into three login modes: After registering the channel, NemoClaw asks whether to rebuild immediately. Running `add` for an already-configured channel simply overwrites the stored credentials where applicable — the operation is idempotent. Channel names are trimmed and lowercased before NemoClaw stores credentials, names bridge providers, or prints rebuild messages. -NemoClaw requires the matching built-in network policy preset YAML to be present; if the file is missing `channels add` exits with a non-zero status before any token prompt, registry write, or rebuild prompt. When the file is present, NemoClaw applies the preset to the sandbox before the rebuild so the bridge has egress to its upstream API; if the apply itself fails, the command exits with a non-zero status without prompting for rebuild — restore the preset YAML and re-run `nemoclaw channels add `. +NemoClaw requires the matching built-in network policy preset YAML to be present. +If the file is missing, `channels add` exits with a non-zero status before any token prompt, registry write, or rebuild prompt. +When the file is present, NemoClaw applies the preset to the sandbox before the rebuild so the bridge has egress to its upstream API. +If the apply itself fails, the command exits with a non-zero status without prompting for rebuild. +Restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. ```console diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index d7320412b2..0a4b8e2bb6 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -838,6 +838,15 @@ export async function addSandboxChannel( console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { + console.error( + ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, + ); + await applyChannelRemoveToGatewayAndRegistry( + sandboxName, + canonical, + getChannelTokenKeys(channel), + ); + clearChannelTokens(channel); process.exit(1); } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 7cc9bfad86..3bf55e9e14 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -88,9 +88,11 @@ const gatewayRuntime = require(${j("gateway-runtime-action.js")}); gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ recovered: true }); const credentials = require(${j("credentials/store.js")}); +const savedCredentialKeys = []; +const deletedCredentialKeys = []; credentials.getCredential = (key) => process.env[key] || null; -credentials.saveCredential = () => true; -credentials.deleteCredential = () => true; +credentials.saveCredential = (key) => { savedCredentialKeys.push(key); return true; }; +credentials.deleteCredential = (key) => { deletedCredentialKeys.push(key); return true; }; credentials.prompt = async (msg) => { throw new Error("unexpected prompt: " + msg); }; const onboard = require(${j("onboard.js")}); @@ -180,7 +182,7 @@ console.log = (...args) => { const channelModule = require(${j("actions/sandbox/policy-channel.js")}); -module.exports = { channelModule, appliedCalls, removedCalls, callOrder, providerCalls, registryUpdates, sessionUpdates, getSessionState: () => sessionState }; +module.exports = { channelModule, appliedCalls, removedCalls, callOrder, providerCalls, registryUpdates, sessionUpdates, savedCredentialKeys, deletedCredentialKeys, getSessionState: () => sessionState }; `; } @@ -397,6 +399,73 @@ process.exit = (code) => { `missing preset YAML must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); }); + + it("rolls back providers, registry, and credentials when applyPreset fails after a successful loadPreset", () => { + const script = `${buildPreamble({ applyPresetResult: false })} +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + savedCredentialKeys: ctx.savedCredentialKeys, + deletedCredentialKeys: ctx.deletedCredentialKeys, + sessionUpdates: ctx.sessionUpdates, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + assert.ok(marker >= 0, `no __RESULT__ marker in stdout:\n${result.stdout}`); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual( + payload.appliedCalls, + [{ sandboxName: "test-sb", presetName: "telegram" }], + `expected one failed applyPreset call; got ${JSON.stringify(payload.appliedCalls)}`, + ); + assert.ok( + payload.registryUpdates.length === 2, + `expected one add update and one rollback update; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.deepEqual(payload.registryUpdates[0].updates.messagingChannels, ["telegram"]); + assert.deepEqual(payload.registryUpdates[1].updates.messagingChannels, []); + assert.deepEqual( + payload.deletedCredentialKeys, + ["TELEGRAM_BOT_TOKEN"], + `expected rollback to clear persisted credentials; got ${JSON.stringify(payload.deletedCredentialKeys)}`, + ); + assert.deepEqual( + payload.sessionUpdates, + [], + `applyPreset returned false before syncSessionPolicyPresetsWithRegistry; session must stay untouched; got ${JSON.stringify(payload.sessionUpdates)}`, + ); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `apply failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + }); }); // Regression: `channels add` was updating the registry but NOT @@ -973,3 +1042,22 @@ global.__testLog = ""; ); }); }); + +describe("channel preset source-of-truth", () => { + it("every channel registered in KNOWN_CHANNELS ships a matching preset YAML on disk", () => { + const { knownChannelNames } = require(path.join(repoRoot, "dist", "lib", "sandbox", "channels.js")) as { + knownChannelNames: () => string[]; + }; + const presetDir = path.join(repoRoot, "nemoclaw-blueprint", "policies", "presets"); + const missing: string[] = []; + for (const name of knownChannelNames()) { + const file = path.join(presetDir, `${name}.yaml`); + if (!fs.existsSync(file)) missing.push(file); + } + assert.deepEqual( + missing, + [], + `every channel in KNOWN_CHANNELS must have a matching preset YAML; missing: ${missing.join(", ")}`, + ); + }); +}); From 0c0a4a93cf875f8822c150d50faa9f9aa4494ae6 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 12:44:28 +0000 Subject: [PATCH 4/8] fix(cli): validate parsed preset content and clear creds before gateway rollback Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 6 +- docs/reference/commands.mdx | 6 +- src/lib/actions/sandbox/policy-channel.ts | 15 ++++- test/channels-add-preset.test.ts | 69 +++++++++++++++++++- 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index 438373639c..477da38e1c 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -165,9 +165,9 @@ It prompts for Telegram, Discord, and Slack tokens, runs an interactive host-sid It registers bridge providers with the OpenShell gateway when tokens were captured, records the channel in the sandbox registry, and asks whether to rebuild immediately. The command accepts mixed-case input such as `Telegram`, then stores and prints the canonical lowercase channel name. `channels add` requires the matching built-in network policy preset YAML to be present. -If the file is missing, the command exits with a non-zero status before any token prompt, registry write, or rebuild prompt, so the sandbox is never left advertising a channel without a matching network policy. -If the file is present, `channels add` applies the preset to the sandbox automatically before the rebuild so the bridge has egress to its upstream API. -If the apply itself fails, the command exits with a non-zero status without prompting for rebuild. +A missing or malformed preset YAML (no `network_policies:` section) aborts the command before any token prompt, registry write, or rebuild prompt, so the sandbox never advertises a channel without a matching network policy. +With the preset file in place, `channels add` applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. +When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for rebuild. Restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. For Telegram, Discord, and Slack, `channels add` also checks the rebuilt runtime for the selected bridge and reports startup, credential, or missing-plugin warnings before returning. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d62723aefb..01d26ef913 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -724,9 +724,9 @@ After registering the channel, NemoClaw asks whether to rebuild immediately. Running `add` for an already-configured channel simply overwrites the stored credentials where applicable — the operation is idempotent. Channel names are trimmed and lowercased before NemoClaw stores credentials, names bridge providers, or prints rebuild messages. NemoClaw requires the matching built-in network policy preset YAML to be present. -If the file is missing, `channels add` exits with a non-zero status before any token prompt, registry write, or rebuild prompt. -When the file is present, NemoClaw applies the preset to the sandbox before the rebuild so the bridge has egress to its upstream API. -If the apply itself fails, the command exits with a non-zero status without prompting for rebuild. +A missing or malformed preset YAML (no `network_policies:` section) aborts `channels add` before any token prompt, registry write, or rebuild prompt. +With the preset file in place, NemoClaw applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. +When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for rebuild. Restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 0a4b8e2bb6..21db528a93 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -793,7 +793,18 @@ export async function addSandboxChannel( return; } - if (policies.loadPreset(canonical) === null) { + const presetContent = policies.loadPreset(canonical); + const presetEntries = + presetContent === null ? null : policies.extractPresetEntries(presetContent); + if (presetContent === null || presetEntries === null) { + if (presetContent !== null && presetEntries === null) { + console.error( + ` Preset YAML for channel '${canonical}' is missing a 'network_policies:' section.`, + ); + } + console.error( + ` Restore the preset YAML and re-run: ${CLI_NAME} ${sandboxName} channels add ${canonical}`, + ); process.exit(1); } @@ -841,12 +852,12 @@ export async function addSandboxChannel( console.error( ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, ); + clearChannelTokens(channel); await applyChannelRemoveToGatewayAndRegistry( sandboxName, canonical, getChannelTokenKeys(channel), ); - clearChannelTokens(channel); process.exit(1); } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 3bf55e9e14..43d3808daf 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -56,6 +56,7 @@ function buildPreamble({ sessionUpdateThrows = false, sessionMissing = false, presetFileMissing = false, + presetMissingNetworkPolicies = false, }: { presetNamesAvailable?: string[]; applyPresetResult?: boolean; @@ -67,6 +68,7 @@ function buildPreamble({ sessionUpdateThrows?: boolean; sessionMissing?: boolean; presetFileMissing?: boolean; + presetMissingNetworkPolicies?: boolean; } = {}): string { const j = (p: string) => JSON.stringify(path.join(repoRoot, "dist", "lib", p)); return String.raw` @@ -121,7 +123,11 @@ const appliedCalls = []; const removedCalls = []; const callOrder = []; policies.listPresets = () => ${JSON.stringify(presetNamesAvailable.map((name) => ({ name })))}; -policies.loadPreset = (name) => ${JSON.stringify(presetFileMissing)} ? null : "network_policies:\n " + name + ":\n egress:\n - host: example.com"; +policies.loadPreset = (name) => { + if (${JSON.stringify(presetFileMissing)}) return null; + if (${JSON.stringify(presetMissingNetworkPolicies)}) return "name: " + name + "\ndescription: \"stub preset without network_policies\"\n"; + return "network_policies:\n " + name + ":\n egress:\n - host: example.com"; +}; policies.applyPreset = (sandboxName, presetName) => { appliedCalls.push({ sandboxName, presetName }); callOrder.push("applyPreset:" + presetName); @@ -398,6 +404,67 @@ process.exit = (code) => { !payload.callOrder.includes("promptAndRebuild"), `missing preset YAML must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); + assert.ok( + result.stderr.includes(`Restore the preset YAML and re-run: nemoclaw test-sb channels add telegram`), + `expected restore-and-re-run hint on stderr; got:\n${result.stderr}`, + ); + }); + + it("aborts non-QR channel when policy preset YAML has no network_policies section", () => { + const script = `${buildPreamble({ presetMissingNetworkPolicies: true })} +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + savedCredentialKeys: ctx.savedCredentialKeys, + deletedCredentialKeys: ctx.deletedCredentialKeys, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.appliedCalls, []); + assert.deepEqual(payload.providerCalls, []); + assert.deepEqual(payload.registryUpdates, []); + assert.deepEqual(payload.savedCredentialKeys, []); + assert.deepEqual(payload.deletedCredentialKeys, []); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `invalid preset must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + assert.ok( + result.stderr.includes("missing a 'network_policies:' section"), + `expected diagnostic about missing network_policies section; got:\n${result.stderr}`, + ); + assert.ok( + result.stderr.includes("Restore the preset YAML and re-run: nemoclaw test-sb channels add telegram"), + `expected restore-and-re-run hint on stderr; got:\n${result.stderr}`, + ); }); it("rolls back providers, registry, and credentials when applyPreset fails after a successful loadPreset", () => { From 3416e2a750e9849b20362b5e5db4cd43d38994d3 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 13:06:24 +0000 Subject: [PATCH 5/8] fix(cli): make channels add rollback best-effort across gateway failures Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/policy-channel.ts | 111 +++++++++++++--------- test/channels-add-preset.test.ts | 76 +++++++++++++++ 2 files changed, 144 insertions(+), 43 deletions(-) diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 21db528a93..62d2cd8705 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -366,7 +366,12 @@ async function applyChannelRemoveToGatewayAndRegistry( sandboxName: string, channelName: string, channelTokenKeys: string[], -): Promise { + options: { bestEffort?: boolean } = {}, +): Promise<{ ok: boolean; residual: string[] }> { + const bestEffort = Boolean(options.bestEffort); + const residual: string[] = []; + let gatewayReachable = true; + if (channelTokenKeys.length > 0) { const recovery = await recoverNamedGatewayRuntime(); if (!recovery.recovered) { @@ -376,7 +381,9 @@ async function applyChannelRemoveToGatewayAndRegistry( console.error( " Re-run after starting the gateway, or run 'openshell gateway start --name nemoclaw'.", ); - process.exit(1); + if (!bestEffort) process.exit(1); + gatewayReachable = false; + residual.push("gateway-providers"); } } @@ -389,28 +396,33 @@ async function applyChannelRemoveToGatewayAndRegistry( // previous run may have already detached, or the channel may have been // configured for a sandbox that is no longer alive. const detachFailures: Array<{ name: string; output: string }> = []; - for (const envKey of channelTokenKeys) { - const name = bridgeProviderName(sandboxName, channelName, envKey); - const result = runOpenshell(["sandbox", "provider", "detach", sandboxName, name], { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - }); - if (result.status !== 0) { - const output = `${result.stdout || ""}${result.stderr || ""}`; - if (!/\bNotFound\b|not found|not attached/i.test(output)) { - detachFailures.push({ name, output: output.trim() }); + if (gatewayReachable) { + for (const envKey of channelTokenKeys) { + const name = bridgeProviderName(sandboxName, channelName, envKey); + const result = runOpenshell(["sandbox", "provider", "detach", sandboxName, name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + const output = `${result.stdout || ""}${result.stderr || ""}`; + if (!/\bNotFound\b|not found|not attached/i.test(output)) { + detachFailures.push({ name, output: output.trim() }); + } } } - } - if (detachFailures.length > 0) { - console.error( - ` Failed to detach bridge provider(s) from sandbox '${sandboxName}': ${detachFailures.map((f) => f.name).join(", ")}.`, - ); - for (const f of detachFailures) { - console.error(` [${f.name}] ${f.output.split("\n").join("\n ")}`); + if (detachFailures.length > 0) { + console.error( + ` Failed to detach bridge provider(s) from sandbox '${sandboxName}': ${detachFailures.map((f) => f.name).join(", ")}.`, + ); + for (const f of detachFailures) { + console.error(` [${f.name}] ${f.output.split("\n").join("\n ")}`); + } + if (!bestEffort) { + console.error(" Registry not updated; re-run after resolving the gateway error."); + process.exit(1); + } + if (!residual.includes("gateway-providers")) residual.push("gateway-providers"); } - console.error(" Registry not updated; re-run after resolving the gateway error."); - process.exit(1); } // Capture each delete's outcome. If any non-NotFound failure surfaces @@ -420,30 +432,35 @@ async function applyChannelRemoveToGatewayAndRegistry( // can't easily recover. Surface the underlying openshell output so the // operator can see exactly why the delete was rejected. const deleteFailures: Array<{ name: string; output: string }> = []; - for (const envKey of channelTokenKeys) { - const name = bridgeProviderName(sandboxName, channelName, envKey); - const result = runOpenshell(["provider", "delete", name], { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - }); - if (result.status !== 0) { - const output = `${result.stdout || ""}${result.stderr || ""}`; - // Treat "not found" as success-equivalent — a previous run may - // have already deleted the provider. - if (!/\bNotFound\b|not found/i.test(output)) { - deleteFailures.push({ name, output: output.trim() }); + if (gatewayReachable) { + const detachFailedSet = new Set(detachFailures.map((f) => f.name)); + for (const envKey of channelTokenKeys) { + const name = bridgeProviderName(sandboxName, channelName, envKey); + if (!bestEffort && detachFailedSet.has(name)) continue; + const result = runOpenshell(["provider", "delete", name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + const output = `${result.stdout || ""}${result.stderr || ""}`; + if (!/\bNotFound\b|not found/i.test(output)) { + deleteFailures.push({ name, output: output.trim() }); + } } } - } - if (deleteFailures.length > 0) { - console.error( - ` Failed to delete bridge provider(s) from the OpenShell gateway: ${deleteFailures.map((f) => f.name).join(", ")}.`, - ); - for (const f of deleteFailures) { - console.error(` [${f.name}] ${f.output.split("\n").join("\n ")}`); + if (deleteFailures.length > 0) { + console.error( + ` Failed to delete bridge provider(s) from the OpenShell gateway: ${deleteFailures.map((f) => f.name).join(", ")}.`, + ); + for (const f of deleteFailures) { + console.error(` [${f.name}] ${f.output.split("\n").join("\n ")}`); + } + if (!bestEffort) { + console.error(" Registry not updated; re-run after resolving the gateway error."); + process.exit(1); + } + if (!residual.includes("gateway-providers")) residual.push("gateway-providers"); } - console.error(" Registry not updated; re-run after resolving the gateway error."); - process.exit(1); } const entry = registry.getSandbox(sandboxName); @@ -459,6 +476,8 @@ async function applyChannelRemoveToGatewayAndRegistry( Object.keys(providerCredentialHashes).length > 0 ? providerCredentialHashes : undefined, }); } + + return { ok: residual.length === 0, residual }; } async function promptAndRebuild(sandboxName: string, actionDesc: string): Promise { @@ -853,11 +872,17 @@ export async function addSandboxChannel( ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, ); clearChannelTokens(channel); - await applyChannelRemoveToGatewayAndRegistry( + const rollback = await applyChannelRemoveToGatewayAndRegistry( sandboxName, canonical, getChannelTokenKeys(channel), + { bestEffort: true }, ); + if (!rollback.ok) { + console.error( + ` ${YW}⚠${R} Rollback could not fully clean ${rollback.residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, + ); + } process.exit(1); } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 43d3808daf..7dd1af5103 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -533,6 +533,82 @@ process.exit = (code) => { `apply failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); }); + + it("completes rollback registry update and reports residual gateway state when openshell detach fails", () => { + const script = `${buildPreamble({ applyPresetResult: false })} +openshellRuntime.runOpenshell = (args) => { + if (Array.isArray(args) && args[0] === "sandbox" && args[1] === "provider" && args[2] === "detach") { + return { status: 1, stdout: "", stderr: "permission denied" }; + } + return { status: 0, stdout: "", stderr: "" }; +}; +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +const stderrChunks = []; +const originalConsoleError = console.error; +console.error = (...args) => { + stderrChunks.push(args.map((a) => (typeof a === "string" ? a : JSON.stringify(a))).join(" ") + "\\n"); + originalConsoleError.apply(console, args); +}; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + console.error = originalConsoleError; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + deletedCredentialKeys: ctx.deletedCredentialKeys, + exitCodes, + stderrCombined: stderrChunks.join(""), + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.appliedCalls, [{ sandboxName: "test-sb", presetName: "telegram" }]); + assert.ok( + payload.registryUpdates.length === 2, + `expected registry add + rollback even when openshell detach fails; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.deepEqual(payload.registryUpdates[1].updates.messagingChannels, []); + assert.deepEqual( + payload.deletedCredentialKeys, + ["TELEGRAM_BOT_TOKEN"], + `expected local credentials cleared before gateway rollback; got ${JSON.stringify(payload.deletedCredentialKeys)}`, + ); + assert.ok( + payload.stderrCombined.includes("Rollback could not fully clean gateway-providers"), + `expected residual-state warning on stderr; got:\n${payload.stderrCombined}`, + ); + assert.ok( + payload.stderrCombined.includes(`'nemoclaw test-sb channels remove telegram'`), + `expected manual cleanup hint on stderr; got:\n${payload.stderrCombined}`, + ); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `rollback path must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + }); }); // Regression: `channels add` was updating the registry but NOT From d7b64028e6b30ea7684fb7240b540a04995920c1 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 13:52:32 +0000 Subject: [PATCH 6/8] fix(cli): parse-validate preset YAML and restore prior state on failed re-add Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 2 +- docs/reference/commands.mdx | 2 +- src/lib/actions/sandbox/policy-channel.ts | 98 ++++++++-- src/lib/policy/index.ts | 1 + test/channels-add-preset.test.ts | 188 ++++++++++++++++++- 5 files changed, 272 insertions(+), 19 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index 477da38e1c..688aeed21a 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -167,7 +167,7 @@ The command accepts mixed-case input such as `Telegram`, then stores and prints `channels add` requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts the command before any token prompt, registry write, or rebuild prompt, so the sandbox never advertises a channel without a matching network policy. With the preset file in place, `channels add` applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for rebuild. +When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. Restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. For Telegram, Discord, and Slack, `channels add` also checks the rebuilt runtime for the selected bridge and reports startup, credential, or missing-plugin warnings before returning. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 01d26ef913..811c2b10fc 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -726,7 +726,7 @@ Channel names are trimmed and lowercased before NemoClaw stores credentials, nam NemoClaw requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts `channels add` before any token prompt, registry write, or rebuild prompt. With the preset file in place, NemoClaw applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for rebuild. +When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. Restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 62d2cd8705..a459592131 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -813,12 +813,12 @@ export async function addSandboxChannel( } const presetContent = policies.loadPreset(canonical); - const presetEntries = - presetContent === null ? null : policies.extractPresetEntries(presetContent); - if (presetContent === null || presetEntries === null) { - if (presetContent !== null && presetEntries === null) { + const presetPolicyKeys = + presetContent === null ? [] : policies.parsePresetPolicyKeys(presetContent); + if (presetContent === null || presetPolicyKeys.length === 0) { + if (presetContent !== null && presetPolicyKeys.length === 0) { console.error( - ` Preset YAML for channel '${canonical}' is missing a 'network_policies:' section.`, + ` Preset YAML for channel '${canonical}' has no parseable entries under 'network_policies:'.`, ); } console.error( @@ -858,6 +858,21 @@ export async function addSandboxChannel( await acquirePasteTokens(canonical, channel, acquired); } + const priorEntry = registry.getSandbox(sandboxName); + const priorMessagingChannels: string[] = priorEntry?.messagingChannels + ? [...priorEntry.messagingChannels] + : []; + const wasAlreadyEnabled = priorMessagingChannels.includes(canonical); + const priorHashes: Record = { + ...((priorEntry?.providerCredentialHashes as Record) || {}), + }; + const channelTokenKeys = getChannelTokenKeys(channel); + const priorCreds: Record = {}; + for (const key of channelTokenKeys) { + const existing = getCredential(key); + if (existing != null) priorCreds[key] = existing; + } + persistChannelTokens(acquired); // Push to the gateway and update the registry NOW so that answering // "rebuild later" (or running non-interactively) does not silently @@ -868,16 +883,12 @@ export async function addSandboxChannel( console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { - console.error( - ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, - ); - clearChannelTokens(channel); - const rollback = await applyChannelRemoveToGatewayAndRegistry( - sandboxName, - canonical, - getChannelTokenKeys(channel), - { bestEffort: true }, - ); + const rollback = await rollbackChannelAdd(sandboxName, channel, canonical, { + wasAlreadyEnabled, + priorMessagingChannels, + priorHashes, + priorCreds, + }); if (!rollback.ok) { console.error( ` ${YW}⚠${R} Rollback could not fully clean ${rollback.residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, @@ -890,6 +901,63 @@ export async function addSandboxChannel( if (rebuilt) verifyChannelBridgeAfterRebuild(sandboxName, canonical); } +async function rollbackChannelAdd( + sandboxName: string, + channel: ChannelDef, + canonical: string, + snapshot: { + wasAlreadyEnabled: boolean; + priorMessagingChannels: string[]; + priorHashes: Record; + priorCreds: Record; + }, +): Promise<{ ok: boolean; residual: string[] }> { + if (snapshot.wasAlreadyEnabled) { + console.error( + ` ${YW}⚠${R} Restoring prior '${canonical}' configuration; new token rotation aborted.`, + ); + clearChannelTokens(channel); + if (Object.keys(snapshot.priorCreds).length > 0) { + persistChannelTokens(snapshot.priorCreds); + } + const residual: string[] = []; + if (Object.keys(snapshot.priorCreds).length > 0) { + try { + const priorTokenDefs = Object.entries(snapshot.priorCreds).map(([envKey, token]) => ({ + name: bridgeProviderName(sandboxName, canonical, envKey), + envKey, + token, + })); + onboardProviders.upsertMessagingProviders(priorTokenDefs, runOpenshell); + } catch (err) { + console.error( + ` ${YW}⚠${R} Failed to restore gateway providers for '${canonical}': ${ + err instanceof Error ? err.message : String(err) + }`, + ); + residual.push("gateway-providers"); + } + } + registry.updateSandbox(sandboxName, { + messagingChannels: snapshot.priorMessagingChannels, + providerCredentialHashes: + Object.keys(snapshot.priorHashes).length > 0 ? snapshot.priorHashes : undefined, + }); + return { ok: residual.length === 0, residual }; + } + + console.error( + ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, + ); + clearChannelTokens(channel); + return applyChannelRemoveToGatewayAndRegistry( + sandboxName, + canonical, + getChannelTokenKeys(channel), + { bestEffort: true }, + ); +} + function applyChannelPresetIfAvailable(sandboxName: string, channelName: string): boolean { try { const applied = policies.applyPreset(sandboxName, channelName); diff --git a/src/lib/policy/index.ts b/src/lib/policy/index.ts index e7218398de..9a6ad9865f 100644 --- a/src/lib/policy/index.ts +++ b/src/lib/policy/index.ts @@ -1283,6 +1283,7 @@ export { listSetupPolicyPresets, clampSetupPolicyPresetNames, extractPresetEntries, + parsePresetPolicyKeys, parseCurrentPolicy, buildPolicySetCommand, buildPolicyGetCommand, diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 7dd1af5103..60489c9c84 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -57,6 +57,7 @@ function buildPreamble({ sessionMissing = false, presetFileMissing = false, presetMissingNetworkPolicies = false, + presetMalformedYaml = false, }: { presetNamesAvailable?: string[]; applyPresetResult?: boolean; @@ -69,6 +70,7 @@ function buildPreamble({ sessionMissing?: boolean; presetFileMissing?: boolean; presetMissingNetworkPolicies?: boolean; + presetMalformedYaml?: boolean; } = {}): string { const j = (p: string) => JSON.stringify(path.join(repoRoot, "dist", "lib", p)); return String.raw` @@ -126,6 +128,7 @@ policies.listPresets = () => ${JSON.stringify(presetNamesAvailable.map((name) => policies.loadPreset = (name) => { if (${JSON.stringify(presetFileMissing)}) return null; if (${JSON.stringify(presetMissingNetworkPolicies)}) return "name: " + name + "\ndescription: \"stub preset without network_policies\"\n"; + if (${JSON.stringify(presetMalformedYaml)}) return "network_policies:\n - [unclosed\n"; return "network_policies:\n " + name + ":\n egress:\n - host: example.com"; }; policies.applyPreset = (sandboxName, presetName) => { @@ -458,8 +461,8 @@ process.exit = (code) => { `invalid preset must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); assert.ok( - result.stderr.includes("missing a 'network_policies:' section"), - `expected diagnostic about missing network_policies section; got:\n${result.stderr}`, + result.stderr.includes("has no parseable entries under 'network_policies:'"), + `expected diagnostic about unparseable network_policies section; got:\n${result.stderr}`, ); assert.ok( result.stderr.includes("Restore the preset YAML and re-run: nemoclaw test-sb channels add telegram"), @@ -467,6 +470,114 @@ process.exit = (code) => { ); }); + it("aborts non-QR channel when policy preset YAML body is malformed", () => { + const script = `${buildPreamble({ presetMalformedYaml: true })} +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + savedCredentialKeys: ctx.savedCredentialKeys, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.appliedCalls, []); + assert.deepEqual(payload.providerCalls, []); + assert.deepEqual(payload.registryUpdates, []); + assert.deepEqual(payload.savedCredentialKeys, []); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `malformed preset must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + assert.ok( + result.stderr.includes("has no parseable entries under 'network_policies:'"), + `expected parse-failure diagnostic; got:\n${result.stderr}`, + ); + assert.ok( + result.stderr.includes("Restore the preset YAML and re-run: nemoclaw test-sb channels add telegram"), + `expected restore-and-re-run hint on stderr; got:\n${result.stderr}`, + ); + }); + + it("aborts QR-paired WhatsApp before registry write when its preset YAML is missing (issue #4548 repro)", () => { + const script = `${buildPreamble({ presetFileMissing: true })} +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "whatsapp" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.appliedCalls, []); + assert.deepEqual(payload.providerCalls, []); + assert.deepEqual( + payload.registryUpdates, + [], + `missing whatsapp.yaml must not flip messagingChannels (#4548); got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `missing whatsapp preset must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + assert.ok( + result.stderr.includes("Restore the preset YAML and re-run: nemoclaw test-sb channels add whatsapp"), + `expected restore-and-re-run hint on stderr; got:\n${result.stderr}`, + ); + }); + it("rolls back providers, registry, and credentials when applyPreset fails after a successful loadPreset", () => { const script = `${buildPreamble({ applyPresetResult: false })} const ctx = module.exports; @@ -609,6 +720,79 @@ process.exit = (code) => { `rollback path must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); }); + + it("restores prior channel config when re-add applyPreset fails on an already-enabled channel", () => { + const script = `${buildPreamble({ applyPresetResult: false })} +registry.getSandbox = () => ({ + name: "test-sb", + agent: "openclaw", + messagingChannels: ["telegram"], + disabledChannels: [], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "prior-hash" }, +}); +credentials.getCredential = (key) => key === "TELEGRAM_BOT_TOKEN" ? "prior-telegram-token" : null; +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + providerCalls: ctx.providerCalls, + registryUpdates: ctx.registryUpdates, + deletedCredentialKeys: ctx.deletedCredentialKeys, + savedCredentialKeys: ctx.savedCredentialKeys, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.appliedCalls, [{ sandboxName: "test-sb", presetName: "telegram" }]); + const lastRegistry = payload.registryUpdates[payload.registryUpdates.length - 1]; + assert.deepEqual( + lastRegistry.updates.messagingChannels, + ["telegram"], + `re-add failure must keep prior 'telegram' in messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.deepEqual( + lastRegistry.updates.providerCredentialHashes, + { TELEGRAM_BOT_TOKEN: "prior-hash" }, + `re-add failure must restore prior credential hashes; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.ok( + payload.savedCredentialKeys.includes("TELEGRAM_BOT_TOKEN"), + `re-add failure must restore prior credentials via saveCredential; got ${JSON.stringify(payload.savedCredentialKeys)}`, + ); + const upsertNames = (payload.providerCalls as Array<{ name: string }>).map((d) => d.name); + assert.ok( + upsertNames.length >= 2, + `expected initial and restorative upsertMessagingProviders calls; got ${JSON.stringify(payload.providerCalls)}`, + ); + assert.ok( + !payload.callOrder.includes("promptAndRebuild"), + `re-add failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, + ); + }); }); // Regression: `channels add` was updating the registry but NOT From c12353992f00cd968c95972a9c149a6065ad2ba1 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 14:25:19 +0000 Subject: [PATCH 7/8] fix(cli): order channels add preflight, snapshot, and rollback for failure safety Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 3 +- docs/reference/commands.mdx | 3 +- src/lib/actions/sandbox/policy-channel.ts | 39 ++++--- test/channels-add-preset.test.ts | 103 +++++++++++++++++-- 4 files changed, 119 insertions(+), 29 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index 688aeed21a..23cce4a6c2 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -167,7 +167,8 @@ The command accepts mixed-case input such as `Telegram`, then stores and prints `channels add` requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts the command before any token prompt, registry write, or rebuild prompt, so the sandbox never advertises a channel without a matching network policy. With the preset file in place, `channels add` applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the apply step itself fails after the registry write on a fresh add, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the same failure happens on a re-add of an already-enabled channel, NemoClaw restores the prior `messagingChannels` entry and the on-disk credentials and attempts to re-upsert the prior bridge providers, but flags `gateway-providers` as residual because the in-flight upsert may have left the gateway with the new token; verify the gateway bridge before relying on the channel. Restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. For Telegram, Discord, and Slack, `channels add` also checks the rebuilt runtime for the selected bridge and reports startup, credential, or missing-plugin warnings before returning. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 811c2b10fc..93c31a4085 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -726,7 +726,8 @@ Channel names are trimmed and lowercased before NemoClaw stores credentials, nam NemoClaw requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts `channels add` before any token prompt, registry write, or rebuild prompt. With the preset file in place, NemoClaw applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the apply step itself fails after the registry write on a fresh add, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the same failure happens on a re-add of an already-enabled channel, NemoClaw restores the prior `messagingChannels` entry and the on-disk credentials and attempts to re-upsert the prior bridge providers, but flags `gateway-providers` as residual because the in-flight upsert may have left the gateway with the new token; verify the gateway bridge before relying on the channel. Restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index a459592131..d617a85cdc 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -807,11 +807,6 @@ export async function addSandboxChannel( process.exit(1); } - if (dryRun) { - console.log(` --dry-run: would enable channel '${canonical}' for '${sandboxName}'.`); - return; - } - const presetContent = policies.loadPreset(canonical); const presetPolicyKeys = presetContent === null ? [] : policies.parsePresetPolicyKeys(presetContent); @@ -827,6 +822,11 @@ export async function addSandboxChannel( process.exit(1); } + if (dryRun) { + console.log(` --dry-run: would enable channel '${canonical}' for '${sandboxName}'.`); + return; + } + // QR-paired channels that own their session inside the sandbox have no // host-side credential to acquire; register the bridge now and let the // operator complete pairing after rebuild. @@ -851,13 +851,6 @@ export async function addSandboxChannel( return; } - const acquired: Record = {}; - if (channel.loginMethod === "host-qr") { - await acquireHostQrChannel(sandboxName, canonical, channel, acquired); - } else { - await acquirePasteTokens(canonical, channel, acquired); - } - const priorEntry = registry.getSandbox(sandboxName); const priorMessagingChannels: string[] = priorEntry?.messagingChannels ? [...priorEntry.messagingChannels] @@ -873,6 +866,13 @@ export async function addSandboxChannel( if (existing != null) priorCreds[key] = existing; } + const acquired: Record = {}; + if (channel.loginMethod === "host-qr") { + await acquireHostQrChannel(sandboxName, canonical, channel, acquired); + } else { + await acquirePasteTokens(canonical, channel, acquired); + } + persistChannelTokens(acquired); // Push to the gateway and update the registry NOW so that answering // "rebuild later" (or running non-interactively) does not silently @@ -916,11 +916,16 @@ async function rollbackChannelAdd( console.error( ` ${YW}⚠${R} Restoring prior '${canonical}' configuration; new token rotation aborted.`, ); + registry.updateSandbox(sandboxName, { + messagingChannels: snapshot.priorMessagingChannels, + providerCredentialHashes: + Object.keys(snapshot.priorHashes).length > 0 ? snapshot.priorHashes : undefined, + }); clearChannelTokens(channel); if (Object.keys(snapshot.priorCreds).length > 0) { persistChannelTokens(snapshot.priorCreds); } - const residual: string[] = []; + const residual: string[] = ["gateway-providers"]; if (Object.keys(snapshot.priorCreds).length > 0) { try { const priorTokenDefs = Object.entries(snapshot.priorCreds).map(([envKey, token]) => ({ @@ -935,15 +940,9 @@ async function rollbackChannelAdd( err instanceof Error ? err.message : String(err) }`, ); - residual.push("gateway-providers"); } } - registry.updateSandbox(sandboxName, { - messagingChannels: snapshot.priorMessagingChannels, - providerCredentialHashes: - Object.keys(snapshot.priorHashes).length > 0 ? snapshot.priorHashes : undefined, - }); - return { ok: residual.length === 0, residual }; + return { ok: false, residual }; } console.error( diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 60489c9c84..4b18d651e9 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -792,6 +792,85 @@ process.exit = (code) => { !payload.callOrder.includes("promptAndRebuild"), `re-add failure must not prompt for rebuild; got order: ${JSON.stringify(payload.callOrder)}`, ); + assert.ok( + result.stderr.includes("Rollback could not fully clean gateway-providers"), + `expected residual-state warning on stderr; got:\n${result.stderr}`, + ); + }); + + it("restores prior registry state even when re-upsert during re-add rollback throws", () => { + const script = `${buildPreamble({ applyPresetResult: false })} +registry.getSandbox = () => ({ + name: "test-sb", + agent: "openclaw", + messagingChannels: ["telegram"], + disabledChannels: [], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "prior-hash" }, +}); +credentials.getCredential = (key) => key === "TELEGRAM_BOT_TOKEN" ? "prior-telegram-token" : null; +let upsertCalls = 0; +onboardProviders.upsertMessagingProviders = (defs) => { + upsertCalls += 1; + providerCalls.push(...defs); + if (upsertCalls >= 2) throw new Error("simulated gateway upsert failure during restore"); +}; +const ctx = module.exports; +const exitCodes = []; +const originalExit = process.exit; +process.exit = (code) => { + exitCodes.push(code ?? 0); + throw new Error("__EXIT__" + (code ?? 0)); +}; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "telegram" }); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + } finally { + process.exit = originalExit; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + registryUpdates: ctx.registryUpdates, + savedCredentialKeys: ctx.savedCredentialKeys, + exitCodes, + }) + "\\n"); +})(); +`; + const result = runScript(script); + assert.equal(result.status, 0, `script failed: ${result.stderr}\n${result.stdout}`); + const marker = result.stdout.lastIndexOf("__RESULT__"); + const payload = JSON.parse(result.stdout.slice(marker + "__RESULT__".length).trim()); + assert.ok(!payload.error, `unexpected error: ${payload.error}\n${payload.stack || ""}`); + + assert.deepEqual(payload.exitCodes, [1]); + const lastRegistry = payload.registryUpdates[payload.registryUpdates.length - 1]; + assert.deepEqual( + lastRegistry.updates.messagingChannels, + ["telegram"], + `registry restoration must precede gateway re-upsert so an upsert failure cannot orphan the channel; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.deepEqual( + lastRegistry.updates.providerCredentialHashes, + { TELEGRAM_BOT_TOKEN: "prior-hash" }, + `prior credential hashes must be restored before any gateway side effect; got ${JSON.stringify(payload.registryUpdates)}`, + ); + assert.ok( + payload.savedCredentialKeys.includes("TELEGRAM_BOT_TOKEN"), + `re-add failure must restore on-disk credentials; got ${JSON.stringify(payload.savedCredentialKeys)}`, + ); + assert.ok( + result.stderr.includes("Failed to restore gateway providers for 'telegram'"), + `expected gateway-provider restoration warning on stderr; got:\n${result.stderr}`, + ); + assert.ok( + result.stderr.includes("Rollback could not fully clean gateway-providers"), + `expected residual-state warning on stderr; got:\n${result.stderr}`, + ); }); }); @@ -1371,20 +1450,30 @@ global.__testLog = ""; }); describe("channel preset source-of-truth", () => { - it("every channel registered in KNOWN_CHANNELS ships a matching preset YAML on disk", () => { + it("every channel registered in KNOWN_CHANNELS ships a preset YAML that parsePresetPolicyKeys() accepts", () => { const { knownChannelNames } = require(path.join(repoRoot, "dist", "lib", "sandbox", "channels.js")) as { knownChannelNames: () => string[]; }; - const presetDir = path.join(repoRoot, "nemoclaw-blueprint", "policies", "presets"); - const missing: string[] = []; + const { loadPreset, parsePresetPolicyKeys } = require(path.join(repoRoot, "dist", "lib", "policy", "index.js")) as { + loadPreset: (name: string) => string | null; + parsePresetPolicyKeys: (content: string | null | undefined) => string[]; + }; + const failures: string[] = []; for (const name of knownChannelNames()) { - const file = path.join(presetDir, `${name}.yaml`); - if (!fs.existsSync(file)) missing.push(file); + const content = loadPreset(name); + if (content === null) { + failures.push(`${name}: preset YAML not found on disk`); + continue; + } + const keys = parsePresetPolicyKeys(content); + if (keys.length === 0) { + failures.push(`${name}: parsePresetPolicyKeys returned no entries`); + } } assert.deepEqual( - missing, + failures, [], - `every channel in KNOWN_CHANNELS must have a matching preset YAML; missing: ${missing.join(", ")}`, + `every channel in KNOWN_CHANNELS must ship a parseable preset YAML; failures: ${failures.join("; ")}`, ); }); }); From 3b3c520b07653471121e4a19838f32ea5ea62cfe Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sun, 31 May 2026 14:49:26 +0000 Subject: [PATCH 8/8] fix(cli): print rollback residual warning before fallible gateway upsert Signed-off-by: Tinson Lai --- docs/manage-sandboxes/messaging-channels.mdx | 2 +- docs/reference/commands.mdx | 4 ++-- src/lib/actions/sandbox/policy-channel.ts | 18 +++++++++++------- test/channels-add-preset.test.ts | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/docs/manage-sandboxes/messaging-channels.mdx b/docs/manage-sandboxes/messaging-channels.mdx index 23cce4a6c2..77554689b0 100644 --- a/docs/manage-sandboxes/messaging-channels.mdx +++ b/docs/manage-sandboxes/messaging-channels.mdx @@ -167,7 +167,7 @@ The command accepts mixed-case input such as `Telegram`, then stores and prints `channels add` requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts the command before any token prompt, registry write, or rebuild prompt, so the sandbox never advertises a channel without a matching network policy. With the preset file in place, `channels add` applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write on a fresh add, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the apply step itself fails after the registry write on a fresh add, NemoClaw attempts to roll back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild; if any gateway-side step (provider detach or delete) fails the rollback continues and prints a `Rollback could not fully clean ` warning so the operator can clean up manually. When the same failure happens on a re-add of an already-enabled channel, NemoClaw restores the prior `messagingChannels` entry and the on-disk credentials and attempts to re-upsert the prior bridge providers, but flags `gateway-providers` as residual because the in-flight upsert may have left the gateway with the new token; verify the gateway bridge before relying on the channel. Restore the preset YAML and re-run `nemoclaw channels add `. Choose the rebuild so the running sandbox image picks up the new channel. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 93c31a4085..116338a6c5 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -726,7 +726,7 @@ Channel names are trimmed and lowercased before NemoClaw stores credentials, nam NemoClaw requires the matching built-in network policy preset YAML to be present. A missing or malformed preset YAML (no `network_policies:` section) aborts `channels add` before any token prompt, registry write, or rebuild prompt. With the preset file in place, NemoClaw applies it to the sandbox before the rebuild so the bridge has egress to its upstream API. -When the apply step itself fails after the registry write on a fresh add, NemoClaw rolls back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild. +When the apply step itself fails after the registry write on a fresh add, NemoClaw attempts to roll back the bridge providers, the `messagingChannels` entry, and the persisted credentials, then exits without prompting for a rebuild; if any gateway-side step (provider detach or delete) fails the rollback continues and prints a `Rollback could not fully clean ` warning so the operator can clean up manually. When the same failure happens on a re-add of an already-enabled channel, NemoClaw restores the prior `messagingChannels` entry and the on-disk credentials and attempts to re-upsert the prior bridge providers, but flags `gateway-providers` as residual because the in-flight upsert may have left the gateway with the new token; verify the gateway bridge before relying on the channel. Restore the preset YAML and re-run `nemoclaw channels add `. For Telegram, Discord, and Slack, a rebuild triggered by `channels add` also verifies that the selected bridge starts and reports credential, startup, or plugin discovery warnings. @@ -737,7 +737,7 @@ $ nemoclaw my-assistant channels add telegram | Flag | Description | |------|-------------| -| `--dry-run` | Validate the channel and token inputs without saving credentials or rebuilding | +| `--dry-run` | Validate the channel name and matching policy preset without prompting for credentials, contacting the gateway, or rebuilding | Slack requires both `SLACK_BOT_TOKEN` (bot user OAuth) and `SLACK_APP_TOKEN` (app-level Socket Mode token); the command prompts for each in turn. Optional Slack allowlists come from `SLACK_ALLOWED_USERS` and `SLACK_ALLOWED_CHANNELS` at rebuild time. diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index d617a85cdc..cfb1036d16 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -883,17 +883,12 @@ export async function addSandboxChannel( console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); if (!applyChannelPresetIfAvailable(sandboxName, canonical)) { - const rollback = await rollbackChannelAdd(sandboxName, channel, canonical, { + await rollbackChannelAdd(sandboxName, channel, canonical, { wasAlreadyEnabled, priorMessagingChannels, priorHashes, priorCreds, }); - if (!rollback.ok) { - console.error( - ` ${YW}⚠${R} Rollback could not fully clean ${rollback.residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, - ); - } process.exit(1); } @@ -926,6 +921,9 @@ async function rollbackChannelAdd( persistChannelTokens(snapshot.priorCreds); } const residual: string[] = ["gateway-providers"]; + console.error( + ` ${YW}⚠${R} Rollback could not fully clean ${residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, + ); if (Object.keys(snapshot.priorCreds).length > 0) { try { const priorTokenDefs = Object.entries(snapshot.priorCreds).map(([envKey, token]) => ({ @@ -949,12 +947,18 @@ async function rollbackChannelAdd( ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messagingChannels and policy state aligned.`, ); clearChannelTokens(channel); - return applyChannelRemoveToGatewayAndRegistry( + const result = await applyChannelRemoveToGatewayAndRegistry( sandboxName, canonical, getChannelTokenKeys(channel), { bestEffort: true }, ); + if (!result.ok) { + console.error( + ` ${YW}⚠${R} Rollback could not fully clean ${result.residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, + ); + } + return result; } function applyChannelPresetIfAvailable(sandboxName: string, channelName: string): boolean { diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 4b18d651e9..4fc875e656 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -525,7 +525,7 @@ process.exit = (code) => { ); }); - it("aborts QR-paired WhatsApp before registry write when its preset YAML is missing (issue #4548 repro)", () => { + it("aborts QR-paired WhatsApp before registry write when its preset YAML is missing", () => { const script = `${buildPreamble({ presetFileMissing: true })} const ctx = module.exports; const exitCodes = []; @@ -566,7 +566,7 @@ process.exit = (code) => { assert.deepEqual( payload.registryUpdates, [], - `missing whatsapp.yaml must not flip messagingChannels (#4548); got ${JSON.stringify(payload.registryUpdates)}`, + `missing whatsapp.yaml must not flip messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, ); assert.ok( !payload.callOrder.includes("promptAndRebuild"),