diff --git a/CHANGELOG.md b/CHANGELOG.md index b0900e71..126bf47f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ Starting from 0.2.0, CLI / Extension / DSH Plugin share the same version number. - [Scroll-to element primitive](docs/scroll-to.md) across CLI, Extension and DSH Plugin, with ancestor-clipped visible bounds, iframe support and cooperative cancellation +- Extension: virtual agent cursor that glides to the target and ripples on click before the + real input is dispatched, plus a current-action line on the control pill naming the tool + and its target +- User takeover across CLI, Extension and daemon: "Take over" / "Return to agent" buttons with + an optional note for the agent, agent input blocked while the user holds control, and + `bsk session status` / `bsk session wait-control` for the agent to observe and wait out the hold ## [0.2.1] - 2026-09-09 diff --git a/apps/extension/src/content/ControlOverlay.tsx b/apps/extension/src/content/ControlOverlay.tsx index 02544b4e..162e01bb 100644 --- a/apps/extension/src/content/ControlOverlay.tsx +++ b/apps/extension/src/content/ControlOverlay.tsx @@ -1,24 +1,83 @@ import { useTranslation } from "@browser-skill/i18n/react"; import { RiStopCircleLine } from "@remixicon/react"; import { useEffect, useRef, useState } from "react"; +import type { OverlayMode } from "@/lib/overlay-bridge"; import logoUrl from "../../assets/logo.png"; +/** The tool the agent is running right now, as narrated by the pill. */ +export interface ControlAction { + /** Wire tool name, e.g. `tool.click`. */ + tool: string; + /** `@ref`, selector, url or key, already truncated by the background. */ + target?: string; +} + export interface ControlOverlayProps { visible: boolean; + /** + * Authoritative control mode. `paused` renders the "you are in control" + * pill: no blocker, no glow, a note field and a return button. + */ + mode: OverlayMode; interrupting: boolean; automationBypass: boolean; + /** Current agent action, or null while idle. */ + currentAction?: ControlAction | null; onInterrupt: () => void; + onReturnControl: (note: string) => void; +} + +/** + * Map a wire tool name to its `controlOverlay.action.*` suffix. Anything we do + * not narrate falls back to the generic "working" copy. + */ +const ACTION_KEY_BY_TOOL: Record = { + "tool.click": "click", + "tool.dblclick": "click", + "tool.hover": "hover", + "tool.fill": "fill", + "tool.press": "press", + "tool.navigate": "navigate", + "tool.navigate_back": "navigate", + "tool.navigate_forward": "navigate", + "tool.scroll": "scroll", + "tool.scroll_to": "scroll", + "tool.wheel": "scroll", + "tool.select": "select", + "tool.upload": "upload", + "tool.download": "download", + "tool.evaluate": "evaluate", + "tool.reload": "reload", +}; + +/** + * Cap on the note the user can hand back with the page. The daemon cuts + * anything longer, and the note lives in its interrupt registry until a waiter + * consumes it, so keep the field well inside that budget. + */ +export const NOTE_MAX_CHARS = 1000; + +const PILL_FONT = + '-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif'; + +function actionKey(tool: string): string { + return ACTION_KEY_BY_TOOL[tool] ?? "working"; } export function ControlOverlay({ visible, + mode, interrupting, automationBypass, + currentAction, onInterrupt, + onReturnControl, }: ControlOverlayProps) { const { t } = useTranslation("extension"); const [show, setShow] = useState(false); + const [note, setNote] = useState(""); const blockerRef = useRef(null); + const paused = mode === "paused"; useEffect(() => { if (visible) { @@ -28,6 +87,11 @@ export function ControlOverlay({ setShow(false); }, [visible]); + // A fresh hold starts with an empty note rather than the previous one. + useEffect(() => { + if (!paused) setNote(""); + }, [paused]); + useEffect(() => { const blocker = blockerRef.current; if (!blocker) return; @@ -45,7 +109,8 @@ export function ControlOverlay({ }, [automationBypass]); useEffect(() => { - if (!visible || automationBypass) return; + // The paused pill never blocks the page — the user is operating it. + if (!visible || paused || automationBypass) return; const stopScroll = (event: WheelEvent | TouchEvent) => { event.preventDefault(); event.stopPropagation(); @@ -56,11 +121,99 @@ export function ControlOverlay({ window.removeEventListener("wheel", stopScroll, { capture: true }); window.removeEventListener("touchmove", stopScroll, { capture: true }); }; - }, [visible, automationBypass]); + }, [visible, paused, automationBypass]); if (!visible) return null; const pointerEvents = automationBypass ? "none" : "auto"; + const action = currentAction ?? null; + + if (paused) { + return ( +
+ browser-skill + + {t("controlOverlay.pausedStatus")} + + setNote(event.target.value)} + onKeyDown={(event) => { + if (event.key !== "Enter") return; + event.preventDefault(); + onReturnControl(note); + }} + style={{ + width: 200, + border: "1px solid #e5e7eb", + borderRadius: 9999, + padding: "8px 14px", + fontSize: 14, + color: "#333", + outline: "none", + fontFamily: PILL_FONT, + }} + /> + +
+ ); + } return ( <> @@ -114,6 +267,7 @@ export function ControlOverlay({
- {t("controlOverlay.status")} + + {t("controlOverlay.status")} + + + {action + ? `${t(`controlOverlay.action.${actionKey(action.tool)}` as "controlOverlay.action.working")}${ + action.target ? ` ${action.target}` : "" + }` + : t("controlOverlay.action.idle")} +
diff --git a/apps/extension/src/content/CursorOverlay.tsx b/apps/extension/src/content/CursorOverlay.tsx new file mode 100644 index 00000000..21c76d0a --- /dev/null +++ b/apps/extension/src/content/CursorOverlay.tsx @@ -0,0 +1,150 @@ +import { useEffect, useState } from "react"; + +/** + * Cosmetic in-page agent cursor. Rendered by the content script inside the + * existing `browser-skill-overlay` shadow tree, so it inherits the host's + * `pointer-events: none` and is removed from compositing while a capture is + * suppressed (`data-bsk-capture-hidden`). + * + * The background sends one {@link CursorState} per action: `move` glides the + * arrow to the target before the real CDP event fires, `click` adds a ripple + * at the same point. When the agent goes quiet for + * {@link IDLE_HIDE_MS} the cursor fades out; a `null` state (session reset / + * explicit hide) removes it immediately. + */ + +export interface CursorRipple { + /** Monotonic per-tab id; each new id replays the ripple animation. */ + id: number; + button: string; +} + +export interface CursorState { + /** Viewport CSS pixel, same space as `Input.dispatchMouseEvent`. */ + x: number; + y: number; + /** Glide time for this move; 0 means jump straight to the point. */ + durationMs: number; + label?: string; + ripple?: CursorRipple; +} + +export interface CursorOverlayProps { + state: CursorState | null; +} + +const IDLE_HIDE_MS = 2500; +const FADE_MS = 300; +const RIPPLE_MS = 400; +const RIPPLE_SIZE_PX = 40; +const MAX_LABEL_CHARS = 40; + +function rippleColor(button: string): string { + if (button === "right") return "59,130,246"; + if (button === "middle") return "168,85,247"; + return "249,115,22"; +} + +function truncateLabel(label: string): string { + return label.length > MAX_LABEL_CHARS ? `${label.slice(0, MAX_LABEL_CHARS)}…` : label; +} + +export function CursorOverlay({ state }: CursorOverlayProps) { + const [visible, setVisible] = useState(true); + + // `state` is a fresh object per bridge message, so the idle timer restarts + // on every move/click. + useEffect(() => { + if (!state) return; + setVisible(true); + const timer = window.setTimeout(() => setVisible(false), IDLE_HIDE_MS); + return () => window.clearTimeout(timer); + }, [state]); + + if (!state) return null; + + const ripple = state.ripple; + + return ( +
+ + + + + {state.label ? ( +
+ {truncateLabel(state.label)} +
+ ) : null} + + {ripple ? ( +
+ ) : null} +
+ ); +} diff --git a/apps/extension/src/content/__tests__/ControlOverlay.test.tsx b/apps/extension/src/content/__tests__/ControlOverlay.test.tsx index 0a3e4f74..e1711874 100644 --- a/apps/extension/src/content/__tests__/ControlOverlay.test.tsx +++ b/apps/extension/src/content/__tests__/ControlOverlay.test.tsx @@ -1,21 +1,25 @@ -import { cleanup, render } from "@testing-library/react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { ControlOverlay } from "../ControlOverlay"; +import { ControlOverlay, NOTE_MAX_CHARS } from "../ControlOverlay"; + +function baseProps() { + return { + visible: true, + mode: "control" as const, + interrupting: false, + automationBypass: false, + onInterrupt: vi.fn(), + onReturnControl: vi.fn(), + }; +} describe("ControlOverlay", () => { afterEach(() => { cleanup(); }); - it("keeps page blocker none under automationBypass but Interrupt stays clickable", () => { - const { container } = render( - {}} - />, - ); + it("keeps page blocker none under automationBypass but Take over stays clickable", () => { + const { container } = render(); const blocker = container.querySelector("[data-slot='control-overlay-blocker']"); expect(blocker).toBeTruthy(); @@ -31,34 +35,100 @@ describe("ControlOverlay", () => { }); it("uses pointer-events auto on blocker when automationBypass is false", () => { - const { container } = render( - {}} - />, - ); + const { container } = render(); const blocker = container.querySelector("[data-slot='control-overlay-blocker']"); expect(blocker).toBeTruthy(); expect((blocker as HTMLElement).style.pointerEvents).toBe("auto"); }); - it("calls onInterrupt from the stop button", () => { - const onInterrupt = vi.fn(); + it("calls onInterrupt from the take-over button", () => { + const props = baseProps(); + const { container } = render(); + + const stopBtn = container.querySelector("[data-slot='control-overlay-stop-all']"); + fireEvent.click(stopBtn as HTMLButtonElement); + + expect(props.onInterrupt).toHaveBeenCalledTimes(1); + }); + + it("shows the idle action line when the agent has nothing in flight", () => { + const { container } = render(); + + const action = container.querySelector("[data-slot='control-overlay-action']"); + expect(action?.textContent).toBe("等待下一步指令"); + }); + + it.each([ + ["tool.click", "@e3", "点击 @e3"], + ["tool.hover", "#js-link-box-pt", "悬停 #js-link-box-pt"], + ["tool.fill", "input#searchInput", "输入 input#searchInput"], + ["tool.navigate", "https://example.test/x", "导航 https://example.test/x"], + ["tool.press", "Enter", "按键 Enter"], + ["tool.wheel", undefined, "滚动"], + ["tool.unknown_thing", undefined, "正在执行…"], + ])("renders the action line for %s", (tool, target, expected) => { const { container } = render( - , + , ); - const stopBtn = container.querySelector("[data-slot='control-overlay-stop-all']"); - (stopBtn as HTMLButtonElement).click(); + const action = container.querySelector("[data-slot='control-overlay-action']"); + expect(action?.textContent).toBe(expected); + expect((action as HTMLElement).style.fontSize).toBe("13px"); + expect((action as HTMLElement).style.color).toBe("#6b7280"); + }); + + it("renders the paused pill with a note field and a return button, and no blocker", () => { + const { container } = render(); + + expect(container.querySelector("[data-slot='control-overlay-blocker']")).toBeNull(); + expect(container.querySelector("[data-slot='control-overlay']")).toBeNull(); + expect( + (container.querySelector("[data-slot='control-overlay-pill']") as HTMLElement).dataset.mode, + ).toBe("paused"); + + const note = container.querySelector("[data-slot='control-overlay-note']"); + expect(note).toBeTruthy(); + expect((note as HTMLInputElement).placeholder).toBe("给 Agent 的备注（可选）"); + + expect(container.querySelector("[data-slot='control-overlay-return']")).toBeTruthy(); + expect(container.querySelector("[data-slot='control-overlay-stop-all']")).toBeNull(); + }); + + it("calls onReturnControl with the note when the return button is clicked", () => { + const props = baseProps(); + const { container } = render(); + + const note = container.querySelector("[data-slot='control-overlay-note']") as HTMLInputElement; + fireEvent.change(note, { target: { value: "我登录好了" } }); + + fireEvent.click(container.querySelector("[data-slot='control-overlay-return']") as HTMLElement); + + expect(props.onReturnControl).toHaveBeenCalledWith("我登录好了"); + }); + + it("caps the note field so an unbounded string never reaches the daemon", () => { + // The daemon cuts the note at 4096 bytes and keeps it in its interrupt + // registry until a waiter consumes it; the field must stay inside that. + const { container } = render(); + + const note = container.querySelector("[data-slot='control-overlay-note']") as HTMLInputElement; + expect(note.maxLength).toBe(NOTE_MAX_CHARS); + expect(NOTE_MAX_CHARS).toBeLessThanOrEqual(4096); + }); + + it("calls onReturnControl when Enter is pressed in the note field", () => { + const props = baseProps(); + const { container } = render(); + + const note = container.querySelector("[data-slot='control-overlay-note']") as HTMLInputElement; + fireEvent.keyDown(note, { key: "Enter" }); + + expect(props.onReturnControl).toHaveBeenCalledWith(""); + }); - expect(onInterrupt).toHaveBeenCalledTimes(1); + it("renders nothing while hidden", () => { + const { container } = render(); + expect(container.querySelector("[data-slot='control-overlay-pill']")).toBeNull(); }); }); diff --git a/apps/extension/src/content/__tests__/CursorOverlay.test.tsx b/apps/extension/src/content/__tests__/CursorOverlay.test.tsx new file mode 100644 index 00000000..2ffb1272 --- /dev/null +++ b/apps/extension/src/content/__tests__/CursorOverlay.test.tsx @@ -0,0 +1,77 @@ +import { act, cleanup, render } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { CursorOverlay, type CursorState } from "../CursorOverlay"; + +describe("CursorOverlay", () => { + afterEach(() => { + cleanup(); + vi.useRealTimers(); + }); + + it("positions the cursor at the requested viewport point", () => { + const { container } = render(); + + const cursor = container.querySelector("[data-slot='agent-cursor']") as HTMLElement; + expect(cursor).toBeTruthy(); + expect(cursor.style.transform).toContain("120px"); + expect(cursor.style.transform).toContain("42px"); + // The glide is a transform transition, not top/left. + expect(cursor.style.transition).toContain("transform 200ms"); + expect(cursor.style.pointerEvents).toBe("none"); + }); + + it("renders a label pill beside the cursor", () => { + const { container } = render( + , + ); + + const label = container.querySelector("[data-slot='agent-cursor-label']") as HTMLElement; + expect(label).toBeTruthy(); + expect(label.textContent).toBe("e12"); + }); + + it("draws a ripple for a click and a fresh one for the next click id", () => { + const first: CursorState = { x: 5, y: 6, durationMs: 0, ripple: { id: 1, button: "left" } }; + const { container, rerender } = render(); + + const ripple = container.querySelector("[data-slot='agent-cursor-ripple']"); + expect(ripple).toBeTruthy(); + expect((ripple as HTMLElement).dataset.button).toBe("left"); + + const second: CursorState = { x: 5, y: 6, durationMs: 0, ripple: { id: 2, button: "left" } }; + rerender(); + + const rerippled = container.querySelector("[data-slot='agent-cursor-ripple']"); + expect(rerippled).toBeTruthy(); + // `key={ripple.id}` remounts the node, so the animation replays. + expect(rerippled).not.toBe(ripple); + expect(container.querySelectorAll("[data-slot='agent-cursor-ripple']")).toHaveLength(1); + }); + + it("renders nothing for a null state", () => { + const { container } = render(); + expect(container.querySelector("[data-slot='agent-cursor']")).toBeNull(); + }); + + it("fades out after the idle window and comes back on the next move", () => { + vi.useFakeTimers(); + const { container, rerender } = render(); + + const cursor = container.querySelector("[data-slot='agent-cursor']") as HTMLElement; + expect(cursor.style.opacity).toBe("1"); + + act(() => { + vi.advanceTimersByTime(2600); + }); + expect( + (container.querySelector("[data-slot='agent-cursor']") as HTMLElement).style.opacity, + ).toBe("0"); + + act(() => { + rerender(); + }); + expect( + (container.querySelector("[data-slot='agent-cursor']") as HTMLElement).style.opacity, + ).toBe("1"); + }); +}); diff --git a/apps/extension/src/content/__tests__/overlay-controller.test.ts b/apps/extension/src/content/__tests__/overlay-controller.test.ts index 334f4078..9e4584c7 100644 --- a/apps/extension/src/content/__tests__/overlay-controller.test.ts +++ b/apps/extension/src/content/__tests__/overlay-controller.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { OverlayController, shouldShowAgentControlOverlay } from "../overlay-controller"; +import { + OverlayController, + shouldShowAgentControlOverlay, + shouldShowInterruptingOverlay, +} from "../overlay-controller"; describe("OverlayController", () => { it("resets agent overlays without clearing user-tab borrow requests", () => { @@ -131,6 +135,66 @@ describe("OverlayController", () => { expect(shouldShowAgentControlOverlay(controller.snapshot())).toBe(true); }); + it("keeps the pill and its blocker up while the take-over request is in flight", () => { + const controller = new OverlayController(); + controller.applyAgentControlMode("sess-1", "interrupting"); + + // `interrupting` is not `control`, so the normal mask predicate is false… + expect(shouldShowAgentControlOverlay(controller.snapshot())).toBe(false); + // …but the pill must stay (disabled, 「接管中…」) until the daemon acks. + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(true); + expect(controller.snapshot().interrupting).toBe(true); + + controller.applyAgentControlMode("sess-1", "paused"); + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(false); + expect(controller.isPausedVisible()).toBe(true); + }); + + it("hides the interrupting pill while another overlay owns the chrome", () => { + const controller = new OverlayController(); + controller.applyAgentControlMode("sess-1", "interrupting"); + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(true); + + controller.setAgentRecordRequest({ id: "rec-1", onFinish: vi.fn() }); + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(false); + + controller.clearAgentRecordRequest("rec-1"); + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(false); + + controller.setControlHintsHidden(true); + expect(shouldShowInterruptingOverlay(controller.snapshot())).toBe(false); + }); + + it("exposes paused visibility only while a session holds control", () => { + const controller = new OverlayController(); + expect(controller.isPausedVisible()).toBe(false); + expect(controller.snapshot().pausedVisible).toBe(false); + + controller.activateAgentSession("sess-1"); + expect(controller.isPausedVisible()).toBe(false); + + controller.applyAgentControlMode("sess-1", "paused"); + expect(controller.isPausedVisible()).toBe(true); + expect(controller.snapshot().pausedVisible).toBe(true); + // The paused pill never blocks the page, but it is not the control mask + // either — the two predicates are mutually exclusive. + expect(controller.isControlVisible()).toBe(false); + expect(shouldShowAgentControlOverlay(controller.snapshot())).toBe(false); + + controller.applyAgentControlMode("sess-1", "control"); + expect(controller.isPausedVisible()).toBe(false); + expect(controller.isControlVisible()).toBe(true); + + controller.applyAgentControlMode("sess-1", "paused"); + controller.resetAgentOverlays("sess-1"); + expect(controller.isPausedVisible()).toBe(false); + expect(controller.snapshot().pausedVisible).toBe(false); + + // No session ⇒ never paused, even if a stale mode lingered. + controller.applyAgentControlMode(null, "hidden"); + expect(controller.isPausedVisible()).toBe(false); + }); + it("hides the control overlay when the user hides control hints", () => { const controller = new OverlayController(); controller.activateAgentSession("sess-1"); diff --git a/apps/extension/src/content/overlay-controller.ts b/apps/extension/src/content/overlay-controller.ts index 3a2c4d8a..1522c250 100644 --- a/apps/extension/src/content/overlay-controller.ts +++ b/apps/extension/src/content/overlay-controller.ts @@ -8,6 +8,8 @@ export interface OverlayState { activeHelp: HelpRequestData | null; activeRecord: RecordRequestData | null; controlVisible: boolean; + /** The `paused` hold pill is showing (see {@link OverlayController.isPausedVisible}). */ + pausedVisible: boolean; interrupting: boolean; activeSessionId: string | null; controlMode: OverlayMode; @@ -26,7 +28,7 @@ export interface OverlayState { controlHintsHidden: boolean; } -type MutableOverlayState = Omit; +type MutableOverlayState = Omit; /** * Owns overlay state by rendering scope. User-tab overlays survive Agent @@ -49,6 +51,7 @@ export class OverlayController { return { ...this.state, controlVisible: this.isControlVisible(), + pausedVisible: this.isPausedVisible(), borrowRequests: [...this.state.borrowRequests], }; } @@ -57,6 +60,14 @@ export class OverlayController { return this.state.activeSessionId !== null && this.state.controlMode === "control"; } + /** + * The user is operating the page by hand. Unlike `control`, the paused pill + * carries no input blocker — only the note field and the return button. + */ + isPausedVisible(): boolean { + return this.state.activeSessionId !== null && this.state.controlMode === "paused"; + } + addBorrowRequest(request: BorrowRequestData): void { this.state.borrowRequests = [ ...this.state.borrowRequests.filter((r) => r.id !== request.id), @@ -159,13 +170,28 @@ export class OverlayController { } } -/** Control mask ("Agent 正在控制") must hide while help/record overlays own the chrome. */ -export function shouldShowAgentControlOverlay(state: OverlayState): boolean { +/** Pill chrome is allowed while the session owns a tab and nothing else owns it. */ +function controlChromeAllowed(state: OverlayState): boolean { return ( - state.controlVisible && + state.activeSessionId !== null && !state.controlHintsHidden && !state.suppressControlAfterRecord && state.activeHelp === null && state.activeRecord === null ); } + +/** Control mask ("Agent 正在控制") must hide while help/record overlays own the chrome. */ +export function shouldShowAgentControlOverlay(state: OverlayState): boolean { + return state.controlVisible && controlChromeAllowed(state); +} + +/** + * The take-over request is in flight: the pill stays put with its disabled + * 「接管中…」button (and its blocker) until the background flips the session to + * `paused` — the user must not be able to click through a page the agent may + * still be driving. + */ +export function shouldShowInterruptingOverlay(state: OverlayState): boolean { + return state.controlMode === "interrupting" && controlChromeAllowed(state); +} diff --git a/apps/extension/src/entrypoints/__tests__/background-interrupt.test.ts b/apps/extension/src/entrypoints/__tests__/background-interrupt.test.ts index c22c4017..65275ba1 100644 --- a/apps/extension/src/entrypoints/__tests__/background-interrupt.test.ts +++ b/apps/extension/src/entrypoints/__tests__/background-interrupt.test.ts @@ -10,17 +10,29 @@ vi.hoisted(() => { ) => cb; }); -import { handleOverlayInterrupt } from "@/entrypoints/background"; +import { + controlModeAfterTakeOver, + handleOverlayInterrupt, + handleOverlayReturnControl, + handleOverlayReturnControlRequest, + shouldControlResumeOnBrowserActivity, +} from "@/entrypoints/background"; + +type InterruptTransport = Parameters[0]; describe("handleOverlayInterrupt", () => { - it("sends a session.user_interrupt event to the daemon and acks ok", async () => { + it("sends session.user_interrupt + session.control_taken and acks ok", async () => { const send = vi.fn().mockReturnValue(undefined); - const transport = { send } as unknown as Parameters[0]; + const transport = { send } as unknown as InterruptTransport; const result = await handleOverlayInterrupt(transport, "sess-1"); expect(send).toHaveBeenCalledWith({ event: "session.user_interrupt", payload: { session_id: "sess-1" }, }); + expect(send).toHaveBeenCalledWith({ + event: "session.control_taken", + payload: { session_id: "sess-1" }, + }); expect(result).toEqual({ ok: true }); }); @@ -28,8 +40,131 @@ describe("handleOverlayInterrupt", () => { const send = vi.fn(() => { throw new Error("ws closed"); }); - const transport = { send } as unknown as Parameters[0]; + const transport = { send } as unknown as InterruptTransport; const result = await handleOverlayInterrupt(transport, "sess-1"); expect(result).toEqual({ ok: false }); }); }); + +describe("handleOverlayReturnControl", () => { + it("sends session.control_returned with the note", () => { + const send = vi.fn().mockReturnValue(undefined); + const transport = { send } as unknown as InterruptTransport; + expect(handleOverlayReturnControl(transport, "sess-1", "我登录好了")).toEqual({ ok: true }); + expect(send).toHaveBeenCalledWith({ + event: "session.control_returned", + payload: { session_id: "sess-1", note: "我登录好了" }, + }); + }); + + it("sends an empty note verbatim", () => { + const send = vi.fn().mockReturnValue(undefined); + const transport = { send } as unknown as InterruptTransport; + handleOverlayReturnControl(transport, "sess-1", ""); + expect(send).toHaveBeenCalledWith({ + event: "session.control_returned", + payload: { session_id: "sess-1", note: "" }, + }); + }); + + it("reports ok=false when the transport is down", () => { + const send = vi.fn(() => { + throw new Error("ws closed"); + }); + const transport = { send } as unknown as InterruptTransport; + expect(handleOverlayReturnControl(transport, "sess-1", "note")).toEqual({ ok: false }); + }); +}); + +describe("handleOverlayReturnControlRequest", () => { + it("sends session.control_returned with the note and flips the mode back to control", () => { + const send = vi.fn(); + const setControlMode = vi.fn(); + const reply = handleOverlayReturnControlRequest( + { hasSession: () => true, transport: { send }, setControlMode }, + { kind: "overlay.return_control", sessionId: "sess-1", note: "all set" }, + ); + + expect(send).toHaveBeenCalledWith({ + event: "session.control_returned", + payload: { session_id: "sess-1", note: "all set" }, + }); + expect(setControlMode).toHaveBeenCalledWith("sess-1", "control"); + expect(reply).toEqual({ ok: true }); + }); + + it("replies ok=false and leaves the mode alone when the session is gone", () => { + const send = vi.fn(); + const setControlMode = vi.fn(); + const reply = handleOverlayReturnControlRequest( + { hasSession: () => false, transport: { send }, setControlMode }, + { kind: "overlay.return_control", sessionId: "sess-gone", note: "" }, + ); + + expect(send).not.toHaveBeenCalled(); + expect(setControlMode).not.toHaveBeenCalled(); + expect(reply).toEqual({ ok: false }); + }); + + it("keeps the user in control when the daemon frame cannot be sent", () => { + const send = vi.fn(() => { + throw new Error("ws closed"); + }); + const setControlMode = vi.fn(); + const reply = handleOverlayReturnControlRequest( + { hasSession: () => true, transport: { send }, setControlMode }, + { kind: "overlay.return_control", sessionId: "sess-1", note: "note" }, + ); + + expect(setControlMode).not.toHaveBeenCalled(); + expect(reply).toEqual({ ok: false }); + }); + + it("treats a non-string note as empty", () => { + const send = vi.fn(); + const reply = handleOverlayReturnControlRequest( + { hasSession: () => true, transport: { send }, setControlMode: vi.fn() }, + { + kind: "overlay.return_control", + sessionId: "sess-1", + note: undefined as unknown as string, + }, + ); + + expect(send).toHaveBeenCalledWith({ + event: "session.control_returned", + payload: { session_id: "sess-1", note: "" }, + }); + expect(reply).toEqual({ ok: true }); + }); +}); + +describe("shouldControlResumeOnBrowserActivity", () => { + it("never auto-resumes a session the user took over", () => { + // The daemon blocks agent input while control is held, but a passive read + // (snapshot / get_html) still reaches the extension — it must not un-pause + // the UI behind the user's back. + expect(shouldControlResumeOnBrowserActivity("paused")).toBe(false); + expect(shouldControlResumeOnBrowserActivity("interrupting")).toBe(false); + }); + + it("keeps resuming from every other state", () => { + expect(shouldControlResumeOnBrowserActivity("control")).toBe(true); + expect(shouldControlResumeOnBrowserActivity("hidden")).toBe(true); + expect(shouldControlResumeOnBrowserActivity(undefined)).toBe(true); + }); +}); + +describe("controlModeAfterTakeOver", () => { + it("parks the session in paused once the daemon has the frame", () => { + expect(controlModeAfterTakeOver(true)).toBe("paused"); + }); + + it("rolls back to control when the take-over frame could not be sent", () => { + // `interrupting` is sticky, so a failed send must not be left in place: + // the pill would keep its disabled button and its input blocker with no + // return button, locking the user out of the page for good. + expect(controlModeAfterTakeOver(false)).toBe("control"); + expect(shouldControlResumeOnBrowserActivity(controlModeAfterTakeOver(false))).toBe(true); + }); +}); diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index d2dc4ab1..416f1e06 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -17,12 +17,15 @@ import { OVERLAY_AUTOMATION_BYPASS, OVERLAY_MSG_INTERRUPT, OVERLAY_MSG_READY, + OVERLAY_MSG_RETURN_CONTROL, OVERLAY_MSG_WHO_AM_I, type OverlayAgentStateMessage, type OverlayInterruptRequest, type OverlayInterruptResponse, type OverlayMessage, type OverlayMode, + type OverlayReturnControlRequest, + type OverlayReturnControlResponse, } from "@/lib/overlay-bridge"; import { POPUP_PORT_NAME, type PopupInbound, type PopupOutbound } from "@/lib/popup-bridge"; import { recordFrameCoordinator } from "@/lib/recording/frame-coordinator"; @@ -207,6 +210,10 @@ export default defineBackground(() => { function onBrowserControlResumed(sessionId: string): void { const ctx = sessions.get(sessionId); if (!ctx) return; + // A user hold is released only by the return button. A passive read + // (snapshot, get_html) still reaches the extension while the daemon blocks + // agent input, and must not un-pause the UI behind the user's back. + if (!shouldControlResumeOnBrowserActivity(controlModes.get(sessionId))) return; setControlMode(sessionId, "control"); } @@ -449,13 +456,31 @@ export default defineBackground(() => { const ctx = sessions.get(req.sessionId); if (ctx) setControlMode(req.sessionId, "interrupting"); void handleOverlayInterrupt(transport, req.sessionId).then((reply) => { - if (reply.ok && sessions.get(req.sessionId)) { - setControlMode(req.sessionId, "paused"); + // A failed take-over must roll the mode back: `interrupting` is sticky + // (`shouldControlResumeOnBrowserActivity`), so leaving it set would + // strand the user behind a blocker with a disabled button and no + // return control. + if (sessions.get(req.sessionId)) { + setControlMode(req.sessionId, controlModeAfterTakeOver(reply.ok)); } sendResponse(reply); }); return true; // keep channel open } + + if (msg.kind === OVERLAY_MSG_RETURN_CONTROL) { + sendResponse( + handleOverlayReturnControlRequest( + { + hasSession: (sessionId) => sessions.get(sessionId) !== null, + transport, + setControlMode, + }, + msg as OverlayReturnControlRequest, + ), + ); + return false; + } return false; }); @@ -514,6 +539,9 @@ export default defineBackground(() => { * connected, etc.). The daemon-side cancellation is fire-and-forget * — a failure here just means the user will need to retry the * interrupt; no daemon state is left half-updated. + * + * This is the user *taking over*: the daemon also starts rejecting agent + * input tools until a matching `session.control_returned` arrives. */ export async function handleOverlayInterrupt( transport: Pick, @@ -524,6 +552,10 @@ export async function handleOverlayInterrupt( event: "session.user_interrupt", payload: { session_id: sessionId }, }); + transport.send({ + event: "session.control_taken", + payload: { session_id: sessionId }, + }); return { ok: true }; } catch (err) { console.warn("[browser-skill] failed to send session.user_interrupt", err); @@ -531,6 +563,71 @@ export async function handleOverlayInterrupt( } } +/** + * Hand control back to the agent. `note` (may be "") is carried to the daemon + * so the agent learns what the user did while it was paused. Same all-or-nothing + * contract as {@link handleOverlayInterrupt}: `{ ok: false }` means the daemon + * never saw the frame (the caller keeps the user in control and retries). + */ +export function handleOverlayReturnControl( + transport: Pick, + sessionId: string, + note: string, +): OverlayReturnControlResponse { + try { + transport.send({ + event: "session.control_returned", + payload: { session_id: sessionId, note }, + }); + return { ok: true }; + } catch (err) { + console.warn("[browser-skill] failed to send session.control_returned", err); + return { ok: false }; + } +} + +/** + * Whether a browser-control RPC arriving for `sessionId` may un-pause its + * overlay. `interrupting` (the take-over request is still in flight) and + * `paused` (the user holds control) are both sticky: only the return button + * releases them. Every other mode — including `undefined` for a session whose + * mode was never recorded — resumes as before. + */ +export function shouldControlResumeOnBrowserActivity(mode: OverlayMode | undefined): boolean { + return mode !== "paused" && mode !== "interrupting"; +} + +/** + * Mode to settle on once the `session.control_taken` frame resolves. A failed + * send must roll back to `control`: `interrupting` is sticky (see + * {@link shouldControlResumeOnBrowserActivity}), so leaving it set strands the + * user behind the input blocker with a disabled button and no return control. + */ +export function controlModeAfterTakeOver(sent: boolean): OverlayMode { + return sent ? "paused" : "control"; +} + +/** + * Background half of `overlay.return_control`: verify the session still exists, + * tell the daemon the user handed control back (with their optional note) and + * re-show the normal pill + blocker. An unknown session replies `{ ok: false }` + * so the content script clears its overlay instead of showing a dead hold. + */ +export function handleOverlayReturnControlRequest( + deps: { + hasSession: (sessionId: string) => boolean; + transport: Pick; + setControlMode: (sessionId: string, mode: OverlayMode) => void; + }, + req: OverlayReturnControlRequest, +): OverlayReturnControlResponse { + if (!deps.hasSession(req.sessionId)) return { ok: false }; + const note = typeof req.note === "string" ? req.note : ""; + const reply = handleOverlayReturnControl(deps.transport, req.sessionId, note); + if (reply.ok) deps.setControlMode(req.sessionId, "control"); + return reply; +} + function makeBorrowNotificationCopy(): BorrowNotificationCopy { return { title: i18n.t("borrowConfirmation.notificationTitle", { ns: "extension" }), diff --git a/apps/extension/src/entrypoints/content.ts b/apps/extension/src/entrypoints/content.ts index b481e051..08739834 100644 --- a/apps/extension/src/entrypoints/content.ts +++ b/apps/extension/src/entrypoints/content.ts @@ -5,17 +5,34 @@ import { flushSync } from "react-dom"; import ReactDOM from "react-dom/client"; import { BorrowConfirmationOverlay } from "@/content/BorrowConfirmationOverlay"; import { ControlOverlay } from "@/content/ControlOverlay"; +import { CursorOverlay, type CursorState } from "@/content/CursorOverlay"; import { createCaptureSuppressController } from "@/content/capture-suppress"; import { HelpRequestOverlay } from "@/content/HelpRequestOverlay"; import { createHelpRequestData } from "@/content/help-request"; import overlayCss from "@/content/overlay.css?inline"; -import { OverlayController, shouldShowAgentControlOverlay } from "@/content/overlay-controller"; +import { + OverlayController, + shouldShowAgentControlOverlay, + shouldShowInterruptingOverlay, +} from "@/content/overlay-controller"; import { RecordOverlay } from "@/content/RecordOverlay"; +import { + ACTION_STATUS_MSG, + type ActionStatusAck, + type ActionStatusMessage, + isActionStatusMessage, +} from "@/lib/action-status-bridge"; import { type CaptureSuppressAck, type CaptureSuppressMessage, isCaptureSuppressMessage, } from "@/lib/capture-suppress-bridge"; +import { + CURSOR_MSG, + type CursorAck, + type CursorMessage, + isCursorMessage, +} from "@/lib/cursor-bridge"; import { HELP_ACK, HELP_FINISH, @@ -38,7 +55,7 @@ import { type OverlayAgentStateMessage, type OverlayAutomationBypassMessage, } from "@/lib/overlay-bridge"; -import { sendInterrupt } from "@/lib/overlay-interrupt-client"; +import { sendInterrupt, sendReturnControl } from "@/lib/overlay-interrupt-client"; import { isRecordCancelMessage, isRecordStartMessage, @@ -75,6 +92,9 @@ export default defineContentScript({ let overlayHost: HTMLElement | null = null; let overlayContainer: HTMLElement | null = null; let activeAgentState: OverlayAgentStateMessage | null = null; + let cursorState: CursorState | null = null; + let actionStatus: { tool: string; target?: string } | null = null; + let cursorRippleId = 0; let hostLossReported = false; let remountInProgress = false; @@ -166,14 +186,18 @@ export default defineContentScript({ function renderReactOverlays(): void { const overlayState = overlays.snapshot(); const controlOverlayVisible = shouldShowAgentControlOverlay(overlayState); + const interruptingOverlayVisible = shouldShowInterruptingOverlay(overlayState); const interactiveOverlayVisible = + overlayState.pausedVisible || overlayState.borrowRequests.length > 0 || overlayState.activeHelp !== null || overlayState.activeRecord !== null; setOverlayHostHiddenFromAccessibility(!interactiveOverlayVisible); + // A hold pill carries no blocker, so the host must not swallow page input. + const blockingControlOverlay = controlOverlayVisible || interruptingOverlayVisible; setOverlaySurfaceState( - controlOverlayVisible, - controlOverlayVisible && overlayState.automationBypassCount === 0, + blockingControlOverlay || overlayState.pausedVisible, + blockingControlOverlay && overlayState.automationBypassCount === 0, ); const root = reactRoot; if (!root) return; @@ -190,11 +214,16 @@ export default defineContentScript({ }), React.createElement(HelpRequestOverlay, { request: overlayState.activeHelp }), React.createElement(RecordOverlay, { request: overlayState.activeRecord }), + React.createElement(CursorOverlay, { state: cursorState }), React.createElement(ControlOverlay, { - visible: controlOverlayVisible, + visible: + controlOverlayVisible || interruptingOverlayVisible || overlayState.pausedVisible, + mode: overlayState.controlMode, interrupting: overlayState.interrupting, automationBypass: overlayState.automationBypassCount > 0, + currentAction: actionStatus, onInterrupt: handleInterrupt, + onReturnControl: handleReturnControl, }), ), ), @@ -220,6 +249,12 @@ export default defineContentScript({ function applyOverlayState(state: OverlayAgentStateMessage): void { activeAgentState = state; overlays.applyAgentControlMode(state.sessionId, state.mode); + // A session that is no longer in control cannot be driving input, so a + // lingering cosmetic cursor would be stale. + if (!overlays.isControlVisible() && !overlays.isPausedVisible()) { + cursorState = null; + actionStatus = null; + } renderAll(); } @@ -229,6 +264,50 @@ export default defineContentScript({ void sendHelpFinish(previousHelp.id, "cancelled"); } activeRecordRequestId = null; + cursorState = null; + actionStatus = null; + renderAll(); + } + + /** + * Cosmetic agent cursor. `move` glides to a viewport point, `click` + * ripples there (keeping the position), `hide` clears it. Every branch + * acks synchronously so the background's await resolves without waiting + * on the animation. + */ + function handleCursorMessage(message: CursorMessage): void { + switch (message.action) { + case "move": + cursorState = { + x: message.x, + y: message.y, + durationMs: message.durationMs, + ...(message.label ? { label: message.label } : {}), + }; + break; + case "click": + cursorRippleId += 1; + cursorState = { + x: message.x, + y: message.y, + durationMs: cursorState?.durationMs ?? 0, + ...(cursorState?.label ? { label: cursorState.label } : {}), + ripple: { id: cursorRippleId, button: message.button ?? "left" }, + }; + break; + case "hide": + cursorState = null; + break; + } + renderAll(); + } + + /** Cosmetic "what is the agent doing" line for the control pill. */ + function handleActionStatusMessage(message: ActionStatusMessage): void { + actionStatus = + message.phase === "start" + ? { tool: message.tool, ...(message.target ? { target: message.target } : {}) } + : null; renderAll(); } @@ -250,6 +329,24 @@ export default defineContentScript({ }); } + /** + * Hand control back to the agent with the user's optional note. The + * background re-shows the pill + blocker; when the session is gone it + * replies `{ ok: false }` and we clear the hold overlay locally. + */ + function handleReturnControl(note: string) { + const sessionId = overlays.snapshot().activeSessionId; + if (!sessionId) { + console.warn("[bsk overlay] return requested with no active session id"); + return; + } + void sendReturnControl((msg) => chrome.runtime.sendMessage(msg), sessionId, note).then( + (reply) => { + if (!reply.ok) resetAgentOverlayState(sessionId); + }, + ); + } + const onMessage = ( message: | BorrowRequestMessage @@ -257,19 +354,43 @@ export default defineContentScript({ | HelpRequestMessage | HelpCancelMessage | CaptureSuppressMessage + | CursorMessage | RecordStartMessage | RecordStopMessage | RecordCancelMessage + | ActionStatusMessage | OverlayAgentOverlayResetMessage | OverlayAgentStateMessage | OverlayAutomationBypassMessage, _sender: chrome.runtime.MessageSender, - sendResponse: (response: BorrowResponseMessage | HelpAckMessage | CaptureSuppressAck) => void, + sendResponse: ( + response: + | BorrowResponseMessage + | HelpAckMessage + | CaptureSuppressAck + | CursorAck + | ActionStatusAck, + ) => void, ) => { if (isCaptureSuppressMessage(message)) { return captureSuppress.handleMessage(message, sendResponse); } + if (isActionStatusMessage(message)) { + handleActionStatusMessage(message); + (sendResponse as unknown as (response: ActionStatusAck) => void)({ + type: ACTION_STATUS_MSG, + ok: true, + }); + return false; + } + + if (isCursorMessage(message)) { + handleCursorMessage(message); + (sendResponse as unknown as (response: CursorAck) => void)({ type: CURSOR_MSG, ok: true }); + return false; + } + if (isRecordStartMessage(message)) { activeRecordRequestId = message.requestId; overlays.setAgentRecordRequest({ @@ -519,6 +640,7 @@ export default defineContentScript({ chrome.storage.onChanged.removeListener(onStorageChange); window.removeEventListener("pageshow", onPageShow); activeRecordRequestId = null; + actionStatus = null; }); }, }); diff --git a/apps/extension/src/lib/__tests__/action-status-bridge.test.ts b/apps/extension/src/lib/__tests__/action-status-bridge.test.ts new file mode 100644 index 00000000..89b29f01 --- /dev/null +++ b/apps/extension/src/lib/__tests__/action-status-bridge.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it, vi } from "vitest"; +import { + ACTION_STATUS_MAX_TARGET_CHARS, + ACTION_STATUS_MSG, + createActionStatusNotifier, + isActionStatusMessage, + truncateActionTarget, +} from "@/lib/action-status-bridge"; + +describe("isActionStatusMessage", () => { + it("accepts start and end frames", () => { + expect( + isActionStatusMessage({ type: ACTION_STATUS_MSG, phase: "start", tool: "tool.click" }), + ).toBe(true); + expect( + isActionStatusMessage({ + type: ACTION_STATUS_MSG, + phase: "start", + tool: "tool.click", + target: "@e3", + }), + ).toBe(true); + expect( + isActionStatusMessage({ type: ACTION_STATUS_MSG, phase: "end", tool: "tool.click" }), + ).toBe(true); + }); + + it("rejects malformed frames", () => { + expect(isActionStatusMessage(null)).toBe(false); + expect(isActionStatusMessage({ type: ACTION_STATUS_MSG, phase: "start" })).toBe(false); + expect(isActionStatusMessage({ type: ACTION_STATUS_MSG, phase: "nope", tool: "t" })).toBe( + false, + ); + expect( + isActionStatusMessage({ type: ACTION_STATUS_MSG, phase: "start", tool: "t", target: 3 }), + ).toBe(false); + }); +}); + +describe("truncateActionTarget", () => { + it("leaves short values alone and cuts long ones", () => { + expect(truncateActionTarget("short")).toBe("short"); + const long = "x".repeat(ACTION_STATUS_MAX_TARGET_CHARS + 10); + expect(truncateActionTarget(long)).toHaveLength(ACTION_STATUS_MAX_TARGET_CHARS); + }); +}); + +describe("createActionStatusNotifier", () => { + it("sends start then end for the same tab", async () => { + const send = vi.fn(async () => undefined); + const notifier = createActionStatusNotifier(send); + + await notifier.start(7, "tool.click", "@e3"); + await notifier.end(7); + + expect(send.mock.calls).toEqual([ + [7, { type: ACTION_STATUS_MSG, phase: "start", tool: "tool.click", target: "@e3" }], + [7, { type: ACTION_STATUS_MSG, phase: "end", tool: "tool.click" }], + ]); + }); + + it("omits the target when none was derived", async () => { + const send = vi.fn(async () => undefined); + const notifier = createActionStatusNotifier(send); + + await notifier.start(7, "tool.scroll_to"); + + expect(send).toHaveBeenCalledWith(7, { + type: ACTION_STATUS_MSG, + phase: "start", + tool: "tool.scroll_to", + }); + }); + + it("swallows delivery failures so a tab without a content script never breaks an action", async () => { + const send = vi.fn(async () => { + throw new Error("Receiving end does not exist"); + }); + const notifier = createActionStatusNotifier(send); + + await expect(notifier.start(7, "tool.click", "@e1")).resolves.toBeUndefined(); + await expect(notifier.end(7)).resolves.toBeUndefined(); + }); + + it("remembers the in-flight tool per tab so end still names it", async () => { + const send = vi.fn(async () => undefined); + const notifier = createActionStatusNotifier(send); + + await notifier.start(1, "tool.fill", "input#q"); + await notifier.start(2, "tool.hover", "@e9"); + await notifier.end(1); + + expect(send).toHaveBeenLastCalledWith(1, { + type: ACTION_STATUS_MSG, + phase: "end", + tool: "tool.fill", + }); + }); +}); diff --git a/apps/extension/src/lib/__tests__/connection-controller.test.ts b/apps/extension/src/lib/__tests__/connection-controller.test.ts index 0414bbbc..6603b1b6 100644 --- a/apps/extension/src/lib/__tests__/connection-controller.test.ts +++ b/apps/extension/src/lib/__tests__/connection-controller.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { MIN_COMPATIBLE_PROTOCOL } from "../../transport/handshake"; +import { MIN_COMPATIBLE_PROTOCOL, PROTOCOL_VERSION } from "../../transport/handshake"; import type { ConnectionStateHandler, FrameHandler, Transport } from "../../transport/transport"; import type { ConnectionState, HandshakeResult, ProtocolFrame } from "../../transport/types"; import { __testing__, ConnectionController } from "../connection-controller"; @@ -27,13 +27,19 @@ function handshake( describe("computeConnectedState (protocol-based compat)", () => { it("returns connected when daemon protocol equals extension protocol", () => { - expect(computeConnectedState(handshake("1.3", "1.3"), MIN_COMPATIBLE_PROTOCOL)).toEqual({ + expect( + computeConnectedState(handshake(PROTOCOL_VERSION, "1.3"), MIN_COMPATIBLE_PROTOCOL), + ).toEqual({ kind: "connected", }); }); it("returns version_skew when daemon protocol minor is newer", () => { - expect(computeConnectedState(handshake("1.4", "1.3"))).toEqual({ + // Derived from the live constant so a protocol bump does not silently + // turn this into an equal-version case. + const [major, minor] = PROTOCOL_VERSION.split("."); + const newer = `${major}.${Number(minor) + 1}`; + expect(computeConnectedState(handshake(newer, "1.3"))).toEqual({ kind: "version_skew", }); }); @@ -65,7 +71,7 @@ describe("computeConnectedState (protocol-based compat)", () => { const result = computeConnectedState({ server: "browser-skill-daemon", version: "0.1.0", - protocol_version: "1.3", + protocol_version: PROTOCOL_VERSION, min_compatible_peer: "0.1.0", }); expect(result).toEqual({ kind: "connected" }); @@ -264,11 +270,11 @@ describe("ConnectionController connectionEnabled", () => { const second = transport.send.mock.calls[1]?.[0] as { id: string }; expect(second.id).not.toBe(first.id); - transport.emitMessage({ id: first.id, result: handshake("1.3", "1.3") }); + transport.emitMessage({ id: first.id, result: handshake(PROTOCOL_VERSION, "1.3") }); await Promise.resolve(); expect(controller.snapshot().state).not.toBe("connected"); - transport.emitMessage({ id: second.id, result: handshake("1.3", "1.3") }); + transport.emitMessage({ id: second.id, result: handshake(PROTOCOL_VERSION, "1.3") }); await vi.waitFor(() => expect(controller.snapshot().state).toBe("connected")); }); }); diff --git a/apps/extension/src/lib/__tests__/cursor-bridge.test.ts b/apps/extension/src/lib/__tests__/cursor-bridge.test.ts new file mode 100644 index 00000000..9af5b98e --- /dev/null +++ b/apps/extension/src/lib/__tests__/cursor-bridge.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from "vitest"; +import { + CURSOR_MSG, + type CursorMessage, + createCursorVisualizer, + cursorMoveDuration, + isCursorMessage, +} from "../cursor-bridge"; + +describe("isCursorMessage", () => { + it("accepts the three cursor actions", () => { + expect(isCursorMessage({ type: CURSOR_MSG, action: "hide" })).toBe(true); + expect(isCursorMessage({ type: CURSOR_MSG, action: "move", x: 1, y: 2, durationMs: 0 })).toBe( + true, + ); + expect(isCursorMessage({ type: CURSOR_MSG, action: "click", x: 1, y: 2 })).toBe(true); + expect( + isCursorMessage({ type: CURSOR_MSG, action: "click", x: 1, y: 2, button: "middle" }), + ).toBe(true); + }); + + it("rejects malformed messages", () => { + expect(isCursorMessage(null)).toBe(false); + expect(isCursorMessage("bsk/cursor")).toBe(false); + expect(isCursorMessage({ type: "other", action: "hide" })).toBe(false); + expect(isCursorMessage({ type: CURSOR_MSG, action: "move", x: 1, y: 2 })).toBe(false); + expect(isCursorMessage({ type: CURSOR_MSG, action: "click", x: 1, y: "2" })).toBe(false); + expect(isCursorMessage({ type: CURSOR_MSG, action: "click", button: "left" })).toBe(false); + expect(isCursorMessage({ type: CURSOR_MSG, action: "warp", x: 1, y: 2 })).toBe(false); + }); +}); + +describe("cursorMoveDuration", () => { + it("is instant without a previous position", () => { + expect(cursorMoveDuration(null, { x: 10, y: 10 })).toBe(0); + expect(cursorMoveDuration(undefined, { x: 10, y: 10 })).toBe(0); + }); + + it("scales with distance inside the 120–500ms clamp", () => { + // 1000px * 0.6 = 600ms → clamped to 500. + expect(cursorMoveDuration({ x: 0, y: 0 }, { x: 1000, y: 0 })).toBe(500); + // 10px * 0.6 = 6ms → clamped up to 120. + expect(cursorMoveDuration({ x: 0, y: 0 }, { x: 10, y: 0 })).toBe(120); + // 400px * 0.6 = 240ms, unclamped. + expect(cursorMoveDuration({ x: 0, y: 0 }, { x: 400, y: 0 })).toBe(240); + // Distance is euclidean: 3-4-5 triangle → 500px * 0.6 = 300ms. + expect(cursorMoveDuration({ x: 0, y: 0 }, { x: 300, y: 400 })).toBe(300); + }); +}); + +describe("createCursorVisualizer", () => { + it("sends a move and waits for the animation before resolving", async () => { + const seen: CursorMessage[] = []; + const sendToTab = vi.fn(async (_tabId: number, message: CursorMessage) => { + seen.push(message); + return { type: CURSOR_MSG, ok: true }; + }); + const cursor = createCursorVisualizer(sendToTab); + const started = performance.now(); + await cursor.move(7, { x: 12, y: 34 }, { durationMs: 120, label: "e3" }); + const elapsed = performance.now() - started; + + expect(seen).toEqual([ + { type: CURSOR_MSG, action: "move", x: 12, y: 34, durationMs: 120, label: "e3" }, + ]); + expect(elapsed).toBeGreaterThanOrEqual(100); + }); + + it("clamps the wait to the animation ceiling", async () => { + const sendToTab = vi.fn(async (_tabId: number, _message: CursorMessage) => ({ + type: CURSOR_MSG, + ok: true, + })); + const cursor = createCursorVisualizer(sendToTab); + const started = performance.now(); + await cursor.move(7, { x: 0, y: 0 }, { durationMs: 5000 }); + expect(performance.now() - started).toBeLessThan(2000); + expect(sendToTab.mock.calls[0]?.[1]).toMatchObject({ durationMs: 800 }); + }); + + it("resolves immediately when the tab has no content script", async () => { + const sendToTab = vi.fn(async (_tabId: number, _message: CursorMessage) => { + throw new Error("Could not establish connection. Receiving end does not exist."); + }); + const cursor = createCursorVisualizer(sendToTab); + const started = performance.now(); + await cursor.move(7, { x: 1, y: 2 }, { durationMs: 500 }); + await cursor.click(7, { x: 1, y: 2 }, { button: "left" }); + await cursor.hide(7); + expect(performance.now() - started).toBeLessThan(200); + expect(sendToTab).toHaveBeenCalledTimes(3); + }); + + it("sends click and hide messages verbatim", async () => { + const sendToTab = vi.fn(async (_tabId: number, _message: CursorMessage) => ({ + type: CURSOR_MSG, + ok: true, + })); + const cursor = createCursorVisualizer(sendToTab); + + await cursor.click(3, { x: 5, y: 6 }, { button: "right" }); + await cursor.click(3, { x: 5, y: 6 }); + await cursor.hide(3); + + expect(sendToTab.mock.calls.map((call) => call[1])).toEqual([ + { type: CURSOR_MSG, action: "click", x: 5, y: 6, button: "right" }, + { type: CURSOR_MSG, action: "click", x: 5, y: 6 }, + { type: CURSOR_MSG, action: "hide" }, + ]); + }); +}); diff --git a/apps/extension/src/lib/action-status-bridge.ts b/apps/extension/src/lib/action-status-bridge.ts new file mode 100644 index 00000000..7190cd58 --- /dev/null +++ b/apps/extension/src/lib/action-status-bridge.ts @@ -0,0 +1,101 @@ +/** + * Wire protocol and background-side helper for the "current action" line the + * control pill shows under 「Agent 正在控制」. + * + * The background brackets every browser-input / navigation RPC with a `start` + * and an `end` message for the target tab, so a human watching the Agent Window + * can see *what* the agent is doing (click `@e3`, navigate to …, type into + * `#search`) instead of just that it is doing something. The content script + * keeps the last `start` until the matching `end`. + * + * Purely cosmetic: every method swallows errors (a tab without the content + * script — chrome://, the Web Store, a closed tab — simply has no pill) and is + * never able to fail or change the tool result it brackets. + */ + +export const ACTION_STATUS_MSG = "bsk/action-status"; + +/** Longest target string the pill displays; longer values are cut to size. */ +export const ACTION_STATUS_MAX_TARGET_CHARS = 60; + +export type ActionStatusPhase = "start" | "end"; + +export interface ActionStatusMessage { + type: typeof ACTION_STATUS_MSG; + phase: ActionStatusPhase; + /** Tool method name as the daemon sends it, e.g. `tool.click`. */ + tool: string; + /** `@ref`, selector, url or key — truncated to {@link ACTION_STATUS_MAX_TARGET_CHARS}. */ + target?: string; +} + +export interface ActionStatusAck { + type: typeof ACTION_STATUS_MSG; + ok: true; +} + +export function isActionStatusMessage(msg: unknown): msg is ActionStatusMessage { + if (typeof msg !== "object" || msg === null) return false; + const m = msg as Record; + if (m.type !== ACTION_STATUS_MSG) return false; + if (m.phase !== "start" && m.phase !== "end") return false; + if (typeof m.tool !== "string") return false; + return m.target === undefined || typeof m.target === "string"; +} + +/** Trim a target to the pill's budget; the CSS adds the ellipsis. */ +export function truncateActionTarget(value: string): string { + return value.length > ACTION_STATUS_MAX_TARGET_CHARS + ? value.slice(0, ACTION_STATUS_MAX_TARGET_CHARS) + : value; +} + +/** Minimal `chrome.tabs.sendMessage` surface so tests can inject a fake. */ +export type ActionStatusSendToTab = ( + tabId: number, + message: ActionStatusMessage, +) => Promise; + +const defaultSendToTab: ActionStatusSendToTab = (tabId, message) => + chrome.tabs.sendMessage(tabId, message); + +export interface ActionStatusNotifier { + /** Announce the tool the agent just started running against `tabId`. */ + start(tabId: number, tool: string, target?: string): Promise; + /** Clear the action line. Reuses the tool remembered by `start` when omitted. */ + end(tabId: number, tool?: string): Promise; +} + +export function createActionStatusNotifier( + sendToTab: ActionStatusSendToTab = defaultSendToTab, +): ActionStatusNotifier { + /** Tool currently in flight per tab, so `end` can name it without bookkeeping. */ + const inflight = new Map(); + + /** Never throws: a failed delivery just means no action line in that tab. */ + async function deliver(tabId: number, message: ActionStatusMessage): Promise { + try { + await sendToTab(tabId, message); + } catch (err) { + console.debug("[bsk action status] message dropped", err); + } + } + + return { + async start(tabId, tool, target) { + inflight.set(tabId, tool); + await deliver(tabId, { + type: ACTION_STATUS_MSG, + phase: "start", + tool, + ...(target ? { target } : {}), + }); + }, + + async end(tabId, tool) { + const resolved = tool ?? inflight.get(tabId) ?? ""; + inflight.delete(tabId); + await deliver(tabId, { type: ACTION_STATUS_MSG, phase: "end", tool: resolved }); + }, + }; +} diff --git a/apps/extension/src/lib/cursor-bridge.ts b/apps/extension/src/lib/cursor-bridge.ts new file mode 100644 index 00000000..5e047e75 --- /dev/null +++ b/apps/extension/src/lib/cursor-bridge.ts @@ -0,0 +1,159 @@ +/** + * Wire protocol and background-side helper for the cosmetic in-page agent + * cursor. + * + * CDP `Input.dispatchMouseEvent` moves the real pointer instantly and + * invisibly, so a human watching the Agent Window cannot tell that the agent + * clicked or hovered anything. The background sends one `bsk/cursor` message + * per action to the tab's content script, which glides a virtual cursor to the + * target before the CDP event fires and ripples on click. + * + * The cursor is purely cosmetic: every method swallows errors (a tab without + * the content script — chrome://, the Web Store, a closed tab — simply has no + * cursor) and is never able to fail or delay an agent action beyond its own + * animation. Coordinates are viewport CSS pixels, the same space + * `Input.dispatchMouseEvent` takes. + */ + +export const CURSOR_MSG = "bsk/cursor"; + +/** Longest wait a `move` may add before the real CDP event is dispatched. */ +export const CURSOR_MAX_ANIMATION_MS = 800; + +/** ms of glide per pixel of travel. */ +const MOVE_MS_PER_PX = 0.6; +const MOVE_MIN_MS = 120; +const MOVE_MAX_MS = 500; + +/** Viewport CSS-pixel point, identical to the CDP input coordinate space. */ +export interface CursorPoint { + x: number; + y: number; +} + +export type CursorButton = "left" | "right" | "middle"; + +export interface CursorMoveMessage { + type: typeof CURSOR_MSG; + action: "move"; + x: number; + y: number; + durationMs: number; + label?: string; +} + +export interface CursorClickMessage { + type: typeof CURSOR_MSG; + action: "click"; + x: number; + y: number; + button?: CursorButton; +} + +export interface CursorHideMessage { + type: typeof CURSOR_MSG; + action: "hide"; +} + +export type CursorMessage = CursorMoveMessage | CursorClickMessage | CursorHideMessage; + +export interface CursorAck { + type: typeof CURSOR_MSG; + ok: true; +} + +export function isCursorMessage(msg: unknown): msg is CursorMessage { + if (typeof msg !== "object" || msg === null) return false; + const m = msg as Record; + if (m.type !== CURSOR_MSG) return false; + if (m.action === "hide") return true; + if (typeof m.x !== "number" || typeof m.y !== "number") return false; + if (m.action === "move") return typeof m.durationMs === "number"; + if (m.action === "click") { + return ( + m.button === undefined || m.button === "left" || m.button === "right" || m.button === "middle" + ); + } + return false; +} + +/** + * Glide time for a move. The first move of a session (`from` unknown) is + * instant; afterwards the duration scales with the distance travelled so short + * hops stay snappy and long jumps stay visible, clamped to 120–500 ms. + */ +export function cursorMoveDuration(from: CursorPoint | null | undefined, to: CursorPoint): number { + if (!from) return 0; + const distance = Math.hypot(to.x - from.x, to.y - from.y); + if (!Number.isFinite(distance)) return 0; + return Math.round(Math.min(Math.max(distance * MOVE_MS_PER_PX, MOVE_MIN_MS), MOVE_MAX_MS)); +} + +/** Minimal `chrome.tabs.sendMessage` surface so tests can inject a fake. */ +export type CursorSendToTab = (tabId: number, message: CursorMessage) => Promise; + +const defaultSendToTab: CursorSendToTab = (tabId, message) => + chrome.tabs.sendMessage(tabId, message); + +export interface CursorVisualizer { + move( + tabId: number, + point: CursorPoint, + opts?: { durationMs?: number; label?: string }, + ): Promise; + click(tabId: number, point: CursorPoint, opts?: { button?: CursorButton }): Promise; + hide(tabId: number): Promise; +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export function createCursorVisualizer( + sendToTab: CursorSendToTab = defaultSendToTab, +): CursorVisualizer { + /** Returns whether the tab actually accepted the message. Never throws. */ + async function deliver(tabId: number, message: CursorMessage): Promise { + try { + await sendToTab(tabId, message); + return true; + } catch (err) { + // No content script in the target tab (chrome://, the Web Store, a tab + // that just closed) → there is no cursor to drive. The action proceeds. + console.debug("[bsk cursor] message dropped", err); + return false; + } + } + + return { + async move(tabId, point, opts = {}) { + const durationMs = Math.min(Math.max(opts.durationMs ?? 0, 0), CURSOR_MAX_ANIMATION_MS); + const message: CursorMoveMessage = { + type: CURSOR_MSG, + action: "move", + x: point.x, + y: point.y, + durationMs, + ...(opts.label ? { label: opts.label } : {}), + }; + if (!(await deliver(tabId, message))) return; + // Give the glide time to be seen before the real CDP event fires. + if (durationMs > 0) await delay(durationMs); + }, + + async click(tabId, point, opts = {}) { + const message: CursorClickMessage = { + type: CURSOR_MSG, + action: "click", + x: point.x, + y: point.y, + ...(opts.button ? { button: opts.button } : {}), + }; + await deliver(tabId, message); + }, + + async hide(tabId) { + await deliver(tabId, { type: CURSOR_MSG, action: "hide" }); + }, + }; +} diff --git a/apps/extension/src/lib/overlay-bridge.ts b/apps/extension/src/lib/overlay-bridge.ts index 199b5f0a..db899f02 100644 --- a/apps/extension/src/lib/overlay-bridge.ts +++ b/apps/extension/src/lib/overlay-bridge.ts @@ -12,11 +12,16 @@ * every inflight + queued tool call for that session with * `ErrorCode::UserAborted`. The Agent Window, CDP attachment, * and conversation context are preserved. + * - `{ kind: "overlay.return_control", sessionId, note }` → the user + * is done operating the page by hand; the background tells the daemon + * (`session.control_returned`) to unblock the agent, then re-shows the + * control pill + input blocker. */ export const OVERLAY_MSG_WHO_AM_I = "overlay.who_am_i"; export const OVERLAY_MSG_READY = "overlay.ready"; export const OVERLAY_MSG_INTERRUPT = "overlay.interrupt"; +export const OVERLAY_MSG_RETURN_CONTROL = "overlay.return_control"; /** * WXT shadow-host element name (`createShadowRootUi({ name })`) and the marker @@ -84,6 +89,18 @@ export interface OverlayInterruptResponse { ok: boolean; } +/** Content script → background: the user hands the page back to the agent. */ +export interface OverlayReturnControlRequest { + kind: typeof OVERLAY_MSG_RETURN_CONTROL; + sessionId: string; + /** Free-form note for the agent; may be "". */ + note: string; +} + +export interface OverlayReturnControlResponse { + ok: boolean; +} + /** Background → content: temporarily disable overlay click blocker for CDP clicks. */ export const OVERLAY_AUTOMATION_BYPASS = "bh-automation-bypass"; @@ -127,4 +144,8 @@ export function isOverlayAgentStateMessage(message: unknown): message is Overlay ); } -export type OverlayMessage = OverlayWhoAmIRequest | OverlayReadyRequest | OverlayInterruptRequest; +export type OverlayMessage = + | OverlayWhoAmIRequest + | OverlayReadyRequest + | OverlayInterruptRequest + | OverlayReturnControlRequest; diff --git a/apps/extension/src/lib/overlay-interrupt-client.ts b/apps/extension/src/lib/overlay-interrupt-client.ts index 98ed4eb7..0e196577 100644 --- a/apps/extension/src/lib/overlay-interrupt-client.ts +++ b/apps/extension/src/lib/overlay-interrupt-client.ts @@ -1,7 +1,10 @@ import { OVERLAY_MSG_INTERRUPT, + OVERLAY_MSG_RETURN_CONTROL, type OverlayInterruptRequest, type OverlayInterruptResponse, + type OverlayReturnControlRequest, + type OverlayReturnControlResponse, } from "@/lib/overlay-bridge"; const DEFAULT_TIMEOUT_MS = 2000; @@ -11,7 +14,7 @@ export interface SendInterruptOptions { } /** - * Round-trip an `overlay.interrupt` message to the background SW. + * Round-trip a `{ ok }`-shaped overlay message to the background SW. * * Resolves to `{ ok: true }` only when the SW explicitly replies * with `ok: true`. All other outcomes — undefined reply, thrown @@ -23,13 +26,11 @@ export interface SendInterruptOptions { * daemon is unreachable. The cancellation itself is fire-and-forget * on the daemon side — a slow ack does not invalidate it. */ -export async function sendInterrupt( - sendMessage: (msg: OverlayInterruptRequest) => Promise, - sessionId: string, - options: SendInterruptOptions = {}, +async function roundTrip( + sendMessage: (msg: TRequest) => Promise, + req: TRequest, + timeoutMs: number, ): Promise { - const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; - const req: OverlayInterruptRequest = { kind: OVERLAY_MSG_INTERRUPT, sessionId }; let timer: ReturnType | null = null; const timeout = new Promise((resolve) => { timer = setTimeout(() => resolve({ ok: false }), timeoutMs); @@ -46,3 +47,32 @@ export async function sendInterrupt( if (timer !== null) clearTimeout(timer); return result; } + +export async function sendInterrupt( + sendMessage: (msg: OverlayInterruptRequest) => Promise, + sessionId: string, + options: SendInterruptOptions = {}, +): Promise { + const req: OverlayInterruptRequest = { kind: OVERLAY_MSG_INTERRUPT, sessionId }; + return roundTrip(sendMessage, req, options.timeoutMs ?? DEFAULT_TIMEOUT_MS); +} + +/** + * Hand control back to the agent: the background tells the daemon + * (`session.control_returned`) to unblock it and flips the overlay back to + * `control`. `{ ok: false }` means the session is gone (or the SW is + * unreachable), in which case the caller clears its overlay. + */ +export async function sendReturnControl( + sendMessage: (msg: OverlayReturnControlRequest) => Promise, + sessionId: string, + note: string, + options: SendInterruptOptions = {}, +): Promise { + const req: OverlayReturnControlRequest = { + kind: OVERLAY_MSG_RETURN_CONTROL, + sessionId, + note, + }; + return roundTrip(sendMessage, req, options.timeoutMs ?? DEFAULT_TIMEOUT_MS); +} diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index 48ed641f..9ea3f3f7 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -342,7 +342,7 @@ describe("ToolDispatcher", () => { }); it("bypasses and restores the control overlay for an upload trigger click", async () => { - const sendMessage = vi.fn(async () => undefined); + const sendMessage = vi.fn(async (_tabId: number, _message: unknown) => undefined); vi.stubGlobal("chrome", { tabs: { get: vi.fn(async () => ({ id: 7, windowId: 4242, active: true })), @@ -432,14 +432,25 @@ describe("ToolDispatcher", () => { await vi.waitFor(() => expect(sent).toHaveLength(1)); expect(sent[0]).toMatchObject({ result: { tab_id: 7, file_names: ["test.png"] } }); - expect(sendMessage).toHaveBeenNthCalledWith(1, 7, { - type: "bh-automation-bypass", - enabled: true, - }); - expect(sendMessage).toHaveBeenNthCalledWith(2, 7, { - type: "bh-automation-bypass", - enabled: false, - }); + // The trigger click also drives the cosmetic cursor; bypass messages are + // still bracketed around it in the order below. The action-status messages + // that wrap the whole RPC sit outside them. + expect(sendMessage.mock.calls.map((call) => call[1])).toEqual([ + { type: "bsk/action-status", phase: "start", tool: "tool.upload", target: "@e1" }, + { type: "bh-automation-bypass", enabled: true }, + { + type: "bsk/cursor", + action: "move", + x: 10, + y: 10, + durationMs: 0, + label: "e1", + }, + { type: "bsk/cursor", action: "click", x: 10, y: 10, button: "left" }, + { type: "bh-automation-bypass", enabled: false }, + { type: "bsk/action-status", phase: "end", tool: "tool.upload" }, + ]); + expect(sendMessage.mock.calls.every((call) => call[0] === 7)).toBe(true); }); it("detaches CDP state before stopping a session", async () => { @@ -767,6 +778,159 @@ describe("ToolDispatcher", () => { expect(onAgentTabClaimed).toHaveBeenCalledWith(7, 1); }); + it("brackets a click with action-status start/end for the resolved tab", async () => { + const sendMessage = vi.fn(async (_tabId: number, _message: unknown) => undefined); + vi.stubGlobal("chrome", { + tabs: { + get: vi.fn(async () => ({ id: 7, windowId: 4242, active: true })), + query: vi.fn(async () => [{ id: 7, windowId: 4242, active: true }]), + sendMessage, + }, + }); + const { transport, sent, deliver } = fakeTransport(); + const sessions = new SessionManager({ + agentWindow: { + create: vi.fn(async () => 4242), + remove: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 7), + }, + }); + const ctx = await sessions.start("aa11"); + ctx.refStore.set("e3", 42, { tabId: 7 }); + const cdp = { + send: vi.fn(async (_tabId: number, method: string) => { + if (method === "DOM.getContentQuads") return { quads: [[0, 0, 20, 0, 20, 20, 0, 20]] }; + if (method === "Page.getLayoutMetrics") + return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; + if (method === "DOM.resolveNode") return { object: { objectId: "node-1" } }; + return {}; + }), + detachSession: vi.fn(async () => {}), + } as unknown as TestDispatcherCdp; + const dispatcher = new ToolDispatcher({ transport, sessions, cdp }); + dispatcher.start(); + + deliver(makeRequest("tool.click", { session_id: "aa11", ref: "e3" })); + await vi.waitFor(() => expect(sent).toHaveLength(1)); + + const statuses = sendMessage.mock.calls + .map(([, message]) => message as { type?: string }) + .filter((message) => message?.type === "bsk/action-status"); + expect(statuses).toEqual([ + { type: "bsk/action-status", phase: "start", tool: "tool.click", target: "@e3" }, + { type: "bsk/action-status", phase: "end", tool: "tool.click" }, + ]); + // The end must land after the tool finished, i.e. after the reply. + const replyIndex = sendMessage.mock.calls.findIndex( + (call) => (call[1] as { phase?: string })?.phase === "end", + ); + expect(replyIndex).toBe(sendMessage.mock.calls.length - 1); + dispatcher.stop(); + }); + + it("sends the action-status end even when the tool throws", async () => { + const sendMessage = vi.fn(async (_tabId: number, _message: unknown) => undefined); + vi.stubGlobal("chrome", { + tabs: { + get: vi.fn(async () => ({ id: 7, windowId: 4242, active: true })), + query: vi.fn(async () => [{ id: 7, windowId: 4242, active: true }]), + sendMessage, + }, + }); + const { transport, sent, deliver } = fakeTransport(); + const sessions = new SessionManager({ + agentWindow: { + create: vi.fn(async () => 4242), + remove: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 7), + }, + }); + await sessions.start("aa11"); + const cdp = { + send: vi.fn(async () => { + throw new Error("cdp blew up"); + }), + detachSession: vi.fn(async () => {}), + } as unknown as TestDispatcherCdp; + const dispatcher = new ToolDispatcher({ transport, sessions, cdp }); + dispatcher.start(); + + deliver(makeRequest("tool.navigate", { session_id: "aa11", url: "https://example.test/a" })); + await vi.waitFor(() => expect(sent).toHaveLength(1)); + await flushMicrotasks(); + + const statuses = sendMessage.mock.calls + .map(([, message]) => message as { type?: string }) + .filter((message) => message?.type === "bsk/action-status"); + expect(statuses[0]).toEqual({ + type: "bsk/action-status", + phase: "start", + tool: "tool.navigate", + target: "https://example.test/a", + }); + expect(statuses.at(-1)).toEqual({ + type: "bsk/action-status", + phase: "end", + tool: "tool.navigate", + }); + dispatcher.stop(); + }); + + it("announces nothing for non-input tools", async () => { + const sendMessage = vi.fn(async (_tabId: number, _message: unknown) => undefined); + const tab = { id: 7, windowId: 4242, active: true, url: "https://example.test" }; + vi.stubGlobal("chrome", { + tabs: { + get: vi.fn(async () => tab), + query: vi.fn(async () => [tab]), + sendMessage, + }, + }); + const { transport, sent, deliver } = fakeTransport(); + const sessions = new SessionManager({ + agentWindow: { + create: vi.fn(async () => 4242), + remove: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => 7), + }, + }); + await sessions.start("aa11"); + const cdp = { + send: vi.fn(async () => ({ cssLayoutViewport: { clientWidth: 800, clientHeight: 600 } })), + detachSession: vi.fn(async () => {}), + ensureConsoleCapture: vi.fn(async () => {}), + ensureNetworkCapture: vi.fn(async () => {}), + networkEntriesSince: vi.fn(() => ({ + tab_id: 7, + entries: [], + next_since: 0, + truncated: false, + })), + consoleEntriesSince: vi.fn(() => ({ + tab_id: 7, + entries: [], + next_since: 0, + truncated: false, + })), + setDeviceMetricsOverride: vi.fn(async () => {}), + clearDeviceMetricsOverride: vi.fn(async () => {}), + setUserAgentOverride: vi.fn(async () => {}), + setTouchEmulationEnabled: vi.fn(async () => {}), + } as unknown as TestDispatcherCdp; + const dispatcher = new ToolDispatcher({ transport, sessions, cdp }); + dispatcher.start(); + + deliver(makeRequest("tool.console", { session_id: "aa11" })); + await vi.waitFor(() => expect(sent).toHaveLength(1)); + + expect( + sendMessage.mock.calls.filter( + ([, message]) => (message as { type?: string })?.type === "bsk/action-status", + ), + ).toEqual([]); + dispatcher.stop(); + }); + it("reasserts remembered hover before follow-up work and releases only after actions", async () => { vi.stubGlobal("chrome", { tabs: { diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 00799d71..41bb7e7a 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -1,7 +1,8 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { SessionManager } from "@/session-manager/manager"; import type { CdpRunner } from "@/tools/shared"; import { + __testing__, handleBlur, handleClick, handleFill, @@ -1262,3 +1263,192 @@ describe("handleSelect", () => { expect(fake.sent.some((c) => c.method === "Runtime.callFunctionOn")).toBe(false); }); }); + +describe("cosmetic cursor wiring", () => { + afterEach(() => { + // The last-cursor-position map is module state: keep tests independent. + __testing__.clearCursorPositions(); + }); + + function fakeCursor() { + return { + move: vi.fn( + async ( + _tabId: number, + _point: { x: number; y: number }, + _opts?: { durationMs?: number; label?: string }, + ) => {}, + ), + click: vi.fn( + async (_tabId: number, _point: { x: number; y: number }, _opts?: { button?: string }) => {}, + ), + hide: vi.fn(async (_tabId: number) => {}), + }; + } + + it("moves the cursor to the click centre before the CDP mouse events", async () => { + const order: string[] = []; + const cursor = fakeCursor(); + cursor.move.mockImplementation(async () => { + order.push("cursor-move"); + }); + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e3", 1234, { tabId: 4 }); + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }), + "Input.dispatchMouseEvent": () => { + order.push("mouse"); + return {}; + }, + }); + + const res = await handleClick( + sm, + { session_id: "aa11", ref: "@e3", button: "right" }, + { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor }, + ); + + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(cursor.move).toHaveBeenCalledWith( + 4, + { x: 60, y: 40 }, + expect.objectContaining({ durationMs: 0, label: "e3" }), + ); + expect(cursor.click).toHaveBeenCalledWith(4, { x: 60, y: 40 }, { button: "right" }); + expect(order[0]).toBe("cursor-move"); + expect(order.filter((entry) => entry === "mouse")).toHaveLength(3); + }); + + it("sizes the next glide from the previous cursor position", async () => { + __testing__.clearCursorPositions(); + const cursor = fakeCursor(); + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e3", 1234, { tabId: 4 }); + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + // 100px wide first, then 90px further right: 90px * 0.6 = 54 → clamp 120. + "DOM.getContentQuads": () => ({ quads: [[0, 0, 100, 0, 100, 100, 0, 100]] }), + "Input.dispatchMouseEvent": () => ({}), + }); + const deps = { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor }; + + await handleClick(sm, { session_id: "aa11", ref: "@e3" }, deps); + await handleClick(sm, { session_id: "aa11", ref: "@e3" }, deps); + + expect(cursor.move.mock.calls[0]?.[2]).toMatchObject({ durationMs: 0 }); + expect(cursor.move.mock.calls[1]?.[2]).toMatchObject({ durationMs: 120 }); + }); + + it("keeps the click result when the cursor visualizer rejects", async () => { + const cursor = { + move: vi.fn(async (_tabId: number, _point: { x: number; y: number }) => { + throw new Error("no content script"); + }), + click: vi.fn(async (_tabId: number, _point: { x: number; y: number }) => { + throw new Error("no content script"); + }), + hide: vi.fn(async (_tabId: number) => {}), + }; + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e3", 1234, { tabId: 4 }); + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }), + "Input.dispatchMouseEvent": () => ({}), + }); + + const res = await handleClick( + sm, + { session_id: "aa11", ref: "@e3" }, + { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor }, + ); + + expect(res).toMatchObject({ tab_id: 4, x: 60, y: 40 }); + }); + + it("moves the cursor before the hover mouseMoved", async () => { + const order: string[] = []; + const cursor = fakeCursor(); + cursor.move.mockImplementation(async () => { + order.push("cursor-move"); + }); + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e3", 1234, { tabId: 4 }); + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }), + "Input.dispatchMouseEvent": () => { + order.push("mouse"); + return {}; + }, + }); + + const res = await handleHover( + sm, + { session_id: "aa11", ref: "@e3", settle_ms: 0 }, + { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor }, + ); + + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(order).toEqual(["cursor-move", "mouse"]); + // Hover never ripples. + expect(cursor.click).not.toHaveBeenCalled(); + }); + + it("does not move the cursor when the signal aborts during the glide", async () => { + const abort = new AbortController(); + const cursor = fakeCursor(); + cursor.move.mockImplementation(async () => { + abort.abort(); + }); + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e3", 1234, { tabId: 4 }); + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }), + "Input.dispatchMouseEvent": () => ({}), + }); + + const res = await handleClick( + sm, + { session_id: "aa11", ref: "@e3" }, + { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor, signal: abort.signal }, + ); + + expect(res).toMatchObject({ code: "cancelled" }); + expect(fake.sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false); + expect(cursor.click).not.toHaveBeenCalled(); + }); + + it("moves to the selector-derived centre and labels it with the selector", async () => { + const cursor = fakeCursor(); + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const fake = makeFakeCdp({ + "DOM.getDocument": () => ({ root: { nodeId: 1 } }), + "DOM.querySelector": () => ({ nodeId: 99 }), + "DOM.describeNode": () => ({ node: { backendNodeId: 7777 } }), + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[0, 0, 50, 0, 50, 50, 0, 50]] }), + "Input.dispatchMouseEvent": () => ({}), + }); + + await handleClick( + sm, + { session_id: "aa11", selector: ".btn-go" }, + { cdp: fake.cdp, tabsApi: fake.tabsApi, cursor }, + ); + + expect(cursor.move).toHaveBeenCalledWith( + 4, + { x: 25, y: 25 }, + expect.objectContaining({ label: ".btn-go" }), + ); + }); +}); diff --git a/apps/extension/src/tools/dispatcher.ts b/apps/extension/src/tools/dispatcher.ts index fdac85dd..10de06a9 100644 --- a/apps/extension/src/tools/dispatcher.ts +++ b/apps/extension/src/tools/dispatcher.ts @@ -1,3 +1,9 @@ +import { + type ActionStatusNotifier, + createActionStatusNotifier, + truncateActionTarget, +} from "@/lib/action-status-bridge"; +import { type CursorVisualizer, createCursorVisualizer } from "@/lib/cursor-bridge"; import type { InteractionPreferenceStore } from "@/lib/interaction-preferences"; import { OVERLAY_AUTOMATION_BYPASS } from "@/lib/overlay-bridge"; import { ScreenshotExports } from "@/long-screenshot/exports"; @@ -149,6 +155,10 @@ export interface DispatcherDeps { interactionPreferences?: InteractionPreferenceStore; /** i18n notification copy for `tool.request_help` (resolved per-call). */ helpNotificationCopy?: () => { title: string; body: string }; + /** Cosmetic in-page cursor shared by every input tool. */ + cursor?: CursorVisualizer; + /** Reports the in-flight input/navigation action to the control pill. */ + actionStatus?: ActionStatusNotifier; } /** @@ -179,6 +189,10 @@ export class ToolDispatcher { private readonly approveBorrow?: BorrowConfirmationApprover; private readonly interactionPreferences?: InteractionPreferenceStore; private readonly helpNotificationCopy?: () => { title: string; body: string }; + /** Cosmetic agent cursor; one instance shared by all input tools. */ + private readonly cursor: CursorVisualizer; + /** Cosmetic "current action" reporter for the control pill. */ + private readonly actionStatus: ActionStatusNotifier; private subscription: { dispose(): void } | null = null; private readonly hoverBypassTabs = new Map(); private readonly hoverLatches = new Map(); @@ -202,6 +216,8 @@ export class ToolDispatcher { this.approveBorrow = deps.approveBorrow; this.interactionPreferences = deps.interactionPreferences; this.helpNotificationCopy = deps.helpNotificationCopy; + this.cursor = deps.cursor ?? createCursorVisualizer(); + this.actionStatus = deps.actionStatus ?? createActionStatusNotifier(); } start(): void { @@ -263,6 +279,9 @@ export class ToolDispatcher { req.method === "tool.session_start" || req.method === "tool.session_stop"; const ac = new AbortController(); this.inflightAbortControllers.set(req.id, ac); + // Bracket the actual page input with a cosmetic "current action" line for + // the Agent Window's control pill. Resolution failures just mean no line. + const actionStatus = await this.beginActionStatus(req); let body: ResponseFrame; let startedSession: string | null = null; try { @@ -301,6 +320,7 @@ export class ToolDispatcher { } } finally { this.inflightAbortControllers.delete(req.id); + await actionStatus?.finish(); } let sent = true; try { @@ -565,6 +585,7 @@ export class ToolDispatcher { tabsApi: chromeTabsApi, signal, bypassOverlay, + cursor: this.cursor, } : undefined, ), @@ -583,6 +604,7 @@ export class ToolDispatcher { bypassOverlay: (tabId, enabled) => this.setHoverBypass((req.params as HoverParams).session_id, tabId, enabled), keepOverlayBypassAfterHover: true, + cursor: this.cursor, } : undefined, ); @@ -642,7 +664,14 @@ export class ToolDispatcher { handleFill( this.sessions, req.params as FillParams, - this.cdp ? { cdp: this.cdp, tabsApi: chromeTabsApi, signal } : undefined, + this.cdp + ? { + cdp: this.cdp, + tabsApi: chromeTabsApi, + signal, + cursor: this.cursor, + } + : undefined, ), signal, ); @@ -678,6 +707,7 @@ export class ToolDispatcher { tabsApi: chromeTabsApi, signal, bypassOverlay, + cursor: this.cursor, }) : Promise.resolve({ code: "unsupported", @@ -695,6 +725,7 @@ export class ToolDispatcher { tabsApi: chromeTabsApi, signal, bypassOverlay, + cursor: this.cursor, }) : Promise.resolve({ code: "unsupported", @@ -824,11 +855,65 @@ export class ToolDispatcher { tab_id?: number; }): Promise { if (params.tab_id !== undefined) return params; - const ctx = lookupSession(this.sessions, params, "hover latch"); - if (isRpcError(ctx)) return params; - const target = await resolveTargetTab(this.sessions, ctx, undefined, chromeTabsApi); - if (isRpcError(target)) return params; - return { session_id: params.session_id, tab_id: target.tabId }; + const tabId = await this.resolveSessionTabId(params); + return tabId === null ? params : { session_id: params.session_id, tab_id: tabId }; + } + + /** + * Resolve `session_id` (+ optional `tab_id`) to the concrete tab the action + * runs against — the same single lookup path every tool handler uses + * (`lookupSession` + `resolveTargetTab`). Returns `null` when the session is + * unknown or the tab cannot be resolved. + */ + private async resolveSessionTabId(params: { + session_id: string; + tab_id?: number; + }): Promise { + const ctx = lookupSession(this.sessions, params, "target tab"); + if (isRpcError(ctx)) return null; + const target = await resolveTargetTab(this.sessions, ctx, params.tab_id, chromeTabsApi); + if (isRpcError(target)) return null; + return target.tabId; + } + + /** + * Announce the in-flight browser input so the control pill can say what the + * agent is doing. Cosmetic and best-effort: an unknown session, an + * unresolvable tab or a missing content script all collapse to "no line", + * and a surprise failure never reaches the tool result. + */ + private async beginActionStatus(req: RequestFrame): Promise<{ finish(): Promise } | null> { + if (!ACTION_STATUS_METHODS.has(req.method)) return null; + const params = (req.params ?? {}) as { + session_id?: unknown; + tab_id?: number; + ref?: unknown; + selector?: unknown; + url?: unknown; + key?: unknown; + }; + const sessionId = params.session_id; + if (typeof sessionId !== "string" || sessionId.length === 0) return null; + try { + const tabId = await this.resolveSessionTabId({ + session_id: sessionId, + ...(typeof params.tab_id === "number" ? { tab_id: params.tab_id } : {}), + }); + if (tabId === null) return null; + await this.actionStatus.start(tabId, req.method, actionStatusTarget(params)); + return { + finish: async () => { + try { + await this.actionStatus.end(tabId); + } catch (err) { + console.debug("[bsk dispatcher] action status end failed", err); + } + }, + }; + } catch (err) { + console.debug("[bsk dispatcher] action status start failed", err); + return null; + } } private hoverLatchesForRequest(params: { session_id: string; tab_id?: number }): HoverLatch[] { @@ -891,6 +976,48 @@ function recordingRuntimeUnavailable(): RpcError { }; } +/** + * Tool methods whose page input / navigation the pill narrates. Mirrors the + * `sessionIdForBrowserControlMethod` classification, minus the browser-state + * tools (tab management, observe, screenshots, help, record) where the pill + * would only claim the agent is "working". + */ +const ACTION_STATUS_METHODS = new Set([ + "tool.click", + "tool.dblclick", + "tool.hover", + "tool.fill", + "tool.press", + "tool.select", + "tool.scroll", + "tool.scroll_to", + "tool.wheel", + "tool.navigate", + "tool.navigate_back", + "tool.navigate_forward", + "tool.reload", + "tool.upload", + "tool.download", + "tool.evaluate", +]); + +/** `ref` → `selector` → `url` → `key`, truncated to the pill's budget. */ +function actionStatusTarget(params: { + ref?: unknown; + selector?: unknown; + url?: unknown; + key?: unknown; +}): string | undefined { + const ref = typeof params.ref === "string" ? params.ref.trim() : ""; + if (ref) return truncateActionTarget(ref.startsWith("@") ? ref : `@${ref}`); + for (const candidate of [params.selector, params.url, params.key]) { + if (typeof candidate === "string" && candidate.length > 0) { + return truncateActionTarget(candidate); + } + } + return undefined; +} + function sessionIdForBrowserControlMethod(req: RequestFrame): string | null { switch (req.method) { case "tool.tab_create": diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index db04ca71..95c154df 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -14,6 +14,12 @@ import { isAbortError } from "./vom/capture-abort"; import { ChromiumCdp } from "@/browser-driver/chromium-cdp"; import type { CdpTarget } from "@/browser-driver/frame-graph"; +import { + type CursorPoint, + type CursorVisualizer, + createCursorVisualizer, + cursorMoveDuration, +} from "@/lib/cursor-bridge"; import type { SessionContext, SessionManager } from "@/session-manager/manager"; import type { BlurParams, @@ -61,6 +67,12 @@ export interface InteractionDeps { bypassOverlay?: (tabId: number, enabled: boolean) => Promise; /** Keep hover hit-testing active for the caller's next observation/action. */ keepOverlayBypassAfterHover?: boolean; + /** + * Cosmetic in-page cursor driven before the real CDP input fires. Optional + * and never load-bearing: a missing visualizer (or a failing one) must leave + * the tool result unchanged. + */ + cursor?: CursorVisualizer; } export interface ResolvedActionTarget { @@ -75,14 +87,86 @@ export interface ResolvedActionTarget { const DEFAULT_TIMEOUT_MS = 30_000; const DEFAULT_HOVER_SETTLE_MS = 200; -let defaultDeps: { cdp: ChromiumCdp; tabsApi: ChromeTabsApi } | null = null; -function getDefaultDeps(): { cdp: ChromiumCdp; tabsApi: ChromeTabsApi } { +let defaultDeps: { + cdp: ChromiumCdp; + tabsApi: ChromeTabsApi; + cursor: CursorVisualizer; +} | null = null; +function getDefaultDeps(): { + cdp: ChromiumCdp; + tabsApi: ChromeTabsApi; + cursor: CursorVisualizer; +} { if (!defaultDeps) { - defaultDeps = { cdp: new ChromiumCdp(), tabsApi: chromeTabsApi }; + defaultDeps = { + cdp: new ChromiumCdp(), + tabsApi: chromeTabsApi, + cursor: createCursorVisualizer(), + }; } return defaultDeps; } +/** + * Last cosmetic-cursor position per tab, used to size the next glide. Purely + * advisory. The CDP attachment id is stored alongside it: a fresh attachment + * (page load, detach/reattach) means the recorded point no longer describes + * this document, so the next glide starts from scratch. + */ +const lastCursorPoints = new Map(); + +/** Selector labels are cosmetic; keep the bridge message small. */ +const MAX_CURSOR_LABEL_CHARS = 60; + +function cursorLabel(source: { usedRef?: string; usedSelector?: string }): string | undefined { + if (source.usedRef) return source.usedRef; + const selector = source.usedSelector; + if (!selector) return undefined; + return selector.length > MAX_CURSOR_LABEL_CHARS + ? `${selector.slice(0, MAX_CURSOR_LABEL_CHARS)}…` + : selector; +} + +/** + * Glide the virtual cursor to `point` so a human watching the Agent Window + * sees the action before it happens. Never throws and never changes the tool + * result; the caller re-checks the abort signal right after this returns. + */ +async function moveCursor( + deps: InteractionDeps, + tabId: number, + point: CursorPoint, + label?: string, +): Promise { + const cursor = deps.cursor; + if (!cursor) return; + const attachmentId = deps.cdp.getAttachmentId?.(tabId); + const previous = lastCursorPoints.get(tabId); + const from = previous && previous.attachmentId === attachmentId ? previous.point : null; + lastCursorPoints.set(tabId, { + point: { x: point.x, y: point.y }, + ...(attachmentId ? { attachmentId } : {}), + }); + const durationMs = cursorMoveDuration(from, point); + try { + await cursor.move(tabId, point, { durationMs, ...(label ? { label } : {}) }); + } catch (err) { + console.debug("[bsk interaction] cursor move failed", err); + } +} + +/** Fire-and-forget click ripple; cosmetic failures never surface. */ +function rippleCursor( + deps: InteractionDeps, + tabId: number, + point: CursorPoint, + button: MouseButton, +): void { + void deps.cursor?.click(tabId, point, { button }).catch((err) => { + console.debug("[bsk interaction] cursor click failed", err); + }); +} + /** * Fold a list of `KeyModifier`s into CDP's bit layout (§4 of the * CDP Input domain): alt=1, ctrl=2, meta=4, shift=8. @@ -516,8 +600,16 @@ export async function clickResolvedTarget( } try { + // Glide the cosmetic cursor first so a human watching the Agent Window + // sees where the click is going before it lands. Cosmetic only: the + // result is unchanged whether this succeeds, fails, or is skipped. + await moveCursor(deps, target.tabId, centre, cursorLabel(resolved)); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "click aborted" }; + } const error = await dispatchClickAtPoint(target.tabId, centre, params, deps); if (error) return error; + rippleCursor(deps, target.tabId, centre, params.button ?? "left"); } finally { if (automationBypassEnabled && deps.bypassOverlay && !deps.keepOverlayBypassAfterHover) { try { @@ -663,11 +755,18 @@ async function clickVisualPoint( } const invalid = await validate(); if (invalid) return { ...invalid, data: { ...invalid.data, effect_state: "none" } }; + // Same cosmetic glide as a ref/selector click: the point already comes + // from the verified mapping, so no extra geometry resolution is needed. + await moveCursor(deps, target.tabId, point, capture.ref); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "click aborted", data: { effect_state: "none" } }; + } const error = await dispatchClickAtPoint(target.tabId, point, params, deps, async () => { await wait(32, deps.signal); // Scheduling opportunity, not a claim of page stability. return validate(); }); if (error) return error; + rippleCursor(deps, target.tabId, point, params.button ?? "left"); return attachDialogs(deps.cdp, target.tabId, dialogCursor, { tab_id: target.tabId, used_ref: capture.ref, @@ -739,6 +838,10 @@ export async function handleHover( } try { + await moveCursor(deps, target.tabId, centre, cursorLabel(node)); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "hover aborted" }; + } await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { type: "mouseMoved", x: centre.x, @@ -1015,6 +1118,11 @@ export async function handleFill( if (throwIfAborted(deps.signal)) { return { code: "cancelled", message: "fill aborted" }; } + // No cursor move here on purpose: this path never resolves element + // geometry (it focuses via `DOM.focus` and types via `Input.insertText`), + // and resolving a quad just to place a cosmetic cursor would add a CDP + // round trip to every fill. `clickResolvedTarget` glides before any + // pointer-driven fill/upload/download trigger. } catch (err) { return fillError("fill_failed", err instanceof Error ? err.message : String(err)); } @@ -1687,4 +1795,6 @@ export const __testing__ = { DEFAULT_TIMEOUT_MS, resolveBackendNode, isFillable, + /** Drop the advisory per-tab cursor positions (tests only). */ + clearCursorPositions: () => lastCursorPoints.clear(), }; diff --git a/apps/extension/src/transport/__tests__/handshake.test.ts b/apps/extension/src/transport/__tests__/handshake.test.ts index 8f4741c6..b6e666f0 100644 --- a/apps/extension/src/transport/__tests__/handshake.test.ts +++ b/apps/extension/src/transport/__tests__/handshake.test.ts @@ -66,7 +66,11 @@ function deferredFakeTransport(): { transport: Transport; emit: (frame: Protocol describe("performHandshake", () => { it("advertises the protocol compatibility boundary", () => { - expect(PROTOCOL_VERSION).toBe("1.3"); + // Must match the daemon's `PROTOCOL_VERSION` in + // `crates/bsk-cli/src/daemon/state.rs`. `1.4` added the user-takeover + // events and RPCs; an older peer drops those frames as unparseable, so + // the skew has to be visible in the handshake. + expect(PROTOCOL_VERSION).toBe("1.4"); expect(MIN_COMPATIBLE_PROTOCOL).toBe("1.0"); }); diff --git a/apps/extension/src/transport/handshake.ts b/apps/extension/src/transport/handshake.ts index c2536cb7..da4e7515 100644 --- a/apps/extension/src/transport/handshake.ts +++ b/apps/extension/src/transport/handshake.ts @@ -7,7 +7,14 @@ import type { ResponseFrame, } from "./types"; -export const PROTOCOL_VERSION = "1.3"; +/** + * Wire protocol version. Bumped to `1.4` by the user-takeover feature, which + * adds the `session.control_taken` / `session.control_returned` events and the + * `session.status` / `session.wait_control` methods. Must stay in sync with + * the daemon's `PROTOCOL_VERSION`; an older daemon drops the new events as + * unparseable frames, so the popup must be able to surface the skew. + */ +export const PROTOCOL_VERSION = "1.4"; /** * Extension semver, injected at build time from `package.json` via * Vite's `define` (see `wxt.config.ts` and `vitest.config.ts`). diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index 52ab60e8..75a71a5e 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -189,6 +189,17 @@ The result `outcome` is one of `continued`, `completed`, `cancelled`, `timed_out resume only after `continued` or `completed`. Treat `cancelled` as rejection and `timed_out` as a blocker; do not repeat that request. Observe again after control returns before using refs. +### User takeover + +The user can press "Take over" in the Agent Window at any time. Then the session is held +(`control=user`) and every browser-input tool call is rejected with `tool dispatch rejected: the +user has taken over this session (control=user)`. That rejection means **stop acting**: do not +retry, and do not route around it through another tool. Run +`bsk session wait-control --session `; it blocks until the user returns control and prints the +user's `note` (read it — it says what they changed). `bsk session status --session ` shows the +current state without blocking. Control returning does not restore your assumptions: re-snapshot +before continuing, because the user may have navigated, filled, or submitted something. + When help is disabled in the extension, make every reasonable effort to complete the task autonomously with BrowserSkill. Do not call `request-help`. If a call returns `disabled`, no human action was confirmed: re-observe and continue working rather @@ -213,7 +224,7 @@ This list of names is complete. Never invent a command outside it; read `bsk --help` for flags instead of guessing them. ```text -session start|stop|list browsers status doctor update logs +session start|stop|list|status|wait-control browsers status doctor update logs navigate navigate-back navigate-forward reload wait-for-navigation wait-ms observe snapshot get-html screenshot console network click hover wheel scroll-to focus blur fill select press evaluate diff --git a/crates/bsk-cli/src/cli/session.rs b/crates/bsk-cli/src/cli/session.rs index fd66580b..f53599e1 100644 --- a/crates/bsk-cli/src/cli/session.rs +++ b/crates/bsk-cli/src/cli/session.rs @@ -11,7 +11,10 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use anyhow::Context; -use bsk_protocol::system::{BrowserStatusEntry, SessionStatusEntry}; +use bsk_protocol::system::{ + BrowserStatusEntry, MAX_WAIT_CONTROL_MS, SessionControl, SessionReturnReceipt, + SessionStatusEntry, WaitControlOutcome, +}; use bsk_protocol::tools::ReturnFailure; use bsk_protocol::{ErrorCode, Method}; use clap::{Args, Subcommand}; @@ -19,6 +22,7 @@ use serde::{Deserialize, Serialize}; use crate::cli::ensure_daemon::ensure_daemon; use crate::cli::error::{self, CliError, Format, RenderExtras}; +use crate::cli::navigate::parse_timeout_ms; use crate::daemon::browsers::EXTENSION_CONNECT_WAIT; const SESSION_STOP_IPC_TIMEOUT: Duration = Duration::from_secs(60 * 60); @@ -46,6 +50,30 @@ pub enum SessionSub { Stop(SessionStopArgs), /// List active sessions. List, + /// Show who currently controls a session (agent or the user). + Status(SessionStatusArgs), + /// Block until the user returns control to the agent. + #[command(name = "wait-control")] + WaitControl(SessionWaitControlArgs), +} + +#[derive(Debug, Clone, Args)] +pub struct SessionStatusArgs { + /// Session id to inspect. + #[arg(long)] + pub session: String, +} + +#[derive(Debug, Clone, Args)] +pub struct SessionWaitControlArgs { + /// Session id to wait on. + #[arg(long)] + pub session: String, + + /// How long to block waiting for the user (default 5m, max 30m). + /// Accepts `5m`, `300s`, `300000ms`. + #[arg(long, default_value = "5m", value_parser = parse_timeout_ms)] + pub timeout: u32, } #[derive(Debug, Clone, Args)] @@ -155,7 +183,40 @@ struct ListReply { sessions: Vec, } -pub fn dispatch(cmd: SessionCmd, format: Format) -> Result<(), CliError> { +#[derive(Debug, Serialize)] +struct SessionStatusParams { + session_id: String, +} + +#[derive(Debug, Deserialize)] +struct SessionStatusReply { + session_id: String, + control: SessionControl, + #[serde(default)] + pending_interrupt: bool, + #[serde(default)] + held_for_ms: Option, + #[serde(default)] + last_return: Option, +} + +#[derive(Debug, Serialize)] +struct SessionWaitControlParams { + session_id: String, + timeout_ms: u64, +} + +#[derive(Debug, Deserialize)] +struct SessionWaitControlReply { + outcome: WaitControlOutcome, + control: SessionControl, + #[serde(default)] + note: Option, + #[serde(default)] + held_ms: Option, +} + +pub fn dispatch(cmd: SessionCmd, format: Format, quiet: bool) -> Result<(), CliError> { let info = ensure_daemon().context("ensure daemon is running")?; match cmd.sub { SessionSub::Start(args) => { @@ -164,6 +225,150 @@ pub fn dispatch(cmd: SessionCmd, format: Format) -> Result<(), CliError> { } SessionSub::Stop(args) => run_stop(info.sock_path, args, format), SessionSub::List => run_list(info.sock_path, format), + SessionSub::Status(args) => run_status(info.sock_path, args, format), + SessionSub::WaitControl(args) => run_wait_control(info.sock_path, args, format, quiet), + } +} + +/// `bsk session status --session `: non-consuming read of the +/// takeover state. Human output is a single `control=…` line (plus the +/// last return note when present) so an agent can grep it cheaply. +fn run_status(sock: PathBuf, args: SessionStatusArgs, format: Format) -> Result<(), CliError> { + let reply: SessionStatusReply = call( + sock, + Method::SessionStatus, + Some(SessionStatusParams { + session_id: args.session, + }), + Duration::from_secs(5), + )?; + match format { + Format::Json => { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "session_id": reply.session_id, + "control": reply.control, + "pending_interrupt": reply.pending_interrupt, + "held_for_ms": reply.held_for_ms, + "last_return": reply.last_return, + })) + .map_err(|e| CliError::Local(anyhow::anyhow!(e)))? + ); + } + Format::Human => { + print!("control={}", reply.control.as_str()); + if reply.control == SessionControl::User { + if let Some(held) = reply.held_for_ms { + print!(" (held {})", format_short_duration(held)); + } else { + print!(" (held)"); + } + } + println!(); + if let Some(receipt) = &reply.last_return + && !receipt.note.is_empty() + { + println!("last note: {}", receipt.note); + } + } + } + Ok(()) +} + +/// `bsk session wait-control --session [--timeout 5m]`: block until +/// control returns to the agent, then print the result (including the +/// user's note, which this call consumes exactly once). +/// +/// Exit codes follow the existing `request-help` convention: +/// `released` / `already_agent` are success, `timed_out` maps to the +/// timeout bucket (4) and `session_gone` to the missing-entity bucket (1). +fn run_wait_control( + sock: PathBuf, + args: SessionWaitControlArgs, + format: Format, + quiet: bool, +) -> Result<(), CliError> { + if u64::from(args.timeout) > MAX_WAIT_CONTROL_MS { + return Err(CliError::Local(anyhow::anyhow!( + "--timeout {} exceeds the {} maximum", + format_short_duration(u64::from(args.timeout)), + format_short_duration(MAX_WAIT_CONTROL_MS), + ))); + } + // Reassure the human that the command is parked on purpose rather than + // hung. Machine consumers (`--json`) and `--quiet` stay silent. + if matches!(format, Format::Human) && !quiet { + eprintln!("waiting for the user to return control…"); + } + let timeout_ms = u64::from(args.timeout); + let reply: SessionWaitControlReply = call( + sock, + Method::SessionWaitControl, + Some(SessionWaitControlParams { + session_id: args.session, + timeout_ms, + }), + // Same slack rule as `request-help`: the client must not tear the + // connection down before the daemon's (long) wait resolves. + Duration::from_millis(timeout_ms).saturating_add(Duration::from_secs(15)), + )?; + match format { + Format::Json => { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "outcome": reply.outcome, + "control": reply.control, + "note": reply.note, + "held_ms": reply.held_ms, + })) + .map_err(|e| CliError::Local(anyhow::anyhow!(e)))? + ); + } + Format::Human => { + print!( + "outcome={} control={}", + reply.outcome.as_str(), + reply.control.as_str() + ); + if let Some(note) = &reply.note { + print!(" note={note:?}"); + } + println!(); + } + } + match reply.outcome { + WaitControlOutcome::Released | WaitControlOutcome::AlreadyAgent => Ok(()), + WaitControlOutcome::TimedOut => Err(CliError::RenderedExit { + exit_code: error::exit_code_for(ErrorCode::Timeout), + }), + WaitControlOutcome::SessionGone => Err(CliError::RenderedExit { + exit_code: error::exit_code_for(ErrorCode::NotFound), + }), + } +} + +/// Human-friendly short duration for `control=user (held 12s)`. +fn format_short_duration(ms: u64) -> String { + let secs = ms / 1_000; + if secs < 60 { + format!("{secs}s") + } else if secs < 3_600 { + let (m, s) = (secs / 60, secs % 60); + if s == 0 { + format!("{m}m") + } else { + format!("{m}m{s}s") + } + } else { + let (h, rem) = (secs / 3_600, secs % 3_600); + let m = rem / 60; + if m == 0 { + format!("{h}h") + } else { + format!("{h}h{m}m") + } } } @@ -572,6 +777,32 @@ fn run_skill_sync_for_session_start(format: Format) { } } +#[cfg(test)] +mod takeover_cli_tests { + use super::*; + use crate::cli::error::exit_code_for; + + #[test] + fn short_duration_renders_seconds_minutes_hours() { + assert_eq!(format_short_duration(0), "0s"); + assert_eq!(format_short_duration(11_800), "11s"); + assert_eq!(format_short_duration(60_000), "1m"); + assert_eq!(format_short_duration(72_000), "1m12s"); + assert_eq!(format_short_duration(3_600_000), "1h"); + assert_eq!(format_short_duration(5_400_000), "1h30m"); + } + + #[test] + fn wait_control_exit_codes_follow_request_help_convention() { + // `request-help` treats a non-`continued` outcome as a non-success + // CLI status. `wait-control` mirrors that: timed_out maps to the + // timeout bucket (4) and session_gone to the missing-entity + // bucket (1). `released`/`already_agent` return `Ok`. + assert_eq!(exit_code_for(ErrorCode::Timeout), 4); + assert_eq!(exit_code_for(ErrorCode::NotFound), 1); + } +} + #[cfg(test)] mod start_params_tests { use super::*; diff --git a/crates/bsk-cli/src/daemon/ipc.rs b/crates/bsk-cli/src/daemon/ipc.rs index 69daf58c..ddf65604 100644 --- a/crates/bsk-cli/src/daemon/ipc.rs +++ b/crates/bsk-cli/src/daemon/ipc.rs @@ -23,8 +23,10 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use bsk_protocol::system::{ - BrowserListParams, BrowserStatusEntry, SessionStatusEntry, StatusParams, StatusResult, - VersionSkewEntry, + BrowserListParams, BrowserStatusEntry, DEFAULT_WAIT_CONTROL_MS, MAX_WAIT_CONTROL_MS, + SessionControl, SessionStatusEntry, SessionStatusParams, SessionStatusResult, + SessionWaitControlParams, SessionWaitControlResult, StatusParams, StatusResult, + VersionSkewEntry, WaitControlOutcome, }; use bsk_protocol::tools::{ DownloadParams, DownloadResult, ReturnFailure, TransferBeginParams, TransferIdParams, @@ -42,6 +44,7 @@ use tracing::{debug, warn}; use super::abort::AbortRegistry; use super::queue::{DEFAULT_TOOL_TIMEOUT, DispatchError}; +use super::session_interrupt::WaitControlResolution; use super::sessions::{ AgentWindowOptions, SessionId, StartSessionError, StopSessionError, snapshot_status_entries, start_session, stop_session, @@ -73,6 +76,11 @@ const MAX_BROWSER_WAIT: Duration = Duration::from_secs(60); const DEFAULT_SESSION_STOP_TIMEOUT: Duration = Duration::from_secs(DEFAULT_TOOL_TIMEOUT.as_secs() + DEFAULT_RPC_TIMEOUT.as_secs() + 5); +/// Exact rejection message emitted when a tool dispatch is gated by a +/// user takeover (`control=user`). Locked by a test because agents parse +/// it to discover the `wait-control` recovery path. +pub const USER_TAKEOVER_REJECTION: &str = "tool dispatch rejected: the user has taken over this session (control=user). Do not retry; run `bsk session wait-control --session ` and continue only after it returns control=agent."; + /// Snapshot of daemon-side bookkeeping needed to answer `system.status`. /// /// The lifecycle / process metadata (pid, ws_port, sock_path, …) is owned @@ -249,6 +257,16 @@ pub fn full_handler(status: DaemonStatus, state: Arc) -> RpcHandler Err(e) => ResponseBody::Err(e), }, Method::SessionList => handle_session_list(&state), + Method::SessionStatus => match handle_session_status(&state, params) { + Ok(v) => ResponseBody::Ok(v), + Err(e) => ResponseBody::Err(e), + }, + Method::SessionWaitControl => { + match handle_session_wait_control(&state, rpc_id, params).await { + Ok(v) => ResponseBody::Ok(v), + Err(e) => ResponseBody::Err(e), + } + } Method::BrowserList => match handle_browser_list(&state, params).await { Ok(v) => ResponseBody::Ok(v), Err(e) => ResponseBody::Err(e), @@ -362,6 +380,22 @@ async fn handle_tool_dispatch( // page state before asking the user, or from cleanly tearing down the // session. Classification lives on `Method::effect()` so adding a new // tool variant requires an explicit classification call. + // + // Checked *before* the one-shot marker: a takeover is a standing + // instruction, so it must win over (and not be consumed by) the + // legacy marker. The held flag is deliberately NOT consumed here — + // it clears only on `session.control_returned` or session teardown. + if method.requires_interrupt_gate() && state.session_interrupts.is_held(&session_id) { + return ResponseBody::Err(RpcError { + code: ErrorCode::UserAborted, + message: USER_TAKEOVER_REJECTION.into(), + data: Some(serde_json::json!({ + "reason": "user_takeover", + "control": "user", + "session_id": session_id.0, + })), + }); + } if method.requires_interrupt_gate() && state.session_interrupts.try_consume(&session_id) { return ResponseBody::Err(RpcError { code: ErrorCode::UserAborted, @@ -1238,6 +1272,151 @@ fn handle_session_list(state: &Arc) -> ResponseBody { ResponseBody::Ok(serde_json::to_value(SessionListResult { sessions }).unwrap_or(Value::Null)) } +/// `session.status`: non-consuming read of a session's takeover state. +/// +/// Daemon-local (never forwarded to the extension) so it works while +/// the extension is busy or the session is held. Returns `not_found` for +/// an unknown session id so the caller can distinguish "agent in +/// control" from "session gone" — both of which have +/// `control = agent`. +fn handle_session_status(state: &Arc, params: Value) -> Result { + let params: SessionStatusParams = serde_json::from_value(params).map_err(|err| RpcError { + code: ErrorCode::InvalidParams, + message: format!("session.status requires {{session_id}}: {err}"), + data: None, + })?; + if params.session_id.is_empty() { + return Err(invalid_params( + "session.status requires a non-empty session_id", + )); + } + let sid = SessionId(params.session_id.clone()); + if state.sessions.get(&sid).is_none() { + return Err(RpcError { + code: ErrorCode::NotFound, + message: format!("session {} unknown", params.session_id), + data: Some(serde_json::json!({ + "reason": "session_gone", + "session_id": params.session_id, + })), + }); + } + let snap = state.session_interrupts.snapshot(&sid); + let result = SessionStatusResult { + session_id: params.session_id, + control: snap.control, + pending_interrupt: snap.pending_interrupt, + held_for_ms: snap.held_for_ms, + last_return: snap.last_return, + }; + Ok(serde_json::to_value(result).unwrap_or(Value::Null)) +} + +/// `session.wait_control`: block until control returns to `agent`, +/// the timeout expires, or the session disappears. +/// +/// Registered against [`AbortRegistry`] under the CLI's `rpc_id` so a +/// SIGINT-driven `cancel { rpc_id }` (the same path `request-help` +/// uses) can unblock the waiter instead of leaving the CLI hanging. +/// +/// `released` consumes the return receipt, so the user's note reaches the +/// agent exactly once. A winner/timed-out race is resolved inside the +/// registry under one lock, so two concurrent waiters can never both +/// report `released`. +async fn handle_session_wait_control( + state: &Arc, + rpc_id: RpcId, + params: Value, +) -> Result { + let params: SessionWaitControlParams = + serde_json::from_value(params).map_err(|err| RpcError { + code: ErrorCode::InvalidParams, + message: format!("session.wait_control requires {{session_id, timeout_ms?}}: {err}"), + data: None, + })?; + if params.session_id.is_empty() { + return Err(invalid_params( + "session.wait_control requires a non-empty session_id", + )); + } + let timeout_ms = params.timeout_ms.unwrap_or(DEFAULT_WAIT_CONTROL_MS); + if timeout_ms > MAX_WAIT_CONTROL_MS { + return Err(invalid_params(format!( + "timeout_ms {timeout_ms} exceeds the {} ms maximum", + MAX_WAIT_CONTROL_MS + ))); + } + let sid = SessionId(params.session_id.clone()); + // A timeout of zero (or a session that is already gone) resolves as a + // fast snapshot rather than parking a task. + if state.sessions.get(&sid).is_none() { + let snap = state.session_interrupts.snapshot(&sid); + let result = SessionWaitControlResult { + outcome: WaitControlOutcome::SessionGone, + control: snap.control, + note: None, + held_ms: None, + }; + return Ok(serde_json::to_value(result).unwrap_or(Value::Null)); + } + let abort_guard = state + .abort_registry + .register(rpc_id) + .map_err(|err| RpcError { + code: ErrorCode::ProtocolError, + message: format!("session.wait_control cancellation registration failed: {err:?}"), + data: None, + })?; + let cancel = abort_guard.token().clone(); + let deadline = tokio::time::Instant::now() + Duration::from_millis(timeout_ms); + let sessions = Arc::clone(&state.sessions); + let sid_for_exists = sid.clone(); + let resolution = tokio::select! { + resolution = state.session_interrupts.wait_for_release(&sid, deadline, move || { + sessions.get(&sid_for_exists).is_some() + }) => resolution, + _ = cancel.cancelled() => { + drop(abort_guard); + return Err(RpcError { + code: ErrorCode::Cancelled, + message: "session.wait_control cancelled".into(), + data: None, + }); + } + }; + drop(abort_guard); + // `session_gone` needs no hold/receipt data; re-read the (possibly + // removed) control entry only for the remaining outcomes. + let snap = state.session_interrupts.snapshot(&sid); + let result = match resolution { + WaitControlResolution::Released(receipt) => SessionWaitControlResult { + outcome: WaitControlOutcome::Released, + control: SessionControl::Agent, + note: Some(receipt.note), + held_ms: Some(receipt.held_ms), + }, + WaitControlResolution::AlreadyAgent => SessionWaitControlResult { + outcome: WaitControlOutcome::AlreadyAgent, + control: SessionControl::Agent, + note: None, + held_ms: None, + }, + WaitControlResolution::TimedOut { held_for_ms } => SessionWaitControlResult { + outcome: WaitControlOutcome::TimedOut, + control: SessionControl::User, + note: None, + held_ms: Some(held_for_ms), + }, + WaitControlResolution::SessionGone => SessionWaitControlResult { + outcome: WaitControlOutcome::SessionGone, + control: snap.control, + note: None, + held_ms: None, + }, + }; + Ok(serde_json::to_value(result).unwrap_or(Value::Null)) +} + async fn handle_browser_list(state: &Arc, params: Value) -> Result { let params: BrowserListParams = parse_params_or_default(params)?; maybe_wait_for_browser(state, params.wait_for_browser_ms).await; @@ -2031,4 +2210,456 @@ mod tests { let _ = tx.send(()); let _ = server.await; } + + // ----- user-takeover gate + session.status / session.wait_control ----- + + /// Build a `DaemonState` with one live session (id `abcd`) owned by a + /// browser that is *not* registered, so nothing can be forwarded to an + /// extension — which is exactly what lets these tests assert "rejected + /// without reaching the extension". + fn state_with_session() -> (Arc, SessionId) { + use crate::daemon::start::DaemonConfig; + let state = Arc::new(DaemonState::new(DaemonConfig::new(0))); + // Reserve under a browser id; `get` only checks the session table. + let sid = state + .sessions + .reserve_id(crate::daemon::browsers::BrowserId("b-1".into()), 8, || 0) + .expect("reserved session id"); + (state, sid) + } + + #[test] + fn user_takeover_rejection_message_is_locked() { + // Agents parse this string to discover the `wait-control` recovery + // path; lock it verbatim. + assert_eq!( + USER_TAKEOVER_REJECTION, + "tool dispatch rejected: the user has taken over this session (control=user). Do not retry; run `bsk session wait-control --session ` and continue only after it returns control=agent." + ); + } + + #[tokio::test] + async fn held_session_rejects_input_tool_with_exact_message() { + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + for method in [ + Method::ToolClick, + Method::ToolObserve, + Method::ToolScreenshotFullPage, + Method::ToolNavigate, + ] { + let body = handle_tool_dispatch( + &state, + "rpc-held".into(), + method.clone(), + serde_json::json!({"session_id": sid.0}), + ) + .await; + match body { + ResponseBody::Err(err) => { + assert_eq!(err.code, ErrorCode::UserAborted, "method {method:?}"); + assert_eq!(err.message, USER_TAKEOVER_REJECTION, "method {method:?}"); + let data = err.data.expect("structured takeover payload"); + assert_eq!(data["reason"], serde_json::json!("user_takeover")); + assert_eq!(data["control"], serde_json::json!("user")); + } + other => panic!("expected held rejection for {method:?}, got {other:?}"), + } + } + } + + #[tokio::test] + async fn returned_control_clears_the_one_shot_marker_so_the_next_input_tool_passes() { + // Real-session bug: the takeover handshake sets *both* the held flag + // and the legacy one-shot marker, then `control_returned` cleared + // only the flag. The agent's first input tool after the return was + // therefore rejected once with "pending user interrupt" even though + // `wait_control` had already told it control=agent. + let (state, sid) = state_with_session(); + state.session_interrupts.mark(&sid); + state.session_interrupts.take_control(&sid); + state + .session_interrupts + .release_control(&sid, Some("done".into())); + + assert!( + !state.session_interrupts.is_pending(&sid), + "returning control must drop the one-shot marker" + ); + assert!(!state.session_interrupts.is_held(&sid)); + + // `session.status` agrees: control=agent, no pending interrupt. + let value = handle_session_status(&state, serde_json::json!({"session_id": sid.0})) + .expect("status for a live session"); + let result: SessionStatusResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.control, SessionControl::Agent); + assert!(!result.pending_interrupt); + + // The gate must now pass. Whatever failure comes back (no dispatch + // queue / no owning browser) must be neither of the two rejections. + let body = handle_tool_dispatch( + &state, + "rpc-after-return".into(), + Method::ToolClick, + serde_json::json!({"session_id": sid.0}), + ) + .await; + if let ResponseBody::Err(err) = &body { + assert_ne!(err.code, ErrorCode::UserAborted, "gate must pass: {err:?}"); + assert!( + !err.message.contains("pending user interrupt"), + "no leftover marker may fire after a return: {err:?}" + ); + assert_ne!(err.message, USER_TAKEOVER_REJECTION); + } + } + + #[tokio::test] + async fn held_rejection_is_not_consuming() { + // Unlike the one-shot marker, the held flag must survive an + // unbounded number of rejected dispatches and only clear on an + // explicit `control_returned`. + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + for i in 0..5 { + let body = handle_tool_dispatch( + &state, + format!("rpc-held-{i}"), + Method::ToolClick, + serde_json::json!({"session_id": sid.0}), + ) + .await; + assert!( + matches!(body, ResponseBody::Err(ref e) if e.code == ErrorCode::UserAborted), + "rejection {i} must be UserAborted" + ); + assert!( + state.session_interrupts.is_held(&sid), + "held flag must survive rejection {i}" + ); + } + state + .session_interrupts + .release_control(&sid, Some("done".into())); + assert!(!state.session_interrupts.is_held(&sid)); + } + + #[tokio::test] + async fn held_session_lets_passive_read_pass() { + // Passive reads must stay transparent: the agent needs to observe + // the page before asking the user a coherent question. + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + // `tool.snapshot` is a passive read; it must NOT be rejected by the + // takeover gate. It will fail later for lack of a dispatch queue, + // but never with the takeover message. + let body = handle_tool_dispatch( + &state, + "rpc-passive".into(), + Method::ToolSnapshot, + serde_json::json!({"session_id": sid.0}), + ) + .await; + if let ResponseBody::Err(err) = &body { + assert_ne!( + err.message, USER_TAKEOVER_REJECTION, + "passive reads must not be takeover-gated" + ); + assert_ne!(err.code, ErrorCode::UserAborted); + } + } + + #[tokio::test] + async fn one_shot_marker_still_rejects_with_interrupt_message() { + // Regression guard: the legacy marker path must keep working and + // keep its own message after the takeover gate was inserted before it. + let (state, sid) = state_with_session(); + state.session_interrupts.mark(&sid); + let body = handle_tool_dispatch( + &state, + "rpc-once".into(), + Method::ToolClick, + serde_json::json!({"session_id": sid.0}), + ) + .await; + match body { + ResponseBody::Err(err) => { + assert_eq!(err.code, ErrorCode::UserAborted); + assert!( + err.message + .starts_with("tool dispatch rejected: pending user interrupt") + ); + assert_ne!(err.message, USER_TAKEOVER_REJECTION); + } + other => panic!("expected interrupt rejection, got {other:?}"), + } + assert!( + !state.session_interrupts.is_pending(&sid), + "one-shot marker is consumed by the rejection" + ); + } + + #[tokio::test] + async fn control_taken_supersedes_pending_interrupt_marker() { + // A takeover sets both the held flag and the one-shot marker. The + // takeover message must win and the marker must remain set, so that + // once control returns an old pending stop still fires. + let (state, sid) = state_with_session(); + state.session_interrupts.mark(&sid); + state.session_interrupts.take_control(&sid); + let body = handle_tool_dispatch( + &state, + "rpc-both".into(), + Method::ToolClick, + serde_json::json!({"session_id": sid.0}), + ) + .await; + match body { + ResponseBody::Err(err) => assert_eq!(err.message, USER_TAKEOVER_REJECTION), + other => panic!("expected takeover rejection, got {other:?}"), + } + assert!( + state.session_interrupts.is_pending(&sid), + "takeover rejection must not consume the one-shot marker" + ); + } + + #[tokio::test] + async fn session_status_reports_agent_by_default() { + let (state, sid) = state_with_session(); + let value = handle_session_status(&state, serde_json::json!({"session_id": sid.0})) + .expect("status for a live session"); + let result: SessionStatusResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.control, SessionControl::Agent); + assert!(!result.pending_interrupt); + assert!(result.held_for_ms.is_none()); + assert!(result.last_return.is_none()); + } + + #[tokio::test] + async fn session_status_is_non_consuming() { + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + state + .session_interrupts + .release_control(&sid, Some("looked at the page".into())); + // A standalone Stop marker set *after* the return is unrelated to the + // takeover, so it must survive repeated status reads. (The takeover's + // own marker is dropped by `release_control`.) + state.session_interrupts.mark(&sid); + + for _ in 0..3 { + let value = handle_session_status(&state, serde_json::json!({"session_id": sid.0})) + .expect("status"); + let result: SessionStatusResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.control, SessionControl::Agent); + assert!( + result.pending_interrupt, + "status must not consume the marker" + ); + let receipt = result.last_return.as_ref().expect("receipt still visible"); + assert_eq!(receipt.note, "looked at the page"); + } + } + + #[tokio::test] + async fn session_status_reports_hold_duration_while_held() { + let (state, sid) = state_with_session(); + state + .session_interrupts + .force_held_since(&sid, 1_600_000_000_000); + let value = handle_session_status(&state, serde_json::json!({"session_id": sid.0})) + .expect("status"); + let result: SessionStatusResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.control, SessionControl::User); + assert!(result.held_for_ms.is_some_and(|ms| ms > 0)); + } + + #[tokio::test] + async fn session_status_unknown_session_is_not_found() { + let (state, _sid) = state_with_session(); + let err = handle_session_status(&state, serde_json::json!({"session_id": "ghost"})) + .expect_err("unknown session must be not_found"); + assert_eq!(err.code, ErrorCode::NotFound); + assert_eq!( + err.data.unwrap()["reason"], + serde_json::json!("session_gone") + ); + } + + #[tokio::test] + async fn session_status_requires_session_id() { + let (state, _sid) = state_with_session(); + for params in [serde_json::json!({}), serde_json::json!({"session_id": ""})] { + let err = handle_session_status(&state, params).expect_err("invalid params"); + assert_eq!(err.code, ErrorCode::InvalidParams); + } + } + + #[tokio::test] + async fn wait_control_returns_already_agent_when_not_held() { + let (state, sid) = state_with_session(); + let value = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": 50}), + ) + .await + .expect("wait_control"); + let result: SessionWaitControlResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.outcome, WaitControlOutcome::AlreadyAgent); + assert_eq!(result.control, SessionControl::Agent); + assert!(result.note.is_none()); + } + + #[tokio::test] + async fn wait_control_released_path_returns_note() { + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + let state_for_release = Arc::clone(&state); + let sid_for_release = sid.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(20)).await; + state_for_release + .session_interrupts + .release_control(&sid_for_release, Some("handled the captcha".into())); + }); + let value = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": 2_000}), + ) + .await + .expect("wait_control"); + let result: SessionWaitControlResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.outcome, WaitControlOutcome::Released); + assert_eq!(result.control, SessionControl::Agent); + assert_eq!(result.note.as_deref(), Some("handled the captcha")); + assert!(result.held_ms.is_some()); + + // The receipt is delivered exactly once. + let second = handle_session_wait_control( + &state, + "rpc-wait-2".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": 50}), + ) + .await + .expect("wait_control"); + let second: SessionWaitControlResult = serde_json::from_value(second).unwrap(); + assert_eq!(second.outcome, WaitControlOutcome::AlreadyAgent); + assert!(second.note.is_none()); + } + + #[tokio::test] + async fn wait_control_timeout_path_keeps_hold() { + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + let value = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": 50}), + ) + .await + .expect("wait_control"); + let result: SessionWaitControlResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.outcome, WaitControlOutcome::TimedOut); + assert_eq!(result.control, SessionControl::User); + assert!(result.held_ms.is_some_and(|ms| ms <= 5_000)); + assert!( + state.session_interrupts.is_held(&sid), + "timeout must not clear the user's hold" + ); + } + + #[tokio::test] + async fn wait_control_session_gone_path() { + let (state, _sid) = state_with_session(); + let value = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": "ghost", "timeout_ms": 50}), + ) + .await + .expect("wait_control"); + let result: SessionWaitControlResult = serde_json::from_value(value).unwrap(); + assert_eq!(result.outcome, WaitControlOutcome::SessionGone); + } + + #[tokio::test] + async fn wait_control_cancel_unblocks_the_waiter() { + let (state, sid) = state_with_session(); + state.session_interrupts.take_control(&sid); + let waiter = { + let state = Arc::clone(&state); + let sid = sid.clone(); + tokio::spawn(async move { + handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": 30_000}), + ) + .await + }) + }; + tokio::time::sleep(Duration::from_millis(20)).await; + assert!(state.abort_registry.cancel(&"rpc-wait".to_string())); + let value = tokio::time::timeout(Duration::from_secs(2), waiter) + .await + .expect("cancel must unblock the waiter promptly") + .unwrap() + .expect_err("cancelled wait returns an error"); + assert_eq!(value.code, ErrorCode::Cancelled); + assert!( + state.abort_registry.is_empty(), + "abort guard must clean up on the cancel path" + ); + } + + #[tokio::test] + async fn wait_control_enforces_the_30_minute_maximum() { + let (state, sid) = state_with_session(); + let err = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0, "timeout_ms": MAX_WAIT_CONTROL_MS + 1}), + ) + .await + .expect_err("over-max timeout rejected"); + assert_eq!(err.code, ErrorCode::InvalidParams); + assert!(err.message.contains("exceeds")); + assert!(state.abort_registry.is_empty(), "no token leaked"); + } + + #[tokio::test] + async fn wait_control_defaults_to_five_minutes() { + assert_eq!(DEFAULT_WAIT_CONTROL_MS, 5 * 60 * 1_000); + assert_eq!(MAX_WAIT_CONTROL_MS, 30 * 60 * 1_000); + // A request without `timeout_ms` must be accepted (and must not be + // rejected for exceeding the cap). + let (state, sid) = state_with_session(); + let value = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"session_id": sid.0}), + ) + .await; + // Session is `agent`, so this resolves immediately rather than + // waiting the full default. + let result: SessionWaitControlResult = + serde_json::from_value(value.expect("default timeout accepted")).unwrap(); + assert_eq!(result.outcome, WaitControlOutcome::AlreadyAgent); + } + + #[tokio::test] + async fn wait_control_requires_session_id() { + let (state, _sid) = state_with_session(); + let err = handle_session_wait_control( + &state, + "rpc-wait".into(), + serde_json::json!({"timeout_ms": 10}), + ) + .await + .expect_err("missing session_id"); + assert_eq!(err.code, ErrorCode::InvalidParams); + } } diff --git a/crates/bsk-cli/src/daemon/session_interrupt.rs b/crates/bsk-cli/src/daemon/session_interrupt.rs index ce9ab696..d9560f37 100644 --- a/crates/bsk-cli/src/daemon/session_interrupt.rs +++ b/crates/bsk-cli/src/daemon/session_interrupt.rs @@ -1,4 +1,7 @@ -//! Per-session "pending interrupt" message. +//! Per-session transient control state: the one-shot "pending interrupt" +//! marker and the user-takeover ("held") flag. +//! +//! ## Pending interrupt //! //! Holds a single-use marker per `SessionId` indicating that the //! user has clicked the agent-window mask's stop button. The next @@ -7,31 +10,142 @@ //! RPCs pass through transparently and do not consume the marker. //! //! The marker is single-use and has no expiry: it sits in the -//! registry until consumed by an input-dispatching call or until the -//! session is torn down. This lets the user's interrupt survive an LLM +//! registry until consumed by an input-dispatching call, dropped by a +//! `session.control_returned` (see below), or until the session is +//! torn down. This lets the user's interrupt survive an LLM //! thinking phase of arbitrary length — the v1 time-window //! mechanism dropped interrupts whenever the LLM took longer to //! respond than the window allowed. //! -//! Independent of `SessionRegistry` because the signal is a -//! transient runtime control state, not a session lifecycle -//! attribute. +//! When the takeover handshake sets *both* states, the return event +//! must clear *both*: `wait_control` already told the agent control is +//! back, so leaving the one-shot marker behind would reject the agent's +//! very next input tool once with a spurious "pending user interrupt". +//! +//! ## User takeover ("held") +//! +//! A session also carries a *control* state: `agent` (default) or +//! `user`. The extension emits `session.control_taken` when the user +//! presses "接管/Take over" and `session.control_returned` when they +//! press "交还/Return to agent". +//! +//! While a session is held, every interrupt-gated `tool.*` call is +//! rejected with `ErrorCode::UserAborted` **without consuming** the +//! held flag: the rejection repeats until control actually returns. +//! The flag is cleared only by an explicit `session.control_returned`, +//! or by session teardown (stop / window close / browser disconnect) — +//! all of which funnel through [`SessionInterruptRegistry::drop_session`]. +//! +//! `session.control_returned` also stores a *return receipt* (the +//! user's note + how long they held control) which +//! `session.wait_control` consumes exactly once so the note is +//! delivered to the agent a single time. `session.status` can still +//! read the receipt until it is consumed. +//! +//! Both states live in one registry, under one mutex, so a single +//! `drop_session` call covers every teardown route. Waiters park on a +//! per-session [`tokio::sync::watch`] channel — the mutex is never held +//! across an `.await`. +//! +//! Independent of `SessionRegistry` because this is transient runtime +//! control state, not a session lifecycle attribute. + +use std::collections::{HashMap, HashSet}; +use std::sync::{Mutex, MutexGuard}; +use std::time::{SystemTime, UNIX_EPOCH}; -use std::collections::HashSet; -use std::sync::Mutex; +use bsk_protocol::SessionControl; +use bsk_protocol::system::SessionReturnReceipt; +use tokio::sync::watch; use super::sessions::SessionId; +/// Non-consuming view of a session's takeover state. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ControlSnapshot { + pub control: SessionControl, + pub pending_interrupt: bool, + /// How long the user has held control, in milliseconds. `None` while + /// control is `agent`. + pub held_for_ms: Option, + /// Most recent return receipt until it is consumed by + /// [`SessionInterruptRegistry::consume_return`]. + pub last_return: Option, +} + +/// One session's control bookkeeping. +struct ControlState { + /// Epoch milliseconds at which the user took over, while held. + held_since_ms: Option, + /// Receipt from the latest return, until consumed. + last_return: Option, + /// Monotonic counter bumped on every change so waiters wake up. The + /// value itself is meaningless; only "did it change" matters. + version: u64, + /// Broadcast channel used to wake `wait_control` waiters. + signal: watch::Sender, +} + +impl ControlState { + fn new() -> Self { + let (signal, _rx) = watch::channel(0); + Self { + held_since_ms: None, + last_return: None, + version: 0, + signal, + } + } + + fn bump(&mut self) { + self.version = self.version.wrapping_add(1); + // A send failure only means no waiter is currently subscribed. + let _ = self.signal.send(self.version); + } +} + +#[derive(Default)] +struct Inner { + pending: HashSet, + control: HashMap, +} + +/// Atomic result of probing the control state for a waiter. +enum PollOutcome { + Released(SessionReturnReceipt), + AlreadyAgent, + StillHeld { held_for_ms: u64 }, +} + +/// Why [`SessionInterruptRegistry::wait_for_release`] returned. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WaitControlResolution { + /// The user returned control while we waited; carries the consumed + /// return receipt. + Released(SessionReturnReceipt), + /// Control was already `agent` (or a racing waiter consumed the + /// receipt first). + AlreadyAgent, + /// The wait expired with control still held by the user. + TimedOut { held_for_ms: u64 }, + /// The session disappeared before/while waiting. + SessionGone, +} + #[derive(Default)] pub struct SessionInterruptRegistry { - inner: Mutex>, + inner: Mutex, } impl std::fmt::Debug for SessionInterruptRegistry { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let len = self.inner.lock().map(|g| g.len()).unwrap_or(0); + let (pending, control) = { + let guard = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + (guard.pending.len(), guard.control.len()) + }; f.debug_struct("SessionInterruptRegistry") - .field("len", &len) + .field("pending", &pending) + .field("control", &control) .finish() } } @@ -41,18 +155,24 @@ impl SessionInterruptRegistry { Self::default() } + fn lock(&self) -> MutexGuard<'_, Inner> { + self.inner + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + } + + // ----- one-shot pending interrupt (unchanged semantics) ----- + /// Mark `sid` as having a pending interrupt. Idempotent — /// repeated marks are a no-op (the marker is a single-use flag, /// not a counter). pub fn mark(&self, sid: &SessionId) { - let mut guard = self.inner.lock().expect("session_interrupt poisoned"); - guard.insert(sid.clone()); + self.lock().pending.insert(sid.clone()); } /// Whether `sid` currently has a pending interrupt marker. pub fn is_pending(&self, sid: &SessionId) -> bool { - let guard = self.inner.lock().expect("session_interrupt poisoned"); - guard.contains(sid) + self.lock().pending.contains(sid) } /// Probe + consume. If `sid` has a pending interrupt, remove it @@ -63,27 +183,284 @@ impl SessionInterruptRegistry { /// `try_consume` calls for the same session return `false` /// until the session is marked again. pub fn try_consume(&self, sid: &SessionId) -> bool { - let mut guard = self.inner.lock().expect("session_interrupt poisoned"); - guard.remove(sid) + self.lock().pending.remove(sid) + } + + // ----- user takeover ("held") ----- + + /// Mark `sid` as held by the user. Idempotent: a repeated + /// `control_taken` (e.g. an extension retry) keeps the original + /// hold start so `held_for_ms` does not reset. + pub fn take_control(&self, sid: &SessionId) { + let mut guard = self.lock(); + let entry = guard + .control + .entry(sid.clone()) + .or_insert_with(ControlState::new); + if entry.held_since_ms.is_none() { + entry.held_since_ms = Some(now_ms()); + } + entry.bump(); } - /// Drop any pending entry for `sid`. **Every** session-teardown - /// path MUST call this so a session torn down while a signal - /// was hot does not leak the entry into the registry - /// indefinitely. Current call sites: + /// Whether the user currently holds control of `sid`. + pub fn is_held(&self, sid: &SessionId) -> bool { + self.lock() + .control + .get(sid) + .is_some_and(|entry| entry.held_since_ms.is_some()) + } + + /// Return control to the agent and record the return receipt. + /// + /// Also drops the legacy one-shot "pending interrupt" marker for + /// `sid`: a takeover sets both states, so returning must clear both, + /// otherwise the next input tool after the return is rejected once + /// with "pending user interrupt" even though `wait_control` already + /// told the agent control is back. The standalone Stop/interrupt flow + /// (a bare [`SessionInterruptRegistry::mark`] with no takeover) is + /// unaffected — it never reaches this method. /// - /// * `stop_session` (session.stop RPC) + /// Returns `true` when the session was actually held. Returning from + /// a session that was not held still stores a receipt with + /// `held_ms = 0` so a `control_returned` that races (or predates) the + /// daemon's view of the takeover never loses the user's note. + /// Wakes every `wait_control` waiter. + pub fn release_control(&self, sid: &SessionId, note: Option) -> bool { + let now = now_ms(); + let mut guard = self.lock(); + guard.pending.remove(sid); + let entry = guard + .control + .entry(sid.clone()) + .or_insert_with(ControlState::new); + let held_since = entry.held_since_ms.take(); + let was_held = held_since.is_some(); + let held_ms = held_since + .map(|start| now.saturating_sub(start)) + .unwrap_or(0); + entry.last_return = Some(SessionReturnReceipt { + note: note.unwrap_or_default(), + held_ms, + returned_at: format_rfc3339_ms(now), + }); + entry.bump(); + was_held + } + + /// Non-consuming read used by `session.status`. + pub fn snapshot(&self, sid: &SessionId) -> ControlSnapshot { + let guard = self.lock(); + let Some(entry) = guard.control.get(sid) else { + return ControlSnapshot { + control: SessionControl::Agent, + pending_interrupt: guard.pending.contains(sid), + held_for_ms: None, + last_return: None, + }; + }; + ControlSnapshot { + control: control_of(entry), + pending_interrupt: guard.pending.contains(sid), + held_for_ms: entry + .held_since_ms + .map(|start| now_ms().saturating_sub(start)), + last_return: entry.last_return.clone(), + } + } + + /// Take the last return receipt, if any. Single-use: the second call + /// returns `None` until the user returns control again. Used by + /// `session.wait_control` so the note is delivered exactly once. + pub fn consume_return(&self, sid: &SessionId) -> Option { + self.lock() + .control + .get_mut(sid) + .and_then(|entry| entry.last_return.take()) + } + + /// Subscribe to control changes for `sid`, creating the bookkeeping + /// entry when it does not exist yet (so a waiter that starts while + /// the session is `agent` still observes a later takeover). + fn signal_for(&self, sid: &SessionId) -> watch::Receiver { + let mut guard = self.lock(); + guard + .control + .entry(sid.clone()) + .or_insert_with(ControlState::new) + .signal + .subscribe() + } + + /// Atomically read the state and, when control is `agent`, consume + /// the return receipt so exactly one waiter sees `Released`. + fn poll_control(&self, sid: &SessionId) -> PollOutcome { + let mut guard = self.lock(); + let Some(entry) = guard.control.get_mut(sid) else { + return PollOutcome::AlreadyAgent; + }; + if entry.held_since_ms.is_some() { + let held_for_ms = entry + .held_since_ms + .map(|start| now_ms().saturating_sub(start)) + .unwrap_or(0); + return PollOutcome::StillHeld { held_for_ms }; + } + match entry.last_return.take() { + Some(receipt) => PollOutcome::Released(receipt), + None => PollOutcome::AlreadyAgent, + } + } + + /// Block until control returns to `agent`, `deadline` passes, or + /// `session_exists()` reports the session gone. + /// + /// The registry mutex is only held for short, synchronous probes — + /// waiting happens on the per-session watch channel, so a + /// long-blocked `session.wait_control` never blocks other sessions' + /// tool dispatch or status reads. + pub async fn wait_for_release( + &self, + sid: &SessionId, + deadline: tokio::time::Instant, + session_exists: impl Fn() -> bool, + ) -> WaitControlResolution { + if !session_exists() { + return WaitControlResolution::SessionGone; + } + let mut rx = self.signal_for(sid); + loop { + match self.poll_control(sid) { + PollOutcome::Released(receipt) => { + return WaitControlResolution::Released(receipt); + } + PollOutcome::AlreadyAgent => return WaitControlResolution::AlreadyAgent, + PollOutcome::StillHeld { .. } => {} + } + if !session_exists() { + return WaitControlResolution::SessionGone; + } + // Scope the borrowed `changed` future so the receiver can be + // re-subscribed after a sender-dropped wake-up. + let signal_result = { + let changed = rx.changed(); + tokio::pin!(changed); + let sleep = tokio::time::sleep_until(deadline); + tokio::pin!(sleep); + tokio::select! { + result = &mut changed => Some(result), + _ = &mut sleep => None, + } + }; + match signal_result { + Some(Ok(())) => {} + Some(Err(_)) => { + // The sender was dropped: `drop_session` removed the + // entry (session torn down) or nobody ever owned it. + // Re-check existence before giving up so a release + // that raced teardown still wins. + if !session_exists() { + return WaitControlResolution::SessionGone; + } + rx = self.signal_for(sid); + } + None => { + // Re-probe once: a release may have landed at the + // deadline and must not be reported as a timeout. + return match self.poll_control(sid) { + PollOutcome::Released(receipt) => WaitControlResolution::Released(receipt), + PollOutcome::AlreadyAgent => WaitControlResolution::AlreadyAgent, + PollOutcome::StillHeld { held_for_ms } => { + WaitControlResolution::TimedOut { held_for_ms } + } + }; + } + } + } + } + + /// Drop every entry for `sid`. **Every** session-teardown path MUST + /// call this so neither a hot interrupt marker nor a held/receipt + /// state leaks into the registry indefinitely, and so waiters parked + /// on the control channel are woken (the sender is dropped). Current + /// call sites: + /// + /// * `stop_session` (session.stop RPC + idle reaper) /// * `forget_session` (extension closed the agent window) - /// * `purge_browser` cascade (browser disconnect — see - /// `daemon/ws.rs`) + /// * `purge_browser` cascade (browser disconnect — see `daemon/ws.rs`) + /// * browser liveness reap (see `daemon/start.rs`) /// - /// If a future code path adds a fourth teardown route, add the + /// If a future code path adds another teardown route, add the /// `drop_session` call there too — there is no static check /// enforcing this. pub fn drop_session(&self, sid: &SessionId) { - let mut guard = self.inner.lock().expect("session_interrupt poisoned"); - guard.remove(sid); + let mut guard = self.lock(); + guard.pending.remove(sid); + guard.control.remove(sid); + } + + #[cfg(test)] + pub(crate) fn inner_pending_len(&self) -> usize { + self.lock().pending.len() + } + + #[cfg(test)] + pub(crate) fn inner_control_len(&self) -> usize { + self.lock().control.len() + } + + #[cfg(test)] + pub(crate) fn force_held_since(&self, sid: &SessionId, held_since_ms: u64) { + let mut guard = self.lock(); + let entry = guard + .control + .entry(sid.clone()) + .or_insert_with(ControlState::new); + entry.held_since_ms = Some(held_since_ms); + } +} + +fn control_of(entry: &ControlState) -> SessionControl { + if entry.held_since_ms.is_some() { + SessionControl::User + } else { + SessionControl::Agent + } +} + +fn now_ms() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +/// Format an epoch-millisecond timestamp as RFC 3339 UTC +/// (`2021-01-01T00:00:00.000Z`). +/// +/// Hand-rolled because the daemon deliberately keeps its dependency +/// surface small and no date-time crate is in the graph. The civil-date +/// conversion is Howard Hinnant's `days_from_civil` inverse, valid for +/// the whole proleptic Gregorian range we can represent in `i64` ms. +fn format_rfc3339_ms(ms: u64) -> String { + let secs = (ms / 1_000) as i64; + let millis = ms % 1_000; + let days = secs.div_euclid(86_400); + let sod = secs.rem_euclid(86_400); + let (hour, minute, second) = (sod / 3_600, (sod % 3_600) / 60, sod % 60); + let z = days + 719_468; + let era = if z >= 0 { z } else { z - 146_096 } / 146_097; + let doe = z - era * 146_097; + let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365; + let mut year = yoe + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let day = doy - (153 * mp + 2) / 5 + 1; + let month = mp + if mp < 10 { 3 } else { -9 }; + if month <= 2 { + year += 1; } + format!("{year:04}-{month:02}-{day:02}T{hour:02}:{minute:02}:{second:02}.{millis:03}Z") } #[cfg(test)] @@ -94,17 +471,22 @@ mod tests { SessionId(s.to_string()) } + fn held_ms_for_test(reg: &SessionInterruptRegistry, s: &SessionId) -> u64 { + reg.snapshot(s).held_for_ms.unwrap_or(0) + } + #[test] fn new_registry_is_empty() { let reg = SessionInterruptRegistry::new(); - assert!(reg.inner.lock().unwrap().is_empty()); + assert_eq!(reg.inner_pending_len(), 0); + assert_eq!(reg.inner_control_len(), 0); } #[test] fn mark_inserts_an_entry() { let reg = SessionInterruptRegistry::new(); reg.mark(&sid("A")); - assert!(reg.inner.lock().unwrap().contains(&sid("A"))); + assert!(reg.is_pending(&sid("A"))); } #[test] @@ -112,7 +494,7 @@ mod tests { let reg = SessionInterruptRegistry::new(); reg.mark(&sid("A")); reg.mark(&sid("A")); - assert_eq!(reg.inner.lock().unwrap().len(), 1); + assert_eq!(reg.inner_pending_len(), 1); } #[test] @@ -120,7 +502,7 @@ mod tests { let reg = SessionInterruptRegistry::new(); reg.mark(&sid("A")); reg.mark(&sid("B")); - assert_eq!(reg.inner.lock().unwrap().len(), 2); + assert_eq!(reg.inner_pending_len(), 2); } #[test] @@ -137,7 +519,7 @@ mod tests { fn try_consume_on_unmarked_session_returns_false() { let reg = SessionInterruptRegistry::new(); assert!(!reg.try_consume(&sid("ghost"))); - assert!(reg.inner.lock().unwrap().is_empty()); + assert_eq!(reg.inner_pending_len(), 0); } #[test] @@ -145,7 +527,7 @@ mod tests { let reg = SessionInterruptRegistry::new(); reg.mark(&sid("A")); assert!(reg.try_consume(&sid("A"))); - assert!(!reg.inner.lock().unwrap().contains(&sid("A"))); + assert!(!reg.is_pending(&sid("A"))); } #[test] @@ -163,7 +545,7 @@ mod tests { reg.mark(&sid("A")); reg.mark(&sid("B")); assert!(reg.try_consume(&sid("A"))); - assert!(reg.inner.lock().unwrap().contains(&sid("B"))); + assert!(reg.is_pending(&sid("B"))); } #[test] @@ -171,14 +553,14 @@ mod tests { let reg = SessionInterruptRegistry::new(); reg.mark(&sid("A")); reg.drop_session(&sid("A")); - assert!(!reg.inner.lock().unwrap().contains(&sid("A"))); + assert!(!reg.is_pending(&sid("A"))); } #[test] fn drop_session_on_unknown_session_is_noop() { let reg = SessionInterruptRegistry::new(); reg.drop_session(&sid("ghost")); - assert!(reg.inner.lock().unwrap().is_empty()); + assert_eq!(reg.inner_pending_len(), 0); } #[test] @@ -187,8 +569,8 @@ mod tests { reg.mark(&sid("A")); reg.mark(&sid("B")); reg.drop_session(&sid("A")); - assert!(!reg.inner.lock().unwrap().contains(&sid("A"))); - assert!(reg.inner.lock().unwrap().contains(&sid("B"))); + assert!(!reg.is_pending(&sid("A"))); + assert!(reg.is_pending(&sid("B"))); } #[test] @@ -198,4 +580,326 @@ mod tests { reg.drop_session(&sid("A")); assert!(!reg.try_consume(&sid("A"))); } + + // ----- takeover lifecycle ----- + + #[test] + fn control_defaults_to_agent() { + let reg = SessionInterruptRegistry::new(); + let snap = reg.snapshot(&sid("A")); + assert_eq!(snap.control, SessionControl::Agent); + assert!(!snap.pending_interrupt); + assert!(snap.held_for_ms.is_none()); + assert!(snap.last_return.is_none()); + assert!(!reg.is_held(&sid("A"))); + } + + #[test] + fn take_control_holds_session_and_is_idempotent() { + let reg = SessionInterruptRegistry::new(); + reg.take_control(&sid("A")); + assert!(reg.is_held(&sid("A"))); + assert_eq!(reg.snapshot(&sid("A")).control, SessionControl::User); + // A repeated take must not reset the hold clock (no observable + // way to inject a clock here, so assert the entry count instead). + reg.take_control(&sid("A")); + assert_eq!(reg.inner_control_len(), 1); + assert!(reg.is_held(&sid("A"))); + } + + #[test] + fn release_control_clears_held_and_records_receipt() { + let reg = SessionInterruptRegistry::new(); + // Backdate the hold start so `held_ms` is a deterministic ~5000ms. + reg.force_held_since(&sid("A"), now_ms().saturating_sub(5_000)); + let was_held = reg.release_control(&sid("A"), Some("filled the form".into())); + assert!(was_held, "release on a held session reports was_held"); + let snap = reg.snapshot(&sid("A")); + assert_eq!(snap.control, SessionControl::Agent); + assert!(snap.held_for_ms.is_none()); + let receipt = snap.last_return.expect("receipt recorded"); + assert_eq!(receipt.note, "filled the form"); + assert!( + (4_900..=6_000).contains(&receipt.held_ms), + "held_ms should reflect the ~5s hold, got {}", + receipt.held_ms + ); + assert_eq!(held_ms_for_test(®, &sid("A")), 0); + assert!(receipt.returned_at.ends_with('Z')); + } + + #[test] + fn release_control_without_hold_stores_empty_receipt() { + let reg = SessionInterruptRegistry::new(); + let was_held = reg.release_control(&sid("A"), None); + assert!(!was_held); + let receipt = reg.snapshot(&sid("A")).last_return.expect("receipt"); + assert_eq!(receipt.note, ""); + assert_eq!(receipt.held_ms, 0); + } + + #[test] + fn held_state_is_not_consumed_by_repeated_reads_or_teardown_free_probes() { + // The IPC gate must NOT be able to clear the flag: it only + // consults `is_held`. Simulate many rejected dispatches. + let reg = SessionInterruptRegistry::new(); + reg.take_control(&sid("A")); + for _ in 0..5 { + assert!(reg.is_held(&sid("A"))); + } + assert!(reg.is_held(&sid("A"))); + } + + #[test] + fn consume_return_is_single_use() { + let reg = SessionInterruptRegistry::new(); + reg.take_control(&sid("A")); + reg.release_control(&sid("A"), Some("done".into())); + let first = reg.consume_return(&sid("A")).expect("first consume wins"); + assert_eq!(first.note, "done"); + assert!( + reg.consume_return(&sid("A")).is_none(), + "second consume must be empty" + ); + // `status` no longer shows the receipt once consumed. + assert!(reg.snapshot(&sid("A")).last_return.is_none()); + } + + #[test] + fn drop_session_clears_held_and_receipt() { + let reg = SessionInterruptRegistry::new(); + reg.take_control(&sid("A")); + reg.release_control(&sid("A"), Some("done".into())); + reg.drop_session(&sid("A")); + let snap = reg.snapshot(&sid("A")); + assert_eq!(snap.control, SessionControl::Agent); + assert!(snap.last_return.is_none()); + assert!(snap.held_for_ms.is_none()); + assert_eq!(reg.inner_control_len(), 0); + } + + #[test] + fn drop_session_clears_held_for_other_sessions_only() { + let reg = SessionInterruptRegistry::new(); + reg.take_control(&sid("A")); + reg.take_control(&sid("B")); + reg.drop_session(&sid("A")); + assert!(!reg.is_held(&sid("A"))); + assert!(reg.is_held(&sid("B"))); + } + + #[test] + fn one_shot_mark_still_works_alongside_held_state() { + let reg = SessionInterruptRegistry::new(); + reg.mark(&sid("A")); + reg.take_control(&sid("A")); + assert!(reg.try_consume(&sid("A")), "mark survives a takeover"); + assert!(!reg.try_consume(&sid("A"))); + assert!(reg.is_held(&sid("A")), "held is independent of the marker"); + } + + #[test] + fn release_control_also_drops_the_one_shot_marker() { + // The takeover handshake sets both states; returning control must + // clear both, otherwise the agent's next input tool is rejected + // once with "pending user interrupt" right after `wait_control` + // told it control is back. + let reg = SessionInterruptRegistry::new(); + reg.mark(&sid("A")); + reg.take_control(&sid("A")); + reg.release_control(&sid("A"), Some("done".into())); + assert!(!reg.is_pending(&sid("A")), "marker dropped on return"); + assert!(!reg.is_held(&sid("A")), "hold cleared on return"); + assert!(!reg.try_consume(&sid("A")), "nothing left to consume"); + assert_eq!(reg.inner_pending_len(), 0); + } + + #[test] + fn release_control_leaves_other_sessions_pending_marker_alone() { + let reg = SessionInterruptRegistry::new(); + reg.mark(&sid("A")); + reg.mark(&sid("B")); + reg.take_control(&sid("A")); + reg.release_control(&sid("A"), None); + assert!(!reg.is_pending(&sid("A"))); + assert!( + reg.is_pending(&sid("B")), + "a return must not clear another session's marker" + ); + } + + #[test] + fn release_control_without_hold_still_drops_a_stray_marker() { + // `control_returned` may arrive after a standalone Stop marked the + // session (or before the daemon saw the takeover); the return is + // still the user handing control back, so the marker must go. + let reg = SessionInterruptRegistry::new(); + reg.mark(&sid("A")); + let was_held = reg.release_control(&sid("A"), None); + assert!(!was_held); + assert!(!reg.is_pending(&sid("A"))); + assert!(!reg.try_consume(&sid("A"))); + } + + #[test] + fn standalone_interrupt_marker_survives_without_a_return() { + // The Stop/interrupt flow (mark without take_control) must keep its + // one-shot semantics: nothing but a dispatch or teardown clears it. + let reg = SessionInterruptRegistry::new(); + reg.mark(&sid("A")); + assert!(reg.is_pending(&sid("A"))); + assert!(reg.try_consume(&sid("A"))); + assert!(!reg.try_consume(&sid("A"))); + } + + // ----- wait_control ----- + + #[tokio::test] + async fn wait_returns_already_agent_without_receipt() { + let reg = SessionInterruptRegistry::new(); + let resolution = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_millis(200), + || true, + ) + .await; + assert_eq!(resolution, WaitControlResolution::AlreadyAgent); + } + + #[tokio::test] + async fn wait_returns_session_gone_when_session_missing() { + let reg = SessionInterruptRegistry::new(); + let resolution = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_millis(200), + || false, + ) + .await; + assert_eq!(resolution, WaitControlResolution::SessionGone); + } + + #[tokio::test] + async fn wait_released_path_returns_the_receipt_once() { + let reg = std::sync::Arc::new(SessionInterruptRegistry::new()); + reg.take_control(&sid("A")); + let reg_for_release = std::sync::Arc::clone(®); + tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + reg_for_release.release_control(&sid("A"), Some("handled login".into())); + }); + let resolution = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_secs(2), + || true, + ) + .await; + match resolution { + WaitControlResolution::Released(receipt) => { + assert_eq!(receipt.note, "handled login"); + assert!(receipt.held_ms <= 2_000); + } + other => panic!("expected released, got {other:?}"), + } + // A second wait sees no receipt. + let second = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_millis(100), + || true, + ) + .await; + assert_eq!(second, WaitControlResolution::AlreadyAgent); + } + + #[tokio::test] + async fn wait_times_out_while_held_and_reports_hold_duration() { + let reg = SessionInterruptRegistry::new(); + reg.force_held_since(&sid("A"), now_ms().saturating_sub(1_500)); + let resolution = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_millis(50), + || true, + ) + .await; + match resolution { + WaitControlResolution::TimedOut { held_for_ms } => { + assert!( + held_for_ms >= 1_500, + "expected ~1.5s hold, got {held_for_ms}" + ); + } + other => panic!("expected timed_out, got {other:?}"), + } + assert!(reg.is_held(&sid("A")), "timeout must not clear the hold"); + } + + #[tokio::test] + async fn wait_wakes_on_drop_session() { + let reg = std::sync::Arc::new(SessionInterruptRegistry::new()); + reg.take_control(&sid("A")); + let reg_for_drop = std::sync::Arc::clone(®); + // The session is "alive" until the drop happens. + let alive = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); + let alive_for_drop = std::sync::Arc::clone(&alive); + tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + alive_for_drop.store(false, std::sync::atomic::Ordering::SeqCst); + reg_for_drop.drop_session(&sid("A")); + }); + let resolution = reg + .wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_secs(2), + || alive.load(std::sync::atomic::Ordering::SeqCst), + ) + .await; + assert_eq!(resolution, WaitControlResolution::SessionGone); + } + + #[tokio::test] + async fn wait_does_not_block_other_registry_users() { + // A waiter must not hold the registry mutex: while one call is + // parked, another thread can still read/consume state. + let reg = std::sync::Arc::new(SessionInterruptRegistry::new()); + reg.take_control(&sid("A")); + let waiter = { + let reg = std::sync::Arc::clone(®); + tokio::spawn(async move { + reg.wait_for_release( + &sid("A"), + tokio::time::Instant::now() + std::time::Duration::from_millis(300), + || true, + ) + .await + }) + }; + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + // These would deadlock if the waiter held the mutex across await. + assert!(reg.is_held(&sid("A"))); + assert_eq!(reg.snapshot(&sid("A")).control, SessionControl::User); + assert_eq!(reg.inner_control_len(), 1); + let _ = waiter.await.unwrap(); + } + + #[test] + fn rfc3339_epoch_formats_as_utc() { + assert_eq!(format_rfc3339_ms(0), "1970-01-01T00:00:00.000Z"); + assert_eq!( + format_rfc3339_ms(1_609_459_200_000), + "2021-01-01T00:00:00.000Z" + ); + assert_eq!( + format_rfc3339_ms(1_609_459_200_123), + "2021-01-01T00:00:00.123Z" + ); + // Leap day boundary. + assert_eq!( + format_rfc3339_ms(1_582_934_400_000), + "2020-02-29T00:00:00.000Z" + ); + } } diff --git a/crates/bsk-cli/src/daemon/state.rs b/crates/bsk-cli/src/daemon/state.rs index ce28dc0a..429f8ce3 100644 --- a/crates/bsk-cli/src/daemon/state.rs +++ b/crates/bsk-cli/src/daemon/state.rs @@ -17,7 +17,12 @@ use super::start::DaemonConfig; use super::ws::WsHandle; pub const DAEMON_VERSION: &str = env!("CARGO_PKG_VERSION"); -pub const PROTOCOL_VERSION: &str = "1.3"; +/// Wire protocol version. Bumped to `1.4` by the user-takeover feature, +/// which adds the `session.control_taken` / `session.control_returned` +/// events and the `session.status` / `session.wait_control` methods. An +/// older peer drops the new events as unparseable frames, so the skew +/// must be visible in the handshake and in `bsk doctor`. +pub const PROTOCOL_VERSION: &str = "1.4"; /// Base wire compatibility. New interaction semantics are checked per operation. pub const MIN_COMPATIBLE_PROTOCOL: &str = "1.0"; /// Legacy app-semver floor used only when `HandshakeResult.min_compatible_peer` diff --git a/crates/bsk-cli/src/daemon/ws.rs b/crates/bsk-cli/src/daemon/ws.rs index 96d60e93..fe445b6d 100644 --- a/crates/bsk-cli/src/daemon/ws.rs +++ b/crates/bsk-cli/src/daemon/ws.rs @@ -507,6 +507,12 @@ async fn handle_inbound_text(state: &Arc, client: &Arc { handle_session_user_interrupt(state, &client.id, &ev.payload); } + bsk_protocol::EventKind::SessionControlTaken => { + handle_session_control_taken(state, &client.id, &ev.payload); + } + bsk_protocol::EventKind::SessionControlReturned => { + handle_session_control_returned(state, &client.id, &ev.payload); + } other => { debug!(event = ?other, "event received (no handler yet)"); } @@ -706,6 +712,137 @@ fn handle_session_user_interrupt( state.session_interrupts.mark(&sid); } +/// Largest `note` the daemon keeps from a `session.control_returned`. +/// +/// The note is free-form user text that sits in the interrupt registry +/// until a `session.wait_control` waiter consumes it, and is echoed back +/// over IPC. A browser must not be able to park an unbounded string +/// there, so anything longer is cut. +pub(crate) const MAX_RETURN_NOTE_BYTES: usize = 4096; + +/// Cut `note` to [`MAX_RETURN_NOTE_BYTES`] on a UTF-8 character boundary, +/// so the stored value is always valid UTF-8 and a multi-byte character +/// is never split in half. +pub(crate) fn truncate_return_note(note: &str) -> String { + if note.len() <= MAX_RETURN_NOTE_BYTES { + return note.to_string(); + } + let mut end = MAX_RETURN_NOTE_BYTES; + while end > 0 && !note.is_char_boundary(end) { + end -= 1; + } + note[..end].to_string() +} + +/// Extension-originated "the user pressed 接管/Take over". Cancels every +/// inflight + queued `tool.*` call for the session (same path as +/// `session.user_interrupt`, including the one-shot marker for older +/// flows) and then parks the session in the `held` control state so +/// every subsequent interrupt-gated call is rejected until the user +/// explicitly returns control. +fn handle_session_control_taken( + state: &Arc, + sender: &BrowserId, + payload: &serde_json::Value, +) { + let Some(sid) = extract_session_id(payload) else { + warn!("session.control_taken event missing session_id"); + return; + }; + // An unknown session id is rejected outright: the control registry is + // keyed by session and nothing else would ever clean up an entry for a + // session that does not exist, so accepting one lets a browser grow the + // registry without bound. + let Some(session) = state.sessions.get(&sid) else { + warn!( + session = %sid, + sender = %sender, + "ignoring session.control_taken for an unknown session" + ); + return; + }; + // Same ownership guard as user_interrupt / window_closed: a browser + // must not be able to freeze another browser's session by claiming a + // takeover of a foreign session_id. + if session.browser_id != *sender { + warn!( + session = %sid, + sender = %sender, + owner = %session.browser_id, + "ignoring session.control_taken from a browser that does not own this session" + ); + return; + } + let snapshots = state.tool_inflight.cancel_session(&sid); + info!(session = %sid, count = snapshots.len(), "user takeover: cancelled inflight tools"); + for snap in snapshots { + if let (Some(browser_id), Some(ws_rpc_id)) = (snap.browser_id, snap.ws_rpc_id) + && let Err(err) = + super::cancel_forward::forward_cancel_to_browser(state, &browser_id, &ws_rpc_id) + { + warn!( + browser = %browser_id, + ws_rpc_id = %ws_rpc_id, + %err, + "failed to forward takeover cancel to extension" + ); + } + } + state.audit.marker(&sid.0, "control_taken"); + // Keep the legacy one-shot marker so older/racing tool flows (and any + // extension that only knows the stop button) still observe a stop. + state.session_interrupts.mark(&sid); + state.session_interrupts.take_control(&sid); +} + +/// Extension-originated "the user pressed 交还/Return to agent". Clears +/// the `held` state *and* the legacy one-shot interrupt marker (so the +/// agent's next input tool is not rejected once after the return), +/// stores the return receipt (note + hold duration) for +/// `session.wait_control`, and wakes every waiter. +fn handle_session_control_returned( + state: &Arc, + sender: &BrowserId, + payload: &serde_json::Value, +) { + let Some(sid) = extract_session_id(payload) else { + warn!("session.control_returned event missing session_id"); + return; + }; + let Some(session) = state.sessions.get(&sid) else { + warn!( + session = %sid, + sender = %sender, + "ignoring session.control_returned for an unknown session" + ); + return; + }; + if session.browser_id != *sender { + warn!( + session = %sid, + sender = %sender, + owner = %session.browser_id, + "ignoring session.control_returned from a browser that does not own this session" + ); + return; + } + // Malformed `note` (present but not a string) is a warn-and-ignore, + // matching the other handlers. A missing / empty note is valid. An + // over-long note is cut rather than rejected: the user's text is the + // point of the feature, so losing the tail beats losing the note. + let note = match payload.get("note") { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::String(text)) => Some(truncate_return_note(text)), + Some(_) => { + warn!(session = %sid, "session.control_returned note is not a string; ignoring it"); + None + } + }; + state.audit.marker(&sid.0, "control_returned"); + let was_held = state.session_interrupts.release_control(&sid, note); + info!(session = %sid, was_held, "user returned control to agent"); +} + #[cfg(test)] mod tests { use super::*; @@ -883,6 +1020,307 @@ mod session_user_interrupt_tests { ); } + #[test] + fn handle_session_control_taken_holds_session_for_owning_browser() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid.0})); + + assert!( + state.session_interrupts.is_held(&sid), + "owning browser may take control of its own session" + ); + } + + #[test] + fn handle_session_control_taken_keeps_one_shot_marker() { + // Old flows only understand the one-shot `mark`. A takeover must + // keep setting it so a racing tool dispatch is still rejected. + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid.0})); + + assert!( + state.session_interrupts.try_consume(&sid), + "takeover must also set the legacy one-shot interrupt marker" + ); + assert!( + state.session_interrupts.is_held(&sid), + "consuming the marker must not clear the held state" + ); + } + + #[test] + fn handle_session_control_taken_cancels_inflight_for_target_session() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid_a = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session a"); + let sid_b = state + .sessions + .reserve_id(owner.clone(), 8, || 1) + .expect("reserved session b"); + assert_ne!(sid_a, sid_b, "the two sessions must be distinct"); + let g_a = state + .tool_inflight + .register("a".into(), sid_a.clone()) + .unwrap(); + let g_b = state + .tool_inflight + .register("b".into(), sid_b.clone()) + .unwrap(); + + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid_a.0})); + + assert_eq!(g_a.entry().cancel_reason(), Some(CancelReason::UserAborted)); + assert!(g_b.entry().cancel_reason().is_none()); + } + + #[test] + fn handle_session_control_taken_ignores_an_unknown_session() { + // Nothing would ever clean up a control entry for a session that + // does not exist, so an unknown id must not create one. + let state = test_only_daemon_state(); + handle_session_control_taken( + &state, + &BrowserId("sender".into()), + &serde_json::json!({"session_id": "ghost"}), + ); + assert_eq!(state.session_interrupts.inner_control_len(), 0); + assert_eq!(state.session_interrupts.inner_pending_len(), 0); + } + + #[test] + fn handle_session_control_returned_ignores_an_unknown_session() { + let state = test_only_daemon_state(); + handle_session_control_returned( + &state, + &BrowserId("sender".into()), + &serde_json::json!({"session_id": "ghost", "note": "orphan"}), + ); + assert_eq!(state.session_interrupts.inner_control_len(), 0); + } + + #[test] + fn handle_session_control_returned_truncates_an_over_long_note() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid.0})); + + // Multi-byte padding so a naive byte cut would split a character and + // the `String` build would panic. + let huge = "\u{4f60}".repeat(MAX_RETURN_NOTE_BYTES); + handle_session_control_returned( + &state, + &owner, + &serde_json::json!({"session_id": sid.0, "note": huge}), + ); + + let receipt = state + .session_interrupts + .snapshot(&sid) + .last_return + .expect("receipt stored"); + assert!( + receipt.note.len() <= MAX_RETURN_NOTE_BYTES, + "note kept {} bytes, over the {MAX_RETURN_NOTE_BYTES} cap", + receipt.note.len() + ); + assert!( + receipt.note.chars().all(|c| c == '\u{4f60}'), + "the cut must land on a character boundary" + ); + } + + #[test] + fn truncate_return_note_keeps_short_notes_verbatim() { + assert_eq!(truncate_return_note(""), ""); + assert_eq!(truncate_return_note("filled the form"), "filled the form"); + let exact = "a".repeat(MAX_RETURN_NOTE_BYTES); + assert_eq!(truncate_return_note(&exact), exact); + } + + #[test] + fn handle_session_control_taken_ignored_from_non_owning_browser() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let attacker = BrowserId("attacker-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + let guard = state + .tool_inflight + .register("rpc-1".into(), sid.clone()) + .unwrap(); + + handle_session_control_taken(&state, &attacker, &serde_json::json!({"session_id": sid.0})); + + assert!( + !state.session_interrupts.is_held(&sid), + "non-owning browser must not be able to hold another session" + ); + assert!( + guard.entry().cancel_reason().is_none(), + "non-owning browser must not cancel another session's tools" + ); + assert!( + !state.session_interrupts.try_consume(&sid), + "non-owning browser must not set the interrupt marker" + ); + } + + #[test] + fn handle_session_control_taken_missing_session_id_is_ignored() { + let state = test_only_daemon_state(); + handle_session_control_taken(&state, &BrowserId("sender".into()), &serde_json::json!({})); + handle_session_control_taken( + &state, + &BrowserId("sender".into()), + &serde_json::json!({"session_id": ""}), + ); + handle_session_control_taken( + &state, + &BrowserId("sender".into()), + &serde_json::json!({"session_id": 42}), + ); + assert_eq!(state.session_interrupts.inner_control_len(), 0); + } + + #[test] + fn handle_session_control_returned_clears_held_and_stores_note() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid.0})); + handle_session_control_returned( + &state, + &owner, + &serde_json::json!({"session_id": sid.0, "note": "filled the form"}), + ); + + let snap = state.session_interrupts.snapshot(&sid); + assert_eq!( + snap.control, + bsk_protocol::SessionControl::Agent, + "returning control must clear the held state" + ); + assert!( + !snap.pending_interrupt, + "returning control must also drop the takeover's one-shot marker" + ); + assert!( + !state.session_interrupts.try_consume(&sid), + "no one-shot interrupt may fire after a return" + ); + let receipt = snap.last_return.expect("return receipt stored"); + assert_eq!(receipt.note, "filled the form"); + } + + #[test] + fn handle_session_control_returned_accepts_missing_or_empty_note() { + for note in [ + None, + Some(serde_json::json!("")), + Some(serde_json::json!(null)), + ] { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + let mut payload = serde_json::json!({"session_id": sid.0}); + if let Some(note) = note { + payload["note"] = note; + } + handle_session_control_returned(&state, &owner, &payload); + let receipt = state + .session_interrupts + .snapshot(&sid) + .last_return + .expect("receipt even without a note"); + assert_eq!(receipt.note, ""); + } + } + + #[test] + fn handle_session_control_returned_ignores_non_string_note() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + handle_session_control_returned( + &state, + &owner, + &serde_json::json!({"session_id": sid.0, "note": 42}), + ); + let receipt = state + .session_interrupts + .snapshot(&sid) + .last_return + .expect("receipt still recorded"); + assert_eq!(receipt.note, ""); + } + + #[test] + fn handle_session_control_returned_ignored_from_non_owning_browser() { + let state = test_only_daemon_state(); + let owner = BrowserId("owner-browser".into()); + let attacker = BrowserId("attacker-browser".into()); + let sid = state + .sessions + .reserve_id(owner.clone(), 8, || 0) + .expect("reserved session id"); + handle_session_control_taken(&state, &owner, &serde_json::json!({"session_id": sid.0})); + + handle_session_control_returned( + &state, + &attacker, + &serde_json::json!({"session_id": sid.0, "note": "attacker"}), + ); + + assert!( + state.session_interrupts.is_held(&sid), + "non-owning browser must not release another session" + ); + let snap = state.session_interrupts.snapshot(&sid); + assert!(snap.last_return.is_none(), "no receipt from the attacker"); + } + + #[test] + fn handle_session_control_returned_missing_session_id_is_ignored() { + let state = test_only_daemon_state(); + handle_session_control_returned( + &state, + &BrowserId("sender".into()), + &serde_json::json!({"note": "orphan"}), + ); + assert_eq!(state.session_interrupts.inner_control_len(), 0); + } + #[test] fn handle_session_window_closed_ignored_from_non_owning_browser() { let state = test_only_daemon_state(); diff --git a/crates/bsk-cli/src/main.rs b/crates/bsk-cli/src/main.rs index 1fd447e4..de31b715 100644 --- a/crates/bsk-cli/src/main.rs +++ b/crates/bsk-cli/src/main.rs @@ -77,7 +77,7 @@ fn dispatch(cli: Cli, format: Format) -> Result<(), CliError> { lines: cmd.lines, }) .map_err(CliError::Local), - Command::Session(cmd) => cli::session::dispatch(cmd, format), + Command::Session(cmd) => cli::session::dispatch(cmd, format, cli.flags.quiet), Command::Browsers => cli::browsers::dispatch(format), Command::Tab(cmd) => cli::tab::dispatch(cmd, format), Command::Window(cmd) => cli::window::dispatch(cmd, format), diff --git a/crates/bsk-cli/tests/cli_parse.rs b/crates/bsk-cli/tests/cli_parse.rs index 59773de5..f13f653d 100644 --- a/crates/bsk-cli/tests/cli_parse.rs +++ b/crates/bsk-cli/tests/cli_parse.rs @@ -502,6 +502,71 @@ fn session_start_window_size_defaults_to_none() { assert!(args.height.is_none()); } +#[test] +fn parses_session_status_and_wait_control() { + let cli = parse(&["bsk", "session", "status", "--session", "s1"]); + let Command::Session(SessionCmd { + sub: SessionSub::Status(args), + }) = cli.command + else { + panic!("expected session status subcommand"); + }; + assert_eq!(args.session, "s1"); + + // `wait-control` defaults to 5 minutes and accepts the shared + // `5m` / `300s` / `300000ms` grammar. + let cli = parse(&["bsk", "session", "wait-control", "--session", "s1"]); + let Command::Session(SessionCmd { + sub: SessionSub::WaitControl(args), + }) = cli.command + else { + panic!("expected session wait-control subcommand"); + }; + assert_eq!(args.session, "s1"); + assert_eq!(args.timeout, 300_000); + + for (raw, expected) in [("5m", 300_000), ("300s", 300_000), ("300000ms", 300_000)] { + let cli = parse(&[ + "bsk", + "session", + "wait-control", + "--session", + "s1", + "--timeout", + raw, + ]); + let Command::Session(SessionCmd { + sub: SessionSub::WaitControl(args), + }) = cli.command + else { + panic!("expected session wait-control subcommand"); + }; + assert_eq!(args.timeout, expected, "timeout grammar {raw}"); + } + + for bad in ["0", "0ms", "abc", "-5s"] { + assert!( + Cli::try_parse_from([ + "bsk", + "session", + "wait-control", + "--session", + "s1", + "--timeout", + bad + ]) + .is_err(), + "timeout {bad} must be rejected" + ); + } +} + +#[test] +fn session_status_requires_session_flag() { + assert!(Cli::try_parse_from(["bsk", "session", "status"]).is_err()); + assert!(Cli::try_parse_from(["bsk", "session", "wait-control"]).is_err()); +} + #[test] fn rejects_out_of_range_session_start_window_size() { assert!(Cli::try_parse_from(["bsk", "session", "start", "--width", "99"]).is_err()); diff --git a/crates/bsk-cli/tests/handshake_compat.rs b/crates/bsk-cli/tests/handshake_compat.rs index 18d4168c..4b00d18b 100644 --- a/crates/bsk-cli/tests/handshake_compat.rs +++ b/crates/bsk-cli/tests/handshake_compat.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use std::time::Duration; +use bsk::daemon::state::PROTOCOL_VERSION; use bsk::daemon::{self, DaemonConfig}; use bsk::ipc_client::IpcClient; use bsk_protocol::system::{HandshakeParams, HandshakeResult, StatusResult}; @@ -108,12 +109,12 @@ async fn send_handshake_with_floors( async fn handshake_ok_when_protocol_matches() { let (handle, _sock) = spawn_daemon().await; let mut ws = open_ws(handle.ws_addr()).await; - let resp = send_handshake(&mut ws, "1.3", env!("CARGO_PKG_VERSION")).await; + let resp = send_handshake(&mut ws, PROTOCOL_VERSION, env!("CARGO_PKG_VERSION")).await; let result: HandshakeResult = match resp.body { ResponseBody::Ok(v) => serde_json::from_value(v).unwrap(), ResponseBody::Err(e) => panic!("expected ok handshake, got {e:?}"), }; - assert_eq!(result.protocol_version, "1.3"); + assert_eq!(result.protocol_version, PROTOCOL_VERSION); assert_eq!( result .min_compatible_peer @@ -147,9 +148,18 @@ async fn handshake_ok_when_app_versions_differ_but_protocol_matches() { async fn handshake_skew_when_protocol_minor_differs() { let (handle, _sock) = spawn_daemon().await; let mut ws = open_ws(handle.ws_addr()).await; + // Derived from the daemon's own constant so a protocol bump cannot + // silently turn this into an equal-version handshake. + let (major, minor) = PROTOCOL_VERSION + .split_once('.') + .expect("major.minor protocol"); + let newer = format!( + "{major}.{}", + minor.parse::().expect("numeric minor") + 1 + ); let resp = send_handshake_with_floors( &mut ws, - "1.4", + &newer, env!("CARGO_PKG_VERSION"), Some("0.0.0"), Some("1.3"), @@ -235,7 +245,8 @@ async fn status_surfaces_version_skew_for_skewed_browser() { browser_name: "chrome".into(), browser_version: "131.0".into(), extension_version: "9.9.9".into(), - extension_protocol_version: "1.4".into(), + // Deliberately one minor behind the daemon so the entry stays skewed. + extension_protocol_version: "1.3".into(), label: "Older".into(), sink: bsk::daemon::browsers::BrowserSink { tx }, pending: Mutex::new(bsk::daemon::browsers::Pending::default()), @@ -263,8 +274,8 @@ async fn status_surfaces_version_skew_for_skewed_browser() { .iter() .find(|s| s.instance_id == "skew-only-test") .expect("status must list our skew client"); - assert_eq!(skew.client_protocol_version, "1.4"); - assert_eq!(skew.server_protocol_version, "1.3"); + assert_eq!(skew.client_protocol_version, "1.3"); + assert_eq!(skew.server_protocol_version, PROTOCOL_VERSION); assert_eq!(skew.client_version, "9.9.9"); let entry = status .browsers diff --git a/crates/bsk-cli/tests/session_takeover.rs b/crates/bsk-cli/tests/session_takeover.rs new file mode 100644 index 00000000..5b96a180 --- /dev/null +++ b/crates/bsk-cli/tests/session_takeover.rs @@ -0,0 +1,463 @@ +//! End-to-end coverage of the user-takeover ("held") control state: +//! `session.control_taken` / `session.control_returned` WS events, the +//! `control=user` rejection of interrupt-gated tool dispatch, and the +//! `session.status` / `session.wait_control` daemon-local RPCs the agent +//! uses to observe and wait out the takeover. +//! +//! Uses the same fake-extension harness as `session_user_interrupt.rs`: +//! a real daemon + IPC socket + WS peer, so the contract exercised here +//! is the wire one, not an internal function call. + +mod support; + +use std::path::PathBuf; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; + +use bsk::daemon::{self, DaemonConfig}; +use bsk::ipc_client::IpcClient; +use bsk_protocol::system::{ + HandshakeParams, HandshakeResult, SessionControl, SessionStatusResult, + SessionWaitControlResult, WaitControlOutcome, +}; +use bsk_protocol::tools::SessionStartResult; +use bsk_protocol::{ + BrowserPeerInfo, ErrorCode, EventFrame, EventKind, Frame, Method, RequestFrame, ResponseBody, + ResponseFrame, +}; +use futures_util::{SinkExt, StreamExt}; +use rand::Rng; +use serde_json::json; +use tokio_tungstenite::tungstenite::handshake::client::generate_key; +use tokio_tungstenite::tungstenite::http::Request; +use tokio_tungstenite::tungstenite::protocol::Message; + +use support::{wait_for_session_interrupt_pending, wait_until}; + +const TEST_EXT_ID: &str = "abcdefghijklmnopabcdefghijklmnop"; +const TAKEOVER_REJECTION: &str = "tool dispatch rejected: the user has taken over this session (control=user). Do not retry; run `bsk session wait-control --session ` and continue only after it returns control=agent."; + +type TestWs = + tokio_tungstenite::WebSocketStream>; + +fn tempfile_path(prefix: &str) -> PathBuf { + let mut p = std::env::temp_dir(); + let mut rng = rand::thread_rng(); + let suffix: String = (0..8) + .map(|_| char::from_digit(rng.gen_range(0..16), 16).unwrap()) + .collect(); + p.push(format!("{prefix}-{}-{suffix}.sock", std::process::id())); + p +} + +async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { + let config = DaemonConfig::new(0); + let sock = tempfile_path("bsk-test-takeover"); + let handle = daemon::run(config, Some(sock.clone())).await.unwrap(); + (handle, sock) +} + +async fn connect_ext(addr: std::net::SocketAddr) -> TestWs { + let origin = format!("chrome-extension://{TEST_EXT_ID}"); + let url = format!("ws://{addr}/"); + let req = Request::builder() + .method("GET") + .uri(&url) + .header("Host", addr.to_string()) + .header("Upgrade", "websocket") + .header("Connection", "Upgrade") + .header("Sec-WebSocket-Version", "13") + .header("Sec-WebSocket-Key", generate_key()) + .header("Origin", origin) + .body(()) + .unwrap(); + let (ws, _resp) = tokio_tungstenite::connect_async(req).await.unwrap(); + ws +} + +async fn handshake_as_ext(ws: &mut TestWs) -> HandshakeResult { + let params = HandshakeParams { + client: "browser-skill-extension".into(), + version: "0.1.0-dev.0".parse().unwrap(), + protocol_version: bsk::daemon::state::PROTOCOL_VERSION.into(), + instance_id: TEST_EXT_ID.into(), + browser: BrowserPeerInfo { + name: "chrome".into(), + version: "131.0".into(), + }, + label: "Test".into(), + min_compatible_peer: Some("0.1.0-dev.0".parse().unwrap()), + min_compatible_protocol: Some("1.0".into()), + }; + let req = RequestFrame { + id: "hs".into(), + method: Method::SystemHandshake, + params: Some(serde_json::to_value(params).unwrap()), + }; + ws.send(Message::Text(serde_json::to_string(&req).unwrap())) + .await + .unwrap(); + let resp = ws.next().await.unwrap().unwrap(); + let text = match resp { + Message::Text(t) => t, + _ => panic!("expected text handshake reply"), + }; + let resp: ResponseFrame = serde_json::from_str(&text).unwrap(); + match resp.body { + ResponseBody::Ok(v) => serde_json::from_value(v).unwrap(), + ResponseBody::Err(e) => panic!("handshake rejected: {e:?}"), + } +} + +#[derive(serde::Serialize)] +struct StartParams { + browser_instance_id: Option, +} + +#[derive(serde::Deserialize, Debug)] +struct StartReply { + session_id: String, +} + +async fn start_session(ipc: &mut IpcClient) -> String { + let start: StartReply = ipc + .call( + "sess-start", + Method::SessionStart, + Some(StartParams { + browser_instance_id: None, + }), + Duration::from_secs(5), + ) + .await + .unwrap() + .expect("session.start succeeded"); + start.session_id +} + +/// Split the WS peer into a sink we can drive and a responder task that +/// answers `tool.session_start` and counts every other forwarded request. +/// `blocked_method` calls are counted but never answered: a correct +/// daemon rejects them before they ever reach the extension. +fn spawn_responder( + ws: TestWs, + counted_method: Method, +) -> ( + Arc>>, + Arc, +) { + let (sink, stream) = ws.split(); + let sink = Arc::new(tokio::sync::Mutex::new(sink)); + let count = Arc::new(AtomicUsize::new(0)); + let sink_for_responder = Arc::clone(&sink); + let count_for_responder = Arc::clone(&count); + tokio::spawn(async move { + let mut stream = stream; + while let Some(Ok(msg)) = stream.next().await { + let Message::Text(text) = msg else { continue }; + let Ok(frame) = serde_json::from_str::(&text) else { + continue; + }; + let Frame::Request(req) = frame else { continue }; + if req.method == Method::ToolSessionStart { + let result = SessionStartResult { + interaction: None, + agent_window_id: Some(1), + }; + let reply = ResponseFrame { + id: req.id, + body: ResponseBody::Ok(serde_json::to_value(result).unwrap()), + }; + let mut g = sink_for_responder.lock().await; + g.send(Message::Text(serde_json::to_string(&reply).unwrap())) + .await + .unwrap(); + } else if req.method == Method::ToolConsole { + // Answer passive reads so the test can assert they pass the + // held gate *and* reach the extension. + let reply = ResponseFrame { + id: req.id, + body: ResponseBody::Ok(json!({ + "tab_id": 7, + "entries": [], + "next_since": 0, + "truncated": false + })), + }; + let mut g = sink_for_responder.lock().await; + g.send(Message::Text(serde_json::to_string(&reply).unwrap())) + .await + .unwrap(); + } else if req.method == counted_method { + count_for_responder.fetch_add(1, Ordering::SeqCst); + } + } + }); + (sink, count) +} + +/// Full takeover lifecycle over the real transport: +/// taken → gated rejection → status=user → wait_control → +/// returned(note) → status=agent with the receipt consumed. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn takeover_blocks_dispatch_until_control_returns() { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let (ws_sink, forwarded_click_count) = spawn_responder(ws, Method::ToolClick); + + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + let session_id = start_session(&mut ipc).await; + + // The user presses "接管/Take over". + { + let event = EventFrame { + event: EventKind::SessionControlTaken, + payload: json!({ "session_id": session_id.clone() }), + }; + let mut g = ws_sink.lock().await; + g.send(Message::Text(serde_json::to_string(&event).unwrap())) + .await + .unwrap(); + } + let state = handle.state(); + wait_for_session_interrupt_pending(&state, &session_id).await; + wait_until("session held", Duration::from_secs(2), || { + state + .session_interrupts + .is_held(&bsk::daemon::sessions::SessionId(session_id.clone())) + }) + .await; + + // `session.status` reports the hold without clearing it. + let status: SessionStatusResult = ipc + .call( + "status-1", + Method::SessionStatus, + Some(json!({ "session_id": session_id.clone() })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("session.status ok"); + assert_eq!(status.control, SessionControl::User); + assert!(status.held_for_ms.is_some()); + + // An interrupt-gated tool call is rejected with the exact message and + // never reaches the extension. + let outcome = ipc + .call::<_, serde_json::Value>( + "click-while-held", + Method::ToolClick, + Some(json!({ "session_id": session_id.clone(), "ref": "@e1" })), + Duration::from_secs(3), + ) + .await + .unwrap(); + let err = outcome.expect_err("held session must reject a click"); + assert_eq!(err.code, ErrorCode::UserAborted); + assert_eq!(err.message, TAKEOVER_REJECTION); + assert_eq!( + forwarded_click_count.load(Ordering::SeqCst), + 0, + "a held session's click must not be forwarded to the extension" + ); + + // Rejection is non-consuming: a second click is rejected the same way. + let outcome = ipc + .call::<_, serde_json::Value>( + "click-while-held-2", + Method::ToolClick, + Some(json!({ "session_id": session_id.clone(), "ref": "@e1" })), + Duration::from_secs(3), + ) + .await + .unwrap(); + let err = outcome.expect_err("held session keeps rejecting"); + assert_eq!(err.code, ErrorCode::UserAborted); + assert_eq!(err.message, TAKEOVER_REJECTION); + + // A passive read still passes through the held gate and reaches the + // extension, proving the gate is narrow. + let console = ipc + .call::<_, serde_json::Value>( + "console-while-held", + Method::ToolConsole, + Some(json!({ "session_id": session_id.clone() })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("passive reads must pass a held session"); + assert_eq!(console["tab_id"], json!(7)); + + // Park a waiter on `session.wait_control`, then return control. + let waiter = { + let sock = sock.clone(); + let session_id = session_id.clone(); + tokio::spawn(async move { + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + ipc.call::<_, SessionWaitControlResult>( + "wait-1", + Method::SessionWaitControl, + Some(json!({ "session_id": session_id, "timeout_ms": 5_000 })), + Duration::from_secs(10), + ) + .await + .unwrap() + .expect("wait_control ok") + }) + }; + // Give the waiter time to park before the release lands. + tokio::time::sleep(Duration::from_millis(150)).await; + { + let event = EventFrame { + event: EventKind::SessionControlReturned, + payload: json!({ + "session_id": session_id.clone(), + "note": "I finished the captcha" + }), + }; + let mut g = ws_sink.lock().await; + g.send(Message::Text(serde_json::to_string(&event).unwrap())) + .await + .unwrap(); + } + let released = tokio::time::timeout(Duration::from_secs(5), waiter) + .await + .expect("wait_control must resolve after control returns") + .unwrap(); + assert_eq!(released.outcome, WaitControlOutcome::Released); + assert_eq!(released.control, SessionControl::Agent); + assert_eq!(released.note.as_deref(), Some("I finished the captcha")); + + // The receipt is delivered exactly once: status no longer shows it, and + // a second wait reports already_agent. + let status: SessionStatusResult = ipc + .call( + "status-2", + Method::SessionStatus, + Some(json!({ "session_id": session_id.clone() })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("session.status ok"); + assert_eq!(status.control, SessionControl::Agent); + assert!( + status.last_return.is_none(), + "wait_control must have consumed the receipt" + ); + + let again: SessionWaitControlResult = ipc + .call( + "wait-2", + Method::SessionWaitControl, + Some(json!({ "session_id": session_id.clone(), "timeout_ms": 50 })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("wait_control ok"); + assert_eq!(again.outcome, WaitControlOutcome::AlreadyAgent); + assert!(again.note.is_none()); + + // Control is back and the takeover's legacy one-shot marker was dropped + // by the return, so the very next click must pass the gate and reach the + // extension. A short daemon-side `timeout_ms` keeps the test fast: the + // responder never answers, so the daemon surfaces `timeout` only after + // the click was actually forwarded. + let click = ipc + .call::<_, serde_json::Value>( + "click-after-release", + Method::ToolClick, + Some(json!({ + "session_id": session_id.clone(), + "ref": "@e1", + "timeout_ms": 300 + })), + Duration::from_secs(10), + ) + .await; + let click = match click { + Ok(Err(err)) => { + assert_ne!( + err.code, + ErrorCode::UserAborted, + "no gate may fire after control returns: {err:?}" + ); + assert!( + !err.message.contains("pending user interrupt"), + "the takeover's one-shot marker must be cleared on return: {err:?}" + ); + err + } + Ok(Ok(value)) => panic!("click should not have succeeded, got {value:?}"), + Err(err) => panic!("IPC round trip must complete, got {err}"), + }; + assert!( + matches!(click.code, ErrorCode::Timeout | ErrorCode::Cancelled), + "unanswered forwarded click surfaces a timeout, got {click:?}" + ); + wait_until( + "click forwarded after release", + Duration::from_secs(2), + || forwarded_click_count.load(Ordering::SeqCst) >= 1, + ) + .await; + + handle.shutdown().await; +} + +/// `session.wait_control` on a session the user never took over returns +/// `already_agent` promptly, and on a session that disappears returns +/// `session_gone`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn wait_control_reports_already_agent_and_session_gone() { + let (handle, sock) = spawn_daemon().await; + let mut ws = connect_ext(handle.ws_addr()).await; + let _ = handshake_as_ext(&mut ws).await; + let (_ws_sink, _count) = spawn_responder(ws, Method::ToolClick); + + let mut ipc = IpcClient::connect(&sock).await.unwrap(); + let session_id = start_session(&mut ipc).await; + + let already: SessionWaitControlResult = ipc + .call( + "wait-agent", + Method::SessionWaitControl, + Some(json!({ "session_id": session_id.clone(), "timeout_ms": 100 })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("wait_control ok"); + assert_eq!(already.outcome, WaitControlOutcome::AlreadyAgent); + + let gone: SessionWaitControlResult = ipc + .call( + "wait-ghost", + Method::SessionWaitControl, + Some(json!({ "session_id": "ghost", "timeout_ms": 100 })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect("wait_control answers session_gone rather than erroring"); + assert_eq!(gone.outcome, WaitControlOutcome::SessionGone); + + let status_err = ipc + .call::<_, serde_json::Value>( + "status-ghost", + Method::SessionStatus, + Some(json!({ "session_id": "ghost" })), + Duration::from_secs(3), + ) + .await + .unwrap() + .expect_err("unknown session must be not_found"); + assert_eq!(status_err.code, ErrorCode::NotFound); + + handle.shutdown().await; +} diff --git a/crates/bsk-protocol/schema/session_status_params.json b/crates/bsk-protocol/schema/session_status_params.json new file mode 100644 index 00000000..fced9759 --- /dev/null +++ b/crates/bsk-protocol/schema/session_status_params.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "SessionStatusParams", + "description": "`session.status` request payload.", + "type": "object", + "required": [ + "session_id" + ], + "properties": { + "session_id": { + "type": "string" + } + } +} diff --git a/crates/bsk-protocol/schema/session_status_result.json b/crates/bsk-protocol/schema/session_status_result.json new file mode 100644 index 00000000..8559aeba --- /dev/null +++ b/crates/bsk-protocol/schema/session_status_result.json @@ -0,0 +1,78 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "SessionStatusResult", + "description": "`session.status` response payload. Non-consuming: reading the takeover state never clears the held flag nor the return receipt.", + "type": "object", + "required": [ + "control", + "pending_interrupt", + "session_id" + ], + "properties": { + "control": { + "$ref": "#/definitions/SessionControl" + }, + "held_for_ms": { + "description": "How long the user has held control, in milliseconds. `None` while `control` is `agent`.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0.0 + }, + "last_return": { + "description": "Most recent return receipt, until it is consumed by `session.wait_control`.", + "anyOf": [ + { + "$ref": "#/definitions/SessionReturnReceipt" + }, + { + "type": "null" + } + ] + }, + "pending_interrupt": { + "description": "Whether the one-shot user-interrupt marker is still pending.", + "type": "boolean" + }, + "session_id": { + "type": "string" + } + }, + "definitions": { + "SessionControl": { + "description": "Who currently owns a session's page control.\n\n`Agent` is the default: the daemon routes the agent's tool calls into the Agent Window. `User` (“held”) means the user pressed “Take over” in the Agent Window; interrupt-gated tool calls are rejected with `ErrorCode::UserAborted` until control is returned.", + "type": "string", + "enum": [ + "agent", + "user" + ] + }, + "SessionReturnReceipt": { + "description": "Receipt recorded when the user returns control to the agent.", + "type": "object", + "required": [ + "held_ms", + "note", + "returned_at" + ], + "properties": { + "held_ms": { + "description": "How long the user held control, in milliseconds.", + "type": "integer", + "format": "uint64", + "minimum": 0.0 + }, + "note": { + "description": "Free-form note the user typed before returning control. May be empty when the user returned control without a message.", + "type": "string" + }, + "returned_at": { + "description": "RFC 3339 (UTC) timestamp of the return.", + "type": "string" + } + } + } + } +} diff --git a/crates/bsk-protocol/schema/session_wait_control_params.json b/crates/bsk-protocol/schema/session_wait_control_params.json new file mode 100644 index 00000000..c08b7b17 --- /dev/null +++ b/crates/bsk-protocol/schema/session_wait_control_params.json @@ -0,0 +1,23 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "SessionWaitControlParams", + "description": "`session.wait_control` request payload.", + "type": "object", + "required": [ + "session_id" + ], + "properties": { + "session_id": { + "type": "string" + }, + "timeout_ms": { + "description": "Maximum block in milliseconds. Defaults to [`DEFAULT_WAIT_CONTROL_MS`]; values above [`MAX_WAIT_CONTROL_MS`] are rejected as `invalid_params`.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0.0 + } + } +} diff --git a/crates/bsk-protocol/schema/session_wait_control_result.json b/crates/bsk-protocol/schema/session_wait_control_result.json new file mode 100644 index 00000000..d75710c5 --- /dev/null +++ b/crates/bsk-protocol/schema/session_wait_control_result.json @@ -0,0 +1,77 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "SessionWaitControlResult", + "description": "`session.wait_control` response payload.", + "type": "object", + "required": [ + "control", + "outcome" + ], + "properties": { + "control": { + "$ref": "#/definitions/SessionControl" + }, + "held_ms": { + "description": "Duration the user held control, in milliseconds.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0.0 + }, + "note": { + "description": "Note from the return receipt. Only `released` delivers it, and only once — the receipt is consumed by that call.", + "type": [ + "string", + "null" + ] + }, + "outcome": { + "$ref": "#/definitions/WaitControlOutcome" + } + }, + "definitions": { + "SessionControl": { + "description": "Who currently owns a session's page control.\n\n`Agent` is the default: the daemon routes the agent's tool calls into the Agent Window. `User` (“held”) means the user pressed “Take over” in the Agent Window; interrupt-gated tool calls are rejected with `ErrorCode::UserAborted` until control is returned.", + "type": "string", + "enum": [ + "agent", + "user" + ] + }, + "WaitControlOutcome": { + "description": "Why `session.wait_control` returned.", + "oneOf": [ + { + "description": "The user returned control while we waited; `note` / `held_ms` carry the consumed return receipt.", + "type": "string", + "enum": [ + "released" + ] + }, + { + "description": "Control was already `agent` when the call started (or a racing waiter consumed the receipt first).", + "type": "string", + "enum": [ + "already_agent" + ] + }, + { + "description": "The wait expired with control still held by the user.", + "type": "string", + "enum": [ + "timed_out" + ] + }, + { + "description": "The session disappeared before/while waiting.", + "type": "string", + "enum": [ + "session_gone" + ] + } + ] + } + } +} diff --git a/crates/bsk-protocol/src/bin/dump-schema.rs b/crates/bsk-protocol/src/bin/dump-schema.rs index 7fc349e2..69a51268 100644 --- a/crates/bsk-protocol/src/bin/dump-schema.rs +++ b/crates/bsk-protocol/src/bin/dump-schema.rs @@ -4,8 +4,9 @@ use std::fs; use std::path::PathBuf; use bsk_protocol::system::{ - BrowserListParams, HandshakeParams, HandshakeResult, PingParams, PingResult, StatusParams, - StatusResult, + BrowserListParams, HandshakeParams, HandshakeResult, PingParams, PingResult, + SessionStatusParams, SessionStatusResult, SessionWaitControlParams, SessionWaitControlResult, + StatusParams, StatusResult, }; use bsk_protocol::tools::*; use bsk_protocol::{CancelParams, CancelResult}; @@ -37,6 +38,11 @@ fn main() { dump!(StatusResult, "system_status_result"); dump!(BrowserListParams, "browser_list_params"); + dump!(SessionStatusParams, "session_status_params"); + dump!(SessionStatusResult, "session_status_result"); + dump!(SessionWaitControlParams, "session_wait_control_params"); + dump!(SessionWaitControlResult, "session_wait_control_result"); + dump!(CancelParams, "cancel_params"); dump!(CancelResult, "cancel_result"); diff --git a/crates/bsk-protocol/src/frame.rs b/crates/bsk-protocol/src/frame.rs index b80eec9b..8111e904 100644 --- a/crates/bsk-protocol/src/frame.rs +++ b/crates/bsk-protocol/src/frame.rs @@ -56,6 +56,14 @@ pub enum EventKind { SessionWindowClosed, #[serde(rename = "session.user_interrupt")] SessionUserInterrupt, + /// The user pressed "Take over" in the Agent Window and now owns the + /// page. Payload: `{ "session_id": "..." }`. + #[serde(rename = "session.control_taken")] + SessionControlTaken, + /// The user pressed "Return to agent" in the Agent Window. Payload: + /// `{ "session_id": "...", "note": "optional string" }`. + #[serde(rename = "session.control_returned")] + SessionControlReturned, #[serde(rename = "session.interaction_changed")] SessionInteractionChanged, #[serde(rename = "browser.disconnected")] @@ -266,6 +274,35 @@ mod tests { assert_eq!(v, serde_json::json!("session.user_interrupt")); } + #[test] + fn session_control_events_serialise_as_dotted_names() { + // The extension hardcodes these literals when the user presses + // Take over / Return to agent; lock the wire names here so a + // rename cannot silently break the takeover handshake. + assert_eq!( + serde_json::to_value(EventKind::SessionControlTaken).unwrap(), + serde_json::json!("session.control_taken") + ); + assert_eq!( + serde_json::to_value(EventKind::SessionControlReturned).unwrap(), + serde_json::json!("session.control_returned") + ); + } + + #[test] + fn session_control_returned_frame_round_trips_with_note() { + let wire = serde_json::json!({ + "event": "session.control_returned", + "payload": { "session_id": "sess-1", "note": "filled the form" } + }); + let frame: EventFrame = serde_json::from_value(wire).unwrap(); + assert_eq!(frame.event, EventKind::SessionControlReturned); + assert_eq!( + frame.payload.get("note").and_then(|v| v.as_str()), + Some("filled the form"), + ); + } + #[test] fn session_user_interrupt_event_frame_round_trips() { let frame = EventFrame { diff --git a/crates/bsk-protocol/src/lib.rs b/crates/bsk-protocol/src/lib.rs index 3da5eb08..e33c7899 100644 --- a/crates/bsk-protocol/src/lib.rs +++ b/crates/bsk-protocol/src/lib.rs @@ -12,8 +12,11 @@ pub use error::{DecodeError, ErrorCode, RpcError}; pub use frame::{EventFrame, EventKind, Frame, RequestFrame, ResponseBody, ResponseFrame, RpcId}; pub use method::Method; pub use system::{ - BrowserListParams, BrowserPeerInfo, BrowserStatusEntry, HandshakeCompat, HandshakeParams, - HandshakeResult, PingParams, PingResult, SessionStatusEntry, StatusParams, StatusResult, - VersionSkewEntry, compare_protocol, evaluate_handshake_compat, + BrowserListParams, BrowserPeerInfo, BrowserStatusEntry, DEFAULT_WAIT_CONTROL_MS, + HandshakeCompat, HandshakeParams, HandshakeResult, MAX_WAIT_CONTROL_MS, PingParams, PingResult, + SessionControl, SessionReturnReceipt, SessionStatusEntry, SessionStatusParams, + SessionStatusResult, SessionWaitControlParams, SessionWaitControlResult, StatusParams, + StatusResult, VersionSkewEntry, WaitControlOutcome, compare_protocol, + evaluate_handshake_compat, }; pub use tools::*; diff --git a/crates/bsk-protocol/src/method.rs b/crates/bsk-protocol/src/method.rs index 2db8b20a..d32b9b8f 100644 --- a/crates/bsk-protocol/src/method.rs +++ b/crates/bsk-protocol/src/method.rs @@ -39,6 +39,12 @@ pub enum Method { SessionStopAll, #[serde(rename = "session.list")] SessionList, + /// Daemon-local: read the session's takeover control state. + #[serde(rename = "session.status")] + SessionStatus, + /// Daemon-local: block until the user returns control (or timeout). + #[serde(rename = "session.wait_control")] + SessionWaitControl, #[serde(rename = "browser.list")] BrowserList, @@ -220,11 +226,18 @@ impl Method { | Method::ToolRecordStop | Method::ToolRecordAwait => MethodEffect::PassiveRead, - // Session lifecycle — not gated. + // Session lifecycle — not gated. Neither takeover + // observation (`session.status`) nor the release wait + // (`session.wait_control`) may be blocked by the held gate: + // they are the very RPCs the agent uses to learn that the + // user still holds control, and gating them would make the + // documented recovery path unreachable. Method::SessionStart | Method::SessionStop | Method::SessionStopAll | Method::SessionList + | Method::SessionStatus + | Method::SessionWaitControl | Method::ToolSessionStart | Method::ToolSessionStop => MethodEffect::ControlPlane, @@ -264,6 +277,38 @@ mod tests { use crate::{CancelParams, CancelResult}; use serde_json::json; + #[test] + fn session_status_method_round_trips() { + let method: Method = serde_json::from_value(json!("session.status")).unwrap(); + assert_eq!(method, Method::SessionStatus); + assert_eq!( + serde_json::to_value(method).unwrap(), + json!("session.status") + ); + } + + #[test] + fn session_wait_control_method_round_trips() { + let method: Method = serde_json::from_value(json!("session.wait_control")).unwrap(); + assert_eq!(method, Method::SessionWaitControl); + assert_eq!( + serde_json::to_value(method).unwrap(), + json!("session.wait_control") + ); + } + + #[test] + fn takeover_observation_methods_are_control_plane_and_ungated() { + // The held gate keys off `requires_interrupt_gate()`; both + // takeover RPCs must stay outside it so an agent holding only a + // `control=user` instruction can still poll and wait. + for method in [Method::SessionStatus, Method::SessionWaitControl] { + assert_eq!(method.effect(), MethodEffect::ControlPlane); + assert!(!method.requires_interrupt_gate()); + assert!(!method.is_mutating()); + } + } + #[test] fn cancel_method_round_trips() { let method: Method = serde_json::from_value(json!("cancel")).unwrap(); diff --git a/crates/bsk-protocol/src/system.rs b/crates/bsk-protocol/src/system.rs index 1bbbf416..3d41e926 100644 --- a/crates/bsk-protocol/src/system.rs +++ b/crates/bsk-protocol/src/system.rs @@ -599,6 +599,210 @@ pub struct StatusResult { pub version_skew_browsers: Vec, } +/// Maximum `session.wait_control` block, in milliseconds (30 minutes). +/// +/// Shared by the daemon (hard limit) and the CLI (early validation) so +/// both sides reject the same inputs. +pub const MAX_WAIT_CONTROL_MS: u64 = 30 * 60 * 1_000; + +/// `session.wait_control` default block when `timeout_ms` is omitted +/// (5 minutes). +pub const DEFAULT_WAIT_CONTROL_MS: u64 = 5 * 60 * 1_000; + +/// Who currently owns a session's page control. +/// +/// `Agent` is the default: the daemon routes the agent's tool calls into +/// the Agent Window. `User` (“held”) means the user pressed “Take +/// over” in the Agent Window; interrupt-gated tool calls are rejected +/// with `ErrorCode::UserAborted` until control is returned. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum SessionControl { + #[default] + Agent, + User, +} + +impl SessionControl { + pub fn as_str(self) -> &'static str { + match self { + Self::Agent => "agent", + Self::User => "user", + } + } +} + +/// Receipt recorded when the user returns control to the agent. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SessionReturnReceipt { + /// Free-form note the user typed before returning control. May be + /// empty when the user returned control without a message. + pub note: String, + /// How long the user held control, in milliseconds. + pub held_ms: u64, + /// RFC 3339 (UTC) timestamp of the return. + pub returned_at: String, +} + +/// `session.status` request payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SessionStatusParams { + pub session_id: String, +} + +/// `session.status` response payload. Non-consuming: reading the +/// takeover state never clears the held flag nor the return receipt. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SessionStatusResult { + pub session_id: String, + pub control: SessionControl, + /// Whether the one-shot user-interrupt marker is still pending. + pub pending_interrupt: bool, + /// How long the user has held control, in milliseconds. `None` while + /// `control` is `agent`. + pub held_for_ms: Option, + /// Most recent return receipt, until it is consumed by + /// `session.wait_control`. + pub last_return: Option, +} + +/// `session.wait_control` request payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SessionWaitControlParams { + pub session_id: String, + /// Maximum block in milliseconds. Defaults to + /// [`DEFAULT_WAIT_CONTROL_MS`]; values above [`MAX_WAIT_CONTROL_MS`] + /// are rejected as `invalid_params`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub timeout_ms: Option, +} + +/// Why `session.wait_control` returned. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum WaitControlOutcome { + /// The user returned control while we waited; `note` / `held_ms` + /// carry the consumed return receipt. + Released, + /// Control was already `agent` when the call started (or a racing + /// waiter consumed the receipt first). + AlreadyAgent, + /// The wait expired with control still held by the user. + TimedOut, + /// The session disappeared before/while waiting. + SessionGone, +} + +impl WaitControlOutcome { + pub fn as_str(self) -> &'static str { + match self { + Self::Released => "released", + Self::AlreadyAgent => "already_agent", + Self::TimedOut => "timed_out", + Self::SessionGone => "session_gone", + } + } +} + +/// `session.wait_control` response payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SessionWaitControlResult { + pub outcome: WaitControlOutcome, + pub control: SessionControl, + /// Note from the return receipt. Only `released` delivers it, and + /// only once — the receipt is consumed by that call. + pub note: Option, + /// Duration the user held control, in milliseconds. + pub held_ms: Option, +} + +#[cfg(test)] +mod session_control_payload_tests { + use super::*; + use serde_json::json; + + #[test] + fn session_control_serialises_as_snake_case() { + assert_eq!( + serde_json::to_value(SessionControl::Agent).unwrap(), + json!("agent") + ); + assert_eq!( + serde_json::to_value(SessionControl::User).unwrap(), + json!("user") + ); + } + + #[test] + fn session_control_defaults_to_agent() { + assert_eq!(SessionControl::default(), SessionControl::Agent); + } + + #[test] + fn wait_control_outcomes_serialise_as_snake_case() { + for (outcome, name) in [ + (WaitControlOutcome::Released, "released"), + (WaitControlOutcome::AlreadyAgent, "already_agent"), + (WaitControlOutcome::TimedOut, "timed_out"), + (WaitControlOutcome::SessionGone, "session_gone"), + ] { + assert_eq!(serde_json::to_value(outcome).unwrap(), json!(name)); + assert_eq!(outcome.as_str(), name); + } + } + + #[test] + fn status_result_keeps_explicit_nulls_for_absent_fields() { + // The agent reads these fields positionally; an absent key and an + // explicit `null` must not be conflated by a consumer. + let result = SessionStatusResult { + session_id: "abcd".into(), + control: SessionControl::Agent, + pending_interrupt: false, + held_for_ms: None, + last_return: None, + }; + let value = serde_json::to_value(&result).unwrap(); + assert_eq!(value["control"], json!("agent")); + assert!(value.get("held_for_ms").is_some_and(|v| v.is_null())); + assert!(value.get("last_return").is_some_and(|v| v.is_null())); + let back: SessionStatusResult = serde_json::from_value(value).unwrap(); + assert_eq!(back, result); + } + + #[test] + fn wait_control_result_round_trips_released_receipt() { + let result = SessionWaitControlResult { + outcome: WaitControlOutcome::Released, + control: SessionControl::Agent, + note: Some("filled the form".into()), + held_ms: Some(12_000), + }; + let value = serde_json::to_value(&result).unwrap(); + assert_eq!(value["outcome"], json!("released")); + assert_eq!(value["note"], json!("filled the form")); + assert_eq!(value["held_ms"], json!(12_000)); + let back: SessionWaitControlResult = serde_json::from_value(value).unwrap(); + assert_eq!(back, result); + } + + #[test] + fn wait_control_params_omit_absent_timeout() { + let params = SessionWaitControlParams { + session_id: "abcd".into(), + timeout_ms: None, + }; + assert_eq!( + serde_json::to_value(¶ms).unwrap(), + json!({"session_id": "abcd"}) + ); + let back: SessionWaitControlParams = + serde_json::from_value(json!({ "session_id": "abcd" })) + .expect("timeout_ms is optional"); + assert_eq!(back.timeout_ms, None); + } +} + #[cfg(test)] mod status_compat_tests { use super::*; diff --git a/packages/i18n/src/locales/en-US/extension.json b/packages/i18n/src/locales/en-US/extension.json index b79ac448..9b1b4114 100644 --- a/packages/i18n/src/locales/en-US/extension.json +++ b/packages/i18n/src/locales/en-US/extension.json @@ -118,8 +118,26 @@ }, "controlOverlay": { "status": "Agent controlling", - "interrupt": "Interrupt", - "interrupting": "Interrupting…" + "takeOver": "Take over", + "takingOver": "Taking over…", + "pausedStatus": "You are in control, agent paused", + "notePlaceholder": "Note for the agent (optional)", + "returnControl": "Return to agent", + "action": { + "idle": "Waiting for the next instruction", + "working": "Working…", + "click": "Click", + "hover": "Hover", + "fill": "Type into", + "press": "Press", + "navigate": "Navigate to", + "scroll": "Scroll", + "select": "Select", + "upload": "Upload to", + "download": "Download from", + "evaluate": "Run script", + "reload": "Reload" + } }, "borrowConfirmation": { "title": "Allow tab borrow?", diff --git a/packages/i18n/src/locales/ko-KR/extension.json b/packages/i18n/src/locales/ko-KR/extension.json index bd87a0da..f90d65aa 100644 --- a/packages/i18n/src/locales/ko-KR/extension.json +++ b/packages/i18n/src/locales/ko-KR/extension.json @@ -118,8 +118,26 @@ }, "controlOverlay": { "status": "에이전트가 제어 중", - "interrupt": "중단", - "interrupting": "중단 중…" + "takeOver": "제어 가져오기", + "takingOver": "제어 가져오는 중…", + "pausedStatus": "직접 조작 중이며 에이전트가 일시중지되었습니다", + "notePlaceholder": "에이전트에게 남길 메모(선택)", + "returnControl": "에이전트에게 반환", + "action": { + "idle": "다음 명령을 기다리는 중", + "working": "실행 중…", + "click": "클릭", + "hover": "호버", + "fill": "입력", + "press": "키 입력", + "navigate": "이동", + "scroll": "스크롤", + "select": "선택", + "upload": "업로드", + "download": "다운로드", + "evaluate": "스크립트 실행", + "reload": "새로고침" + } }, "borrowConfirmation": { "title": "탭 사용을 허용할까요?", diff --git a/packages/i18n/src/locales/zh-CN/extension.json b/packages/i18n/src/locales/zh-CN/extension.json index 04735e4d..f851bae8 100644 --- a/packages/i18n/src/locales/zh-CN/extension.json +++ b/packages/i18n/src/locales/zh-CN/extension.json @@ -118,8 +118,26 @@ }, "controlOverlay": { "status": "Agent 正在控制", - "interrupt": "中断", - "interrupting": "中断中…" + "takeOver": "接管", + "takingOver": "接管中…", + "pausedStatus": "你正在操作，Agent 已暂停", + "notePlaceholder": "给 Agent 的备注（可选）", + "returnControl": "交还给 Agent", + "action": { + "idle": "等待下一步指令", + "working": "正在执行…", + "click": "点击", + "hover": "悬停", + "fill": "输入", + "press": "按键", + "navigate": "导航", + "scroll": "滚动", + "select": "选择", + "upload": "上传", + "download": "下载", + "evaluate": "执行脚本", + "reload": "刷新" + } }, "borrowConfirmation": { "title": "允许借用标签页？", diff --git a/skill/SKILL.md b/skill/SKILL.md index 52ab60e8..75a71a5e 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -189,6 +189,17 @@ The result `outcome` is one of `continued`, `completed`, `cancelled`, `timed_out resume only after `continued` or `completed`. Treat `cancelled` as rejection and `timed_out` as a blocker; do not repeat that request. Observe again after control returns before using refs. +### User takeover + +The user can press "Take over" in the Agent Window at any time. Then the session is held +(`control=user`) and every browser-input tool call is rejected with `tool dispatch rejected: the +user has taken over this session (control=user)`. That rejection means **stop acting**: do not +retry, and do not route around it through another tool. Run +`bsk session wait-control --session `; it blocks until the user returns control and prints the +user's `note` (read it — it says what they changed). `bsk session status --session ` shows the +current state without blocking. Control returning does not restore your assumptions: re-snapshot +before continuing, because the user may have navigated, filled, or submitted something. + When help is disabled in the extension, make every reasonable effort to complete the task autonomously with BrowserSkill. Do not call `request-help`. If a call returns `disabled`, no human action was confirmed: re-observe and continue working rather @@ -213,7 +224,7 @@ This list of names is complete. Never invent a command outside it; read `bsk --help` for flags instead of guessing them. ```text -session start|stop|list browsers status doctor update logs +session start|stop|list|status|wait-control browsers status doctor update logs navigate navigate-back navigate-forward reload wait-for-navigation wait-ms observe snapshot get-html screenshot console network click hover wheel scroll-to focus blur fill select press evaluate