diff --git a/include/ghostty.h b/include/ghostty.h index 4c050afc6ed..04bd8c57495 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -947,6 +947,21 @@ typedef struct { size_t len; } ghostty_action_mouse_over_link_s; +// cmux fork: (B) ExternalHover — apprt.action.ExternalLinkHover. `token_bits` +// mirrors renderer.link.HoverActivationToken (opaque, 4x u64); the host must +// treat it as opaque identity, never interpret individual words. `active` +// mirrors the ack semantics table in the design doc: true means this token is +// now the rendered hover state, false means it has been withdrawn. +// A host handler for external_link_hover runs on the renderer thread. It must +// not call ghostty_surface_free for the surface being reported, and must not +// block waiting for a free issued elsewhere to complete. A host that wants to +// tear down a surface in response to a hover event must post that work to +// another queue and return immediately. +typedef struct { + uint64_t token_bits[4]; + bool active; +} ghostty_action_external_link_hover_s; + // apprt.action.SizeLimit typedef struct { uint32_t min_width; @@ -1168,6 +1183,7 @@ typedef enum { GHOSTTY_ACTION_READONLY, GHOSTTY_ACTION_COPY_TITLE_TO_CLIPBOARD, GHOSTTY_ACTION_SELECTION_CHANGED, + GHOSTTY_ACTION_EXTERNAL_LINK_HOVER, } ghostty_action_tag_e; typedef union { @@ -1209,6 +1225,7 @@ typedef union { ghostty_action_search_total_s search_total; ghostty_action_search_selected_s search_selected; ghostty_action_readonly_e readonly; + ghostty_action_external_link_hover_s external_link_hover; } ghostty_action_u; typedef struct { @@ -1699,6 +1716,163 @@ GHOSTTY_API bool ghostty_surface_copy_selection_to_clipboard_bounded( GHOSTTY_API bool ghostty_surface_read_text(ghostty_surface_t, ghostty_selection_s, ghostty_text_s*); +// cmux fork: same as ghostty_surface_read_text, but does not unwrap +// soft-wrapped row boundaries into one logical line. Required for callers +// that map a screen row/column back to an offset in the returned text (e.g. +// Cmd-click path/link resolution), where joining two physical rows into one +// line would desync that mapping. +GHOSTTY_API bool ghostty_surface_read_text_physical_rows(ghostty_surface_t, + ghostty_selection_s, + ghostty_text_s*); + +// cmux fork: (B) ExternalHover — one cell range to underline. `row` is an +// ABSOLUTE VIEWPORT row (the same coordinate space as +// ghostty_surface_grid_metrics's rows / GHOSTTY_POINT_VIEWPORT) — NOT +// relative to `top_row`. It must still fall within +// [top_row, top_row + row_count) — the scope the accompanying setter call +// claims — or the call rejects it. +typedef struct { + uint16_t row; + uint16_t start_column; + uint16_t end_column; +} ghostty_external_hover_cell_range_s; + +// cmux fork: (B) ExternalHover — let the embedding host claim interactive +// hover rendering for a resolved link over `[top_row, top_row+row_count)` +// (inclusive-exclusive, VIEWPORT-RELATIVE physical rows — the same +// coordinate space GHOSTTY_POINT_VIEWPORT and +// ghostty_surface_grid_metrics's rows use, not absolute/scrollback- +// inclusive screen rows), instead of Ghostty's own regex-based hover. +// `text`/`text_len` must be the exact physical-row text for that same +// range — read it with ghostty_surface_read_text_physical_rows using a +// GHOSTTY_POINT_VIEWPORT selection over the identical `[top_row, +// top_row+row_count)` rows — in the same non-unwrapped form that call +// returns. Ghostty fingerprints this text itself and re-validates it +// every frame, so a stale or mismatched text argument only ever fails +// safe (the call returns false, or a later frame invalidates the hover). +// Every entry in `ranges` must fall within `[top_row, top_row+row_count)` +// (see ghostty_external_hover_cell_range_s) or the call rejects outright. +// On success, writes an opaque activation token to `out_token_bits` (must +// be later passed verbatim to ghostty_surface_clear_external_link_hover) +// and returns true; returns false without writing `out_token_bits` if the +// row scope is out of bounds, the text is too large, or any range is +// invalid or out of scope. +// `host_event_id` (cmux fork: (C) ExternalHover diagnostics, design +// v4 §2) is the host's own correlation id for this hover activation +// (e.g. the surface-local monotonic id that produced this candidate). +// It has NO effect on accept/reject behavior or ABI shape across build +// configurations — Debug, Release, and ReleaseFast all take this same +// parameter — and is only ever recorded into diagnostic ring entries +// drained via ghostty_surface_drain_external_hover_diagnostics, subject +// to that mechanism's own on/off gate. Pass 0 if the host has no +// meaningful id to correlate. +GHOSTTY_API bool ghostty_surface_set_external_link_hover( + ghostty_surface_t, + uint32_t top_row, + uint32_t row_count, + const char* text, + size_t text_len, + const ghostty_external_hover_cell_range_s* ranges, + size_t range_count, + uint64_t out_token_bits[4], + uint64_t host_event_id); + +// cmux fork: (B) ExternalHover — release a hover claimed by a prior +// ghostty_surface_set_external_link_hover call, identified by the token that +// call returned. A no-op if `token_bits` no longer matches the currently +// active hover (already invalidated by a newer event, a screen change, or a +// prior clear). +GHOSTTY_API void ghostty_surface_clear_external_link_hover( + ghostty_surface_t, + const uint64_t token_bits[4]); + +// cmux fork: (C) ExternalHover diagnostics — bug C (#8810) hover lifecycle +// tracing (design-hover-diagnostics-v4-final.md). One fixed-size POD +// diagnostic entry from a surface's internal ring buffer. No strings, no +// pointers: `source`/`reason`/`verdict` are enum raw values the host +// decodes itself (an unrecognized raw value must format as +// "unknown()", never crash — the ABI can drift ahead of an +// out-of-date host). `event` is whatever `host_event_id` the setter call +// that produced this activation passed in; entries with no associated +// activation (e.g. a setter rejection) still carry the `host_event_id` +// of that specific call. `flags` bit 0 is `firstForActivation`. This +// struct's layout (field order and widths) mirrors Zig's +// `renderer/link.zig` `ExternalHoverDiagEntry` `extern struct` exactly — +// keep both in sync. +typedef struct { + uint64_t event; + uint8_t source; + uint8_t reason; + uint8_t verdict; + uint8_t flags; + uint32_t seq; +} ghostty_external_hover_diag_entry_s; + +// cmux fork: keep the two ExternalHover POD layouts stable for both C and C++ +// consumers. These declarations intentionally pin every field offset as well +// as the total size; the Zig `extern struct` definitions mirror these values. +#ifdef __cplusplus + #define GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT static_assert +#else + #define GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT _Static_assert +#endif + +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + sizeof(ghostty_external_hover_cell_range_s) == 6, + "ghostty_external_hover_cell_range_s size changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_cell_range_s, row) == 0, + "ghostty_external_hover_cell_range_s.row offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_cell_range_s, start_column) == 2, + "ghostty_external_hover_cell_range_s.start_column offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_cell_range_s, end_column) == 4, + "ghostty_external_hover_cell_range_s.end_column offset changed"); + +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + sizeof(ghostty_external_hover_diag_entry_s) == 16, + "ghostty_external_hover_diag_entry_s size changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, event) == 0, + "ghostty_external_hover_diag_entry_s.event offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, source) == 8, + "ghostty_external_hover_diag_entry_s.source offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, reason) == 9, + "ghostty_external_hover_diag_entry_s.reason offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, verdict) == 10, + "ghostty_external_hover_diag_entry_s.verdict offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, flags) == 11, + "ghostty_external_hover_diag_entry_s.flags offset changed"); +GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT( + offsetof(ghostty_external_hover_diag_entry_s, seq) == 12, + "ghostty_external_hover_diag_entry_s.seq offset changed"); + +#undef GHOSTTY_EXTERNAL_HOVER_STATIC_ASSERT + +// cmux fork: (C) ExternalHover diagnostics — destructively drains up to +// `out_capacity` of the oldest live diagnostic entries from this +// surface's fixed ring into `out_entries`, returning the number actually +// copied (0..out_capacity; any remainder stays in the ring for a later +// call — nothing is discarded except by the ring's own bounded +// overflow). `out_dropped_count_cumulative` receives the ring's +// monotonic cumulative overflow-drop count, NOT a delta — the caller +// must track its own previous value per surface and compute +// `droppedDelta = current - previous` itself, so the same cumulative +// value is never double-reported across two drains. Present in every +// build configuration (Debug/Release/ReleaseFast) with this identical +// signature; when the diagnostics gate is off this returns 0 and leaves +// `out_dropped_count_cumulative` at 0 without touching the ring. +GHOSTTY_API size_t ghostty_surface_drain_external_hover_diagnostics( + ghostty_surface_t, + ghostty_external_hover_diag_entry_s* out_entries, + size_t out_capacity, + uint64_t* out_dropped_count_cumulative); + // cmux fork: read clipboard-formatted plain text from inclusive absolute screen // rows without mutating the active selection. This preserves clipboard trimming // and codepoint-map settings for off-viewport copy-mode fallback copies. diff --git a/src/Surface.zig b/src/Surface.zig index af0b6611242..574b912d31e 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1900,6 +1900,17 @@ fn modsChanged(self: *Surface, mods: input.Mods) void { defer self.renderer_state.mutex.unlock(global.io()); self.renderer_state.mouse.mods = self.mouseModsWithCapture(self.mouse.mods); + // cmux fork: (B) ExternalHover — a normalized-mods change + // invalidates any in-flight hover activation token, since the + // token is minted from these same mods. Only bumped here, + // inside the gate above, so it fires once per real mods + // change (never per event). `hover_context_epoch` (renamed + // from `hover_input_epoch` — (B) flicker fix §3) now carries + // ONLY mods/eligibility ABA guarding; pointer/cell validity is + // range-containment plus the input-time invalidation in + // `cursorPosCallback`, not this epoch. + self.renderer_state.mouse.hover_context_epoch +%= 1; + // We use the clear screen dirty flag to force a rebuild of all // rows because changing mouse mods can affect the highlight state // of a link. If there is no link this seems very wasteful but @@ -2423,20 +2434,29 @@ pub fn dumpText( ) !Text { self.renderer_state.mutex.lockUncancelable(global.io()); defer self.renderer_state.mutex.unlock(global.io()); - return try self.dumpTextLocked(alloc, sel); + return try self.dumpTextLocked(alloc, sel, true); } /// Same as `dumpText` but assumes the renderer state mutex is already /// held. +/// +/// `unwrap` controls whether soft-wrapped rows are joined into one logical +/// line (the historical behavior, used for clipboard-style reads) or each +/// emit their own newline so the result has one line per physical screen +/// row (cmux fork: needed by callers that map a screen row/column back to +/// an offset in the returned text, e.g. terminal path/link resolution — +/// see ghostty_surface_read_text_physical_rows). pub fn dumpTextLocked( self: *Surface, alloc: Allocator, sel: terminal.Selection, + unwrap: bool, ) !Text { // Read out the text const text = try self.io.terminal.screens.active.selectionString(alloc, .{ .sel = sel, .trim = false, + .unwrap = unwrap, }); errdefer alloc.free(text); @@ -5522,6 +5542,158 @@ pub fn mouseCaptured(self: *Surface) bool { return self.io.terminal.flags.mouse_event != .none; } +// cmux fork: (B) ExternalHover — the embedding host's hover-override +// setter/clear. Both only mutate `renderer_state.mouse.external_hover` +// (and mark the hover row dirty so a render is scheduled); neither calls +// into the apprt inline. The render loop is the only place a +// `GHOSTTY_ACTION_EXTERNAL_LINK_HOVER`-style transition is ever produced, +// after releasing this same mutex — see `generic.zig` and +// `Thread.notifyExternalHoverTransition`. + +/// Mints a fresh `HoverActivationToken` from the current pointer/mods/ +/// epoch and `joined_physical_rows_text` (which must be exactly what +/// `ghostty_surface_read_text_physical_rows` returns for `[top_row, +/// top_row+row_count)`, read by the host under the same mutex it now +/// calls this with), and stores `ranges` under it. Returns +/// `HoverActivationToken.zero` on any rejection: an out-of-bounds scope, +/// oversized content, or oversized/invalid ranges (see +/// `link.ExternalHover.set`) — the host must treat a zero token as "not +/// set" and never store it as a pending/accepted owner. +/// +/// (C) diagnostics — `host_event_id` is design v4 §2's correlation +/// bridge: the SAME value the host will later look for in drained +/// diagnostic entries. Every early-return below funnels through the +/// single `reason` block so the production accept/reject decision and +/// the diagnostic push it drives always agree (design v4 §7 guard 1) — +/// there is no separate "diagnostic reason" re-derivation anywhere in +/// this function. +pub fn setExternalLinkHover( + self: *Surface, + top_row: u32, + row_count: u32, + joined_physical_rows_text: []const u8, + ranges: []const rendererpkg.link.ExternalHoverCellRange, + host_event_id: u64, +) rendererpkg.link.HoverActivationToken { + self.renderer_state.mutex.lockUncancelable(global.io()); + defer self.renderer_state.mutex.unlock(global.io()); + + const zero = rendererpkg.link.HoverActivationToken.zero; + const Reason = rendererpkg.link.ExternalHoverDiagReason; + + const reason: Reason = reason: { + if (row_count == 0) break :reason .zeroRowCount; + // cmux fork: (B) wiring review Blocking 6 — reject outright while + // hover is currently ineligible (selection/drag/mouse-capture in + // progress), the same gate native link hover already respects. A + // setter call racing an eligibility change must never install an + // override the input path has just decided hover shouldn't show. + if (!self.renderer_state.mouse.hover_eligible) break :reason .hoverIneligible; + const screens = &self.renderer_state.terminal.screens; + const rows: u32 = @intCast(screens.active.pages.rows); + if (top_row >= rows or row_count > rows - top_row) break :reason .scopeOutOfBounds; + + // (B) flicker fix §4 — viewport identity (scrollbar row-space + // revision + offset) folds into the physical proof alongside + // content, so a setter call at one scroll position can never + // validate against a later frame at a different one. See + // `buildPhysicalSnapshotToken`'s doc. + const scrollbar = screens.active.pages.scrollbar(); + const physical = rendererpkg.link.buildPhysicalSnapshotToken( + @intFromPtr(self.renderer_state.terminal), + rendererpkg.link.externalHoverScreenKeyByte(screens.active_key), + screens.generation(screens.active_key), + top_row, + row_count, + screens.active.pages.cols, + joined_physical_rows_text, + scrollbar.row_space_revision, + scrollbar.offset, + ) orelse break :reason .snapshotBuildFailed; + + const mods_bits: input.Mods.Backing = @bitCast(self.renderer_state.mouse.mods); + // (B) flicker fix §3 — the activation token minted here remains + // the host-visible clear/ack identity (still hashing pointer/ + // mods/epoch, same as before), but is no longer the value + // render-time validity is decided from — `set` below stores + // `physical`/`context_epoch` separately for that. See + // `ExternalHover`'s doc. + const activation = rendererpkg.link.buildHoverActivationToken( + physical, + self.renderer_state.mouse.pointer_cell, + mods_bits, + self.renderer_state.mouse.hover_context_epoch, + ); + + // (B) flicker fix §1's setter containment guard lives inside + // `set` itself (see its doc) — the current pointer must be + // non-null and inside `ranges`, checked at the moment of the + // call, since the pointer can have moved between the host's + // currentness check and this C-boundary call. + break :reason self.renderer_state.mouse.external_hover.set( + activation, + physical, + self.renderer_state.mouse.hover_context_epoch, + self.renderer_state.mouse.pointer_cell, + top_row, + row_count, + ranges, + host_event_id, + ); + }; + + if (reason != .none) { + // (C) diagnostics — design v4 §2: "setter が reject された場合も、 + // その call の event と reason を同期的に ring へ積む(reject は + // activation を作らないため)". No activation exists yet at this + // point, so this pushes directly rather than going through + // `ExternalHover.recordRenderVerdict` (which requires an active + // activation). + self.renderer_state.mouse.external_hover_diag.push(.{ + .event = host_event_id, + .source = @intFromEnum(rendererpkg.link.ExternalHoverDiagSource.setter), + .reason = @intFromEnum(reason), + }); + return zero; + } + + self.renderer_state.terminal.screens.active.dirty.hyperlink_hover = true; + self.queueRender() catch |err| { + log.warn("failed to queue render after external hover set err={}", .{err}); + // (C) diagnostics — design v4 §4: a `queueRender` failure right + // after an accepted setter is the direct reason no render/ack + // ever follows. The activation is invalidated before returning so + // callers cannot observe a live token that will never be delivered. + self.renderer_state.mouse.external_hover_diag.push(.{ + .event = host_event_id, + .source = @intFromEnum(rendererpkg.link.ExternalHoverDiagSource.setter), + .reason = @intFromEnum(rendererpkg.link.ExternalHoverDiagReason.renderQueueFailed), + }); + self.renderer_state.mouse.external_hover.invalidate(); + return zero; + }; + return self.renderer_state.mouse.external_hover.token; +} + +/// Discards the override if it's still `token`. Idempotent success: this +/// always returns (nothing to return — success is the only outcome) +/// whether or not `token` was actually still current, since either way +/// the postcondition "`token` is not the active override" holds afterward. +/// The host calls this immediately after a hover recompute finds no +/// candidate, so a stale override cannot outlive the mouse having moved off +/// it just because no further render happened to re-validate it in time. +pub fn clearExternalLinkHover(self: *Surface, token: rendererpkg.link.HoverActivationToken) void { + self.renderer_state.mutex.lockUncancelable(global.io()); + defer self.renderer_state.mutex.unlock(global.io()); + if (!self.renderer_state.mouse.external_hover.active()) return; + if (!self.renderer_state.mouse.external_hover.token.eql(token)) return; + self.renderer_state.mouse.external_hover.invalidate(); + self.renderer_state.terminal.screens.active.dirty.hyperlink_hover = true; + self.queueRender() catch |err| { + log.warn("failed to queue render after external hover clear err={}", .{err}); + }; +} + /// Called for mouse button press/release events. This will return true /// if the mouse event was consumed in some way (i.e. the program is capturing /// mouse events). If the event was not consumed, then false is returned. @@ -6508,9 +6680,26 @@ pub fn cursorPosCallback( // log.debug("cursor pos x={} y={} mods={?}", .{ pos.x, pos.y, mods }); + // cmux fork: (B) ExternalHover flicker fix (review-flicker-fix-confirm.md + // §1's blocking finding) — computed once, reused below to guard the + // ONLY place `pointer_cell` may be reassigned to a real viewport cell. + // The pre-existing negative-position branch below sets `pointer_cell = + // null` and does NOT return (by design: mods/selection/scroll + // processing further down must still run for a negative-position + // event), so without this guard the unconditional common-path + // reassignment a few lines later would silently overwrite that `null` + // with `posToViewport`'s clamped `(0, 0)` — `Coordinate.convert(.grid)` + // clamps negative coordinates to the grid origin rather than returning + // an out-of-bounds sentinel. That reintroduces exactly the bug this + // fix closes: a viewport-exit event would otherwise look like a + // legitimate move to cell (0,0), which the new range-containment check + // could then wrongly validate if `(0,0)` happens to fall inside the + // active override's ranges. + const is_out_of_viewport = pos.x < 0 or pos.y < 0; + // If the position is negative, it is outside our viewport and // we need to clear any hover states. - if (pos.x < 0 or pos.y < 0) { + if (is_out_of_viewport) { // Reset our hyperlink state self.mouse.link_point = null; if (self.mouse.over_link) { @@ -6534,6 +6723,21 @@ pub fn cursorPosCallback( // No mouse point so we don't highlight links self.renderer_state.mouse.point = null; + // cmux fork: (B) ExternalHover flicker fix — `pointer_cell` + // update and the input-time destructive invalidation this fix + // requires are both handled by the SAME pure method + // (`Mouse.updateExternalHoverPointerCell`, `renderer/State.zig`) + // the common path below also calls, extracted specifically so + // the exact clamped-`(0,0)`-must-not-resurrect regression this + // branch guards against is unit-testable without a live + // `Surface`. Passing `null` (never `pos_vp`, which is computed + // further below and would be a clamped `(0, 0)` for a negative + // position) is what actually closes that regression. + // `hover_eligible` stays a direct assignment here — it's not + // part of that shared pointer-cell contract. + _ = self.renderer_state.mouse.updateExternalHoverPointerCell(null); + self.renderer_state.mouse.hover_eligible = false; + // Mark the link's row as dirty, but continue with updating the // mouse state below so we can scroll when our position is negative. self.renderer_state.terminal.screens.active.dirty.hyperlink_hover = true; @@ -6563,6 +6767,64 @@ pub fn cursorPosCallback( // event. self.renderer_state.mouse.point = null; + // cmux fork: (B) ExternalHover — `pointer_cell` tracks every in-bounds + // cell regardless of native hover's outcome (unlike `point` above, + // which native hover resolution overwrites below only when it finds a + // link). Guarded by `!is_out_of_viewport` (see its doc above) — the + // negative-position branch already assigned `pointer_cell = null` as + // this event's ONLY assignment, and `pos_vp` here is a clamped `(0, + // 0)` that must never overwrite it. + // + // `hover_context_epoch` (renamed from `hover_input_epoch` — (B) + // flicker fix §3 narrows its role) is no longer bumped for a plain + // cell change: range containment now decides in-range validity + // (`ExternalHover.validateOrInvalidate`), and any range-EXIT is + // handled immediately below via `invalidateIfPointerLeftRanges`, not + // by a delayed epoch mismatch. Bumping it here would be redundant at + // best and, before this fix, was the only thing standing between a + // same-cell-resent-native regression and a real one (see + // `TerminalHoverIndicatorState`'s host-side doc for the cmux side of + // that same lesson). + if (!is_out_of_viewport) { + // (B) flicker fix §1 — range-exit (not just viewport-exit above) + // destructively invalidates immediately, at input time. Moving + // from a cell inside the active override's ranges to one outside + // them is the same coalescing-hazard ABA case viewport exit is; + // moving between two cells that are BOTH inside the ranges must + // NOT invalidate (that's the whole point of this fix — the + // indicator stays put while the pointer travels along the same + // resolved link). Unlike the viewport-exit branch above (which + // already marks the row dirty unconditionally for its own, + // unrelated reasons), this path has no other dirty/render trigger + // for a same-frame indicator update, so explicitly queue one only + // when an invalidation actually happened. + if (self.renderer_state.mouse.updateExternalHoverPointerCell(pos_vp)) { + self.renderer_state.terminal.screens.active.dirty.hyperlink_hover = true; + self.queueRender() catch |err| { + log.warn("failed to queue render after external hover range-exit invalidate err={}", .{err}); + }; + } + } + + // Hover (native or external) is suppressed while a left-button + // gesture (selection or link-activation drag) is in progress, in + // addition to the existing mouse-reporting/mods gate that already + // decides native hover eligibility. + // + // cmux fork: (B) wiring review Blocking 6 — bump the epoch when + // eligibility itself flips, not only when `pointer_cell` moves. A + // selection starting (or mouse capture beginning) while the pointer + // sits still over the same cell must still invalidate a token minted + // while hover was eligible; without this, that token's epoch would + // stay unchanged and `validateOrInvalidate` could keep treating it as + // current even though eligibility now says hover shouldn't render. + const prior_hover_eligible = self.renderer_state.mouse.hover_eligible; + self.renderer_state.mouse.hover_eligible = self.mouseLinkRefreshAllowed() and + self.mouse.click_state[@intFromEnum(input.MouseButton.left)] != .press; + if (prior_hover_eligible != self.renderer_state.mouse.hover_eligible) { + self.renderer_state.mouse.hover_context_epoch +%= 1; + } + // If we have an inspector, we need to always record position information if (self.inspector) |insp| { insp.mouse.last_xpos = pos.x; diff --git a/src/apprt/action.zig b/src/apprt/action.zig index 59761dbb234..f80399ce57a 100644 --- a/src/apprt/action.zig +++ b/src/apprt/action.zig @@ -349,6 +349,20 @@ pub const Action = union(Key) { /// through the normal surface APIs. This carries no payload. selection_changed, + /// cmux fork: (B) ExternalHover — the render loop's own bounded + /// active/inactive transition for the embedding host's hover + /// override. See `ExternalLinkHover` and `renderer/link.zig`'s + /// `ExternalHover`. Appended at the end (not inserted alongside + /// `mouse_over_link`) so it never shifts the ordinal value of any + /// pre-existing tag — see "Action.Key preserves the public C ABI". + /// + /// A host handler for `external_link_hover` runs on the renderer thread. + /// It must not call `ghostty_surface_free` for the surface being reported, + /// and must not block waiting for a free issued elsewhere to complete. A + /// host that wants to tear down a surface in response to a hover event must + /// post that work to another queue and return immediately. + external_link_hover: ExternalLinkHover, + /// Sync with: ghostty_action_tag_e pub const Key = enum(c_int) { quit, @@ -417,6 +431,7 @@ pub const Action = union(Key) { readonly, copy_title_to_clipboard, selection_changed, + external_link_hover, test "ghostty.h Action.Key" { try lib.checkGhosttyHEnum(Key, "GHOSTTY_ACTION_"); @@ -431,6 +446,10 @@ pub const Action = union(Key) { @as(c_int, 65), @intFromEnum(Key.selection_changed), ); + try std.testing.expectEqual( + @as(c_int, 66), + @intFromEnum(Key.external_link_hover), + ); } }; @@ -468,9 +487,14 @@ pub const Action = union(Key) { // For ABI compatibility, we expect that this is our union size. // At the time of writing, we don't promise ABI compatibility // so we can change this but I want to be aware of it. + // + // cmux fork: (B) ExternalHover's `ExternalLinkHover.C` (a 32-byte + // `[4]u64` token plus a bool) is now the union's largest member, + // growing this from the upstream 16/24 to 40/40 on 4-/8-byte + // pointer builds respectively. assert(@sizeOf(CValue) == switch (@sizeOf(usize)) { - 4 => 16, - 8 => 24, + 4 => 40, + 8 => 40, else => unreachable, }); } @@ -678,6 +702,31 @@ pub const MouseOverLink = struct { } }; +/// cmux fork: (B) ExternalHover — bounded value payload for the +/// `external_link_hover` action. Deliberately just a token and an active +/// flag: the host owns the token->path mapping itself (see +/// `renderer/link.zig`'s `HoverActivationToken` doc), so no path string +/// crosses the C ABI here. +/// A host handler for `external_link_hover` runs on the renderer thread. It +/// must not call `ghostty_surface_free` for the surface being reported, and +/// must not block waiting for a free issued elsewhere to complete. A host that +/// wants to tear down a surface in response to a hover event must post that +/// work to another queue and return immediately. +pub const ExternalLinkHover = struct { + token: renderer.link.HoverActivationToken, + active: bool, + + // Sync with: ghostty_action_external_link_hover_s + pub const C = extern struct { + token_bits: [4]u64, + active: bool, + }; + + pub fn cval(self: ExternalLinkHover) C { + return .{ .token_bits = self.token.bits, .active = self.active }; + } +}; + pub const SizeLimit = extern struct { min_width: u32, min_height: u32, diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 1e50dfabcba..b864e4371f5 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -823,6 +823,70 @@ test "embedded surface teardown completes before a retained action returns" { ); } +test "external link hover handler posts teardown and returns before teardown runs" { + const Event = enum { + handler_entered, + teardown_posted, + handler_returned, + teardown_ran, + }; + const Observation = struct { + events: [4]Event = undefined, + count: usize = 0, + + fn record(self: *@This(), event: Event) void { + std.debug.assert(self.count < self.events.len); + self.events[self.count] = event; + self.count += 1; + } + }; + const Callbacks = struct { + fn action( + app: *App, + _: apprt.Target.C, + action_value: apprt.Action.C, + ) callconv(.c) bool { + if (action_value.key != .external_link_hover) return false; + const observation: *Observation = @ptrCast(@alignCast(app.opts.userdata.?)); + observation.record(.handler_entered); + observation.record(.teardown_posted); + observation.record(.handler_returned); + return true; + } + }; + + var observation: Observation = .{}; + var app: App = undefined; + app.opts.userdata = &observation; + app.opts.action = Callbacks.action; + var surface: CoreSurface = undefined; + + const accepted = try app.performAction( + .{ .surface = &surface }, + .external_link_hover, + .{ + .token = .{ .bits = .{ 1, 2, 3, 4 } }, + .active = true, + }, + ); + try std.testing.expect(accepted); + + // Models the host queue consuming the posted teardown only after the + // renderer-thread action handler has returned. + observation.record(.teardown_ran); + const expected = [_]Event{ + .handler_entered, + .teardown_posted, + .handler_returned, + .teardown_ran, + }; + try std.testing.expectEqualSlices( + Event, + &expected, + observation.events[0..observation.count], + ); +} + pub const Surface = struct { app: *App, platform: Platform, @@ -2864,7 +2928,7 @@ pub const CAPI = struct { const core_sel = core_surface.io.terminal.screens.active.selection orelse return false; // Read the text from the selection. - return readTextLocked(surface, core_sel, result); + return readTextLocked(surface, core_sel, result, true); } /// Read clipboard-formatted plain text from the active selection while @@ -2914,7 +2978,111 @@ pub const CAPI = struct { surface.core_surface.renderer_state.terminal.screens.active, ) orelse return false; - return readTextLocked(surface, core_sel, result); + return readTextLocked(surface, core_sel, result, true); + } + + /// cmux fork: same as `ghostty_surface_read_text`, but every physical + /// screen row emits its own newline instead of joining soft-wrapped + /// rows into one logical line. Callers that need to map a screen + /// row/column (e.g. from a mouse click) back to an offset in the + /// returned text must use this variant — with the default unwrapping + /// behavior, a row that soft-wraps onto the next collapses two + /// physical rows into one line, which desyncs any row-index-based + /// lookup into the result. + export fn ghostty_surface_read_text_physical_rows( + surface: *Surface, + sel: Selection, + result: *Text, + ) bool { + surface.core_surface.renderer_state.mutex.lockUncancelable(global.io()); + defer surface.core_surface.renderer_state.mutex.unlock(global.io()); + + const core_sel = sel.core( + surface.core_surface.renderer_state.terminal.screens.active, + ) orelse return false; + + return readTextLocked(surface, core_sel, result, false); + } + + /// cmux fork: (B) ExternalHover — claim interactive hover rendering for a + /// resolved link over `[top_row, top_row+row_count)`, a VIEWPORT-RELATIVE + /// physical-row range (the same coordinate space as + /// `ghostty_surface_grid_metrics`'s rows / `GHOSTTY_POINT_VIEWPORT`). + /// `text`/`text_len` must be the physical-row text for that exact range + /// (see `ghostty_surface_read_text_physical_rows` with a + /// `GHOSTTY_POINT_VIEWPORT` selection); `ranges`/`range_count` are the + /// cells to underline — `row` in each is an ABSOLUTE VIEWPORT row (NOT + /// relative to `top_row`; see `ghostty_external_hover_cell_range_s` and + /// `renderer/link.zig`'s `ExternalHover.set`/`replaceCells`), and must + /// still fall within `[top_row, top_row+row_count)` or the call rejects. + /// On success writes the activation token to `out_token_bits` and + /// returns true. `Surface.setExternalLinkHover` takes the renderer + /// mutex itself; this wrapper must not lock it too. + export fn ghostty_surface_set_external_link_hover( + surface: *Surface, + top_row: u32, + row_count: u32, + text: [*]const u8, + text_len: usize, + ranges: [*]const renderer.link.ExternalHoverCellRange, + range_count: usize, + out_token_bits: *[4]u64, + host_event_id: u64, + ) bool { + const token = surface.core_surface.setExternalLinkHover( + top_row, + row_count, + text[0..text_len], + ranges[0..range_count], + host_event_id, + ); + if (token.eql(renderer.link.HoverActivationToken.zero)) return false; + out_token_bits.* = token.bits; + return true; + } + + /// cmux fork: (B) ExternalHover — release a hover claimed by a prior + /// `ghostty_surface_set_external_link_hover` call. A no-op if the token + /// no longer matches the currently active hover. + export fn ghostty_surface_clear_external_link_hover( + surface: *Surface, + token_bits: *const [4]u64, + ) void { + surface.core_surface.clearExternalLinkHover(.{ .bits = token_bits.* }); + } + + /// cmux fork: (C) ExternalHover diagnostics — bug C (#8810) hover + /// lifecycle tracing. Destructively drains up to `out_capacity` + /// oldest live diagnostic entries from this surface's fixed POD ring + /// into `out_entries`, returning the number actually copied (never + /// more than `out_capacity`; any remainder stays in the ring for a + /// later call). `out_dropped_count_cumulative` receives the ring's + /// monotonic cumulative overflow-drop count — NOT a delta; the host + /// must retain its own previous value per surface and compute + /// `droppedDelta = current - previous` itself (design v4 §3.3), so + /// the same cumulative value is never double-reported across drains. + /// + /// Present in ALL build configurations with an identical signature + /// (design v4 §7 guard 5) — when the diagnostics gate + /// (`CMUX_EXTERNAL_HOVER_DIAGNOSTICS=1`) is off, this returns 0 + /// without touching the renderer mutex or the ring at all (guard 4: + /// gate off means no ring append/drain). + export fn ghostty_surface_drain_external_hover_diagnostics( + surface: *Surface, + out_entries: [*]renderer.link.ExternalHoverDiagEntry, + out_capacity: usize, + out_dropped_count_cumulative: *u64, + ) usize { + if (!renderer.link.externalHoverDiagnosticsEnabled()) { + out_dropped_count_cumulative.* = 0; + return 0; + } + surface.core_surface.renderer_state.mutex.lockUncancelable(global.io()); + defer surface.core_surface.renderer_state.mutex.unlock(global.io()); + const ring = &surface.core_surface.renderer_state.mouse.external_hover_diag; + const n = ring.drain(out_entries[0..out_capacity]); + out_dropped_count_cumulative.* = ring.dropped_count; + return n; } /// cmux fork: read clipboard-formatted plain text from inclusive absolute @@ -3046,6 +3214,7 @@ pub const CAPI = struct { surface: *Surface, core_sel: terminal.Selection, result: *Text, + unwrap: bool, ) bool { const core_surface = &surface.core_surface; @@ -3053,6 +3222,7 @@ pub const CAPI = struct { const text = core_surface.dumpTextLocked( global.alloc(), core_sel, + unwrap, ) catch |err| { log.warn("error reading text err={}", .{err}); return false; @@ -5298,7 +5468,7 @@ pub const CAPI = struct { }; // Read the selection - return readTextLocked(ptr, sel, result); + return readTextLocked(ptr, sel, result, true); } export fn ghostty_inspector_metal_init(ptr: *Inspector, device: objc.c.id) bool { diff --git a/src/renderer.zig b/src/renderer.zig index f47ac7f1125..f3e32eee495 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -16,6 +16,7 @@ const message = @import("renderer/message.zig"); const size = @import("renderer/size.zig"); pub const frame_lease = @import("renderer/frame_lease.zig"); pub const external_frame = @import("renderer/external_frame.zig"); +pub const link = @import("renderer/link.zig"); pub const shadertoy = @import("renderer/shadertoy.zig"); pub const Backend = @import("renderer/backend.zig").Backend; pub const GenericRenderer = @import("renderer/generic.zig").Renderer; @@ -145,6 +146,7 @@ test { _ = message; _ = frame_lease; _ = external_frame; + _ = link; _ = shadertoy; _ = size; _ = Thread; diff --git a/src/renderer/State.zig b/src/renderer/State.zig index f1f9c25dc15..ab2fbc78560 100644 --- a/src/renderer/State.zig +++ b/src/renderer/State.zig @@ -114,6 +114,115 @@ pub const Mouse = struct { /// This could really just be mods in general and we probably will /// move it out of mouse state at some point. mods: inputpkg.Mods = .{}, + + // cmux fork: (B) ExternalHover — a lock-protected, mouse-move-driven + // identity distinct from `point`/`mods` above. `point` only tracks + // cells where *native* hover found a link and resets whenever it + // doesn't (see `Surface.cursorPosCallback`); the embedding host needs + // every in-bounds cell regardless of native hover's own outcome. + + /// Updated on every in-bounds mouse event, independent of whether + /// native link hover resolved anything this event. `null` outside the + /// viewport. + pointer_cell: ?terminalpkg.point.Coordinate = null, + + /// (B) flicker fix §3 — renamed from `hover_input_epoch`: role + /// narrowed to normalized-mods and hover-eligibility ABA guarding + /// only. A plain in-bounds cell change no longer bumps this — + /// `ExternalHover.validateOrInvalidate`'s range-containment check + /// decides in-range validity, and `Surface.cursorPosCallback`'s + /// input-time `invalidateIfPointerLeftRanges` call decides range/ + /// viewport exit immediately, rather than waiting for this epoch to + /// go stale on the next render frame (which could miss an + /// A->outside->A sequence coalesced within a single frame). + hover_context_epoch: u64 = 0, + + /// Whether link hover (native or external) is currently permitted. + /// Computed by the input path under this same mutex; the renderer + /// thread never reads `Surface.mouse` directly to derive this. + hover_eligible: bool = true, + + /// The embedding host's resolved hover override, when active. See + /// `renderer/link.zig`'s `ExternalHover` doc for the full contract. + external_hover: renderer.link.ExternalHover = .{}, + + /// cmux fork: (C) ExternalHover diagnostics — the per-surface fixed + /// POD ring bug C's diagnostics drain from. Lives alongside + /// `external_hover` since both are written only while this same + /// mutex is held (`Surface.setExternalLinkHover`'s setter path, + /// `generic.zig`'s render-loop validation, and + /// `updateExternalHoverPointerCell`'s input-time invalidation). + external_hover_diag: renderer.link.ExternalHoverDiagRing = .{}, + + /// The last `(token, active)` pair delivered to the apprt via an + /// `ExternalHoverTransition` snapshot. Compared each render frame + /// against the current `external_hover` state to detect a change + /// worth notifying; read/written only from the render loop under this + /// mutex, never touched by the input path. + external_hover_last_delivered_token: renderer.link.HoverActivationToken = .zero, + external_hover_last_delivered_active: bool = false, + + /// At most one not-yet-delivered transition. Set by the render loop + /// under this mutex; fetched-and-cleared by + /// `Thread.notifyExternalHoverTransition` under a brief, separate + /// acquisition of this same mutex (mirroring the existing + /// `notifySelectionChanged` precedent) — the actual apprt call always + /// happens after that acquisition ends, never while holding it. + external_hover_pending_transition: ?renderer.link.ExternalHoverTransition = null, + + /// (B) wiring review Blocking 5 — the ack reducer's own record of + /// what the apprt has actually confirmed, per final-spec's ack + /// semantics table. Distinct from `external_hover_last_delivered_*` + /// above, which only tracks what this thread last HANDED to the + /// apprt, not what it acked. Read/written only by + /// `Thread.notifyExternalHoverTransition`'s ack reducer, under a + /// brief separate mutex acquisition — never inside the render loop's + /// own critical section. + external_hover_ack_last_published: renderer.link.HoverActivationToken = .zero, + /// An `inactive` transition whose ack came back false/error, staged + /// for exactly one bounded retry. Never an unconditional resend loop: + /// `external_hover_ack_retry_attempted` bounds this to one attempt + /// per token. + external_hover_ack_pending_retry: ?renderer.link.ExternalHoverTransition = null, + /// Whether the current `external_hover_ack_pending_retry` token has + /// already had its one retry attempt. Reset only when a genuinely new + /// transition (not a retry) is fetched. + external_hover_ack_retry_attempted: bool = false, + + /// (B) flicker fix (review-flicker-fix-confirm.md §1) — the pure + /// state transition `Surface.cursorPosCallback` applies to + /// `pointer_cell`/`external_hover` for one cursor position update. + /// Extracted onto `Mouse` itself (a plain struct with no apprt/config + /// dependency) specifically so it's unit-testable without a live + /// `Surface` — there is no lightweight `Surface` test fixture in this + /// codebase, and building one is out of scope for a correctness fix. + /// + /// - `new_pointer_cell`: the real, non-clamped in-viewport cell, or + /// `null` for a viewport-exit event. Callers MUST pass `null` for + /// viewport exit — never `posToViewport`'s result for a negative + /// position, which clamps to `(0, 0)` rather than signaling + /// out-of-bounds. Passing that clamped value through unconditionally + /// was the pre-existing bug review-flicker-fix-confirm.md §1 found: + /// a viewport-exit event would silently look like a legitimate move + /// to cell `(0, 0)`, which could then wrongly re-validate an active + /// override whose ranges happen to include it. + /// + /// Always updates `pointer_cell`, then destructively invalidates + /// `external_hover` immediately if it's active and `new_pointer_cell` + /// is outside its ranges (or `null`) — see + /// `link.ExternalHover.invalidateIfPointerLeftRanges`'s doc for why + /// this can't wait for the next render frame. + /// + /// - Returns whether an active override was just invalidated, so the + /// caller can conditionally mark the hover row dirty and queue a + /// render. + pub fn updateExternalHoverPointerCell( + self: *Mouse, + new_pointer_cell: ?terminalpkg.point.Coordinate, + ) bool { + self.pointer_cell = new_pointer_cell; + return self.external_hover.invalidateIfPointerLeftRanges(new_pointer_cell, &self.external_hover_diag); + } }; /// The pre-edit state. See Surface.preeditCallback for more information. @@ -233,3 +342,84 @@ test "preedit range shifts left at right edge" { try testing.expectEqual(@as(terminalpkg.size.CellCountInt, 9), range.end); try testing.expectEqual(@as(usize, 0), range.cp_offset); } + +// impl-flicker-fix — review-flicker-fix-confirm.md §1 / §5 items 3-4. +// `Mouse` is a plain struct (no apprt/config dependency), so +// `updateExternalHoverPointerCell` is testable directly without a live +// `Surface` — there is no lightweight `Surface` test fixture in this +// codebase to build a "Surface-level" test against otherwise, but this +// exercises the exact same pure state transition `cursorPosCallback` +// calls into, real state transitions end to end (not source-shape +// assertions). + +test "Mouse.updateExternalHoverPointerCell invalidates on gap cells, out-of-range cells, and viewport exit" { + const testing = std.testing; + var mouse: Mouse = .{}; + const token: renderer.link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: renderer.link.PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + + // Two ranges on the same row with a gap between them: [0, 2) and + // [5, 7). A cell in the gap is in-scope (same row) but in neither + // range. + try testing.expectEqual(renderer.link.ExternalHoverDiagReason.none, mouse.external_hover.set(token, physical, 0, .{ .x = 0, .y = 0 }, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + .{ .row = 0, .start_column = 5, .end_column = 7 }, + }, 0)); + mouse.pointer_cell = .{ .x = 0, .y = 0 }; + try testing.expect(mouse.updateExternalHoverPointerCell(.{ .x = 3, .y = 0 })); + try testing.expect(!mouse.external_hover.active()); + + // Re-set, then move to a cell on a DIFFERENT row than any range — + // out of scope entirely, not just a gap. + try testing.expectEqual(renderer.link.ExternalHoverDiagReason.none, mouse.external_hover.set(token, physical, 0, .{ .x = 0, .y = 0 }, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + try testing.expect(mouse.updateExternalHoverPointerCell(.{ .x = 0, .y = 9 })); + try testing.expect(!mouse.external_hover.active()); + + // Re-set, then leave the viewport entirely (`null`). + try testing.expectEqual(renderer.link.ExternalHoverDiagReason.none, mouse.external_hover.set(token, physical, 0, .{ .x = 0, .y = 0 }, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + try testing.expect(mouse.updateExternalHoverPointerCell(null)); + try testing.expect(!mouse.external_hover.active()); + try testing.expect(mouse.pointer_cell == null); + + // review-flicker-fix-confirm.md §1's negative-position `(0, 0)` clamp + // finding: even if a LATER call wrongly passed `(0, 0)` as though it + // were a real in-viewport cell inside what USED to be the active + // override's own ranges, invalidation is one-way (see `ExternalHover`'s + // ABA doc) — there is nothing left to resurrect. This is exactly what + // `Surface.cursorPosCallback`'s `is_out_of_viewport` guard prevents by + // never passing `posToViewport`'s clamped result through as + // `new_pointer_cell` for a negative position in the first place. + try testing.expect(!mouse.updateExternalHoverPointerCell(.{ .x = 0, .y = 0 })); + try testing.expect(!mouse.external_hover.active()); +} + +test "Mouse.updateExternalHoverPointerCell closes the A->outside->A ABA case through input processing alone" { + const testing = std.testing; + var mouse: Mouse = .{}; + const token: renderer.link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: renderer.link.PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell_a: terminalpkg.point.Coordinate = .{ .x = 0, .y = 0 }; + const outside: terminalpkg.point.Coordinate = .{ .x = 9, .y = 9 }; + + try testing.expectEqual(renderer.link.ExternalHoverDiagReason.none, mouse.external_hover.set(token, physical, 0, cell_a, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + mouse.pointer_cell = cell_a; + + // A -> outside -> A, entirely through input processing (this method) + // — no render frame (no call to `validateOrInvalidate`) ever observes + // any of this, the exact coalescing hazard review-flicker-fix-confirm.md + // §1 requires closing at input time instead. + try testing.expect(mouse.updateExternalHoverPointerCell(outside)); + try testing.expect(!mouse.external_hover.active()); + _ = mouse.updateExternalHoverPointerCell(cell_a); + + // The old override must NOT be active again just because the pointer + // coincidentally returned to its original cell — only a fresh `set` + // can reactivate it. + try testing.expect(!mouse.external_hover.active()); +} diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index ad2b10cdcaa..b67c354a68c 100644 --- a/src/renderer/Thread.zig +++ b/src/renderer/Thread.zig @@ -979,6 +979,7 @@ pub fn renderNow(self: *Thread) void { }; self.notifySelectionChanged(); + self.notifyExternalHoverTransition(); self.updateFrame(self.effectiveCursorBlinkVisible()) catch |err| { log.warn("renderNow: error updating frame err={}", .{err}); @@ -1002,6 +1003,7 @@ pub fn renderNowWithPresentation( }; self.notifySelectionChanged(); + self.notifyExternalHoverTransition(); self.updateFrame(self.effectiveCursorBlinkVisible()) catch |err| { log.warn("renderNowWithPresentation: error updating frame err={}", .{err}); @@ -1673,10 +1675,51 @@ fn drawForcedVisibilityRegainFrame(self: *Thread) DrawFrameResult { return self.drawFrame(true); } +fn updateFrameAndNotifyExternalHover( + context: anytype, + cursor_blink_visible: bool, +) !void { + try context.renderer.updateFrame(context.state, cursor_blink_visible); + context.notifyExternalHoverTransition(); +} + fn updateFrame(self: *Thread, cursor_blink_visible: bool) !void { self.instrumentation.emit(.update_frame_begin); defer self.instrumentation.emit(.update_frame_end); - try self.renderer.updateFrame(self.state, cursor_blink_visible); + try updateFrameAndNotifyExternalHover(self, cursor_blink_visible); +} + +const ExternalHoverUpdateOrderProbe = struct { + events: [2]u8 = .{ 0, 0 }, + event_count: usize = 0, + state: *u8, + renderer: Renderer, + + const Renderer = struct { + probe: *ExternalHoverUpdateOrderProbe, + + fn updateFrame(self: *Renderer, state: *u8, _: bool) !void { + _ = state; + self.probe.events[self.probe.event_count] = 1; + self.probe.event_count += 1; + } + }; + + fn notifyExternalHoverTransition(self: *ExternalHoverUpdateOrderProbe) void { + self.events[self.event_count] = 2; + self.event_count += 1; + } +}; + +test "external hover transition delivery follows the same successful render pass" { + var state: u8 = 0; + var probe: ExternalHoverUpdateOrderProbe = undefined; + probe.state = &state; + probe.renderer = .{ .probe = &probe }; + + try updateFrameAndNotifyExternalHover(&probe, false); + try std.testing.expectEqual(@as(usize, 2), probe.event_count); + try std.testing.expectEqual([2]u8{ 1, 2 }, probe.events); } fn setRendererVisible(self: *Thread, visible: bool) void { @@ -2045,6 +2088,7 @@ fn renderCallback( // Selection activity is a lock-free terminal-wide epoch, so hidden // surfaces can keep accessibility state current without rebuilding. t.notifySelectionChanged(); + t.notifyExternalHoverTransition(); // Preserve terminal dirty state while hidden. The visibility regain path // consumes the accumulated row union in one update before presenting. @@ -3018,6 +3062,256 @@ test "visibility regain renders exactly once per wake" { try std.testing.expectEqual(1, deferred_events.count(.draw_frame_end)); } +/// cmux fork: (B) ExternalHover — deliver any transition the render loop +/// produced (see `generic.zig`), or a staged bounded retry, to the apprt. +/// The fetch-and-clear is a brief, separate acquisition of `self.state`'s +/// mutex — never the render's own long critical section — and the actual +/// `performAction` call happens after that acquisition ends, matching +/// the established contract that the apprt is never invoked while this +/// mutex is held. +/// +/// A genuinely new transition always takes priority over — and clears — +/// any staged retry: it already re-derives whatever outcome the retry +/// was chasing, from fresh state, so retrying the stale one afterward +/// would be redundant at best and could reorder acks at worst. +/// +/// The `external_link_hover` host handler below runs on this renderer thread. +/// It must not call `ghostty_surface_free` for the reported surface or block +/// waiting for a free issued elsewhere. Teardown requested in response to this +/// action must be posted to another queue so the handler returns immediately. +fn notifyExternalHoverTransition(self: *Thread) void { + const transition = fetch: { + self.state.lockDemand(global.io()); + defer self.state.unlockDemand(global.io()); + if (self.state.mouse.external_hover_pending_transition) |t| { + self.state.mouse.external_hover_pending_transition = null; + self.state.mouse.external_hover_ack_pending_retry = null; + self.state.mouse.external_hover_ack_retry_attempted = false; + break :fetch t; + } + if (self.state.mouse.external_hover_ack_pending_retry) |t| { + self.state.mouse.external_hover_ack_pending_retry = null; + break :fetch t; + } + return; + }; + + const result = self.surface.rtApp().performAction( + .{ .surface = self.surface.core() }, + .external_link_hover, + .{ .token = transition.token, .active = transition.active }, + ); + self.applyExternalHoverAck(transition, result); +} + +/// (B) wiring review Blocking 5 — final-spec's ack semantics table, +/// consuming `performAction`'s actual result instead of discarding it. +/// `error` and a returned `false` (unhandled) are the same "not accepted" +/// outcome per final-spec. +/// +/// `active(true)` acks are never retried on failure: the next real state +/// change (a new candidate, an invalidation) produces its own fresh +/// transition from current state, and final-spec explicitly calls for no +/// unconditional resend loop. Only a failed `inactive` ack retries — and +/// only once, via `external_hover_ack_retry_attempted` — since a stuck +/// "should be cleared" outcome is the one case final-spec calls out by +/// name as needing a bounded resend ("後続 frame で再送"). +fn applyExternalHoverAck( + self: *Thread, + transition: rendererpkg.link.ExternalHoverTransition, + result: anyerror!bool, +) void { + const accepted = result catch |err| blk: { + log.warn("apprt failed external_link_hover notification err={}", .{err}); + break :blk false; + }; + + // Compute what to do under the lock, but call `wakeup.notify()` only + // after releasing it — matching the established contract that no + // cross-thread call happens while this mutex is held. + const should_wake_for_retry = wake: { + self.state.lockDemand(global.io()); + defer self.state.unlockDemand(global.io()); + const mouse = &self.state.mouse; + const outcome = externalHoverAckReducer(.{ + .last_published = mouse.external_hover_ack_last_published, + .retry_attempted = mouse.external_hover_ack_retry_attempted, + }, transition, accepted); + mouse.external_hover_ack_last_published = outcome.last_published; + mouse.external_hover_ack_retry_attempted = outcome.retry_attempted; + if (outcome.pending_retry) |retry| mouse.external_hover_ack_pending_retry = retry; + break :wake outcome.should_wake; + }; + + if (should_wake_for_retry) { + self.wakeup.notify() catch |err| { + log.warn("failed to wake renderer thread for external hover ack retry err={}", .{err}); + }; + } +} + +/// (B) wiring review Blocking 5 — the pure decision at the heart of +/// `applyExternalHoverAck`, split out so it's unit-testable without a +/// live `Thread`/`Surface`/apprt (this file has no harness for either). +const ExternalHoverAckState = struct { + last_published: rendererpkg.link.HoverActivationToken, + retry_attempted: bool, +}; + +const ExternalHoverAckOutcome = struct { + last_published: rendererpkg.link.HoverActivationToken, + retry_attempted: bool, + pending_retry: ?rendererpkg.link.ExternalHoverTransition, + should_wake: bool, +}; + +fn externalHoverAckReducer( + state: ExternalHoverAckState, + transition: rendererpkg.link.ExternalHoverTransition, + accepted: bool, +) ExternalHoverAckOutcome { + if (transition.active) { + // active(T2): true commits lastPublished unconditionally; false/ + // error leaves it untouched (T2 was never actually published). + // Either way there is nothing to stage for retry — see the doc + // comment on `applyExternalHoverAck`. + return .{ + .last_published = if (accepted) transition.token else state.last_published, + .retry_attempted = state.retry_attempted, + .pending_retry = null, + .should_wake = false, + }; + } + + // inactive(T): true clears lastPublished (only if it was still T — a + // newer active() may have already replaced it); false/error stages + // exactly one bounded retry. + if (accepted) { + return .{ + .last_published = if (state.last_published.eql(transition.token)) + rendererpkg.link.HoverActivationToken.zero + else + state.last_published, + .retry_attempted = state.retry_attempted, + .pending_retry = null, + .should_wake = false, + }; + } + if (state.retry_attempted) { + return .{ + .last_published = state.last_published, + .retry_attempted = state.retry_attempted, + .pending_retry = null, + .should_wake = false, + }; + } + return .{ + .last_published = state.last_published, + .retry_attempted = true, + .pending_retry = transition, + .should_wake = true, + }; +} + +test "externalHoverAckReducer: active mismatch (false/error) leaves lastPublished untouched" { + const link = rendererpkg.link; + const t1: link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const t2: link.HoverActivationToken = .{ .bits = .{ 2, 2, 2, 2 } }; + + const outcome = externalHoverAckReducer( + .{ .last_published = t1, .retry_attempted = false }, + .{ .token = t2, .active = true }, + false, + ); + try std.testing.expect(outcome.last_published.eql(t1)); + try std.testing.expect(!outcome.should_wake); + try std.testing.expect(outcome.pending_retry == null); +} + +test "externalHoverAckReducer: active accepted commits unconditionally, even replacing a different token" { + const link = rendererpkg.link; + const t1: link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const t2: link.HoverActivationToken = .{ .bits = .{ 2, 2, 2, 2 } }; + + const outcome = externalHoverAckReducer( + .{ .last_published = t1, .retry_attempted = false }, + .{ .token = t2, .active = true }, + true, + ); + try std.testing.expect(outcome.last_published.eql(t2)); +} + +test "externalHoverAckReducer: inactive mismatch (true, but not the current lastPublished) leaves it untouched" { + const link = rendererpkg.link; + const t1: link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const t2: link.HoverActivationToken = .{ .bits = .{ 2, 2, 2, 2 } }; + + // lastPublished is T1 (a newer active() already replaced whatever T2 + // was); a delayed inactive(T2) ack succeeding must not touch T1. + const outcome = externalHoverAckReducer( + .{ .last_published = t1, .retry_attempted = false }, + .{ .token = t2, .active = false }, + true, + ); + try std.testing.expect(outcome.last_published.eql(t1)); +} + +test "externalHoverAckReducer: inactive accepted for the matching token clears lastPublished" { + const link = rendererpkg.link; + const t1: link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + + const outcome = externalHoverAckReducer( + .{ .last_published = t1, .retry_attempted = false }, + .{ .token = t1, .active = false }, + true, + ); + try std.testing.expect(outcome.last_published.eql(link.HoverActivationToken.zero)); +} + +test "externalHoverAckReducer: a failed inactive ack stages exactly one retry" { + const link = rendererpkg.link; + const t1: link.HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + + const first = externalHoverAckReducer( + .{ .last_published = t1, .retry_attempted = false }, + .{ .token = t1, .active = false }, + false, + ); + try std.testing.expect(first.should_wake); + try std.testing.expect(first.retry_attempted); + try std.testing.expect(first.pending_retry != null); + + // A second failure for the SAME token (retry_attempted now true, as + // the caller would have persisted from `first`) never re-arms — + // final-spec explicitly rules out an unconditional resend loop. + const second = externalHoverAckReducer( + .{ .last_published = first.last_published, .retry_attempted = first.retry_attempted }, + .{ .token = t1, .active = false }, + false, + ); + try std.testing.expect(!second.should_wake); + try std.testing.expect(second.pending_retry == null); +} + +test "externalHoverAckReducer: a genuinely new active transition for a different token is unaffected by a prior retry_attempted flag" { + // This models the caller-side contract (notifyExternalHoverTransition + // resets retry_attempted=false whenever it fetches a real, non-retry + // transition) rather than the reducer enforcing it itself — the + // reducer has no way to distinguish "new token" from "same token + // retried" on its own, since it only ever sees one transition at a + // time. Confirms the reducer's active(true) path is independent of + // retry_attempted either way. + const link = rendererpkg.link; + const t2: link.HoverActivationToken = .{ .bits = .{ 2, 2, 2, 2 } }; + + const outcome = externalHoverAckReducer( + .{ .last_published = link.HoverActivationToken.zero, .retry_attempted = true }, + .{ .token = t2, .active = true }, + true, + ); + try std.testing.expect(outcome.last_published.eql(t2)); +} + /// Notify the apprt when the active selection changes. The activity epoch is /// atomic, so this path never acquires the terminal mutex. fn notifySelectionChanged(self: *Thread) void { diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index 82791f05d3e..93dc685e9f4 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -44,6 +44,33 @@ const DisplayLink = switch (builtin.os.tag) { }; const log = std.log.scoped(.generic_renderer); +/// Returns the transition snapshot produced by one render pass, if the +/// external-hover state changed since the previous pass. +fn externalHoverTransitionSnapshot( + active: bool, + token: link.HoverActivationToken, + last_active: bool, + last_token: link.HoverActivationToken, +) ?link.ExternalHoverTransition { + if (active == last_active and token.eql(last_token)) return null; + return .{ .token = token, .active = active }; +} +test "external hover inactive transition carries the invalidated token once" { + const token: link.HoverActivationToken = .{ .bits = .{ 1, 2, 3, 4 } }; + const invalidated = link.HoverActivationToken.zero; + + const transition = externalHoverTransitionSnapshot( + false, + invalidated, + true, + token, + ) orelse return error.TestUnexpectedResult; + try std.testing.expect(!transition.active); + try std.testing.expect(transition.token.eql(invalidated)); + try std.testing.expect( + externalHoverTransitionSnapshot(false, invalidated, false, invalidated) == null, + ); +} /// Keeps prepared frame damage retryable until every fallible draw stage has /// completed. A failure forces the next draw through the full redraw path even @@ -1531,7 +1558,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Get our OSC8 links we're hovering if we have a mouse. // This requires terminal state because of URLs. - const links: terminal.RenderState.CellSet = osc8: { + var links: terminal.RenderState.CellSet = osc8: { // If our mouse isn't hovering, we have no links. const vp = state.mouse.point orelse break :osc8 .empty; @@ -1548,10 +1575,194 @@ pub fn Renderer(comptime GraphicsAPI: type) type { }; }; + // cmux fork: (B) ExternalHover priority — OSC8 always wins + // outright. If it's present this frame, any active + // external override is a stale observation (the pointer + // moved onto a real OSC8 link) and must be discarded now, + // so a later coincidental token match can never resurrect + // it (ABA protection — see `link.ExternalHover`'s doc). + // + // Otherwise, re-fingerprint the *same* row scope the + // active token was minted over (never re-derive it from + // the current mouse cell — see `ExternalHover.top_row`/ + // `row_count`) so an in-place text rewrite under a + // stationary pointer invalidates it too, not just + // pointer/mods movement. This read is bounded to + // `link.max_snapshot_rows` physical rows and uses the + // frame's own arena (bulk-freed with the rest of this + // frame, not a per-frame heap allocation). + const external_active = external_active: { + if (links.count() > 0) { + // (C) diagnostics — design v4 §5's `ghostlyValidation` + // verdict `osc8Present`. Must push BEFORE + // `invalidate()` clears `diagnostic_event`/the + // emitted-verdict bookkeeping this reads (design + // v4 §7 guard 2/3). + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .osc8Present); + state.mouse.external_hover.invalidate(); + break :external_active false; + } + if (!state.mouse.external_hover.active()) + break :external_active false; + + // cmux fork: (B) wiring review Blocking 6 — an + // ineligible hover state (selection/drag/mouse-capture + // in progress) destructively invalidates, the same as + // an OSC8 link taking over above. Without this, an + // override minted just before eligibility dropped + // could otherwise keep re-validating (its token/scope + // content may not have changed at all) and stay + // rendered through a selection drag. + if (!state.mouse.hover_eligible) { + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .hoverIneligible); + state.mouse.external_hover.invalidate(); + break :external_active false; + } + + const screens = &state.terminal.screens; + const top_row = state.mouse.external_hover.top_row; + const row_count = state.mouse.external_hover.row_count; + const grid_rows: u32 = @intCast(screens.active.pages.rows); + const grid_cols = screens.active.pages.cols; + if (top_row >= grid_rows or row_count > grid_rows - top_row) { + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .scopeOutOfBounds); + state.mouse.external_hover.invalidate(); + break :external_active false; + } + + const top_left = screens.active.pages.pin(.{ + .viewport = .{ .x = 0, .y = top_row }, + }) orelse { + // No distinct "pin failed" verdict exists (design + // v4 §4's enum) — a pin failure after the bounds + // check above already passed is the same class of + // scope inconsistency `scopeOutOfBounds` reports. + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .scopeOutOfBounds); + state.mouse.external_hover.invalidate(); + break :external_active false; + }; + const bottom_right = screens.active.pages.pin(.{ + .viewport = .{ .x = grid_cols -| 1, .y = top_row + row_count - 1 }, + }) orelse { + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .scopeOutOfBounds); + state.mouse.external_hover.invalidate(); + break :external_active false; + }; + const text = state.terminal.screens.active.selectionString(arena_alloc, .{ + .sel = terminal.Selection.init(top_left, bottom_right, false), + .trim = false, + .unwrap = false, + }) catch |err| { + // A transient allocation failure isn't evidence the + // content changed — don't destructively invalidate + // on it, just skip revalidation this frame. Not a + // terminal outcome, so no diagnostic entry either + // (design v4 §8 explicitly scopes this out). + log.warn("error re-fingerprinting external hover scope err={}", .{err}); + break :external_active state.mouse.external_hover.active(); + }; + + // (B) flicker fix §4 — same viewport identity + // (row-space revision + offset) the setter folded in, + // so a scroll between mint and this frame invalidates + // even when content/scope/screen identity alone + // wouldn't have. + const hover_scrollbar = screens.active.pages.scrollbar(); + const physical = link.buildPhysicalSnapshotToken( + @intFromPtr(state.terminal), + link.externalHoverScreenKeyByte(screens.active_key), + screens.generation(screens.active_key), + top_row, + row_count, + grid_cols, + text, + hover_scrollbar.row_space_revision, + hover_scrollbar.offset, + ) orelse { + state.mouse.external_hover.recordRenderVerdict(&state.mouse.external_hover_diag, .snapshotBuildFailed); + state.mouse.external_hover.invalidate(); + break :external_active false; + }; + // (B) flicker fix §2/§3 — render validity is now + // decided by independently checking live pointer/ + // ranges containment, physical identity, context + // epoch, and eligibility (see + // `ExternalHover.validateOrInvalidate`'s doc) — no + // opaque activation token is reconstructed or compared + // here anymore, since that conflated "which cell" with + // "still the same link" in a way that couldn't + // distinguish moving within the same ranges from a + // real invalidation (the flicker this fix closes). + // + // (C) diagnostics — `validateOrInvalidate` itself + // pushes the `source=render` entry (including + // first-for-activation/suppression bookkeeping) and + // returns the structured verdict; `.valid` is the + // only verdict that keeps the override active. + break :external_active state.mouse.external_hover.validateOrInvalidate( + state.mouse.pointer_cell, + physical, + state.mouse.hover_context_epoch, + state.mouse.hover_eligible, + &state.mouse.external_hover_diag, + ) == .valid; + }; + + if (external_active) { + state.mouse.external_hover.replaceCells( + arena_alloc, + &links, + @intCast(state.terminal.screens.active.pages.rows), + state.terminal.screens.active.pages.cols, + ) catch |err| { + log.warn("error replacing external hover cells err={}", .{err}); + }; + } + + // Deliver any transition (active -> inactive, inactive -> + // active, or one active token -> another) as a plain value + // snapshot the renderer thread picks up after this mutex + // is released — see `Thread.notifyExternalHoverTransition` + // and the doc on `Surface.external_hover_transition`. This + // is the *only* place a transition is ever produced. + if (externalHoverTransitionSnapshot( + external_active, + state.mouse.external_hover.token, + state.mouse.external_hover_last_delivered_active, + state.mouse.external_hover_last_delivered_token, + )) |transition| { + state.mouse.external_hover_last_delivered_active = transition.active; + state.mouse.external_hover_last_delivered_token = transition.token; + state.mouse.external_hover_pending_transition = transition; + + // (C) diagnostics — #8810 426ms-delay investigation + // (diagnostics-only, no behavior change): marks the + // exact moment THIS transition value snapshot was + // created, so the host can compare it against the + // later `stage=callbackEntry` line (Swift's + // `performAction` callback entry, `Sources/ + // GhosttyTerminalView.swift`'s + // `GHOSTTY_ACTION_EXTERNAL_LINK_HOVER` case) to see + // whether the delay is in this render loop (it + // wouldn't be, if this entry's own log timestamp + // lands promptly) or downstream, in the renderer + // thread's wakeup/delivery to the apprt + // (`Thread.notifyExternalHoverTransition`). + state.mouse.external_hover_diag.push(.{ + .event = state.mouse.external_hover.diagnostic_event, + .source = @intFromEnum(link.ExternalHoverDiagSource.render), + .flags = link.external_hover_diag_flag_transition_snapshot, + }); + } + // OSC8 is the canonical link when present. Otherwise copy // regex candidates and stable cell identities while the // terminal lock is held, then run regexes after unlocking. + // An active external override also suppresses regex hover, + // the same way OSC8 does — it's a different, host-owned + // canonical candidate for the same pointer. const regex_hover: ?link.PreparedHover = regex_hover: { + if (external_active) break :regex_hover null; break :regex_hover self.config.links.prepareHover( arena_alloc, state.terminal.screens.active, diff --git a/src/renderer/link.zig b/src/renderer/link.zig index 65eb56a90a1..e75b4bec3fe 100644 --- a/src/renderer/link.zig +++ b/src/renderer/link.zig @@ -1725,3 +1725,1364 @@ test "renderPreparedAlways mods no match" { try testing.expect(!result.contains(.{ .x = 1, .y = 1 })); try testing.expect(!result.contains(.{ .x = 1, .y = 2 })); } + +// cmux fork: (B) ExternalHover — lets the embedding host (which has context +// Ghostty intentionally doesn't, like a working directory and filesystem +// existence) own interactive hover rendering for a resolved link, instead +// of a native regex partial match drawing a competing underline. See +// `ExternalHover` below and its usage in `generic.zig`'s render loop and +// `Surface.setExternalLinkHover`. + +/// Opaque cross-thread/cross-ABI identity for a captured physical-row +/// content snapshot: surface+screen identity, the row scope the snapshot +/// was taken over (fixed at mint time — never re-derived from a "current" +/// mouse cell), and a bounded content fingerprint of exactly that scope. +/// Two tokens compare equal only if all four words match; a real content or +/// scope change is astronomically unlikely to produce a matching token by +/// chance, which is the only property this type needs (it is a fast-path +/// equality gate, not a security boundary). +pub const PhysicalSnapshotToken = extern struct { + bits: [4]u64, + + pub const zero: PhysicalSnapshotToken = .{ .bits = .{ 0, 0, 0, 0 } }; + + pub fn eql(a: PhysicalSnapshotToken, b: PhysicalSnapshotToken) bool { + return std.mem.eql(u64, &a.bits, &b.bits); + } +}; + +/// Maximum physical rows a single `PhysicalSnapshotToken` may fingerprint. +/// The cmux click/hover resolver reads at most 3 rows (previous/clicked/ +/// next); this leaves margin without letting a pathological caller make +/// fingerprinting unbounded. +pub const max_snapshot_rows: usize = 8; + +/// Maximum columns per fingerprinted row. Fingerprinting a row wider than +/// this fails closed (returns `null`) rather than truncating, since a +/// truncated fingerprint could match content it never actually observed. +pub const max_snapshot_row_columns: usize = 512; + +/// Maximum UTF-8 payload accepted by one physical snapshot fingerprint. +/// This is an independent resource bound, not a column-derived estimate: +/// terminal cells may contain arbitrarily many combining code points. +pub const max_snapshot_text_bytes: usize = 64 * 1024; + +/// Builds a `PhysicalSnapshotToken` from a caller-supplied row range and its +/// joined physical-row text (one line per physical row, in the exact form +/// `ghostty_surface_read_text_physical_rows` returns for the same range — +/// see the cmux fork's (A) addition), or `null` if `row_count` or the text +/// length exceed the bounds above. Pure and does not itself allocate, so it +/// never needs a live `Screen` to unit test; the caller (the render loop, +/// re-fingerprinting every frame, and the setter, fingerprinting once at mint +/// time) is responsible for producing that text via a bounded, +/// frame-arena-scoped read — never an unbounded per-frame heap allocation. +/// (B) flicker fix §4 (review-flicker-fix-confirm.md §3) — `row_space_revision` +/// and `viewport_offset` (from `PageList.scrollbar()`) fold into the scope +/// hash so a token minted at one scroll position can never validate at a +/// different one. Neither `ScreenSet.generation` nor `row_space_revision` +/// alone changes on an ordinary scroll (`row_space_revision` only bumps when +/// retained rows' absolute offsets are reassigned, e.g. scrollback trim/resize) +/// — only `viewport_offset` reliably does, so the pair is required together; +/// `viewport_offset` identifies WHICH rows are visible, while the existing +/// content fingerprint identifies WHAT those rows show, and neither +/// substitutes for the other. +pub fn buildPhysicalSnapshotToken( + surface_id: u64, + screen_key_byte: u8, + screen_generation: usize, + top_row: u32, + row_count: u32, + grid_columns: usize, + joined_physical_rows_text: []const u8, + row_space_revision: u64, + viewport_offset: usize, +) ?PhysicalSnapshotToken { + if (row_count == 0 or row_count > max_snapshot_rows) return null; + if (grid_columns == 0 or grid_columns > max_snapshot_row_columns) return null; + if (joined_physical_rows_text.len > max_snapshot_text_bytes) return null; + + var content_hash = std.hash.Wyhash.init(surface_id); + content_hash.update(std.mem.asBytes(&screen_key_byte)); + content_hash.update(std.mem.asBytes(&screen_generation)); + content_hash.update(joined_physical_rows_text); + const content_word = content_hash.final(); + + var scope_hash = std.hash.Wyhash.init(surface_id +% 1); + scope_hash.update(std.mem.asBytes(&screen_key_byte)); + scope_hash.update(std.mem.asBytes(&screen_generation)); + scope_hash.update(std.mem.asBytes(&top_row)); + scope_hash.update(std.mem.asBytes(&row_count)); + scope_hash.update(std.mem.asBytes(&row_space_revision)); + scope_hash.update(std.mem.asBytes(&viewport_offset)); + const scope_word = scope_hash.final(); + + return .{ .bits = .{ content_word, scope_word, top_row, row_count } }; +} + +/// A `PhysicalSnapshotToken` combined with the pointer cell, normalized +/// mods, and hover-input epoch active when the token was minted. This is +/// the unit of identity `ExternalHover.validateOrInvalidate` checks on +/// every render: a mismatch in the underlying content, the row scope, or +/// the pointer context all invalidate it. The setter mints this itself and +/// returns it to the host as an out parameter — the host never +/// reconstructs one from a snapshot token, so it can't accidentally widen +/// what a stale token matches. +pub const HoverActivationToken = extern struct { + bits: [4]u64, + + pub const zero: HoverActivationToken = .{ .bits = .{ 0, 0, 0, 0 } }; + + pub fn eql(a: HoverActivationToken, b: HoverActivationToken) bool { + return std.mem.eql(u64, &a.bits, &b.bits); + } +}; + +/// Combines a physical snapshot token with pointer/mods/epoch context into +/// a `HoverActivationToken`. Each output word is an independent Wyhash of +/// every input (with a distinct seed), so equality reduces to a plain +/// 4-word memcmp without needing to decode or partially compare fields. +pub fn buildHoverActivationToken( + physical: PhysicalSnapshotToken, + pointer_cell: ?point.Coordinate, + mods_bits: u16, + epoch: u64, +) HoverActivationToken { + const cell_x: u32 = if (pointer_cell) |c| c.x else std.math.maxInt(u32); + const cell_y: u32 = if (pointer_cell) |c| c.y else std.math.maxInt(u32); + + var bits: [4]u64 = undefined; + inline for (&bits, 0..) |*out, i| { + var hash = std.hash.Wyhash.init(@as(u64, i) +% 0x9E3779B97F4A7C15); + hash.update(std.mem.asBytes(&physical.bits)); + hash.update(std.mem.asBytes(&cell_x)); + hash.update(std.mem.asBytes(&cell_y)); + hash.update(std.mem.asBytes(&mods_bits)); + hash.update(std.mem.asBytes(&epoch)); + out.* = hash.final(); + } + return .{ .bits = bits }; +} + +/// One half-open viewport row range the host resolved as part of a hover +/// candidate. Half-open: `[start_column, end_column)`. +pub const ExternalHoverCellRange = extern struct { + row: u16, + start_column: u16, + end_column: u16, +}; + +comptime { + std.debug.assert(@sizeOf(ExternalHoverCellRange) == 6); + std.debug.assert(@offsetOf(ExternalHoverCellRange, "row") == 0); + std.debug.assert(@offsetOf(ExternalHoverCellRange, "start_column") == 2); + std.debug.assert(@offsetOf(ExternalHoverCellRange, "end_column") == 4); +} + +/// A host-resolved path can cross at most the visible viewport. Keeping +/// the ranges inline (no allocation) avoids allocator ownership and +/// cross-thread lifetime concerns on the mouse-move hot path. +pub const max_external_hover_ranges: usize = 256; +/// Total cells across all ranges, independent of range count, so a few +/// very wide ranges can't blow past the render-loop's per-frame cell +/// budget the way `max_external_hover_ranges` alone would allow. +pub const max_external_hover_cells: u32 = 4096; + +/// Whether `ranges` contains `cell` — `cell.y` matches some range's `row` +/// (an absolute viewport row, per `ExternalHoverCellRange`'s doc) and +/// `cell.x` falls in that range's half-open `[start_column, end_column)`. +/// Shared by `ExternalHover.set`'s setter-containment guard and +/// `validateOrInvalidate`'s render-time check — the same "is the pointer +/// currently over this candidate" question, asked at two different times +/// (review-flicker-fix-confirm.md §1). +pub fn rangesContainCell(ranges: []const ExternalHoverCellRange, cell: point.Coordinate) bool { + for (ranges) |r| { + if (r.row == cell.y and cell.x >= r.start_column and cell.x < r.end_column) return true; + } + return false; +} + +// cmux fork: (C) ExternalHover diagnostics — bug C (#8810) hover lifecycle +// tracing (design-hover-diagnostics-v4-final.md). POD-only: entries carry +// enum raw values, never strings — string formation happens exclusively on +// the host side, after a destructive drain has released the renderer +// mutex. See `ExternalHoverDiagRing`'s doc for the ring itself. + +/// Debug-only diagnostics gate, but present in ALL build modes (Debug, +/// Release, ReleaseFast) — NOT `builtin.mode`-gated. Dogfood runs a Debug +/// cmux app against a ReleaseFast GhosttyKit, so gating this behind +/// `std.debug.runtime_safety`/`builtin.mode` would silently produce zero +/// diagnostics in exactly the build combination that matters. Read once +/// (lock-free memoized read, benign to race since the computed value is +/// idempotent) from `CMUX_EXTERNAL_HOVER_DIAGNOSTICS=1`, mirroring the +/// host's own gate-once contract (design v4 §6.2). +var external_hover_diag_gate_state: std.atomic.Value(u8) = .init(0); // 0=unread 1=false 2=true + +pub fn externalHoverDiagnosticsEnabled() bool { + const cached = external_hover_diag_gate_state.load(.monotonic); + if (cached != 0) return cached == 2; + const enabled = if (std.c.getenv("CMUX_EXTERNAL_HOVER_DIAGNOSTICS")) |raw| + std.mem.eql(u8, std.mem.span(raw), "1") + else + false; + external_hover_diag_gate_state.store(if (enabled) @as(u8, 2) else 1, .monotonic); + return enabled; +} + +/// `source` field of `ExternalHoverDiagEntry` — which lifecycle stage +/// produced this entry. No `.none`: every entry has exactly one source. +pub const ExternalHoverDiagSource = enum(u8) { + setter = 1, + input = 2, + render = 3, +}; + +/// `reason` field — populated for `source=setter` (setter rejection, plus +/// the post-accept `renderQueueFailed` side-failure) and `source=input` +/// (input-time range/viewport exit). `.none` (raw 0) means "not +/// applicable to this entry", always distinguishable from a real reason +/// (which starts at 1) so a zeroed/never-written slot can never be +/// misread as one. +pub const ExternalHoverDiagReason = enum(u8) { + none = 0, + zeroRowCount = 1, + hoverIneligible = 2, + scopeOutOfBounds = 3, + snapshotBuildFailed = 4, + pointerMissing = 5, + pointerNotInRanges = 6, + rangeCountExceeded = 7, + rangeOutOfScope = 8, + rangeEmptyOrInverted = 9, + cellBudgetExceeded = 10, + viewportExit = 11, + renderQueueFailed = 12, +}; + +/// `verdict` field — populated for `source=render` (per-frame +/// validation). `.none` (raw 0) means "not applicable" (every +/// setter/input entry leaves this at `.none`). +pub const ExternalHoverDiagVerdict = enum(u8) { + none = 0, + valid = 1, + osc8Present = 2, + hoverIneligible = 3, + scopeOutOfBounds = 4, + pointerMissing = 5, + pointerNotInRanges = 6, + viewportExit = 7, + physicalTokenMismatch = 8, + contextEpochMismatch = 9, + snapshotBuildFailed = 10, + renderQueueFailed = 11, +}; + +/// `flags` bit 0: this is the first render-validation entry for the +/// activation `event` currently identifies. The entry itself must carry +/// this — a host that infers "first" from log history gets it wrong +/// across a ring overflow, which can drop the actual first entry. +pub const external_hover_diag_flag_first_for_activation: u8 = 1 << 0; + +/// `flags` bit 1 — diagnostics-only, added for the #8810 investigation +/// into the ~426ms delay between setter acceptance and transition +/// delivery. Set on a `source=render` entry pushed at the EXACT point +/// `generic.zig`'s render loop creates a transition value snapshot +/// (`state.mouse.external_hover_pending_transition = .{...}`) — distinct +/// from the ordinary per-frame validation entry `recordRenderVerdict` +/// already pushes a few lines earlier in the same render-loop pass. +/// Reuses the existing ring/gate/drain path (no new mechanism): this bit +/// is the only way a decoder tells the two entry kinds apart, since both +/// share `source=render` and the same `event`. `verdict`/`reason` are +/// left at `.none` on this entry — it isn't itself a validation +/// judgment, just a timestamp marker for when the snapshot was made. +pub const external_hover_diag_flag_transition_snapshot: u8 = 1 << 1; + +/// One fixed-size diagnostic entry. `extern struct` with explicit field +/// order so the Zig writer and the host's Swift decoder agree on layout +/// without a shared header — `ghostty_external_hover_diag_entry_s` in +/// `include/ghostty.h` mirrors this exactly, field-for-field. +pub const ExternalHoverDiagEntry = extern struct { + event: u64 = 0, + source: u8 = 0, + reason: u8 = 0, + verdict: u8 = 0, + flags: u8 = 0, + seq: u32 = 0, +}; + +comptime { + std.debug.assert(@sizeOf(ExternalHoverDiagEntry) == 16); + std.debug.assert(@alignOf(ExternalHoverDiagEntry) == 8); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "event") == 0); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "source") == 8); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "reason") == 9); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "verdict") == 10); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "flags") == 11); + std.debug.assert(@offsetOf(ExternalHoverDiagEntry, "seq") == 12); +} + +/// Fixed 64-entry POD ring buffer, one per surface (see +/// `renderer/State.zig`'s `Mouse.external_hover_diag`). `push` is the only +/// hot-path entry point: called only while the caller already holds +/// `renderer_state.mutex` (`Surface.zig`'s setter and `generic.zig`'s +/// render loop), does no allocation, and never fails — on overflow it +/// silently discards the oldest entry and bumps `dropped_count`. +pub const ExternalHoverDiagRing = struct { + pub const capacity: usize = 64; + + entries: [capacity]ExternalHoverDiagEntry = [_]ExternalHoverDiagEntry{.{}} ** capacity, + /// Index of the OLDEST live entry. + head: u32 = 0, + /// Number of live entries, `0...capacity`. + len: u32 = 0, + /// Monotonic cumulative count of entries ever discarded by overflow. + /// Never reset, never wrapped in practice (a u64 would take centuries + /// of 64-entry overflows at any plausible hover rate); the host keeps + /// its own previous value per surface and reports only the delta + /// (design v4 §3.3) since the same cumulative value must never be + /// double-reported across drains. + dropped_count: u64 = 0, + /// Monotonic per-push sequence number, independent of `dropped_count` + /// — lets the host detect gaps/reordering even within one drain. + next_seq: u32 = 0, + + /// Appends `entry` (with `seq` overwritten by the ring's own + /// counter). Caller must already hold the renderer mutex. A no-op if + /// the diagnostics gate is off — this is the single choke point every + /// diagnostic append goes through, so "gate off means no ring + /// append" (design v4 §7 guard 4) holds regardless of call site. + pub fn push(self: *ExternalHoverDiagRing, entry: ExternalHoverDiagEntry) void { + if (!externalHoverDiagnosticsEnabled()) return; + self.pushUnchecked(entry); + } + + /// The gate-free append logic `push` delegates to. Exposed + /// separately so ring-behavior unit tests (FIFO/wrap/overflow) can + /// exercise it independent of the process-memoized diagnostics gate + /// (`externalHoverDiagnosticsEnabled`'s cached value can't be reset + /// mid test-binary once another test has resolved it) — every real + /// production call site goes through `push`, never this directly. + pub fn pushUnchecked(self: *ExternalHoverDiagRing, entry: ExternalHoverDiagEntry) void { + var e = entry; + e.seq = self.next_seq; + self.next_seq +%= 1; + if (self.len == capacity) { + // Overflow: drop the oldest entry, which is exactly the slot + // we're about to overwrite. + self.head = (self.head + 1) % @as(u32, capacity); + // review non-blocking N1 — saturating, not wrapping: this + // field's own doc above promises "monotonic cumulative", and + // a `+%=` wrap back to 0 would violate that (and would read + // to the host as "nothing has ever been dropped" right after + // the wrap, the opposite of what actually happened). + self.dropped_count +|= 1; + } else { + self.len += 1; + } + const write_index = (self.head + self.len - 1) % @as(u32, capacity); + self.entries[write_index] = e; + } + + /// Destructively drains up to `out.len` of the oldest live entries + /// into `out`, advancing `head` and decrementing `len` by exactly the + /// number copied. Entries beyond `out.len` are left in the ring (NOT + /// discarded) — the caller can call again to continue draining. Only + /// ever call while holding the renderer mutex; unlocking, enum + /// decoding, string formation, and logging must all happen strictly + /// after this returns (design v4 §3.3). + pub fn drain(self: *ExternalHoverDiagRing, out: []ExternalHoverDiagEntry) usize { + const n: u32 = @intCast(@min(out.len, self.len)); + var i: u32 = 0; + while (i < n) : (i += 1) { + out[i] = self.entries[(self.head + i) % @as(u32, capacity)]; + } + self.head = (self.head + n) % @as(u32, capacity); + self.len -= n; + return n; + } +}; + +test "ExternalHoverDiagEntry is a 16-byte, 8-byte-aligned POD" { + try std.testing.expectEqual(@as(usize, 16), @sizeOf(ExternalHoverDiagEntry)); + try std.testing.expectEqual(@as(usize, 8), @alignOf(ExternalHoverDiagEntry)); +} + +// Raw discriminant values cross the C ABI (`include/ghostty.h`'s +// `ghostty_external_hover_diag_entry_s`'s `source`/`reason`/`verdict` +// bytes) and are decoded by the host's own copy of these enums — +// reordering a variant would silently reinterpret every already-shipped +// entry as a different meaning. Pin them. +test "ExternalHoverDiagSource/Reason/Verdict raw values are pinned (host ABI stability)" { + const testing = std.testing; + try testing.expectEqual(@as(u8, 1), @intFromEnum(ExternalHoverDiagSource.setter)); + try testing.expectEqual(@as(u8, 2), @intFromEnum(ExternalHoverDiagSource.input)); + try testing.expectEqual(@as(u8, 3), @intFromEnum(ExternalHoverDiagSource.render)); + + try testing.expectEqual(@as(u8, 0), @intFromEnum(ExternalHoverDiagReason.none)); + try testing.expectEqual(@as(u8, 1), @intFromEnum(ExternalHoverDiagReason.zeroRowCount)); + try testing.expectEqual(@as(u8, 2), @intFromEnum(ExternalHoverDiagReason.hoverIneligible)); + try testing.expectEqual(@as(u8, 3), @intFromEnum(ExternalHoverDiagReason.scopeOutOfBounds)); + try testing.expectEqual(@as(u8, 4), @intFromEnum(ExternalHoverDiagReason.snapshotBuildFailed)); + try testing.expectEqual(@as(u8, 5), @intFromEnum(ExternalHoverDiagReason.pointerMissing)); + try testing.expectEqual(@as(u8, 6), @intFromEnum(ExternalHoverDiagReason.pointerNotInRanges)); + try testing.expectEqual(@as(u8, 7), @intFromEnum(ExternalHoverDiagReason.rangeCountExceeded)); + try testing.expectEqual(@as(u8, 8), @intFromEnum(ExternalHoverDiagReason.rangeOutOfScope)); + try testing.expectEqual(@as(u8, 9), @intFromEnum(ExternalHoverDiagReason.rangeEmptyOrInverted)); + try testing.expectEqual(@as(u8, 10), @intFromEnum(ExternalHoverDiagReason.cellBudgetExceeded)); + try testing.expectEqual(@as(u8, 11), @intFromEnum(ExternalHoverDiagReason.viewportExit)); + try testing.expectEqual(@as(u8, 12), @intFromEnum(ExternalHoverDiagReason.renderQueueFailed)); + + try testing.expectEqual(@as(u8, 0), @intFromEnum(ExternalHoverDiagVerdict.none)); + try testing.expectEqual(@as(u8, 1), @intFromEnum(ExternalHoverDiagVerdict.valid)); + try testing.expectEqual(@as(u8, 2), @intFromEnum(ExternalHoverDiagVerdict.osc8Present)); + try testing.expectEqual(@as(u8, 3), @intFromEnum(ExternalHoverDiagVerdict.hoverIneligible)); + try testing.expectEqual(@as(u8, 4), @intFromEnum(ExternalHoverDiagVerdict.scopeOutOfBounds)); + try testing.expectEqual(@as(u8, 5), @intFromEnum(ExternalHoverDiagVerdict.pointerMissing)); + try testing.expectEqual(@as(u8, 6), @intFromEnum(ExternalHoverDiagVerdict.pointerNotInRanges)); + try testing.expectEqual(@as(u8, 7), @intFromEnum(ExternalHoverDiagVerdict.viewportExit)); + try testing.expectEqual(@as(u8, 8), @intFromEnum(ExternalHoverDiagVerdict.physicalTokenMismatch)); + try testing.expectEqual(@as(u8, 9), @intFromEnum(ExternalHoverDiagVerdict.contextEpochMismatch)); + try testing.expectEqual(@as(u8, 10), @intFromEnum(ExternalHoverDiagVerdict.snapshotBuildFailed)); + try testing.expectEqual(@as(u8, 11), @intFromEnum(ExternalHoverDiagVerdict.renderQueueFailed)); +} + +test "ExternalHoverDiagRing.pushUnchecked appends in FIFO order; drain returns oldest first" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{}; + + ring.pushUnchecked(.{ .event = 1 }); + ring.pushUnchecked(.{ .event = 2 }); + ring.pushUnchecked(.{ .event = 3 }); + try testing.expectEqual(@as(u32, 3), ring.len); + + var out: [2]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(@as(usize, 2), ring.drain(&out)); + try testing.expectEqual(@as(u64, 1), out[0].event); + try testing.expectEqual(@as(u64, 2), out[1].event); + try testing.expectEqual(@as(u32, 1), ring.len); + + // The remainder (event 3) is still there for a follow-up drain. + try testing.expectEqual(@as(usize, 1), ring.drain(&out)); + try testing.expectEqual(@as(u64, 3), out[0].event); + try testing.expectEqual(@as(u32, 0), ring.len); +} + +test "ExternalHoverDiagRing.pushUnchecked assigns a monotonic per-entry seq" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{}; + ring.pushUnchecked(.{ .event = 10 }); + ring.pushUnchecked(.{ .event = 20 }); + + var out: [2]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(@as(usize, 2), ring.drain(&out)); + try testing.expectEqual(@as(u32, 0), out[0].seq); + try testing.expectEqual(@as(u32, 1), out[1].seq); +} + +test "ExternalHoverDiagRing.pushUnchecked wraps head/write-index around capacity" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{}; + + // Fill, drain most of it, then push more — this exercises a + // write-index/head that has wrapped past the physical array's end, + // not just a ring that has never wrapped. + for (0..ExternalHoverDiagRing.capacity) |i| { + ring.pushUnchecked(.{ .event = @intCast(i) }); + } + var out: [ExternalHoverDiagRing.capacity - 4]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(out.len, ring.drain(&out)); + try testing.expectEqual(@as(u32, 4), ring.len); + + // head now sits at index (capacity - 4) mod capacity; the next + // several pushes wrap the physical write index around the array. + for (0..10) |i| { + ring.pushUnchecked(.{ .event = 1000 + @as(u64, i) }); + } + try testing.expectEqual(@as(u32, 14), ring.len); + + var out2: [14]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(out2.len, ring.drain(&out2)); + // The 4 originally-remaining entries (events capacity-4..capacity-1) + // must still come out FIRST, in order, ahead of the 10 new ones. + for (0..4) |i| { + try testing.expectEqual(@as(u64, ExternalHoverDiagRing.capacity - 4 + i), out2[i].event); + } + for (0..10) |i| { + try testing.expectEqual(@as(u64, 1000 + i), out2[4 + i].event); + } +} + +test "ExternalHoverDiagRing.pushUnchecked overflow drops the oldest entry and bumps dropped_count once per drop" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{}; + + for (0..ExternalHoverDiagRing.capacity) |i| { + ring.pushUnchecked(.{ .event = @intCast(i) }); + } + try testing.expectEqual(@as(u64, 0), ring.dropped_count); + try testing.expectEqual(@as(u32, ExternalHoverDiagRing.capacity), ring.len); + + // One more push over a full ring: oldest (event 0) is dropped, len + // stays saturated at capacity, dropped_count bumps by exactly 1. + ring.pushUnchecked(.{ .event = 9999 }); + try testing.expectEqual(@as(u64, 1), ring.dropped_count); + try testing.expectEqual(@as(u32, ExternalHoverDiagRing.capacity), ring.len); + + var out: [ExternalHoverDiagRing.capacity]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(out.len, ring.drain(&out)); + // event 0 is gone; event 1 is now the oldest survivor, and the new + // push (9999) is the newest entry. + try testing.expectEqual(@as(u64, 1), out[0].event); + try testing.expectEqual(@as(u64, 9999), out[out.len - 1].event); + + // Overflowing a second time bumps dropped_count again — the host + // computes its own delta across drains, but the ring's own + // cumulative counter itself must never reset or double-count a + // single drop. + for (0..ExternalHoverDiagRing.capacity) |i| { + ring.pushUnchecked(.{ .event = @intCast(i) }); + } + ring.pushUnchecked(.{ .event = 8888 }); + try testing.expectEqual(@as(u64, 2), ring.dropped_count); +} + +test "ExternalHoverDiagRing.dropped_count saturates instead of wrapping at u64 max" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{ .dropped_count = std.math.maxInt(u64) }; + + for (0..ExternalHoverDiagRing.capacity) |i| { + ring.pushUnchecked(.{ .event = @intCast(i) }); + } + // One more push over a full ring, with `dropped_count` already + // pinned at the max: a wrapping add would silently roll this back + // to 0, which the host would read as "nothing has ever been + // dropped" — the exact opposite of what happened. A saturating add + // stays pinned at the max instead. + ring.pushUnchecked(.{ .event = 9999 }); + try testing.expectEqual(@as(u64, std.math.maxInt(u64)), ring.dropped_count); +} + +// #8810 426ms-delay investigation: the transition-snapshot flag must be +// independently readable from the pre-existing first-for-activation flag +// (both are bits of the same `flags` byte) so a decoder can tell a +// transition-snapshot entry apart from an ordinary render-verdict entry +// for the SAME activation without relying on push order/seq alone. +test "external_hover_diag_flag_transition_snapshot is distinct from and composable with first_for_activation" { + const testing = std.testing; + try testing.expectEqual(@as(u8, 2), external_hover_diag_flag_transition_snapshot); + try testing.expect(external_hover_diag_flag_transition_snapshot != external_hover_diag_flag_first_for_activation); + + var ring: ExternalHoverDiagRing = .{}; + ring.pushUnchecked(.{ + .event = 42, + .source = @intFromEnum(ExternalHoverDiagSource.render), + .flags = external_hover_diag_flag_first_for_activation | external_hover_diag_flag_transition_snapshot, + }); + var out: [1]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(@as(usize, 1), ring.drain(&out)); + try testing.expect(out[0].flags & external_hover_diag_flag_first_for_activation != 0); + try testing.expect(out[0].flags & external_hover_diag_flag_transition_snapshot != 0); +} + +test "ExternalHoverDiagRing.drain never copies more than out.len and leaves the remainder in place" { + const testing = std.testing; + var ring: ExternalHoverDiagRing = .{}; + ring.pushUnchecked(.{ .event = 1 }); + ring.pushUnchecked(.{ .event = 2 }); + ring.pushUnchecked(.{ .event = 3 }); + + var out: [0]ExternalHoverDiagEntry = undefined; + try testing.expectEqual(@as(usize, 0), ring.drain(&out)); + try testing.expectEqual(@as(u32, 3), ring.len); +} + +// Design v4 §7 guard 4: when the diagnostics gate is off, `push` must +// not append to the ring at all. `externalHoverDiagnosticsEnabled`'s +// result is memoized process-wide on first read, so this test can't +// force the gate on/off mid test-binary — it instead pins the +// observable contract at the level every real caller actually uses +// (`push`, not `pushUnchecked`): in this test binary's environment +// (`CMUX_EXTERNAL_HOVER_DIAGNOSTICS` unset), `push` is a no-op. +test "ExternalHoverDiagRing.push is a no-op when the diagnostics gate is off" { + const testing = std.testing; + try testing.expect(!externalHoverDiagnosticsEnabled()); + var ring: ExternalHoverDiagRing = .{}; + ring.push(.{ .event = 42 }); + try testing.expectEqual(@as(u32, 0), ring.len); + try testing.expectEqual(@as(u64, 0), ring.dropped_count); +} + +/// Host-resolved link-hover override. When active, it owns interactive +/// hover rendering in place of Ghostty's own regex/OSC8 hover for the same +/// pointer — see `generic.zig`'s render-loop priority. +/// +/// Invalidation is destructive and one-way: once `validateOrInvalidate` +/// (or the input-time `invalidateIfPointerLeftRanges`) observes an +/// invalidating condition, the state is discarded immediately, so a later +/// coincidental match of the *same* stale identity can never resurrect it +/// (ABA protection). The only way back to `active() == true` is a fresh +/// `set` call with a fresh token. +/// +/// (B) flicker fix §3 (review-flicker-fix-confirm.md §2's blocking +/// finding) — `token`, `physical`, and `context_epoch` are deliberately +/// separate fields, not one opaque hash: `token` is the host-visible +/// clear/transition/ack identity ONLY (still minted from physical/ +/// pointer/mods/epoch, so it's unique per activation, but never itself +/// compared for render validity); `physical` and `context_epoch` are what +/// `validateOrInvalidate` actually checks, independently, alongside live +/// range containment. An earlier revision folded pointer cell into the +/// same opaque token render validity was decided from, which made "ignore +/// cell movement within the same ranges, but still catch mods/eligibility +/// ABA" impossible to express — a real dogfood regression (indicator +/// flicker while moving along a stable, still-valid link) traced to +/// exactly that conflation. +pub const ExternalHover = struct { + token: HoverActivationToken = HoverActivationToken.zero, + /// Fixed scope/content/viewport identity captured at `set` time — + /// compared against a fresh re-fingerprint every render frame + /// (`validateOrInvalidate`), never re-derived from "the current + /// mouse cell". Catches an in-place text rewrite or scroll under a + /// stationary pointer, not just pointer/mods movement. + physical: PhysicalSnapshotToken = PhysicalSnapshotToken.zero, + /// Monotonic ABA guard for normalized mods and hover eligibility + /// ONLY — see `hover_context_epoch`'s doc in `renderer/State.zig`. A + /// plain in-bounds pointer/cell change never bumps the epoch that + /// mints this, so moving between two cells inside the same `ranges` + /// never invalidates on epoch grounds; `validateOrInvalidate`'s range + /// check is what actually gates pointer/cell validity. + context_epoch: u64 = 0, + /// The physical row scope `token`/`physical` were minted over. The + /// render loop re-reads exactly this scope every frame (never + /// re-deriving it from the current mouse cell) to rebuild a fresh + /// `PhysicalSnapshotToken` for `validateOrInvalidate`. + top_row: u32 = 0, + row_count: u32 = 0, + ranges: [max_external_hover_ranges]ExternalHoverCellRange = undefined, + len: u16 = 0, + + // cmux fork: (C) ExternalHover diagnostics — activation-scoped + // bookkeeping, reset by `set`/`invalidate`, never touched by + // `replaceCells`/`active`. `diagnostic_event` is the host's + // `host_event_id` for the setter call that created this activation + // (design v4 §1's correlation key's `event` half — `surfaceSerial` is + // a host-only addition, never stored here). Left at 0 whenever the + // diagnostics gate is off, so a gate-off activation never leaks an + // event id even if the gate flips on mid-activation. + diagnostic_event: u64 = 0, + /// Whether `recordRenderVerdict` has already emitted a render-verdict + /// entry for this activation — the first one always fires regardless + /// of verdict; only the 2nd-and-later repeat of the SAME verdict is + /// suppressed (design v4 §4). + diag_emitted_first_verdict: bool = false, + /// Raw `ExternalHoverDiagVerdict` of the last verdict actually + /// emitted (or `.none` before any has been). Compared, not + /// re-derived, so a verdict change (even between two "invalid" + /// reasons) always emits. + diag_last_verdict: u8 = @intFromEnum(ExternalHoverDiagVerdict.none), + + pub fn active(self: *const ExternalHover) bool { + return self.len > 0; + } + + /// Pushes a `source=render` diagnostic entry for `verdict` unless + /// this activation already emitted this exact verdict (2nd-and-later + /// frame suppression — design v4 §4). A no-op if no activation is + /// active (`len == 0`) — there is nothing to attribute a render + /// verdict to. Must be called BEFORE any subsequent `invalidate()`, + /// since `invalidate()` zeroes `diagnostic_event`/the emitted-verdict + /// bookkeeping this reads (design v4 §7 guard 2/3: determine the + /// structured verdict before mutating state, never re-infer after). + pub fn recordRenderVerdict( + self: *ExternalHover, + ring: *ExternalHoverDiagRing, + verdict: ExternalHoverDiagVerdict, + ) void { + if (!self.active()) return; + const first = !self.diag_emitted_first_verdict; + const verdict_raw = @intFromEnum(verdict); + const suppress = !first and self.diag_last_verdict == verdict_raw; + if (!suppress) { + ring.push(.{ + .event = self.diagnostic_event, + .source = @intFromEnum(ExternalHoverDiagSource.render), + .verdict = verdict_raw, + .flags = if (first) external_hover_diag_flag_first_for_activation else 0, + }); + } + self.diag_emitted_first_verdict = true; + self.diag_last_verdict = verdict_raw; + } + + /// Returns whether `self` is still valid, independently checking + /// (review §2's required split, all four independent — see the type + /// doc): + /// + /// 1. `current_pointer` is non-null and inside `self.ranges`. + /// 2. `current_physical` matches `self.physical`. + /// 3. `current_context_epoch` matches `self.context_epoch`. + /// 4. `hover_eligible == true`. + /// + /// (OSC8 priority — review's independent condition 5 — is the + /// existing, unchanged destructive `invalidate()` call the render + /// loop already makes BEFORE ever reaching this check when an OSC8 + /// link is present this frame; it is not re-checked here.) + /// + /// Any failure destructively invalidates before returning — see the + /// type doc for why this must be one-way. + /// + /// (C) diagnostics — returns the single structured + /// `ExternalHoverDiagVerdict` this call determined (`.none` if there + /// was no activation to validate), reused for BOTH the production + /// accept/reject decision and the diagnostic entry `recordVerdict` + /// pushes — never re-derived after the fact (design v4 §7 guards + /// 1/2). The check order below (pointer-null, then eligibility, + /// ranges, physical, epoch) is behavior-identical to the prior + /// combined OR-check: accept/reject depends only on whether ANY + /// check fails, never on which one is checked first — the order only + /// picks which single reason is reported when more than one would + /// fail simultaneously. + pub fn validateOrInvalidate( + self: *ExternalHover, + current_pointer: ?point.Coordinate, + current_physical: PhysicalSnapshotToken, + current_context_epoch: u64, + hover_eligible: bool, + diag: *ExternalHoverDiagRing, + ) ExternalHoverDiagVerdict { + if (self.len == 0) return .none; + const verdict: ExternalHoverDiagVerdict = verdict: { + const cell = current_pointer orelse break :verdict .pointerMissing; + if (!hover_eligible) break :verdict .hoverIneligible; + if (!rangesContainCell(self.ranges[0..self.len], cell)) break :verdict .pointerNotInRanges; + if (!self.physical.eql(current_physical)) break :verdict .physicalTokenMismatch; + if (self.context_epoch != current_context_epoch) break :verdict .contextEpochMismatch; + break :verdict .valid; + }; + self.recordRenderVerdict(diag, verdict); + if (verdict != .valid) self.invalidate(); + return verdict; + } + + /// (B) flicker fix §1 — the input-time counterpart to + /// `validateOrInvalidate`'s render-time check. Destructively + /// invalidates immediately if active and `new_pointer_cell` is + /// outside `self.ranges` (or `null`, i.e. viewport exit), rather than + /// waiting for the next render frame — mouse events and render frames + /// aren't 1:1, so a render-time-only check can miss an + /// A->outside->A sequence coalesced within a single frame (the exact + /// ABA case `validateOrInvalidate`'s token-mismatch check already + /// closes for content/scope changes, now closed for pointer movement + /// too). Never itself checks physical/context/eligibility — those + /// stay `validateOrInvalidate`'s job; this is pointer/ranges only. + /// + /// - Returns `true` if this call actually invalidated (so the caller + /// can conditionally mark the hover row dirty and queue a render — + /// see `Surface.cursorPosCallback`); `false` if already inactive or + /// still valid (still active and either the pointer stayed inside + /// `self.ranges`, or the caller is between-events with no pointer + /// change at all). + /// + /// (C) diagnostics — design v4 §7 guard 3: pushes `source=input` + /// (`reason=viewportExit` for a `null` cell, `pointerNotInRanges` + /// otherwise) BEFORE `invalidate()` clears the state this needs + /// (`diagnostic_event`), never after. + pub fn invalidateIfPointerLeftRanges( + self: *ExternalHover, + new_pointer_cell: ?point.Coordinate, + diag: *ExternalHoverDiagRing, + ) bool { + if (!self.active()) return false; + if (new_pointer_cell) |cell| { + if (rangesContainCell(self.ranges[0..self.len], cell)) return false; + } + const reason: ExternalHoverDiagReason = if (new_pointer_cell == null) + .viewportExit + else + .pointerNotInRanges; + diag.push(.{ + .event = self.diagnostic_event, + .source = @intFromEnum(ExternalHoverDiagSource.input), + .reason = @intFromEnum(reason), + }); + self.invalidate(); + return true; + } + + /// Unconditionally discards state, regardless of token. Used when the + /// core itself observes a competing signal (an OSC8 link present this + /// frame) that must never coexist with a possibly-stale override. + pub fn invalidate(self: *ExternalHover) void { + self.token = HoverActivationToken.zero; + self.physical = PhysicalSnapshotToken.zero; + self.context_epoch = 0; + self.top_row = 0; + self.row_count = 0; + self.len = 0; + self.diagnostic_event = 0; + self.diag_emitted_first_verdict = false; + self.diag_last_verdict = @intFromEnum(ExternalHoverDiagVerdict.none); + } + + /// Validates and stores `ranges` under `token`/`physical`/ + /// `context_epoch`/`top_row`/`row_count`. Rejects (returns `false`, + /// state unchanged) if: + /// - `pointer_cell` is `null` or outside `ranges` — (B) flicker fix + /// §1's setter-containment guard: the current pointer can have + /// moved between the host's currentness check and this call, so + /// the setter itself must re-verify, not just trust the caller. + /// - the range count or total cell count exceeds the bounds above. + /// - any range is empty/inverted, or falls outside `[top_row, top_row + /// + row_count)`. + /// + /// Ranges are assumed ordered and non-overlapping by the caller; this + /// does not itself check for overlap (the render-time defensive + /// bounds check in `replaceCells` only needs per-range validity, not + /// global non-overlap, to stay safe). + /// + /// (C) diagnostics — returns the single structured + /// `ExternalHoverDiagReason` (`.none` on success), reused for BOTH + /// the production accept/reject decision and the diagnostic entry the + /// caller (`Surface.setExternalLinkHover`) pushes on rejection — see + /// design v4 §7 guard 1. `event` is `host_event_id` from the C ABI + /// (design v4 §2); stored into `diagnostic_event` only when the + /// diagnostics gate is on (received but not stored when off), and the + /// activation's verdict-suppression bookkeeping is reset regardless + /// (design v4 §4's "setter accepted 時に diagnosticEvent と + /// lastVerdict を同時に設定"). + pub fn set( + self: *ExternalHover, + token: HoverActivationToken, + physical: PhysicalSnapshotToken, + context_epoch: u64, + pointer_cell: ?point.Coordinate, + top_row: u32, + row_count: u32, + ranges: []const ExternalHoverCellRange, + event: u64, + ) ExternalHoverDiagReason { + if (row_count == 0) return .zeroRowCount; + if (ranges.len > max_external_hover_ranges) return .rangeCountExceeded; + const cell = pointer_cell orelse return .pointerMissing; + if (!rangesContainCell(ranges, cell)) return .pointerNotInRanges; + var total: u32 = 0; + for (ranges) |r| { + // r.row is an absolute viewport row (see replaceCells, which + // draws it directly with no top_row offset) — reject any range + // that doesn't actually fall within the scope this call is + // claiming without allowing `top_row + row_count` to overflow. + if (r.row < top_row) return .rangeOutOfScope; + if (@as(u32, r.row) - top_row >= row_count) return .rangeOutOfScope; + if (r.start_column >= r.end_column) return .rangeEmptyOrInverted; + const width = @as(u32, r.end_column) - r.start_column; + if (width > max_external_hover_cells - total) return .cellBudgetExceeded; + total += width; + } + self.token = token; + self.physical = physical; + self.context_epoch = context_epoch; + self.top_row = top_row; + self.row_count = row_count; + self.len = @intCast(ranges.len); + @memcpy(self.ranges[0..ranges.len], ranges); + self.diagnostic_event = if (externalHoverDiagnosticsEnabled()) event else 0; + self.diag_emitted_first_verdict = false; + self.diag_last_verdict = @intFromEnum(ExternalHoverDiagVerdict.none); + return .none; + } + + /// Replaces `result`'s contents with this override's cells, re-checking + /// each range against the *current* grid bounds (`rows`/`cols`) even + /// though `set` already validated shape — a resize between `set` and + /// this render could otherwise let a stale range read past the grid. + /// A no-op (result cleared, nothing added) when inactive. + /// + /// CodeRabbit round-2 item 8 is intentionally waived: cmux supplies + /// exact half-open ranges for non-ASCII cells, so extending a host range + /// to a spacer column here could underline a cell the host did not own. + pub fn replaceCells( + self: *const ExternalHover, + alloc: Allocator, + result: *terminal.RenderState.CellSet, + rows: terminal.size.CellCountInt, + cols: terminal.size.CellCountInt, + ) Allocator.Error!void { + result.clearRetainingCapacity(); + if (!self.active()) return; + for (self.ranges[0..self.len]) |range| { + if (range.row >= rows) continue; + if (range.end_column > cols) continue; + if (range.start_column >= range.end_column) continue; + for (range.start_column..range.end_column) |column| { + try result.put(alloc, .{ .x = @intCast(column), .y = range.row }, {}); + } + } + } +}; + +/// The value snapshot a render pass hands off to the renderer thread for +/// out-of-mutex apprt delivery — see `generic.zig`'s render loop and +/// `Thread.notifyExternalHoverTransition` (mirrors the existing +/// `notifySelectionChanged` precedent: mutex-protected code only ever +/// writes a plain value here, never calls into the apprt itself). +pub const ExternalHoverTransition = struct { + token: HoverActivationToken, + active: bool, +}; + +/// A single-byte discriminant for the active screen (primary/alternate), +/// used consistently by both `Surface.setExternalLinkHover` and +/// `generic.zig`'s per-frame re-fingerprint so the two sides of a +/// `PhysicalSnapshotToken` comparison always agree on this bit. +pub fn externalHoverScreenKeyByte(key: terminal.ScreenSet.Key) u8 { + return switch (key) { + .primary => 0, + .alternate => 1, + }; +} + +test "physical snapshot token differs on content, scope, screen identity, or viewport" { + const testing = std.testing; + const base = buildPhysicalSnapshotToken(1, 0, 0, 5, 1, 80, "abc", 0, 0).?; + + try testing.expect(!base.eql(buildPhysicalSnapshotToken(1, 0, 0, 5, 1, 80, "abd", 0, 0).?)); + try testing.expect(!base.eql(buildPhysicalSnapshotToken(1, 0, 0, 6, 1, 80, "abc", 0, 0).?)); + try testing.expect(!base.eql(buildPhysicalSnapshotToken(1, 1, 0, 5, 1, 80, "abc", 0, 0).?)); + try testing.expect(!base.eql(buildPhysicalSnapshotToken(2, 0, 0, 5, 1, 80, "abc", 0, 0).?)); + // (B) flicker fix §4 — same content/scope/screen identity, different + // viewport identity (row-space revision, offset) must still differ. + try testing.expect(!base.eql(buildPhysicalSnapshotToken(1, 0, 0, 5, 1, 80, "abc", 1, 0).?)); + try testing.expect(!base.eql(buildPhysicalSnapshotToken(1, 0, 0, 5, 1, 80, "abc", 0, 1).?)); + try testing.expect(base.eql(buildPhysicalSnapshotToken(1, 0, 0, 5, 1, 80, "abc", 0, 0).?)); +} + +test "physical snapshot token enforces grid columns and explicit text byte bounds" { + const old_column_derived_bound = max_snapshot_rows * max_snapshot_row_columns; + const ascii = [_]u8{'a'} ** (old_column_derived_bound - 1); + try std.testing.expect( + buildPhysicalSnapshotToken(1, 0, 0, 0, 1, 80, &ascii, 0, 0) != null, + ); + + const multibyte = [_]u8{ 0xC3, 0xA9 } ** ((old_column_derived_bound / 2) + 1); + try std.testing.expect(multibyte.len > old_column_derived_bound); + try std.testing.expect(multibyte.len < max_snapshot_text_bytes); + try std.testing.expect( + buildPhysicalSnapshotToken(1, 0, 0, 0, 1, 80, &multibyte, 0, 0) != null, + ); + + const oversized_text = [_]u8{'a'} ** (max_snapshot_text_bytes + 1); + try std.testing.expect( + buildPhysicalSnapshotToken(1, 0, 0, 0, 1, 80, &oversized_text, 0, 0) == null, + ); + try std.testing.expect( + buildPhysicalSnapshotToken(1, 0, 0, 0, 1, max_snapshot_row_columns + 1, "", 0, 0) == null, + ); + try std.testing.expect( + buildPhysicalSnapshotToken(1, 0, 0, 0, max_snapshot_rows + 1, 80, "", 0, 0) == null, + ); + try std.testing.expect(buildPhysicalSnapshotToken(1, 0, 0, 0, 0, 80, "", 0, 0) == null); +} + +test "hover activation token differs on pointer cell, mods, or epoch" { + const testing = std.testing; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 2, 3, 4 } }; + const cell: point.Coordinate = .{ .x = 2, .y = 3 }; + + const base = buildHoverActivationToken(physical, cell, 0, 10); + try testing.expect(base.eql(buildHoverActivationToken(physical, cell, 0, 10))); + try testing.expect(!base.eql(buildHoverActivationToken(physical, .{ .x = 3, .y = 3 }, 0, 10))); + try testing.expect(!base.eql(buildHoverActivationToken(physical, cell, 1, 10))); + try testing.expect(!base.eql(buildHoverActivationToken(physical, cell, 0, 11))); + try testing.expect(!base.eql(buildHoverActivationToken(physical, null, 0, 10))); +} + +test "ExternalHover destructively invalidates on physical/context mismatch (ABA protection)" { + const testing = std.testing; + const alloc = testing.allocator; + + var hover: ExternalHover = .{}; + var diag: ExternalHoverDiagRing = .{}; + const token_a: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical_a: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical_b: PhysicalSnapshotToken = .{ .bits = .{ 2, 2, 2, 2 } }; + const cell: point.Coordinate = .{ .x = 1, .y = 0 }; + + try testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token_a, physical_a, 5, cell, 0, 1, &.{.{ .row = 0, .start_column = 0, .end_column = 3 }}, 0)); + try testing.expect(hover.active()); + try testing.expectEqual(ExternalHoverDiagVerdict.valid, hover.validateOrInvalidate(cell, physical_a, 5, true, &diag)); + + // A physical mismatch discards state... + try testing.expectEqual(ExternalHoverDiagVerdict.physicalTokenMismatch, hover.validateOrInvalidate(cell, physical_b, 5, true, &diag)); + try testing.expect(!hover.active()); + + // ...so a later re-observation of the *original* physical/context + // does not resurrect it: the only way back is a fresh `set`. + try testing.expectEqual(ExternalHoverDiagVerdict.none, hover.validateOrInvalidate(cell, physical_a, 5, true, &diag)); + try testing.expect(!hover.active()); + + var result: terminal.RenderState.CellSet = .empty; + defer result.deinit(alloc); + try hover.replaceCells(alloc, &result, 10, 10); + try testing.expectEqual(@as(usize, 0), result.count()); +} +test "ExternalHover.set rejects a zero-row scope before range validation" { + var hover: ExternalHover = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + try std.testing.expectEqual( + ExternalHoverDiagReason.zeroRowCount, + hover.set(token, physical, 0, cell, 0, 0, &.{}, 0), + ); + try std.testing.expect(!hover.active()); +} + +test "ExternalHover.set rejects ranges past the count or cell bound" { + var hover: ExternalHover = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + // Inverted range: containment passes via the first (normal) range, so + // the second (inverted, width 0) range is what the per-range + // validation loop actually rejects on — a range whose containment + // could never itself succeed can't otherwise reach the inversion + // check at all. + try std.testing.expectEqual(ExternalHoverDiagReason.rangeEmptyOrInverted, hover.set(token, physical, 0, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + .{ .row = 0, .start_column = 5, .end_column = 5 }, + }, 0)); + try std.testing.expect(!hover.active()); + + // Total cells past the bound. + try std.testing.expectEqual(ExternalHoverDiagReason.cellBudgetExceeded, hover.set(token, physical, 0, cell, 0, 1, &.{.{ .row = 0, .start_column = 0, .end_column = max_external_hover_cells + 1 }}, 0)); + try std.testing.expect(!hover.active()); + + // Exactly at the bound succeeds. + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 0, cell, 0, 1, &.{.{ .row = 0, .start_column = 0, .end_column = @intCast(max_external_hover_cells) }}, 0)); + try std.testing.expect(hover.active()); +} + +// cmux fork: (B) wiring review Blocking 2 — `range.row` is an absolute +// viewport row, NOT relative to `top_row` (see `replaceCells`, which uses +// `range.row` directly as the drawn cell's `.y` with no offset by +// `top_row`). A range whose row falls outside `[top_row, top_row + +// row_count)` can never legitimately belong to a scope `set` was just +// asked to claim, so it must be rejected the same way an inverted or +// oversized range already is. +test "ExternalHover.set rejects a range whose row falls outside [top_row, top_row + row_count)" { + var hover: ExternalHover = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + + // Scope is rows [5, 8) (top_row=5, row_count=3). Row 4 is just before + // it, row 8 is just past it — both out of scope. The pointer used for + // each sub-case is inside the ONE range being set, so the setter's + // containment guard passes and the out-of-scope check is what + // actually rejects here (not containment). + try std.testing.expectEqual(ExternalHoverDiagReason.rangeOutOfScope, hover.set(token, physical, 0, .{ .x = 0, .y = 4 }, 5, 3, &.{.{ .row = 4, .start_column = 0, .end_column = 1 }}, 0)); + try std.testing.expect(!hover.active()); + try std.testing.expectEqual(ExternalHoverDiagReason.rangeOutOfScope, hover.set(token, physical, 0, .{ .x = 0, .y = 8 }, 5, 3, &.{.{ .row = 8, .start_column = 0, .end_column = 1 }}, 0)); + try std.testing.expect(!hover.active()); + + // Every row actually inside [5, 8) succeeds — pointer at row 5, which + // is one of the ranges being set. + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 0, .{ .x = 0, .y = 5 }, 5, 3, &.{ + .{ .row = 5, .start_column = 0, .end_column = 1 }, + .{ .row = 6, .start_column = 0, .end_column = 1 }, + .{ .row = 7, .start_column = 0, .end_column = 1 }, + }, 0)); + try std.testing.expect(hover.active()); +} + +// cmux fork: (B) wiring review Blocking 2 — `top_row`/`row_count` are +// VIEWPORT-RELATIVE physical rows, not absolute/scrollback-inclusive +// screen rows: `Surface.setExternalLinkHover`'s bound check and the +// render loop's re-fingerprint (`generic.zig`) both pin `top_row` as +// `.viewport`. A caller (or a doc reader) who instead treated it as an +// absolute row would read/underline the wrong line the moment the +// viewport has scrolled away from the bottom. This exercises the exact +// `pages.pin(.{.viewport = ...})` lookup those call sites use, at a +// nonzero viewport scroll offset and a nonzero `top_row`, and confirms it +// tracks the viewport rather than resolving to a fixed absolute row. +test "viewport-relative row lookup tracks the viewport at a nonzero scroll offset and top_row" { + const testing = std.testing; + const alloc = testing.allocator; + + var t: Terminal = try .init(std.testing.io, alloc, .{ .cols = 10, .rows = 3 }); + defer t.deinit(alloc); + var stream = t.vtStream(); + defer stream.deinit(); + stream.nextSlice("line0\r\nline1\r\nline2\r\nline3\r\nline4\r\nline5\r\n"); + t.scrollViewport(.top); + + const screen = t.screens.active; + const readViewportRow = struct { + fn call(s: *Screen, a: std.mem.Allocator, row: u32) ![:0]const u8 { + const top_left = s.pages.pin(.{ .viewport = .{ .x = 0, .y = row } }).?; + const bottom_right = s.pages.pin(.{ .viewport = .{ .x = 9, .y = row } }).?; + return s.selectionString(a, .{ + .sel = terminal.Selection.init(top_left, bottom_right, false), + .trim = false, + .unwrap = false, + }); + } + }.call; + + // top_row = 1 (nonzero) while the viewport itself sits at a nonzero + // scroll offset (scrolled to the top of scrollback, not the bottom). + const before = try readViewportRow(screen, alloc, 1); + defer alloc.free(before); + + t.scrollViewport(.{ .delta = 1 }); + const after = try readViewportRow(screen, alloc, 1); + defer alloc.free(after); + + // The SAME viewport row (1) must resolve to different content once + // the viewport has moved — an absolute-row interpretation would have + // returned identical text both times, since nothing at that fixed + // absolute row changed. + try testing.expect(!std.mem.eql(u8, before, after)); +} + +// design-hover-diagnostics-v4-final.md §8 — Ghostty selection read focused +// test: reads a genuinely multi-row (row_count=3) span at a NONZERO +// top_row with trim=false/unwrap=false — the exact selection shape both +// the setter's physical fingerprint (`ghostty_surface_read_text_physical_rows`) +// and `generic.zig`'s per-frame re-fingerprint read — across a hard +// newline, a blank row, and a row containing wide/combining glyphs, and +// confirms rows outside the window never leak into the result. +test "multi-row physical selection read at a nonzero top_row preserves hard newlines, blank rows, and wide/combining glyphs" { + const testing = std.testing; + const alloc = testing.allocator; + + var t: Terminal = try .init(std.testing.io, alloc, .{ .cols = 10, .rows = 5 }); + defer t.deinit(alloc); + var stream = t.vtStream(); + defer stream.deinit(); + // Row 0: plain ascii, OUTSIDE the [1, 4) window under test. + // Row 1: plain ascii — top of the window. + // Row 2: blank. + // Row 3: a wide CJK glyph followed by a combining accent — bottom of + // the window. + // Row 4: plain ascii, OUTSIDE the window. + stream.nextSlice("skip0\r\n" ++ + "row1\r\n" ++ + "\r\n" ++ + "\u{4E2D}e\u{0301}\r\n" ++ + "skip4\r\n"); + // Anchor the viewport at absolute row 0 — otherwise the trailing + // `\r\n` after "skip4" scrolls row 0 ("skip0") into scrollback and + // shifts every viewport row index down by one. + t.scrollViewport(.top); + + const screen = t.screens.active; + const top_row: u32 = 1; + const row_count: u32 = 3; + const top_left = screen.pages.pin(.{ .viewport = .{ .x = 0, .y = top_row } }).?; + const bottom_right = screen.pages.pin(.{ .viewport = .{ .x = 9, .y = top_row + row_count - 1 } }).?; + const text = try screen.selectionString(alloc, .{ + .sel = terminal.Selection.init(top_left, bottom_right, false), + .trim = false, + .unwrap = false, + }); + defer alloc.free(text); + + // Exactly `row_count` physical rows, one newline per row boundary + // (not unwrapped/joined) — the host's downstream split step + // (`splitPhysicalViewportRows`) depends on this exact shape. + var lines = std.mem.splitScalar(u8, text, '\n'); + var line_count: usize = 0; + var saw_row1 = false; + var saw_blank = false; + var saw_wide_combining = false; + while (lines.next()) |line| { + line_count += 1; + if (std.mem.indexOf(u8, line, "row1") != null) saw_row1 = true; + if (std.mem.trim(u8, line, " ").len == 0) saw_blank = true; + if (std.mem.indexOf(u8, line, "\u{4E2D}") != null and + std.mem.indexOf(u8, line, "\u{0301}") != null) saw_wide_combining = true; + } + try testing.expectEqual(@as(usize, row_count), line_count); + try testing.expect(saw_row1); + try testing.expect(saw_blank); + try testing.expect(saw_wide_combining); + try testing.expect(std.mem.indexOf(u8, text, "skip0") == null); + try testing.expect(std.mem.indexOf(u8, text, "skip4") == null); +} + +test "ExternalHover.replaceCells re-validates ranges against current grid bounds" { + const testing = std.testing; + const alloc = testing.allocator; + + var hover: ExternalHover = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + // Set while the grid was still 10x10. + try testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 0, .{ .x = 0, .y = 2 }, 0, 10, &.{ + .{ .row = 2, .start_column = 0, .end_column = 5 }, + .{ .row = 9, .start_column = 0, .end_column = 5 }, // will be out of bounds after "resize" + }, 0)); + + var result: terminal.RenderState.CellSet = .empty; + defer result.deinit(alloc); + + // A resize down to 5 rows makes the second range stale; replaceCells + // must silently drop it rather than reading past the grid. + try hover.replaceCells(alloc, &result, 5, 10); + try testing.expect(result.contains(.{ .x = 0, .y = 2 })); + try testing.expect(!result.contains(.{ .x = 0, .y = 9 })); +} + +// impl-flicker-fix — review-flicker-fix-confirm.md §5's required tests +// (items 1-11 for Ghostty pure/core; items 3-4 landed as +// `Mouse.updateExternalHoverPointerCell` tests in `renderer/State.zig`, +// since they need `pointer_cell` state a bare `ExternalHover` doesn't +// carry on its own). + +// 2. A 2-row candidate: the pointer moving from the upper row's range to +// the lower row's range (still the SAME resolved link, same physical/ +// context) must stay valid — this is exactly what makes a hard-wrapped +// path's underline+indicator stable while the pointer travels its full +// length. +test "ExternalHover.validateOrInvalidate stays valid moving from a 2-row candidate's upper range to its lower range" { + var hover: ExternalHover = .{}; + var diag: ExternalHoverDiagRing = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 2, 3, 4 } }; + const upper: point.Coordinate = .{ .x = 5, .y = 0 }; + const lower: point.Coordinate = .{ .x = 2, .y = 1 }; + + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 7, upper, 0, 2, &.{ + .{ .row = 0, .start_column = 0, .end_column = 10 }, + .{ .row = 1, .start_column = 0, .end_column = 5 }, + }, 0)); + + try std.testing.expectEqual(ExternalHoverDiagVerdict.valid, hover.validateOrInvalidate(lower, physical, 7, true, &diag)); + try std.testing.expect(hover.active()); +} + +// 5. Setter-time containment: `set` itself rejects when the pointer is +// outside the ranges being claimed, independent of every other guard +// (shape, count, cell bound) already covered above. +test "ExternalHover.set rejects when the current pointer is outside the ranges being claimed" { + var hover: ExternalHover = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + + // Pointer at (9, 9) — nowhere near the range being claimed. + try std.testing.expectEqual(ExternalHoverDiagReason.pointerNotInRanges, hover.set(token, physical, 0, .{ .x = 9, .y = 9 }, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + try std.testing.expect(!hover.active()); + + // No pointer at all (viewport exit at the exact moment of the call). + try std.testing.expectEqual(ExternalHoverDiagReason.pointerMissing, hover.set(token, physical, 0, null, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + try std.testing.expect(!hover.active()); +} + +// 6. mods A->B->A without any render-time validation in between: the +// context epoch bumped by the B transition must not equal the ORIGINAL +// epoch just because mods returned to their original value — the caller +// (`Surface.modsChanged`) bumps monotonically on every real mods change, +// never decrementing back. +test "ExternalHover.validateOrInvalidate rejects a stale context epoch after mods A->B->A" { + var hover: ExternalHover = .{}; + var diag: ExternalHoverDiagRing = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + // Minted at epoch 5 (mods "A"). + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 5, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + + // mods change to "B" bumps the epoch to 6, then back to "A" bumps it + // AGAIN to 7 (monotonic, never restored to the original 5) — neither + // intermediate epoch, nor the "back to A" epoch, equals the ORIGINAL + // epoch 5 this override was minted under. + try std.testing.expectEqual(ExternalHoverDiagVerdict.contextEpochMismatch, hover.validateOrInvalidate(cell, physical, 7, true, &diag)); + try std.testing.expect(!hover.active()); +} + +// 7. eligibility true->false->true without any render-time validation in +// between: the old state must not revive just because eligibility +// happened to return to `true` — `validateOrInvalidate` destructively +// invalidates the FIRST time it observes `hover_eligible == false`, +// which review's own final-spec table also requires as an immediate +// render guard, not merely a deferred one. +test "ExternalHover.validateOrInvalidate does not revive old state after eligibility true->false->true" { + var hover: ExternalHover = .{}; + var diag: ExternalHoverDiagRing = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical, 0, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + + // Eligibility drops — destructively invalidates immediately. + try std.testing.expectEqual(ExternalHoverDiagVerdict.hoverIneligible, hover.validateOrInvalidate(cell, physical, 0, false, &diag)); + try std.testing.expect(!hover.active()); + + // Eligibility returns to true, same cell/physical/epoch as before — + // still must not revive; only a fresh `set` can reactivate. + try std.testing.expectEqual(ExternalHoverDiagVerdict.none, hover.validateOrInvalidate(cell, physical, 0, true, &diag)); + try std.testing.expect(!hover.active()); +} + +// 8. Scope content change (a physical mismatch) still invalidates, same +// as before this fix — `validateOrInvalidate` independently checks +// physical identity as one of its four conditions. (Screen switch, +// resize-bounds failure, and OSC8 priority are exercised by +// `generic.zig`'s own unchanged destructive-invalidate call sites, not +// re-tested here — they were never part of this fix's diff.) +test "ExternalHover.validateOrInvalidate invalidates on a physical (scope/content) mismatch" { + var hover: ExternalHover = .{}; + var diag: ExternalHoverDiagRing = .{}; + const token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical_a: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const physical_b: PhysicalSnapshotToken = .{ .bits = .{ 2, 2, 2, 2 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(token, physical_a, 0, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + try std.testing.expectEqual(ExternalHoverDiagVerdict.physicalTokenMismatch, hover.validateOrInvalidate(cell, physical_b, 0, true, &diag)); + try std.testing.expect(!hover.active()); +} + +// 11. A stale `clearExternalLinkHover(oldToken)` must not clear a NEWER +// active token — `Surface.clearExternalLinkHover`'s existing guard +// (`if (!self.renderer_state.mouse.external_hover.token.eql(token)) +// return;`) is unchanged by this fix; this pins that contract directly +// against `ExternalHover.token`, the one field this fix deliberately +// keeps as the host-visible clear identity (see the type's doc). +test "a clear for a token that is no longer the active owner is a no-op (existing contract)" { + var hover: ExternalHover = .{}; + const old_token: HoverActivationToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const new_token: HoverActivationToken = .{ .bits = .{ 2, 2, 2, 2 } }; + const physical: PhysicalSnapshotToken = .{ .bits = .{ 1, 1, 1, 1 } }; + const cell: point.Coordinate = .{ .x = 0, .y = 0 }; + + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(old_token, physical, 0, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + // A newer activation replaces it (mirroring a fresh `set` for a + // different candidate/token, the way the real host clear/set flow + // would produce a "new owner" between an old clear request being + // issued and actually processed). + try std.testing.expectEqual(ExternalHoverDiagReason.none, hover.set(new_token, physical, 0, cell, 0, 1, &.{ + .{ .row = 0, .start_column = 0, .end_column = 2 }, + }, 0)); + + // The exact guard `Surface.clearExternalLinkHover` applies before + // ever calling `invalidate()`. + if (hover.active() and hover.token.eql(old_token)) hover.invalidate(); + + try std.testing.expect(hover.active()); + try std.testing.expect(hover.token.eql(new_token)); +} diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 0be22fccffd..88b138eb6c9 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2984,6 +2984,16 @@ pub const SelectionString = struct { /// If true, trim whitespace around the selection. trim: bool = true, + /// If true, soft-wrapped row boundaries are not emitted, so the result + /// reads as unwrapped logical lines (the historical, default behavior). + /// If false, every physical row boundary in the selection is preserved + /// as its own line break — callers that need to map a screen + /// row/column back to an offset in the returned text (cmux fork: see + /// ghostty_surface_read_text_physical_rows) require this, since the + /// default join collapses two physical rows into one line and desyncs + /// any row-index-based lookup into the result. + unwrap: bool = true, + /// If non-null, a stringmap will be written here. This will use /// the same allocator as the call to selectionString. The string will /// be duplicated here and in the return value so both must be freed. @@ -3013,7 +3023,7 @@ pub fn selectionString( self, .{ .emit = .plain, - .unwrap = true, + .unwrap = opts.unwrap, .trim = opts.trim, }, ); @@ -10763,6 +10773,119 @@ test "Screen: selectionString soft wrap" { const expected = "2EFGH3IJ"; try testing.expectEqualStrings(expected, contents); } + + // cmux fork: unwrap=false preserves the soft-wrap boundary as a + // newline instead of joining the two physical rows into one line. + { + const sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 1 } }).?, + s.pages.pin(.{ .screen = .{ .x = 2, .y = 2 } }).?, + false, + ); + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + .unwrap = false, + }); + defer alloc.free(contents); + const expected = "2EFGH\n3IJ"; + try testing.expectEqualStrings(expected, contents); + } +} + +// cmux fork: ghostty_surface_read_text_physical_rows callers map a screen +// row/column back to an offset in the returned text, so hard newlines and +// leading/inner blank rows must round-trip as their own line — none silently +// dropped or merged into a neighbor — regardless of `unwrap`, since none of +// these row boundaries are soft wraps. Sandwiching each blank row between +// real content on both sides (rather than trailing off the selection on a +// never-written row) keeps this independent of how a selection's own +// trailing edge is represented. +test "Screen: selectionString unwrap preserves hard newlines and blank rows" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var s = try init(io, alloc, .{ .cols = 5, .rows = 6, .max_scrollback = 0 }); + defer s.deinit(); + // row0="" (leading blank), row1="A", row2="" (inner blank), row3="B", + // row4="" (inner blank), row5="C". + try s.testWriteString("\nA\n\nB\n\nC"); + + inline for (.{ true, false }) |unwrap| { + const sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?, + s.pages.pin(.{ .screen = .{ .x = 0, .y = 5 } }).?, + false, + ); + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + .unwrap = unwrap, + }); + defer alloc.free(contents); + const expected = "\nA\n\nB\n\nC"; + try testing.expectEqualStrings(expected, contents); + } +} + +// cmux fork: a wide character at the exact wrap boundary must not shift +// which physical row its neighbors land on — the row split still lines up +// with on-screen position even though the wide cell's trailing spacer +// consumes a column without emitting a character. +test "Screen: selectionString unwrap=false keeps row identity across a wide char at the wrap boundary" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var s = try init(io, alloc, .{ .cols = 4, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + // row0 = "AB" + wide '⚡' filling cols 0-3 exactly (wrap=true); row1 = "CD". + const str = "AB⚡CD"; + try s.testWriteString(str); + + const sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?, + s.pages.pin(.{ .screen = .{ .x = 1, .y = 1 } }).?, + false, + ); + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + .unwrap = false, + }); + defer alloc.free(contents); + const expected = "AB⚡\nCD"; + try testing.expectEqualStrings(expected, contents); +} + +// cmux fork: a combining mark attaches to the preceding cell rather than +// consuming a column of its own, so it must not perturb which physical row +// a later character lands in once that row soft-wraps. +test "Screen: selectionString unwrap=false keeps row identity across a combining mark" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var s = try init(io, alloc, .{ .cols = 3, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + // row0 = "e" + combining acute (´) + "A" (3 cols, wrap=true); row1 = "BC". + const str = "e\u{0301}ABC"; + try s.testWriteString(str); + + const sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?, + s.pages.pin(.{ .screen = .{ .x = 1, .y = 1 } }).?, + false, + ); + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + .unwrap = false, + }); + defer alloc.free(contents); + const expected = "e\u{0301}AB\nC"; + try testing.expectEqualStrings(expected, contents); } test "Screen: selectionString wide char" {