This is a comment left during a code review.
Path: src/browser/pw-session.ts
Line: 30

Comment:
**Importing unexported private functions**

`buildTempDownloadPath` and `saveDownloadPayload` are defined as plain (unexported) functions in `pw-tools-core.downloads.ts` (lines 24 and 85 respectively). Neither has the `export` keyword, so this named import is a TypeScript compile error.

Additionally, `pw-tools-core.downloads.ts` already imports `{ ensurePageState, getPageForTargetId, refLocator, restoreRoleRefsForTarget }` from `./pw-session.js`, so adding the reverse import here introduces a **circular dependency** between the two modules. Node.js / TypeScript module loading can tolerate some cycles, but this cycle means one module may see the other as partially initialized, leading to subtle runtime failures.

Both functions need to be either:
1. Extracted into a shared, lower-level utility module with no dependencies on `pw-session.ts`, and then imported by both files, or
2. Re-implemented inline in `pw-session.ts` without cross-importing

```suggestion
import { buildTempDownloadPath, saveDownloadPayload } from "./pw-tools-core.downloads.js";
```
(This line should be removed and replaced with one of the approaches described above.)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/browser/pw-session.ts
Line: 296-304

Comment:
**Default handler always fires even when an explicit waiter is active**

The comment on line 299 says "Save to default temp dir if **no explicit waiter is active**", but there is no such guard. When `waitForDownloadViaPlaywright` or `downloadViaPlaywright` is called, `createPageDownloadWaiter` in `pw-tools-core.downloads.ts` registers a second `page.on("download", ...)` listener. Because the default handler below was registered earlier (in `ensurePageState`), the download event fires **the default handler first**, which calls `download.saveAs()` to a temp path. Immediately after, the explicit waiter fires and also calls `download.saveAs()` to the user-specified path.

Calling `saveAs` twice on the same Playwright `Download` object is not guaranteed to succeed — if the underlying stream is consumed on the first call, the second `saveAs` (the one the user actually cares about) will silently fail with the error swallowed by the outer `awaitDownloadPayload` → `saveDownloadPayload` error path.

The `state.armIdDownload` field already tracks whether an explicit waiter is armed. Use it as a guard:

```suggestion
    // Default download handler to avoid "Download is starting" errors for unhandled downloads
    page.on("download", async (download) => {
      // Only handle the download if no explicit waiter is active
      if (state.armIdDownload !== 0) {
        return;
      }
      try {
        // Save to default temp dir if no explicit waiter is active
        await saveDownloadPayload(download, buildTempDownloadPath(download.suggestedFilename() || "download.bin"));
      } catch {
        // Ignore download save failures for default handler
      }
    });
```

Note: `armIdDownload` starts at `0` and is incremented by `bumpDownloadArmId()` each time a waiter is armed, then **not reset back to `0`** when the waiter completes. You may need a separate boolean flag (e.g. `hasActiveDownloadWaiter`) that is set/cleared around the waiter lifecycle instead.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/browser/pw-session.ts
Line: 322

Comment:
**`BrowserContext.setDefaultDownloadPath()` does not exist in Playwright's public API**

Playwright's `BrowserContext` type does not expose a `setDefaultDownloadPath` method. The download directory is configured at context-creation time via the `downloadsPath` option passed to `browser.newContext({ downloadsPath: '...' })`. Calling a non-existent method will produce a TypeScript compile error (`Property 'setDefaultDownloadPath' does not exist on type 'BrowserContext'`) and a runtime `TypeError`.

For CDP-attached contexts where you cannot control creation options, the idiomatic approach is to use the lower-level CDP command `Browser.setDownloadBehavior` (via `context.newCDPSession`) to configure download behaviour after the fact:

```ts
// Best-effort: configure CDP download behaviour for this context
try {
  const session = await context.newCDPSession(context.pages()[0] ?? await context.newPage());
  await session.send("Browser.setDownloadBehavior", {
    behavior: "allow",
    downloadPath: DEFAULT_DOWNLOAD_DIR,
  });
  await session.detach().catch(() => {});
} catch {
  // Best-effort; ignore if the context does not support this command
}
```

How can I resolve this? If you propose a fix, please make it concise.