Skip to content

Commit 183a9c8

Browse files
laitingshengprekshivyasdeepujaingithub-actions[bot]
authored
fix(onboard): retire a messaging channel preset the host no longer configures (#9296)
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 plain sentences: what changes and why. Describe before-and-after behavior when it applies. Follow the NemoClaw Writing Guide: https://github.com/NVIDIA/NemoClaw/blob/main/WRITING.md. Do not add unrelated prose cleanup. --> A messaging channel's network egress preset was re-applied on later onboarding runs even after the operator cleared every input the product exposes, because onboarding re-derived "the operator stopped configuring this channel" at each consumer instead of recording it, and one consumer could always miss it. Onboarding now re-reads host inputs on every reuse path, records the removal in the messaging plan that reaches the registry, derives its disabled-channel list from the applied preset list as well as the plans, and names the channel whose egress it dropped. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes #9283 ## Changes <!-- List concrete changes. If this adds an abstraction, configuration, fallback, migration, or compatibility path, name its current requirement and consumer, explain why a direct change is insufficient, and identify the test that protects it. --> - `selectionFromRegistryPlan` in `src/lib/onboard/machine/handlers/sandbox-messaging.ts` runs the host-input check on the channel-lifecycle branch as well. A plan last written by `add-channel`, `remove-channel`, `start-channel`, or `stop-channel` previously returned its recorded selection unchanged, so a channel the host no longer configures stayed selected and `detectUnconfiguredMessagingChannels` skipped it downstream. Covered by `omits a removed host-backed channel from a lifecycle-workflow registry plan` and `keeps a still-configured channel in a lifecycle-workflow registry plan`. - `filterUnconfiguredHostChannelsFromSelection` in the same file marks the channel inactive and disabled in the returned plan through the new `disableChannelsInPlan`, instead of filtering only the derived selection. The plan is what reaches the registry and the next run, so recording the removal once replaces rediscovering it at every reader. Covered by `records the removal in the plan so a later reader cannot re-enable it`, and by the two updated `#9109` cases. - `handlePoliciesState` in `src/lib/onboard/machine/handlers/policies.ts` adds the channels behind the sandbox's applied presets to the candidates it checks. The registry `policies` list outlives every messaging plan that named the channel, so without this candidate source a sandbox can carry a channel's egress with nothing left able to retire it. Covered by `disables a channel whose preset is applied but which no plan still names` and `leaves a still-configured channel enabled when its preset is applied`. - `messagingChannelsForPolicyPresets` in `src/lib/onboard/messaging-policy-presets.ts` maps preset names back to their channels for that check, alongside the existing channel-to-preset direction. - `filterUnconfiguredHostChannelsFromSelection` reports the channel it disabled, so a removal is visible in onboarding output rather than silent. - `docs/manage-sandboxes/enable-channels-during-onboarding.mdx` documents removing a channel by clearing its host inputs, including the in-sandbox QR pairing exemption. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review <!-- Required for code and documentation changes after the changes and applicable validation are complete. Keep one review checkbox and one instance of each visible or hidden field. For Evidence, list changed documentation paths. For documentation-only changes, also state that the writing rules and documentation style were reviewed. For other results, explain why no documentation change is needed or why the review is blocked. For Agent, use a consistent product and surface name, such as Codex Desktop, Codex CLI, Claude Code, or Cursor. After committing all review changes, put `git rev-parse --short HEAD` and `git rev-parse --short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review and refresh that metadata after any new commit. This receipt is advisory during the data-collection pilot. --> - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/manage-sandboxes/enable-channels-during-onboarding.mdx`. The review returned request-changes. It found that the draft told the reader to clear the stored credential with `credentials reset`, which takes an OpenShell provider name and does not change what onboarding detects, because a channel token is read from the environment alone. The corrections were dropping that step, quoting the CLI output with its real indentation, introducing the output block with `Expected output:`, splitting one sentence that carried three instructions, and routing the frontmatter for the new section. - Agent: Claude Code <!-- docs-review-head-sha: 55c71c1 --> <!-- docs-review-agents-blob-sha: b9fb6a9 --> ## DGX Station Hardware Evidence <!-- Required only when scripts/prepare-dgx-station-host.sh changes. Maintainers must review the linked evidence before approving or merging. This is human-reviewed evidence, not authenticated hardware provenance. Exceptional bypasses use existing repository governance and must be documented on the PR. --> - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run src/lib/onboard/machine/handlers/sandbox-messaging.test.ts src/lib/onboard/machine/handlers/policies.test.ts src/lib/onboard/messaging-policy-presets.test.ts src/lib/onboard/policy-selection-application.test.ts` — 4 files, 73 tests passed, rerun after merging `origin/main`. Reverting only the two changed handler sources makes 6 of the new and updated cases fail, so they guard the behavior rather than restate it. `npm run typecheck:cli` and `npm run lint` are clean. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added clearer onboarding guidance for stopping channel configuration, including removal behavior, network-policy cleanup, credential handling, and QR-paired channel exceptions. * Automatically disables messaging channels removed from the active configuration while preserving channels that remain configured. * Reports disabled channels during onboarding and sandbox lifecycle workflows. * **Bug Fixes** * Improved reconciliation of previously applied channel settings with the current messaging plan. * Preserved valid channel selections when policy presets are applied. * Ensured removed channels remain disabled across resumed and reused onboarding flows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: Deepak Jain <deepujain@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent ffbe742 commit 183a9c8

9 files changed

Lines changed: 375 additions & 30 deletions

docs/manage-sandboxes/enable-channels-during-onboarding.mdx

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
title: "Enable Channels During Onboarding"
55
sidebar-title: "Enable Channels During Onboarding"
66
description: "Select messaging channels and supply their credentials or pairing inputs during NemoClaw onboarding."
7-
description-agent: "Explains the interactive and scripted onboarding flows for selecting messaging channels and creating OpenShell bridge providers. Use when enabling channels on a new sandbox."
8-
keywords: ["nemoclaw onboard messaging", "messaging channel picker", "channel environment variables"]
7+
description-agent: "Explains the interactive and scripted onboarding flows for selecting messaging channels, creating OpenShell bridge providers, and removing a channel by clearing its host inputs. Use when enabling or disabling channels during onboarding."
8+
keywords: ["nemoclaw onboard messaging", "messaging channel picker", "channel environment variables", "disable messaging channel"]
99
content:
1010
type: "how_to"
1111
agent-variants: ["openclaw", "hermes"]
@@ -25,7 +25,7 @@ Refer to [Set Up Google Chat](set-up-google-chat) before selecting it.
2525
</AgentOnly>
2626

2727
If you select no channels, pressing **Enter** skips messaging setup.
28-
If a token-based channel token is not already in the environment or credential store, the wizard prompts for it and saves it.
28+
If the current host inputs do not include a token-based channel token, the wizard prompts for it and stages it for the current onboarding process.
2929

3030
If you enable WeChat, the wizard renders a QR code, polls Tencent's iLink gateway, and captures the bot token after you scan the QR with WeChat on your phone.
3131
The login has an eight-minute deadline, refreshes the QR up to three times on expiry, and follows iLink's IDC redirects automatically.
@@ -78,6 +78,28 @@ $$nemoclaw onboard
7878
Complete the wizard so the blueprint can create OpenShell providers where needed, such as `<sandbox>-telegram-bridge`, `<sandbox>-teams-bridge`, or `<sandbox>-wechat-bridge`.
7979
The wizard writes channel configuration into the image through `NEMOCLAW_MESSAGING_CHANNELS_B64` and starts the sandbox.
8080

81+
## Stop Configuring a Channel
82+
83+
Onboarding reads the host inputs on every run, so clearing a channel's inputs and re-onboarding removes it.
84+
Unset the channel's environment variables.
85+
Run onboarding again.
86+
87+
NemoClaw reports the removal and drops the channel's network policy preset with it, so the sandbox does not keep the wider egress of a channel it no longer serves.
88+
89+
Expected output:
90+
91+
```text
92+
No host inputs configure discord; disabling the channel and its network egress.
93+
[non-interactive] Applying policy presets: npm, pypi
94+
```
95+
96+
Onboarding uses the current host inputs to determine whether a token-based channel remains configured.
97+
`$$nemoclaw credentials reset` takes an OpenShell provider name and does not change those host inputs.
98+
99+
A QR-paired channel such as WhatsApp is exempt.
100+
The host holds no value that reports whether the pairing is still live, so an absent host input is not evidence that you removed the channel.
101+
Use [`channels remove`](manage-messaging-channels) for those.
102+
81103
## Verify the Result
82104

83105
After the sandbox is running, send a message to the configured bot or app.

src/lib/onboard/machine/handlers/policies.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,47 @@ describe("handlePoliciesState", () => {
100100
);
101101
});
102102

