fix(telegram): honor DM allowlist aliases#4554
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
📝 WalkthroughWalkthroughAdds alias-aware Telegram messaging config resolution (canonicalizes legacy keys into TELEGRAM_ALLOWED_IDS), backfills and warns in onboarding/post-rebuild flows, emits runtime diagnostics for Bot API sendMessage/getUpdates and config state, updates e2e scripts for alias handling and optional inbound-reply probes, expands tests, and updates docs. ChangesTelegram messaging DM allowlist aliases and diagnostics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 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 |
|
🌿 Preview your docs: https://nvidia-preview-pr-4554.docs.buildwithfern.com/nemoclaw |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: Dispatch required scenario E2E:
Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Selective E2E Results — ✅ All requested jobs passedRun: 26668622469
|
PR Review AdvisorFindings: 0 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nemoclaw-blueprint/scripts/telegram-diagnostics.js`:
- Around line 159-164: The current logic emits the "DM allowlist is empty"
warning unconditionally; restrict that warning to only run when account.dmPolicy
=== 'allowlist'. Change the block around account.dmPolicy and allowFrom so that
you check account.dmPolicy === 'allowlist' first, then inside that branch emit
the configured message when allowFrom.length > 0 and emit the "allowlist is
empty" warning only when allowFrom.length === 0; keep the existing emit messages
and the allowFrom length pluralization logic intact (refer to account.dmPolicy,
allowFrom and the emit calls).
In `@src/lib/actions/sandbox/policy-channel.ts`:
- Around line 599-603: The warning currently triggers when account?.dmPolicy !==
"allowlist" or allowFrom.length === 0, causing false positives; update the
condition in the block that logs the Telegram allowlist warning (where
account?.dmPolicy and allowFrom are referenced in policy-channel.ts) to only run
when account?.dmPolicy === "allowlist" && allowFrom.length === 0 so the message
about an empty allowlist is emitted only when allowlist mode is active and there
are no allowed senders.
🪄 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: Enterprise
Run ID: 14be2789-68ba-4da2-8047-83a9cc901e02
📒 Files selected for processing (14)
docs/manage-sandboxes/messaging-channels.mdxdocs/reference/troubleshooting.mdxnemoclaw-blueprint/scripts/telegram-diagnostics.jsskills/nemoclaw-user-manage-sandboxes/references/messaging-channels.mdskills/nemoclaw-user-reference/references/troubleshooting.mdsrc/lib/actions/sandbox/policy-channel.tssrc/lib/messaging-channel-config.test.tssrc/lib/messaging-channel-config.tssrc/lib/onboard/messaging-channel-setup.test.tssrc/lib/onboard/messaging-channel-setup.tssrc/lib/onboard/messaging-config.test.tssrc/lib/onboard/messaging-config.tstest/generate-openclaw-config.test.tstest/telegram-diagnostics.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26668726735
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26669016534
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26669332308
|
Selective E2E Results — ✅ All requested jobs passedRun: 26669565696
|
## Summary - Adds the v0.0.56 release notes section with links to the deeper docs pages for installer, status, inference, messaging, policy, and lifecycle changes. - Updates source docs for the remaining release-prep gaps around `uv` in the PyPI preset, compact WhatsApp pairing guidance, and `nemoclaw inference set` command boundaries. - Refreshes generated `nemoclaw-user-*` skills and removes skipped experimental command terms from generated skill surfaces. ## Source summary - #4613 -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents that public installs and `nemoclaw update` follow the maintained `lkg` tag by default. - #4419 -> `docs/about/release-notes.mdx`: Notes that non-interactive Linux installs can reactivate Docker group membership and continue in one installer run when `sg docker` is available. - #4550 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures live sandbox agent-version probing for status, connect, and upgrade checks. - #4609 -> `docs/inference/use-local-inference.mdx`, `docs/about/release-notes.mdx`: Captures the GPU Docker-driver host-network local-inference reachability gate. - #4607 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents compact WhatsApp QR pairing guidance and gateway/session diagnostics. - #4582 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Reflects Slack credential validation before enabling the channel. - #4554 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Keeps Telegram allowlist alias guidance in the generated user skills and release notes. - #4563 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Includes the new `nemoclaw <name> skill remove <skill>` command in command docs and release notes. - #4566 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents the `nemoclaw inference set` redirect boundary when `--provider` or `--model` is missing. - #4323 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures per-sandbox status JSON support. - #4506 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures debug command sandbox-name validation and safer tarball writing. - #4569 -> `docs/network-policy/integration-policy-examples.mdx`, `docs/about/release-notes.mdx`: Documents that the `pypi` preset allows `/usr/local/bin/uv`. - #4579 -> `docs/network-policy/integration-policy-examples.mdx`, `docs/about/release-notes.mdx`: Captures observable Jira preset validation guidance. - #4229 -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Documents user-data preservation defaults for uninstall. - #4399 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures CPU-only sandbox intent preservation across rebuilds. - #4058 -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Captures safer snapshot restore behavior around existing destinations. - #4155 and #4460 -> skipped by `docs/.docs-skip`: Removed skipped experimental command terms from source docs and generated skill evals instead of documenting those features. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run docs` (passes; Fern reports the pre-existing light-mode accent contrast warning) - `rg "permissive mode|shields down|shields up|shields status|config rotate-token|rotate-token" .agents/skills` (no matches) - `npm run build:cli` (run to refresh local CLI artifacts for the pre-push TypeScript hook) - Commit hooks passed, including `NEMOCLAW_* env-var documentation gate`, `Verify docs-to-skills output`, `markdownlint-cli2`, `gitleaks`, and `Test (skills YAML)`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded Model Router setup with YAML examples, flow diagrams, and credential handling; strengthened agent-config immutability and integrity guidance; messaging channels updated (Telegram aliases, WhatsApp pairing/diagnostics); CLI docs revised (GPU detection, inference set behavior, uninstall/rebuild preservation); overview rebranded to NemoClaw and added v0.0.56 release notes. * **New Features** * Added `nemoclaw <name> channels status` (messaging diagnostics, JSON); added `nemoclaw <name> skill remove`; Hermes no longer marked experimental; DGX Spark quickstart sandbox-name note. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
TELEGRAM_AUTHORIZED_CHAT_IDSandTELEGRAM_CHAT_IDas compatibility aliases forTELEGRAM_ALLOWED_IDS, while persisting and baking only the canonicalTELEGRAM_ALLOWED_IDSvalue.channels add telegram, resume/rebuild, and OpenClaw config generation so Telegram DMs getdmPolicy: allowlist/allowFromwhen QA-style env vars are used.getUpdatesmetadata, and outboundsendMessageattempts without logging tokens, IDs, or message text.Related Issue
Closes #4553
Problem
The #4553 repro exported
TELEGRAM_CHAT_IDandTELEGRAM_AUTHORIZED_CHAT_IDS, but NemoClaw only consumedTELEGRAM_ALLOWED_IDSwhen constructing the OpenClawallowFrommap. That made the bridge look healthy because polling started, while the rebuilt sandbox could still have no explicit DM allowlist. The repro also used Telegram Bot APIsendMessage, which sends from the bot to the chat and therefore proves outbound API access, not inbound user-to-agent routing.Live Inbound Validation Surface
To reproduce the user-visible acceptance path with a real Telegram client and an alias-derived allowlist:
When the script prompts, send a fresh direct message from the allowed Telegram client to the bot. The check then waits for both
[telegram] [default] inbound update receivedand[telegram] [default] outbound sendMessage attemptedin/tmp/gateway.log.Verification
npm run build:clinpm run typecheck:clinpx vitest run src/lib/messaging-channel-config.test.ts src/lib/onboard/messaging-config.test.ts src/lib/onboard/messaging-channel-setup.test.ts src/lib/actions/sandbox/telegram-channel-bridge-verification.test.ts test/generate-openclaw-config.test.ts test/telegram-diagnostics.test.ts --testTimeout 60000npx vitest run test/telegram-diagnostics.test.ts --testTimeout 60000npx vitest run --project cli test/repro-2666-silent-list-status.test.ts --testTimeout 15000npx prek run shfmt biome-format --files test/e2e/test-messaging-providers.sh --stage pre-pushbash -n test/e2e/test-messaging-providers.shshellcheck test/e2e/test-messaging-providers.shnpm run docs:strict(0 errors, 2 existing Fern warnings hidden by default)git diff --checkNightly E2E
Current head:
fb394dd95129022d0835ce024c2f4573db74bc1f.Selected PR-head nightly passed: https://github.com/NVIDIA/NemoClaw/actions/runs/26669565696
Required jobs passed:
messaging-providers-e2echannels-add-remove-e2emessaging-compatible-endpoint-e2eOlder selected runs at
66be908cc2efdd37b9a6ae4ad6668409a6a2d673,fdec626e8ddfed3f1de1a9000fbfcaf18ad9fc8b, andc08a458ccf970c0f35bb9ea4e5f2fd1809be573bare superseded.