From a9c7444139e39ca3344da1e9fdf15723f8eb06d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Chaonan=E2=80=9D?= Date: Wed, 2 Sep 2026 18:56:08 +0800 Subject: [PATCH 1/6] feat(extension): make the local daemon port configurable in the popup The extension was hardwired to 52800; persist a custom loopback port and reconnect when it changes so users can match a non-default daemon. Co-authored-by: Cursor --- apps/extension/PRIVACY.md | 4 +- apps/extension/PRIVACY.zh-CN.md | 4 +- apps/extension/src/entrypoints/background.ts | 31 +++- .../src/entrypoints/popup/App.test.tsx | 155 +++++++++++++++++- apps/extension/src/entrypoints/popup/App.tsx | 79 ++++++++- .../entrypoints/popup/use-connection-state.ts | 9 +- .../src/entrypoints/popup/use-daemon-port.ts | 71 ++++++++ .../src/lib/__tests__/instance-id.test.ts | 20 +++ apps/extension/src/lib/instance-id.ts | 17 ++ apps/extension/src/lib/popup-bridge.ts | 9 +- .../__tests__/daemon-endpoint.test.ts | 43 +++++ .../transport/__tests__/ws-transport.test.ts | 15 ++ .../src/transport/daemon-endpoint.ts | 39 +++++ apps/extension/src/transport/ws-transport.ts | 9 +- docs/architecture.md | 2 +- .../i18n/src/locales/en-US/extension.json | 5 + .../i18n/src/locales/zh-CN/extension.json | 5 + 17 files changed, 488 insertions(+), 29 deletions(-) create mode 100644 apps/extension/src/entrypoints/popup/use-daemon-port.ts create mode 100644 apps/extension/src/transport/__tests__/daemon-endpoint.test.ts create mode 100644 apps/extension/src/transport/daemon-endpoint.ts diff --git a/apps/extension/PRIVACY.md b/apps/extension/PRIVACY.md index bfe46754..f3b3c94d 100644 --- a/apps/extension/PRIVACY.md +++ b/apps/extension/PRIVACY.md @@ -57,7 +57,7 @@ The Extension requests the following Chrome permissions. Each is used solely for ## 6. Where Data Goes -All Extension activity stays on the user's local device. The only network traffic the Extension generates is a WebSocket connection to `ws://127.0.0.1:52800` (loopback only). What the AI agent connected to that local daemon does with the data afterwards (for example, sending a screenshot to an LLM provider) is governed by the privacy policy of that agent or LLM provider, **not** by this policy. BrowserSkill is not a party to those communications. +All Extension activity stays on the user's local device. The only network traffic the Extension generates is a WebSocket connection to the local bsk daemon on `127.0.0.1` (loopback only; default port **52800**, configurable in the extension popup). What the AI agent connected to that local daemon does with the data afterwards (for example, sending a screenshot to an LLM provider) is governed by the privacy policy of that agent or LLM provider, **not** by this policy. BrowserSkill is not a party to those communications. ## 7. Data Retention @@ -80,7 +80,7 @@ The Extension is a developer tool and is not directed at children under 13. It d ## 10. Security -Because the Extension communicates only with `127.0.0.1`, no data is exposed to the network. Users should still avoid running BrowserSkill in untrusted environments, since any local process able to bind to `127.0.0.1:52800` could send commands to the Extension. Run BrowserSkill only on machines you control. +Because the Extension communicates only with `127.0.0.1`, no data is exposed to the network. Users should still avoid running BrowserSkill in untrusted environments, since any local process able to bind to the configured loopback port could send commands to the Extension. Run BrowserSkill only on machines you control. ## 11. Open Source and Auditability diff --git a/apps/extension/PRIVACY.zh-CN.md b/apps/extension/PRIVACY.zh-CN.md index 1cfa0082..e2591c07 100644 --- a/apps/extension/PRIVACY.zh-CN.md +++ b/apps/extension/PRIVACY.zh-CN.md @@ -54,7 +54,7 @@ BrowserSkill **不会**： ## 6. 数据流向 -本扩展的所有活动都停留在用户本地设备上。本扩展产生的唯一网络流量是与 `ws://127.0.0.1:52800`（仅回环地址）的 WebSocket 连接。连接到该本地守护进程的 AI 助手在拿到数据之后如何处理（例如将截图发送给某个 LLM 服务），由该助手或 LLM 提供商自身的隐私政策约束，**不在本政策范围内**。BrowserSkill 不参与那些通信。 +本扩展的所有活动都停留在用户本地设备上。本扩展产生的唯一网络流量是与本地 bsk 守护进程在 `127.0.0.1`（仅回环地址；默认端口 **52800**，可在扩展弹窗中配置）上的 WebSocket 连接。连接到该本地守护进程的 AI 助手在拿到数据之后如何处理（例如将截图发送给某个 LLM 服务），由该助手或 LLM 提供商自身的隐私政策约束，**不在本政策范围内**。BrowserSkill 不参与那些通信。 ## 7. 数据保留 @@ -76,7 +76,7 @@ BrowserSkill **不会**： ## 10. 安全性 -由于本扩展仅与 `127.0.0.1` 通信，因此不会向网络暴露任何数据。但用户仍应避免在不可信的环境中运行 BrowserSkill —— 任何能够绑定到 `127.0.0.1:52800` 的本地进程理论上都可以向本扩展发送指令。请仅在您本人控制的机器上运行 BrowserSkill。 +由于本扩展仅与 `127.0.0.1` 通信，因此不会向网络暴露任何数据。但用户仍应避免在不可信的环境中运行 BrowserSkill —— 任何能够绑定到所配置回环端口的本地进程理论上都可以向本扩展发送指令。请仅在您本人控制的机器上运行 BrowserSkill。 ## 11. 开源与可审计性 diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 4edd0a24..36a4e06d 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -4,7 +4,9 @@ import { ConnectionController } from "@/lib/connection-controller"; import { startHeartbeat } from "@/lib/heartbeat"; import { getConnectionEnabled, + getDaemonPort, setConnectionEnabled as persistConnectionEnabled, + STORAGE_KEYS, setLabel, } from "@/lib/instance-id"; import { startKeepalive } from "@/lib/keepalive"; @@ -41,6 +43,7 @@ import { attachRecordStepListener, type RecordRuntimeDeps, } from "@/tools/record"; +import { resolveDaemonWsUrl } from "@/transport/daemon-endpoint"; import { detectBrowserMeta } from "@/transport/handshake"; import type { Transport } from "@/transport/transport"; import { WSTransport } from "@/transport/ws-transport"; @@ -54,6 +57,27 @@ export default defineBackground(() => { let overlayGeneration = 0; const controlModes = new Map(); + async function applyDaemonPort(port: number): Promise { + const url = resolveDaemonWsUrl(port); + if (!transport.setUrl(url)) return; + await transport.disconnect(); + if (!controller.isConnectionEnabled) return; + try { + await transport.connect(); + } catch (err) { + console.debug("[browser-skill] reconnect after port change failed", err); + } + } + + if (typeof chrome !== "undefined" && chrome.storage?.onChanged) { + chrome.storage.onChanged.addListener((changes, areaName) => { + if (areaName !== "local") return; + const change = changes[STORAGE_KEYS.DAEMON_PORT]; + if (!change || typeof change.newValue !== "number") return; + void applyDaemonPort(change.newValue); + }); + } + function setControlMode(sessionId: string, mode: OverlayMode): void { if (controlModes.get(sessionId) === mode) return; controlModes.set(sessionId, mode); @@ -279,6 +303,8 @@ export default defineBackground(() => { void (async () => { const connectionEnabled = await getConnectionEnabled(); + const port = await getDaemonPort(); + transport.setUrl(resolveDaemonWsUrl(port)); await controller.attach(transport, detectBrowserMeta(), connectionEnabled, { beforeDisconnect: async () => { const report = await cleanupAfterDisconnect(); @@ -345,11 +371,6 @@ export default defineBackground(() => { if (msg && typeof msg === "object" && "kind" in msg) { if (msg.kind === "set_label") { void setLabel(msg.value).then(() => controller.refreshLabel()); - } else if (msg.kind === "set_port") { - // Placeholder for the future custom-port UI; warn loudly so - // any reintroduced popup control is caught instead of - // silently doing nothing (review M4/M5 C2). - console.warn("[browser-skill] set_port is not wired yet; ignoring", msg.value); } else if (msg.kind === "set_connection_enabled") { void controller .setConnectionEnabled(msg.value) diff --git a/apps/extension/src/entrypoints/popup/App.test.tsx b/apps/extension/src/entrypoints/popup/App.test.tsx index daec6379..6fa9145c 100644 --- a/apps/extension/src/entrypoints/popup/App.test.tsx +++ b/apps/extension/src/entrypoints/popup/App.test.tsx @@ -2,6 +2,7 @@ import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/re import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { SnapshotInfo } from "@/lib/connection-controller"; import { STORAGE_KEYS } from "@/lib/instance-id"; +import { DEFAULT_DAEMON_PORT } from "@/transport/daemon-endpoint"; import { EXTENSION_VERSION } from "@/transport/handshake"; import { App } from "./App"; import { useConnectionState } from "./use-connection-state"; @@ -66,9 +67,32 @@ describe("App", () => { render(); expect(screen.getByText("未连接")).toBeTruthy(); + expect(screen.getByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeTruthy(); expect(screen.queryByText("请先打开 BrowserSkill。")).toBeNull(); }); + it("shows the connection switch off and hides transport errors when disconnected", () => { + mockUseConnectionState.mockReturnValue({ + snapshot: { + ...baseSnapshot, + lastError: "[WSTransport] disconnect during connect", + }, + statusState: "disconnected", + setLabel, + setConnectionEnabled, + }); + + render(); + + expect(screen.getByText("未连接")).toBeTruthy(); + expect(screen.getByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeTruthy(); + expect(screen.queryByText("端口不匹配")).toBeNull(); + expect( + screen.getByRole("switch", { name: "BrowserSkill 连接" }).getAttribute("aria-checked"), + ).toBe("false"); + expect(screen.queryByText("[WSTransport] disconnect during connect")).toBeNull(); + }); + it("does not render record UI on the main view", () => { render(); @@ -136,6 +160,13 @@ describe("App", () => { }); it("renders the connection toggle with switch semantics", () => { + mockUseConnectionState.mockReturnValue({ + snapshot: { ...baseSnapshot, state: "connected" }, + statusState: "connected", + setLabel, + setConnectionEnabled, + }); + render(); const toggle = screen.getByRole("switch", { name: "BrowserSkill 连接" }); @@ -143,6 +174,13 @@ describe("App", () => { }); it("calls setConnectionEnabled(false) when the toggle is turned off", () => { + mockUseConnectionState.mockReturnValue({ + snapshot: { ...baseSnapshot, state: "connected" }, + statusState: "connected", + setLabel, + setConnectionEnabled, + }); + render(); fireEvent.click(screen.getByRole("switch", { name: "BrowserSkill 连接" })); @@ -160,6 +198,7 @@ describe("App", () => { render(); expect(screen.getByText("连接已关闭")).toBeTruthy(); + expect(screen.queryByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeNull(); expect( screen.getByRole("switch", { name: "BrowserSkill 连接" }).getAttribute("aria-checked"), ).toBe("false"); @@ -354,14 +393,20 @@ describe("control hints toggle", () => { const info = await screen.findByRole("button", { name: "控制提示说明" }); expect(info).toBeTruthy(); - const tooltip = screen.getByRole("tooltip"); - expect(tooltip.textContent).toBe("Agent 控制页面时显示提示条和橙色闪光。"); + const tooltip = screen.getByText("Agent 控制页面时显示提示条和橙色闪光。"); + expect(tooltip.getAttribute("role")).toBe("tooltip"); // Hidden until the info button is hovered or focused. expect(tooltip.className).toContain("opacity-0"); }); it("uses the same switch component and size for both settings rows", async () => { stubChromeStorage(); + mockUseConnectionState.mockReturnValue({ + snapshot: { ...baseSnapshot, state: "connected" }, + statusState: "connected", + setLabel: vi.fn(), + setConnectionEnabled: vi.fn(), + }); render(); @@ -374,3 +419,109 @@ describe("control hints toggle", () => { expect(hintsToggle.className).toBe(connectionToggle.className); }); }); + +describe("daemon port input", () => { + function stubChromeStorage(initial: Record = {}) { + const store = { ...initial }; + vi.stubGlobal("chrome", { + runtime: { lastError: undefined }, + storage: { + local: { + get: (keys: string | string[], cb: (items: Record) => void) => { + const items: Record = {}; + for (const k of Array.isArray(keys) ? keys : [keys]) { + if (k in store) items[k] = store[k]; + } + cb(items); + }, + set: (items: Record, cb?: () => void) => { + Object.assign(store, items); + cb?.(); + }, + }, + onChanged: { + addListener: vi.fn(), + removeListener: vi.fn(), + }, + }, + }); + return store; + } + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it("prefills the port from storage", async () => { + stubChromeStorage({ [STORAGE_KEYS.DAEMON_PORT]: 53200 }); + + render(); + + const input = await screen.findByRole("textbox", { name: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).value).toBe("53200")); + }); + + it("persists a valid port on blur", async () => { + const store = stubChromeStorage(); + + render(); + + const input = await screen.findByRole("textbox", { name: "连接端口" }); + fireEvent.change(input, { target: { value: "53200" } }); + fireEvent.blur(input); + + expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200); + expect((input as HTMLInputElement).value).toBe("53200"); + }); + + it("persists a valid port when Enter blurs the field", async () => { + const store = stubChromeStorage(); + + render(); + + const input = await screen.findByRole("textbox", { name: "连接端口" }); + fireEvent.change(input, { target: { value: "53200" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200); + }); + + it("shows an error and does not write invalid ports", async () => { + const store = stubChromeStorage(); + + render(); + + const input = await screen.findByRole("textbox", { name: "连接端口" }); + fireEvent.change(input, { target: { value: "abc" } }); + fireEvent.blur(input); + + expect(screen.getByText("请输入 1 到 65535 之间的端口号。")).toBeTruthy(); + expect(store[STORAGE_KEYS.DAEMON_PORT]).toBeUndefined(); + }); + + it("stores the default port when the field is cleared", async () => { + const store = stubChromeStorage({ [STORAGE_KEYS.DAEMON_PORT]: 53200 }); + + render(); + + const input = await screen.findByRole("textbox", { name: "连接端口" }); + fireEvent.change(input, { target: { value: "" } }); + fireEvent.blur(input); + + expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(DEFAULT_DAEMON_PORT); + expect((input as HTMLInputElement).value).toBe(String(DEFAULT_DAEMON_PORT)); + }); + + it("keeps the port hint copy in an accessible info tooltip", async () => { + stubChromeStorage(); + + render(); + + const info = await screen.findByRole("button", { name: "连接端口说明" }); + expect(info).toBeTruthy(); + const tooltip = screen.getByText("扩展通过此端口连接本机 daemon。非必要请勿修改。"); + expect(tooltip.getAttribute("role")).toBe("tooltip"); + expect(tooltip.className).toContain("opacity-0"); + }); +}); diff --git a/apps/extension/src/entrypoints/popup/App.tsx b/apps/extension/src/entrypoints/popup/App.tsx index 0756e9cb..f773cce4 100644 --- a/apps/extension/src/entrypoints/popup/App.tsx +++ b/apps/extension/src/entrypoints/popup/App.tsx @@ -15,6 +15,7 @@ import { POPUP_FEATURES, type PopupView } from "./features"; import { Switch } from "./switch"; import { type PopupStatusState, useConnectionState } from "./use-connection-state"; import { useControlHintsHidden } from "./use-control-hints-hidden"; +import { useDaemonPort } from "./use-daemon-port"; const STATE_LABEL_KEYS = { disconnected: "popup.stateLabel.disconnected", @@ -41,6 +42,12 @@ export function App() { const { t } = useTranslation("extension"); const { snapshot, statusState, setConnectionEnabled } = useConnectionState(); const [controlHintsHidden, setControlHintsHidden] = useControlHintsHidden(); + const { + draft: daemonPortDraft, + setDraft: setDaemonPortDraft, + commit: commitDaemonPort, + invalid: daemonPortInvalid, + } = useDaemonPort(); const [view, setView] = useState("main"); const [copiedInstanceId, setCopiedInstanceId] = useState(false); const [purposeDraft, setPurposeDraft] = useState(""); @@ -79,6 +86,7 @@ export function App() { }, [copiedTick]); const isSkewed = statusState === "version_skew"; + const isDisconnected = statusState === "disconnected"; const connectionLive = statusState === "connected" || isSkewed; const daemonVersion = snapshot.handshake?.version ?? "—"; const daemonProtocol = snapshot.handshake?.protocol_version ?? "—"; @@ -198,13 +206,21 @@ export function App() { {t(STATE_BADGE_KEYS[statusState])} + {isDisconnected && ( +

