feat: add settings for daemon host and port in browser extension#636
feat: add settings for daemon host and port in browser extension#636zarkin404 wants to merge 4 commits intojackwener:mainfrom
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Useful feature for remote daemon setups. A few issues I noticed:
-
Duplicate functions in dist/background.js —
setFileInputFilesandhandleSetFileInputeach appear twice in the diff. Looks like a merge artifact. This will cause runtime issues. -
dist/ should be rebuilt, not hand-edited — the dist changes mix functional changes with formatting (var→const/let, indentation). Better to just rebuild from src and commit the output.
-
bind-currentaction removed from handleCommand switch — the original code handlescase "bind-current"but the new version drops it. This is a regression. -
Version bump — package-lock.json 1.5.4→1.5.5 should be left to the maintainer.
-
Security note for the use case — the PR describes exposing the daemon port to the public internet via frpc. The daemon currently has minimal auth (X-OpenCLI header only). Might be worth adding a warning in the popup UI or docs that remote exposure requires additional protection (e.g. VPN, SSH tunnel, or waiting for token auth to land).
f6ecf39 to
0c9d14a
Compare
- Introduced a settings section in the popup to allow users to configure the daemon host and port. - Implemented storage functionality to save and retrieve these settings. - Updated connection logic to use the configured host and port for WebSocket connections. - Enhanced the background script to handle changes in the stored settings and reconnect accordingly. - Bumped version to 1.5.5 to reflect these changes. Made-with: Cursor
0c9d14a to
32248c0
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the update — the duplicate functions, bind-current regression, and version bump from the first round are all resolved now. Two issues from round 1 remain, plus a new one found on closer inspection:
Still open from round 1:
-
dist/background.js — still appears to be hand-edited rather than rebuilt from src. Safer to
cd extension && npm run buildand commit the output. -
Security note for remote exposure — the frpc use case in the PR description exposes the daemon port to the public internet. Worth adding a brief warning in the popup or docs that this requires additional protection (VPN, SSH tunnel, etc.) since the daemon has minimal auth.
New — WebSocket race condition (critical):
- When settings change,
storage.onChangedcloses the old WS and immediately callsconnect()which creates a new one. But theonopen/onclose/onerrorcallbacks all operate on the globalwsvariable without checking whether the socket that fired the event is still the active one. If the old connection'sonclosefires after the new connection is created, it will null out the newwsand reset state. Fix: capture the socket in a local variable and guard each callback:
const socket = new WebSocket(wsUrl);
ws = socket;
socket.onclose = () => {
if (ws !== socket) return; // stale socket, ignore
ws = null;
// ...
};Minor — host input validation:
- The host input field accepts values like
http://1.2.3.4orexample.com:19825, which get concatenated into malformed URLs likehttp://http://1.2.3.4:19825/ping. Could either strip scheme/port on save, or validate and show an error ("enter hostname or IP only, no scheme or port").
Guard websocket callbacks against stale sockets, validate and normalize daemon host input, and document the risks of exposing the daemon remotely. Made-with: Cursor
|
已处理这轮 review 提到的 4 个点:
另外补了测试覆盖:
|
Astro-Han
left a comment
There was a problem hiding this comment.
The 4 items from round 2 all look addressed in b201f4ea:
- ✅ Stale socket guard (
if (ws !== socket) return) on all callbacks - ✅ Host validation + normalization
- ✅ Security warning in popup and docs
- ✅ dist rebuilt from src
Tests for host normalization and stale onclose are solid additions.
One remaining race condition (critical):
The stale-socket guard covers callbacks after a WebSocket is created, but there's a window between await fetch(pingUrl) and new WebSocket(wsUrl) where a concurrent connect() call can get stomped:
- Call A enters
connect()with old settings, blocks onawait fetch(oldPingUrl) - User changes settings →
storage.onChangedfires → Call B runsconnect(), creates new socket with the new address - Call A's fetch finally returns. A doesn't check whether it's been superseded, proceeds to
new WebSocket(oldWsUrl), and overwrites the globalws
Result: the extension silently reconnects to the old daemon; the user's newly saved settings appear to have no effect. The existing stale-onclose test doesn't catch this because it only covers callback-phase races, not in-flight-connect races.
Suggested fix — generation counter:
let connectGeneration = 0;
// in storage.onChanged handler, before void connect():
connectGeneration++;
// in connect(), capture generation at entry:
const gen = connectGeneration;
// ... after await fetch() returns ...
if (gen !== connectGeneration) return; // settings changed while we were waitingMinor (non-blocking):
storage.onChanged handler resets reconnectAttempts but doesn't clear reconnectTimer. If a reconnect was already scheduled it'll still fire after the settings-triggered connect(). Harmless due to guards in connect(), but adding clearTimeout/null before void connect() would be cleaner.
Use the socket's own url and readyState instead of duplicating active and pending URL state, while keeping the in-flight connect guard and reconnect timer cleanup. Made-with: Cursor
Astro-Han
left a comment
There was a problem hiding this comment.
LGTM — the connectAttemptId generation guard and reconnectTimer cleanup both look good. All issues from previous rounds are resolved.
Description
Related issue:
Type of Change
Checklist
Documentation (if adding/modifying an adapter)
docs/adapters/(if new adapter)docs/adapters/index.mdtable (if new adapter)docs/.vitepress/config.mts(if new adapter)README.md/README.zh-CN.mdwhen command discoverability changedCliErrorsubclasses instead of rawErrorScreenshots / Output
PR 目的
有些场合下,浏览器和 OpenCli 的运行环境并不一定在同一台主机上,譬如 OpenCli 被远程服务器上的 OpenClaw 启动,而自己的电脑则运行这安装了插件的浏览器,所以需要浏览器连到远程服务器上的 OpenCli Daemon,远程服务器可以通过 frpc 将 Daemon 的监听端口暴露到公网,以便于插件链接,进而规避目标平台的 IP 检测或者其他风控检测。
附 frpc.toml 配置:
浏览器插件配置:
Daemon host: 服务器 ip
Daemon port: 23333 (记得防火墙和安全组要放行该端口)