fix(discord): route gateway through sandbox proxy#3935
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
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)
📝 WalkthroughWalkthroughGenerator now sets ChangesDiscord Proxy Routing Configuration
Sequence Diagram(s)sequenceDiagram
participant ConfigGenerator as generate-openclaw-config.py
participant Sandbox as /sandbox/.openclaw/openclaw.json
participant TestRunner as test-messaging-providers.sh
participant FakeClient as fake-gateway Node client
ConfigGenerator->>Sandbox: write channels.discord.accounts.*.proxy
TestRunner->>Sandbox: read discord account proxy (M9b)
alt proxy present
TestRunner->>FakeClient: start with proxy_url (M13d-config)
FakeClient->>DiscordGateway: connect via proxy (proxyUrl)
DiscordGateway-->>FakeClient: UPGRADE / HELLO / READY / HEARTBEAT_ACK
FakeClient-->>TestRunner: transcript contains expected markers
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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 |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: Review used trusted deterministic context, linked issue text/comments, GraphQL/status data, drift/overlap evidence, and the provided read-only diff; no scripts, tests, package-manager commands, workflow dispatches, or networked validation were executed by this advisor.; PR title/body/comments/issue text were treated as untrusted evidence and mapped only to diff/test/CI evidence where available.; Required messaging-providers-e2e passed for earlier SHA 9726cb2, but not for current head 3673bdc.; Runtime OpenClaw behavior for the Discord Full advisor summaryPR Review AdvisorBase: The Discord proxy wiring change is focused and has useful unit/E2E-script coverage, but merge is blocked because CI/mergeability are not green and required messaging-providers-e2e has not passed for the current head SHA 3673bdc. Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
Selective E2E Results — ✅ All requested jobs passedRun: 26193019896
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26194160470
|
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/test-messaging-providers.sh`:
- Around line 1459-1467: The script dereferences dc_proxy while running under
set -u which can cause an exit if dc_proxy is unset; update the checks and the
function call to use a safe default expansion. Replace occurrences of -n
"$dc_proxy" and -z "$dc_proxy" with -n "${dc_proxy:-}" and -z "${dc_proxy:-}",
and pass "${dc_proxy:-}" into run_fake_discord_gateway_node_client (and any
other direct uses) so the condition and call do not nounset-fail when dc_proxy
is not defined; keep the existing logic with fake_gateway_ready and
run_fake_discord_gateway_node_client unchanged.
🪄 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: d7abd9ef-c427-4e76-9f51-8a5d008a8cf6
📒 Files selected for processing (2)
test/e2e/lib/discord-gateway-proof.shtest/e2e/test-messaging-providers.sh
Selective E2E Results — ✅ All requested jobs passedRun: 26194905795
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26195549058
|
Selective E2E Results — ❌ Some jobs failedRun: 26195713286
|
Selective E2E Results — ✅ All requested jobs passedRun: 26195749695
|
Selective E2E Results — ✅ All requested jobs passedRun: 26195768070
|
Selective E2E Results — ✅ All requested jobs passedRun: 26196860706
|
Summary
Fixes #3894
Verification
python3 -m py_compile scripts/generate-openclaw-config.pygit diff --checknpx vitest run test/generate-openclaw-config.test.ts(76 tests passed)Notes
Summary by CodeRabbit
Bug Fixes
Tests