Skip to content

feat(messaging): add workflow planner#4076

Open
sandl99 wants to merge 1 commit into
u/sdang/messaging-manifest-compiler-3994from
u/sdang/messaging-workflow-planner-3995
Open

feat(messaging): add workflow planner#4076
sandl99 wants to merge 1 commit into
u/sdang/messaging-manifest-compiler-3994from
u/sdang/messaging-workflow-planner-3995

Conversation

@sandl99
Copy link
Copy Markdown
Contributor

@sandl99 sandl99 commented May 22, 2026

Summary

Adds the phase-1 messaging workflow planner for onboard, channel add/remove/start/stop, and rebuild flows. The planner computes configured, active, and disabled channel state before delegating to the manifest compiler, keeping #3995 architecture-only and out of production CLI paths.

Related Issue

Fixes #3995

Changes

  • Add MessagingWorkflowPlanner with pure planOnboard, planAddChannel, planRemoveChannel, planStartChannel, planStopChannel, and planRebuild methods.
  • Preserve stopped-but-configured channels, remove channels from configured and disabled state on remove, and carry registry/session snapshots through rebuild planning.
  • Refine MessagingCompilerWorkflow to the planner workflows and restrict enrollment hooks to selected onboard/add-channel plans.
  • Add workflow planner tests for lifecycle state transitions, deterministic unsupported-channel reporting, rebuild preservation, secret-free serialization, and avoiding re-enrollment of existing configured channels.

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
  • make 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)

Additional verification performed:

  • npm test -- --project cli src/lib/messaging passes.
  • npm run typecheck:cli passes.
  • npm run lint -- src/lib/messaging passes with the existing unrelated warning in src/lib/onboard/child-exit-tracker.test.ts.
  • npm run source-shape:check passes.
  • git diff --check passes.
  • Commit and push hooks were bypassed because the full hook path is currently blocked by unrelated CLI doctor/debug/snapshot failures on this stack.

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

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this May 22, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: dce0d082-9878-490c-ad76-b295730921c1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch u/sdang/messaging-workflow-planner-3995

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: channels-stop-start-e2e, channels-add-remove-e2e, messaging-providers-e2e
Optional E2E: token-rotation-e2e, network-policy-e2e, hermes-slack-e2e, hermes-discord-e2e

Dispatch hint: channels-stop-start-e2e,channels-add-remove-e2e,messaging-providers-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/u/sdang/messaging-manifest-compiler-3994
Head: HEAD
Confidence: high

Required E2E

  • channels-stop-start-e2e (high): Most directly covers this PR's new start-channel, stop-channel, remove-channel, and rebuild workflow semantics across OpenClaw and Hermes and all built-in messaging channels, including configured/disabled registry state, provider persistence/deletion, policy preset activation, and rendered agent config.
  • channels-add-remove-e2e (medium): Covers onboard with no messaging channel followed by channels add/remove and rebuild. This validates add-channel enrollment/activation, removal from configured state, provider detach, and messaging policy preset application/removal for the compiler/planner paths changed here.
  • messaging-providers-e2e (high): Validates the full messaging manifest output chain for provider creation, placeholder rendering, credential isolation, L7 proxy rewrite, and messaging API reachability for Telegram/Discord/Slack/WeChat plus WhatsApp no-provider behavior. Required because compiler changes affect credentialBindings, active channel selection, and secret-free plans.

Optional E2E

  • token-rotation-e2e (medium): Useful adjacent credential confidence because re-running onboard with changed messaging tokens depends on workflow/onboard semantics and provider binding updates, but the direct provider and channel lifecycle checks above are the merge-blocking coverage.
  • network-policy-e2e (medium): Optional broader safety check for policy behavior. The PR changes which messaging channel manifests feed policy planning, while messaging-specific preset application is already covered by the required channel lifecycle tests.
  • hermes-slack-e2e (medium): Optional focused Hermes render/provider validation for Slack-specific env placeholders and policy behavior, useful because manifest compilation also targets Hermes render output.
  • hermes-discord-e2e (medium): Optional focused Hermes Discord onboarding validation for top-level config shape and placeholder isolation, complementing the broader multi-channel lifecycle test.

