feat(components): support remote RPC server connections via full URL configuration#2350
feat(components): support remote RPC server connections via full URL configuration#2350dividedmind wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for configuring RPC connections using full URLs (including remote hosts and HTTPS→WSS WebSocket upgrades) while preserving existing port-based configuration behavior across the components package and CLI UI entrypoint.
Changes:
- Introduces
RPCUrlUtils.parseRpcUrlto normalize port/URL inputs into HTTP + WS endpoints. - Updates RPC client wiring (
AppMapRPC,RPC,ChatSearch,ReviewBackend) to acceptrpcUrl/URL strings in addition to ports. - Adds/updates unit tests covering URL parsing and constructor behavior; updates CLI
navie.htmlbootstrap to acceptrpcUrlquery param.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/tests/unit/lib/RPCUrlUtils.spec.js | Adds unit tests for URL/port parsing behavior and edge cases. |
| packages/components/tests/unit/chat/AppMapRPC.spec.js | Extends constructor tests for URL inputs and backward compatibility. |
| packages/components/src/pages/ChatSearch.vue | Adds appmapRpcUrl prop and uses it when constructing AppMapRPC. |
| packages/components/src/lib/ReviewBackend.ts | Makes RPC configuration accept rpcUrl as an alternative to rpcPort. |
| packages/components/src/lib/RPCUrlUtils.ts | New utility to normalize RPC configuration into HTTP + WS URLs. |
| packages/components/src/lib/RPC.ts | Updates browser RPC client to fetch() against a normalized HTTP URL instead of hardcoded localhost. |
| packages/components/src/lib/AppMapRPC.ts | Extends constructor to accept URL strings; updates WebSocket subscribe to use derived WS URL. |
| packages/cli/src/html/navie.js | Accepts rpcUrl query parameter and maps it to the ChatSearch prop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hleb-rubanau
left a comment
There was a problem hiding this comment.
In general, it's very well instrumented with great test coverage.
However, some Copilot nitpicks may still worth attention, plus please consider my comment regarding default scheme (which is optional suggestion, for your decision).
Once these comments/discussions would be processed, I won't see any obstacle for approval. :)
hleb-rubanau
left a comment
There was a problem hiding this comment.
The only change I request is adding a small remark somewhere in the docs, warning users that specifying remote server without protocol schema is intended for local setups in secure environments and will be resolved to http (not https) by default.
It does not affect functionality and may never be needed in practice.
However, I believe this small disclaimer could prevent big surprises in the future -- both for company and customers.
5365d0c to
ee8f313
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…configuration Add ability to connect to remote RPC servers by accepting full URLs (e.g., 'http://remote:8080', 'https://api.example.com/rpc') in addition to port numbers. This removes the hardcoded localhost limitation while maintaining full backward compatibility with existing port-based configuration. Changes: - Add RPCUrlUtils module for parsing URLs and port numbers - Update AppMapRPC, RPC, ChatSearch, ReviewBackend, and CLI to accept URLs - Convert HTTPS → WSS automatically for secure WebSocket connections - Add comprehensive unit and integration tests - Preserve backward compatibility with existing port-only usage New usage examples: new AppMapRPC('http://remote-server:8080') new AppMapRPC('https://api.example.com/rpc') <v-chat-search :appmap-rpc-url="'http://remote:8080'" /> Backward compatible: new AppMapRPC(3000) // Still works as before <v-chat-search :appmap-rpc-port="3000" /> // Still works
ee8f313 to
ecda556
Compare
Add ability to connect to remote RPC servers by accepting full URLs (e.g.,
'http://remote:8080','https://api.example.com/rpc') in addition to port numbers. This removes the hardcoded localhost limitation while maintaining full backward compatibility with existing port-based configuration. Allowing specifying full URL will make it possible to connect to RPC servers on other hosts; this scenario can occur eg. in the context of remote (eg. SSH, WSL, devcontainer) VSCode development.Changes:
New usage examples:
Backward compatible: