refactor(messaging): persist session registry state as plans

[sandl99]

Summary

Migrate onboard session and sandbox registry messaging persistence from legacy flat fields to the manifest-backed messaging plan. This removes legacy conflict backfill paths and keeps channel config, active/disabled state, and rebuild/session resume behavior plan-driven.

Related Issue

Fixes #4947
Parent: #3896

Changes

• Store session messaging state only as messagingPlan and registry messaging state only as messaging.plan.
• Derive configured, active, disabled, and config values from plans across onboard, rebuild, inventory, doctor, status, and channel lifecycle flows.
• Remove legacy conflict-detection backfill code and tests for messagingChannels, disabledChannels, and messagingChannelConfig.
• Update unit, integration, and e2e fixtures to assert plan-backed registry/session state.

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

Passed: npm run typecheck:cli, npm run test-size:check, npm run source-shape:check, npm run build:cli, shell syntax checks for edited e2e scripts, and focused Vitest coverage for the messaging/session/registry paths (23 files, 426 tests).

Attempted npm test, npx prek run --all-files, commit hooks, and push hooks. They fail in unrelated CLI tests: auto-pair timing in test/nemoclaw-start.test.ts, debug wrapper timeouts in test/secret-redaction.test.ts, policy disclosure timeout in test/policies.test.ts, and intermittent doctor gateway timeout tests in test/cli/doctor-gateway-token.test.ts.

• 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

• Refactor

  • Messaging moved from legacy channel arrays to a single schema-versioned messaging plan; registry and session persist structured messaging state and selection flows now use the plan.
  • Channel add/remove and rebuild flows updated to use plan-based state and simplified conflict handling.

• Bug Fixes

  • Status/overlap reporting and conflict checks now rely on stored plans and run best-effort without failing the command.

• Tests

  • Test suites, E2E scripts, and fixtures updated to the new messaging plan format.

[copy-pr-bot]

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]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/lib/onboard/machine/handlers/sandbox.ts (1)

265-302: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve existing session plan when no fresher plan source exists.

At Line 265, messagingPlan is initialized to null, then persisted at Lines 299-301 and Line 345. If both env and registry return no plan, this overwrites an existing session.messagingPlan with null during resume, dropping persisted channel/disabled state for later resume/rebuild paths.

Suggested fix

-    let messagingPlan: SandboxMessagingPlan | null = null;
+    let messagingPlan: SandboxMessagingPlan | null = session?.messagingPlan ?? null;
...
     session = deps.updateSession((current) => {
-      current.messagingPlan = messagingPlan;
+      current.messagingPlan = messagingPlan ?? current.messagingPlan ?? null;
       return current;
     });

Also applies to: 337-346

🤖 Prompt for 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.

In `@src/lib/onboard/machine/handlers/sandbox.ts` around lines 265 - 302, The code
currently initializes messagingPlan to null and then writes it into session via
deps.updateSession, which can erase an existing session.messagingPlan when
neither env nor registry provide a fresher plan; change the logic so
messagingPlan is initialized from the current session (e.g., messagingPlan =
session?.messagingPlan) and only overwrite it when
deps.readMessagingPlanFromEnv() or
deps.getRegistrySandboxMessagingPlan(sandboxName) return a non-null plan; ensure
the final deps.updateSession((current) => { current.messagingPlan =
messagingPlan ?? current.messagingPlan; return current; }) preserves the prior
session.messagingPlan when no new plan is found.

src/lib/messaging/applier/host-state-applier.test.ts (1)

53-75: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Strengthen this test to actually prove legacy-field cleanup on existing entries.

Line 54 seeds an entry without legacy keys, so Lines 74-75 pass even if cleanup regresses. Seed legacy fields first, then assert all three are removed (messagingChannels, messagingChannelConfig, disabledChannels).

