Skip to content

test(e2e): tighten WeChat channel assertions - #4340

Merged
cv merged 4 commits into
mainfrom
u/sdang/wechat-e2e-registry-assertions
May 29, 2026
Merged

test(e2e): tighten WeChat channel assertions#4340
cv merged 4 commits into
mainfrom
u/sdang/wechat-e2e-registry-assertions

Conversation

@sandl99

@sandl99 sandl99 commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Tightens the WeChat messaging-provider E2E coverage so the non-interactive WECHAT_BOT_TOKEN path fails when required OpenClaw state is missing. This covers the regression class where channels.openclaw-weixin or the plugin install registry is not
restored.

Related Issue

Related #4237: #4237 is not a bug, while this PR is to tighten the WeChat validation in non-interactive flow

Changes

  • Add an E2E assertion for plugins.installs.openclaw-weixin in openclaw.json.
  • Change missing channels.openclaw-weixin.accounts[$WECHAT_ACCOUNT].enabled from skip to fail.
  • Change missing WeChat account state files and account index from skip to fail.

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)
  • Test-only change

Verification

  • 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)

Additional checks run:

  • bash -n test/e2e/test-messaging-providers.sh
  • git diff --check
  • npx vitest run test/e2e/scenario-framework-tests/e2e-convention-lint.test.ts test/e2e/scenario-framework-tests/e2e-coverage-report.test.ts

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Tests
    • Added a new end-to-end check enforcing WeChat plugin installation metadata and enabled entry, ensuring the plugin is installed, sourced from npm, pinned to a concrete version, and has an install path.
    • Hardened WeChat E2E checks to fail (with explicit error messages) when the channel or per-account credential files are missing or the account is disabled, improving test reliability and faster detection.

Review Change Stack

@copy-pr-bot

copy-pr-bot Bot commented May 27, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@sandl99 sandl99 self-assigned this May 27, 2026
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 705c0859-4950-41b4-9f6b-a50fa1a77b38

📥 Commits

Reviewing files that changed from the base of the PR and between 0e257b6 and 5ab5ec4.

📒 Files selected for processing (1)
  • test/e2e/test-messaging-providers.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e/test-messaging-providers.sh

📝 Walkthrough

Walkthrough

This PR hardens WeChat E2E test assertions by adding M-W7 to validate plugin install metadata and converting M-W8, M-W9, and M-W10 to fail (instead of skip) when WeChat plugin/account configuration or credential files are missing.

Changes

WeChat E2E Test Validation

Layer / File(s) Summary
WeChat test assertion validation
test/e2e/test-messaging-providers.sh
Adds new check M-W7 to validate plugins.installs.openclaw-weixin in /sandbox/.openclaw/openclaw.json. Changes M-W8 to fail when channels.openclaw-weixin is disabled, M-W9 to fail when per-account credential file is missing, and M-W10 to fail when accounts index (accounts.json) is missing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Suggested labels

enhancement: testing

Suggested reviewers

  • cv
  • ericksoa

Poem

🐰 A rabbit hops through test assertions true,
WeChat plugins checked and file paths reviewed,
Skips turned to fails so gaps won't roam,
Configs inspected, each account called home,
A tiny cheer — the tests are pruned and renewed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test(e2e): tighten WeChat channel assertions' accurately and concisely summarizes the main change—strengthening E2E test assertions for the WeChat channel to fail on missing required state instead of skipping.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch u/sdang/wechat-e2e-registry-assertions

Comment @coderabbitai help to get the list of available commands and usage tips.

@sandl99 sandl99 added the E2E label May 27, 2026
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: messaging-providers-e2e

Dispatch hint: messaging-providers-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No merge-blocking E2E is required because this PR is tests-only and cannot change NemoClaw runtime or user flows. Running the touched messaging-providers E2E is useful as an optional validation of the test change itself.

Optional E2E

  • messaging-providers-e2e (medium; about 75 minute timeout and creates a sandbox): Optional self-validation of the modified E2E script. This job runs test/e2e/test-messaging-providers.sh and would confirm the new WeChat M-W7/M-W8/M-W9/M-W10 assertions are valid against the current sandbox build.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: E2E / Nightly
  • jobs input: messaging-providers-e2e

