feat(cli): add --remote-debugging-port to preview and play#983
Conversation
Adds a Chromium remote debugging port flag for preview and play. The flag is only passed when launching an explicit browser/profile. HyperFrames still does not own CDP automation.
miguel-heygen
left a comment
There was a problem hiding this comment.
Good PR overall — clean design, solid test coverage for the parser, and the security rationale for requiring --user-data-dir is well thought out. A few things to address before merging:
Bug: port parsing in play.ts happens after the server starts
In play.ts, parseRemoteDebuggingPort is called after the HTTP server is created, bound to a port, and "Player running" / "Press Ctrl+C to stop" is printed. If the user passes an invalid value like 9222abc, they'll see the full server startup output, then hit the error + early return — leaving a running server behind that never gets cleaned up.
In preview.ts the parsing is correctly placed before runDevMode/runLocalStudioMode/runEmbeddedMode. The play.ts parsing should be moved up next to the existing --user-data-dir validation block, before any server setup.
Duplicated validation logic
The --remote-debugging-port requires --browser-path / requires --user-data-dir guard is copy-pasted identically in both play.ts and preview.ts. Consider extracting a validateRemoteDebuggingPortDeps(args) function alongside parseRemoteDebuggingPort in openBrowser.ts to keep them in sync.
Redundant test
"prepends --remote-debugging-port before the URL" and "includes all flags together" have identical inputs and identical assertions. One should be removed or given a distinct scenario (e.g., test that remoteDebuggingPort without userDataDir omits the flag from the args array).
Minor
parseRemoteDebuggingPortacceptsunknownbut every caller passesstring | undefined. Thenullcheck is dead code. Narrowing the param tostring | undefinedwould be more honest about the contract.- The 4-line JSDoc on
parseRemoteDebuggingPortis heavier than the surrounding file style — the function name already says everything the comment does. - Neither
play.tsnorpreview.tsadds an example for the new flag to theexamplesarray. Per project conventions these show up in--helpoutput.
- play.ts: move --remote-debugging-port parse+deps validation before any server setup so an invalid value exits cleanly instead of leaking a listening socket (the original bug — server printed 'Player running' and 'Press Ctrl+C to stop' before failing). - Extract validateRemoteDebuggingPortDeps() in openBrowser.ts to keep preview.ts and play.ts in sync instead of copy-pasting the dep checks. - Narrow parseRemoteDebuggingPort param to string | undefined; drop the dead null branch and the redundant String() / Number.isInteger() now that the regex already constrains the input. - buildBrowserArgs: omit --remote-debugging-port when userDataDir is missing so a CDP endpoint cannot leak into the user's main profile even if a caller bypasses the CLI validation layer. - Replace the duplicated buildBrowserArgs case with one that proves this defense-in-depth behaviour; add unit tests for validateRemoteDebuggingPortDeps. - Drop the heavy JSDoc on parseRemoteDebuggingPort to match the file's surrounding style. - Both commands: align --remote-debugging-port description (it now matches the actual 'requires --browser-path and --user-data-dir' contract) and add a CDP example to the --help output.
|
Thanks again for the review. Addressed the feedback in 4affe2b:
Also updated the PR description's Testing section to reflect the added Validated with package-scoped build/test/typecheck, oxlint, and oxfmt --check. |
Summary
Adds
--remote-debugging-port <port>tohyperframes previewandhyperframes play, completing the small browser-control trio started in #871 and #884:--no-openfor workflows that manage the browser lifecycle separately ("such as CDP or Playwright").--browser-pathand--user-data-dirfor launching an isolated browser profile.Default behavior is unchanged. Without
--browser-path, HyperFrames still uses the existing default-browser path through theopenpackage. With--no-open, no browser is launched.Why
External DevTools clients — Playwright
chromium.connectOverCDP(), Puppeteerpuppeteer.connect({ browserURL }),chrome-devtools-mcp, and similar — need a predictable DevTools endpoint to attach to. Today, HyperFrames either opens the user's default browser (no CDP port), or — with #884 — spawns a separate browser with--browser-pathbut no way to specify the debugging port.Typical usage:
The attached client can then connect to
http://127.0.0.1:9222, while HyperFrames still owns the preview/player server, file watching, and shutdown.Why
--user-data-diris requiredThe
--remote-debugging-portflag exposes a local API that can read DOM, cookies, and network traffic of every tab in the launched browser. Requiring an explicit--user-data-dirkeeps that port off the user's default profile, where it would otherwise expose any signed-in sessions the user happens to have open elsewhere. Some Chromium-based browsers may also reject or ignore remote debugging on a default profile, so failing fast at the CLI gives users a clearer error before launch.Behavior
--remote-debugging-port <port>is accepted on bothpreviewandplay.--browser-pathand--user-data-dir. Each missing dependency fails with a specific error message before the server starts.1to65535. Decimals, negative numbers, non-numeric input, andparseInt-style trailing junk (e.g.9222abc) are all rejected with the same error.--browser-path. The default-browser path (theopenpackage) is untouched.--no-openstill takes priority and launches no browser.Non-goals
--browser-pathpoints at a Chromium-compatible binary. Non-Chromium browsers simply ignore the flag. Users opting into--remote-debugging-portare expected to know they're targeting Chromium.Testing
Automated:
bun run --cwd packages/cli build bun run --filter @hyperframes/cli test bun run --filter @hyperframes/cli typecheck bunx oxlint packages/cli/src/commands/play.ts packages/cli/src/commands/preview.ts packages/cli/src/utils/openBrowser.ts packages/cli/src/utils/openBrowser.test.ts bunx oxfmt --check packages/cli/src/commands/play.ts packages/cli/src/commands/preview.ts packages/cli/src/utils/openBrowser.ts packages/cli/src/utils/openBrowser.test.tsResults on this branch:
@hyperframes/clibuild: passed.@hyperframes/clitest: 353 tests passed (23 inopenBrowser.test.ts).@hyperframes/clitypecheck: passed.oxlint: 0 errors.oxfmt --check: clean.New unit tests cover:
parseRemoteDebuggingPort: valid range, boundary values1/65535, decimals, negatives, non-numeric input, and trailing-junk rejection.validateRemoteDebuggingPortDeps: no-op when unset, success when all required flags are present, and specific errors for missing--browser-path/--user-data-dir.buildBrowserArgs: flag ordering with--user-data-dir+--remote-debugging-port, and defense-in-depth behavior that omits--remote-debugging-portwhenuserDataDiris missing.