Suggested test adjustment

   it("stores only the new messaging state on an existing sandbox entry", () => {
     registryMock.__setSandbox("demo", {
       name: "demo",
+      messagingChannels: ["telegram"],
+      messagingChannelConfig: { telegram: { requireMention: true } },
+      disabledChannels: ["telegram"],
     });
     const plan = makePlan(["telegram"]);
@@
     expect(registryMock.__getSandbox("demo")).not.toHaveProperty("messagingChannels");
+    expect(registryMock.__getSandbox("demo")).not.toHaveProperty("messagingChannelConfig");
     expect(registryMock.__getSandbox("demo")).not.toHaveProperty("disabledChannels");
   });

🤖 Prompt for 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.

In `@src/lib/messaging/applier/host-state-applier.test.ts` around lines 53 - 75,
The test currently seeds a sandbox without legacy keys so it can't prove
cleanup; update the setup for registryMock.__setSandbox("demo", ...) to include
legacy fields messagingChannels, messagingChannelConfig, and disabledChannels
with sample values, then call
MessagingHostStateApplier.applyPlanToRegistry("demo", plan) and assert it
returns true, assert registry.updateSandbox was called with the new messaging
object (schemaVersion and plan), and assert registryMock.__getSandbox("demo") no
longer has properties messagingChannels, messagingChannelConfig, or
disabledChannels while still matching the new messaging structure.

src/lib/onboard.ts (1)

368-381: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use configured plan channels here, not only active ones.

getRecordedMessagingChannelsForResumeFromState() is the resume seed for channel selection, so passing getActiveChannelsFromPlan(...) drops plan entries that are configured but currently disabled. On rebuild/resume that can turn a stopped channel into “not selected”, which loses the state channels start needs to reattach cached credentials.

Suggested fix

-const { getActiveChannelsFromPlan } = messagingPlanSession;
+const { getChannelsFromPlan } = messagingPlanSession;
...
-    sessionMessagingChannels: getActiveChannelsFromPlan(session?.messagingPlan),
+    sessionMessagingChannels: getChannelsFromPlan(session?.messagingPlan),

As per coding guidelines, src/lib/onboard.ts changes affect channel disable/enable lifecycle across rebuild with cached credentials, and the PR objective here is to keep configured, active, and disabled plan state distinct.

Also applies to: 5198-5206

🤖 Prompt for 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.

In `@src/lib/onboard.ts` around lines 368 - 381, The code is using
getActiveChannelsFromPlan(...) as the seed for
getRecordedMessagingChannelsForResumeFromState(), which drops plan entries that
are configured but currently disabled; change the call so the resume seed uses
the plan's configured channels instead of only active ones — i.e., pass the
configured channel list (obtainable from the plan/config: use
messagingPlanSession or
collectMessagingBuildConfig/getStoredMessagingChannelConfig outputs) into
getRecordedMessagingChannelsForResumeFromState rather than
getActiveChannelsFromPlan, so disabled-but-configured channels are preserved for
resume and credential reattachment.

Source: Coding guidelines

🤖 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 `@src/lib/policy/index.ts`:
- Around line 779-782: The console.log that reports "Widening sandbox egress —
adding: ..." is currently executed right after const endpoints =
getPresetEndpoints(presetContent) and can fire even when the subsequent
precondition checks abort (lines around the apply-preconditions block). Move the
log statement so it runs only after the precondition pass/abort logic (i.e.,
after the block that currently decides to abort on failure), ensuring you still
reference the same endpoints variable returned by getPresetEndpoints; keep the
log position immediately before the code that actually applies the egress
changes so operators only see the "adding" message when changes will be applied.

In `@test/e2e/docs/parity-inventory.generated.json`:
- Line 7989: The generated JSON fixture parity-inventory.generated.json is
missing SPDX metadata; update the generation routine that produces this file so
the output includes SPDX fields (e.g., a top-level "$comment" object or entries)
containing SPDX-FileCopyrightText and SPDX-License-Identifier like other
fixtures; modify the generator (the code that writes
parity-inventory.generated.json) to prepend or embed the SPDX "$comment"
metadata before/alongside the root object (preserving keys such as "text") so
the produced JSON matches the SPDX format used by other test fixtures.