New E2E recommendations

  • messaging workflow planner dry-run/shadow plans (medium): Existing E2E scripts validate applied CLI behavior, but there is no dedicated E2E that captures and validates the serialized SandboxMessagingPlan produced by the new MessagingWorkflowPlanner before application, including workflow values and selected/configured/disabled flags.
    • Suggested test: Add an E2E or scenario-suite assertion that runs messaging workflow dry-run/shadow output for onboard, add-channel, stop-channel, start-channel, remove-channel, and rebuild, then validates the emitted plan schema, active channel set, credentialBindings, networkPolicy entries, and absence of raw secrets.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: channels-stop-start-e2e,channels-add-remove-e2e,messaging-providers-e2e

@github-actions
Copy link
Copy Markdown
Contributor

PR Review Advisor

Recommendation: blocked
Confidence: medium
Analyzed HEAD: 4a910ed7a22772a404c32370b9d3721ba4e56d59
Findings: 1 blocker(s), 2 warning(s), 0 suggestion(s)

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

Limitations: Review is based on the provided trusted context plus read-only inspection of changed files; no tests or scripts were executed.; Required E2E job results for channels-stop-start-e2e, channels-add-remove-e2e, and messaging-providers-e2e were not available in the provided status rollup.; Review thread state was reported as unavailable by the deterministic gate context.; This PR is stacked on open manifest compiler work (#4069), so final drift depends on the landing state of the base branch.

Workflow run

Full advisor summary

PR Review Advisor

Base: origin/u/sdang/messaging-manifest-compiler-3994
Head: HEAD
Analyzed SHA: 4a910ed7a22772a404c32370b9d3721ba4e56d59
Recommendation: blocked
Confidence: medium

The workflow planner implementation and unit coverage look aligned with #3995, but the E2E Advisor marked three messaging lifecycle/provider E2Es as required and no evidence shows they passed for head SHA 4a910ed.

Gate status

  • CI: pass — 5 required status context(s) completed with no failures. Non-required contexts still pending: 3; failed: 0.
  • Mergeability: warning — mergeStateStatus=UNSTABLE
  • Review threads: unknown — No review thread state was available.
  • Risky code tested: pass — No risky code areas detected by path heuristics.

🔴 Blockers

  • Required messaging E2E jobs are not shown as passed for this head SHA: The trusted E2E Advisor comment requires channels-stop-start-e2e, channels-add-remove-e2e, and messaging-providers-e2e for this PR. The status rollup only shows the E2E recommendation job itself; it does not show those required E2E jobs passing for 4a910ed. Because this PR changes workflow planning for channel lifecycle, credential bindings, active/disabled state, policy/render inputs, and provider behavior, unit tests alone do not prove the end-to-end sandbox/runtime behavior.
    • Recommendation: Run or confirm the required E2E jobs for head SHA 4a910ed and attach passing evidence before merge.
    • Evidence: E2E Advisor required: channels-stop-start-e2e, channels-add-remove-e2e, messaging-providers-e2e. statusCheckRollup includes E2E recommendation=SUCCESS but no matching required E2E job results.

🟡 Warnings

  • Merge state is unstable: GitHub reports mergeStateStatus=UNSTABLE even though required checks are currently passing. Non-required build/e2e-related contexts are also still in progress in the provided status rollup.
    • Recommendation: Wait for pending non-required contexts to complete and re-check mergeability before merging.
    • Evidence: mergeStateStatus=UNSTABLE; pending contexts include PR review advisor, test-e2e-ollama-proxy, build-sandbox-images, and build-sandbox-images-arm64.
  • Stacked branch overlaps open manifest compiler work: This PR patches files that also appear in open PR feat(messaging): add manifest compiler #4069, and its base branch is the manifest compiler branch rather than main. That is expected for a stacked messaging series, but it increases drift risk if feat(messaging): add manifest compiler #4069 changes before this PR lands.
    • Recommendation: Confirm feat(messaging): add manifest compiler #4069 is landed or that this PR has been rebased onto the final manifest compiler state before merging the stack.
    • Evidence: Open overlap feat(messaging): add manifest compiler #4069 touches src/lib/messaging/compiler/index.ts, manifest-compiler.test.ts, manifest-compiler.ts, manifest/types.test.ts, and manifest/types.ts; recent history shows these files were introduced/changed by f388104 feat(messaging): add manifest compiler.

🔵 Suggestions

  • None.

Acceptance coverage

  • unknown — Part of Refactor messaging integrations into a manifest-first planning architecture #3896 phase 1.: The PR implements the phase-1 workflow planner requested by [Messaging] Add workflow planner for onboard and channel lifecycle #3995, but no direct evidence for parent issue Refactor messaging integrations into a manifest-first planning architecture #3896 completion was provided.
  • met — Add MessagingWorkflowPlanner, the pure workflow-level planner that aggregates compiled manifests into full sandbox messaging plans for onboard, add/remove/start/stop, and rebuild flows.: src/lib/messaging/compiler/workflow-planner.ts adds MessagingWorkflowPlanner with planOnboard, planAddChannel, planRemoveChannel, planStartChannel, planStopChannel, and planRebuild; compileWorkflow delegates to ManifestCompiler to return SandboxMessagingPlan.
  • met — Add src/lib/messaging/compiler/workflow-planner.ts.: New file src/lib/messaging/compiler/workflow-planner.ts is present and exported from src/lib/messaging/compiler/index.ts.
  • met — Implement pure planning methods: planOnboard, planAddChannel, planRemoveChannel, planStartChannel, planStopChannel, and planRebuild.: Methods are implemented in workflow-planner.ts and covered in workflow-planner.test.ts.
  • met — Model configured vs active vs disabled channels explicitly.: ManifestCompiler.compileChannel computes selected/configured/disabled/active, and planner tests assert these flags for onboard, add-channel, stop-channel, start-channel, remove-channel, and rebuild.
  • met — Preserve stopped-but-configured channels for later start and rebuild recovery.: planStopChannel keeps configuredChannels and adds the target to disabledChannels; planStartChannel removes the target from disabledChannels; planRebuild preserves disabledChannels that remain configured.
  • met — Represent remove as removing configured channel state, not just disabling it.: planRemoveChannel removes context.channelId from configuredChannels and disabledChannels; test verifies telegram is absent after removal.
  • met — Aggregate channel plans into one SandboxMessagingPlan.: compileWorkflow returns this.compiler.compile(compilerContext), and ManifestCompiler.compile returns a single SandboxMessagingPlan containing channels, credentialBindings, networkPolicy, agentRender, buildSteps, stateUpdates, and healthChecks.
  • met — Add dry-run and shadow-mode friendly plan output tests.: workflow-planner.test.ts includes returns serializable, secret-free plans suitable for dry-run and shadow output, checking JSON round-trip, no function values, placeholder presence, and no raw secret leakage.
  • metstop-channel plans keep the channel in configured state and add it to disabled state.: workflow-planner.test.ts stop-channel test expects telegram configured=true, disabled=true, active=false.
  • metstart-channel plans keep the channel configured, remove it from disabled state, and make it active.: workflow-planner.test.ts start-channel test expects telegram configured=true, disabled=false, active=true.
  • metremove-channel plans remove the channel from configured and disabled state.: workflow-planner.test.ts remove-channel test expects telegram to be undefined and remaining disabled state only for wechat.
  • metrebuild plans preserve configured channels and disabled channels from registry/session snapshots.: workflow-planner.test.ts rebuild test expects telegram, discord, and wechat channels with discord configured=true and disabled=true.
  • metonboard plans represent selected channels, credential bindings, build inputs, policy, render, state updates, and health checks.: planOnboard sets selectedChannels and configuredChannels to the selected list; existing manifest-compiler tests validate credentialBindings, buildSteps, networkPolicy, agentRender, stateUpdates, and healthChecks for onboard workflow.
  • met — Unsupported channels are reported deterministically.: assertSupportedChannels sorts unsupportedIds before throwing; workflow-planner.test.ts verifies the deterministic message Unsupported messaging channel(s) for openclaw: discord, slack.
  • met — Planner contains no channel-specific behavior beyond manifest discovery and selection.: workflow-planner.ts uses registry.list(), supportedAgents, supportedChannelIds, and generic channel set helpers; no hardcoded channel IDs were found in the planner implementation.
  • met — No CLI command imports or invokes the planner in production yet.: Search evidence only found MessagingWorkflowPlanner references in compiler index export, workflow-planner.ts, and workflow-planner.test.ts; no CLI command imports were found.
  • met — Do not replace onboard.ts.: No changed file replaces onboard.ts; changed files are limited to messaging compiler/manifest types and tests.
  • met — Do not replace policy-channel.ts.: No changed file replaces policy-channel.ts; policy output remains generated through the existing manifest compiler policy resolver.
  • met — Do not invoke real hooks or appliers.: Planner delegates to ManifestCompiler with supplied hook registries; tests use fake hook registrations. No applier invocation is introduced.

Security review

  • pass — Secrets and Credentials: No hardcoded secrets were added. Secret inputs are represented through credentialAvailable flags and placeholders such as openshell:resolve:env:TELEGRAM_BOT_TOKEN; tests assert raw token values do not appear in serialized plans.
  • pass — Input Validation and Data Sanitization: Planner inputs are typed channel IDs and arrays; unsupported channels are allowlisted through manifest registry/support filters and rejected deterministically. No new eval, shell execution, unsafe deserialization, URL parsing, or filesystem path handling was introduced.
  • pass — Authentication and Authorization: No endpoints, auth flows, token validation, or authorization checks are added or modified. The change is a pure planner/compiler layer.
  • pass — Dependencies and Third-Party Libraries: No new dependencies or package manager configuration changes were introduced.
  • pass — Error Handling and Logging: The new error path reports unsupported channel IDs and agent ID only; it does not log or expose raw secrets, stack traces, internal paths, or credentials.
  • pass — Cryptography and Data Protection: Not applicable — no cryptographic operations, encryption, hashing, signing, or key handling were introduced.
  • pass — Configuration and Security Headers: No HTTP endpoints, CORS/CSP settings, Dockerfiles, container permissions, or runtime security headers are changed.
  • warning — Security Testing: Unit tests cover secret-free serialization and lifecycle state transitions, but E2E Advisor requires messaging lifecycle/provider E2E coverage for credential bindings, provider behavior, policy application, and runtime reachability; those required E2Es are not shown as passed for the head SHA.
  • warning — Holistic Security Posture: The implementation avoids obvious sandbox escape, SSRF, policy bypass, credential leakage, and installer/workflow boundary issues, but runtime messaging plan effects touch provider/policy/render behavior and need the required E2E confirmation before the security posture can be considered complete.

Test / E2E status

  • Test depth: e2e_required — Runtime/sandbox/infrastructure paths need real execution coverage: src/lib/messaging/compiler/index.ts, src/lib/messaging/compiler/manifest-compiler.ts, src/lib/messaging/compiler/workflow-planner.ts, src/lib/messaging/manifest/types.ts. The new unit tests are broad for pure planner behavior, but required E2Es are needed for applied CLI/provider/policy/render behavior.
  • E2E Advisor: missing
  • Required E2E jobs: channels-stop-start-e2e, channels-add-remove-e2e, messaging-providers-e2e
  • Missing for analyzed SHA: channels-stop-start-e2e, channels-add-remove-e2e, messaging-providers-e2e

✅ What looks good

  • Planner implementation is generic and registry-driven rather than hardcoding channel-specific lifecycle behavior.
  • Unit tests cover onboard, add, remove, start, stop, rebuild, unsupported-channel ordering, no re-enrollment of existing configured channels, and secret-free serialization.
  • Enrollment hook execution was narrowed to selected active onboard/add-channel workflows, reducing accidental re-enrollment side effects.
  • No new dependencies, Dockerfiles, workflow files, shell execution, or credential literals were added.

Review completeness

  • Review is based on the provided trusted context plus read-only inspection of changed files; no tests or scripts were executed.
  • Required E2E job results for channels-stop-start-e2e, channels-add-remove-e2e, and messaging-providers-e2e were not available in the provided status rollup.
  • Review thread state was reported as unavailable by the deterministic gate context.
  • This PR is stacked on open manifest compiler work (feat(messaging): add manifest compiler #4069), so final drift depends on the landing state of the base branch.
  • Human maintainer review required: yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant