test(e2e): restore messaging bash parity - #5807
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates messaging plan persistence tests, Telegram plan invariants, Discord and WhatsApp provider checks, hosted inference job environment validation, and the Discord pairing proxy assertion. ChangesMessaging test updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/e2e-scenario/live/messaging-providers.test.ts`:
- Around line 905-907: The new redaction assertion in the messaging-providers
e2e test is currently vacuous because `gatewayCaptureText` falls back to an
empty string when the fake Gateway capture file is missing. Update the
`fakeGateway.captureFile` handling in `messaging-providers.test.ts` so the test
first asserts the file exists, then reads it unconditionally into
`gatewayCaptureText` before performing the `M13f` redaction check. Keep the fix
scoped to the test setup around `fakeGateway.captureFile` and the subsequent
capture-text assertions so the test fails if the artifact is never produced.
🪄 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: a5ec0f6c-4cfb-4680-b857-d52ee4e72437
📒 Files selected for processing (4)
src/lib/messaging/plan-validation.test.tstest/e2e-scenario/live/channels-add-remove.test.tstest/e2e-scenario/live/messaging-providers.test.tstest/e2e-scenario/live/openclaw-discord-pairing.test.ts
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28186432462
|
|
Selective P0-C E2E status update for run https://github.com/NVIDIA/NemoClaw/actions/runs/28187168691:
Regular PR checks are green at current SHA |
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28187168691
|
|
Selective P0-C E2E final status for run https://github.com/NVIDIA/NemoClaw/actions/runs/28187168691:\n\n- |
8fdb454 to
d69d2ac
Compare
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28193989752
|
Vitest E2E Scenario Results —
|
| Job | Result |
|---|---|
| agent-turn-latency-vitest | |
| bedrock-runtime-compatible-anthropic-vitest | |
| brave-search-vitest | |
| channels-add-remove-vitest | |
| channels-stop-start-vitest | |
| cloud-inference-vitest | |
| cloud-onboard-vitest | |
| common-egress-agent-vitest | |
| concurrent-gateway-ports-vitest | |
| credential-migration-vitest | |
| credential-sanitization-vitest | |
| cron-preflight-inference-local-vitest | |
| device-auth-health-vitest | |
| diagnostics-vitest | |
| double-onboard-vitest | |
| full-e2e-vitest | |
| gateway-drift-preflight-vitest | |
| gateway-guard-recovery | |
| gateway-health-honest-vitest | |
| gpu-double-onboard-vitest | |
| gpu-e2e-vitest | |
| hermes-discord-vitest | |
| hermes-e2e-vitest | |
| hermes-inference-switch-vitest | |
| hermes-root-entrypoint-smoke-vitest | |
| hermes-sandbox-secret-boundary-vitest | |
| hermes-slack-vitest | |
| inference-routing-vitest | |
| issue-2478-crash-loop-recovery-vitest | |
| issue-4434-tui-unreachable-inference-vitest | |
| issue-4462-scope-upgrade-approval-vitest | |
| jetson-nvmap-gpu-vitest | |
| kimi-inference-compat-vitest | |
| launchable-smoke-vitest | |
| live-scenarios | |
| messaging-compatible-endpoint-vitest | |
| messaging-providers-vitest | |
| model-router-provider-routed-inference-vitest | |
| network-policy-vitest | |
| ollama-auth-proxy-vitest | |
| onboard-negative-paths-vitest | |
| onboard-repair-vitest | |
| onboard-resume-vitest | |
| openclaw-discord-pairing-vitest | |
| openclaw-inference-switch-vitest | |
| openclaw-skill-cli-vitest | |
| openclaw-slack-pairing-vitest | |
| openclaw-tui-chat-correlation-vitest | |
| openshell-gateway-upgrade-vitest | |
| openshell-version-pin-vitest | |
| overlayfs-autofix-vitest | |
| rebuild-hermes-stale-base-vitest | |
| rebuild-hermes-vitest | |
| rebuild-openclaw-vitest | |
| runtime-overrides-vitest | |
| sandbox-rebuild-vitest | |
| sandbox-survival-vitest | |
| sessions-agents-cli-vitest | |
| shields-config-vitest | |
| skill-agent-vitest | |
| snapshot-commands-vitest | |
| spark-install-vitest | |
| state-backup-restore-vitest | |
| telegram-injection-vitest | |
| token-rotation-vitest | |
| tunnel-lifecycle-vitest | |
| upgrade-stale-sandbox-vitest |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/e2e-scenarios/workflow-boundary.mts`:
- Around line 5944-5948: `COMPATIBLE_API_KEY` is only being checked for value
staging inside the run step, but it also needs a boundary guard outside the run
step so it cannot be exposed in the workflow environment. Add a non-run-step
protection for `COMPATIBLE_API_KEY` in the same area that already guards
`NVIDIA_INFERENCE_API_KEY`, using the existing
`requireEnvDoesNotExposeSecret`/workflow boundary checks around
`workflow-boundary.mts` to enforce that the secret is not present as an env key
before the step runs.
🪄 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: 65b47cbb-c4a1-476f-af98-d89e5f3420a2
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltools/e2e-scenarios/workflow-boundary.mts
wscurran
added
area: e2e
|
✨
Related open issues: |
Vitest E2E Scenario Results — ✅ All requested jobs passedRun: 28194650942
|
## 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 -->
3 participants
Summary
Restore issue #5800 parity package
P0-Cfor 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
P0-C — Messaging / Discord / channel paritytest/e2e: yes — refactor(messaging): compact persisted messaging plans #5328, test(e2e): update Discord proxy expectation #5391, test(e2e): migrate OpenClaw Discord pairing to Vitest #5581, test(e2e): fix Discord gateway capture proof #5624, fix(discord): stop emitting a non-loopback per-account proxy (#5544) #5571, test(e2e): harden WhatsApp policy assertions #5704Parity map
nonesrc/lib/messaging/plan-validation.test.ts;test/e2e-scenario/live/channels-add-remove.test.tsnonesrc/lib/messaging/applier/host-state-applier.test.tsnonetest/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.tshermetic-defaulttest/e2e-scenario/live/openclaw-discord-pairing.test.ts; support boundary/helper testshermetic-defaulttest/e2e-scenario/live/messaging-providers.test.ts; existing Hermes/OpenClaw Discord capture assertions and support testsnonetest/e2e-scenario/live/messaging-providers.test.ts;test/policies.test.tsInference mode support
nonefor config/unit assertions;hermetic-defaultfor fake Discord Gateway/live sandbox token-rewrite assertions.NVIDIA_INFERENCE_API_KEYpath where their broader scenario requires install/onboard.28194650942passedmessaging-providers-vitest,channels-add-remove-vitest, andopenclaw-discord-pairing-vitestat531acd9f8. Follow-up head46e004e3only tightens local workflow-boundary assertions forCOMPATIBLE_API_KEY.channels-add-remove-vitestalso passed the hosted-compatible workflow path after stagingNVIDIA_INFERENCE_API_KEYasCOMPATIBLE_API_KEY.Validation
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.tsnpx 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.tsnpx vitest run --project cli --maxWorkers 1 --no-fileParallelism --testTimeout 30000 test/generate-openclaw-config.test.ts -t "Discord|proxy|non-Slack"npx vitest run --project cli --maxWorkers 1 --no-fileParallelism test/policies.test.ts -t "whatsapp"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.ts28194650942:messaging-providers-vitest,channels-add-remove-vitest,openclaw-discord-pairing-vitestall passed.Follow-ups / waivers
Summary by CodeRabbit
agentRenderand per-channelhooksabsence.