---

Outside diff comments:
In `@src/lib/messaging/applier/host-state-applier.test.ts`:
- Around line 53-75: The test currently seeds a sandbox without legacy keys so
it can't prove cleanup; update the setup for registryMock.__setSandbox("demo",
...) to include legacy fields messagingChannels, messagingChannelConfig, and
disabledChannels with sample values, then call
MessagingHostStateApplier.applyPlanToRegistry("demo", plan) and assert it
returns true, assert registry.updateSandbox was called with the new messaging
object (schemaVersion and plan), and assert registryMock.__getSandbox("demo") no
longer has properties messagingChannels, messagingChannelConfig, or
disabledChannels while still matching the new messaging structure.

In `@src/lib/onboard.ts`:
- Around line 368-381: The code is using getActiveChannelsFromPlan(...) as the
seed for getRecordedMessagingChannelsForResumeFromState(), which drops plan
entries that are configured but currently disabled; change the call so the
resume seed uses the plan's configured channels instead of only active ones —
i.e., pass the configured channel list (obtainable from the plan/config: use
messagingPlanSession or
collectMessagingBuildConfig/getStoredMessagingChannelConfig outputs) into
getRecordedMessagingChannelsForResumeFromState rather than
getActiveChannelsFromPlan, so disabled-but-configured channels are preserved for
resume and credential reattachment.

In `@src/lib/onboard/machine/handlers/sandbox.ts`:
- Around line 265-302: The code currently initializes messagingPlan to null and
then writes it into session via deps.updateSession, which can erase an existing
session.messagingPlan when neither env nor registry provide a fresher plan;
change the logic so messagingPlan is initialized from the current session (e.g.,
messagingPlan = session?.messagingPlan) and only overwrite it when
deps.readMessagingPlanFromEnv() or
deps.getRegistrySandboxMessagingPlan(sandboxName) return a non-null plan; ensure
the final deps.updateSession((current) => { current.messagingPlan =
messagingPlan ?? current.messagingPlan; return current; }) preserves the prior
session.messagingPlan when no new plan is found.

🪄 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: 68f84691-37d0-4121-9c42-9e97991dc3f2

📥 Commits

Reviewing files that changed from the base of the PR and between d9db199 and a30c537.

📒 Files selected for processing (65)

