Skip to content

feat(messaging): persist channel manifest plans#4536

Open
sandl99 wants to merge 1 commit into
u/sdang/3896-phase-2-messaging-enrollmentfrom
u/sdang/build-messaging-manifest-plan-in-rebuild
Open

feat(messaging): persist channel manifest plans#4536
sandl99 wants to merge 1 commit into
u/sdang/3896-phase-2-messaging-enrollmentfrom
u/sdang/build-messaging-manifest-plan-in-rebuild

Conversation

@sandl99
Copy link
Copy Markdown
Contributor

@sandl99 sandl99 commented May 29, 2026

Summary

Persist manifest messaging plans through channel lifecycle operations so add, stop, start, remove, and rebuild can carry the new architecture state in SandboxEntry while legacy registry fields continue to work.

Related Issue

Fixes #4535
Refs #3896

Changes

  • Added MessagingWorkflowPlanner helpers that merge a compiled add-channel plan into a stored sandbox plan and mutate stored plans for stop/start/remove/rebuild.
  • Updated channels add, channels stop, channels start, and channels remove to write SandboxEntry.messaging.plan without removing legacy registry updates.
  • Staged stored manifest plans during rebuild through the existing messaging plan env path.
  • Added planner tests for add merge, stop/start mutation, remove pruning, rebuild staging from stored plans, and no-compile behavior when no stored plan exists.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

Release Notes

  • New Features

    • Improved messaging channel management with manifest-driven configuration for Discord, Telegram, Slack, WeChat, and WhatsApp
    • Support for multiple authentication modes including token-based and QR code enrollment
    • Enhanced channel validation and reachability checks during setup
  • Bug Fixes

    • More reliable credential handling and credential binding resolution
    • Better error messaging and validation for channel configuration
  • Tests

    • Expanded test coverage for channel enrollment workflows and manifest validation

@sandl99 sandl99 self-assigned this May 29, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 29, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

Introduces manifest-driven messaging: adds manifests and hooks, compiler/planner, and appliers; updates onboarding to select/apply plans and persist them; integrates manifest lifecycle into sandbox actions and rebuild; extends registry to store messaging plans; updates tests and E2E.

Changes

Core messaging manifests, hooks, compiler, and appliers

Layer / File(s) Summary
Manifests and messaging plan/types
src/lib/messaging/channels/*/manifest.ts, src/lib/messaging/channels/index.ts, src/lib/messaging/manifest/types.ts, src/lib/messaging/index.ts
Adds built-in manifests and overhauls manifest/plan contracts; provides built-in registry and messaging barrel.
Hooks system: types, registry, runner, and common/built-in hooks
src/lib/messaging/hooks/**/*, src/lib/messaging/channels/telegram/hooks/*, src/lib/messaging/channels/wechat/hooks/*
Implements hook infrastructure and shared/built-in hooks (token-paste, config-prompt, reachability, QR login, seed, health-check).
Compiler, engines, planner, utils, and barrels
src/lib/messaging/compiler/**/*, src/lib/messaging/utils.ts
Compiles manifests into plans via engines and planner; adds helper utils and barrels.
Applier layer: types, setup/env encoding, host-state, agent-config, credentials, policy, filters
src/lib/messaging/applier/**/*
Encodes plans to env, applies to sandbox via OpenShell (agent config, credentials, policy), filters enabled entries, and persists/merges host state.

Onboarding integration and messaging selection flow

