diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index db33e0b2e67..29b60e44910 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -162,7 +162,10 @@ export async function addSandboxPolicy( if (confirm.trim().toLowerCase().startsWith("n")) return; } - policies.applyPreset(sandboxName, answer); + if (!policies.applyPreset(sandboxName, answer)) { + process.exit(1); + } + syncSessionPolicyPresetsWithRegistry(sandboxName, answer, "add"); } /** @@ -212,6 +215,11 @@ async function applyExternalPreset( const result = policies.applyPresetContent(sandboxName, loaded.presetName, loaded.content, { custom: { sourcePath: path.resolve(filePath) }, }); + if (result !== false) { + // Custom presets share the registry slot with built-ins (customPolicies + // in policy/index.ts:684), so they need the same session-sync. + syncSessionPolicyPresetsWithRegistry(sandboxName, loaded.presetName, "add"); + } return result !== false; } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); @@ -723,6 +731,7 @@ function applyChannelPresetIfAvailable(sandboxName: string, channelName: string) ); return false; } + syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "add"); return true; } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -785,18 +794,53 @@ function clearSandboxChannelDurableState(sandboxName: string, channelName: strin return true; } -// Drop the channel name from session.policyPresets so onboard --resume's -// preset reconciliation does not re-apply the preset we just removed (#3998). -function dropChannelFromSessionPolicyPresets(channelName: string): void { - onboardSession.updateSession((current) => { - if (Array.isArray(current.policyPresets)) { - const filtered = current.policyPresets.filter((preset) => preset !== channelName); - if (filtered.length !== current.policyPresets.length) { - current.policyPresets = filtered; +// Mirror a registry-side preset add/remove into `session.policyPresets`. +// Without this, a later `rebuild` re-enters onboard resume, reads the +// stale session, and narrows the preset back away — see #3437 follow-up. +// Best-effort: registry has already succeeded; failure paths log and +// swallow so the caller's flow is never broken by a session I/O error. +function syncSessionPolicyPresetsWithRegistry( + sandboxName: string, + presetName: string, + action: "add" | "remove", +): void { + let session: ReturnType; + try { + session = onboardSession.loadSession(); + } catch { + return; + } + // No session = nothing to sync. Foreign sandbox = leave its intent alone. + if (!session) return; + if (session.sandboxName !== sandboxName) return; + + const current = Array.isArray(session.policyPresets) ? session.policyPresets : []; + const has = current.includes(presetName); + // Skip the file write when the desired state already holds. + if (action === "add" && has) return; + if (action === "remove" && !has) return; + + try { + onboardSession.updateSession((s) => { + const arr = Array.isArray(s.policyPresets) ? [...s.policyPresets] : []; + if (action === "add") { + if (!arr.includes(presetName)) arr.push(presetName); + } else { + const idx = arr.indexOf(presetName); + if (idx >= 0) arr.splice(idx, 1); } - } - return current; - }); + s.policyPresets = arr; + return s; + }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error( + ` ${YW}⚠${R} Could not record '${presetName}' preset ${action} in onboard session: ${msg}`, + ); + console.error( + ` Registry is consistent; rerun '${CLI_NAME} ${sandboxName} policy-${action === "add" ? "add" : "remove"} ${presetName}' after rebuild if needed.`, + ); + } } // Mirror of applyChannelPresetIfAvailable. When the channel-named built-in @@ -809,9 +853,11 @@ function dropChannelFromSessionPolicyPresets(channelName: string): void { function removeChannelPresetIfPresent(sandboxName: string, channelName: string): void { const builtinPresets = new Set(policies.listPresets().map((p) => p.name)); if (!builtinPresets.has(channelName)) { + syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); return; } if (!policies.getAppliedPresets(sandboxName).includes(channelName)) { + syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); return; } try { @@ -823,6 +869,8 @@ function removeChannelPresetIfPresent(sandboxName: string, channelName: string): console.error( ` Run manually after rebuild with: ${CLI_NAME} ${sandboxName} policy-remove ${channelName}`, ); + } else { + syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); } } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -863,7 +911,12 @@ export async function removeSandboxChannel( const isQrChannel = channelUsesInSandboxQrPairing(channel); const registryEntry = registry.getSandbox(sandboxName); - const sessionForSandbox = onboardSession.loadSession(); + let sessionForSandbox: ReturnType = null; + try { + sessionForSandbox = onboardSession.loadSession(); + } catch { + sessionForSandbox = null; + } const sessionPolicyPresets = sessionForSandbox?.sandboxName === sandboxName && Array.isArray(sessionForSandbox.policyPresets) @@ -901,7 +954,6 @@ export async function removeSandboxChannel( } removeChannelPresetIfPresent(sandboxName, canonical); - dropChannelFromSessionPolicyPresets(canonical); // Token-based channels: best-effort tidy of any leftover dir. Token // revocation already prevents the bot from authenticating, so a @@ -1047,4 +1099,5 @@ export async function removeSandboxPolicy( if (!policies.removePreset(sandboxName, answer)) { process.exit(1); } + syncSessionPolicyPresetsWithRegistry(sandboxName, answer, "remove"); } diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index 7e213b76e45..31a87dc05ed 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -48,17 +48,36 @@ function runScript(scriptBody: string, extraEnv: Record = {}): S function buildPreamble({ presetNamesAvailable = ["telegram", "slack", "discord", "npm", "github"], applyPresetResult = true, + appliedPresets = [] as string[], sandboxAgent = "openclaw", + sessionSandboxName = "test-sb", + sessionPolicyPresets = ["npm", "pypi", "huggingface", "brew"] as string[] | null, + sessionLoadThrows = false, + sessionUpdateThrows = false, + sessionMissing = false, }: { presetNamesAvailable?: string[]; applyPresetResult?: boolean; + appliedPresets?: string[]; sandboxAgent?: string; + sessionSandboxName?: string | null; + sessionPolicyPresets?: string[] | null; + sessionLoadThrows?: boolean; + sessionUpdateThrows?: boolean; + sessionMissing?: boolean; } = {}): string { const j = (p: string) => JSON.stringify(path.join(repoRoot, "dist", "lib", p)); return String.raw` const resolver = require(${j("adapters/openshell/resolve.js")}); resolver.resolveOpenshell = () => "/fake/openshell"; +const openshellRuntime = require(${j("adapters/openshell/runtime.js")}); +openshellRuntime.runOpenshell = () => ({ status: 0, stdout: "", stderr: "" }); + +const processRecovery = require(${j("actions/sandbox/process-recovery.js")}); +processRecovery.executeSandboxExecCommand = () => ({ status: 0, stdout: "NEMOCLAW_CHANNEL_CLEAR_OK", stderr: "" }); +processRecovery.executeSandboxCommand = () => null; + const runner = require(${j("runner.js")}); runner.run = () => ({ status: 0, stdout: "", stderr: "" }); runner.runCapture = () => ""; @@ -95,6 +114,7 @@ registry.updateSandbox = (name, updates) => { const policies = require(${j("policy/index.js")}); const appliedCalls = []; +const removedCalls = []; const callOrder = []; policies.listPresets = () => ${JSON.stringify(presetNamesAvailable.map((name) => ({ name })))}; policies.applyPreset = (sandboxName, presetName) => { @@ -102,7 +122,48 @@ policies.applyPreset = (sandboxName, presetName) => { callOrder.push("applyPreset:" + presetName); return ${JSON.stringify(applyPresetResult)}; }; -policies.getAppliedPresets = () => []; +policies.removePreset = (sandboxName, presetName) => { + removedCalls.push({ sandboxName, presetName }); + callOrder.push("removePreset:" + presetName); + return true; +}; +policies.getAppliedPresets = () => ${JSON.stringify(appliedPresets)}; + +// Stub onboardSession so the new policyPresets-sync helper has something +// to read/write. The test asserts on sessionUpdates to verify the +// helper kept session.policyPresets aligned with the registry. +const onboardSession = require(${j("state/onboard-session.js")}); +const sessionUpdates = []; +const sessionLoadConfig = ${JSON.stringify({ + sessionSandboxName, + sessionPolicyPresets, + sessionLoadThrows, + sessionMissing, + })}; +const sessionUpdateThrows = ${JSON.stringify(sessionUpdateThrows)}; +let sessionState = sessionLoadConfig.sessionMissing + ? null + : { + sandboxName: sessionLoadConfig.sessionSandboxName, + policyPresets: Array.isArray(sessionLoadConfig.sessionPolicyPresets) + ? [...sessionLoadConfig.sessionPolicyPresets] + : sessionLoadConfig.sessionPolicyPresets, + }; +onboardSession.loadSession = () => { + if (sessionLoadConfig.sessionLoadThrows) throw new Error("simulated load failure"); + return sessionState; +}; +onboardSession.updateSession = (mutator) => { + if (sessionUpdateThrows) throw new Error("simulated save failure"); + // Mirror the real updateSession contract: load → mutate → save. + if (!sessionState) sessionState = { sandboxName: null, policyPresets: null }; + const next = mutator(sessionState) || sessionState; + sessionState = next; + sessionUpdates.push({ + policyPresets: Array.isArray(next.policyPresets) ? [...next.policyPresets] : next.policyPresets, + }); + return next; +}; // Tag the rebuild-prompt branch via stdout so we can compare ordering. // In NEMOCLAW_NON_INTERACTIVE mode, promptAndRebuild logs "Change queued." @@ -116,7 +177,7 @@ console.log = (...args) => { const channelModule = require(${j("actions/sandbox/policy-channel.js")}); -module.exports = { channelModule, appliedCalls, callOrder, providerCalls, registryUpdates }; +module.exports = { channelModule, appliedCalls, removedCalls, callOrder, providerCalls, registryUpdates, sessionUpdates, getSessionState: () => sessionState }; `; } @@ -316,3 +377,298 @@ const ctx = module.exports; ); }); }); + +// Regression: `channels add` was updating the registry but NOT +// session.policyPresets. A later `rebuild` re-entered onboard in resume +// mode, read the stale session, and the policy-selection step narrowed +// the channel's preset back away. The new sandbox booted with the +// channel auto-launched but no matching network policy active, so the +// bridge's Slack/Telegram/Discord WebClient hit 403s and stayed wedged +// even after Step 5.5 of rebuild reapplied the preset from the backup +// manifest. +// +// These tests pin down the invariant: after a successful preset apply +// via channels-add, session.policyPresets must contain the channel +// name; after a successful preset remove via channels-remove, it must +// not. Edge cases (no session, foreign sandbox, save failure) must not +// abort the operation. +describe("channels add/remove keeps session.policyPresets in sync with registry", () => { + it("appends the channel preset to session.policyPresets after a successful add", () => { + const script = `${buildPreamble({ + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm", "pypi", "huggingface", "brew"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // Exactly one update — the helper short-circuits when the desired + // membership already holds, so duplicate writes would be a bug. + assert.equal( + payload.sessionUpdates.length, + 1, + `expected exactly one session update; got ${JSON.stringify(payload.sessionUpdates)}`, + ); + assert.deepEqual(payload.sessionUpdates[0].policyPresets, [ + "npm", + "pypi", + "huggingface", + "brew", + "slack", + ]); + assert.deepEqual(payload.finalSession.policyPresets, [ + "npm", + "pypi", + "huggingface", + "brew", + "slack", + ]); + }); + + it("does not touch the session when it tracks a different sandbox", () => { + const script = `${buildPreamble({ + sessionSandboxName: "other-sb", + sessionPolicyPresets: ["npm", "github"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + appliedCalls: ctx.appliedCalls, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // applyPreset still runs against the registry — the preset is the + // channel's egress contract and lives in registry, not session. + assert.deepEqual(payload.appliedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + // But the foreign session's policyPresets must be left untouched — + // otherwise we corrupt the other sandbox's resume state. + assert.deepEqual( + payload.sessionUpdates, + [], + `session belonging to a different sandbox must not be mutated; got ${JSON.stringify(payload.sessionUpdates)}`, + ); + assert.deepEqual(payload.finalSession.policyPresets, ["npm", "github"]); + }); + + it("succeeds even when no onboard session file exists", () => { + const script = `${buildPreamble({ sessionMissing: true })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + sessionUpdates: ctx.sessionUpdates, + appliedCalls: ctx.appliedCalls, + callOrder: ctx.callOrder, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // Registry mutation still happens; only the session-sync side-effect + // is skipped (there is no intent record to keep aligned). + assert.deepEqual(payload.appliedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.deepEqual(payload.sessionUpdates, []); + assert.ok(payload.callOrder.includes("promptAndRebuild")); + }); + + it("does not abort channels-add when session save fails", () => { + const script = `${buildPreamble({ + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm", "pypi", "huggingface", "brew"], + sessionUpdateThrows: true, + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxChannel("test-sb", { channel: "slack" }); + 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"); + } +})(); +`; + 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 || ""}`); + + // Even though session.updateSession threw, the channel add flow + // still completed: preset applied to registry, rebuild prompted. + // Session-sync is best-effort. + assert.deepEqual(payload.appliedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.ok(payload.callOrder.includes("promptAndRebuild")); + }); + + it("removes the channel preset from session.policyPresets after a successful remove", () => { + const script = `${buildPreamble({ + appliedPresets: ["slack"], + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm", "slack", "github"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.removeSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + removedCalls: ctx.removedCalls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + callOrder: ctx.callOrder, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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.removedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.equal( + payload.sessionUpdates.length, + 1, + `expected exactly one session update; got ${JSON.stringify(payload.sessionUpdates)}`, + ); + assert.deepEqual(payload.sessionUpdates[0].policyPresets, ["npm", "github"]); + assert.deepEqual(payload.finalSession.policyPresets, ["npm", "github"]); + assert.ok(payload.callOrder.includes("promptAndRebuild")); + }); + + it("does not touch a foreign session during channels-remove", () => { + const script = `${buildPreamble({ + appliedPresets: ["slack"], + sessionSandboxName: "other-sb", + sessionPolicyPresets: ["slack", "npm"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.removeSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + removedCalls: ctx.removedCalls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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.removedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.deepEqual( + payload.sessionUpdates, + [], + `session belonging to a different sandbox must not be mutated; got ${JSON.stringify(payload.sessionUpdates)}`, + ); + assert.deepEqual(payload.finalSession.policyPresets, ["slack", "npm"]); + }); + + it("succeeds during channels-remove when no onboard session file exists", () => { + const script = `${buildPreamble({ + appliedPresets: ["slack"], + sessionMissing: true, + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.removeSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + removedCalls: ctx.removedCalls, + sessionUpdates: ctx.sessionUpdates, + callOrder: ctx.callOrder, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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.removedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.deepEqual(payload.sessionUpdates, []); + assert.ok(payload.callOrder.includes("promptAndRebuild")); + }); + + it("does not abort channels-remove when session save fails", () => { + const script = `${buildPreamble({ + appliedPresets: ["slack"], + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm", "slack"], + sessionUpdateThrows: true, + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.removeSandboxChannel("test-sb", { channel: "slack" }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + removedCalls: ctx.removedCalls, + callOrder: ctx.callOrder, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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.removedCalls, [{ sandboxName: "test-sb", presetName: "slack" }]); + assert.ok(payload.callOrder.includes("promptAndRebuild")); + }); +}); diff --git a/test/policies.test.ts b/test/policies.test.ts index 53c9693e7d9..b1cefaafcee 100644 --- a/test/policies.test.ts +++ b/test/policies.test.ts @@ -74,6 +74,7 @@ policies.listPresets = () => [ policies.getAppliedPresets = () => []; policies.applyPreset = (sandboxName, presetName) => { calls.push({ type: "apply", sandboxName, presetName }); + return true; }; process.argv = ["node", "nemoclaw.js", "test-sandbox", "policy-add", ...${JSON.stringify(extraArgs)}]; Promise.resolve(require(${CLI_PATH}).mainPromise).finally(() => { diff --git a/test/policy-add-remove-session-sync.test.ts b/test/policy-add-remove-session-sync.test.ts new file mode 100644 index 00000000000..7b1098b674e --- /dev/null +++ b/test/policy-add-remove-session-sync.test.ts @@ -0,0 +1,342 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Regression test for the same session/registry divergence that motivated +// the channels-add fix (see test/channels-add-preset.test.ts). The bug +// surfaced first via `nemoclaw channels add slack` → `rebuild` +// (registry got slack, session did not, rebuild's resume step narrowed +// it back away). The exact same divergence applies to the standalone +// preset-mutation CLIs: +// +// - `nemoclaw policy-add ` (built-in preset) +// - `nemoclaw policy-add --from-file …` (custom preset YAML) +// - `nemoclaw policy-remove ` (any preset) +// +// All three call `policies.applyPreset` / `policies.applyPresetContent` / +// `policies.removePreset` to mutate the registry; none of them previously +// touched `session.policyPresets`. These tests pin down the invariant +// that after the channels-add fix was generalised, all three paths now +// keep session in sync with registry, with the same best-effort error +// handling. + +import assert from "node:assert/strict"; +import { spawnSync, type SpawnSyncReturns } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, it } from "vitest"; + +const repoRoot = path.join(import.meta.dirname, ".."); + +function runScript(scriptBody: string, extraEnv: Record = {}): SpawnSyncReturns { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-sync-")); + const scriptPath = path.join(tmpDir, "script.js"); + fs.writeFileSync(scriptPath, scriptBody); + const result = spawnSync(process.execPath, [scriptPath], { + cwd: repoRoot, + encoding: "utf-8", + env: { + ...process.env, + HOME: tmpDir, + NEMOCLAW_NON_INTERACTIVE: "1", + ...extraEnv, + }, + timeout: 15000, + }); + fs.rmSync(tmpDir, { recursive: true, force: true }); + return result; +} + +// Stub every module that addSandboxPolicy / removeSandboxPolicy touches. +// The only side effect we actually want to observe is on the onboardSession +// stub, so everything else is faked to a no-op success. +function buildPreamble({ + presetNamesAvailable = ["github", "npm", "pypi"], + appliedPresets = [] as string[], + applyPresetResult = true, + sessionSandboxName = "test-sb" as string | null, + sessionPolicyPresets = ["npm"] as string[] | null, + sessionMissing = false, +}: { + presetNamesAvailable?: string[]; + appliedPresets?: string[]; + applyPresetResult?: boolean; + sessionSandboxName?: string | null; + sessionPolicyPresets?: string[] | null; + sessionMissing?: boolean; +} = {}): string { + const j = (p: string) => JSON.stringify(path.join(repoRoot, "dist", "lib", p)); + return String.raw` +const onboard = require(${j("onboard.js")}); +onboard.isNonInteractive = () => true; + +const credentials = require(${j("credentials/store.js")}); +credentials.prompt = async () => "y"; + +const policies = require(${j("policy/index.js")}); +const calls = { apply: [], applyContent: [], remove: [] }; +policies.listPresets = () => ${JSON.stringify(presetNamesAvailable.map((name) => ({ name })))}; +policies.getAppliedPresets = () => ${JSON.stringify(appliedPresets)}; +policies.loadPreset = (name) => ({ name, network_policies: {} }); +policies.getPresetEndpoints = () => []; +policies.getMessagingPresetWarning = () => null; +policies.selectFromList = async (items) => items[0]?.name || null; +policies.applyPreset = (sandboxName, presetName) => { + calls.apply.push({ sandboxName, presetName }); + return ${JSON.stringify(applyPresetResult)}; +}; +policies.applyPresetContent = (sandboxName, presetName) => { + calls.applyContent.push({ sandboxName, presetName }); + return true; +}; +policies.removePreset = (sandboxName, presetName) => { + calls.remove.push({ sandboxName, presetName }); + return true; +}; +// loadPresetFromFile is used by --from-file path. +policies.loadPresetFromFile = (filePath) => ({ + presetName: "custom-preset-from-file", + content: { network_policies: {} }, +}); + +const onboardSession = require(${j("state/onboard-session.js")}); +const sessionUpdates = []; +let sessionState = ${ + sessionMissing + ? "null" + : `{ + sandboxName: ${JSON.stringify(sessionSandboxName)}, + policyPresets: ${JSON.stringify(sessionPolicyPresets)}, +}` + }; +onboardSession.loadSession = () => sessionState; +onboardSession.updateSession = (mutator) => { + if (!sessionState) sessionState = { sandboxName: null, policyPresets: null }; + const next = mutator(sessionState) || sessionState; + sessionState = next; + sessionUpdates.push({ + policyPresets: Array.isArray(next.policyPresets) ? [...next.policyPresets] : next.policyPresets, + }); + return next; +}; + +const channelModule = require(${j("actions/sandbox/policy-channel.js")}); + +module.exports = { channelModule, calls, sessionUpdates, getSessionState: () => sessionState }; +`; +} + +describe("policy-add / policy-remove keep session.policyPresets in sync with registry", () => { + it("appends the built-in preset to session.policyPresets after policy-add", () => { + const script = `${buildPreamble({ + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxPolicy("test-sb", { preset: "github", yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // Contract 1: applyPreset called exactly once with the chosen preset. + assert.deepEqual(payload.calls.apply, [{ sandboxName: "test-sb", presetName: "github" }]); + // Contract 2: session updated exactly once, github appended. + assert.equal(payload.sessionUpdates.length, 1); + assert.deepEqual(payload.sessionUpdates[0].policyPresets, ["npm", "github"]); + assert.deepEqual(payload.finalSession.policyPresets, ["npm", "github"]); + }); + + it("does not sync session.policyPresets when built-in policy-add fails", () => { + const script = `${buildPreamble({ + applyPresetResult: false, + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm"], + })} +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.addSandboxPolicy("test-sb", { preset: "github", yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + exitCodes, + }) + "\\n"); + } catch (err) { + if (!String(err && err.message).startsWith("__EXIT__")) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + return; + } + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + exitCodes, + }) + "\\n"); + } finally { + process.exit = originalExit; + } +})(); +`; + 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.calls.apply, [{ sandboxName: "test-sb", presetName: "github" }]); + assert.deepEqual(payload.exitCodes, [1]); + assert.deepEqual(payload.sessionUpdates, []); + assert.deepEqual(payload.finalSession.policyPresets, ["npm"]); + }); + + it("appends the custom preset (--from-file) to session.policyPresets", () => { + // Write a tiny YAML file the stubbed loadPresetFromFile will pretend to parse. + const presetFile = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-preset-")); + const yamlPath = path.join(presetFile, "custom.yaml"); + fs.writeFileSync(yamlPath, "name: custom-preset-from-file\nnetwork_policies: {}\n"); + + const script = `${buildPreamble({ + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxPolicy("test-sb", { fromFile: ${JSON.stringify(yamlPath)}, yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\n"); + } +})(); +`; + const result = runScript(script); + fs.rmSync(presetFile, { recursive: true, force: true }); + 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 || ""}`); + + // The custom-preset path goes through applyPresetContent (NOT applyPreset). + assert.deepEqual(payload.calls.applyContent, [ + { sandboxName: "test-sb", presetName: "custom-preset-from-file" }, + ]); + assert.equal(payload.sessionUpdates.length, 1); + assert.deepEqual(payload.sessionUpdates[0].policyPresets, ["npm", "custom-preset-from-file"]); + }); + + it("removes the preset from session.policyPresets after policy-remove", () => { + const script = `${buildPreamble({ + appliedPresets: ["npm", "github"], + sessionSandboxName: "test-sb", + sessionPolicyPresets: ["npm", "github"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.removeSandboxPolicy("test-sb", { preset: "github", yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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.calls.remove, [{ sandboxName: "test-sb", presetName: "github" }]); + assert.equal(payload.sessionUpdates.length, 1); + assert.deepEqual(payload.sessionUpdates[0].policyPresets, ["npm"]); + }); + + it("does not touch a session belonging to a different sandbox", () => { + const script = `${buildPreamble({ + sessionSandboxName: "other-sb", + sessionPolicyPresets: ["pypi"], + })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxPolicy("test-sb", { preset: "github", yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + finalSession: ctx.getSessionState(), + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // Registry mutation still happens — that lives per-sandbox in the + // OpenShell policy engine, not in the session file. + assert.deepEqual(payload.calls.apply, [{ sandboxName: "test-sb", presetName: "github" }]); + // But session for "other-sb" must be left alone. + assert.deepEqual(payload.sessionUpdates, []); + assert.deepEqual(payload.finalSession.policyPresets, ["pypi"]); + }); + + it("completes policy-add when no onboard session exists", () => { + const script = `${buildPreamble({ sessionMissing: true })} +const ctx = module.exports; +(async () => { + try { + await ctx.channelModule.addSandboxPolicy("test-sb", { preset: "github", yes: true }); + process.stdout.write("\\n__RESULT__" + JSON.stringify({ + calls: ctx.calls, + sessionUpdates: ctx.sessionUpdates, + }) + "\\n"); + } catch (err) { + process.stdout.write("\\n__RESULT__" + JSON.stringify({ error: err.message, stack: err.stack }) + "\\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 || ""}`); + + // Registry mutation succeeded; session-sync was a no-op (no session + // to keep in sync). policy-add must NOT abort the operation in this case. + assert.deepEqual(payload.calls.apply, [{ sandboxName: "test-sb", presetName: "github" }]); + assert.deepEqual(payload.sessionUpdates, []); + }); +});