Wirasm · Wirasm · Sep 3, 2026 · Sep 2, 2026
diff --git a/Patches/libghostty-spm-open-url-handled.patch b/Patches/libghostty-spm-open-url-handled.patch
@@ -0,0 +1,68 @@
+From c950adc65394f3dafd240ffa7e7ba445439b3ef7 Mon Sep 17 00:00:00 2001
+From: Rasmus Widing <rasmus.widing@gmail.com>
+Date: Wed, 2 Sep 2026 15:34:50 +0300
+Subject: [PATCH] fix(callbacks): report a handled open_url as handled
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Every action reported `false`, which ghostty reads as "the apprt did not
+handle this". For `open_url` its default is `internal_os.open`
+(Surface.zig `openUrl`), so a ⌘-click on a link in a pane opened twice:
+once through the host's delegate, and once through a `/usr/bin/open`
+that ghostty spawned itself.
+
+The second open is not merely redundant. Ghostty reads the child's
+stderr on a detached thread whose loop breaks only on `EndOfStream`, and
+after `/usr/bin/open` exits the reader returns zero-length slices
+forever instead — so the thread never leaves the loop, never reaches
+`exe.wait()`, and spins logging `open stderr=` at roughly 9k lines a
+second (ghostty-org/ghostty#13480, closed unmerged). Measured on a helm
+that had been up nine days: seven such threads, seven unreaped children,
+398% CPU, and 586 CPU-hours burnt. It never recovers — only a restart
+clears it, and each click adds another.
+
+It also meant the host's URL policy decided nothing. helm drops any
+scheme outside its allowlist, and ghostty opened it regardless.
+
+Reporting ownership needs the answer synchronously, so the main-thread
+case is split out: both `open_url` emitters run on the main thread, and
+an action arriving off it keeps the old async dispatch and the old
+`false`. Ownership is claimed only for `open_url`, and only when a
+delegate actually took it — every other action keeps ghostty's default.
+
+This is a backport. Upstream fixed it identically in 1.5.2; the code
+here is byte-identical to that so it retires cleanly on a repin.
+---
+ .../Controller/TerminalController+Callbacks.swift   | 13 ++++++++++---
+ 1 file changed, 10 insertions(+), 3 deletions(-)
+
+diff --git a/Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift b/Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift
+index 76632aa..cb66dec 100644
+--- a/Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift
++++ b/Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift
+@@ -36,11 +36,18 @@ private enum TerminalCallbacks {
+         let bridge = Unmanaged<TerminalCallbackBridge>
+             .fromOpaque(bridgePtr)
+             .takeUnretainedValue()
+-        terminalRunOnMain {
++        guard Thread.isMainThread else {
++            terminalRunOnMain { bridge.handleAction(action) }
++            return false
++        }
++        return MainActor.assumeIsolated {
+             bridge.handleAction(action)
++            // Core spawns /usr/bin/open for an open_url reported unhandled,
++            // so a host delegate that took the URL is reported as handling
++            // it. Both open_url emitters run on the main thread.
++            return action.tag == GHOSTTY_ACTION_OPEN_URL
++                && bridge.delegate is any TerminalSurfaceOpenURLDelegate
+         }
+-
+-        return false
+     }
+
+     static func closeSurface(
+-- 
+2.49.0
+
diff --git a/Sources/Helm/Terminals/TerminalSession.swift b/Sources/Helm/Terminals/TerminalSession.swift
@@ -623,6 +623,14 @@ extension TerminalSession: TerminalSurfaceLifecycleDelegate,
     /// the whole security story: terminal content is untrusted, so anything
     /// but http/https/file/mailto is dropped silently.
     ///
+    /// **That sentence is only true because the action callback reports this action as
+    /// handled**, which it did not until `Patches/libghostty-spm-open-url-handled.patch`.
+    /// ghostty reads an unhandled `open_url` as its own to perform and falls back to
+    /// `internal_os.open` (`Surface.zig:4415`), so every URL dropped below was opened by
+    /// ghostty anyway — the allowlist decided nothing, and the fallback also leaked a
+    /// spinning thread and an unreaped child per click. `docs/VENDORED.md` has the
+    /// measurement; `TerminalOpenURLOwnershipTests` pins the conformance it turns on.
+    ///
     /// It used to point out of the app: every link went to `NSWorkspace`, so a rendered
     /// report tabbed you into a browser — the trip helm exists to absorb. A link to
     /// something the canvas renders now opens **in helm** instead.

diff --git a/Tests/HelmTests/Terminals/TerminalOpenURLOwnershipTests.swift b/Tests/HelmTests/Terminals/TerminalOpenURLOwnershipTests.swift
@@ -0,0 +1,81 @@
+import GhosttyKit
+import GhosttyTerminal
+import XCTest
+
+@testable import Helm
+
+/// A ⌘-click on a link used to open it twice, and the second open never stopped running.
+///
+/// **The rule: an action helm's delegate took is reported to ghostty as taken.**
+///
+/// ghostty reads a `false` from the action callback as *the apprt did not handle this* and
+/// runs its own default. For `open_url` that default is `internal_os.open`
+/// (`Surface.zig:4415`), which spawns `/usr/bin/open` and reads its stderr on a detached
+/// thread. That thread's loop breaks only on `EndOfStream`, and after the child exits the
+/// reader returns zero-length slices instead — so it never leaves the loop, never reaches
+/// `exe.wait()`, and spins logging `open stderr=` at roughly 9k lines a second
+/// (ghostty-org/ghostty#13480, closed unmerged). Measured on a helm up for nine days: seven
+/// such threads, seven unreaped children, 398% CPU, 586 CPU-hours. Only a restart clears it,
+/// and every click adds another one.
+///
+/// The fix is one expression in the vendored wrapper's action callback
+/// (`Patches/libghostty-spm-open-url-handled.patch`), and it claims ownership only when a
+/// delegate actually took the URL:
+///
+///     action.tag == GHOSTTY_ACTION_OPEN_URL
+///         && bridge.delegate is any TerminalSurfaceOpenURLDelegate
+///
+/// **So the conformance below is load-bearing, which is the whole reason this file exists.**
+/// `scripts/patch-libghostty.sh` greps for a marker symbol, which proves the patch was
+/// *applied* and says nothing about whether its condition can still be met. Drop
+/// `TerminalSurfaceOpenURLDelegate` from `TerminalSession` and nothing fails to compile:
+/// the callback simply reports `false` again, ghostty resumes opening every link itself, and
+/// the leak is back with no diagnostic anywhere. That is a silent regression a marker grep
+/// cannot see, so it is pinned here instead.
+///
+/// What is deliberately *not* here: the callback itself. `TerminalCallbackBridge` is internal
+/// to `GhosttyTerminal` and the callback needs live `ghostty_app_t`/`ghostty_surface_t`
+/// pointers, so the predicate cannot be executed from this side.
+///
+/// **The live half is NOT covered here and was not run.** Proving the leak is gone needs a
+/// real link ⌘-clicked in a real pane, and a ⌘-click needs a display and an Accessibility
+/// grant no agent has (AGENTS.md). The check is: ⌘-click a link, then
+/// `ps -M -p <helm pid>` for a new thread pinned near 57%, and
+/// `ps -Ao pid,ppid,stat | awk '$2==<helm pid> && $3 ~ /Z/'` for a new zombie. Neither should
+/// appear, and on a build without this patch both do.
+final class TerminalOpenURLOwnershipTests: XCTestCase {
+    /// The condition the patch tests at runtime, asserted against the type helm actually
+    /// installs as the surface delegate.
+    func testTerminalSessionTakesOpenURLSoGhosttyDoesNotOpenItToo() {
+        XCTAssertTrue(
+            (TerminalSession.self as Any.Type) is (any TerminalSurfaceOpenURLDelegate.Type),
+            "TerminalSession must conform to TerminalSurfaceOpenURLDelegate: the vendored callback claims open_url only when it does, and without it ghostty falls back to /usr/bin/open and leaks a spinning thread per ⌘-click"
+        )
+    }
+
+    /// The other half of the same sentence, and the reason a bare `true` would be wrong:
+    /// ownership is claimed for `open_url` alone. Every other action keeps ghostty's default,
+    /// because no other fallback was measured and claiming one would suppress it blind.
+    func testOwnershipIsClaimedForOpenURLAndNothingElse() {
+        XCTAssertNotEqual(
+            GHOSTTY_ACTION_OPEN_URL,
+            GHOSTTY_ACTION_SET_TITLE,
+            "the patch keys on this tag; if the constants ever collapse the condition stops discriminating"
+        )
+    }
+
+    /// Taking the action is not the same as opening the URL, and the difference is the point
+    /// of the allowlist. helm drops a scheme outside it and *still* owns the action — before
+    /// the patch ghostty opened the dropped URL anyway, which made `TerminalURLPolicy`
+    /// advisory rather than the security boundary `TerminalSession` documents it as.
+    func testADroppedSchemeIsStillHelmsDecisionToMake() {
+        XCTAssertNil(
+            TerminalURLPolicy.validated("javascript:alert(1)"),
+            "the policy must still refuse this"
+        )
+        XCTAssertTrue(
+            (TerminalSession.self as Any.Type) is (any TerminalSurfaceOpenURLDelegate.Type),
+            "and refusing it must not read to ghostty as unhandled, or the fallback opens what the policy just dropped"
+        )
+    }
+}
diff --git a/docs/VENDORED.md b/docs/VENDORED.md
@@ -134,10 +134,23 @@ clones and patches it.
   revision `b0930320739324886590e865d571eb5dd7073912` — the same exact pin
   docs/SPIKE.md records. The patch does not touch the binary target, so the
   `GhosttyKit.xcframework.zip` URL and checksum are upstream's, unchanged.
+
+  **The script fetches that SHA explicitly, and has to.** A clone carries only what
+  its branches reach, and upstream has since moved past the branch that held
+  `b093032` — so a fresh clone does not contain the pinned commit at all. The
+  checkout after it died with `fatal: unable to read tree (b093032…)` on every new
+  worktree while an older `vendor/`, cloned before the prune, kept verifying fine.
+  The object is still served, it is just no longer advertised by any ref, so
+  `git fetch origin <sha>` is what keeps a pin-by-SHA actually reachable. Found by
+  running the script in a fresh worktree, where it had been failing silently: the
+  invocation was piped into `tail`, and a pipeline reports the *last* command's
+  status, so `set -e` inside the script could not surface it. AGENTS.md documents
+  that shape for `log show`; it bites here identically.
 - **Patches**, applied in this order by `scripts/patch-libghostty.sh` (both are `git
   format-patch` output; applied with `git am`):
   1. `Patches/libghostty-spm-multi-surface-wakeup.patch` — the multi-surface wakeup fix.
   2. `Patches/libghostty-spm-clipboard-destination.patch` — the clipboard-destination fix (#297).
+  3. `Patches/libghostty-spm-open-url-handled.patch` — report a handled `open_url` as handled.
 
   **Their diffstats are deliberately not restated here.** `git format-patch` already writes one
   into each patch file, so a copy in this document is a second spelling of a number nothing
@@ -237,6 +250,59 @@ owns — and are tracked separately.
 this decision from helm's own gate — a marker grep proves a patch was applied and says
 nothing about what it decides.
 
+## Patch 3 — report a handled `open_url` as handled
+
+**What it fixes.** `TerminalCallbacks.action` returned `false` for every action, and ghostty
+reads `false` as *the apprt did not handle this*. For `open_url` its default is
+`internal_os.open` (`Surface.zig:4415`, the only call site), so a ⌘-click on a link in a pane
+opened twice: once through helm's delegate, and once through a `/usr/bin/open` ghostty spawned
+itself.
+
+**The second open never stopped running.** Ghostty reads that child's stderr on a detached
+thread whose loop breaks only on `EndOfStream`; after `/usr/bin/open` exits the reader returns
+zero-length slices instead, so the thread never leaves the loop, never reaches `exe.wait()`,
+and logs `open stderr=` at roughly 9k lines a second forever. Upstream ghostty knows
+(ghostty-org/ghostty#13480) and the fix there is closed unmerged, so this is not something a
+newer XCFramework fixes.
+
+Measured on the operator's helm, up nine days, 2026-09-02:
+
+```
+7 spinning threads, ~57% of a core each        398% process CPU
+7 unreaped `/usr/bin/open` children            586 CPU-hours burnt
+93% of each thread's samples in os_log         logd 43%, analyticsd 49%
+```
+
+`sample` put every one of those threads in `zig_os_log_with_type` with **zero** I/O syscalls,
+hitting `__FIREHOSE_CLIENT_THROTTLED_DUE_TO_HEAVY_LOGGING__`. Their seven TIDs matched the
+seven zombies one-for-one, and the four oldest zombies matched the four threads with the
+largest accumulated CPU. It never recovers: only a restart clears it, and each ⌘-click adds
+another.
+
+**It also made `TerminalURLPolicy` advisory.** helm drops any scheme outside its allowlist,
+and `TerminalSession` calls that allowlist "the whole security story" — but dropping it
+returned `false` like everything else, so ghostty opened the dropped URL anyway.
+
+The patch claims ownership for `open_url`, and only when a delegate actually took it. The
+answer is needed synchronously, so the main-thread case is split out; both `open_url` emitters
+run on the main thread, and an action arriving off it keeps the old async dispatch and the old
+`false`. No other action's return value changes — no other fallback was measured, and claiming
+one would suppress it blind.
+
+**Why helm needs it.** helm hosts long-lived agents in its panes and cannot restart to shed a
+leak without killing them.
+
+**This one is a backport, which is why it is written to disappear.** Upstream fixed it
+identically in 1.5.2; the code here is byte-identical to upstream's, so a repin to ≥1.5.2
+retires this patch on its own rather than needing a rebase.
+**What is not proved by any gate**: that a ⌘-click no longer leaks. That needs a display and
+an Accessibility grant, so it is the operator's check — ⌘-click a link, then look for a new
+~57% thread in `ps -M -p <pid>` and a new zombie child. Neither should appear.
+`Tests/HelmTests/Terminals/TerminalOpenURLOwnershipTests.swift` pins the conformance the
+condition depends on — drop `TerminalSurfaceOpenURLDelegate` from `TerminalSession` and
+nothing fails to compile, the callback just reports `false` again and the leak returns with no
+diagnostic anywhere.
+
 ## Where the pin lives, and how it retires
 
 **Where the pin lives now.** A local path dependency is not recorded in
@@ -257,6 +323,12 @@ local pin puts it back into `Package.resolved` where the rest of the deps are.
 They retire independently: upstream taking one and not the other leaves `Patches/` holding
 whichever is left, and the script's array is already per-patch.
 
+**Patch 3 is already retired upstream, and that is an argument for repinning rather than a
+reason to do it today.** libghostty-spm 1.5.2 carries the same fix, so a repin drops that
+patch outright. It also moves the ghostty XCFramework several versions, which is a change with
+its own blast radius and its own gate run — worth doing deliberately, not as a side effect of
+a leak fix.
+
 Until one of those happens the branch does not build from a clean clone without
 running `scripts/patch-libghostty.sh` first. Verify the patches with:
 

diff --git a/scripts/patch-libghostty.sh b/scripts/patch-libghostty.sh
@@ -27,6 +27,7 @@ base_tag=1.3.1
 patches=(
   "libghostty-spm-multi-surface-wakeup.patch|wakeupSubscribers|Sources/GhosttyTerminal/Controller/TerminalController.swift"
   "libghostty-spm-clipboard-destination.patch|TerminalClipboardDestination|Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift"
+  "libghostty-spm-open-url-handled.patch|TerminalSurfaceOpenURLDelegate|Sources/GhosttyTerminal/Controller/TerminalController+Callbacks.swift"
 )
 
 for entry in "${patches[@]}"; do
@@ -96,6 +97,13 @@ if [ -d "$vendor" ]; then
 fi
 
 git clone https://github.com/Lakr233/libghostty-spm.git "$vendor"
+# A clone carries only what its branches reach, and upstream has since moved on
+# from the branch that once held $base — so a fresh clone does not contain the
+# pinned commit at all and the checkout below died with "unable to read tree" on
+# every new worktree, while an existing vendor/ from before the prune kept
+# working. Fetching the SHA directly is what makes a pin by SHA stay reachable;
+# the object is still served, it is just no longer advertised by any ref.
+git -C "$vendor" fetch --no-tags origin "$base"
 git -C "$vendor" checkout -b helm/patches "$base"
 for entry in "${patches[@]}"; do
   git -C "$vendor" am "$root/Patches/${entry%%|*}"