From cfa0b6f854e7f15717db385df0e43f974a4b1e67 Mon Sep 17 00:00:00 2001 From: Igor Soarez Date: Sat, 29 Aug 2026 22:59:29 +0100 Subject: fix(setup): skip local-browser precondition for every remote target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-flight check only skipped isBrowserRunning() when BU_CDP_HTTP was set, so a machine with no local browser failed /browser-setup with 'No browser instance running' before discovery ever dialed the built-in castle default. Resolve the target the same way discovery does and skip the local-process check whenever it resolves remote — env var, /browser-target choice, or default. A resolved local target still requires a local browser, and an unreachable remote still fails loudly in step 4. --- CHANGELOG.md | 1 + FORK.md | 6 ++++++ src/setup.ts | 12 +++++++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b4738..4fefe5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to pi-browser-harness will be documented in this file. ### Fixed +- **`/browser-setup` no longer demands a local browser when the target is remote.** The pre-flight check enumerated local Chromium processes unless `BU_CDP_HTTP` was set, so a machine with no local browser failed setup with "No browser instance running" even though the fork's built-in default (castle) was reachable. The precondition is now skipped whenever `resolveCdpTarget` resolves to a remote target — env var, `/browser-target` choice, or the built-in default. - **`pi -p` exits after using browser tools.** The client's socket to the daemon is kept open across sessions by design, and in non-interactive mode it was the last handle keeping Node alive, so a print-mode run that had connected to the browser printed its answer and then hung until killed. The socket no longer holds the event loop; in-flight requests keep their own timers, so nothing awaited is affected. (#39) - **`browser_download` no longer redirects the user's own downloads for good.** `Browser.setDownloadBehavior` was sent without a `browserContextId`, which Chrome applies to its default context — the one the user's own tabs are in — and the override lives on the browser connection, which the daemon keeps open across pi sessions. Every later download in that Chrome, the user's own included, went to the tool's temp directory, and nothing undid it when the session ended. The tool now takes `restore: true` to reset the behavior, and a clean session shutdown restores the default whenever this session's override is still in force (a killed session cannot, so call `restore: true` as soon as you are done). - **Browser state is no longer appended to the session on every callback.** Every ownership change, every `session_tree` event (even one that merely restored the entry already on the branch) and every shutdown appended a `browser-harness-state` entry, so the append-only session file and Pi Web's `+N custom` chain grew without bound. A snapshot is now written only when it differs from the last one persisted or restored; tab order does not count as a difference. (#26) diff --git a/FORK.md b/FORK.md index c273980..853b812 100644 --- a/FORK.md +++ b/FORK.md @@ -104,6 +104,12 @@ git push --force-with-lease origin master Conflicts to expect are confined to `src/cdp/discovery.ts` (the `discoverEndpoint()` short-circuit and the `host` parameter on `queryLiveWsUrl`) and `src/setup.ts` (the skipped local-process precondition). + +The setup precondition skips `isBrowserRunning()` whenever the resolved target +is remote — by env var, `/browser-target` choice, or the built-in default — not +merely when `BU_CDP_HTTP` is set. A machine with no local browser (any host +relying on the castle default) would otherwise fail `/browser-setup` with "No +browser instance running" before discovery ever dialed the remote. If upstream ever reworks daemon discovery, re-check whether the patch is still needed at all. diff --git a/src/setup.ts b/src/setup.ts index 44dfc2c..f6f58e3 100644 --- a/src/setup.ts +++ b/src/setup.ts @@ -2,6 +2,8 @@ import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-age import type { BrowserClient } from "./client"; import { ensureDaemon } from "./daemon/spawn"; import { DAEMON_SOCKET_PATH } from "./daemon/protocol"; +import { resolveCdpTarget } from "./cdp/endpoint"; +import { readTarget } from "./cdp/target-store"; import { isBrowserRunning } from "./profile/browser-process"; import { promptForProfileIfUnset } from "./profile/command"; @@ -24,9 +26,13 @@ export type SetupResult = { success: true; data: string } | { success: false; er export async function performSetup(client: BrowserClient, ctx?: ExtensionContext): Promise { // Step 1: Check Chrome is running // LOCAL PATCH: isBrowserRunning() enumerates *local* processes, so it can - // never see a browser reached via BU_CDP_HTTP. Skip the precondition in that - // case — step 4 still fails loudly if the remote browser is unreachable. - if (!process.env["BU_CDP_HTTP"]?.trim()) { + // never see a browser reached over the network. Skip the precondition + // whenever the resolved target is remote — via BU_CDP_HTTP, a /browser-target + // choice, or the built-in default (see src/cdp/endpoint.ts for precedence). + // Only a resolved local target still needs a local browser process. Step 4 + // fails loudly either way if the chosen browser is unreachable. + const target = resolveCdpTarget(process.env, await readTarget()); + if (target.kind === "local") { const chromeRunning = await isBrowserRunning(); if (!chromeRunning) { return { success: false, error: "No browser instance running. Please open your browser and then run /browser-setup." }; -- cgit v1.2.3