From 388c1283bcde1a50d1a3afc4f16b76980b1e74da Mon Sep 17 00:00:00 2001 From: drakezhang Date: Tue, 1 Sep 2026 21:03:22 +0800 Subject: [PATCH] fix: clarify invalid selectors and dedupe diagnostics --- apps/extension/src/entrypoints/background.ts | 2 +- .../src/lib/__tests__/diagnostics.test.ts | 24 ++++++++++++++++++ apps/extension/src/lib/diagnostics.ts | 25 +++++++++++++------ .../src/tools/__tests__/interaction.test.ts | 24 ++++++++++++++++++ apps/extension/src/tools/interaction.ts | 16 ++++++++++++ apps/extension/src/transport/types.ts | 1 + apps/extension/wxt.config.ts | 2 +- crates/bsk-cli/src/cli/error.rs | 17 +++++++++++++ crates/bsk-cli/src/cli/render_error.rs | 8 ++++++ crates/bsk-cli/src/daemon/start.rs | 2 +- 10 files changed, 111 insertions(+), 10 deletions(-) diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 4d93d16..1a2acb3 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -49,7 +49,7 @@ import { WSTransport } from "@/transport/ws-transport"; export default defineBackground(() => { const diagnostics = new DiagnosticLogger(); diagnostics.record("extension.worker.started", { - diagnostic_build: "lumi-session-loss-v1", + diagnostic_build: "lumi-session-loss-v2", extension_version: __BSK_EXT_VERSION__, extension_runtime_id: chrome.runtime.id, daemon_ws_url: __BSK_DAEMON_WS_URL__, diff --git a/apps/extension/src/lib/__tests__/diagnostics.test.ts b/apps/extension/src/lib/__tests__/diagnostics.test.ts index ff9d39c..1673592 100644 --- a/apps/extension/src/lib/__tests__/diagnostics.test.ts +++ b/apps/extension/src/lib/__tests__/diagnostics.test.ts @@ -58,4 +58,28 @@ describe("DiagnosticLogger", () => { { event: "session.stop.requested", fields: { session_id: "abcd" } }, ]); }); + + it("does not resend an acknowledged event when send records a new event", async () => { + const storage = new MemoryStorage(); + const batches: Array<{ payload?: { entries?: Array<{ event: string }> } }> = []; + const logger = new DiagnosticLogger(storage); + logger.record("dispatcher.rpc.received", { rpc_id: "one" }); + await logger.flush(); + logger.bindTransport((frame) => { + batches.push(frame as { payload?: { entries?: Array<{ event: string }> } }); + if (batches.length === 1) { + logger.record("dispatcher.rpc.response_sent", { rpc_id: "one" }); + } + }); + + logger.setTransportReady(true); + await logger.flush(); + await logger.flush(); + + expect(batches.map((batch) => batch.payload?.entries?.map((entry) => entry.event))).toEqual([ + ["dispatcher.rpc.received"], + ["dispatcher.rpc.response_sent"], + ]); + expect(storage.value.bsk_diagnostic_events_v1).toEqual([]); + }); }); diff --git a/apps/extension/src/lib/diagnostics.ts b/apps/extension/src/lib/diagnostics.ts index 10fede8..91779b2 100644 --- a/apps/extension/src/lib/diagnostics.ts +++ b/apps/extension/src/lib/diagnostics.ts @@ -145,13 +145,24 @@ export class DiagnosticLogger { return; } const sentIds = new Set(entries.map((entry) => entry.id)); - const current = await this.storage.get(STORAGE_KEY); - const currentEntries = Array.isArray(current[STORAGE_KEY]) - ? (current[STORAGE_KEY] as DiagnosticEntry[]) - : []; - await this.storage.set({ - [STORAGE_KEY]: currentEntries.filter((entry) => !sentIds.has(entry.id)), - }); + // Serialize acknowledgement behind every record write that may have + // started while send() was in progress. Without this, a concurrent + // record can read the pre-ack queue and write an already-sent entry + // back into storage, causing the next batch to duplicate it. + this.storageQueue = this.storageQueue + .then(async () => { + const current = await this.storage?.get(STORAGE_KEY); + const currentEntries = Array.isArray(current?.[STORAGE_KEY]) + ? (current[STORAGE_KEY] as DiagnosticEntry[]) + : []; + await this.storage?.set({ + [STORAGE_KEY]: currentEntries.filter((entry) => !sentIds.has(entry.id)), + }); + }) + .catch((error) => { + console.warn("[bsk diagnostic] failed to acknowledge event batch", error); + }); + await this.storageQueue; })().catch((error) => { console.warn("[bsk diagnostic] flush failed", error); }); diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index b78877a..e9b27f8 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -456,6 +456,30 @@ describe("handleClick", () => { expect(res).toMatchObject({ code: "not_found", data: { reason: "selector_not_found" } }); }); + it("returns invalid_selector when Chrome rejects non-CSS selector syntax", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const fake = makeFakeCdp({ + "DOM.getDocument": () => ({ root: { nodeId: 1 } }), + "DOM.querySelector": () => { + throw new Error('{"code":-32000,"message":"DOM Error while querying"}'); + }, + }); + const res = await handleClick( + sm, + { session_id: "aa11", selector: "text=下午 12~18点" }, + { cdp: fake.cdp, tabsApi: fake.tabsApi }, + ); + expect(res).toMatchObject({ + code: "invalid_params", + message: "selector is not valid CSS: text=下午 12~18点", + data: { + reason: "invalid_selector", + selector: "text=下午 12~18点", + }, + }); + }); + it("respects the AbortSignal", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); const ctx = await sm.start("aa11"); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 182cabd..4aecfeb 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -106,6 +106,14 @@ function isAbortLikeError(err: unknown): boolean { return err instanceof DOMException && err.name === "AbortError"; } +function isInvalidCssSelectorError(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return ( + message.includes("DOM Error while querying") || + /invalid selector|not a valid selector|querySelector.*failed/i.test(message) + ); +} + async function wait(ms: number, signal?: AbortSignal): Promise { if (signal?.aborted) throw new DOMException("aborted", "AbortError"); await new Promise((resolve, reject) => { @@ -208,6 +216,14 @@ async function resolveBackendNode( } return { backendNodeId, cdpTarget: { tabId: target.tabId }, usedSelector: params.selector }; } catch (err) { + if (isInvalidCssSelectorError(err)) { + return rpcError( + "invalid_params", + "invalid_selector", + `selector is not valid CSS: ${params.selector}`, + { selector: params.selector }, + ); + } return { code: "cdp_failed", message: err instanceof Error ? err.message : String(err), diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index dc60fe2..b6dcd4f 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -24,6 +24,7 @@ export type RpcErrorReason = | "agent_window_scope" | "element_not_visible" | "ref_not_found" + | "invalid_selector" | "selector_not_found" | "target_not_fillable" | "target_not_select" diff --git a/apps/extension/wxt.config.ts b/apps/extension/wxt.config.ts index 47f3e79..c7e04b6 100644 --- a/apps/extension/wxt.config.ts +++ b/apps/extension/wxt.config.ts @@ -23,7 +23,7 @@ export default defineConfig({ modules: ["@wxt-dev/module-react"], manifest: { name: "BrowserSkill (Lumi Diagnostics)", - version_name: `${EXTENSION_VERSION}-lumi-session-loss-v1`, + version_name: `${EXTENSION_VERSION}-lumi-session-loss-v2`, description: "Let AI agents use your logged-in browser in a separate Agent Window—without interrupting your work. Powered by the bsk CLI.", // Flat debugger sessions are required to address out-of-process iframes. diff --git a/crates/bsk-cli/src/cli/error.rs b/crates/bsk-cli/src/cli/error.rs index f8a7b1e..58c2540 100644 --- a/crates/bsk-cli/src/cli/error.rs +++ b/crates/bsk-cli/src/cli/error.rs @@ -497,4 +497,21 @@ mod tests { .contains("CSS selector") ); } + + #[test] + fn invalid_selector_explains_css_contract_in_json() { + let cli = CliError::from_rpc(RpcError { + code: ErrorCode::InvalidParams, + message: "selector is not valid CSS: text=submit".into(), + data: Some(serde_json::json!({ "reason": "invalid_selector" })), + }); + let exit = cli.exit_code(); + let hint = hint_for(&cli, render_info_for(&cli).as_ref()); + let json = json_error_string(&cli, exit, hint); + let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); + let hint = parsed.get("hint").and_then(|v| v.as_str()).unwrap(); + assert!(hint.contains("CSS selector")); + assert!(hint.contains("text=")); + assert!(hint.contains("@eN")); + } } diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index db52f47..d6df269 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -41,6 +41,7 @@ pub mod reason { pub const AGENT_WINDOW_SCOPE: &str = "agent_window_scope"; pub const ELEMENT_NOT_VISIBLE: &str = "element_not_visible"; pub const REF_NOT_FOUND: &str = "ref_not_found"; + pub const INVALID_SELECTOR: &str = "invalid_selector"; pub const SELECTOR_NOT_FOUND: &str = "selector_not_found"; pub const TARGET_NOT_FILLABLE: &str = "target_not_fillable"; pub const TARGET_NOT_SELECT: &str = "target_not_select"; @@ -221,6 +222,13 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend hint: Some("verify the CSS selector or wait for the element to appear before retrying"), exit_code: base.exit_code, }, + (ErrorCode::InvalidParams, reason::INVALID_SELECTOR) => RenderInfo { + summary: "selector is not valid CSS", + hint: Some( + "use a standard CSS selector or a fresh @eN ref; Playwright text= selectors and XPath are not supported", + ), + exit_code: base.exit_code, + }, (ErrorCode::InvalidParams, reason::TARGET_NOT_FILLABLE) => RenderInfo { summary: "target element is not fillable", hint: Some( diff --git a/crates/bsk-cli/src/daemon/start.rs b/crates/bsk-cli/src/daemon/start.rs index 35a8ca0..bc2b7bd 100644 --- a/crates/bsk-cli/src/daemon/start.rs +++ b/crates/bsk-cli/src/daemon/start.rs @@ -40,7 +40,7 @@ pub(crate) const DAEMONIZED_ENV: &str = "BSK_DAEMONIZED"; /// daemon lock, IPC socket, and WS port — before taking over. pub(crate) const DAEMON_REPLACEMENT_WAIT_ENV: &str = "BSK_DAEMON_REPLACES_PID"; -const LUMI_DIAGNOSTIC_BUILD: &str = "lumi-session-loss-v1"; +const LUMI_DIAGNOSTIC_BUILD: &str = "lumi-session-loss-v2"; /// Concrete daemon configuration resolved from CLI flags / defaults. #[derive(Debug, Clone)]