fix(download): bound trigger lifetime and retire cancelled input

.github/workflows/ci.yml

@@ -183,6 +183,27 @@ jobs:
           New-Item -ItemType Directory -Force -Path $env:TEMP | Out-Null
           node scripts/check-dsh-package.mjs
 
+  browser-download:
+    name: Browser download cancellation regression
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v6
+      - uses: pnpm/action-setup@v6
+      - uses: actions/setup-node@v6
+        with:
+          node-version: 22
+          cache: pnpm
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+      - name: Prepare extension types
+        run: pnpm --filter @browser-skill/extension exec wxt prepare
+      - name: Run download cancellation browser regression
+        working-directory: apps/extension
+        env:
+          BSK_CLICK_CHROME: google-chrome
+        run: pnpm exec vitest run src/tools/__tests__/download-deadline.browser.test.ts
+
   browser-input:
     name: Browser input readiness regression
     runs-on: ubuntu-latest

apps/extension/src/browser-driver/chromium-cdp.ts

@@ -44,6 +44,15 @@ import {
 
 export type CdpDebuggee = chrome.debugger.Debuggee & { sessionId?: string };
 
+/** Guards checked at native dispatch, after any asynchronous connection setup. */
+export interface CdpDispatchGuard {
+  signal?: AbortSignal;
+  /** Only use this live attachment; never reconnect a cleanup command. */
+  attachmentId?: string;
+  /** Called immediately before native send, never for a cancelled or stale command. */
+  onDispatch?(): void;
+}
+
 /**
  * Minimal slice of `chrome.debugger` the rest of the extension
  * depends on. Stays as an explicit interface so vitest can inject a
@@ -350,6 +359,29 @@ export class ChromiumCdp {
     }
   }
 
+  async sendGuarded<T = unknown>(
+    target: CdpTarget,
+    method: string,
+    params: object | undefined,
+    guard: CdpDispatchGuard,
+  ): Promise<T> {
+    guard.signal?.throwIfAborted();
+    if (guard.attachmentId === undefined) await this.ensureAttached(target.tabId);
+    try {
+      return (await this.command(target, method, params ?? {}, undefined, () => {
+        guard.signal?.throwIfAborted();
+        if (
+          guard.attachmentId !== undefined &&
+          this.getAttachmentId(target.tabId) !== guard.attachmentId
+        )
+          throw new Error("Debugger attachment changed before dispatch");
+        guard.onDispatch?.();
+      })) as T;
+    } catch (err) {
+      throw normalizeError(err);
+    }
+  }
+
   async getFrameGraph(tabId: number): Promise<CdpFrameGraph> {
     await this.ensureAttached(tabId);
     // Ordinary configuration failures leave the graph to the root tree. A
@@ -531,7 +563,9 @@ export class ChromiumCdp {
   }
 
   /** Detach if attached; never throws. */
-  async detach(tabId: number): Promise<void> {
+  async detach(tabId: number, expectedAttachmentId?: string): Promise<void> {
+    if (expectedAttachmentId !== undefined && this.getAttachmentId(tabId) !== expectedAttachmentId)
+      return;
     this.attachmentVersions.set(tabId, (this.attachmentVersions.get(tabId) ?? 0) + 1);
     const existing = this.detachInFlight.get(tabId);
     if (existing) {
@@ -671,11 +705,15 @@ export class ChromiumCdp {
     method: string,
     params: object,
     readTimeoutMs?: number,
+    beforeDispatch?: () => void,
   ): Promise<unknown> {
     return this.readGate.run(
       target,
       method,
-      () => this.api.sendCommand(target, method, params),
+      () => {
+        beforeDispatch?.();
+        return this.api.sendCommand(target, method, params);
+      },
       readTimeoutMs,
     );
   }

apps/extension/src/tools/__tests__/download-deadline.browser.test.ts

@@ -0,0 +1,294 @@
+// @vitest-environment node
+// Opt in with BSK_CLICK_CHROME; the harness owns an isolated browser/profile.
+import { describe, expect, it } from "vitest";
+import { type CdpDebuggerApi, ChromiumCdp } from "@/browser-driver/chromium-cdp";
+import { SessionManager } from "@/session-manager/manager";
+import { handleDownload } from "../download";
+import type { CdpRunner } from "../shared";
+
+type Send = <T = Record<string, unknown>>(
+  method: string,
+  params?: object,
+  sessionId?: string,
+) => Promise<T>;
+
+async function browser(
+  run: (
+    send: Send,
+    cdp: ChromiumCdp,
+    sessionId: () => string,
+    api: CdpDebuggerApi,
+  ) => Promise<void>,
+) {
+  const { withChrome } = await import(
+    new URL(
+      "../../../../../evals/browser/cases/regression/snapshot-coordinates/chrome.mjs",
+      import.meta.url,
+    ).href
+  );
+  await withChrome(
+    {
+      executable: process.env.BSK_CLICK_CHROME,
+      deviceScale: 1,
+      zoom: 1,
+      startupTimeout: 30_000,
+    },
+    async (send: Send) => {
+      const { targetId } = await send<{ targetId: string }>("Target.createTarget", {
+        url: "about:blank",
+      });
+      let activeSession = "";
+      const api: CdpDebuggerApi = {
+        attach: async () => {
+          const reply = await send<{ sessionId: string }>("Target.attachToTarget", {
+            targetId,
+            flatten: true,
+          });
+          activeSession = reply.sessionId;
+        },
+        detach: async () => {
+          await send("Target.detachFromTarget", { sessionId: activeSession });
+          activeSession = "";
+        },
+        sendCommand: async (_target, method, params) => send(method, params, activeSession),
+        onEvent: { addListener() {}, removeListener() {} } as unknown as CdpDebuggerApi["onEvent"],
+        onDetach: {
+          addListener() {},
+          removeListener() {},
+        } as unknown as CdpDebuggerApi["onDetach"],
+      };
+      const cdp = new ChromiumCdp(api);
+      try {
+        await run(send, cdp, () => activeSession, api);
+      } finally {
+        await cdp.detach(7);
+      }
+    },
+  );
+}
+
+function manager() {
+  return new SessionManager({
+    agentWindow: {
+      create: async () => ({ windowId: 100, initialTabIds: [7] }),
+      remove: async () => {},
+      ensureActiveTab: async () => 7,
+    },
+  });
+}
+const tab = { id: 7, windowId: 100, active: true, url: "about:blank" } as chrome.tabs.Tab;
+const tabsApi = { get: async () => tab, query: async () => [tab] };
+
+describe.skipIf(!process.env.BSK_CLICK_CHROME)("download trigger retirement", () => {
+  it.each([
+    "mouseMoved",
+    "mousePressed",
+  ])("never resumes native input after cancellation at %s", async (heldType) => {
+    await browser(async (_send, cdp) => {
+      const sessions = manager();
+      const ctx = await sessions.start("download-retire");
+      await cdp.send(7, "Runtime.evaluate", {
+        expression: `
+        document.body.innerHTML = '<button id="export" style="width:200px;height:100px">Export</button>';
+        window.startedExports = 0;
+        document.querySelector('#export').onclick = () => { window.startedExports++; };
+      `,
+      });
+      const abort = new AbortController();
+      let release!: (reply: unknown) => void;
+      let reached!: () => void;
+      const ready = new Promise<void>((resolve) => {
+        reached = resolve;
+      });
+      const reply = new Promise((resolve) => {
+        release = resolve;
+      });
+      const inputs: string[] = [];
+      const holdReply = async <T>(
+        method: string,
+        params: object | undefined,
+        value: T,
+      ): Promise<T> => {
+        if (method === "Input.dispatchMouseEvent") {
+          const type = (params as { type: string }).type;
+          inputs.push(type);
+          if (type === heldType) {
+            reached();
+            await reply;
+          }
+        }
+        return value;
+      };
+      const wrapped: CdpRunner = {
+        send: async (tabId, method, params) =>
+          holdReply(method, params, await cdp.send(tabId, method, params)),
+        sendGuarded: async (target, method, params, guard) =>
+          holdReply(method, params, await cdp.sendGuarded(target, method, params, guard)),
+        detach: cdp.detach.bind(cdp),
+        getAttachmentId: cdp.getAttachmentId.bind(cdp),
+        onEvent: () => ({ dispose() {} }),
+      };
+      const event = () => ({ addListener() {}, removeListener() {} });
+      const downloads = {
+        onCreated: event(),
+        onChanged: event(),
+        onDeterminingFilename: event(),
+        search: async () => [],
+        cancel: async () => {},
+        removeFile: async () => {},
+      };
+      const pending = handleDownload(
+        sessions,
+        {
+          session_id: ctx.sessionId,
+          tab_id: 7,
+          selector: "#export",
+          browser_relative_dir: "BrowserSkill/retire",
+          timeout_ms: 2_000,
+        },
+        {
+          cdp: wrapped,
+          tabsApi,
+          downloads,
+          signal: abort.signal,
+          navigationTargets: { onCreatedNavigationTarget: event() },
+        },
+      );
+      try {
+        await ready;
+        abort.abort();
+        const result = await pending;
+        expect(result).toHaveProperty("code");
+        const count = async () =>
+          (
+            await cdp.send<{ result: { value: number } }>(7, "Runtime.evaluate", {
+              expression: "startedExports",
+              returnByValue: true,
+            })
+          ).result.value;
+        const expected = heldType === "mousePressed" ? 1 : 0;
+        expect(await count()).toBe(expected);
+        const before = [...inputs];
+        release({});
+        await new Promise((resolve) => setTimeout(resolve, 50));
+        expect(await count()).toBe(expected);
+        expect(inputs).toEqual(before);
+        expect(inputs).toEqual(
+          heldType === "mousePressed"
+            ? ["mouseMoved", "mousePressed", "mouseReleased"]
+            : ["mouseMoved"],
+        );
+      } finally {
+        release({});
+        await pending;
+      }
+    });
+  }, 40_000);
+
+  it("does not deliver a queued mouse move after cancellation during reattachment", async () => {
+    await browser(async (_send, cdp, _sessionId, api) => {
+      const sessions = manager();
+      const ctx = await sessions.start("download-reattach");
+      await cdp.send(7, "Runtime.evaluate", {
+        expression: `
+          document.body.innerHTML = '<button id="export" style="width:200px;height:100px">Export</button>';
+          window.moves = 0; window.startedExports = 0;
+          document.addEventListener('mousemove', () => { window.moves++; });
+          document.querySelector('#export').onclick = () => { window.startedExports++; };
+        `,
+      });
+      const abort = new AbortController();
+      let release!: () => void;
+      let reached!: () => void;
+      let settled!: () => void;
+      const held = new Promise<void>((resolve) => {
+        release = resolve;
+      });
+      const ready = new Promise<void>((resolve) => {
+        reached = resolve;
+      });
+      const inputSettled = new Promise<void>((resolve) => {
+        settled = resolve;
+      });
+      const attach = api.attach;
+      const sendCommand = api.sendCommand;
+      const inputs: string[] = [];
+      api.sendCommand = (target, method, params) => {
+        if (method === "Input.dispatchMouseEvent") inputs.push((params as { type: string }).type);
+        return sendCommand(target, method, params);
+      };
+      let interceptMove = true;
+      const wrapped: CdpRunner = {
+        send: cdp.send.bind(cdp),
+        sendGuarded: async (target, method, params, guard) => {
+          if (method === "Input.dispatchMouseEvent" && interceptMove) {
+            interceptMove = false;
+            await cdp.detach(7);
+            api.attach = async (target, version) => {
+              await attach(target, version);
+              reached();
+              await held;
+            };
+            try {
+              return await cdp.sendGuarded(target, method, params, guard);
+            } finally {
+              settled();
+            }
+          }
+          return cdp.sendGuarded(target, method, params, guard);
+        },
+        detach: cdp.detach.bind(cdp),
+        getAttachmentId: cdp.getAttachmentId.bind(cdp),
+        onEvent: () => ({ dispose() {} }),
+      };
+      const event = () => ({ addListener() {}, removeListener() {} });
+      const pending = handleDownload(
+        sessions,
+        {
+          session_id: ctx.sessionId,
+          tab_id: 7,
+          selector: "#export",
+          browser_relative_dir: "BrowserSkill/reattach",
+          timeout_ms: 2_000,
+        },
+        {
+          cdp: wrapped,
+          tabsApi,
+          signal: abort.signal,
+          navigationTargets: { onCreatedNavigationTarget: event() },
+          downloads: {
+            onCreated: event(),
+            onChanged: event(),
+            onDeterminingFilename: event(),
+            search: async () => [],
+            cancel: async () => {},
+            removeFile: async () => {},
+          },
+        },
+      );
+      try {
+        await ready;
+        abort.abort();
+        expect(await pending).toHaveProperty("code");
+        expect(inputs).toEqual([]);
+        release();
+        await inputSettled;
+        expect(inputs).toEqual([]);
+        const { result } = await cdp.send<{ result: { value: number[] } }>(7, "Runtime.evaluate", {
+          expression: "[moves, startedExports]",
+          returnByValue: true,
+        });
+        expect(result.value).toEqual([0, 0]);
+        console.log(
+          "DOWNLOAD_REATTACH_PROOF",
+          JSON.stringify({ inputs, moves: result.value[0], startedExports: result.value[1] }),
+        );
+      } finally {
+        abort.abort();
+        release();
+        await pending;
+        api.attach = attach;
+      }
+    });
+  }, 40_000);
+});