• ci/test-file-size-budget.json
• src/lib/actions/inference-route-api.test.ts
• src/lib/actions/inference-set.test.ts
• src/lib/actions/sandbox/channel-status.test.ts
• src/lib/actions/sandbox/channel-status.ts
• src/lib/actions/sandbox/doctor.ts
• src/lib/actions/sandbox/policy-channel-conflict.test.ts
• src/lib/actions/sandbox/policy-channel.ts
• src/lib/actions/sandbox/rebuild.ts
• src/lib/inventory/index.test.ts
• src/lib/inventory/index.ts
• src/lib/messaging/applier/conflict-detection-legacy.test.ts
• src/lib/messaging/applier/conflict-detection/backfill.ts
• src/lib/messaging/applier/conflict-detection/entries.ts
• src/lib/messaging/applier/conflict-detection/index.ts
• src/lib/messaging/applier/conflict-detection/probe.ts
• src/lib/messaging/applier/conflict-detection/types.ts
• src/lib/messaging/applier/host-state-applier.test.ts
• src/lib/messaging/compiler/workflow-planner.test.ts
• src/lib/messaging/compiler/workflow-planner.ts
• src/lib/onboard.ts
• src/lib/onboard/channel-state.test.ts
• src/lib/onboard/channel-state.ts
• src/lib/onboard/machine/events.ts
• src/lib/onboard/machine/handlers/policies.test.ts
• src/lib/onboard/machine/handlers/policies.ts
• src/lib/onboard/machine/handlers/sandbox-messaging-plan.test.ts
• src/lib/onboard/machine/handlers/sandbox.test.ts
• src/lib/onboard/machine/handlers/sandbox.ts
• src/lib/onboard/messaging-config.ts
• src/lib/onboard/messaging-credentials.ts
• src/lib/onboard/messaging-plan-session.test.ts
• src/lib/onboard/messaging-plan-session.ts
• src/lib/onboard/messaging-reuse.test.ts
• src/lib/onboard/messaging-reuse.ts
• src/lib/onboard/session-updates.ts
• src/lib/policy/index.ts
• src/lib/sandbox/whatsapp-diagnostics.ts
• src/lib/state/onboard-session.test.ts
• src/lib/state/onboard-session.ts
• src/lib/state/registry-messaging.ts
• src/lib/state/registry.ts
• src/lib/status-command-deps.ts
• test/channels-add-preset.test.ts
• test/channels-remove-full-teardown.test.ts
• test/cli/connect-recovery.test.ts
• test/cli/doctor-gateway-token.test.ts
• test/cli/list-share-live-inference.test.ts
• test/cli/logs.test.ts
• test/e2e/docs/parity-inventory.generated.json
• test/e2e/test-channels-add-remove.sh
• test/e2e/test-channels-stop-start.sh
• test/e2e/test-messaging-providers.sh
• test/e2e/test-rebuild-hermes.sh
• test/helpers/messaging-plan-fixtures.ts
• test/helpers/sandbox-handler-fixtures.ts
• test/nemoclaw-start.test.ts
• test/onboard-messaging.test.ts
• test/onboard.test.ts
• test/policies.test.ts
• test/rebuild-credential-preflight.test.ts
• test/rebuild-shields-auto-unlock.test.ts
• test/registry.test.ts
• test/repro-2201.test.ts
• test/secret-redaction.test.ts

💤 Files with no reviewable changes (9)

• src/lib/messaging/applier/conflict-detection/probe.ts
• src/lib/messaging/applier/conflict-detection-legacy.test.ts
• src/lib/messaging/applier/conflict-detection/backfill.ts
• src/lib/onboard/session-updates.ts
• test/onboard.test.ts
• src/lib/messaging/applier/conflict-detection/types.ts
• src/lib/messaging/compiler/workflow-planner.test.ts
• src/lib/messaging/applier/conflict-detection/index.ts
• src/lib/state/onboard-session.ts

[coderabbitai]

🧹 Nitpick comments (1)

test/policies.test.ts (1)

735-735: 💤 Low value

Consider simplifying the mock spy implementation.

The mockImplementation throws a sentinel error when the disclosure string is logged, but the test at Line 747-Line 752 still verifies the log by checking mock.calls. The throw is caught by the inner try-catch at Line 742-Line 745, making it redundant. Either:

• Remove the throw and keep the no-op mock with the mock.calls assertion, or
• Catch and verify the sentinel error explicitly instead of checking mock.calls.

As written, the throw stops applyPreset mid-execution but the test doesn't rely on that behavior.

♻️ Proposed simplification

-      const logSpy = vi.spyOn(console, "log").mockImplementation((message) => { if (typeof message === "string" && message.includes("Widening sandbox egress")) throw new Error("__disclosure_logged__"); });
+      const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});

The test assertion at Line 747-Line 752 already verifies the log was called with the correct message.

🤖 Prompt for 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.

