Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions src/lib/actions/sandbox/policy-list-render.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

// Regression for #5967: `nemoclaw <sandbox> policy-list` must render `● discord`
// (and any enabled messaging channel preset) once it is recorded in the registry
// and active on the gateway. This is the reporter's observation step — the
// rendered marker the operator actually reads — complementing the merge/persist
// tests that cover the upstream state policy-list consumes.

import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

vi.mock("../../policy", async (importOriginal) => {
const actual = await importOriginal<typeof import("../../policy")>();
return {
...actual,
listPresets: vi.fn(),
listCustomPresets: vi.fn(),
getAppliedPresets: vi.fn(),
getGatewayPresets: vi.fn(),
};
});

import * as policies from "../../policy";
import { listSandboxPolicies } from "./policy-channel";

const mocked = vi.mocked(policies);

describe("listSandboxPolicies rendering (#5967)", () => {
let logSpy: ReturnType<typeof vi.spyOn>;
let lines: string[];

beforeEach(() => {
lines = [];
logSpy = vi.spyOn(console, "log").mockImplementation((...args: unknown[]) => {
lines.push(args.join(" "));
});
mocked.listPresets.mockReturnValue([
{
name: "discord",
description: "Discord API, gateway, and CDN access",
file: "discord.yaml",
},
{
name: "slack",
description: "Slack API, Socket Mode, and webhooks access",
file: "slack.yaml",
},
{ name: "npm", description: "npm and Yarn registry access", file: "npm.yaml" },
]);
mocked.listCustomPresets.mockReturnValue([]);
});

afterEach(() => {
logSpy.mockRestore();
vi.clearAllMocks();
});

// Match the rendered marker + preset name directly. The row may carry a
// provenance tag (e.g. `● discord [user-added] — …`) between the name and the
// description, so keying off the marker+name is robust to that suffix.
const lineFor = (preset: string) =>
lines.find((line) => new RegExp(`[●○] ${preset}\\b`).test(line)) ?? "";

it("marks an enabled Discord preset applied (●) when it is in both registry and gateway", () => {
// The #5967 fix persists `discord` to registry.policies AND applies it to the
// gateway, so policy-list must render it as applied.
mocked.getAppliedPresets.mockReturnValue(["discord", "npm"]);
mocked.getGatewayPresets.mockReturnValue(["discord", "npm"]);

listSandboxPolicies("nemoclaw-5967");

expect(lineFor("discord")).toContain("● discord");
expect(lineFor("npm")).toContain("● npm");
// A channel that was never configured stays unapplied.
expect(lineFor("slack")).toContain("○ slack");
expect(lineFor("slack")).not.toContain("● slack");
});

it("renders the pre-fix regression (○ discord) when Discord is dropped from registry and gateway", () => {
// Before the fix the explicit-selection path dropped discord from both the
// persisted registry list and the reconciled gateway set.
mocked.getAppliedPresets.mockReturnValue(["npm", "pypi"]);
mocked.getGatewayPresets.mockReturnValue(["npm", "pypi"]);

listSandboxPolicies("nemoclaw-5967");

expect(lineFor("discord")).toContain("○ discord");
expect(lineFor("discord")).not.toContain("● discord");
});

it("flags a registry/gateway mismatch when Discord is recorded but not active on the gateway", () => {
mocked.getAppliedPresets.mockReturnValue(["discord", "npm"]);
mocked.getGatewayPresets.mockReturnValue(["npm"]);

listSandboxPolicies("nemoclaw-5967");

expect(lineFor("discord")).toContain("○ discord");
expect(lineFor("discord")).toContain("recorded locally, not active on gateway");
});
});
74 changes: 70 additions & 4 deletions src/lib/onboard/messaging-policy-presets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
allMessagingChannelPolicyPresets,
hasDisabledMessagingPolicyPreset,
mergeAppliedPolicyPresetsForDisabledMessagingCleanup,
mergeEnabledMessagingChannelPolicyPresets,
mergePolicyMessagingChannels,
mergeRebuildMessagingPolicyPresets,
mergeRequiredMessagingChannelPolicyPresets,
pruneDisabledMessagingPolicyPresets,
requiredMessagingChannelPolicyPresets,
} from "./messaging-policy-presets";
Expand All @@ -21,16 +21,35 @@ describe("messaging policy presets", () => {
});

it("merges required messaging presets into an existing selection", () => {
expect(mergeRequiredMessagingChannelPolicyPresets(["npm", "pypi"], ["slack"])).toEqual([
expect(mergeEnabledMessagingChannelPolicyPresets(["npm", "pypi"], ["slack"])).toEqual([
"npm",
"pypi",
"slack",
]);
});

it("does not add a required preset that is not available to the sandbox", () => {
// #5967: a channel that is not flagged requiredAtCreate (Discord, Telegram,
// WhatsApp, Teams, WeChat) still needs its egress preset merged so policy
// finalization persists it and policy-list marks it applied.
it("merges an enabled channel preset that is not required at create time", () => {
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["discord"])).toEqual([
"npm",
"discord",
]);
expect(requiredMessagingChannelPolicyPresets(["discord"])).toEqual([]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["slack", "discord"])).toEqual([
"npm",
"slack",
"discord",
]);
});

it("does not add a channel preset that is not available to the sandbox", () => {
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["slack"], new Set(["npm"]))).toEqual(
["npm"],
);
expect(
mergeRequiredMessagingChannelPolicyPresets(["npm"], ["slack"], new Set(["npm"])),
mergeEnabledMessagingChannelPolicyPresets(["npm"], ["discord"], new Set(["npm"])),
).toEqual(["npm"]);
});

