test(e2e): restore messaging bash parity - #5806
Conversation
📝 WalkthroughWalkthroughThe PR adds assertions that compacted messaging plans drop derived workflow sections and that live messaging/provider tests omit persisted render, hook, token, and proxy data while preserving required policy fields. ChangesMessaging plan persistence and redaction checks
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 docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
![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 `@src/lib/messaging/plan-validation.test.ts`:
- Line 150: The new plan-validation test file is missing the repository-required
SPDX copyright and Apache-2.0 license header. Add the standard SPDX header at
the top of the file before the first test in plan-validation.test.ts, following
the same format used in other JS/TS test files in the repository.
🪄 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: 996feaf4-35a4-4352-8a03-eaa7eebeedf2
📒 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
| }); | ||
| }); | ||
|
|
||
| it("keeps compact persisted plans free of derived workflow sections", () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required SPDX header to this file.
This changed .test.ts file is missing the repository-mandated SPDX copyright and license header.
Suggested header
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0As per coding guidelines, **/*.{js,ts,tsx,sh} must include the SPDX copyright and Apache-2.0 license header.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("keeps compact persisted plans free of derived workflow sections", () => { | |
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |
| // SPDX-License-Identifier: Apache-2.0 |
🤖 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/plan-validation.test.ts` at line 150, The new
plan-validation test file is missing the repository-required SPDX copyright and
Apache-2.0 license header. Add the standard SPDX header at the top of the file
before the first test in plan-validation.test.ts, following the same format used
in other JS/TS test files in the repository.
Source: Coding guidelines
Vitest E2E Scenario Results —
|
| Job | Result |
|---|---|
| channels-add-remove-vitest | |
| messaging-providers-vitest | |
| openclaw-discord-pairing-vitest |
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28186092387
|
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-906: The Gateway capture file existence check in
messaging-providers.test.ts is currently only a soft assertion, so execution can
still reach the readFileSync call and fail with ENOENT. Replace the check around
fakeGateway.captureFile with a hard precondition before reading, and keep the
subsequent parsing/diagnostics in the same gated block so the test fails at the
intended assertion instead of crashing later.
🪄 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: 6ec9d1c5-f419-4677-a13d-d6c312d6a2a3
📒 Files selected for processing (2)
test/e2e-scenario/live/messaging-providers-helpers.tstest/e2e-scenario/live/messaging-providers.test.ts
| check(fs.existsSync(fakeGateway.captureFile), "M13f: fake Gateway capture file exists"); | ||
| const gatewayCaptureText = fs.readFileSync(fakeGateway.captureFile, "utf8"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the file read with a hard precondition.
check() is a soft assertion here, so a missing capture file still falls through to readFileSync() and crashes with ENOENT. That turns the intended assertion into an opaque test abort and skips the rest of the diagnostics in this block.
Suggested fix
- check(fs.existsSync(fakeGateway.captureFile), "M13f: fake Gateway capture file exists");
- const gatewayCaptureText = fs.readFileSync(fakeGateway.captureFile, "utf8");
+ if (!fs.existsSync(fakeGateway.captureFile)) {
+ throw new Error("M13f: fake Gateway capture file exists");
+ }
+ const gatewayCaptureText = fs.readFileSync(fakeGateway.captureFile, "utf8");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| check(fs.existsSync(fakeGateway.captureFile), "M13f: fake Gateway capture file exists"); | |
| const gatewayCaptureText = fs.readFileSync(fakeGateway.captureFile, "utf8"); | |
| if (!fs.existsSync(fakeGateway.captureFile)) { | |
| throw new Error("M13f: fake Gateway capture file exists"); | |
| } | |
| const gatewayCaptureText = fs.readFileSync(fakeGateway.captureFile, "utf8"); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 905-905: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(fakeGateway.captureFile, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 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/e2e-scenario/live/messaging-providers.test.ts` around lines 905 - 906,
The Gateway capture file existence check in messaging-providers.test.ts is
currently only a soft assertion, so execution can still reach the readFileSync
call and fail with ENOENT. Replace the check around fakeGateway.captureFile with
a hard precondition before reading, and keep the subsequent parsing/diagnostics
in the same gated block so the test fails at the intended assertion instead of
crashing later.
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28190315340
|
Vitest E2E Scenario Results —
|
| Job | Result |
|---|---|
| messaging-providers-vitest | |
| openclaw-discord-pairing-vitest | ✅ success |
wscurran
added
area: e2e
|
✨
Related open issues: |
Vitest E2E Scenario Results — ✅ All requested jobs passedRun: 28194778783
|
## 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 -->
4 participants
Summary
Restore issue #5800 parity package
P0-Cfor 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
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
agentRenderand per-channelhooksare derived runtime data, not durable registry/session state.nonesrc/lib/messaging/plan-validation.test.ts;test/e2e-scenario/live/channels-add-remove.test.ts; existingchannels-stop-start-helpers.tsaccount.proxy.nonetest/e2e-scenario/live/messaging-providers.test.ts;test/e2e-scenario/live/openclaw-discord-pairing.test.tstightened to require emptyaccountProxynonetest/e2e-scenario/live/messaging-providers.test.tscapture assertionnonetest/e2e-scenario/live/openclaw-discord-pairing.test.ts;test/e2e-scenario/support-tests/openclaw-discord-*nonetest/e2e-scenario/live/messaging-providers.test.tsnow checks pre/post policy text and Node binary scopeInference mode support
nonefor new/tightened assertions; live scenario install still uses existingNVIDIA_INFERENCE_API_KEYboundary where the pre-existing scenario requires it.NEMOCLAW_RUN_E2E_SCENARIOS=1but not executed without real sandbox/secrets.Validation
git diff --checknpm ci --ignore-scriptsnpm run build:clinpm run typecheck:clinpx 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.tsnpx vitest run src/lib/messaging/plan-validation.test.ts src/lib/state/onboard-session.test.ts test/registry.test.tsNEMOCLAW_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)messaging-providers-vitest: passed on PR headf6a00eb— https://github.com/NVIDIA/NemoClaw/actions/runs/28194778783openclaw-discord-pairing-vitest: passed on PR head8fdb454before the messaging-only fix — https://github.com/NVIDIA/NemoClaw/actions/runs/28190315340/job/83502969520channels-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 commitran the full pre-commit test hook and failed in unrelated CLI timeout/fake-runtime tests; this PR was committed with focused validation above aftertypecheck:cliwas fixed.Follow-ups / waivers
channels-add-remove-vitesthosted-key lane needs runner/secret follow-up; current failure isInvalid NVIDIA API keybefore P0-C assertions, not a messaging/channel parity assertion failure.Summary by CodeRabbit
Bug Fixes
Tests