Skip to content

fix(pi-extension): run script/executable PLANNOTATOR_BROWSER paths directly on macOS - #1429

Merged
backnotprop merged 2 commits into
backnotprop:mainfrom
punk-dev-robot:pi-extension-browser-script-path
Sep 15, 2026
Merged

backnotprop merged 2 commits into
backnotprop:mainfrom
punk-dev-robot:pi-extension-browser-script-path

Conversation

@punk-dev-robot

Copy link
Copy Markdown
Contributor

Fixes #1391.

On darwin the pi extension's openBrowser always ran open -a $PLANNOTATOR_BROWSER <url>, treating the value as an app bundle. A script or executable path fails with LaunchServices error -10811, the error is swallowed by the detached spawn, and the extension reports "review opened" while nothing opens.

This mirrors the branch the plannotator binary already has in packages/server/browser.ts: a value containing / that does not end with .app is executed directly with the URL as its argument; app names and .app paths keep going through open -a.

Changes

  • apps/pi-extension/server/network.ts: add the direct-exec branch for slash-containing non-.app PLANNOTATOR_BROWSER values on darwin.
  • apps/pi-extension/server/network.test.ts: behavioral test — PLANNOTATOR_BROWSER pointing at a shell script gets executed with the URL as $1 (skipped on win32).

Testing

bun test apps/pi-extension/server/network.test.ts — new test passes; the 3 pre-existing port-binding failures on my machine fail identically on a clean main checkout (local environment, unrelated).

PunkDevRobot and others added 2 commits September 13, 2026 13:33
…rectly on macOS

open -a treats the value as an app bundle and fails with LaunchServices
-10811 for script paths, with the error swallowed by the detached spawn.
Mirror the binary's openBrowser branch: a value containing '/' that does
not end with '.app' is executed directly with the URL as its argument.

Fixes backnotprop#1391
…darwin CI

The backnotprop#1391 test reaches the darwin branch only on a real Mac; `bun test` runs
on ubuntu, where a configured browser falls through to the generic branch and
the regression is unguarded. Fake the platform the way the backnotprop#1472 WSL tests
already do, and cover both arms: a slash-containing non-.app value is spawned
directly, while an app name or .app bundle still goes through `open -a`.
@backnotprop
backnotprop force-pushed the pi-extension-browser-script-path branch from e65717d to d0be01b Compare September 13, 2026 20:39
@backnotprop

Copy link
Copy Markdown
Owner

Reviewed and rebased onto current main (one conflict in network.ts with #1472's viaCmdExe rewrite, resolved by keeping main's condition and re-applying your darwin split above it). Your commit is intact; I added one follow-up commit that fakes the platform so the darwin branch is also exercised on the ubuntu CI runner, otherwise the regression guard only ran on a real Mac. Pi suite is 280/280 locally. Will merge once CI is green. Thanks for the fix and for the patience.

@backnotprop
backnotprop merged commit 3e5e0b7 into backnotprop:main Sep 15, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pi-extension: PLANNOTATOR_BROWSER silently fails for script/executable paths on macOS

2 participants