Skip to content

feat: add settings for daemon host and port in browser extension#636

Open
zarkin404 wants to merge 4 commits intojackwener:mainfrom
zarkin404:feat/customize-endpoints
Open

feat: add settings for daemon host and port in browser extension#636
zarkin404 wants to merge 4 commits intojackwener:mainfrom
zarkin404:feat/customize-endpoints

Conversation

@zarkin404
Copy link
Copy Markdown

Description

  • 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.

Related issue:

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • [] I updated tests or docs if needed
  • [] I included output or screenshots when useful

Documentation (if adding/modifying an adapter)

  • Added doc page under docs/adapters/ (if new adapter)
  • Updated docs/adapters/index.md table (if new adapter)
  • Updated sidebar in docs/.vitepress/config.mts (if new adapter)
  • Updated README.md / README.zh-CN.md when command discoverability changed
  • Used positional args for the command's primary subject unless a named flag is clearly better
  • Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

PR 目的

有些场合下,浏览器和 OpenCli 的运行环境并不一定在同一台主机上,譬如 OpenCli 被远程服务器上的 OpenClaw 启动,而自己的电脑则运行这安装了插件的浏览器,所以需要浏览器连到远程服务器上的 OpenCli Daemon,远程服务器可以通过 frpc 将 Daemon 的监听端口暴露到公网,以便于插件链接,进而规避目标平台的 IP 检测或者其他风控检测。

附 frpc.toml 配置:

[[proxies]]
name = "opencli"
type = "tcp"
localPort = 19825
remotePort = 23333

浏览器插件配置:
Daemon host: 服务器 ip
Daemon port: 23333 (记得防火墙和安全组要放行该端口)

@zarkin404 zarkin404 changed the title feat: add settings for daemon host and port in popup feat: add settings for daemon host and port in browser extension Mar 31, 2026
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful feature for remote daemon setups. A few issues I noticed:

  1. Duplicate functions in dist/background.jssetFileInputFiles and handleSetFileInput each appear twice in the diff. Looks like a merge artifact. This will cause runtime issues.

  2. 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.

  3. bind-current action removed from handleCommand switch — the original code handles case "bind-current" but the new version drops it. This is a regression.

  4. Version bump — package-lock.json 1.5.4→1.5.5 should be left to the maintainer.

  5. 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).

@zarkin404 zarkin404 force-pushed the feat/customize-endpoints branch from f6ecf39 to 0c9d14a Compare April 2, 2026 09:00
- 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
@zarkin404 zarkin404 force-pushed the feat/customize-endpoints branch from 0c9d14a to 32248c0 Compare April 2, 2026 09:04
@zarkin404 zarkin404 requested a review from Astro-Han April 2, 2026 09:05
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. dist/background.js — still appears to be hand-edited rather than rebuilt from src. Safer to cd extension && npm run build and commit the output.

  2. 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):

  1. When settings change, storage.onChanged closes the old WS and immediately calls connect() which creates a new one. But the onopen/onclose/onerror callbacks all operate on the global ws variable without checking whether the socket that fired the event is still the active one. If the old connection's onclose fires after the new connection is created, it will null out the new ws and 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:

  1. The host input field accepts values like http://1.2.3.4 or example.com:19825, which get concatenated into malformed URLs like http://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
@zarkin404
Copy link
Copy Markdown
Author

已处理这轮 review 提到的 4 个点:

  1. 修复了 storage.onChanged 触发重连时的 WebSocket race condition。现在所有 onopen / onmessage / onclose / onerror 回调都会先校验触发事件的 socket 是否仍然是当前活动连接,避免旧连接的延迟回调清掉新状态。
  2. 补了 daemon host 输入校验,popup 现在会拒绝带 scheme / port 的值,并提示 Enter hostname or IP only, no scheme or port.。同时也加了 host 规范化逻辑,能兼容历史上已经保存的 http://1.2.3.4、example.com:19825 这类值,避免继续拼出 malformed URL。
  3. 在 popup 和文档里补充了远程暴露风险提示,明确说明如果通过 frpc / 公网 tunnel / 非本地网络路径连接 daemon,需要额外加 VPN、SSH tunnel 或其他带认证的保护层,因为 daemon 本身只有最小鉴权。
  4. extension/dist/background.js 已重新通过 cd extension && npm run build 生成,不是手工修改产物。

另外补了测试覆盖:

  1. 新增了 host 规范化测试
  2. 新增了 stale websocket close 事件不会污染新连接状态的测试

@zarkin404 zarkin404 requested a review from Astro-Han April 2, 2026 09:47
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 4 items from round 2 all look addressed in b201f4ea:

  1. ✅ Stale socket guard (if (ws !== socket) return) on all callbacks
  2. ✅ Host validation + normalization
  3. ✅ Security warning in popup and docs
  4. ✅ 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:

  1. Call A enters connect() with old settings, blocks on await fetch(oldPingUrl)
  2. User changes settings → storage.onChanged fires → Call B runs connect(), creates new socket with the new address
  3. Call A's fetch finally returns. A doesn't check whether it's been superseded, proceeds to new WebSocket(oldWsUrl), and overwrites the global ws

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 waiting

Minor (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
@zarkin404 zarkin404 requested a review from Astro-Han April 2, 2026 13:22
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the connectAttemptId generation guard and reconnectTimer cleanup both look good. All issues from previous rounds are resolved.

@zarkin404 zarkin404 requested a review from Astro-Han April 2, 2026 14:06
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants