fix(openclaw): reapply WeChat channel seed after config mutations#3839
Conversation
ericksoa
added
v0.0.46
|
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 (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughSeed script now ensures ChangesOpenClaw WeChat Channel Seeding Build Order
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Selective E2E Results — ✅ All requested jobs passedRun: 26131352906
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/seed-wechat-accounts.test.ts (1)
262-262: ⚡ Quick winMake the channel-membership assertion order-insensitive.
This assertion is a bit brittle: channel key order can change without changing behavior. Assert presence instead of exact insertion order.
Suggested change
- expect(Object.keys(cfg.channels)).toEqual(["telegram", "slack", "openclaw-weixin"]); + expect(Object.keys(cfg.channels).sort()).toEqual( + ["telegram", "slack", "openclaw-weixin"].sort(), + );🤖 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/seed-wechat-accounts.test.ts` at line 262, The test currently asserts exact key order with expect(Object.keys(cfg.channels)).toEqual([...]); change it to an order-insensitive presence check: assert that Object.keys(cfg.channels) contains the three channel names and that the length matches, e.g. replace the equality assertion on cfg.channels keys with expect(Object.keys(cfg.channels)).toEqual(expect.arrayContaining(["telegram","slack","openclaw-weixin"])) plus a length assertion like expect(Object.keys(cfg.channels)).toHaveLength(3) so the test no longer depends on key order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/seed-wechat-accounts.test.ts`:
- Line 262: The test currently asserts exact key order with
expect(Object.keys(cfg.channels)).toEqual([...]); change it to an
order-insensitive presence check: assert that Object.keys(cfg.channels) contains
the three channel names and that the length matches, e.g. replace the equality
assertion on cfg.channels keys with
expect(Object.keys(cfg.channels)).toEqual(expect.arrayContaining(["telegram","slack","openclaw-weixin"]))
plus a length assertion like expect(Object.keys(cfg.channels)).toHaveLength(3)
so the test no longer depends on key order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2650b0c1-fe2c-48f8-a6e5-235456b838a6
📒 Files selected for processing (1)
test/seed-wechat-accounts.test.ts
Selective E2E Results — ✅ All requested jobs passedRun: 26131398241
|
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Merge gate verdict: ready.
- CI: 27 checks green
- Conflicts: clean (
mergeable: MERGEABLE;BLOCKEDhere just means awaiting approval, notDIRTY) - CodeRabbit: no unresolved major/critical findings
- Risky-code-tested: new test in
test/seed-wechat-accounts.test.tsreproduces the failingchannels-stop-start-e2escenario faithfully
Fix restores the seed invocation dropped by #3682, matching the intent already documented in scripts/generate-openclaw-config.py:539 ("The block is written AFTER 'openclaw plugins install' runs, by scripts/seed-wechat-accounts.py").
Non-blocking nit: the PR body promises a "Dockerfile ordering regression guard," but the diff only adds a script-level test. Either drop that line from the body or add a short string-assertion in test/sandbox-provisioning.test.ts that the new seed RUN token appears after openclaw plugins install /opt/nemoclaw and before the gateway-token sanitization block.
Selective E2E Results — ❌ Some jobs failedRun: 26131873648
|
…hannel-seed-after-doctor Signed-off-by: Aaron Erickson <aerickson@nvidia.com> # Conflicts: # Dockerfile
…hannel-seed-after-doctor # Conflicts: # Dockerfile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
415-418: Run the Dockerfile E2E matrix for this layer-order change.Since Line 415–Line 418 changes plugin install/enable/inspect and late seeding in one build layer, please run
cloud-e2e,sandbox-survival-e2e,hermes-e2e, andrebuild-openclaw-e2eon this branch before merge.As per coding guidelines, "Dockerfile ... Layer ordering, permissions, and baked config changes are only testable with a real container build" and the listed "E2E test recommendation" jobs should be run.
🤖 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 `@Dockerfile` around lines 415 - 418, The Dockerfile change bundles openclaw plugin install/enable/inspect and the python seeding step into one build layer (the RUN block invoking "openclaw plugins install /opt/nemoclaw", "openclaw plugins enable nemoclaw", "openclaw plugins inspect nemoclaw --json", and "python3 /usr/local/lib/nemoclaw/seed-wechat-accounts.py"), so before merging run the full container-based E2E matrix to validate layer ordering/permissions/config: execute cloud-e2e, sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e against this branch and confirm no failures related to the plugin installation, inspection, or seeding steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Dockerfile`:
- Around line 415-418: The Dockerfile change bundles openclaw plugin
install/enable/inspect and the python seeding step into one build layer (the RUN
block invoking "openclaw plugins install /opt/nemoclaw", "openclaw plugins
enable nemoclaw", "openclaw plugins inspect nemoclaw --json", and "python3
/usr/local/lib/nemoclaw/seed-wechat-accounts.py"), so before merging run the
full container-based E2E matrix to validate layer ordering/permissions/config:
execute cloud-e2e, sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e
against this branch and confirm no failures related to the plugin installation,
inspection, or seeding steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a1ed534-b2c5-4202-8621-48f7baa6885d
📒 Files selected for processing (1)
Dockerfile
…-after-doctor' into fix/wechat-openclaw-channel-seed-after-doctor
Selective E2E Results — ❌ Some jobs failedRun: 26133709555
|
Selective E2E Results — ✅ All requested jobs passedRun: 26134561802
|
Selective E2E Results — ❌ Some jobs failedRun: 26134651312
|
3 participants
Summary
seed-wechat-accounts.pyafter OpenClaw doctor/plugin-install has had a chance to rewriteopenclaw.json.plugins.installs.openclaw-weixinandplugins.entries.openclaw-weixin.enabledbefore writingchannels.openclaw-weixin.Diagnosis
The
v0.0.46candidate nightly failed only inchannels-stop-start-e2eon OpenClaw WeChat. The failing run hadNEMOCLAW_WECHAT_CONFIG_B64in the Docker build args and provider/registry state contained WeChat, but final/sandbox/.openclaw/openclaw.jsonlisted onlydefaults,discord,slack,telegram, andwhatsapp. That meant WeChat selection and credential registration worked, but the OpenClaw channel registration was lost before the final image/config hash.A first late-seed attempt fixed that channels assertion, but
messaging-providers-e2ethen exposed the remaining mismatch: the gateway rejectedchannels.openclaw-weixinas an unknown channel id. The refined fix restores the plugin install/entry metadata before adding the channel block, so the gateway knows the upstream WeChat channel id when validating config.Nightly evidence: https://github.com/NVIDIA/NemoClaw/actions/runs/26129059025/job/76849779069
Advisor evidence from the first PR attempt: https://github.com/NVIDIA/NemoClaw/actions/runs/26131873648
Verification
npx vitest run test/seed-wechat-accounts.test.ts test/generate-openclaw-config.test.ts --testTimeout 60000npx vitest run test/sandbox-provisioning.test.ts --testTimeout 60000npm run build:cligit diff --checkNote: local commit/push hooks were interrupted because their broad CLI coverage gate launched unrelated sandbox-style tests; the focused release-fix verification above passed.
Summary by CodeRabbit
Tests
Bug Fixes
Chores
Documentation