Expand Down Expand Up @@ -103,4 +122,51 @@ describe("messaging policy presets", () => {
mergeAppliedPolicyPresetsForDisabledMessagingCleanup(["npm"], ["npm", "github"], ["slack"]),
).toEqual(["npm"]);
});

// #5967 is channel-agnostic: every non-`requiredAtCreate` channel (Telegram,
// Teams, WhatsApp, WeChat) must merge and prune exactly like Discord. Cover the
// remaining channels explicitly so a future channel-table regression cannot pass
// on Slack/Discord alone.
it("merges every enabled non-required channel preset, not just Slack and Discord (#5967)", () => {
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["telegram"])).toEqual([
"npm",
"telegram",
]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["teams"])).toEqual(["npm", "teams"]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["whatsapp"])).toEqual([
"npm",
"whatsapp",
]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["wechat"])).toEqual([
"npm",
"wechat",
]);
});

it("prunes every disabled non-required channel preset (#5967)", () => {
expect(pruneDisabledMessagingPolicyPresets(["npm", "whatsapp"], ["whatsapp"])).toEqual(["npm"]);
expect(pruneDisabledMessagingPolicyPresets(["npm", "wechat"], ["wechat"])).toEqual(["npm"]);
});

it("leaves the selection untouched when no channels are enabled (#5967)", () => {
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], [])).toEqual(["npm"]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], null)).toEqual(["npm"]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], undefined)).toEqual(["npm"]);
});

it("yields no preset for an unknown channel name (#5967)", () => {
expect(allMessagingChannelPolicyPresets(["nonexistent"])).toEqual([]);
expect(mergeEnabledMessagingChannelPolicyPresets(["npm"], ["nonexistent"])).toEqual(["npm"]);
});

// Drift guard (#5967): the suggestion path's `add(channel)` shortcut was
// removed in favor of resolving presets through the channel→preset registry,
// and several call sites assume a channel's egress preset shares its name.
// Pin that 1:1 mapping for every shipped channel so a future preset rename
// (which would silently desync suggestions from finalization) fails here.
it("maps each messaging channel to a same-named egress preset (#5967)", () => {
for (const channel of ["slack", "discord", "telegram", "teams", "whatsapp", "wechat"]) {
expect(allMessagingChannelPolicyPresets([channel])).toEqual([channel]);
}
});
});
16 changes: 14 additions & 2 deletions src/lib/onboard/messaging-policy-presets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,19 @@ export function requiredMessagingChannelPolicyPresets(
return required;
}

