fix(onboard): prune inactive messaging presets for OpenClaw - #10490
fix(onboard): prune inactive messaging presets for OpenClaw#10490hunglp6d wants to merge 30 commits into
Conversation
#10281 stopped Hermes from carrying a repository messaging preset for a channel the sandbox does not configure. OpenClaw reaches the same selection paths, so it kept applying a preset for an unconfigured channel, including the Discord preset that names a sandbox-scoped credential provider create-plan validation rejects. Resolve the agent set from supportedAgents in the channel manifests instead of naming Hermes in the guard, and rename the helper to match what it now covers. An agent that appears in no manifest keeps its selection untouched, a missing messaging plan stays no authority to call a channel inactive, and a custom preset that shadows the name is still preserved. Read the rebuild agent from the messaging plan rather than the registry entry. getSandboxAgentRegistryFields stores agent as null for OpenClaw, so the registry field made every default OpenClaw rebuild skip the guard. Build rebuild's enabled-channel set with getActiveChannelIdsFromPlan. Filtering on `disabled` alone counted a configured channel the plan cannot start, which kept its egress preset and left the same dangling credential binding behind. Give the rebuild fixtures production-shaped plans. The existing custom-policy fixture passes even when the shared helper is a no-op, and the lifecycle fixture omitted `active` and its credential hashes, so it read as fully inactive once rebuild resolved the canonical active set. This supersedes #10359, which reported the OpenClaw gap from a fork branch where live E2E cannot reach the credentialed targets. Refs #10153 Co-authored-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Hung Le <hple@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughMessaging preset pruning now supports Hermes and OpenClaw. Onboarding reconciliation and sandbox rebuild flows use typed messaging plans, active channel IDs, and credential availability checks. ChangesMessaging preset pruning
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change consistently prunes inactive messaging presets for OpenClaw across onboarding, selection, and rebuild flows. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 64591d4 in the TypeScript / code-coverage/cliThe overall line coverage in commit 64591d4 in the Show a line coverage summary of the most impacted files.
Updated |
hunglp6d
added
area: onboarding
Three rebuild tests each rebuilt the full SandboxMessagingPlan shape and differed only by agent and channel activity, so a schema change needed three synchronized edits. Give the file one plan factory and one channel factory, both typed from RebuildBackupPhaseInput so they track the production shape. Assertions are unchanged. The suite still fails when the shared pruning helper is a no-op, and the OpenClaw rows still fail when rebuild reads the registry agent instead of the messaging plan. Refs #10153 Signed-off-by: Hung Le <hple@nvidia.com>
|
Advisor findings for Code / Reduction — fixed. The three rebuild tests each rebuilt the full messaging-plan shape. Behavior — not reproduced, no change. One related edge case is disclosed in the description rather than fixed here: custom-preset names are compared lowercased by the pruner and verbatim by the rebuild normalization and restore boundaries. It predates this PR and needs malformed persisted state to reach. The other eight specialists reported no finding. |
The previous commit removed repeated plan literals by adding a factory local to rebuild-backup-phase.test.ts, which left three owners of the SandboxMessagingPlan shape: the shared fixture in test/helpers, the backup-phase local factory, and the rebuild-flow local factory. Extend makeMessagingPlan additively instead, then delete both local owners: - channelOverrides expresses a configured channel the rebuild workflow planner recomputes as inactive while `disabled` stays false, which the channel and disabled lists cannot express on their own. Override inputs are cloned so repeated calls and caller-owned inputs stay isolated, and the #8357 isolation test now covers that option. - messagingCredentialBindingsForChannels resolves bindings from the channel manifests, so a fixture with active credential-bearing channels satisfies enforceMessagingChannelConflicts without restating manifest data. Its doc names them synthetic conflict-preflight bindings, because providerName stays a template and each hash is a deterministic sentinel. Existing callers pass neither option and are unchanged. All fourteen consumers of the shared fixture pass. The rebuild suites still fail when the shared pruning helper is a no-op, and the OpenClaw rows still fail when rebuild reads the registry agent instead of the messaging plan. Refs #10153 Signed-off-by: Hung Le <hple@nvidia.com>
|
Advisor findings for Code / Reduction — accepted, fixed. The finding is right that Cross-review of that consolidation found two things worth fixing, both addressed here: the new override assigned the caller's Unrelated red check: |
The shared fixture gained a `channelOverrides` option with one consumer, so the rebuild test's inactive-channel state stopped being local to the behavior it verifies while the shared contract grew an option nobody else needs. Build that plan in the test instead: take the ordinary plan from makeMessagingPlan and map the Discord channel onto an inactive one carrying its unavailable required secret. The shared fixture returns to its previous shape, and its isolation test returns to covering the options it still has. Also correct two comments. The application-boundary comment called policy-resume-selection.test.ts end-to-end coverage when it is a source test, and the shared-fixture comments carried background that review context already records. Refs #10153 Signed-off-by: Hung Le <hple@nvidia.com>
|
Advisor findings for Code / Reduction — accepted. The For the record, this reverses the direction of the previous round's Code / Reduction finding, which asked for consolidation into the shared fixture. The current shape satisfies both: plan construction still goes through the shared owner, and the one-off state stays in the test that needs it. Documentation — accepted. The comment said end-to-end coverage lives in Dependency use — declined. The finding states that Verification: all fourteen consumers of the shared fixture pass (323 tests), growth guardrails 32 passed, |
|
Advisor findings for Behavior — not reproduced, no change. This repeats the finding declined against The finding notes it did not run tests. Two independent cross-reviews reached the same conclusion, the second tracing |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Use the messaging-owned selector during rebuild. Policy pruning now matches all channel effects. Clarify the known-state and custom-policy conditions in the pruning comments. Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…ssaging-presets Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…ssaging-presets Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…ssaging-presets Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Candidate E2E is blocked by the current trusted workflow boundary. Issue #10153 requires I did not dispatch a broader run because it cannot supply the required evidence. A maintainer decision is required:
All repository gates and the required E2E decision remain pending. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Merge-train update The latest PR commit now includes current main. Focused validation passed 92 tests. CLI type-checking, repository checks, validate:pr, normal hooks, DCO, and GitHub commit verification also pass. Correction to the earlier E2E note: the current trusted workflow can provide applicable credentialed profiles for this NVIDIA-owned PR branch. However, it can reach openclaw-slack-pairing only through the complete default PR selection. That broad run exposes applicable long-lived credentials to candidate-controlled jobs and can create external resources. A focused Slack selector is not authorized. I did not dispatch the broad run. A maintainer must choose one action:
Fresh CI and automated review are running. The E2E decision and independent approval remain blocking. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Merge-train update after the current-main refresh The provider-authority correction is now on the branch at revision
One architecture decision remains. Accepted issue #10514 makes OpenShell the only durable policy source and forbids NemoClaw from storing a desired policy tier. Ordinary resume therefore has no authoritative tier. Two behaviors are possible:
I did not add tier state or select either behavior without maintainer agreement. Correction to earlier E2E notes: the current trusted workflow supports the focused selector Fresh CI and Advisor are running. The tier decision, focused E2E decision, and independent approval block merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-10490.docs.buildwithfern.com/nemoclaw |
|
PR Review Advisor finished for commit |
|
Final merge-train state for branch revision All current automated checks completed without failure against base and workflow revision
One architecture decision remains under #10514. OpenShell is the only durable policy source, and the desired policy tier is not persisted. Resume therefore has no tier provenance. Preserving live The trusted manual PR workflow can select only Independent approval and the architecture decision still block merge. No merge or bypass was attempted. |
2 participants
Outcome
OpenClaw and Hermes rebuilds now derive messaging policy and credential-provider authority from the canonical active-channel selection. An inactive configured channel no longer keeps its built-in egress or provider binding, while active channels and same-named custom policy entries remain intact.
Reason
The existing inactive-preset guard did not cover every OpenClaw selection path. Rebuild also filtered only channels marked disabled, so a configured channel with
active: falsecould retain policy and provider authority. This left policy entries that referenced providers the rebuilt sandbox could not use.Related issues
Refs #10153
Changes
Verification
Review notes
The final change affects 11 files with 396 additions and 156 deletions. The 552 lines of churn are flagged for focused review.
Sensitive paths include onboarding, policy selection, messaging, and sandbox rebuild. The change narrows authority and does not create a product surface or restore durable NemoClaw policy state.
PR #10359 carried the first attempt at the same OpenClaw gap. This PR preserves its contributor attribution and contains the in-repository branch needed for candidate messaging qualification.
The required
openclaw-slack-pairingcandidate E2E remains outstanding. Do not approve or merge until that run passes for the published revision and every repository gate passes.Signed-off-by: Ho Lim subhoya@gmail.com
Signed-off-by: Hung Le hple@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com