refactor(messaging): compact persisted messaging plans - #5328
Merged
Conversation
Contributor
|
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 (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR separates runtime messaging plans (including derived ChangesMessaging Plan Persistence: Separation of Runtime and Disk Representations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
Contributor
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-advisor-raw-output.txt |
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorFailed: Could not parse JSON from advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-scenario-advisor-raw-output.txt |
Contributor
PR Review AdvisorFindings: 0 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
sandl99
added
refactor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 (1)
src/lib/messaging/compiler/workflow-planner.ts (1)
178-202:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep credential availability parsing aligned with the validated sandbox plan.
readSandboxEntryPlan()rejects persisted plans whosesandboxNameoragentno longer match the current sandbox, butcredentialAvailabilityFromSandboxEntry()ignores those selectors and still merges credential flags from the same raw payload. A stale or mis-associated registry plan can therefore suppress credential prompts for add/rebuild even when the planner refused to trust the plan itself. Pass the same selectors here, or derive availability fromreadSandboxEntryPlan(), so both paths trust the same persisted state.🤖 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/compiler/workflow-planner.ts` around lines 178 - 202, credentialAvailabilityFromSandboxEntry currently parses the raw sandboxEntry with parseSandboxMessagingPlan and can return availability from stale plans; change it to use the validated plan path (readSandboxEntryPlan) or call the same selector logic used by readSandboxEntryPlan so only plans whose sandboxName/agent match the current sandbox are trusted — locate credentialAvailabilityFromSandboxEntry, replace the parseSandboxMessagingPlan(sandboxEntry?.messaging?.plan) usage with the validated plan object returned by readSandboxEntryPlan(sandboxEntry, /* provide same selectors/context used elsewhere */) or add the same sandbox/agent checks before merging availability for each credential in registry.get(channelId); ensure you still iterate channelIds and manifest.credentials but derive bindings from the validated plan so availability is only set when readSandboxEntryPlan would accept the plan.
🧹 Nitpick comments (1)
src/lib/state/onboard-session.test.ts (1)
620-624: ⚡ Quick winAssert
hooksnormalization after reload as well.This test proves
hooksare stripped on disk, but the post-loadSession()assertion only checksagentRender. Adding achannels[0].hooks === []assertion would cover the second half of the compact/normalize contract too.✅ Suggested assertion update
const raw = JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf-8")); expect(raw.messagingPlan.agentRender).toBeUndefined(); expect(raw.messagingPlan.channels[0].hooks).toBeUndefined(); - expect(requireLoadedSession(session.loadSession()).messagingPlan?.agentRender).toEqual([]); + const reloadedPlan = requireLoadedSession(session.loadSession()).messagingPlan; + expect(reloadedPlan?.agentRender).toEqual([]); + expect(reloadedPlan?.channels[0]?.hooks).toEqual([]); });🤖 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/state/onboard-session.test.ts` around lines 620 - 624, The test parses the saved session and already asserts that on-disk messagingPlan.agentRender and channels[0].hooks are undefined but only checks agentRender after reload; update the post-load assertions to also verify hooks normalization by adding an assertion that requireLoadedSession(session.loadSession()).messagingPlan?.channels?.[0]?.hooks equals [] so the reload contract for hooks is covered alongside agentRender (referencing loadSession, requireLoadedSession, messagingPlan, and channels[0].hooks).
🤖 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/state/registry.ts`:
- Around line 319-349: normalizeRegistry/normalizeSandboxEntry dereference
malformed sandbox entries (e.g., null) causing TypeError during registry reads;
update normalizeRegistry (and serializeRegistryForDisk if symmetric behavior is
needed) to skip or sanitize non-object sandbox values before calling
normalizeSandboxEntry/serializeSandboxEntryForDisk by filtering
Object.entries(data.sandboxes ?? {}) to only include entries where typeof entry
=== "object" && entry !== null (or replace broken entries with a safe
default/empty entry), and ensure normalizeSandboxEntry still handles missing
messaging via cloneSandboxMessagingState(entry.messaging) without assuming entry
is an object.
In `@test/registry.test.ts`:
- Around line 245-246: The test callbacks use implicitly-typed parameters;
explicitly annotate them to satisfy TypeScript by typing the iterator parameters
for the arrays. Change the first callback to something like (entry: typeof
hydrated.agentRender[number]) => entry.channelId === "telegram" and the second
to (hook: typeof hydrated.channels[0].hooks[number]) => hook.channelId ===
"telegram", or import the concrete types (e.g., AgentRenderEntry / ChannelHook)
and use (entry: AgentRenderEntry) and (hook: ChannelHook) in the .some callbacks
for hydrated.agentRender and hydrated.channels[0].hooks respectively.
---
Outside diff comments:
In `@src/lib/messaging/compiler/workflow-planner.ts`:
- Around line 178-202: credentialAvailabilityFromSandboxEntry currently parses
the raw sandboxEntry with parseSandboxMessagingPlan and can return availability
from stale plans; change it to use the validated plan path
(readSandboxEntryPlan) or call the same selector logic used by
readSandboxEntryPlan so only plans whose sandboxName/agent match the current
sandbox are trusted — locate credentialAvailabilityFromSandboxEntry, replace the
parseSandboxMessagingPlan(sandboxEntry?.messaging?.plan) usage with the
validated plan object returned by readSandboxEntryPlan(sandboxEntry, /* provide
same selectors/context used elsewhere */) or add the same sandbox/agent checks
before merging availability for each credential in registry.get(channelId);
ensure you still iterate channelIds and manifest.credentials but derive bindings
from the validated plan so availability is only set when readSandboxEntryPlan
would accept the plan.
---
Nitpick comments:
In `@src/lib/state/onboard-session.test.ts`:
- Around line 620-624: The test parses the saved session and already asserts
that on-disk messagingPlan.agentRender and channels[0].hooks are undefined but
only checks agentRender after reload; update the post-load assertions to also
verify hooks normalization by adding an assertion that
requireLoadedSession(session.loadSession()).messagingPlan?.channels?.[0]?.hooks
equals [] so the reload contract for hooks is covered alongside agentRender
(referencing loadSession, requireLoadedSession, messagingPlan, and
channels[0].hooks).
🪄 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: 164fdd91-c4b9-4483-9b6e-ca6e5f1b5c07
📒 Files selected for processing (14)
src/lib/messaging/applier/host-state-applier.tssrc/lib/messaging/compiler/workflow-planner.tssrc/lib/messaging/index.tssrc/lib/messaging/persistence.tssrc/lib/messaging/plan-validation.test.tssrc/lib/messaging/plan-validation.tssrc/lib/onboard/messaging-channel-setup.tssrc/lib/state/onboard-session.test.tssrc/lib/state/onboard-session.tssrc/lib/state/registry-messaging.tssrc/lib/state/registry.tstest/e2e/test-channels-add-remove.shtest/e2e/test-channels-stop-start.shtest/registry.test.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27420277847
|
cv
approved these changes
Jun 13, 2026
This was referenced Jun 13, 2026
13 tasks
cv
pushed a commit
that referenced
this pull request
Jun 17, 2026
## Summary Refreshes release-prep documentation for NemoClaw v0.0.65. Adds the v0.0.65 release-notes section and refreshes generated `nemoclaw-user-*` skills from the Fern MDX source docs. ## Changes - Added the v0.0.65 release notes to `docs/about/release-notes.mdx` with links to the deeper docs pages for lifecycle, troubleshooting, inference, CLI commands, messaging, credentials, network policy, Hermes, and sub-agents. - Regenerated the `nemoclaw-user-*` skills with `scripts/docs-to-skills.py` so release-prep skill output matches the merged source docs. - Used the v0.0.65 announcement discussion as release context: #5472. ## Source Summary - #2492 -> `docs/about/release-notes.mdx`: Documents deadline-based gateway wait reliability in the v0.0.65 recovery summary. - #4958 -> `docs/about/release-notes.mdx`: Documents re-execed OpenClaw gateway health check recovery in the sandbox recovery summary. - #5163 -> `docs/about/release-notes.mdx`: Documents safer uninstall TTY confirmation behavior in the day-two CLI summary. - #5178 -> `docs/about/release-notes.mdx`: Documents fail-closed config restore merge behavior in the rebuild and restore summary. - #5179 -> `docs/about/release-notes.mdx`: Documents WeChat QR token redaction in the messaging summary. - #5182 -> `docs/about/release-notes.mdx`: Documents sustained gateway serving checks in the recovery summary. - #5194 -> `docs/about/release-notes.mdx`: Documents model-router teardown during uninstall in the day-two CLI summary. - #5195 -> `docs/about/release-notes.mdx`: Documents Shields auto-restore lock reconfirmation in the rebuild and restore summary. - #5198 -> `docs/about/release-notes.mdx`: Documents Docker Desktop WSL CDI injection failure handling in the onboarding diagnostics summary. - #5201 -> `docs/about/release-notes.mdx`: Documents sandbox download/upload wrappers and sessions export in the day-two CLI summary. - #5205 -> `docs/about/release-notes.mdx`: Documents reporter-owned model metadata preservation in the rebuild and restore summary. - #5214 -> `docs/about/release-notes.mdx`: Documents managed vLLM model preflight before side effects in the inference setup summary. - #5215 -> `docs/about/release-notes.mdx`: Documents managed vLLM extra serve arguments in the inference setup summary. - #5216 -> `docs/about/release-notes.mdx`: Documents silent OpenClaw runtime fallback surfacing in the onboarding diagnostics summary. - #5225 -> `docs/about/release-notes.mdx`: Documents persisted sandbox gateway lookup in the gateway recovery summary. - #5238 -> `docs/about/release-notes.mdx`: Documents sub-agent gateway dial-back through the sandbox interface in the Hermes and sub-agent summary. - #5248 -> `docs/about/release-notes.mdx`: Documents Discord per-account proxy resolution in the messaging summary. - #5264 -> `docs/about/release-notes.mdx`: Documents reserved Hermes port `8642` handling in the Hermes compatibility summary. - #5267 -> `docs/about/release-notes.mdx`: Documents the narrower Hermes baseline policy in the Hermes compatibility summary. - #5321 -> `docs/about/release-notes.mdx`: Documents restored gateway guard chains in the gateway recovery summary. - #5328 -> `docs/about/release-notes.mdx`: Documents compact persisted messaging plans in the messaging summary. - #5338 -> `docs/about/release-notes.mdx`: Documents manifest channel migration in the messaging summary. - #5352 -> `docs/about/release-notes.mdx`: Documents persisted agent preservation through registry recovery in the rebuild and restore summary. - #5371 -> `.agents/skills/nemoclaw-user-reference/references/commands.md`: Refreshes generated skill output for custom build cache and layer-ordering source docs. - #5379 -> `docs/about/release-notes.mdx`: Documents dashboard port allocation across multiple NemoClaw gateways in the recovery summary. - #5382 -> `docs/about/release-notes.mdx`: Documents recovery when an active gateway has no sandbox spec in the recovery summary. - #5389 -> `.agents/skills/nemoclaw-user-reference/references/troubleshooting.md`: Refreshes generated skill output for declared agent `forward_ports` recovery source docs. - #5400 -> `docs/about/release-notes.mdx`: Documents bounded compatible endpoint probes in the inference setup summary. - #5410 -> `docs/about/release-notes.mdx`: Documents provider credential hash removal from sandbox registry entries in the messaging summary. - #5418 -> `docs/about/release-notes.mdx`: Documents summarized inference validation failures in the onboarding diagnostics summary. - #5457 -> `docs/about/release-notes.mdx`: Documents context-window recomputation after runtime model switches in the inference setup summary. - #5463 -> `docs/about/release-notes.mdx`: Documents cleanup of hard-coded messaging channel stragglers in the messaging summary. ## Skipped - #5366 matched `docs/.docs-skip` entries through skipped experimental paths, so this PR does not add new release-note text for that commit. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Verification notes: - `npm run docs` passed after rerunning outside the sandbox. Fern reported 0 errors and 1 hidden warning. - The first sandboxed `npm run docs` attempt failed before validation because `tsx` could not create its local IPC pipe under sandbox restrictions. - `npm run build:cli` passed before push to refresh the local `dist/` artifacts used by the CLI typecheck hook. - `npm test` was not run because this is a docs-only release refresh. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Released NemoClaw v0.0.65 with improved gateway/sandbox recovery, safer day-two workflows, and enhanced Hermes compatibility. * Added managed vLLM extra-arguments configuration via `NEMOCLAW_VLLM_EXTRA_ARGS_JSON`. * Added Hermes troubleshooting guidance for port forwarding and health checks. * **Documentation** * Updated NVIDIA Endpoints/NIM setup and examples to use `NVIDIA_INFERENCE_API_KEY`. * Refined NVIDIA network policy and Model Router API base configuration. * Expanded CLI/environment variable documentation (including sub-agent gateway connectivity) and plugin build performance tips. * **Tests** * Expanded Vitest-backed E2E release validation coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Jun 22, 2026
This was referenced Jun 25, 2026
jyaunches
added a commit
that referenced
this pull request
Jun 25, 2026
## Summary Restore issue #5800 parity package `P0-C` for merged bash-suite messaging/Discord/WhatsApp deltas only. ## Related Issues Refs #5800 Refs #5098 Refs #5328 Refs #5391 Refs #5581 Refs #5624 Refs #5571 Refs #5704 ## Scope gate - Package: `P0-C — Messaging / Discord / channel parity` - Included PRs all merged and touched `test/e2e`: yes — #5328, #5391, #5581, #5624, #5571, #5704 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | C1 | #5328 | Compact persisted messaging plans omit derived render/build/runtime/state/health sections while retaining durable channel/config/credential/policy shape. | `none` | `src/lib/messaging/plan-validation.test.ts`; `test/e2e-scenario/live/channels-add-remove.test.ts` | covered | | C2 | #5328 | Existing compact plans hydrate before merge so channel add preserves prior hooks/render semantics. | `none` | existing `src/lib/messaging/applier/host-state-applier.test.ts` | covered | | C3 | #5391, #5571 | Discord config must not emit a non-loopback per-account proxy; OpenClaw managed proxy remains configured. | `none` | `test/discord-template-resolver-proxy.test.ts`; `test/generate-openclaw-config.test.ts`; `test/e2e-scenario/live/messaging-providers.test.ts`; `test/e2e-scenario/live/openclaw-discord-pairing.test.ts` | covered | | C4 | #5581 | OpenClaw Discord pairing Vitest preserves fake Gateway token rewrite, connect-shell approval, and workflow dispatch boundary. | `hermetic-default` | existing `test/e2e-scenario/live/openclaw-discord-pairing.test.ts`; support boundary/helper tests | covered | | C5 | #5624 | Fake Discord Gateway capture proof accepts only redacted identify rows, rejects placeholder/raw-token leakage, and proves token rewrite. | `hermetic-default` | `test/e2e-scenario/live/messaging-providers.test.ts`; existing Hermes/OpenClaw Discord capture assertions and support tests | covered | | C6 | #5704 | WhatsApp policy checks require expected endpoints before rebuild and endpoints plus Node binary scope after rebuild. | `none` | `test/e2e-scenario/live/messaging-providers.test.ts`; `test/policies.test.ts` | covered | ## Inference mode support - Default mode for touched live targets: `none` for config/unit assertions; `hermetic-default` for fake Discord Gateway/live sandbox token-rewrite assertions. - Real inference support preserved: not applicable to this package’s messaging/provider contracts; live sandbox targets still use existing `NVIDIA_INFERENCE_API_KEY` path where their broader scenario requires install/onboard. - Modes validated in this PR: unit/support hermetic commands below; selective live E2E run `28194650942` passed `messaging-providers-vitest`, `channels-add-remove-vitest`, and `openclaw-discord-pairing-vitest` at `531acd9f8`. Follow-up head `46e004e3` only tightens local workflow-boundary assertions for `COMPATIBLE_API_KEY`. - If not validated with real inference: package contracts are messaging/config/proxy/capture policy boundaries; `channels-add-remove-vitest` also passed the hosted-compatible workflow path after staging `NVIDIA_INFERENCE_API_KEY` as `COMPATIBLE_API_KEY`. ## Validation - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism src/lib/messaging/plan-validation.test.ts src/lib/messaging/applier/host-state-applier.test.ts test/discord-template-resolver-proxy.test.ts` - [x] `npx vitest run --project e2e-vitest-support --maxWorkers 1 --no-fileParallelism test/e2e-scenario/support-tests/openclaw-discord-legacy-capture.test.ts test/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism --testTimeout 30000 test/generate-openclaw-config.test.ts -t "Discord|proxy|non-Slack"` - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism test/policies.test.ts -t "whatsapp"` - [x] `npx vitest run --project e2e-vitest-support --maxWorkers 1 --no-fileParallelism test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] Selective live E2E workflow `28194650942`: `messaging-providers-vitest`, `channels-add-remove-vitest`, `openclaw-discord-pairing-vitest` all passed. ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved messaging plan persistence validation to ensure only required fields are stored; derived workflow sections and per-channel hook data are no longer persisted. * Strengthened live channel add/remove assertions to enforce `agentRender` and per-channel `hooks` absence. * Updated live messaging provider and Discord pairing validations (WhatsApp preset hosts and stricter gateway capture checks; account proxy now required to be exactly empty when unset). * **Tests / CI** * Enhanced Vitest/e2e scenario test tooling and environment setup for hosted-compatible inference, including compatible API key staging and more robust Discord gateway capture/proxy handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
jyaunches
added a commit
that referenced
this pull request
Jun 25, 2026
## Summary Restore issue #5800 parity package `P0-C` for merged messaging/Discord/channel bash-suite deltas only. ## Related Issues Refs #5800 Refs #5098 Refs #5328 Refs #5391 Refs #5581 Refs #5624 Refs #5571 Refs #5704 ## Scope gate - Package: `P0-C — Messaging / Discord / channel parity` - Included PRs all merged and touched `test/e2e`: yes — #5328, #5391, #5581, #5624, #5571, #5704 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | C1 | #5328 | Persisted messaging plans stay compact: `agentRender` and per-channel `hooks` are derived runtime data, not durable registry/session state. | `none` | `src/lib/messaging/plan-validation.test.ts`; `test/e2e-scenario/live/channels-add-remove.test.ts`; existing `channels-stop-start-helpers.ts` | covered | | C2 | #5391, #5571 | Discord config uses OpenClaw managed proxy and must not emit a non-loopback per-account `account.proxy`. | `none` | Existing `test/e2e-scenario/live/messaging-providers.test.ts`; `test/e2e-scenario/live/openclaw-discord-pairing.test.ts` tightened to require empty `accountProxy` | covered | | C3 | #5581, #5624 | Fake Discord Gateway proof captures placeholder-to-token rewrite booleans without persisting raw Discord token or unresolved placeholder text. | `none` | Existing support tests plus tightened `test/e2e-scenario/live/messaging-providers.test.ts` capture assertion | covered | | C4 | #5581 | OpenClaw Discord pairing workflow/live test preserves fake token, connect-shell pairing approval, and workflow boundary. | `none` | Existing `test/e2e-scenario/live/openclaw-discord-pairing.test.ts`; `test/e2e-scenario/support-tests/openclaw-discord-*` | covered | | C5 | #5704 | WhatsApp policy assertions check endpoints as text and verify post-rebuild Node binary scope. | `none` | `test/e2e-scenario/live/messaging-providers.test.ts` now checks pre/post policy text and Node binary scope | covered | ## Inference mode support - Default mode for touched live targets: `none` for new/tightened assertions; live scenario install still uses existing `NVIDIA_INFERENCE_API_KEY` boundary where the pre-existing scenario requires it. - Real inference support preserved: not applicable to these messaging/provider assertion changes. - Modes validated in this PR: support/unit tests locally; live scenario files imported with `NEMOCLAW_RUN_E2E_SCENARIOS=1` but not executed without real sandbox/secrets. - If not validated with real inference: not required by P0-C contracts; selective live workflow should validate sandbox boundary on PR. ## Validation - [x] `git diff --check` - [x] `npm ci --ignore-scripts` - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts test/e2e-scenario/support-tests/openclaw-discord-legacy-capture.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] `npx vitest run src/lib/messaging/plan-validation.test.ts src/lib/state/onboard-session.test.ts test/registry.test.ts` - [x] `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/channels-add-remove.test.ts test/e2e-scenario/live/messaging-providers.test.ts test/e2e-scenario/live/openclaw-discord-pairing.test.ts test/e2e-scenario/live/channels-stop-start.test.ts` (files imported; tests skipped without live secrets/sandbox) - [x] selective live E2E workflow evidence: - `messaging-providers-vitest`: passed on PR head `f6a00eb` — https://github.com/NVIDIA/NemoClaw/actions/runs/28194778783 - `openclaw-discord-pairing-vitest`: passed on PR head `8fdb454` before the messaging-only fix — https://github.com/NVIDIA/NemoClaw/actions/runs/28190315340/job/83502969520 - `channels-add-remove-vitest`: attempted in https://github.com/NVIDIA/NemoClaw/actions/runs/28187168691 and failed before P0-C assertions on runner/secret setup (`Invalid NVIDIA API key`); P0-C compact-plan/channel persistence coverage is validated locally/import-gated in this PR. Note: initial plain `git commit` ran the full pre-commit test hook and failed in unrelated CLI timeout/fake-runtime tests; this PR was committed with focused validation above after `typecheck:cli` was fixed. ## Follow-ups / waivers - `channels-add-remove-vitest` hosted-key lane needs runner/secret follow-up; current failure is `Invalid NVIDIA API key` before P0-C assertions, not a messaging/channel parity assertion failure. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured persisted messaging plans only retain core channel/network settings; derived workflow data (including agent render and per-channel hooks) is no longer carried into saved plans. * **Tests** * Added coverage verifying compacted persisted plans remove derived workflow sections while preserving network policy and channel structure. * Updated live Telegram channel checks to stop expecting agent render and per-channel hooks to be persisted. * Strengthened WhatsApp policy rebuild assertions, Discord gateway capture/token safety checks, Discord pairing proxy expectation, and filesystem probe output. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Restore issue NVIDIA#5800 parity package `P0-C` for merged bash-suite messaging/Discord/WhatsApp deltas only. ## Related Issues Refs NVIDIA#5800 Refs NVIDIA#5098 Refs NVIDIA#5328 Refs NVIDIA#5391 Refs NVIDIA#5581 Refs NVIDIA#5624 Refs NVIDIA#5571 Refs NVIDIA#5704 ## Scope gate - Package: `P0-C — Messaging / Discord / channel parity` - Included PRs all merged and touched `test/e2e`: yes — NVIDIA#5328, NVIDIA#5391, NVIDIA#5581, NVIDIA#5624, NVIDIA#5571, NVIDIA#5704 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | C1 | NVIDIA#5328 | Compact persisted messaging plans omit derived render/build/runtime/state/health sections while retaining durable channel/config/credential/policy shape. | `none` | `src/lib/messaging/plan-validation.test.ts`; `test/e2e-scenario/live/channels-add-remove.test.ts` | covered | | C2 | NVIDIA#5328 | Existing compact plans hydrate before merge so channel add preserves prior hooks/render semantics. | `none` | existing `src/lib/messaging/applier/host-state-applier.test.ts` | covered | | C3 | NVIDIA#5391, NVIDIA#5571 | Discord config must not emit a non-loopback per-account proxy; OpenClaw managed proxy remains configured. | `none` | `test/discord-template-resolver-proxy.test.ts`; `test/generate-openclaw-config.test.ts`; `test/e2e-scenario/live/messaging-providers.test.ts`; `test/e2e-scenario/live/openclaw-discord-pairing.test.ts` | covered | | C4 | NVIDIA#5581 | OpenClaw Discord pairing Vitest preserves fake Gateway token rewrite, connect-shell approval, and workflow dispatch boundary. | `hermetic-default` | existing `test/e2e-scenario/live/openclaw-discord-pairing.test.ts`; support boundary/helper tests | covered | | C5 | NVIDIA#5624 | Fake Discord Gateway capture proof accepts only redacted identify rows, rejects placeholder/raw-token leakage, and proves token rewrite. | `hermetic-default` | `test/e2e-scenario/live/messaging-providers.test.ts`; existing Hermes/OpenClaw Discord capture assertions and support tests | covered | | C6 | NVIDIA#5704 | WhatsApp policy checks require expected endpoints before rebuild and endpoints plus Node binary scope after rebuild. | `none` | `test/e2e-scenario/live/messaging-providers.test.ts`; `test/policies.test.ts` | covered | ## Inference mode support - Default mode for touched live targets: `none` for config/unit assertions; `hermetic-default` for fake Discord Gateway/live sandbox token-rewrite assertions. - Real inference support preserved: not applicable to this package’s messaging/provider contracts; live sandbox targets still use existing `NVIDIA_INFERENCE_API_KEY` path where their broader scenario requires install/onboard. - Modes validated in this PR: unit/support hermetic commands below; selective live E2E run `28194650942` passed `messaging-providers-vitest`, `channels-add-remove-vitest`, and `openclaw-discord-pairing-vitest` at `531acd9f8`. Follow-up head `46e004e3` only tightens local workflow-boundary assertions for `COMPATIBLE_API_KEY`. - If not validated with real inference: package contracts are messaging/config/proxy/capture policy boundaries; `channels-add-remove-vitest` also passed the hosted-compatible workflow path after staging `NVIDIA_INFERENCE_API_KEY` as `COMPATIBLE_API_KEY`. ## Validation - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism src/lib/messaging/plan-validation.test.ts src/lib/messaging/applier/host-state-applier.test.ts test/discord-template-resolver-proxy.test.ts` - [x] `npx vitest run --project e2e-vitest-support --maxWorkers 1 --no-fileParallelism test/e2e-scenario/support-tests/openclaw-discord-legacy-capture.test.ts test/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism --testTimeout 30000 test/generate-openclaw-config.test.ts -t "Discord|proxy|non-Slack"` - [x] `npx vitest run --project cli --maxWorkers 1 --no-fileParallelism test/policies.test.ts -t "whatsapp"` - [x] `npx vitest run --project e2e-vitest-support --maxWorkers 1 --no-fileParallelism test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] Selective live E2E workflow `28194650942`: `messaging-providers-vitest`, `channels-add-remove-vitest`, `openclaw-discord-pairing-vitest` all passed. ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved messaging plan persistence validation to ensure only required fields are stored; derived workflow sections and per-channel hook data are no longer persisted. * Strengthened live channel add/remove assertions to enforce `agentRender` and per-channel `hooks` absence. * Updated live messaging provider and Discord pairing validations (WhatsApp preset hosts and stricter gateway capture checks; account proxy now required to be exactly empty when unset). * **Tests / CI** * Enhanced Vitest/e2e scenario test tooling and environment setup for hosted-compatible inference, including compatible API key staging and more robust Discord gateway capture/proxy handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Restore issue NVIDIA#5800 parity package `P0-C` for merged messaging/Discord/channel bash-suite deltas only. ## Related Issues Refs NVIDIA#5800 Refs NVIDIA#5098 Refs NVIDIA#5328 Refs NVIDIA#5391 Refs NVIDIA#5581 Refs NVIDIA#5624 Refs NVIDIA#5571 Refs NVIDIA#5704 ## Scope gate - Package: `P0-C — Messaging / Discord / channel parity` - Included PRs all merged and touched `test/e2e`: yes — NVIDIA#5328, NVIDIA#5391, NVIDIA#5581, NVIDIA#5624, NVIDIA#5571, NVIDIA#5704 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | C1 | NVIDIA#5328 | Persisted messaging plans stay compact: `agentRender` and per-channel `hooks` are derived runtime data, not durable registry/session state. | `none` | `src/lib/messaging/plan-validation.test.ts`; `test/e2e-scenario/live/channels-add-remove.test.ts`; existing `channels-stop-start-helpers.ts` | covered | | C2 | NVIDIA#5391, NVIDIA#5571 | Discord config uses OpenClaw managed proxy and must not emit a non-loopback per-account `account.proxy`. | `none` | Existing `test/e2e-scenario/live/messaging-providers.test.ts`; `test/e2e-scenario/live/openclaw-discord-pairing.test.ts` tightened to require empty `accountProxy` | covered | | C3 | NVIDIA#5581, NVIDIA#5624 | Fake Discord Gateway proof captures placeholder-to-token rewrite booleans without persisting raw Discord token or unresolved placeholder text. | `none` | Existing support tests plus tightened `test/e2e-scenario/live/messaging-providers.test.ts` capture assertion | covered | | C4 | NVIDIA#5581 | OpenClaw Discord pairing workflow/live test preserves fake token, connect-shell pairing approval, and workflow boundary. | `none` | Existing `test/e2e-scenario/live/openclaw-discord-pairing.test.ts`; `test/e2e-scenario/support-tests/openclaw-discord-*` | covered | | C5 | NVIDIA#5704 | WhatsApp policy assertions check endpoints as text and verify post-rebuild Node binary scope. | `none` | `test/e2e-scenario/live/messaging-providers.test.ts` now checks pre/post policy text and Node binary scope | covered | ## Inference mode support - Default mode for touched live targets: `none` for new/tightened assertions; live scenario install still uses existing `NVIDIA_INFERENCE_API_KEY` boundary where the pre-existing scenario requires it. - Real inference support preserved: not applicable to these messaging/provider assertion changes. - Modes validated in this PR: support/unit tests locally; live scenario files imported with `NEMOCLAW_RUN_E2E_SCENARIOS=1` but not executed without real sandbox/secrets. - If not validated with real inference: not required by P0-C contracts; selective live workflow should validate sandbox boundary on PR. ## Validation - [x] `git diff --check` - [x] `npm ci --ignore-scripts` - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts test/e2e-scenario/support-tests/openclaw-discord-legacy-capture.test.ts test/e2e-scenario/support-tests/openclaw-discord-workflow-boundary.test.ts` - [x] `npx vitest run src/lib/messaging/plan-validation.test.ts src/lib/state/onboard-session.test.ts test/registry.test.ts` - [x] `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/channels-add-remove.test.ts test/e2e-scenario/live/messaging-providers.test.ts test/e2e-scenario/live/openclaw-discord-pairing.test.ts test/e2e-scenario/live/channels-stop-start.test.ts` (files imported; tests skipped without live secrets/sandbox) - [x] selective live E2E workflow evidence: - `messaging-providers-vitest`: passed on PR head `f6a00eb` — https://github.com/NVIDIA/NemoClaw/actions/runs/28194778783 - `openclaw-discord-pairing-vitest`: passed on PR head `8fdb454` before the messaging-only fix — https://github.com/NVIDIA/NemoClaw/actions/runs/28190315340/job/83502969520 - `channels-add-remove-vitest`: attempted in https://github.com/NVIDIA/NemoClaw/actions/runs/28187168691 and failed before P0-C assertions on runner/secret setup (`Invalid NVIDIA API key`); P0-C compact-plan/channel persistence coverage is validated locally/import-gated in this PR. Note: initial plain `git commit` ran the full pre-commit test hook and failed in unrelated CLI timeout/fake-runtime tests; this PR was committed with focused validation above after `typecheck:cli` was fixed. ## Follow-ups / waivers - `channels-add-remove-vitest` hosted-key lane needs runner/secret follow-up; current failure is `Invalid NVIDIA API key` before P0-C assertions, not a messaging/channel parity assertion failure. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured persisted messaging plans only retain core channel/network settings; derived workflow data (including agent render and per-channel hooks) is no longer carried into saved plans. * **Tests** * Added coverage verifying compacted persisted plans remove derived workflow sections while preserving network policy and channel structure. * Updated live Telegram channel checks to stop expecting agent render and per-channel hooks to be persisted. * Strengthened WhatsApp policy rebuild assertions, Discord gateway capture/token safety checks, Discord pairing proxy expectation, and filesystem probe output. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR compacts persisted messaging plans so onboard sessions and the sandbox registry no longer store manifest-derived render and hook metadata. Runtime paths hydrate those fields from built-in messaging manifests before rebuild and resume planning.
Changes
src/lib/messaging/persistence.tsto stripagentRenderandchannels[].hooksfor disk writes and hydrate them for runtime consumers.onboard-session.jsonandsandboxes.jsonwhile keeping registry and session readers compatible with compact persisted plans.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
Bug Fixes
New Features
Tests