103+
it("disables a channel whose preset is applied but which no plan still names (#9283)", async () => {
104+
const { deps, calls } = createDeps({
105+
getActiveSandbox: vi.fn(() => ({
106+
messaging: null,
107+
policies: ["npm", "pypi", "discord"],
108+
})),
109+
detectUnconfiguredMessagingChannels: vi.fn(
110+
(planChannels: readonly string[]) => [...planChannels],
111+
),
112+
});
113+
114+
await handlePoliciesState({ ...baseOptions(deps), selectedMessagingChannels: [] });
115+
116+
expect(deps.detectUnconfiguredMessagingChannels).toHaveBeenCalledWith(["discord"], [], null);
117+
expect(calls.setupPolicies).toHaveBeenCalledWith(
118+
"my-assistant",
119+
expect.objectContaining({ enabledChannels: [], disabledChannels: ["discord"] }),
120+
);
121+
});
122+
123+
it("leaves a still-configured channel enabled when its preset is applied (#9283)", async () => {
124+
const { deps, calls } = createDeps({
125+
getActiveSandbox: vi.fn(() => ({
126+
messaging: null,
127+
policies: ["npm", "discord"],
128+
})),
129+
});
130+
131+
await handlePoliciesState({ ...baseOptions(deps), selectedMessagingChannels: ["discord"] });
132+
133+
expect(deps.detectUnconfiguredMessagingChannels).toHaveBeenCalledWith(
134+
["discord"],
135+
["discord"],
136+
null,
137+
);
138+
expect(calls.setupPolicies).toHaveBeenCalledWith(
139+
"my-assistant",
140+
expect.objectContaining({ enabledChannels: ["discord"], disabledChannels: [] }),
141+
);
142+
});
143+
103144
it("keeps a still-configured channel enabled", async () => {
104145
const { deps, calls } = createDeps({
105146
getActiveSandbox: vi.fn(() => ({

src/lib/onboard/machine/handlers/policies.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
getActiveChannelsFromPlan,
88
getDisabledChannelsFromPlan,
99
} from "../../messaging-plan-session";
10+
import { messagingChannelsForPolicyPresets } from "../../messaging-policy-presets";
1011
import type { HostLocalInferenceSandboxProofAuthority } from "../../runtime-provider/host-local-inference-routing";
1112
import { advanceTo, type OnboardStateTransitionResult } from "../result";
1213

@@ -26,6 +27,8 @@ export interface PolicyPresetEntry {
2627
export interface ActiveSandboxPolicyState {
2728
messaging?: { plan: SandboxMessagingPlan } | null;
2829
policyTier?: string | null;
30+
/** Preset names already applied to the sandbox, as recorded in the registry. */
31+
policies?: string[] | null;
2932
}
3033

3134
export interface PolicyResumeSelection {
@@ -177,8 +180,16 @@ export async function handlePoliciesState<Agent, WebSearchConfig>({
177180
// run re-applies its egress preset. Adding it to `disabledChannels` here lets
178181
// the existing disabled-channel pruning drop the preset from both the merged
179182
// selection and the previously-applied set.
183+
//
184+
// The applied preset list is the third candidate source because it outlives
185+
// the plans: a sandbox can carry a channel's egress in `policies` after every
186+
// plan that named the channel is gone, and only a candidate here can retire
187+
// it.
188+
const appliedPresetMessagingChannels = messagingChannelsForPolicyPresets(
189+
activeSandbox?.policies,
190+
);
180191
const unconfiguredMessagingChannels = deps.detectUnconfiguredMessagingChannels(
181-
[...recordedMessagingChannels, ...activeMessagingChannels],
192+
[...recordedMessagingChannels, ...activeMessagingChannels, ...appliedPresetMessagingChannels],
182193
selectedMessagingChannels,
183194
agent,
184195
);

src/lib/onboard/machine/handlers/sandbox-messaging.test.ts

Lines changed: 128 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from "../../../state/onboard-checkpoint-types";
1717
import { createSession, type Session } from "../../../state/onboard-session";
1818
import { setupMessagingChannels } from "../../messaging-channel-setup";
19+
import { getActiveChannelsFromPlan } from "../../messaging-plan-session";
1920
import {
2021
hasMessagingCredentialDrift,
2122
reconcileReusedSandboxMessaging,
@@ -186,6 +187,21 @@ function discordPlan(credentialHash: string): SandboxMessagingPlan {
186187
};
187188
}
188189

190+
function withChannelDisabled(
191+
plan: SandboxMessagingPlan,
192+
channelId: string,
193+
): SandboxMessagingPlan {
194+
return {
195+
...plan,
196+
channels: plan.channels.map((channel) =>
197+
channel.channelId === channelId
198+
? { ...channel, active: false, selected: false, disabled: true }
199+
: channel,
200+
),
201+
disabledChannels: [...new Set([...plan.disabledChannels, channelId])],
202+
};
203+
}
204+
189205
function whatsappPlan(): SandboxMessagingPlan {
190206
return {
191207
...telegramPlan(""),
@@ -359,14 +375,12 @@ describe("reconcileReusedSandboxMessaging", () => {
359375
it("does not clear an equal recorded plan from a different authority", () => {
360376
const plan = telegramPlan(hashCredential("123456:registry-token") ?? "");
361377
const clearPlanEnv = vi.fn();
362-
// Keep the channel host-configured so this case stays about plan equality,
363-
// not the #9283 unconfigured-channel selection filter.
364378
vi.stubEnv("TELEGRAM_BOT_TOKEN", "123456:registry-token");
365379

366380
const result = reconcileReusedSandboxMessaging(
367381
structuredClone(plan),
368382
{ name: "openclaw" },
369-
{ clearPlanEnv },
383+
{ clearPlanEnv, note: vi.fn(), writePlanToEnv: vi.fn() },
370384
plan,
371385
);
372386

@@ -386,10 +400,13 @@ describe("reconcileReusedSandboxMessaging", () => {
386400
plan,
387401
);
388402

389-
// The plan still records the channel — only the reported selection drops
390-
// it, so the policies handler classifies it as unconfigured and prunes its
391-
// egress preset instead of re-applying it on every later onboarding run.
392-
expect(result).toEqual({ plan, selectedChannels: [], changed: false });
403+
// Persist the removal so later readers cannot re-enable the channel and
404+
// re-apply its egress preset.
405+
expect(result).toEqual({
406+
plan: withChannelDisabled(plan, "discord"),
407+
selectedChannels: [],
408+
changed: true,
409+
});
393410
expect(clearPlanEnv).not.toHaveBeenCalled();
394411
});
395412

@@ -425,13 +442,11 @@ describe("reconcileReusedSandboxMessaging", () => {
425442
});
426443

427444
it("removes every unsupported channel artifact from a reused plan", () => {
428-
// Keep the channel host-configured so this case stays about unsupported
429-
// artifact removal, not the #9283 unconfigured-channel selection filter.
430445
vi.stubEnv("TELEGRAM_BOT_TOKEN", "123456:registry-token");
431446
const result = reconcileReusedSandboxMessaging(
432447
mixedChannelPlan(),
433448
{ name: "openclaw" },
434-
{ clearPlanEnv() {} },
449+
{ clearPlanEnv() {}, note() {}, writePlanToEnv() {} },
435450
);
436451
const filtered = result.plan;
437452

@@ -466,6 +481,25 @@ describe("reconcileReusedSandboxMessaging", () => {
466481
healthChecks: ["telegram"],
467482
});
468483
});
484+
485+
it("disables and stages an unconfigured host-backed channel for Ready sandbox reuse (#9283)", () => {
486+
const plan = discordPlan(hashCredential("previous-discord-token") ?? "");
487+
const deps = reconcileDeps([]);
488+
vi.stubEnv("DISCORD_BOT_TOKEN", "");
489+
490+
const result = reconcileReusedSandboxMessaging(
491+
plan,
492+
{ name: "openclaw" },
493+
deps,
494+
structuredClone(plan),
495+
);
496+
const disabledPlan = withChannelDisabled(plan, "discord");
497+
498+
expect(result).toEqual({ plan: disabledPlan, selectedChannels: [], changed: true });
499+
expect(deps.writePlanToEnv).toHaveBeenLastCalledWith(disabledPlan);
500+
expect(deps.clearPlanEnv).not.toHaveBeenCalled();
501+
expect(deps.note).toHaveBeenCalledWith(expect.stringContaining("No host inputs configure"));
502+
});
469503
});
470504

471505
describe("reconcileSandboxMessaging plan authority", () => {
@@ -536,7 +570,86 @@ describe("reconcileSandboxMessaging plan authority", () => {
536570
});
537571

538572
expect(deps.setupMessagingChannels).not.toHaveBeenCalled();
539-
expect(result).toEqual({ plan: registryPlan, selectedChannels: [] });
573+
expect(result).toEqual({
574+
plan: withChannelDisabled(registryPlan, "discord"),
575+
selectedChannels: [],
576+
});
577+
});
578+
579+
it("records the removal in the plan so a later reader cannot re-enable it (#9283)", async () => {
580+
const registryPlan = discordPlan(hashCredential("previous-discord-token") ?? "");
581+
const disabledPlan = withChannelDisabled(registryPlan, "discord");
582+
const deps = reconcileDeps([]);
583+
deps.getRegistrySandboxMessagingAuthority.mockReturnValue({
584+
authoritative: true,
585+
plan: registryPlan,
586+
});
587+
vi.stubEnv("DISCORD_BOT_TOKEN", "");
588+
589+
const result = await reconcileSandboxMessaging({
590+
resume: false,
591+
session: null,
592+
sandboxName: "alpha",
593+
agent: { name: "openclaw" },
594+
deps,
595+
});
596+
597+
expect(getActiveChannelsFromPlan(result.plan)).toEqual([]);
598+
expect(result.plan?.disabledChannels).toEqual(["discord"]);
599+
expect(deps.writePlanToEnv).toHaveBeenLastCalledWith(disabledPlan);
600+
expect(deps.note).toHaveBeenCalledWith(expect.stringContaining("No host inputs configure"));
601+
});
602+
603+
it("omits a removed host-backed channel from a lifecycle-workflow registry plan (#9283)", async () => {
604+
const registryPlan = {
605+
...discordPlan(hashCredential("previous-discord-token") ?? ""),
606+
workflow: "add-channel" as const,
607+
};
608+
const deps = reconcileDeps([]);
609+
deps.getRegistrySandboxMessagingAuthority.mockReturnValue({
610+
authoritative: true,
611+
plan: registryPlan,
612+
});
613+
vi.stubEnv("DISCORD_BOT_TOKEN", "");
614+
615+
const result = await reconcileSandboxMessaging({
616+
resume: false,
617+
session: null,
618+
sandboxName: "alpha",
619+
agent: { name: "openclaw" },
620+
deps,
621+
});
622+
623+
expect(deps.setupMessagingChannels).not.toHaveBeenCalled();
624+
expect(result).toEqual({
625+
plan: withChannelDisabled(registryPlan, "discord"),
626+
selectedChannels: [],
627+
});
628+
});
629+
630+
it("keeps a still-configured channel in a lifecycle-workflow registry plan (#9283)", async () => {
631+
const token = "still-configured-discord-token";
632+
const registryPlan = {
633+
...discordPlan(hashCredential(token) ?? ""),
634+
workflow: "add-channel" as const,
635+
};
636+
const deps = reconcileDeps([]);
637+
deps.getRegistrySandboxMessagingAuthority.mockReturnValue({
638+
authoritative: true,
639+
plan: registryPlan,
640+
});
641+
vi.stubEnv("DISCORD_BOT_TOKEN", token);
642+
643+
const result = await reconcileSandboxMessaging({
644+
resume: false,
645+
session: null,
646+
sandboxName: "alpha",
647+
agent: { name: "openclaw" },
648+
deps,
649+
});
650+
651+
expect(result.selectedChannels).toEqual(["discord"]);
652+
expect(result.plan?.disabledChannels).toEqual([]);
540653
});
541654

542655
it("omits a removed host-backed channel from a completed registry resume (#9109)", async () => {
@@ -557,7 +670,10 @@ describe("reconcileSandboxMessaging plan authority", () => {
557670
});
558671

559672
expect(deps.setupMessagingChannels).not.toHaveBeenCalled();
560-
expect(result).toEqual({ plan: registryPlan, selectedChannels: [] });
673+
expect(result).toEqual({
674+
plan: withChannelDisabled(registryPlan, "discord"),
675+
selectedChannels: [],
676+
});
561677
});
562678

563679
it("omits a retired host-backed channel from recorded resume channels (#9283)", async () => {

0 commit comments

Comments
 (0)