Layer / File(s) Summary
Onboarding messaging setup entrypoint and utilities
src/lib/onboard/messaging-channel-setup.ts
Replaces per-channel prompts with manifest-driven planning and env plan write; supports interactive selection and QR help.
Onboard entrypoints, machine handler contract changes, and state helpers
src/lib/onboard.ts, src/lib/onboard/machine/handlers/sandbox.ts, src/lib/onboard/messaging-state.ts
Threads sandboxName; stages env plan on register; updates handler signature; removes legacy helpers.
Onboarding tests for selection and plan emission
src/lib/onboard/*test.ts, test/onboard-messaging.test.ts
Adjusts tests to validate selection, env plan presence, and new handler calls.

Sandbox actions: policy-channel and rebuild plan staging

Layer / File(s) Summary
policy-channel.ts manifest-driven add/enable/disable/remove
src/lib/actions/sandbox/policy-channel.ts
Uses manifests and planner to add channels, persist plans/disabled state, and apply to registry.
rebuild.ts stages messaging manifest plan
src/lib/actions/sandbox/rebuild.ts
Stages and writes rebuild plan to env; aborts safely on failures.
CLI tests for channels add and E2E provider/reachability adjustments
test/channels-add-preset.test.ts, test/e2e/*
Captures planner invocations and adjusts Telegram reachability skip in E2E.

State registry messaging persistence

Layer / File(s) Summary
registry types and clone helper
src/lib/state/registry.ts
Adds SandboxMessagingState and persists normalized plans into registry entries.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant Planner
  participant SetupApplier
  participant Registry
  CLI->>Planner: buildPlan(workflow: onboard/add/rebuild)
  Planner-->>CLI: SandboxMessagingPlan
  CLI->>SetupApplier: writePlanToEnv(plan)
  CLI->>Registry: applyPlanToRegistry(plan, mode)
  Registry-->>CLI: messaging state stored/merged
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Possibly related PRs

  • NVIDIA/NemoClaw#4003 — Earlier introduction of manifest registry/types that this PR builds upon.
  • NVIDIA/NemoClaw#4001 — Touches the same policy-channel teardown flow now extended to persist manifest remove plans.
  • NVIDIA/NemoClaw#3925 — Modifies rebuild logic in the same area where this PR stages manifest plans.

Suggested labels

enhancement: messaging, refactor, NemoClaw CLI

Suggested reviewers

  • ericksoa
  • cv
  • jyaunches

Poem

A rabbit taps keys in a moonlit den,
Manifests bloom where hardcodes had been;
Hooks hop in sequence, plans ride the breeze,
Env carries secrets with careful ease.
Rebuilds remember, sandboxes sing—
Policies nest where credentials spring. 🐇✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch u/sdang/build-messaging-manifest-plan-in-rebuild

@sandl99 sandl99 changed the base branch from main to u/sdang/3896-phase-2-messaging-enrollment May 29, 2026 17:21
@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: messaging-providers-e2e, channels-add-remove-e2e, channels-stop-start-e2e, rebuild-openclaw-e2e, cloud-onboard-e2e
Optional E2E: network-policy-e2e, openclaw-slack-pairing-e2e, openclaw-discord-pairing-e2e, hermes-slack-e2e

Dispatch hint: messaging-providers-e2e,channels-add-remove-e2e,channels-stop-start-e2e,rebuild-openclaw-e2e,cloud-onboard-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • messaging-providers-e2e (high (~75 min timeout; Docker/OpenShell sandbox; NVIDIA_API_KEY)): Directly validates the changed manifest-driven provider/placeholder/config/network path for Telegram, Discord, Slack, WeChat, and WhatsApp, including credential isolation and channel bridge reachability.
  • channels-add-remove-e2e (high (~75 min timeout; Docker/OpenShell sandbox; NVIDIA_API_KEY)): Covers the user-facing channels add and channels remove lifecycle changed in policy-channel.ts, including auto policy preset application, provider detach, rebuild, and preventing removed channels from reappearing.
  • channels-stop-start-e2e (very high (~120 min timeout; multiple Docker/OpenShell sandboxes; NVIDIA_API_KEY)): Required because stop/start now persists manifest plans and must disable/re-enable channels across rebuilds. This job exercises OpenClaw and Hermes across Telegram, Discord, WeChat, Slack, and WhatsApp.
  • rebuild-openclaw-e2e (high (~60 min timeout; Docker/OpenShell sandbox; NVIDIA_API_KEY)): rebuild.ts now stages messaging manifest plans before backup/delete/recreate. A rebuild E2E is needed to prove the new preflight does not break baseline OpenClaw rebuild, state preservation, policy preservation, or post-rebuild inference.
  • cloud-onboard-e2e (medium/high (~45 min default timeout; Docker/OpenShell sandbox; NVIDIA_API_KEY)): src/lib/onboard.ts and onboarding messaging state changed. This validates the clean install/onboard path and registry/session/policy state outside the specialized messaging lifecycle tests.

Optional E2E

  • network-policy-e2e (medium/high): Useful adjacent confidence for the policy resolver/applier and policy-channel changes, but messaging-providers and channels-add-remove already cover channel policy preset application.
  • openclaw-slack-pairing-e2e (medium/high): Optional deeper coverage for Slack-specific manifest changes and OpenClaw bridge/pairing runtime behavior beyond provider placeholder wiring.
  • openclaw-discord-pairing-e2e (medium/high): Optional deeper coverage for Discord-specific manifest changes and gateway/pairing runtime behavior beyond the generic provider E2E.
  • hermes-slack-e2e (medium/high): Optional agent-specific validation because the new agent config applier has Hermes-specific render targets and the PR changes Slack manifest/provider behavior.

New E2E recommendations

  • manifest-driven messaging rebuild preservation (high): Existing rebuild-openclaw validates baseline rebuild and channel lifecycle tests validate add/remove/stop/start, but there is no focused E2E that onboards multiple manifest-backed channels, runs rebuild, and asserts registry.messaging, OpenShell providers, policy presets, and agent config are all reconstructed from the persisted manifest plan.
    • Suggested test: Add a messaging-manifest-rebuild-e2e script that configures at least Telegram+Slack+WhatsApp, rebuilds the sandbox, and verifies provider attachment, placeholder config, no secret leaks, active policy entries, and disabled-channel filtering after rebuild.
  • WeChat host-QR enrollment (medium): The WeChat host-QR hooks and session metadata persistence changed substantially, but current hermetic E2Es appear to use fake token/account environment values rather than driving the actual host-side QR/iLink login flow.
    • Suggested test: Add a hermetic host-QR mock E2E for WeChat that exercises channels add wechat without preseeded WECHAT_* metadata, captures hook output, persists session metadata, rebuilds, and verifies seeded OpenClaw account state.

Dispatch hint

  • Workflow: .github/workflows/nightly-e2e.yaml
  • jobs input: messaging-providers-e2e,channels-add-remove-e2e,channels-stop-start-e2e,rebuild-openclaw-e2e,cloud-onboard-e2e

@github-actions
Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 2 needs attention, 5 worth checking, 0 nice ideas
Top item: Revalidate stored messaging plans against manifests before rebuild/apply

Review findings

🛠️ Needs attention

  • Channel lifecycle persistence is only tested at planner boundaries (src/lib/actions/sandbox/policy-channel.ts:649): The linked issue is about production lifecycle operations (`channels add`, `stop`, `start`, `remove`, and `rebuild`) preserving stored manifest plans while legacy fields continue to work. The tests cover pure planner and host-state applier behavior, but I did not find direct tests for the production `policy-channel.ts` and `rebuild.ts` wiring that combines registry writes, session/env updates, policy application, gateway-provider updates, and rebuild prompts.
    • Recommendation: Add focused command-handler tests with mocked registry/session/OpenShell/policy dependencies proving: add writes `SandboxEntry.messaging.plan` and legacy fields; stop/start/remove update both legacy state and the stored plan; rebuild writes `NEMOCLAW_MESSAGING_PLAN_B64` from a stored plan and clears it when no stored plan exists; legacy no-plan sandboxes still rebuild from `messagingChannels`/`disabledChannels`.
    • Evidence: Planner tests cover `buildChannelAddPlanFromSandboxEntry`, stop/start/remove, and rebuild from stored plan, but grep found no corresponding tests for `persistManifestChannelDisabledPlan`, `persistManifestChannelRemovePlan`, or `stageMessagingManifestPlanForRebuild` production callers.
  • Large-file growth adds several new monolith hotspots: This PR adds or grows multiple large TypeScript files while also touching already-large sandbox lifecycle files. The new manifest architecture is broad enough that future security reviews and maintenance will be harder if compiler/applier/test responsibilities remain concentrated in these files.
    • Recommendation: Split the largest new files by responsibility before merge, or offset the growth with extraction in the same PR. Prioritize `agent-config.ts`, `workflow-planner.ts`, `manifest-compiler.ts`, and the large test files so high-risk sandbox/policy/credential paths remain reviewable.
    • Evidence: Deterministic drift data flags `setup-applier.test.ts` 770 lines, `manifest-compiler.test.ts` 710, `workflow-planner.test.ts` 683, `agent-config.ts` 605, `workflow-planner.ts` 467, `manifest-compiler.ts` 446, plus growth in already-large `policy-channel.ts` and `rebuild.ts`.

🔎 Worth checking

  • Source-of-truth review needed: SandboxEntry.messaging.plan as durable messaging source: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `buildRebuildPlanFromSandboxEntry()` reuses `entry.messaging.plan`; `readSandboxEntryPlan()` only checks schema version, sandbox name, and agent.
  • Source-of-truth review needed: Best-effort onboard-session persistence in channel add: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `persistManifestMessagingConfig()` and `persistWechatConfigFromEnv()` swallow session update failures; `safeLoadOnboardSession()` catches and returns null.
  • Stored messaging plans are trusted without deep manifest revalidation (src/lib/messaging/compiler/workflow-planner.ts:238): The new durable `SandboxEntry.messaging.plan` becomes the source used for lifecycle mutations and rebuild staging, but `readSandboxEntryPlan()` accepts a stored plan when only `schemaVersion`, `sandboxName`, and `agent` match. Nested render targets, provider names, policy keys, hook references, plugin install data, and other plan fields are not recompiled or strictly validated against the current built-in manifests before later staging/use. This creates a blueprint-tampering and stale-plan risk if registry state is corrupted, edited, or produced by older manifest code.
    • Recommendation: Treat the stored plan as cache/state, not authority: recompile rebuild/lifecycle plans from current manifests plus persisted channel input state, or add deep validation that every stored channel, credential binding, policy entry, render target, hook, and build-file entry matches the registered manifest contract before it can be staged or applied.
    • Evidence: `buildRebuildPlanFromSandboxEntry()` returns `setPlanDisabledChannels(existingPlan, ...)`; `readSandboxEntryPlan()` only checks top-level version/name/agent before cloning the stored plan.
  • Manifest-driven plugin install uses an npm spec without integrity pinning (src/lib/messaging/channels/wechat/hooks/seed-openclaw-account.ts:11): The WeChat seed hook writes an OpenClaw plugin install entry for `@tencent-weixin/openclaw-weixin@2.4.3`. This is version-pinned, but it is still a package-manager spec rather than an integrity-pinned or allowlisted artifact at the new manifest/build-file boundary.
    • Recommendation: Document and enforce the installer trust model for manifest-generated plugin installs. Prefer an allowlisted plugin catalog with expected registry and integrity/digest metadata, or validate this spec at the installer boundary before applying it to sandbox config.
    • Evidence: `WECHAT_PLUGIN_SPEC = "@tencent-weixin/openclaw-weixin@2.4.3"`; `buildWechatSeedOpenClawAccountOutputs()` writes it into `plugins.installs[openclaw-weixin].spec`.
  • Best-effort session persistence lacks regression coverage and removal criteria (src/lib/actions/sandbox/policy-channel.ts:798): `persistManifestMessagingConfig()`, `persistWechatConfigFromEnv()`, and `safeLoadOnboardSession()` deliberately tolerate session load/update failures while relying on registry or immediate rebuild behavior. The comments explain the fallback at a high level, but the invalid session states and deferred-rebuild guarantees are not fully covered by targeted tests.
    • Recommendation: Add tests for corrupted/missing/wrong-sandbox onboard session while running `channels add`, including deferred rebuild behavior for WeChat metadata and config. Document when this workaround can be removed, or consolidate on a single durable source so session best-effort handling is no longer needed.
    • Evidence: `onboardSession.updateSession()` failures are swallowed with comments such as `Best-effort: registry state still carries the config when available`; `safeLoadOnboardSession()` catches and returns null.

🌱 Nice ideas

  • None.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@wscurran wscurran added enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. fix labels May 29, 2026
@wscurran
Copy link
Copy Markdown
Contributor

@cv cv added the Integration: WhatsApp Use this label to identify WhatsApp communication integration issues with NemoClaw. label May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. fix Integration: WhatsApp Use this label to identify WhatsApp communication integration issues with NemoClaw.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 2b: persist manifest plans for channel lifecycle and rebuild

3 participants