export function mergeRequiredMessagingChannelPolicyPresets(
// Merge the policy presets every enabled messaging channel needs into a
// selection. An enabled channel cannot function without its network-egress
// preset, so that preset must survive policy finalization regardless of how the
// operator arrived at the selection (interactive tier, env-driven custom list,
// or a recorded resume set). We intentionally merge *all* of a channel's
// presets, not just the create-time `requiredAtCreate` ones: `requiredAtCreate`
// governs whether a preset is injected into the boot policy at sandbox-create
// time (only Slack today), while finalization applies any newly-merged preset
// to the live gateway itself. Using only the create-time-required set here drops
// every other channel's preset (Discord, Telegram, WhatsApp, Teams, WeChat) from
// the persisted selection, so `policy-list` shows them unapplied even though the
// channel was configured during onboard. See #5967.
export function mergeEnabledMessagingChannelPolicyPresets(
selectedPresets: string[],
channels: string[] | null | undefined,
knownPresetNames?: Iterable<string> | null,
Expand All @@ -61,7 +73,7 @@ export function mergeRequiredMessagingChannelPolicyPresets(
const selected = new Set(merged);
const known = knownPresetNames ? new Set(knownPresetNames) : null;

for (const preset of requiredMessagingChannelPolicyPresets(channels)) {
for (const preset of allMessagingChannelPolicyPresets(channels)) {
if (known && !known.has(preset)) continue;
if (selected.has(preset)) continue;
merged.push(preset);
Expand Down
7 changes: 3 additions & 4 deletions src/lib/onboard/openclaw-otel-policy-presets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { afterEach, describe, expect, it, vi } from "vitest";

vi.mock("./messaging-policy-presets", () => ({
mergeRequiredMessagingChannelPolicyPresets: (presets: string[]) => presets,
mergeEnabledMessagingChannelPolicyPresets: (presets: string[]) => presets,
requiredMessagingChannelPolicyPresets: () => [],
pruneDisabledMessagingPolicyPresets: (presets: string[]) => presets,
mergeAppliedPolicyPresetsForDisabledMessagingCleanup: (presets: string[]) => presets,
Expand All @@ -16,14 +16,13 @@ vi.mock("./hermes-managed-tools", () => ({
HERMES_TOOL_GATEWAY_PRESET_NAMES: new Set(),
}));

import { mergeRequiredSetupPolicyPresets } from "./policy-selection";

import {
OPENCLAW_OTEL_LOCAL_POLICY_PRESET,
isOpenclawOtelEnabled,
mergeRequiredOpenclawOtelPolicyPresets,
OPENCLAW_OTEL_LOCAL_POLICY_PRESET,
requiredOpenclawOtelPolicyPresets,
} from "./openclaw-otel-policy-presets";
import { mergeRequiredSetupPolicyPresets } from "./policy-selection";

describe("openclaw-otel-policy-presets", () => {
const originalOtel = process.env.NEMOCLAW_OPENCLAW_OTEL;
Expand Down
31 changes: 31 additions & 0 deletions src/lib/onboard/policy-preset-persistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,35 @@ describe("persistFinalizedPolicyPresets (#4621)", () => {
policyPresetsFinalized: true,
});
});

// #5967 was a registry-persistence regression: an enabled messaging channel's
// preset reached the live gateway but was dropped from the registry `policies`
// write, so `policy-list` (which reads registry.policies) rendered `○`. Using
// the REAL built-in preset catalog proves Discord and Slack are recognized as
// built-ins and are written back to the registry, not filtered out.
it("persists enabled messaging channel presets (Discord, Slack) to the registry (#5967)", () => {
// Model the registry as observable state and read it back through the same
// boundary policy-list uses (registry.getSandbox().policies) rather than
// inspecting updateSandbox's call shape.
const entry = { name: "sb", policies: ["npm"] } as Partial<registry.SandboxEntry> & {
policies: string[];
policyPresetsFinalized?: boolean;
};
vi.spyOn(registry, "getCustomPolicies").mockReturnValue([]);
vi.spyOn(registry, "getSandbox").mockImplementation((name) =>
name === "sb" ? (entry as registry.SandboxEntry) : null,
);
vi.spyOn(registry, "updateSandbox").mockImplementation((_name, fields) => {
Object.assign(entry, fields);
return true;
});

persistFinalizedPolicyPresets("sb", ["npm", "pypi", "discord", "slack"]);

const stored = registry.getSandbox("sb");
expect(stored?.policyPresetsFinalized).toBe(true);
// Discord and Slack survive the built-in filter and are stored where
// policy-list reads them — the #5967 registry-persistence guarantee.
expect([...(stored?.policies ?? [])].sort()).toEqual(["discord", "npm", "pypi", "slack"]);
});
});
15 changes: 10 additions & 5 deletions src/lib/onboard/policy-selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import {
mergeRequiredHermesToolGatewayPolicyPresets,
} from "./hermes-managed-tools";
import {
mergeRequiredMessagingChannelPolicyPresets,
allMessagingChannelPolicyPresets,
mergeEnabledMessagingChannelPolicyPresets,
pruneDisabledMessagingPolicyPresets,
requiredMessagingChannelPolicyPresets,
} from "./messaging-policy-presets";
import { mergeRequiredOpenclawOtelPolicyPresets } from "./openclaw-otel-policy-presets";
import { seedInitialPolicyContext } from "./policy-context-seed";
Expand Down Expand Up @@ -118,7 +118,7 @@ export function mergeRequiredSetupPolicyPresets(
): string[] {
const agentFilteredPresets = filterSetupPolicyPresetNamesForAgent(policyPresets, options.agent);
const mergedPresets = mergeRequiredOpenclawOtelPolicyPresets(
mergeRequiredMessagingChannelPolicyPresets(
mergeEnabledMessagingChannelPolicyPresets(
mergeRequiredHermesToolGatewayPolicyPresets(
agentFilteredPresets,
options.hermesToolGateways,
Expand Down Expand Up @@ -189,8 +189,13 @@ export function computeSetupPresetSuggestions(
for (const preset of allHermesToolGatewayPolicyPresets()) add(preset);
}
if (Array.isArray(enabledChannels)) {
for (const channel of enabledChannels) add(channel);
for (const preset of requiredMessagingChannelPolicyPresets(enabledChannels)) add(preset);
// Suggest every enabled channel's egress preset, matching the set
// finalization merges via `mergeEnabledMessagingChannelPolicyPresets`.
// Resolving through the channel→preset registry keeps the suggestion path
// correct for any channel (and any future preset rename) without relying on
// the channel name coinciding with its preset name or on `requiredAtCreate`
// (#5967).
for (const preset of allMessagingChannelPolicyPresets(enabledChannels)) add(preset);
}
if (Array.isArray(options.hermesToolGateways)) {
for (const preset of options.hermesToolGateways) {
Expand Down
Loading
Loading