Skip to content

Add terminal link destination chooser - #28

Merged
howdeploy merged 2 commits into
howdeploy:mainfrom
TroopJostle:feat/terminal-link-chooser
Sep 5, 2026
Merged

Add terminal link destination chooser#28
howdeploy merged 2 commits into
howdeploy:mainfrom
TroopJostle:feat/terminal-link-chooser

Conversation

@TroopJostle

Copy link
Copy Markdown
Contributor

Summary

  • detect plain HTTP(S) terminal output with the xterm web-links addon
  • route OSC 8 hyperlinks and detected URLs through one Canvas/system-browser chooser
  • open system-browser links through a trusted-renderer-only IPC bridge with HTTP(S), length, and credential validation
  • keep the embedded browser hidden while the modal chooser is open

This is the terminal-link-only half of #26. The project-path paste feature is intentionally excluded and is proposed separately in #27.

Verification

  • npm run typecheck
  • npm test (537/537 passing)
  • focused terminal-link and transformed-coordinate tests (5/5 passing)

@howdeploy

Copy link
Copy Markdown
Owner

Solid work — intercepting OSC 8 through linkHandler (instead of letting xterm fall back to confirm() + window.open()), reusing normalizeExternalUrl across all three open paths, and hiding the native view while the dialog is up are all the right calls. Two items before merge:

  1. Required: don't route unsafe URLs into the search fallback on the canvas-browser path. If an OSC 8 link carries credentials (https://user:secret@host) or a non-HTTP(S) scheme, the canvas-browser destination goes through openBrowserBrowserServicepolicy.normalizeHumanInput, which turns a rejected URL into a https://duckduckgo.com/?q=<full original URL> search query — the credentials leak to the search engine and the UX is unexpected. Please validate the URL before showing the chooser (or reject with an explicit error on the canvas path) so only clean HTTP(S) URLs without credentials can reach either destination. The system-browser path already does this correctly via normalizeExternalUrl.
  2. Process note, no action needed: this PR adds @xterm/addon-web-links. Repo policy requires explicit maintainer approval for new packages — confirmed and approved here; it's the official xterm addon with no peer conflicts.

Minor, non-blocking observations (fine as follow-ups):

  • Global capture-phase shortcuts (Home / Rename window) still fire while the chooser dialog is open.
  • On the system-browser failure path the user sees the raw Error invoking remote method … prefix — worth mapping to a localized message.

Since there are no CI checks on the branch, please confirm npm test, npm run typecheck, and npm run build pass locally on the head commit.

@TroopJostle

Copy link
Copy Markdown
Contributor Author

Fixed in b01c0fa. Terminal OSC 8 URLs now go through the shared normalizeExternalUrl validator before the destination chooser is shown, so credentialed, non-HTTP(S), malformed, and overlong URLs are rejected with an error and cannot reach either the canvas browser or system browser. I also added a regression assertion for the pre-chooser validation path.

Confirmed on the updated head:

  • npm test — 537 passed
  • npm run typecheck — passed
  • npm run build — passed

@howdeploy
howdeploy merged commit c197c16 into howdeploy:main Sep 5, 2026
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.

2 participants