+ {t("popup.daemonUnreachable")} +

+ )} {isSkewed && (

- {snapshot.lastError && ( +

+
+ + + + + + {t("popup.daemonPortHint")} + + + + ) => + setDaemonPortDraft(event.target.value) + } + onBlur={commitDaemonPort} + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + commitDaemonPort(); + } + }} + className="mt-0 h-5 w-24 shrink-0 rounded-md px-2 py-0 text-center text-sm leading-none shadow-none" + aria-invalid={daemonPortInvalid || undefined} + data-slot="popup-daemon-port-input" + /> +
+ {daemonPortInvalid && ( +

+ {t("popup.daemonPortInvalid")} +

+ )} +
+ + {snapshot.lastError && !isDisconnected && (
void; + commit: () => void; + invalid: boolean; +} { + const [draft, setDraftState] = useState(""); + const [invalid, setInvalid] = useState(false); + const draftRef = useRef(draft); + draftRef.current = draft; + + useEffect(() => { + if (typeof chrome === "undefined" || !chrome.storage?.local) return undefined; + let cancelled = false; + getDaemonPort() + .then((port) => { + if (!cancelled) setDraftState(String(port)); + }) + .catch((err) => { + console.debug("[browser-skill] daemon port read failed", err); + }); + const onChanged = (changes: Record, areaName: string) => { + if (areaName !== "local") return; + const change = changes[STORAGE_KEYS.DAEMON_PORT]; + if (change && typeof change.newValue === "number") { + setDraftState(String(change.newValue)); + setInvalid(false); + } + }; + chrome.storage.onChanged.addListener(onChanged); + return () => { + cancelled = true; + chrome.storage.onChanged.removeListener(onChanged); + }; + }, []); + + const commit = useCallback(() => { + const parsed = parseDaemonPortInput(draftRef.current); + if (parsed === null) { + setInvalid(true); + return; + } + setInvalid(false); + setDraftState(String(parsed)); + if (typeof chrome === "undefined" || !chrome.storage?.local) return; + setDaemonPort(parsed).catch((err) => { + console.debug("[browser-skill] daemon port write failed", err); + }); + }, []); + + useEffect(() => { + const onPageHide = () => commit(); + window.addEventListener("pagehide", onPageHide); + return () => window.removeEventListener("pagehide", onPageHide); + }, [commit]); + + const setDraft = useCallback((value: string) => { + setDraftState(value); + setInvalid(false); + }, []); + + return { draft, setDraft, commit, invalid }; +} diff --git a/apps/extension/src/lib/__tests__/instance-id.test.ts b/apps/extension/src/lib/__tests__/instance-id.test.ts index 39544865..4629eee9 100644 --- a/apps/extension/src/lib/__tests__/instance-id.test.ts +++ b/apps/extension/src/lib/__tests__/instance-id.test.ts @@ -1,12 +1,15 @@ import { describe, expect, it, vi } from "vitest"; +import { DEFAULT_DAEMON_PORT } from "@/transport/daemon-endpoint"; import { getConnectionEnabled, getControlHintsHidden, + getDaemonPort, getLabel, getOrCreateInstanceId, STORAGE_KEYS, setConnectionEnabled, setControlHintsHidden, + setDaemonPort, setLabel, } from "../instance-id"; @@ -117,4 +120,21 @@ describe("instance-id", () => { expect(store[STORAGE_KEYS.CONTROL_HINTS_HIDDEN]).toBe(true); expect(await getControlHintsHidden(backend)).toBe(true); }); + + it("getDaemonPort returns default when storage is empty", async () => { + const { backend } = fakeStorage(); + expect(await getDaemonPort(backend)).toBe(DEFAULT_DAEMON_PORT); + }); + + it("getDaemonPort normalizes invalid stored values to default", async () => { + const { backend } = fakeStorage({ [STORAGE_KEYS.DAEMON_PORT]: 0 }); + expect(await getDaemonPort(backend)).toBe(DEFAULT_DAEMON_PORT); + }); + + it("setDaemonPort persists the value retrievable by getDaemonPort", async () => { + const { backend, store } = fakeStorage(); + await setDaemonPort(53200, backend); + expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200); + expect(await getDaemonPort(backend)).toBe(53200); + }); }); diff --git a/apps/extension/src/lib/instance-id.ts b/apps/extension/src/lib/instance-id.ts index 087d15f0..8e8d4574 100644 --- a/apps/extension/src/lib/instance-id.ts +++ b/apps/extension/src/lib/instance-id.ts @@ -1,7 +1,10 @@ +import { normalizeDaemonPort } from "@/transport/daemon-endpoint"; + const STORAGE_KEY = "bsk_instance_id"; const LABEL_STORAGE_KEY = "bh_label"; const CONNECTION_ENABLED_KEY = "bh_connection_enabled"; const CONTROL_HINTS_HIDDEN_KEY = "bsk_control_hints_hidden"; +const DAEMON_PORT_KEY = "bsk_daemon_port"; export interface StorageBackend { get(keys: string | string[]): Promise>; @@ -127,9 +130,23 @@ export async function setControlHintsHidden( await storage.set({ [CONTROL_HINTS_HIDDEN_KEY]: hidden }); } +/** Defaults to {@link DEFAULT_DAEMON_PORT} when unset or invalid. */ +export async function getDaemonPort(storage: StorageBackend = defaultStorage()): Promise { + const items = await storage.get(DAEMON_PORT_KEY); + return normalizeDaemonPort(items[DAEMON_PORT_KEY]); +} + +export async function setDaemonPort( + port: number, + storage: StorageBackend = defaultStorage(), +): Promise { + await storage.set({ [DAEMON_PORT_KEY]: port }); +} + export const STORAGE_KEYS = { INSTANCE_ID: STORAGE_KEY, LABEL: LABEL_STORAGE_KEY, CONNECTION_ENABLED: CONNECTION_ENABLED_KEY, CONTROL_HINTS_HIDDEN: CONTROL_HINTS_HIDDEN_KEY, + DAEMON_PORT: DAEMON_PORT_KEY, } as const; diff --git a/apps/extension/src/lib/popup-bridge.ts b/apps/extension/src/lib/popup-bridge.ts index fdb8a01a..09df08c3 100644 --- a/apps/extension/src/lib/popup-bridge.ts +++ b/apps/extension/src/lib/popup-bridge.ts @@ -5,19 +5,14 @@ import type { SnapshotInfo } from "./connection-controller"; * - Background pushes `{ kind: "snapshot", data: SnapshotInfo }`. * - Popup sends `{ kind: "set_label" }`, `{ kind: "set_connection_enabled" }`, etc. * - * NOTE(review M4/M5 C2): the `set_port` variant is defined as a - * placeholder so the future custom-port UI does not have to re-design - * the bridge, but the popup does NOT render a control that emits it - * yet and the background does NOT route it. The wiring requires - * ConnectionController to dispose its current Transport and persist - * the port in chrome.storage; tracked as a follow-up. + * Daemon port preference is persisted via `chrome.storage.local` instead + * of this bridge (see `use-daemon-port.ts`). */ export const POPUP_PORT_NAME = "popup"; export type PopupOutbound = | { kind: "set_label"; value: string } - | { kind: "set_port"; value: number } | { kind: "set_connection_enabled"; value: boolean }; export type PopupInbound = { kind: "snapshot"; data: SnapshotInfo }; diff --git a/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts b/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts new file mode 100644 index 00000000..02590467 --- /dev/null +++ b/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_DAEMON_PORT, + normalizeDaemonPort, + parseDaemonPortInput, + resolveDaemonWsUrl, +} from "../daemon-endpoint"; + +describe("daemon-endpoint", () => { + it("resolveDaemonWsUrl(DEFAULT) matches the build-time constant", () => { + expect(resolveDaemonWsUrl(DEFAULT_DAEMON_PORT)).toBe(__BSK_DAEMON_WS_URL__); + }); + + it("resolveDaemonWsUrl replaces only the port", () => { + expect(resolveDaemonWsUrl(53200)).toBe("ws://127.0.0.1:53200"); + }); + + it("normalizeDaemonPort falls back to default for invalid values", () => { + expect(normalizeDaemonPort(undefined)).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort(0)).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort(65536)).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort("abc")).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort(1.5)).toBe(DEFAULT_DAEMON_PORT); + }); + + it("normalizeDaemonPort accepts valid numbers and numeric strings", () => { + expect(normalizeDaemonPort(53200)).toBe(53200); + expect(normalizeDaemonPort("53200")).toBe(53200); + }); + + it("parseDaemonPortInput treats empty as default and rejects invalid", () => { + expect(parseDaemonPortInput("")).toBe(DEFAULT_DAEMON_PORT); + expect(parseDaemonPortInput(" ")).toBe(DEFAULT_DAEMON_PORT); + expect(parseDaemonPortInput("abc")).toBeNull(); + expect(parseDaemonPortInput("0")).toBeNull(); + expect(parseDaemonPortInput("65536")).toBeNull(); + }); + + it("parseDaemonPortInput accepts valid ports", () => { + expect(parseDaemonPortInput("53200")).toBe(53200); + expect(parseDaemonPortInput(" 52800 ")).toBe(52800); + }); +}); diff --git a/apps/extension/src/transport/__tests__/ws-transport.test.ts b/apps/extension/src/transport/__tests__/ws-transport.test.ts index 566ab8ab..f314358c 100644 --- a/apps/extension/src/transport/__tests__/ws-transport.test.ts +++ b/apps/extension/src/transport/__tests__/ws-transport.test.ts @@ -270,4 +270,19 @@ describe("WSTransport", () => { lastSocket().receive({ id: "2", result: 2 }); expect(handler).toHaveBeenCalledTimes(1); }); + + it("setUrl returns false for the same URL and uses the new URL on the next connect", async () => { + const t = new WSTransport({ + url: "ws://127.0.0.1:52800", + webSocketFactory: (url) => new FakeSocket(url) as unknown as WebSocket, + }); + expect(t.setUrl("ws://127.0.0.1:52800")).toBe(false); + expect(t.setUrl("ws://127.0.0.1:53200")).toBe(true); + + const p = t.connect(); + expect(lastSocket().url).toBe("ws://127.0.0.1:53200"); + lastSocket().open(); + await p; + expect(t.state).toBe("connected"); + }); }); diff --git a/apps/extension/src/transport/daemon-endpoint.ts b/apps/extension/src/transport/daemon-endpoint.ts new file mode 100644 index 00000000..311fbe32 --- /dev/null +++ b/apps/extension/src/transport/daemon-endpoint.ts @@ -0,0 +1,39 @@ +/** Build-time default WebSocket URL (see wxt.config.ts). */ +const base = new URL(__BSK_DAEMON_WS_URL__); + +export const DEFAULT_DAEMON_PORT = Number(base.port) || 52800; + +const MIN_PORT = 1; +const MAX_PORT = 65535; + +function isValidPort(n: number): boolean { + return Number.isInteger(n) && n >= MIN_PORT && n <= MAX_PORT; +} + +/** + * Compose the daemon WebSocket URL for a loopback port, preserving the + * build-time host and path from {@link __BSK_DAEMON_WS_URL__}. + */ +export function resolveDaemonWsUrl(port: number): string { + const path = base.pathname === "/" ? "" : base.pathname; + return `${base.protocol}//${base.hostname}:${port}${path}`; +} + +/** Non-integer / out-of-range / unset → {@link DEFAULT_DAEMON_PORT}. */ +export function normalizeDaemonPort(raw: unknown): number { + if (typeof raw === "number" && isValidPort(raw)) return raw; + if (typeof raw === "string" && raw.trim() !== "") { + const parsed = Number.parseInt(raw.trim(), 10); + if (isValidPort(parsed)) return parsed; + } + return DEFAULT_DAEMON_PORT; +} + +/** Input semantics: empty string → default port; invalid → null. */ +export function parseDaemonPortInput(text: string): number | null { + const trimmed = text.trim(); + if (trimmed === "") return DEFAULT_DAEMON_PORT; + const parsed = Number.parseInt(trimmed, 10); + if (!isValidPort(parsed)) return null; + return parsed; +} diff --git a/apps/extension/src/transport/ws-transport.ts b/apps/extension/src/transport/ws-transport.ts index 2f577820..6974b6a9 100644 --- a/apps/extension/src/transport/ws-transport.ts +++ b/apps/extension/src/transport/ws-transport.ts @@ -40,7 +40,7 @@ interface MessageLikeEvent { * (1s, 2s, 4s, …, capped at 5s) until `disconnect()` is called. */ export class WSTransport implements Transport { - private readonly url: string; + private url: string; private readonly factory: WebSocketFactory; private readonly initialDelayMs: number; private readonly maxDelayMs: number; @@ -69,6 +69,13 @@ export class WSTransport implements Transport { return this.currentState; } + /** Returns whether the URL changed. Does not reconnect. */ + setUrl(url: string): boolean { + if (url === this.url) return false; + this.url = url; + return true; + } + connect(): Promise { if (this.currentState === "connected") return Promise.resolve(); diff --git a/docs/architecture.md b/docs/architecture.md index cfaf9e59..e318fb63 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -43,7 +43,7 @@ Key modules: ### bsk daemon (same binary: `bsk daemon`) -- Listens on loopback WebSocket (default **52800**) for extensions. +- Listens on loopback WebSocket (default **52800**, configurable in the extension popup) for extensions. - Validates `Origin: chrome-extension://…` on handshake. - Maintains `browsers` (connected extensions) and `sessions` (Agent Window bindings). - **Per-session queue** serializes tool calls targeting one session. diff --git a/packages/i18n/src/locales/en-US/extension.json b/packages/i18n/src/locales/en-US/extension.json index 62851c84..b1b1cfd1 100644 --- a/packages/i18n/src/locales/en-US/extension.json +++ b/packages/i18n/src/locales/en-US/extension.json @@ -26,6 +26,11 @@ "controlHintsToggleTitle": "Control hints", "controlHintsToggleHint": "Show the status pill and orange glow while the Agent controls a page.", "controlHintsInfoLabel": "About control hints", + "daemonPortLabel": "Connection port", + "daemonPortHint": "This port is used to connect to the local daemon. Do not change it unless necessary.", + "daemonPortInfoLabel": "About connection port", + "daemonPortInvalid": "Enter a port between 1 and 65535.", + "daemonUnreachable": "Can't connect. Check the daemon is running and the port matches.", "versionSkewWarning": "Extension protocol v{{extensionProtocol}}, CLI protocol v{{cliProtocol}}. Protocol versions differ — please upgrade.", "upgradeAvailable": "Upgradable", "launcher": { diff --git a/packages/i18n/src/locales/zh-CN/extension.json b/packages/i18n/src/locales/zh-CN/extension.json index 338d5df1..b6043235 100644 --- a/packages/i18n/src/locales/zh-CN/extension.json +++ b/packages/i18n/src/locales/zh-CN/extension.json @@ -26,6 +26,11 @@ "controlHintsToggleTitle": "控制提示", "controlHintsToggleHint": "Agent 控制页面时显示提示条和橙色闪光。", "controlHintsInfoLabel": "控制提示说明", + "daemonPortLabel": "连接端口", + "daemonPortHint": "扩展通过此端口连接本机 daemon。非必要请勿修改。", + "daemonPortInfoLabel": "连接端口说明", + "daemonPortInvalid": "请输入 1 到 65535 之间的端口号。", + "daemonUnreachable": "无法连接，请确认 daemon 已启动且端口一致。", "versionSkewWarning": "扩展协议 v{{extensionProtocol}}，CLI 协议 v{{cliProtocol}}。协议版本不同，请及时升级。", "upgradeAvailable": "可升级", "launcher": { From 873971f4cbac5b8c47af6bc60ccd10749551f66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Chaonan=E2=80=9D?= Date: Wed, 2 Sep 2026 19:18:03 +0800 Subject: [PATCH 2/6] fix(extension): enlarge the popup daemon port input vertically The field was too short to read; grow its height without widening the card. Co-authored-by: Cursor --- apps/extension/src/entrypoints/popup/App.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/extension/src/entrypoints/popup/App.tsx b/apps/extension/src/entrypoints/popup/App.tsx index f773cce4..906ba977 100644 --- a/apps/extension/src/entrypoints/popup/App.tsx +++ b/apps/extension/src/entrypoints/popup/App.tsx @@ -313,7 +313,7 @@ export function App() { commitDaemonPort(); } }} - className="mt-0 h-5 w-24 shrink-0 rounded-md px-2 py-0 text-center text-sm leading-none shadow-none" + className="mt-0 h-7.5 w-19 shrink-0 rounded-md px-2 py-0 text-center text-sm leading-none shadow-none" aria-invalid={daemonPortInvalid || undefined} data-slot="popup-daemon-port-input" /> From 11cd12c9cdc9a954e1139bc32f760053ed0691a0 Mon Sep 17 00:00:00 2001 From: polarday <61319465+shnpd@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:57:12 +0800 Subject: [PATCH 3/6] Fix regex for validating daemon port input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 如果有字符就报错，不直接转为数字 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/extension/src/transport/daemon-endpoint.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/extension/src/transport/daemon-endpoint.ts b/apps/extension/src/transport/daemon-endpoint.ts index 311fbe32..6ca38224 100644 --- a/apps/extension/src/transport/daemon-endpoint.ts +++ b/apps/extension/src/transport/daemon-endpoint.ts @@ -33,6 +33,7 @@ export function normalizeDaemonPort(raw: unknown): number { export function parseDaemonPortInput(text: string): number | null { const trimmed = text.trim(); if (trimmed === "") return DEFAULT_DAEMON_PORT; + if (!/^\d+$/.test(trimmed)) return null; const parsed = Number.parseInt(trimmed, 10); if (!isValidPort(parsed)) return null; return parsed; From 9a5d45d156a8942dfec4b33d30c2937da8025c6f Mon Sep 17 00:00:00 2001 From: polarday <61319465+shnpd@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:58:49 +0800 Subject: [PATCH 4/6] Fix comment formatting in useDaemonPort function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复注释不准确 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/extension/src/entrypoints/popup/use-daemon-port.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/extension/src/entrypoints/popup/use-daemon-port.ts b/apps/extension/src/entrypoints/popup/use-daemon-port.ts index 7a6efcb6..64eb4fa1 100644 --- a/apps/extension/src/entrypoints/popup/use-daemon-port.ts +++ b/apps/extension/src/entrypoints/popup/use-daemon-port.ts @@ -4,7 +4,7 @@ import { parseDaemonPortInput } from "@/transport/daemon-endpoint"; /** * Popup-side daemon port preference stored in `chrome.storage.local`. - * Commits on blur, Enter (via blur), and pagehide. + * Commits on blur, Enter, and pagehide. */ export function useDaemonPort(): { draft: string; From f97db9c39c8748b0224f5ab416a96be5de365ba1 Mon Sep 17 00:00:00 2001 From: drakezhang Date: Thu, 10 Sep 2026 18:28:23 +0800 Subject: [PATCH 5/6] fix(extension): make daemon port changes safe Coordinate endpoint changes, session cleanup and reconnect requests in the connection controller without waiting for unreachable sockets. Preserve connection preferences and protocol diagnostics, and explicitly save popup port drafts with loading and write-error handling. Cover storage ordering, rapid configuration changes, disconnect cleanup, handshake cancellation and transport backoff with regression tests. --- apps/extension/src/entrypoints/background.ts | 72 ++--- .../src/entrypoints/popup/App.test.tsx | 36 ++- apps/extension/src/entrypoints/popup/App.tsx | 35 +- .../entrypoints/popup/use-daemon-port.test.ts | 201 ++++++++++++ .../src/entrypoints/popup/use-daemon-port.ts | 150 ++++++--- .../__tests__/connection-controller.test.ts | 2 +- .../connection-reconfiguration.test.ts | 302 ++++++++++++++++++ .../__tests__/daemon-port-preference.test.ts | 78 +++++ .../src/lib/__tests__/keepalive.test.ts | 9 + .../src/lib/connection-controller.ts | 222 +++++++------ .../src/lib/daemon-port-preference.ts | 32 ++ apps/extension/src/lib/keepalive.ts | 5 +- .../__tests__/daemon-endpoint.test.ts | 3 + .../src/transport/daemon-endpoint.ts | 3 +- docs/architecture.md | 2 +- .../i18n/src/locales/en-US/extension.json | 6 +- .../i18n/src/locales/zh-CN/extension.json | 6 +- 17 files changed, 945 insertions(+), 219 deletions(-) create mode 100644 apps/extension/src/entrypoints/popup/use-daemon-port.test.ts create mode 100644 apps/extension/src/lib/__tests__/connection-reconfiguration.test.ts create mode 100644 apps/extension/src/lib/__tests__/daemon-port-preference.test.ts create mode 100644 apps/extension/src/lib/daemon-port-preference.ts diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 36a4e06d..f42425cc 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -1,12 +1,11 @@ import { i18n } from "@browser-skill/i18n"; import { ChromiumCdp } from "@/browser-driver/chromium-cdp"; import { ConnectionController } from "@/lib/connection-controller"; +import { watchDaemonPort } from "@/lib/daemon-port-preference"; import { startHeartbeat } from "@/lib/heartbeat"; import { getConnectionEnabled, - getDaemonPort, setConnectionEnabled as persistConnectionEnabled, - STORAGE_KEYS, setLabel, } from "@/lib/instance-id"; import { startKeepalive } from "@/lib/keepalive"; @@ -57,26 +56,13 @@ export default defineBackground(() => { let overlayGeneration = 0; const controlModes = new Map(); - async function applyDaemonPort(port: number): Promise { + const daemonPort = watchDaemonPort((port) => { const url = resolveDaemonWsUrl(port); - if (!transport.setUrl(url)) return; - await transport.disconnect(); - if (!controller.isConnectionEnabled) return; - try { - await transport.connect(); - } catch (err) { - console.debug("[browser-skill] reconnect after port change failed", err); - } - } - - if (typeof chrome !== "undefined" && chrome.storage?.onChanged) { - chrome.storage.onChanged.addListener((changes, areaName) => { - if (areaName !== "local") return; - const change = changes[STORAGE_KEYS.DAEMON_PORT]; - if (!change || typeof change.newValue !== "number") return; - void applyDaemonPort(change.newValue); + void controller.reconfigureTransport(url, () => { + transport.setUrl(url); }); - } + }); + let preferenceWrites = Promise.resolve(); function setControlMode(sessionId: string, mode: OverlayMode): void { if (controlModes.get(sessionId) === mode) return; @@ -259,7 +245,7 @@ export default defineBackground(() => { // the BrowserSkill connection. startKeepalive({ transport, - shouldConnect: () => controller.isConnectionEnabled, + requestConnect: () => controller.requestConnect(), }); // Application-level heartbeat (Chrome 116+): while the post-handshake @@ -281,13 +267,7 @@ export default defineBackground(() => { // the worker and let us reconnect immediately instead of waiting for // the next 30s alarm tick. They only help when the daemon is actually // running; a cold daemon is (re)spawned by the next `bsk` command. - const reconnectIfNeeded = () => { - if (!controller.isConnectionEnabled) return; - if (transport.state === "connected") return; - void transport.connect().catch((err) => { - console.debug("[browser-skill] wake reconnect attempt failed", err); - }); - }; + const reconnectIfNeeded = () => controller.requestConnect(); if (typeof chrome.runtime?.onStartup?.addListener === "function") { chrome.runtime.onStartup.addListener(reconnectIfNeeded); } @@ -302,22 +282,18 @@ export default defineBackground(() => { } void (async () => { - const connectionEnabled = await getConnectionEnabled(); - const port = await getDaemonPort(); - transport.setUrl(resolveDaemonWsUrl(port)); + const [connectionEnabled] = await Promise.all([getConnectionEnabled(), daemonPort.ready]); + const cleanup = async () => { + const report = await cleanupAfterDisconnect(); + if (report.failures.length > 0) { + throw new Error( + `Session cleanup incomplete: ${report.failures.map((failure) => failure.message).join("; ")}`, + ); + } + }; await controller.attach(transport, detectBrowserMeta(), connectionEnabled, { - beforeDisconnect: async () => { - const report = await cleanupAfterDisconnect(); - if (report.failures.length > 0) { - console.warn("[browser-skill] session cleanup before disconnect was incomplete", report); - } - }, - onDisconnected: async () => { - const report = await cleanupAfterDisconnect(); - if (report.failures.length > 0) { - console.warn("[browser-skill] session cleanup after disconnect was incomplete", report); - } - }, + beforeDisconnect: cleanup, + onDisconnected: cleanup, }); })().catch((err) => { console.error("[browser-skill] controller failed to attach", err); @@ -372,9 +348,13 @@ export default defineBackground(() => { if (msg.kind === "set_label") { void setLabel(msg.value).then(() => controller.refreshLabel()); } else if (msg.kind === "set_connection_enabled") { - void controller - .setConnectionEnabled(msg.value) - .then(() => persistConnectionEnabled(msg.value)); + void controller.setConnectionEnabled(msg.value); + // Persist user intent in message order, independently of slow cleanup. + preferenceWrites = preferenceWrites + .then(() => persistConnectionEnabled(msg.value)) + .catch((err) => { + console.error("[browser-skill] connection preference write failed", err); + }); } } }); diff --git a/apps/extension/src/entrypoints/popup/App.test.tsx b/apps/extension/src/entrypoints/popup/App.test.tsx index 6fa9145c..4c10f79c 100644 --- a/apps/extension/src/entrypoints/popup/App.test.tsx +++ b/apps/extension/src/entrypoints/popup/App.test.tsx @@ -71,11 +71,11 @@ describe("App", () => { expect(screen.queryByText("请先打开 BrowserSkill。")).toBeNull(); }); - it("shows the connection switch off and hides transport errors when disconnected", () => { + it("keeps the connection switch usable and shows protocol errors when disconnected", () => { mockUseConnectionState.mockReturnValue({ snapshot: { ...baseSnapshot, - lastError: "[WSTransport] disconnect during connect", + lastError: "version_too_old: protocol-major mismatch", }, statusState: "disconnected", setLabel, @@ -85,12 +85,14 @@ describe("App", () => { render(); expect(screen.getByText("未连接")).toBeTruthy(); - expect(screen.getByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeTruthy(); + expect(screen.queryByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeNull(); expect(screen.queryByText("端口不匹配")).toBeNull(); expect( screen.getByRole("switch", { name: "BrowserSkill 连接" }).getAttribute("aria-checked"), - ).toBe("false"); - expect(screen.queryByText("[WSTransport] disconnect during connect")).toBeNull(); + ).toBe("true"); + expect(screen.getByText("version_too_old: protocol-major mismatch")).toBeTruthy(); + fireEvent.click(screen.getByRole("switch", { name: "BrowserSkill 连接" })); + expect(setConnectionEnabled).toHaveBeenCalledWith(false); }); it("does not render record UI on the main view", () => { @@ -462,29 +464,31 @@ describe("daemon port input", () => { await waitFor(() => expect((input as HTMLInputElement).value).toBe("53200")); }); - it("persists a valid port on blur", async () => { + it("persists a valid port with the save button", async () => { const store = stubChromeStorage(); render(); const input = await screen.findByRole("textbox", { name: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); fireEvent.change(input, { target: { value: "53200" } }); - fireEvent.blur(input); + fireEvent.click(screen.getByRole("button", { name: "保存端口" })); - expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200); + await waitFor(() => expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200)); expect((input as HTMLInputElement).value).toBe("53200"); }); - it("persists a valid port when Enter blurs the field", async () => { + it("persists a valid port on Enter", async () => { const store = stubChromeStorage(); render(); const input = await screen.findByRole("textbox", { name: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); fireEvent.change(input, { target: { value: "53200" } }); fireEvent.keyDown(input, { key: "Enter" }); - expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200); + await waitFor(() => expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200)); }); it("shows an error and does not write invalid ports", async () => { @@ -493,8 +497,9 @@ describe("daemon port input", () => { render(); const input = await screen.findByRole("textbox", { name: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); fireEvent.change(input, { target: { value: "abc" } }); - fireEvent.blur(input); + fireEvent.click(screen.getByRole("button", { name: "保存端口" })); expect(screen.getByText("请输入 1 到 65535 之间的端口号。")).toBeTruthy(); expect(store[STORAGE_KEYS.DAEMON_PORT]).toBeUndefined(); @@ -506,10 +511,11 @@ describe("daemon port input", () => { render(); const input = await screen.findByRole("textbox", { name: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); fireEvent.change(input, { target: { value: "" } }); - fireEvent.blur(input); + fireEvent.click(screen.getByRole("button", { name: "保存端口" })); - expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(DEFAULT_DAEMON_PORT); + await waitFor(() => expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(DEFAULT_DAEMON_PORT)); expect((input as HTMLInputElement).value).toBe(String(DEFAULT_DAEMON_PORT)); }); @@ -520,7 +526,9 @@ describe("daemon port input", () => { const info = await screen.findByRole("button", { name: "连接端口说明" }); expect(info).toBeTruthy(); - const tooltip = screen.getByText("扩展通过此端口连接本机 daemon。非必要请勿修改。"); + const tooltip = screen.getByText( + "通过此端口连接本机 daemon。保存修改会结束当前会话，并在连接开关开启时重新连接。", + ); expect(tooltip.getAttribute("role")).toBe("tooltip"); expect(tooltip.className).toContain("opacity-0"); }); diff --git a/apps/extension/src/entrypoints/popup/App.tsx b/apps/extension/src/entrypoints/popup/App.tsx index 906ba977..affa4524 100644 --- a/apps/extension/src/entrypoints/popup/App.tsx +++ b/apps/extension/src/entrypoints/popup/App.tsx @@ -47,6 +47,10 @@ export function App() { setDraft: setDaemonPortDraft, commit: commitDaemonPort, invalid: daemonPortInvalid, + loaded: daemonPortLoaded, + saving: daemonPortSaving, + dirty: daemonPortDirty, + error: daemonPortError, } = useDaemonPort(); const [view, setView] = useState("main"); const [copiedInstanceId, setCopiedInstanceId] = useState(false); @@ -206,14 +210,14 @@ export function App() { {t(STATE_BADGE_KEYS[statusState])}
- {isDisconnected && ( + {isDisconnected && !snapshot.lastError && (

) => setDaemonPortDraft(event.target.value) } - onBlur={commitDaemonPort} + disabled={!daemonPortLoaded || daemonPortSaving} onKeyDown={(event) => { if (event.key === "Enter") { event.preventDefault(); - commitDaemonPort(); + void commitDaemonPort(); } }} className="mt-0 h-7.5 w-19 shrink-0 rounded-md px-2 py-0 text-center text-sm leading-none shadow-none" @@ -318,6 +322,27 @@ export function App() { data-slot="popup-daemon-port-input" /> +

+ +
+ {daemonPortError && ( +

+ {t( + daemonPortError === "read" + ? "popup.daemonPortReadFailed" + : "popup.daemonPortWriteFailed", + )} +

+ )} {daemonPortInvalid && (

- {snapshot.lastError && !isDisconnected && ( + {snapshot.lastError && (

) => void; +let changed: (changes: Record, area: string) => void; +let runtime: { lastError?: { message: string } }; +let writes: Array<{ items: Record; done: () => void }>; + +beforeEach(() => { + runtime = {}; + writes = []; + vi.stubGlobal("chrome", { + runtime, + storage: { + local: { + get: (_: string, cb: typeof read) => { + read = cb; + }, + set: (items: Record, done: () => void) => { + writes.push({ items, done }); + }, + }, + onChanged: { + addListener: (cb: typeof changed) => { + changed = cb; + }, + removeListener: vi.fn(), + }, + }, + }); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +async function load(port = 53200) { + await act(async () => read({ [key]: port })); +} + +describe("useDaemonPort", () => { + it("never writes an unread preference on edit, submit, blur, pagehide or unmount", async () => { + const hook = renderHook(() => useDaemonPort()); + act(() => hook.result.current.setDraft("53300")); + await act(async () => { + await hook.result.current.commit(); + }); + act(() => { + window.dispatchEvent(new Event("blur")); + window.dispatchEvent(new Event("pagehide")); + }); + expect(hook.result.current.loaded).toBe(false); + expect(writes).toHaveLength(0); + hook.unmount(); + await load(); + expect(writes).toHaveLength(0); + }); + + it("retains the latest storage change when initial reading finishes later", async () => { + const hook = renderHook(() => useDaemonPort()); + act(() => changed({ [key]: { newValue: 53300 } }, "local")); + await load(); + expect(hook.result.current.draft).toBe("53300"); + expect(writes).toHaveLength(0); + }); + + it("does not overwrite a local edit with a storage notification", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft("53400")); + act(() => changed({ [key]: { newValue: 53300 } }, "local")); + expect(hook.result.current.draft).toBe("53400"); + expect(hook.result.current.dirty).toBe(true); + act(() => { + window.dispatchEvent(new Event("pagehide")); + }); + expect(writes).toHaveLength(0); + }); + + it("follows subsequent storage changes once an edit matches the saved value", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft("53300")); + act(() => changed({ [key]: { newValue: 53300 } }, "local")); + expect(hook.result.current.dirty).toBe(false); + act(() => changed({ [key]: { newValue: 53400 } }, "local")); + expect(hook.result.current.draft).toBe("53400"); + expect(hook.result.current.dirty).toBe(false); + }); + + it("normalizes an unchanged value without writing or reconnecting", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft(" 053200 ")); + await act(async () => { + await hook.result.current.commit(); + }); + expect(writes).toHaveLength(0); + expect(hook.result.current.draft).toBe("53200"); + expect(hook.result.current.dirty).toBe(false); + }); + + it.each(["abc", "53200abc", "1.5", "0", "65536"])("rejects %s without writing", async (value) => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft(value)); + await act(async () => { + await hook.result.current.commit(); + }); + expect(hook.result.current.invalid).toBe(true); + expect(writes).toHaveLength(0); + }); + + it("blocks duplicate saves and edits until an explicit default reset is saved", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft("")); + let saving!: Promise; + act(() => { + saving = hook.result.current.commit(); + }); + expect(hook.result.current.saving).toBe(true); + act(() => hook.result.current.setDraft("53400")); + await act(async () => { + await hook.result.current.commit(); + }); + expect(writes).toHaveLength(1); + expect(writes[0].items).toEqual({ [key]: DEFAULT_DAEMON_PORT }); + await act(async () => { + writes[0].done(); + await saving; + }); + expect(hook.result.current.draft).toBe(String(DEFAULT_DAEMON_PORT)); + expect(hook.result.current.dirty).toBe(false); + expect(hook.result.current.saving).toBe(false); + }); + + it("retains the draft after a failed save and allows retry", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft("53300")); + let saving!: Promise; + act(() => { + saving = hook.result.current.commit(); + }); + await act(async () => { + runtime.lastError = { message: "write failed" }; + writes[0].done(); + delete runtime.lastError; + await saving; + }); + expect(hook.result.current.error).toBe("write"); + expect(hook.result.current.draft).toBe("53300"); + expect(hook.result.current.dirty).toBe(true); + act(() => { + saving = hook.result.current.commit(); + }); + await act(async () => { + writes[1].done(); + await saving; + }); + expect(hook.result.current.error).toBeNull(); + expect(hook.result.current.dirty).toBe(false); + }); + + it("shows read failure without interpreting the empty draft as a reset", async () => { + const hook = renderHook(() => useDaemonPort()); + await act(async () => { + runtime.lastError = { message: "read failed" }; + read({}); + delete runtime.lastError; + }); + expect(hook.result.current.error).toBe("read"); + expect(hook.result.current.loaded).toBe(false); + await act(async () => { + await hook.result.current.commit(); + }); + expect(writes).toHaveLength(0); + }); + + it("does not replace newer persisted state with a late write completion", async () => { + const hook = renderHook(() => useDaemonPort()); + await load(); + act(() => hook.result.current.setDraft("53300")); + let saving!: Promise; + act(() => { + saving = hook.result.current.commit(); + }); + act(() => changed({ [key]: { newValue: 53400 } }, "local")); + await act(async () => { + writes[0].done(); + await saving; + }); + expect(hook.result.current.draft).toBe("53400"); + expect(hook.result.current.dirty).toBe(false); + }); +}); diff --git a/apps/extension/src/entrypoints/popup/use-daemon-port.ts b/apps/extension/src/entrypoints/popup/use-daemon-port.ts index 64eb4fa1..b6c8b76b 100644 --- a/apps/extension/src/entrypoints/popup/use-daemon-port.ts +++ b/apps/extension/src/entrypoints/popup/use-daemon-port.ts @@ -1,71 +1,115 @@ import { useCallback, useEffect, useRef, useState } from "react"; -import { getDaemonPort, STORAGE_KEYS, setDaemonPort } from "@/lib/instance-id"; +import { watchDaemonPort } from "@/lib/daemon-port-preference"; +import { setDaemonPort } from "@/lib/instance-id"; import { parseDaemonPortInput } from "@/transport/daemon-endpoint"; -/** - * Popup-side daemon port preference stored in `chrome.storage.local`. - * Commits on blur, Enter, and pagehide. - */ -export function useDaemonPort(): { - draft: string; - setDraft: (value: string) => void; - commit: () => void; +type PortState = { + savedPort: number | null; + // null means no local edit: show the latest persisted value. + draft: string | null; + saving: boolean; invalid: boolean; -} { - const [draft, setDraftState] = useState(""); - const [invalid, setInvalid] = useState(false); - const draftRef = useRef(draft); - draftRef.current = draft; + error: "read" | "write" | null; +}; + +/** Explicitly save a draft; opening or closing the popup never writes a preference. */ +export function useDaemonPort() { + const [state, setState] = useState({ + savedPort: null, + draft: null, + saving: false, + invalid: false, + error: null, + }); + const savingRef = useRef(false); + const mounted = useRef(false); + const storageRevision = useRef(0); useEffect(() => { - if (typeof chrome === "undefined" || !chrome.storage?.local) return undefined; - let cancelled = false; - getDaemonPort() - .then((port) => { - if (!cancelled) setDraftState(String(port)); - }) - .catch((err) => { - console.debug("[browser-skill] daemon port read failed", err); - }); - const onChanged = (changes: Record, areaName: string) => { - if (areaName !== "local") return; - const change = changes[STORAGE_KEYS.DAEMON_PORT]; - if (change && typeof change.newValue === "number") { - setDraftState(String(change.newValue)); - setInvalid(false); - } - }; - chrome.storage.onChanged.addListener(onChanged); + mounted.current = true; + if (typeof chrome === "undefined" || !chrome.storage?.local) { + setState((s) => ({ ...s, error: "read" })); + return () => { + mounted.current = false; + }; + } + const preference = watchDaemonPort((port) => { + storageRevision.current += 1; + setState((s) => ({ + ...s, + savedPort: port, + draft: + !s.saving && (s.draft === String(s.savedPort) || s.draft === String(port)) + ? null + : s.draft, + error: s.error === "read" ? null : s.error, + })); + }); + void preference.ready.catch(() => { + if (mounted.current) setState((s) => ({ ...s, error: "read" })); + }); return () => { - cancelled = true; - chrome.storage.onChanged.removeListener(onChanged); + mounted.current = false; + preference.dispose(); }; }, []); - const commit = useCallback(() => { - const parsed = parseDaemonPortInput(draftRef.current); + const draft = state.draft ?? String(state.savedPort ?? ""); + const dirty = state.savedPort !== null && draft !== String(state.savedPort); + const commit = useCallback(async () => { + if (state.savedPort === null || savingRef.current) return; + const parsed = parseDaemonPortInput(draft); if (parsed === null) { - setInvalid(true); + setState((s) => ({ ...s, invalid: true })); return; } - setInvalid(false); - setDraftState(String(parsed)); - if (typeof chrome === "undefined" || !chrome.storage?.local) return; - setDaemonPort(parsed).catch((err) => { - console.debug("[browser-skill] daemon port write failed", err); - }); - }, []); - - useEffect(() => { - const onPageHide = () => commit(); - window.addEventListener("pagehide", onPageHide); - return () => window.removeEventListener("pagehide", onPageHide); - }, [commit]); + if (parsed === state.savedPort) { + setState((s) => ({ ...s, draft: null, invalid: false, error: null })); + return; + } + savingRef.current = true; + const revision = storageRevision.current; + setState((s) => ({ ...s, saving: true, invalid: false, error: null })); + try { + await setDaemonPort(parsed); + if (mounted.current) { + setState((s) => ({ + ...s, + // A storage event is newer than the snapshot at submission time. + savedPort: storageRevision.current === revision ? parsed : s.savedPort, + draft: null, + saving: false, + })); + } + } catch { + if (mounted.current) setState((s) => ({ ...s, saving: false, error: "write" })); + } finally { + savingRef.current = false; + } + }, [draft, state.savedPort]); const setDraft = useCallback((value: string) => { - setDraftState(value); - setInvalid(false); + if (savingRef.current) return; + setState((s) => + s.savedPort === null + ? s + : { + ...s, + draft: value === String(s.savedPort) ? null : value, + invalid: false, + error: null, + }, + ); }, []); - return { draft, setDraft, commit, invalid }; + return { + draft, + setDraft, + commit, + dirty, + loaded: state.savedPort !== null, + saving: state.saving, + invalid: state.invalid, + error: state.error, + }; } diff --git a/apps/extension/src/lib/__tests__/connection-controller.test.ts b/apps/extension/src/lib/__tests__/connection-controller.test.ts index e6c88d9e..0c4cf0f5 100644 --- a/apps/extension/src/lib/__tests__/connection-controller.test.ts +++ b/apps/extension/src/lib/__tests__/connection-controller.test.ts @@ -238,7 +238,7 @@ describe("ConnectionController connectionEnabled", () => { const first = transport.send.mock.calls[0]?.[0] as { id: string }; transport.emitState("disconnected"); - transport.emitState("connected"); + await vi.waitFor(() => expect(transport.send).toHaveBeenCalledTimes(2)); const second = transport.send.mock.calls[1]?.[0] as { id: string }; expect(second.id).not.toBe(first.id); diff --git a/apps/extension/src/lib/__tests__/connection-reconfiguration.test.ts b/apps/extension/src/lib/__tests__/connection-reconfiguration.test.ts new file mode 100644 index 00000000..aba5ad2f --- /dev/null +++ b/apps/extension/src/lib/__tests__/connection-reconfiguration.test.ts @@ -0,0 +1,302 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { PROTOCOL_VERSION } from "@/transport/handshake"; +import { WSTransport } from "@/transport/ws-transport"; +import { ConnectionController } from "../connection-controller"; +import { getLabel } from "../instance-id"; + +vi.mock("../instance-id", () => ({ + getOrCreateInstanceId: vi.fn(async () => "a1b2c3d4"), + getLabel: vi.fn(async () => "test-label"), +})); + +class Socket extends EventTarget { + readyState = 0; + sent: Array<{ id: string }> = []; + constructor(readonly url: string) { + super(); + } + close() { + this.readyState = 3; + this.dispatchEvent(new Event("close")); + } + open() { + this.readyState = 1; + this.dispatchEvent(new Event("open")); + } + send(text: string) { + this.sent.push(JSON.parse(text)); + } + reply(protocol = PROTOCOL_VERSION) { + this.dispatchEvent( + new MessageEvent("message", { + data: JSON.stringify({ + id: this.sent[0].id, + result: { server: "browser-skill-daemon", version: "0.1.0", protocol_version: protocol }, + }), + }), + ); + } +} + +function deferred() { + let resolve!: () => void; + let reject!: (err: Error) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} + +async function flush() { + for (let i = 0; i < 20; i += 1) await Promise.resolve(); +} + +function setup(cleanup = vi.fn(async () => {})) { + const sockets: Socket[] = []; + const transport = new WSTransport({ + url: "ws://127.0.0.1:52800", + webSocketFactory: (url) => { + const socket = new Socket(url); + sockets.push(socket); + return socket as unknown as WebSocket; + }, + }); + const controller = new ConnectionController(); + const configure = (port: number) => { + const url = `ws://127.0.0.1:${port}`; + return controller.reconfigureTransport(url, () => { + transport.setUrl(url); + }); + }; + const attach = (enabled = true) => + controller.attach(transport, { name: "Chrome", version: "125" }, enabled, { + beforeDisconnect: cleanup, + onDisconnected: cleanup, + }); + return { controller, transport, sockets, configure, attach, cleanup }; +} + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe("connection reconfiguration with WSTransport", () => { + it("finishes initialization even when the first socket never opens", async () => { + const s = setup(); + s.controller.requestConnect(); + expect(s.sockets).toHaveLength(0); + await s.configure(53200); + await s.attach(); + expect(s.sockets.map((socket) => socket.url)).toEqual(["ws://127.0.0.1:53200"]); + await s.configure(53300); + expect(s.sockets.at(-1)?.url).toBe("ws://127.0.0.1:53300"); + await s.controller.setConnectionEnabled(false); + expect(s.transport.state).toBe("disconnected"); + expect(s.controller.snapshot().lastError).toBeNull(); + }); + + it("uses configuration and user preference changes received during attach", async () => { + const gate = deferred(); + vi.mocked(getLabel).mockImplementationOnce(async () => { + await gate.promise; + return "test-label"; + }); + const s = setup(); + const attaching = s.attach(); + await flush(); + await s.configure(53200); + await s.controller.setConnectionEnabled(false); + s.controller.requestConnect(); + gate.resolve(); + await attaching; + expect(s.sockets).toHaveLength(0); + await s.controller.setConnectionEnabled(true); + expect(s.sockets.at(-1)?.url).toBe("ws://127.0.0.1:53200"); + }); + + it("waits for one cleanup, coalesces changes, and rejects wake requests during cleanup", async () => { + const gate = deferred(); + const s = setup(vi.fn(() => gate.promise)); + await s.attach(); + s.sockets[0].open(); + s.sockets[0].reply(); + await flush(); + const first = s.configure(53200); + await flush(); + const last = s.configure(53300); + s.controller.requestConnect(); + await vi.advanceTimersByTimeAsync(30_000); + expect(s.sockets).toHaveLength(1); + expect(s.cleanup).toHaveBeenCalledOnce(); + expect(s.controller.snapshot().state).toBe("disconnected"); + gate.resolve(); + await Promise.all([first, last]); + expect(s.sockets.map((socket) => socket.url)).toEqual([ + "ws://127.0.0.1:52800", + "ws://127.0.0.1:53300", + ]); + await s.configure(53300); + expect(s.sockets).toHaveLength(2); + expect(s.cleanup).toHaveBeenCalledOnce(); + }); + + it("does not reconnect if disabled during a port change", async () => { + const gate = deferred(); + const s = setup(vi.fn(() => gate.promise)); + await s.attach(); + s.sockets[0].open(); + const changing = s.configure(53200); + await flush(); + const disabling = s.controller.setConnectionEnabled(false); + gate.resolve(); + await Promise.all([changing, disabling]); + s.controller.requestConnect(); + expect(s.sockets).toHaveLength(1); + expect(s.controller.snapshot().connectionEnabled).toBe(false); + expect(s.controller.snapshot().lastError).toBeNull(); + }); + + it("honors re-enabling and a port update while user-disable cleanup is pending", async () => { + const gate = deferred(); + const s = setup(vi.fn(() => gate.promise)); + await s.attach(); + s.sockets[0].open(); + const disabling = s.controller.setConnectionEnabled(false); + await flush(); + // Losing the socket during cleanup must not resurrect its retry timer. + s.sockets[0].close(); + await s.controller.setConnectionEnabled(true); + const changing = s.configure(53200); + await vi.advanceTimersByTimeAsync(30_000); + expect(s.sockets).toHaveLength(1); + gate.resolve(); + await Promise.all([disabling, changing]); + expect(s.cleanup).toHaveBeenCalledOnce(); + expect(s.sockets).toHaveLength(2); + expect(s.sockets[1].url).toBe("ws://127.0.0.1:53200"); + expect(s.controller.snapshot().connectionEnabled).toBe(true); + }); + + it("cancels a pending handshake retry when reconfiguring", async () => { + const gate = deferred(); + const s = setup(vi.fn(() => gate.promise)); + await s.attach(); + s.sockets[0].open(); + s.sockets[0].dispatchEvent( + new MessageEvent("message", { + data: JSON.stringify({ + id: s.sockets[0].sent[0].id, + error: { code: "protocol_error", message: "bad handshake" }, + }), + }), + ); + await flush(); + const changing = s.configure(53200); + await vi.advanceTimersByTimeAsync(10_000); + expect(s.sockets).toHaveLength(1); + gate.resolve(); + await changing; + s.sockets[1].open(); + s.sockets[1].reply(); + await flush(); + expect(s.controller.snapshot().lastError).toBeNull(); + expect(s.controller.snapshot().state).toBe("connected"); + }); + + it("allows wake-driven recovery after upgrading a rejected daemon", async () => { + const s = setup(); + await s.attach(); + s.sockets[0].open(); + s.sockets[0].reply("99.0"); + await flush(); + s.controller.requestConnect(); + s.sockets[1].open(); + s.sockets[1].reply(); + await flush(); + expect(s.controller.snapshot().state).toBe("connected"); + expect(s.controller.snapshot().lastError).toBeNull(); + }); + + it("shares cleanup already running after unexpected loss", async () => { + const gate = deferred(); + const s = setup(vi.fn(() => gate.promise)); + await s.attach(); + s.sockets[0].open(); + s.sockets[0].close(); + await flush(); + const changing = s.configure(53200); + gate.resolve(); + await changing; + expect(s.cleanup).toHaveBeenCalledOnce(); + expect(s.sockets).toHaveLength(2); + expect(s.sockets[1].url).toBe("ws://127.0.0.1:53200"); + }); + + it("keeps failed cleanup blocking the new endpoint until a retry succeeds", async () => { + const cleanup = vi + .fn() + .mockRejectedValueOnce(new Error("tab return failed")) + .mockResolvedValue(undefined); + const s = setup(cleanup); + await s.attach(); + s.sockets[0].open(); + await s.configure(53200); + expect(s.sockets).toHaveLength(1); + expect(s.controller.snapshot().lastError).toBe("tab return failed"); + s.controller.requestConnect(); + await flush(); + expect(s.sockets.at(-1)?.url).toBe("ws://127.0.0.1:53200"); + expect(s.cleanup).toHaveBeenCalledTimes(2); + }); + + it("preserves transport backoff for unreachable endpoints", async () => { + const s = setup(); + await s.attach(); + s.sockets[0].close(); + await flush(); + expect(s.sockets).toHaveLength(1); + expect(s.cleanup).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1_000); + expect(s.sockets).toHaveLength(2); + s.sockets[1].close(); + await vi.advanceTimersByTimeAsync(1_999); + expect(s.sockets).toHaveLength(2); + await vi.advanceTimersByTimeAsync(1); + expect(s.sockets).toHaveLength(3); + }); + + it("keeps protocol rejection visible and allows a different endpoint", async () => { + const s = setup(); + await s.attach(); + s.sockets[0].open(); + s.sockets[0].reply("99.0"); + await flush(); + expect(s.controller.snapshot().lastError).toContain("version_too_old"); + await vi.advanceTimersByTimeAsync(30_000); + expect(s.sockets).toHaveLength(1); + await s.configure(53200); + s.sockets[1].open(); + s.sockets[1].reply(); + await flush(); + expect(s.controller.snapshot().state).toBe("connected"); + expect(s.controller.snapshot().lastError).toBeNull(); + }); + + it("does not let a late old handshake clobber the new connection", async () => { + const s = setup(); + await s.attach(); + s.sockets[0].open(); + await s.configure(53200); + s.sockets[1].open(); + s.sockets[1].reply(); + await flush(); + s.sockets[0].reply("99.0"); + await flush(); + expect(s.controller.snapshot().state).toBe("connected"); + expect(s.controller.snapshot().lastError).toBeNull(); + }); +}); diff --git a/apps/extension/src/lib/__tests__/daemon-port-preference.test.ts b/apps/extension/src/lib/__tests__/daemon-port-preference.test.ts new file mode 100644 index 00000000..2714b48c --- /dev/null +++ b/apps/extension/src/lib/__tests__/daemon-port-preference.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { DEFAULT_DAEMON_PORT } from "@/transport/daemon-endpoint"; +import { watchDaemonPort } from "../daemon-port-preference"; +import { STORAGE_KEYS } from "../instance-id"; + +function storage() { + let read!: (items: Record) => void; + let changed!: (changes: Record, area: string) => void; + const removeListener = vi.fn(); + vi.stubGlobal("chrome", { + runtime: {}, + storage: { + local: { + get: (_: string, cb: typeof read) => { + read = cb; + }, + }, + onChanged: { + addListener: (cb: typeof changed) => { + changed = cb; + }, + removeListener, + }, + }, + }); + return { + read: (value: unknown) => read({ [STORAGE_KEYS.DAEMON_PORT]: value }), + change: (value: unknown, area = "local") => + changed({ [STORAGE_KEYS.DAEMON_PORT]: { newValue: value } }, area), + removeListener, + }; +} + +afterEach(() => vi.unstubAllGlobals()); + +describe("watchDaemonPort", () => { + it("uses the latest change instead of a late initial read", async () => { + const store = storage(); + const onPort = vi.fn(); + const watch = watchDaemonPort(onPort); + store.change(53200); + store.change(53300); + store.read(52800); + await watch.ready; + expect(onPort.mock.calls).toEqual([[53200], [53300]]); + watch.dispose(); + expect(store.removeListener).toHaveBeenCalledOnce(); + }); + + it("normalizes removed, malformed and numeric-string values consistently", async () => { + const store = storage(); + const onPort = vi.fn(); + const watch = watchDaemonPort(onPort); + store.read("53200"); + await watch.ready; + store.change(53300, "sync"); + store.change(undefined); + store.change("53200abc"); + store.change("53400"); + expect(onPort.mock.calls).toEqual([ + [53200], + [DEFAULT_DAEMON_PORT], + [DEFAULT_DAEMON_PORT], + [53400], + ]); + watch.dispose(); + }); + + it("does not deliver an initial read after disposal", async () => { + const store = storage(); + const onPort = vi.fn(); + const watch = watchDaemonPort(onPort); + watch.dispose(); + store.read(53200); + await watch.ready; + expect(onPort).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/extension/src/lib/__tests__/keepalive.test.ts b/apps/extension/src/lib/__tests__/keepalive.test.ts index 1c7efc2f..ec630556 100644 --- a/apps/extension/src/lib/__tests__/keepalive.test.ts +++ b/apps/extension/src/lib/__tests__/keepalive.test.ts @@ -147,4 +147,13 @@ describe("startKeepalive", () => { await Promise.resolve(); expect(transport.connectCalls).toBe(1); }); + it("delegates reconnect policy to the controller callback", async () => { + const transport = makeTransport("disconnected"); + const requestConnect = vi.fn(); + const handle = startKeepalive({ transport, alarms: makeAlarms(), requestConnect }); + await handle.tickForTest(); + expect(requestConnect).toHaveBeenCalledOnce(); + expect(transport.connect).not.toHaveBeenCalled(); + handle.dispose(); + }); }); diff --git a/apps/extension/src/lib/connection-controller.ts b/apps/extension/src/lib/connection-controller.ts index 687fe8af..2da2c724 100644 --- a/apps/extension/src/lib/connection-controller.ts +++ b/apps/extension/src/lib/connection-controller.ts @@ -46,11 +46,17 @@ export class ConnectionController { private connectionEnabled = true; private listeners = new Set(); private lifecycleHooks: ConnectionLifecycleHooks = {}; - private disconnectRecovery: Promise | null = null; + private ready = false; + private preferenceChanged = false; + private transition: Promise | null = null; + private cleanupNeeded = false; + private hasConnected = false; + private configurationKey: string | null = null; + private pendingConfiguration: (() => void) | null = null; private connectionGeneration = 0; private handshakeAbort: AbortController | null = null; private handshakeRetryTimer: ReturnType | null = null; - private suppressDisconnectRecovery = false; + private intentionalDisconnects = 0; get isConnectionEnabled(): boolean { return this.connectionEnabled; @@ -85,69 +91,86 @@ export class ConnectionController { lifecycleHooks: ConnectionLifecycleHooks = {}, ): Promise { this.transport = transport; - this.connectionEnabled = connectionEnabled; + if (!this.preferenceChanged) this.connectionEnabled = connectionEnabled; this.lifecycleHooks = lifecycleHooks; this.instanceId = await getOrCreateInstanceId(); this.label = await getLabel(); - transport.onConnectionStateChange((s) => { - if (s === "disconnected") { - // An in-flight handshake belongs to the dead connection — cancel it so - // a late resolution can never clobber the next attempt. + transport.onConnectionStateChange((state) => { + if (state === "disconnected") { this.cancelHandshake(); this.handshake = null; - if (this.connectionEnabled) { - this.setState("disconnected"); - if (this.suppressDisconnectRecovery) { - // Deliberate disconnect from runHandshake (rejected peer or failed - // handshake bounce); the caller owns the reconnect policy there. - this.suppressDisconnectRecovery = false; - } else { - void this.recoverFromDisconnect(); - } + this.setState("disconnected"); + if (this.intentionalDisconnects > 0) return; + if (this.transition) { + // The transport schedules its retry after notifying listeners. Cancel + // it after that notification, including loss during disable cleanup. + void Promise.resolve() + .then(() => { + if (this.transition) return this.disconnectTransport(); + }) + .catch((err) => this.reportError(err)); + } else if (this.connectionEnabled && this.hasConnected) { + void this.teardown(false); } return; } - if (!this.connectionEnabled) return; - if (s === "connected") { + if (!this.connectionEnabled || this.transition) return; + if (state === "connected") { + this.hasConnected = true; this.startHandshake(browser); - return; + } else { + this.setState(state); } - this.setState(s); }); - transport.onMessage(() => { - // Real RPC dispatching is wired in M5 (tools/dispatcher.ts). - }); - - if (this.connectionEnabled) { - try { - await transport.connect(); - } catch (err) { - this.lastError = err instanceof Error ? err.message : String(err); - this.setState("disconnected"); - } - } else { - this.applyDisabledState(); - } + this.applyPendingConfiguration(); + this.ready = true; + this.fire(); + if (this.connectionEnabled) this.requestConnect(); + else await this.teardown(true); } async setConnectionEnabled(enabled: boolean): Promise { + this.preferenceChanged = true; if (this.connectionEnabled === enabled) return; this.connectionEnabled = enabled; + this.fire(); + if (!this.ready) return; + if (!enabled) await this.teardown(true); + else this.requestConnect(); + } + + /** Apply only the latest configuration, after the old sessions are cleaned up. */ + reconfigureTransport(key: string, apply: () => void): Promise { + if (key === this.configurationKey) return this.transition ?? Promise.resolve(); + this.configurationKey = key; + this.pendingConfiguration = apply; + if (!this.ready) return Promise.resolve(); + return this.teardown(false); + } - if (!enabled) { - await this.applyDisabledState(); + /** Shared entry point for startup, wake events, keepalive, and handshake retries. */ + requestConnect(): void { + if (!this.ready || !this.connectionEnabled || this.transition) return; + if (this.cleanupNeeded) { + void this.teardown(false); return; } - - this.fire(); - if (!this.transport) return; + if (this.handshakeRetryTimer || !this.transport || this.transport.state !== "disconnected") + return; + const generation = this.connectionGeneration; + // Never hold the lifecycle operation open while an unreachable endpoint is + // retrying: disabling or changing its configuration must remain possible. + const onError = (err: unknown) => { + if (generation !== this.connectionGeneration || !this.connectionEnabled || this.transition) + return; + this.reportError(err); + }; try { - await this.transport.connect(); + void this.transport.connect().catch(onError); } catch (err) { - this.lastError = err instanceof Error ? err.message : String(err); - this.setState("disconnected"); + onError(err); } } @@ -170,7 +193,7 @@ export class ConnectionController { signal: AbortSignal, ): Promise { if (!this.transport) return; - if (!this.connectionEnabled) return; + if (!this.connectionEnabled || this.transition) return; this.setState("connecting"); try { const outcome = await performHandshake( @@ -188,11 +211,8 @@ export class ConnectionController { if (verdict.kind === "rejected") { this.lastError = `version_too_old: ${verdict.reason}`; this.handshake = null; - // Rejected peers must not be auto-retried: mark this disconnect as - // deliberate so the listener skips unexpected-loss recovery. - this.suppressDisconnectRecovery = true; - await this.transport.disconnect().catch(() => {}); - this.setState("disconnected"); + // Preserve the rejection until the next explicit/wake-driven attempt. + await this.disconnectTransport(); return; } this.clearHandshakeRetry(); @@ -202,59 +222,77 @@ export class ConnectionController { if (generation !== this.connectionGeneration || signal.aborted || isAbortError(err)) return; this.handshake = null; this.lastError = err instanceof Error ? err.message : String(err); - // Bounce the half-open socket deliberately; the retry timer below owns - // the reconnect, so skip unexpected-loss recovery for this disconnect. - this.suppressDisconnectRecovery = true; - await this.transport.disconnect().catch(() => {}); - this.setState("disconnected"); - this.scheduleHandshakeRetry(browser); + const disconnect = this.disconnectTransport(); + const disconnectedGeneration = this.connectionGeneration; + await disconnect; + if ( + disconnectedGeneration !== this.connectionGeneration || + this.transition || + !this.connectionEnabled + ) + return; + this.scheduleHandshakeRetry(); } finally { if (generation === this.connectionGeneration) this.handshakeAbort = null; } } - private async applyDisabledState(): Promise { + /** Only finite teardown work is shared; the next connect is interruptible. */ + private teardown(cleanupBeforeDisconnect: boolean): Promise { + if (this.transition) return this.transition; + this.cleanupNeeded = true; this.cancelHandshake(); this.clearHandshakeRetry(); this.handshake = null; this.lastError = null; - await this.lifecycleHooks.beforeDisconnect?.(); - if (this.transport) { - await this.transport.disconnect().catch(() => {}); - } - const was = this.currentState; - this.setState("disconnected"); - if (was === "disconnected") this.fire(); - } - - private recoverFromDisconnect(): Promise { - if (this.disconnectRecovery) return this.disconnectRecovery; - const recovery = (async () => { - // WSTransport schedules its reconnect timer immediately after notifying - // state listeners. Yield once, then disconnect explicitly so that timer - // is cancelled before local session teardown begins. - await Promise.resolve(); - if (!this.connectionEnabled || !this.transport) return; - // Another path already reconnected while we yielded — nothing to do. - if (this.currentState !== "disconnected") return; - await this.transport.disconnect().catch(() => {}); - try { - await this.lifecycleHooks.onDisconnected?.(); - } catch (err) { - console.warn("[browser-skill] session cleanup after disconnect failed", err); - } - if (!this.connectionEnabled) return; + this.fire(); + // Install the gate before disconnect can synchronously notify listeners. + this.transition = Promise.resolve().then(async () => { try { - await this.transport.connect(); - } catch (err) { - this.lastError = err instanceof Error ? err.message : String(err); + if (cleanupBeforeDisconnect) { + // Preserve the existing safe user-disable ordering. + try { + await this.lifecycleHooks.beforeDisconnect?.(); + } finally { + await this.disconnectTransport(); + } + } else { + await this.disconnectTransport(); + await (this.lifecycleHooks.onDisconnected ?? this.lifecycleHooks.beforeDisconnect)?.(); + } + this.applyPendingConfiguration(); + this.cleanupNeeded = false; + this.hasConnected = false; this.setState("disconnected"); + } catch (err) { + // Keep the pending configuration and cleanup gate for a later retry. + this.reportError(err); + } finally { + this.transition = null; + if (!this.cleanupNeeded) this.requestConnect(); } - })(); - this.disconnectRecovery = recovery.finally(() => { - this.disconnectRecovery = null; }); - return this.disconnectRecovery; + return this.transition; + } + + private applyPendingConfiguration(): void { + this.pendingConfiguration?.(); + this.pendingConfiguration = null; + } + + private async disconnectTransport(): Promise { + this.intentionalDisconnects += 1; + try { + await this.transport?.disconnect(); + } finally { + this.intentionalDisconnects -= 1; + } + } + + private reportError(err: unknown): void { + this.lastError = err instanceof Error ? err.message : String(err); + this.setState("disconnected"); + this.fire(); } private cancelHandshake(): void { @@ -263,15 +301,11 @@ export class ConnectionController { this.handshakeAbort = null; } - private scheduleHandshakeRetry(browser: { name: string; version: string }): void { + private scheduleHandshakeRetry(): void { if (this.handshakeRetryTimer || !this.connectionEnabled) return; this.handshakeRetryTimer = setTimeout(() => { this.handshakeRetryTimer = null; - if (!this.connectionEnabled || !this.transport) return; - void this.transport.connect().catch((err) => { - this.lastError = err instanceof Error ? err.message : String(err); - this.setState("disconnected"); - }); + this.requestConnect(); }, HANDSHAKE_RETRY_DELAY_MS); } diff --git a/apps/extension/src/lib/daemon-port-preference.ts b/apps/extension/src/lib/daemon-port-preference.ts new file mode 100644 index 00000000..231a9cb2 --- /dev/null +++ b/apps/extension/src/lib/daemon-port-preference.ts @@ -0,0 +1,32 @@ +import { normalizeDaemonPort } from "@/transport/daemon-endpoint"; +import { getDaemonPort, STORAGE_KEYS } from "./instance-id"; + +/** Observe before reading so a late initial read cannot replace a newer change. */ +export function watchDaemonPort(onPort: (port: number) => void): { + ready: Promise; + dispose: () => void; +} { + let disposed = false; + let changed = false; + const onChanged = (changes: Record, areaName: string) => { + if (disposed || areaName !== "local" || !changes[STORAGE_KEYS.DAEMON_PORT]) return; + changed = true; + onPort(normalizeDaemonPort(changes[STORAGE_KEYS.DAEMON_PORT].newValue)); + }; + chrome.storage.onChanged.addListener(onChanged); + const ready = getDaemonPort().then( + (port) => { + if (!disposed && !changed) onPort(port); + }, + (err) => { + if (!disposed && !changed) throw err; + }, + ); + return { + ready, + dispose: () => { + disposed = true; + chrome.storage.onChanged.removeListener(onChanged); + }, + }; +} diff --git a/apps/extension/src/lib/keepalive.ts b/apps/extension/src/lib/keepalive.ts index f667c60d..5e7fc67f 100644 --- a/apps/extension/src/lib/keepalive.ts +++ b/apps/extension/src/lib/keepalive.ts @@ -54,6 +54,8 @@ export interface KeepaliveOptions { transport: Transport; /** When provided and returns false, skip reconnect attempts on alarm ticks. */ shouldConnect?: () => boolean; + /** Let the connection owner coordinate cleanup and retry policy. */ + requestConnect?: () => void | Promise; alarms?: AlarmsApi; /** Override the alarm name (tests). */ alarmName?: string; @@ -82,7 +84,8 @@ export function startKeepalive(options: KeepaliveOptions): KeepaliveHandle { if (options.shouldConnect && !options.shouldConnect()) return; if (options.transport.state === "connected") return; try { - await options.transport.connect(); + if (options.requestConnect) await options.requestConnect(); + else await options.transport.connect(); } catch (err) { console.debug("[bsk keepalive] connect attempt failed", err); } diff --git a/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts b/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts index 02590467..d09f6e6d 100644 --- a/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts +++ b/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts @@ -21,6 +21,9 @@ describe("daemon-endpoint", () => { expect(normalizeDaemonPort(65536)).toBe(DEFAULT_DAEMON_PORT); expect(normalizeDaemonPort("abc")).toBe(DEFAULT_DAEMON_PORT); expect(normalizeDaemonPort(1.5)).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort("53200abc")).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort("1.5")).toBe(DEFAULT_DAEMON_PORT); + expect(normalizeDaemonPort("1e3")).toBe(DEFAULT_DAEMON_PORT); }); it("normalizeDaemonPort accepts valid numbers and numeric strings", () => { diff --git a/apps/extension/src/transport/daemon-endpoint.ts b/apps/extension/src/transport/daemon-endpoint.ts index 6ca38224..3cc41489 100644 --- a/apps/extension/src/transport/daemon-endpoint.ts +++ b/apps/extension/src/transport/daemon-endpoint.ts @@ -23,8 +23,7 @@ export function resolveDaemonWsUrl(port: number): string { export function normalizeDaemonPort(raw: unknown): number { if (typeof raw === "number" && isValidPort(raw)) return raw; if (typeof raw === "string" && raw.trim() !== "") { - const parsed = Number.parseInt(raw.trim(), 10); - if (isValidPort(parsed)) return parsed; + return parseDaemonPortInput(raw) ?? DEFAULT_DAEMON_PORT; } return DEFAULT_DAEMON_PORT; } diff --git a/docs/architecture.md b/docs/architecture.md index e318fb63..a89e3d59 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -43,7 +43,7 @@ Key modules: ### bsk daemon (same binary: `bsk daemon`) -- Listens on loopback WebSocket (default **52800**, configurable in the extension popup) for extensions. +- Listens on loopback WebSocket (default **52800**, configurable with `bsk daemon start --port`) for extensions. The extension popup saves the matching connection port; saving ends existing sessions and reconnects when enabled. - Validates `Origin: chrome-extension://…` on handshake. - Maintains `browsers` (connected extensions) and `sessions` (Agent Window bindings). - **Per-session queue** serializes tool calls targeting one session. diff --git a/packages/i18n/src/locales/en-US/extension.json b/packages/i18n/src/locales/en-US/extension.json index b1b1cfd1..6de55955 100644 --- a/packages/i18n/src/locales/en-US/extension.json +++ b/packages/i18n/src/locales/en-US/extension.json @@ -27,9 +27,13 @@ "controlHintsToggleHint": "Show the status pill and orange glow while the Agent controls a page.", "controlHintsInfoLabel": "About control hints", "daemonPortLabel": "Connection port", - "daemonPortHint": "This port is used to connect to the local daemon. Do not change it unless necessary.", + "daemonPortHint": "Connect to the local daemon on this port. Saving a change ends current sessions and reconnects if the connection is enabled.", "daemonPortInfoLabel": "About connection port", "daemonPortInvalid": "Enter a port between 1 and 65535.", + "daemonPortSave": "Save port", + "daemonPortSaving": "Saving…", + "daemonPortReadFailed": "Could not load the port. Reopen this popup to retry.", + "daemonPortWriteFailed": "Could not save the port. Please try again.", "daemonUnreachable": "Can't connect. Check the daemon is running and the port matches.", "versionSkewWarning": "Extension protocol v{{extensionProtocol}}, CLI protocol v{{cliProtocol}}. Protocol versions differ — please upgrade.", "upgradeAvailable": "Upgradable", diff --git a/packages/i18n/src/locales/zh-CN/extension.json b/packages/i18n/src/locales/zh-CN/extension.json index b6043235..0165372c 100644 --- a/packages/i18n/src/locales/zh-CN/extension.json +++ b/packages/i18n/src/locales/zh-CN/extension.json @@ -27,9 +27,13 @@ "controlHintsToggleHint": "Agent 控制页面时显示提示条和橙色闪光。", "controlHintsInfoLabel": "控制提示说明", "daemonPortLabel": "连接端口", - "daemonPortHint": "扩展通过此端口连接本机 daemon。非必要请勿修改。", + "daemonPortHint": "通过此端口连接本机 daemon。保存修改会结束当前会话，并在连接开关开启时重新连接。", "daemonPortInfoLabel": "连接端口说明", "daemonPortInvalid": "请输入 1 到 65535 之间的端口号。", + "daemonPortSave": "保存端口", + "daemonPortSaving": "保存中…", + "daemonPortReadFailed": "无法读取端口，请重新打开弹窗重试。", + "daemonPortWriteFailed": "端口保存失败，请重试。", "daemonUnreachable": "无法连接，请确认 daemon 已启动且端口一致。", "versionSkewWarning": "扩展协议 v{{extensionProtocol}}，CLI 协议 v{{cliProtocol}}。协议版本不同，请及时升级。", "upgradeAvailable": "可升级", From 35c890fe044304ecb59ead168a12b8ac00f41ccf Mon Sep 17 00:00:00 2001 From: drakezhang Date: Thu, 10 Sep 2026 20:04:39 +0800 Subject: [PATCH 6/6] fix(extension): clarify daemon port settings Explain that the daemon must already listen on the selected port and that the extension setting does not change the local CLI configuration. Show the saved daemon WebSocket address beside the save button. --- apps/extension/src/entrypoints/popup/App.test.tsx | 2 +- apps/extension/src/entrypoints/popup/App.tsx | 12 ++++++++++-- .../src/entrypoints/popup/use-daemon-port.ts | 1 + packages/i18n/src/locales/en-US/extension.json | 2 +- packages/i18n/src/locales/ko-KR/extension.json | 2 +- packages/i18n/src/locales/zh-CN/extension.json | 2 +- 6 files changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/extension/src/entrypoints/popup/App.test.tsx b/apps/extension/src/entrypoints/popup/App.test.tsx index ebae7452..400e709a 100644 --- a/apps/extension/src/entrypoints/popup/App.test.tsx +++ b/apps/extension/src/entrypoints/popup/App.test.tsx @@ -575,7 +575,7 @@ describe("daemon port input", () => { const info = await screen.findByRole("button", { name: "连接端口说明" }); expect(info).toBeTruthy(); const tooltip = screen.getByText( - "通过此端口连接本机 daemon。保存修改会结束当前会话，并在连接开关开启时重新连接。", + "通过此端口连接本机 daemon，请先启动 daemon 并让其监听此端口。此设置仅更改扩展的连接地址，不会修改本地 CLI 配置。保存修改会结束当前会话，并在连接开关开启时重新连接。", ); expect(tooltip.getAttribute("role")).toBe("tooltip"); expect(tooltip.className).toContain("opacity-0"); diff --git a/apps/extension/src/entrypoints/popup/App.tsx b/apps/extension/src/entrypoints/popup/App.tsx index 44b5f56d..29ee388e 100644 --- a/apps/extension/src/entrypoints/popup/App.tsx +++ b/apps/extension/src/entrypoints/popup/App.tsx @@ -8,6 +8,7 @@ import { RiInformationLine, } from "@remixicon/react"; import { type ChangeEvent, useEffect, useState } from "react"; +import { resolveDaemonWsUrl } from "@/transport/daemon-endpoint"; import { PROTOCOL_VERSION } from "@/transport/handshake"; import functionIconUrl from "../../../assets/function.svg"; import { ConnectionStatusIndicator } from "./connection-status-indicator"; @@ -43,6 +44,7 @@ export function App() { const { snapshot, statusState, setConnectionEnabled } = useConnectionState(); const [controlHintsHidden, setControlHintsHidden] = useControlHintsHidden(); const { + savedPort: daemonPort, draft: daemonPortDraft, setDraft: setDaemonPortDraft, commit: commitDaemonPort, @@ -322,12 +324,18 @@ export function App() { data-slot="popup-daemon-port-input" />
-
+
+ + {daemonPort === null ? "—" : resolveDaemonWsUrl(daemonPort)} +