Skip to content

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

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

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.

@jackwener
Copy link
Copy Markdown
Owner

Hi @zarkin404, thank you for this PR and the very clear use-case writeup — the frpc workflow you described is genuinely useful, and the implementation work here (race-protected reconnect, host normalization, storage-onChanged reload, tests, security warnings) is solid.

After reviewing the design with the team, we've decided not to merge this approach. The reasoning, briefly:

The daemon's WebSocket protocol currently has no built-in authentication. With this PR, an extension popup field is enough to expose that protocol over a public network — anything that can reach the port can read cookies from any logged-in site, execute arbitrary JavaScript in any tab, take screenshots, and so on. Even with the warning text, this asks every user to make a security call we don't think we should put behind a popup field.

The thing is, your stated goal — remote orchestration with the local browser session — doesn't actually need an extension change. The daemon and extension can stay on localhost, and opencli running on a remote machine can reach your local daemon via a reverse tunnel:

# From your local machine
ssh -R 19825:127.0.0.1:19825 user@remote-server

While that SSH session is open, the remote opencli connects to its own localhost:19825 and the traffic is forwarded back to your laptop. Same outcome (remote orchestration + local browser + your home IP), zero code changes, daemon never leaves localhost, SSH provides authenticated transport. frp works analogously when SSH isn't an option.

We've documented this pattern in #1337 (docs/guide/remote-orchestration.md) along with the rationale for not exposing the daemon directly. If/when the daemon protocol gains authentication, native extension-side remote support is something we'd revisit then — at that point the design space changes meaningfully.

Closing this PR with that in mind. Two follow-ups where your help would be very welcome:

  1. You're the right author for the frp section of the new doc — you've actually run that flow end-to-end. If you'd be open to a follow-up PR refining the frp instructions in docs/guide/remote-orchestration.md once docs(guide): add remote-orchestration page (SSH/frpc reverse tunnel) #1337 lands (especially the two-end frpc setup so the remote sees daemon as localhost), that would be excellent.
  2. A Chinese translation at docs/zh/guide/remote-orchestration.md would also be valuable — your original PR description was a great explainer of the use case.

Thank you again for the contribution and for the clear writeup. Sorry for the long path to this conclusion.

@jackwener
Copy link
Copy Markdown
Owner

Closing per #1337 / review discussion. Thanks again @zarkin404.

@jackwener jackwener closed this May 5, 2026
jackwener added a commit that referenced this pull request May 5, 2026
#1337)

Document the pattern for running opencli on a remote machine while keeping
the daemon and Chrome on the local machine. Reverse-tunnel local 19825
back to the remote (via SSH -R or frp) so the remote opencli still talks
to its own loopback and the daemon never leaves localhost.

Captures the rationale we landed on after reviewing #636: native
extension-to-remote-daemon support is deferred until the daemon protocol
gains authentication; in the meantime this is the safe, zero-code path
that achieves the same outcome.
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.

3 participants