feat(channels): add host-side channels command + block sandbox-side mutation#2139
Merged
feat(channels): add host-side channels command + block sandbox-side mutation#2139
Conversation
…utation `openclaw channels add|remove` fails with EACCES inside the sandbox because `/sandbox/.openclaw/openclaw.json` is read-only (Landlock + DAC hardening), and `nemoclaw stop` only tears down auxiliary host services. That leaves no supported path to add or remove a messaging channel after onboarding short of rerunning `nemoclaw onboard --resume` and navigating the full TUI. Add a surgical host-side verb and block the sandbox-side mutation with a clear redirect: - New `nemoclaw <name> channels <list|add|remove>` handlers that persist / clear tokens under `~/.nemoclaw/credentials.json` and offer to rebuild the sandbox so the image reflects the change. Honour `NEMOCLAW_NON_INTERACTIVE=1` by queuing the change and pointing at `nemoclaw <name> rebuild` for the follow-up. - Extend `install_configure_guard` in `nemoclaw-start.sh` to intercept `openclaw channels <mutator>` from inside the sandbox with an actionable error pointing at the host-side verbs. Read-only subcommands (`list`, help) fall through to the real binary. Consolidate channel metadata into `src/lib/sandbox-channels.ts` as the single source of truth — `setupMessagingChannels` in `onboard.ts` now materialises its array view via `listChannels()` instead of carrying a duplicate inline definition. Fixes #2097. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds host-side messaging-channel management: new Changes
Sequence DiagramsequenceDiagram
participant User as "User"
participant Host as "nemoclaw (host)"
participant Creds as "~/.nemoclaw/credentials.json"
participant Rebuild as "sandboxRebuild"
participant Sandbox as "NemoClaw Sandbox"
User->>Host: nemoclaw <name> channels add <channel>
Host->>Host: validate channel (getChannelDef)
Host->>Host: determine token keys (getChannelTokenKeys)
alt Interactive
Host->>User: prompt for missing tokens
User->>Host: provide tokens
else Non-Interactive
Host->>Host: fail if tokens missing or queue change
end
Host->>Creds: persistChannelTokens()
Creds-->>Host: saved
alt Rebuild requested/confirmed
Host->>Rebuild: sandboxRebuild(--yes)
Rebuild->>Sandbox: rebuild image with updated creds
Sandbox-->>Rebuild: complete
else Queued for manual rebuild
Host-->>User: instruct "nemoclaw <name> rebuild"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
laitingsheng
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
4503-4503: Derive the toggle hint fromMESSAGING_CHANNELS.length.Now that the channel list is data-driven, the selector prompt in
setupMessagingChannels()is the only place still assuming exactly three entries (Press 1-3 to toggle). The next channel addition will make that prompt wrong even though the selector logic already supports arbitrary lengths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` at line 4503, The selector prompt in setupMessagingChannels() hardcodes "Press 1-3 to toggle" but MESSAGING_CHANNELS is now data-driven; update setupMessagingChannels() to compute the toggle hint from MESSAGING_CHANNELS.length (e.g., build "Press 1-N to toggle" or list the valid indices dynamically) and replace the hardcoded string so the prompt always reflects the current length of the MESSAGING_CHANNELS array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/troubleshooting.md`:
- Around line 452-456: Update the section heading to reflect the new behavior:
replace the current heading "`openclaw channels add` or `remove` fails with
`EACCES` inside the sandbox" with a phrasing that indicates NemoClaw surfaces an
actionable error (e.g. "NemoClaw surfaces an actionable error when `openclaw
channels add` or `remove` is run inside the sandbox"); ensure the body still
explains that the sandbox keeps /sandbox/.openclaw/openclaw.json read-only
(Landlock + filesystem hardening) and that NemoClaw intercepts and presents its
own user-facing error instead of a raw EACCES trace.
In `@src/lib/sandbox-channels.ts`:
- Around line 77-79: Extend the set of persisted keys cleared when removing a
channel so we remove all channel-specific identifiers (not just bot tokens):
update getChannelTokenKeys (and the corresponding logic used by
clearChannelTokens) to return envKey, appTokenEnvKey plus identifiers such as
DISCORD_SERVER_ID, DISCORD_USER_ID, DISCORD_REQUIRE_MENTION,
TELEGRAM_ALLOWED_IDS (and any other channel-specific keys used elsewhere).
Ensure clearChannelTokens iterates this expanded list and deletes every returned
key from persisted credentials so no stale identifiers remain after channels
remove; apply the same change to the analogous code referenced around lines
87-90.
In `@src/nemoclaw.ts`:
- Around line 2651-2653: Update the help text for the "nemoclaw <name> channels
list" entries to accurately reflect behavior: change the description from "Show
configured messaging channels" to something like "List supported/available
messaging channels (does not check configured credentials)" in both occurrences
(the help string around the "nemoclaw <name> channels list" entry at the top and
the duplicate at lines ~2875–2877). Locate the help text strings for the
channels subcommands in src/nemoclaw.ts (the "nemoclaw <name> channels list"
literal) and revise both descriptions so the wording matches the actual behavior
of listing static known channels rather than reading configured channels.
- Around line 1669-1746: sandboxChannelsAdd and sandboxChannelsRemove accept any
flags/extra args and proceed to mutate credentials; validate inputs first by
ensuring args contain exactly one non-flag positional (channelArg) and that all
flags are from the allowed set (only "--dry-run") before doing any token
prompting or calling persistChannelTokens/clearChannelTokens. Modify
sandboxChannelsAdd and sandboxChannelsRemove to parse args up-front: collect
positional args and flag names, reject when positional count != 1 or any flag is
not "--dry-run" (print usage/valid channels and exit), then continue with
existing logic (prompting, persisting, clearing) so no mutations occur for bad
input.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Line 4503: The selector prompt in setupMessagingChannels() hardcodes "Press
1-3 to toggle" but MESSAGING_CHANNELS is now data-driven; update
setupMessagingChannels() to compute the toggle hint from
MESSAGING_CHANNELS.length (e.g., build "Press 1-N to toggle" or list the valid
indices dynamically) and replace the hardcoded string so the prompt always
reflects the current length of the MESSAGING_CHANNELS array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 514261e2-a3fc-491c-af43-5ca94c3e6f67
📒 Files selected for processing (10)
.agents/skills/nemoclaw-user-reference/references/commands.md.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/commands.mddocs/reference/troubleshooting.mdscripts/nemoclaw-start.shsrc/lib/onboard.tssrc/lib/sandbox-channels.test.tssrc/lib/sandbox-channels.tssrc/nemoclaw.tstest/nemoclaw-start.test.ts
… list behaviour - `channels list` is a static reference, not a credentials check; update the main help block and the `channels` dispatch-fallback usage line so neither claims to show "configured" channels. - Retitle the troubleshooting entry to describe the sandbox-side guard (`openclaw channels add|remove` is blocked), since users no longer see a raw `EACCES` — the entrypoint's guard intercepts and prints a redirect to the host-side verbs first. Addresses review feedback on #2139. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nemoclaw.ts`:
- Around line 1684-1714: The dry-run branch is checked after
getChannelTokenKeys() and prompting logic, so channels add --dry-run still reads
credentials and prompts; move the dryRun check up to immediately after channel
validation and before calling getChannelTokenKeys or any credential
lookup/prompting: if (dryRun) should log the --dry-run message referencing
channelArg and sandboxName and return early, avoiding creation/use of tokenKeys,
acquired, getCredential, isNonInteractive checks, and askPrompt calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06297fdb-6cd2-48e8-b360-28c89d56d47f
📒 Files selected for processing (3)
.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdsrc/nemoclaw.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/reference/troubleshooting.md
- .agents/skills/nemoclaw-user-reference/references/troubleshooting.md
…, clarify docs - Replace the hardcoded "Press 1-3 to toggle" label in `setupMessagingChannels` with `Press 1-N to toggle`, where N is derived from `MESSAGING_CHANNELS.length`, so adding a future channel does not silently invalidate the prompt. - Move the `--dry-run` short-circuit in `channels add` to fire right after channel validation, before any credential lookup or interactive prompt — dry-run now never reads credentials or asks the user for tokens. - Expand the "channels blocked inside the sandbox" troubleshooting entry with a concrete explanation of what "baked config" means: the selected channel list is passed into `docker build` via `NEMOCLAW_MESSAGING_CHANNELS_B64` and written into `/sandbox/.openclaw/openclaw.json` as a layer of the sandbox image, then mounted read-only at runtime. Addresses review feedback on #2139. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
wscurran
added
the
priority: high
ericksoa
approved these changes
Apr 21, 2026
Contributor
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed: well-structured host-side channels management feature. Clean module extraction, proper sandbox guard, tests and docs all in order. All CodeRabbit findings addressed. LGTM.
miyoungc
added a commit
that referenced
this pull request
Apr 22, 2026
## Summary Bumps the published doc version to `0.0.22` and documents the user-visible CLI behavior changes to `nemoclaw <name> connect` that landed since v0.0.21. Drafted via the `nemoclaw-contributor-update-docs` skill against commits in `v0.0.21..origin/main`, filtered through `docs/.docs-skip`. ## Changes - **`docs/project.json`** and **`docs/versions1.json`**: bump the published version from `0.0.20` to `0.0.22`; insert a `0.0.21` entry into the version list so the history stays contiguous. - **`docs/reference/commands.md`** → `nemoclaw <name> connect`: document two new behaviors. - Readiness poll with `NEMOCLAW_CONNECT_TIMEOUT` (integer seconds; default `120`) that replaces the silent hang when the sandbox is not yet `Ready` — right after onboarding, while the 2.4 GB image is still pulling (#466). - Post-connect hint is now agent-aware, names the correct TUI command for the sandbox's agent, and tells you to use `/exit` to leave the chat before `exit` returns you to the host shell (#2080). Feature PRs that shipped their own docs in the same commit are intentionally not re-documented here: - `channels list/add/remove` (#2139) — command reference and the "`openclaw channels` blocked inside the sandbox" troubleshooting entry landed with the feature. - `nemoclaw gc` (#2176) — documented as part of the destroy/rebuild image cleanup PR. Skipped per `docs/.docs-skip`: - `e6bad533 fix(shields): verify config lock and fail hard on re-lock failure (#2066)` — matched `skip-features: src/lib/shields.ts`. Other commits in the range (#2141 OpenShell version bump, #1819 plugin banner live inference probe, #2085 / #2146 Slack Socket Mode fixes, #2110 axios proxy fix, #1818 NIM curl timeouts, #1824 onboard gateway bootstrap recovery, and assorted CI / test / install plumbing) are internal behavior refinements with no doc-relevant surface change. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes for the modified files via the pre-commit hook, including `Regenerate agent skills from docs` (source ↔ generated parity confirmed) - [ ] `npm test` passes — skipped; the one pre-existing `test/cli.test.ts > unknown command exits 1` failure on `origin/main` is unrelated to these markdown/JSON-only changes - [ ] Tests added or updated for new or changed behavior — n/a, doc-only - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) — not run locally - [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) — n/a, no new pages ## AI Disclosure - [x] AI-assisted — tool: Claude Code --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * `connect` now displays the sandbox phase while waiting for readiness and honors a configurable timeout via NEMOCLAW_CONNECT_TIMEOUT (default 120s). * TTY hints are agent-aware and instruct using `/exit` before returning to the host shell. * **Documentation** * Command docs updated to describe polling, timeout, and TTY guidance. * Project/docs metadata updated for versions 0.0.21 and 0.0.22 (package version bumped to 0.0.22). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
openclaw channels add|removefails with EACCES inside the sandbox because/sandbox/.openclaw/openclaw.jsonis read-only (Landlock + DAC hardening), andnemoclaw stoponly tears down auxiliary host services. That leaves no supported path to add or remove a messaging channel after onboarding short of rerunningnemoclaw onboard --resumeand navigating the full TUI.Add a surgical host-side verb and block the sandbox-side mutation with a clear redirect:
nemoclaw <name> channels <list|add|remove>handlers that persist / clear tokens under~/.nemoclaw/credentials.jsonand offer to rebuild the sandbox so the image reflects the change. HonourNEMOCLAW_NON_INTERACTIVE=1by queuing the change and pointing atnemoclaw <name> rebuildfor the follow-up.install_configure_guardinnemoclaw-start.shto interceptopenclaw channels <mutator>from inside the sandbox with an actionable error pointing at the host-side verbs. Read-only subcommands (list, help) fall through to the real binary.Consolidate channel metadata into
src/lib/sandbox-channels.tsas the single source of truth —setupMessagingChannelsinonboard.tsnow materialises its array view vialistChannels()instead of carrying a duplicate inline definition.Fixes #2097.
Summary
Related Issue
Changes
Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
nemoclaw <name> channels list|add|removewith--dry-run.listis read-only;addpersists tokens (idempotent),removeclears them. Slack requires both bot+app tokens. Rebuild offered or queued when NEMOCLAW_NON_INTERACTIVE=1.Documentation
Tests