@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. Changed file is under legacy test/e2e/ and outside the scenario E2E suite/workflows/catalogs/runtime; scenario E2E is not affected.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 2 worth checking, 0 nice ideas
Since last review: 2 prior items resolved, 1 still applies, 0 new items found

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: WeChat plugin install registry assertion in M-W7: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The new regex permits @tencent-weixin/openclaw-weixin@0.0.0 and the installPath check permits any non-empty string. Dockerfile.base and WECHAT_PLUGIN_SPEC point to @2.4.3, while seed unit tests show both exact default restoration and compatibility-preserving older pinned records.
  • M-W7 still does not enforce the trusted pinned WeChat install record (test/e2e/test-messaging-providers.sh:1766): The new M-W7 check is a useful improvement over the previous startswith() assertion because it rejects floating tags such as @latest and also verifies the plugin entry is enabled. However, for an installer-trust boundary it still accepts any semver for @tencent-weixin/openclaw-weixin and any non-empty installPath, and it does not require the install record to remain pinned. That means this E2E could pass with a downgraded/unreviewed package version or a tampered plugin path while reporting that the spec is pinned.
    • Recommendation: Make M-W7 compare against the trusted source-of-truth record for this default E2E path, or at least require the expected pinned fields and a trusted installPath. For example, assert the current default spec @tencent-weixin/openclaw-weixin@2.4.3 when the test starts from the current Dockerfile/source defaults, require pinned metadata if it is expected to survive, and constrain installPath to the expected OpenClaw npm/plugin location rather than any non-empty string.
    • Evidence: M-W7 currently uses re.fullmatch(r"@tencent-weixin/openclaw-weixin@\d+\.\d+\.\d+", spec), checks only bool(install_path.strip()), and does not inspect pinned. Nearby source installs @tencent-weixin/openclaw-weixin@2.4.3 --pin in Dockerfile.base and scripts/seed-wechat-accounts.py defines WECHAT_PLUGIN_SPEC = @tencent-weixin/openclaw-weixin@2.4.3; unit tests assert exact default install records while also preserving existing older pinned specs.

🌱 Nice ideas

  • None.
Since last review details

Current findings:

  • Source-of-truth review needed: WeChat plugin install registry assertion in M-W7: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The new regex permits @tencent-weixin/openclaw-weixin@0.0.0 and the installPath check permits any non-empty string. Dockerfile.base and WECHAT_PLUGIN_SPEC point to @2.4.3, while seed unit tests show both exact default restoration and compatibility-preserving older pinned records.
  • M-W7 still does not enforce the trusted pinned WeChat install record (test/e2e/test-messaging-providers.sh:1766): The new M-W7 check is a useful improvement over the previous startswith() assertion because it rejects floating tags such as @latest and also verifies the plugin entry is enabled. However, for an installer-trust boundary it still accepts any semver for @tencent-weixin/openclaw-weixin and any non-empty installPath, and it does not require the install record to remain pinned. That means this E2E could pass with a downgraded/unreviewed package version or a tampered plugin path while reporting that the spec is pinned.
    • Recommendation: Make M-W7 compare against the trusted source-of-truth record for this default E2E path, or at least require the expected pinned fields and a trusted installPath. For example, assert the current default spec @tencent-weixin/openclaw-weixin@2.4.3 when the test starts from the current Dockerfile/source defaults, require pinned metadata if it is expected to survive, and constrain installPath to the expected OpenClaw npm/plugin location rather than any non-empty string.
    • Evidence: M-W7 currently uses re.fullmatch(r"@tencent-weixin/openclaw-weixin@\d+\.\d+\.\d+", spec), checks only bool(install_path.strip()), and does not inspect pinned. Nearby source installs @tencent-weixin/openclaw-weixin@2.4.3 --pin in Dockerfile.base and scripts/seed-wechat-accounts.py defines WECHAT_PLUGIN_SPEC = @tencent-weixin/openclaw-weixin@2.4.3; unit tests assert exact default install records while also preserving existing older pinned specs.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26527016252
Target ref: u/sdang/wechat-e2e-registry-assertions
Requested jobs: all (no filter)
Summary: 3 passed, 0 failed, 2 skipped

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

@sandl99 sandl99 added integration: wechat WeChat integration behavior VRDC Issues and PRs submitted by NVIDIA VRDC test team. labels May 27, 2026
@sandl99
sandl99 marked this pull request as ready for review May 27, 2026 17:21
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26527044069
Target ref: u/sdang/wechat-e2e-registry-assertions
Requested jobs: messaging-providers-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
messaging-providers-e2e ✅ success

@sandl99
sandl99 requested review from cv and ericksoa May 27, 2026 17:37
@ericksoa ericksoa added v0.0.55 and removed v0.0.53 labels May 27, 2026
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@jyaunches
jyaunches self-requested a review May 29, 2026 17:48
Tighten M-W7 in response to PR review advisor feedback on #4340:

- Reject floating install specs (e.g. "@latest") by matching the spec
  against a strict semver pattern. Source of truth is
  scripts/seed-wechat-accounts.py (WECHAT_PLUGIN_SPEC=@2.4.3) and
  Dockerfile.base, both of which install a concrete pinned version.
- Also assert plugins.entries.openclaw-weixin.enabled is true, so M-W7
  covers the full set of expected post-rewrite state listed in #4237.

Test-only change; no runtime behavior touched.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches jyaunches added the R3 label May 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26656015631
Target ref: u/sdang/wechat-e2e-registry-assertions
Requested jobs: messaging-providers-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
messaging-providers-e2e ✅ success

@jyaunches jyaunches added v0.0.56 and removed v0.0.55 labels May 29, 2026
@cv
cv merged commit 697cd5b into main May 29, 2026
87 checks passed
@cv
cv deleted the u/sdang/wechat-e2e-registry-assertions branch May 29, 2026 21:41
@wscurran wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure and removed E2E labels Jun 3, 2026
@wscurran wscurran added the chore Build, CI, dependency, or tooling maintenance label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance integration: wechat WeChat integration behavior VRDC Issues and PRs submitted by NVIDIA VRDC test team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants