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 e9ee4fe6..72819fa4 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -1,6 +1,7 @@ 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, @@ -43,6 +44,7 @@ import { } from "@/tools/record"; import { chromeTabsApi } from "@/tools/shared"; import { chromeTabMutationApi } from "@/tools/tabs"; +import { resolveDaemonWsUrl } from "@/transport/daemon-endpoint"; import { detectBrowserMeta } from "@/transport/handshake"; import type { Transport } from "@/transport/transport"; import { WSTransport } from "@/transport/ws-transport"; @@ -61,6 +63,14 @@ export default defineBackground(() => { let overlayGeneration = 0; const controlModes = new Map(); + const daemonPort = watchDaemonPort((port) => { + const url = resolveDaemonWsUrl(port); + void controller.reconfigureTransport(url, () => { + transport.setUrl(url); + }); + }); + let preferenceWrites = Promise.resolve(); + function setControlMode(sessionId: string, mode: OverlayMode): void { if (controlModes.get(sessionId) === mode) return; controlModes.set(sessionId, mode); @@ -285,7 +295,7 @@ export default defineBackground(() => { // the BrowserSkill connection. startKeepalive({ transport, - shouldConnect: () => controller.isConnectionEnabled, + requestConnect: () => controller.requestConnect(), }); // Application-level heartbeat (Chrome 116+): while the post-handshake @@ -307,13 +317,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); } @@ -328,20 +332,18 @@ export default defineBackground(() => { } void (async () => { - const connectionEnabled = await getConnectionEnabled(); + 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); @@ -399,15 +401,14 @@ 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) - .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 6def1ff7..400e709a 100644 --- a/apps/extension/src/entrypoints/popup/App.test.tsx +++ b/apps/extension/src/entrypoints/popup/App.test.tsx @@ -3,6 +3,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, PROTOCOL_VERSION } from "@/transport/handshake"; import { App } from "./App"; import { useConnectionState } from "./use-connection-state"; @@ -68,9 +69,34 @@ describe("App", () => { render(); expect(screen.getByText("未连接")).toBeTruthy(); + expect(screen.getByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeTruthy(); expect(screen.queryByText("请先打开 BrowserSkill。")).toBeNull(); }); + it("keeps the connection switch usable and shows protocol errors when disconnected", () => { + mockUseConnectionState.mockReturnValue({ + snapshot: { + ...baseSnapshot, + lastError: "version_too_old: protocol-major mismatch", + }, + statusState: "disconnected", + setLabel, + setConnectionEnabled, + }); + + render(); + + expect(screen.getByText("未连接")).toBeTruthy(); + expect(screen.queryByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeNull(); + expect(screen.queryByText("端口不匹配")).toBeNull(); + expect( + screen.getByRole("switch", { name: "BrowserSkill 连接" }).getAttribute("aria-checked"), + ).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", () => { render(); @@ -138,6 +164,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 连接" }); @@ -145,6 +178,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 连接" })); @@ -162,6 +202,7 @@ describe("App", () => { render(); expect(screen.getByText("连接已关闭")).toBeTruthy(); + expect(screen.queryByText("无法连接，请确认 daemon 已启动且端口一致。")).toBeNull(); expect( screen.getByRole("switch", { name: "BrowserSkill 连接" }).getAttribute("aria-checked"), ).toBe("false"); @@ -402,14 +443,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(); @@ -422,3 +469,115 @@ 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 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.click(screen.getByRole("button", { name: "保存端口" })); + + await waitFor(() => expect(store[STORAGE_KEYS.DAEMON_PORT]).toBe(53200)); + expect((input as HTMLInputElement).value).toBe("53200"); + }); + + 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" }); + + await waitFor(() => 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: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); + fireEvent.change(input, { target: { value: "abc" } }); + fireEvent.click(screen.getByRole("button", { name: "保存端口" })); + + 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: "连接端口" }); + await waitFor(() => expect((input as HTMLInputElement).disabled).toBe(false)); + fireEvent.change(input, { target: { value: "" } }); + fireEvent.click(screen.getByRole("button", { name: "保存端口" })); + + await waitFor(() => 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，请先启动 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 cc9287b1..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"; @@ -15,6 +16,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 +43,17 @@ export function App() { const { t } = useTranslation("extension"); const { snapshot, statusState, setConnectionEnabled } = useConnectionState(); const [controlHintsHidden, setControlHintsHidden] = useControlHintsHidden(); + const { + savedPort: daemonPort, + draft: daemonPortDraft, + 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); const [purposeDraft, setPurposeDraft] = useState(""); @@ -79,6 +92,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 ?? "—"; @@ -205,6 +219,14 @@ export function App() { /> + {isDisconnected && !snapshot.lastError && ( +

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

+ )} {isSkewed && (

+

+
+ + + + + + {t("popup.daemonPortHint")} + + + + ) => + setDaemonPortDraft(event.target.value) + } + disabled={!daemonPortLoaded || daemonPortSaving} + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + 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" + aria-invalid={daemonPortInvalid || undefined} + data-slot="popup-daemon-port-input" + /> +
+
+ + {daemonPort === null ? "—" : resolveDaemonWsUrl(daemonPort)} + + +
+ {daemonPortError && ( +

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

+ )} + {daemonPortInvalid && ( +

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

+ )} +
+ {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 new file mode 100644 index 00000000..0f83182f --- /dev/null +++ b/apps/extension/src/entrypoints/popup/use-daemon-port.ts @@ -0,0 +1,116 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { watchDaemonPort } from "@/lib/daemon-port-preference"; +import { setDaemonPort } from "@/lib/instance-id"; +import { parseDaemonPortInput } from "@/transport/daemon-endpoint"; + +type PortState = { + savedPort: number | null; + // null means no local edit: show the latest persisted value. + draft: string | null; + saving: boolean; + invalid: boolean; + 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(() => { + 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 () => { + mounted.current = false; + preference.dispose(); + }; + }, []); + + 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) { + setState((s) => ({ ...s, invalid: true })); + return; + } + 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) => { + if (savingRef.current) return; + setState((s) => + s.savedPort === null + ? s + : { + ...s, + draft: value === String(s.savedPort) ? null : value, + invalid: false, + error: null, + }, + ); + }, []); + + return { + savedPort: state.savedPort, + 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..ebef9aea --- /dev/null +++ b/apps/extension/src/lib/__tests__/connection-reconfiguration.test.ts @@ -0,0 +1,303 @@ +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); + expect(s.controller.snapshot().state).toBe("disconnected"); + 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__/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/__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..7a7d1cfb 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,78 @@ 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 +302,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/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/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/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..d09f6e6d --- /dev/null +++ b/apps/extension/src/transport/__tests__/daemon-endpoint.test.ts @@ -0,0 +1,46 @@ +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); + 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", () => { + 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..3cc41489 --- /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() !== "") { + return parseDaemonPortInput(raw) ?? DEFAULT_DAEMON_PORT; + } + 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; + if (!/^\d+$/.test(trimmed)) return null; + 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 d7887dbc..8ed30143 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 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 62851c84..fe51cc8d 100644 --- a/packages/i18n/src/locales/en-US/extension.json +++ b/packages/i18n/src/locales/en-US/extension.json @@ -26,6 +26,15 @@ "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": "Connect to the local daemon on this port. Start the daemon and have it listen on this port first. This setting only changes the extension's connection address, not the local CLI configuration. 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", "launcher": { diff --git a/packages/i18n/src/locales/ko-KR/extension.json b/packages/i18n/src/locales/ko-KR/extension.json index 32520cc1..c6870bca 100644 --- a/packages/i18n/src/locales/ko-KR/extension.json +++ b/packages/i18n/src/locales/ko-KR/extension.json @@ -26,6 +26,15 @@ "controlHintsToggleTitle": "제어 표시", "controlHintsToggleHint": "에이전트가 페이지를 제어하는 동안 상태 표시와 주황색 테두리를 표시합니다.", "controlHintsInfoLabel": "제어 표시에 관하여", + "daemonPortLabel": "연결 포트", + "daemonPortHint": "이 포트를 통해 로컬 daemon에 연결합니다. 먼저 daemon을 시작하고 이 포트에서 수신하도록 설정하세요. 이 설정은 확장 프로그램의 연결 주소만 변경하며 로컬 CLI 설정은 변경하지 않습니다. 변경 사항을 저장하면 현재 세션이 종료되며, 연결이 활성화되어 있으면 다시 연결합니다.", + "daemonPortInfoLabel": "연결 포트 안내", + "daemonPortInvalid": "1~65535 사이의 포트 번호를 입력하세요.", + "daemonPortSave": "포트 저장", + "daemonPortSaving": "저장 중…", + "daemonPortReadFailed": "포트를 불러올 수 없습니다. 팝업을 다시 열어 재시도하세요.", + "daemonPortWriteFailed": "포트를 저장하지 못했습니다. 다시 시도하세요.", + "daemonUnreachable": "연결할 수 없습니다. daemon이 실행 중인지, 포트가 일치하는지 확인하세요.", "versionSkewWarning": "확장 프로그램 프로토콜 v{{extensionProtocol}}, CLI 프로토콜 v{{cliProtocol}}. 프로토콜 버전이 다릅니다. 업그레이드해 주세요.", "upgradeAvailable": "업그레이드 가능", "launcher": { diff --git a/packages/i18n/src/locales/zh-CN/extension.json b/packages/i18n/src/locales/zh-CN/extension.json index 338d5df1..46c56fa5 100644 --- a/packages/i18n/src/locales/zh-CN/extension.json +++ b/packages/i18n/src/locales/zh-CN/extension.json @@ -26,6 +26,15 @@ "controlHintsToggleTitle": "控制提示", "controlHintsToggleHint": "Agent 控制页面时显示提示条和橙色闪光。", "controlHintsInfoLabel": "控制提示说明", + "daemonPortLabel": "连接端口", + "daemonPortHint": "通过此端口连接本机 daemon，请先启动 daemon 并让其监听此端口。此设置仅更改扩展的连接地址，不会修改本地 CLI 配置。保存修改会结束当前会话，并在连接开关开启时重新连接。", + "daemonPortInfoLabel": "连接端口说明", + "daemonPortInvalid": "请输入 1 到 65535 之间的端口号。", + "daemonPortSave": "保存端口", + "daemonPortSaving": "保存中…", + "daemonPortReadFailed": "无法读取端口，请重新打开弹窗重试。", + "daemonPortWriteFailed": "端口保存失败，请重试。", + "daemonUnreachable": "无法连接，请确认 daemon 已启动且端口一致。", "versionSkewWarning": "扩展协议 v{{extensionProtocol}}，CLI 协议 v{{cliProtocol}}。协议版本不同，请及时升级。", "upgradeAvailable": "可升级", "launcher": {