fix(openclaw): prefer OpenShell loopback proxy#4005
Open
ericksoa wants to merge 4 commits into
Open
Conversation
Contributor
|
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 (5)
📝 WalkthroughWalkthroughAdds OPENSHELL_LOOPBACK_PROXY_URL (validated/normalized loopback HTTP URL), prefers it when generating Discord proxy entries, skips launching NemoClaw’s Discord loopback helper when OpenShell provides a matching reachable proxy, and updates tests and an inline Dockerfile comment. ChangesDiscord OpenShell loopback proxy
Sequence Diagram(s)sequenceDiagram
participant OpenShell
participant StartScript as start_discord_loopback_proxy
participant OpenClaw as openclaw.json
participant Probe as loopback_probe
participant NemoClawHelper
OpenShell->>StartScript: provide OPENSHELL_LOOPBACK_PROXY_URL
StartScript->>OpenClaw: read discord_proxy_url
StartScript->>Probe: probe OPENSHELL_LOOPBACK_PROXY_URL reachability
Probe-->>StartScript: reachable / not reachable
StartScript->>StartScript: canonicalize & compare OPENSHELL URL vs discord_proxy_url
alt match & reachable
StartScript-->>NemoClawHelper: skip launch ("Discord loopback proxy provided by OpenShell")
else mismatch or unreachable
StartScript->>NemoClawHelper: start helper (fallback)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/generate-openclaw-config.py`:
- Around line 98-109: The function _valid_loopback_http_proxy_url currently
returns the original raw input which allows schemeless values like
"127.0.0.1:45211" to pass validation but be emitted without "http://"; change it
to return a canonical normalized URL string instead: after parsing with
_normalize_url_for_parse and confirming parsed.scheme, parsed.hostname and
loopback, construct and return a normalized URL using parsed.scheme (default to
"http" if missing), parsed.hostname, and parsed.port (include port only if
present) so callers always receive a full "http://host[:port]" string; update
the return path that currently returns value to return this constructed
canonical URL.
In `@scripts/nemoclaw-start.sh`:
- Around line 1467-1471: The IPv6 loopback case in
is_openshell_loopback_proxy_url uses a case-pattern with unescaped brackets
(http://[::1]:*) which does not match literal URLs like http://[::1]:45211;
update the function to match IPv6 loopback correctly by either escaping the
literal brackets in the case pattern (so the pattern matches the literal '[' and
']') or replace that arm with a POSIX-compatible regex test (e.g., use grep -E
or bash =~) that matches ^http://\[::1\]:[0-9]+$; if you must keep the case form
and ShellCheck SC2102 still flags it, add a targeted suppression for that arm or
switch to the regex approach to avoid the ShellCheck warning.
🪄 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: 3ae6fd46-4fe5-46c9-ba61-ef4f906e5bb8
📒 Files selected for processing (6)
Dockerfilescripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/e2e/test-messaging-providers.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
Contributor
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: Review used supplied deterministic context and the provided diff; no PR scripts, package-manager commands, Docker builds, or tests were executed.; No passing Full advisor summaryPR Review AdvisorBase: Blocked on hard gates: required CI context Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26239803330
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/nemoclaw-start.sh`:
- Around line 1534-1541: The current conditional misattributes causes when
configured_discord_proxy equals OPENSHELL_LOOPBACK_PROXY_URL but
openshell_loopback_proxy_is_reachable returns false to log a "does not match"
message; update the branching around configured_discord_proxy,
OPENSHELL_LOOPBACK_PROXY_URL and openshell_loopback_proxy_is_reachable so there
are three distinct outcomes: (1) equal AND reachable -> skip helper (existing
branch), (2) equal BUT NOT reachable -> log that the URL matches config but the
OpenShell loopback probe failed (use symbols configured_discord_proxy,
OPENSHELL_LOOPBACK_PROXY_URL, openshell_loopback_proxy_is_reachable), and (3)
not equal -> keep the current "does not match" or "could not be verified"
messages for non-empty or empty config respectively.
In `@test/e2e/test-messaging-providers.sh`:
- Around line 934-935: The test currently assigns expected_dc_proxy from
OPENSHELL_LOOPBACK_PROXY_URL without reusing the generator's loopback
validation, so supply the same validation used by build_config(): parse
OPENSHELL_LOOPBACK_PROXY_URL and only use it if it is a valid loopback HTTP URL
(scheme http, host is localhost/127.0.0.1/::1 or an explicit loopback IP, and
optionally includes a port); otherwise fall back to default_dc_proxy. Update the
expected_dc_proxy assignment to perform that validation (mirroring
build_config()'s logic) before choosing OPENSHELL_LOOPBACK_PROXY_URL,
referencing the variables default_dc_proxy and expected_dc_proxy and the
generator function build_config() as the source of truth for the validation
rules.
🪄 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: 8a79762f-b03d-4c13-80dd-0e5aa53ecc24
📒 Files selected for processing (5)
scripts/generate-openclaw-config.pyscripts/nemoclaw-start.shtest/e2e/test-messaging-providers.shtest/generate-openclaw-config.test.tstest/nemoclaw-start.test.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26241223951
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26241739522
|
11 tasks
Contributor
Author
|
Follow-up for the CodeRabbit findings is pushed in What changed:
Validation run:
CI, CodeRabbit, and the E2E advisor are now re-running on the new head. |
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26246026845
|
This was referenced May 21, 2026
wscurran
added
enhancement: integration
Contributor
|
✨ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OPENSHELL_LOOPBACK_PROXY_URLfor OpenClaw Discord config when OpenShell provides a valid loopback HTTP proxy URL.NEMOCLAW_PROXY_PORTunlessNEMOCLAW_DISCORD_PROXY_PORTis explicitly set, so fallback config and helper startup stay aligned.Dependency
Methodology
OPENSHELL_LOOPBACK_PROXY_URL, and NemoClaw uses that value only when it is a valid loopback HTTP URL.Findings Incorporated
NEMOCLAW_PROXY_PORT, and the helper uses the same default unlessNEMOCLAW_DISCORD_PROXY_PORTis explicitly set.Combined Validation
18080, dashboard port18889, unique Docker network, and unique OpenShell Docker sandbox namespace.083c0663187a3e93e60cd4d32b30053475cb0890,openshell 0.0.47-dev.3+gb75abad.dd022b578806aa3880b33ac2c3fe6a86f230ff33.openshell-sandboxbuilt from the OpenShell PR and mounted through[openshell.drivers.docker].supervisor_bin.NEMOCLAW_OPENSHELL_CHANNEL=dev, the existing NemoClaw dev-channel allowance for OpenShell dev builds above the current blueprint max.What Is Proven
M9bproved generatedopenclaw.jsonbakeshttp://127.0.0.1:3128into the Discord account proxy.M13d-configproved the proxy URL read fromopenclaw.jsonreaches a fake Discord Gateway through OpenShell.M13d/M13eproved the native WebSocket upgrade, HELLO, placeholder IDENTIFY, READY, and heartbeat ACK completed through the OpenShell path.M13fproved the fake Gateway received the host-side Discord token while the sandbox-visibleIDENTIFYused only the placeholder.M13gproved an unregistered WebSocket credential placeholder is rejected before upstream token exposure.Remaining E2E Caveat
S1, an existing Slack guard probe that hardcodes internal port18789. This isolated run intentionally usedNEMOCLAW_DASHBOARD_PORT=18889to avoid a local port collision, so that check probed the wrong port.S2still passed and showed the Slack guard caught the fake-tokeninvalid_authpath without crashing the gateway. TheS1failure is outside the Discord loopback proxy and WebSocket credential-rewrite path proven above.Test Plan
git diff --checkshellcheck scripts/nemoclaw-start.sh test/e2e/test-messaging-providers.shnpm run source-shape:checknpm test -- --run test/generate-openclaw-config.test.ts test/nemoclaw-start.test.ts --testTimeout 20000M9b,M13d-config,M13d,M13f,M13gpassed.Summary by CodeRabbit