Changing the remote CDP endpoint leaks the previous WebSocket, leaving its offline emulation active

Open

Dawssonopened 5d ago

Summary

With agent-browser 0.35.1, when one daemon/session is reused while AGENT_BROWSER_CDP changes between commands (as a remote browser service with one-use attachment URLs requires), the previous CDP WebSocket is never closed. Network emulation set on the old connection stays active, so set offline off sent through the new connection can't clear it.

Steps to reproduce

Run set offline on with AGENT_BROWSER_CDP=ws://…/attachment-a.
Run set offline off with AGENT_BROWSER_CDP=ws://…/attachment-b, same --session, same Chromium target.
Both commands print ✓ Done, and navigator.onLine can report true, but navigation and fetch still fail with net::ERR_INTERNET_DISCONNECTED (pages land on chrome-error://chromewebdata/). A new tab works.

Against a live remote Chromium, seven commands left seven agent-browser WebSockets open on the same target. Closing only the old socket that sent offline: true restored the existing tab immediately.

Cause

main.rs:1450 sends a launch per CDP invocation, and actions.rs:4322 reconnects when the launch URL hash changes (actions.rs:307 uses the full URL as connection identity).
BrowserManager::close doesn't disconnect an external CDP socket.
CdpClient holds its reader and keepalive JoinHandles with no shutdown or Drop. When it's replaced, the detached tasks keep the split WebSocket alive.
network.rs:28 sends Network.emulateNetworkConditions correctly; the offline value itself is fine. The rule simply belongs to the abandoned CDP session.

Expected

Replacing or closing an external CDP connection should explicitly close its WebSocket, stop its reader/keepalive tasks, and resolve pending requests, without sending Browser.close to the user's browser.

Minimal reproduction (no Chromium needed)

A fake CDP server and two commands on different endpoint URLs. Run with bun test (set ISSUE84_AGENT_BROWSER to the binary path if agent-browser isn't on PATH). On 0.35.1 the last assertion fails: both /attachment-a and /attachment-b are still connected.

import { expect, test } from "bun:test";
import { mkdtemp, rm } from "node:fs/promises";

test("replacing a CDP endpoint disconnects its old offline session", async () => {
  const sockets = new Set<any>();
  const emulation: { connection: string; offline: boolean }[] = [];
  const server = Bun.serve<{ connection: string }>({
    hostname: "127.0.0.1",
    port: 0,
    fetch(request, server) {
      if (server.upgrade(request, { data: { connection: new URL(request.url).pathname } })) return;
      return new Response("WebSocket required", { status: 426 });
    },
    websocket: {
      open(socket) {
        sockets.add(socket);
      },
      close(socket) {
        sockets.delete(socket);
      },
      message(socket, raw) {
        const command = JSON.parse(String(raw));
        let result: any = {};
        if (command.method === "Target.getTargets") {
          result = {
            targetInfos: [
              {
                targetId: "fixture",
                type: "page",
                title: "Fixture",
                url: "https://example.test/",
                attached: false,
              },
            ],
          };
        } else if (command.method === "Target.attachToTarget") {
          result = { sessionId: `fixture-${socket.data.connection}` };
        } else if (command.method === "Runtime.evaluate") {
          result = { result: { type: "number", value: 1 } };
        } else if (command.method === "Browser.getVersion") {
          result = {
            protocolVersion: "1.3",
            product: "Chrome/fixture",
            revision: "fixture",
            userAgent: "fixture",
            jsVersion: "fixture",
          };
        } else if (command.method === "Network.emulateNetworkConditions") {
          emulation.push({ connection: socket.data.connection, offline: command.params.offline });
        }
        socket.send(
          JSON.stringify({
            id: command.id,
            result,
            ...(command.sessionId ? { sessionId: command.sessionId } : {}),
          }),
        );
      },
    },
  });
  const socketDirectory = await mkdtemp("/tmp/issue84-fake-");
  const env = Object.fromEntries(
    Object.entries(process.env).filter(([key]) => !key.startsWith("AGENT_BROWSER_")),
  );
  async function command(endpoint: string, ...args: string[]) {
    const child = Bun.spawn(
      [process.env.ISSUE84_AGENT_BROWSER ?? "agent-browser", "--session", "fixture", ...args],
      {
        env: {
          ...env,
          AGENT_BROWSER_SOCKET_DIR: socketDirectory,
          AGENT_BROWSER_CDP: `ws://127.0.0.1:${server.port}/${endpoint}`,
          AGENT_BROWSER_EXECUTABLE_PATH: "/bin/false",
          AGENT_BROWSER_IDLE_TIMEOUT_MS: "30000",
        },
        stdout: "pipe",
        stderr: "pipe",
      },
    );
    const [stdout, stderr, code] = await Promise.all([
      new Response(child.stdout).text(),
      new Response(child.stderr).text(),
      child.exited,
    ]);
    expect({ code, output: (stdout + stderr).trim() }).toEqual({
      code: 0,
      output: args[0] === "close" ? "✓ Browser closed" : "✓ Done",
    });
  }
  try {
    await command("attachment-a", "set", "offline", "on");
    await command("attachment-b", "set", "offline", "off");
    await Bun.sleep(200);
    expect(emulation).toEqual([
      { connection: "/attachment-a", offline: true },
      { connection: "/attachment-b", offline: false },
    ]);
    console.log(
      JSON.stringify({ openConnections: [...sockets].map((s) => s.data.connection), emulation }),
    );
    expect([...sockets].map((s) => s.data.connection)).toEqual(["/attachment-b"]);
  } finally {
    await command("attachment-b", "close");
    for (const socket of sockets) socket.close();
    server.stop(true);
    await rm(socketDirectory, { recursive: true, force: true });
  }
}, 20000);

Activity

No activity yet.