In `@test/policies.test.ts` at line 735, The console.log spy implementation
currently throws a sentinel error which short-circuits applyPreset; change the
mock to a no-op recorder instead (i.e., vi.spyOn(console,
"log").mockImplementation(() => {}) so it doesn't throw) and keep the assertion
that inspects logSpy.mock.calls to verify the "Widening sandbox egress" message;
reference the existing logSpy and the applyPreset call so you update the spy
used by that test.

🤖 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.

Nitpick comments:
In `@test/policies.test.ts`:
- Line 735: The console.log spy implementation currently throws a sentinel error
which short-circuits applyPreset; change the mock to a no-op recorder instead
(i.e., vi.spyOn(console, "log").mockImplementation(() => {}) so it doesn't
throw) and keep the assertion that inspects logSpy.mock.calls to verify the
"Widening sandbox egress" message; reference the existing logSpy and the
applyPreset call so you update the spy used by that test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 149e3ac7-9734-4d70-8aec-193ce65eed01

📥 Commits

Reviewing files that changed from the base of the PR and between a30c537 and 6a79070.

📒 Files selected for processing (3)

• src/lib/policy/index.ts
• test/e2e/docs/parity-inventory.generated.json
• test/policies.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/e2e/docs/parity-inventory.generated.json

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27188836892
Target ref: feat/messaging-plan-session-registry
Requested jobs: all (no filter)
Summary: 64 passed, 0 failed, 2 skipped

Job	Result
agent-turn-latency-e2e	✅ success
bedrock-runtime-compatible-anthropic-e2e	✅ success
brave-search-e2e	✅ success
channels-add-remove-e2e	✅ success
channels-stop-start-e2e	✅ success
cloud-e2e	✅ success
cloud-inference-e2e	✅ success
cloud-onboard-e2e	✅ success
common-egress-agent-e2e	✅ success
concurrent-gateway-ports-e2e	✅ success
credential-migration-e2e	✅ success
credential-sanitization-e2e	✅ success
device-auth-health-e2e	✅ success
diagnostics-e2e	✅ success
docs-validation-e2e	✅ success
double-onboard-e2e	✅ success
gpu-double-onboard-e2e	⏭️ skipped
gpu-e2e	⏭️ skipped
hermes-anthropic-inference-switch-e2e	✅ success
hermes-dashboard-e2e	✅ success
hermes-discord-e2e	✅ success
hermes-e2e	✅ success
hermes-inference-switch-e2e	✅ success
hermes-onboard-security-posture-e2e	✅ success
hermes-root-entrypoint-smoke-e2e	✅ success
hermes-secret-boundary-e2e	✅ success
hermes-slack-e2e	✅ success
inference-routing-e2e	✅ success
issue-2478-crash-loop-recovery-e2e	✅ success
issue-3600-gpu-proof-optional-e2e	✅ success
issue-4434-tui-unreachable-inference-e2e	✅ success
issue-4462-gateway-pinned-approval-characterization-e2e	✅ success
issue-4462-scope-upgrade-approval-e2e	✅ success
kimi-inference-compat-e2e	✅ success
launchable-smoke-e2e	✅ success
messaging-compatible-endpoint-e2e	✅ success
messaging-providers-e2e	✅ success
network-policy-e2e	✅ success
onboard-negative-paths-e2e	✅ success
onboard-repair-e2e	✅ success
onboard-resume-e2e	✅ success
openclaw-anthropic-inference-switch-e2e	✅ success
openclaw-discord-pairing-e2e	✅ success
openclaw-inference-switch-e2e	✅ success
openclaw-onboard-security-posture-e2e	✅ success
openclaw-skill-cli-e2e	✅ success
openclaw-slack-pairing-e2e	✅ success
openclaw-tui-chat-correlation-e2e	✅ success
openshell-gateway-upgrade-e2e	✅ success
overlayfs-autofix-e2e	✅ success
rebuild-hermes-e2e	✅ success
rebuild-hermes-stale-base-e2e	✅ success
rebuild-openclaw-e2e	✅ success
runtime-overrides-e2e	✅ success
sandbox-operations-e2e	✅ success
sandbox-survival-e2e	✅ success
sessions-agents-cli-e2e	✅ success
shields-config-e2e	✅ success
skill-agent-e2e	✅ success
snapshot-commands-e2e	✅ success
state-backup-restore-e2e	✅ success
telegram-injection-e2e	✅ success
token-rotation-e2e	✅ success
tunnel-lifecycle-e2e	✅ success
upgrade-stale-sandbox-e2e	✅ success
vm-driver-privileged-exec-routing-e2e	✅ success