Skip to content

Tahoe 26 Liquid Glass integration: theme picker hooks, layer-background mode, focus-report leak fix#40

Open
rodchristiansen wants to merge 11 commits intomanaflow-ai:mainfrom
rodchristiansen:cmux-tahoe26-liquid-glass
Open

Tahoe 26 Liquid Glass integration: theme picker hooks, layer-background mode, focus-report leak fix#40
rodchristiansen wants to merge 11 commits intomanaflow-ai:mainfrom
rodchristiansen:cmux-tahoe26-liquid-glass

Conversation

@rodchristiansen
Copy link
Copy Markdown

@rodchristiansen rodchristiansen commented Apr 14, 2026

Companion to cmux #2647 (Adopt macOS 26 Liquid Glass design) and cmux #2646 (sidebar sections). Both cmux PRs pin to this ghostty SHA; merging here lets them bump pointer back to manaflow-ai/ghostty#main.

Summary

Three logical groups, combined into one PR because cmux tests them together:

cmux theme picker integration

  • Add cmux theme picker helper hooks
  • Fix cmux theme picker preview writes
  • Improve cmux theme picker footer contrast
  • Respect system theme in cmux picker
  • Skip theme detection in cmux picker
  • Match Ghostty theme picker startup
  • Harden cmux theme override writes

macos-background-from-layer config (needed for Liquid Glass translucent backgrounds)

  • Add macos-background-from-layer config flag
  • Skip fullscreen bg draw call in layer-background mode

Focus-report leak fix (ghostty-org/ghostty ghostty-org#2446)

  • Seed initial focus state and avoid startup focus-report leak
  • Merge of origin/main into the focus-reporting-leak branch (11th commit)

Test plan


Summary by cubic

Adds macOS layer-provided background support for Tahoe 26 Liquid Glass, integrates cmux theme picker hooks, and fixes a startup focus-report leak. The renderer can now skip its fullscreen background pass on macOS, and surfaces seed initial focus so DECSET 1004 doesn't emit at startup.

  • New Features

    • macos-background-from-layer (macOS): lets the host CALayer provide the background; renderer zeros bg alpha and skips the fullscreen fill. Enables Liquid Glass behavior.
    • Theme picker hooks for cmux: picker renders cmux theme data with safe preview writes, startup parity, and system theme respect.
  • Bug Fixes

    • Focus-report leak: surfaces accept an initial focused state and seed the terminal before IO. Enabling DECSET 1004 no longer sends an immediate focus sequence.

Written for commit 9fa3ab0. Summary will update on new commits.

lawrencecchen and others added 11 commits March 13, 2026 17:21
When enabled, the renderer sets bg_color alpha to 0 so the host app
can provide the terminal background via CALayer.backgroundColor.
This allows instant coverage during view resizes without alpha
double-stacking between the GPU bg pass and the layer background.
In addition to zeroing bg_color alpha (needed for cell compositing),
explicitly skip the fullscreen background fill draw step. This avoids
relying on alpha=0 as a no-op for the draw pass and makes the intent
explicit.
Copilot AI review requested due to automatic review settings April 14, 2026 18:44
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

Warning

Rate limit exceeded

@rodchristiansen has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 59 minutes and 34 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 34 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 016f6a3d-e5f0-4be0-8409-f75fed2afa70

📥 Commits

Reviewing files that changed from the base of the PR and between 3b684a0 and 9fa3ab0.

📒 Files selected for processing (7)
  • include/ghostty.h
  • macos/Sources/Ghostty/Surface View/SurfaceView.swift
  • src/Surface.zig
  • src/apprt/embedded.zig
  • src/config/Config.zig
  • src/renderer/generic.zig
  • src/termio/stream_handler.zig
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Ghostty to better support macOS 26 “Liquid Glass” embedding needs and fixes a focus-reporting leak by aligning behavior with xterm focus reporting semantics.

Changes:

  • Stop emitting an immediate focus sequence when DECSET 1004 (focus reporting) is enabled, and add a regression test.
  • Add macos-background-from-layer config and update the renderer to skip the fullscreen background fill when the host supplies background via CALayer.
  • Introduce an “initial focused state” plumbed through the embedded runtime/C API and macOS Swift surface configuration, and seed terminal focus state during surface init.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/termio/stream_handler.zig Avoid immediate focus report on enabling focus mode; add test to prevent regressions.
src/renderer/generic.zig Add derived config flag and skip fullscreen background fill in layer-background mode; adjust bg alpha behavior on macOS.
src/config/Config.zig Add macos-background-from-layer configuration flag and documentation.
src/apprt/embedded.zig Add focus state to embedded Surface + Options and expose it for initial seeding/inheritance.
src/Surface.zig Read initial focus state from runtime surface and seed terminal/render-thread focus flags.
macos/Sources/Ghostty/Surface View/SurfaceView.swift Plumb initial focus through Swift configuration into the C surface config.
include/ghostty.h Extend ghostty_surface_config_s with a focused field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Surface.zig
Comment on lines +580 to 581
render_thread.flags.focused = initial_focused;

Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

render_thread.flags.focused is seeded from initial_focused, but the renderer implementation itself is still initialized with focused = true (see renderer/generic.zig). If initial_focused is ever false, the renderer thread will later process the first .focus = true message and call renderer.setFocus(true), which currently asserts because the renderer already thinks it is focused. This also means an initially-unfocused surface would render with focused cursor/state until a focus transition occurs.

To fix this, ensure the renderer thread applies the initial focus state to the renderer before the event loop starts (and before starting cursor blink), e.g. on thread startup call renderer.setFocus(false) when flags.focused is false, or otherwise synchronize renderer focus with initial_focused without relying on a subsequent focus transition.

Copilot uses AI. Check for mistakes.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 14, 2026

Greptile Summary

This PR wires up three features for the cmux/Liquid Glass integration: cmux theme-picker hooks (manual IO mode + write callback), a macos-background-from-layer config flag that skips the fullscreen GPU background pass so a SwiftUI .glass material can show through, and the focus-report startup leak fix (seeds terminal focus state before the IO thread starts so DECSET 1004 no longer synthesises a spurious CSI I/O on enable).

  • ghostty_surface_free_text ABI crash: the Zig export was reduced to one parameter, but the public header still declares two params and SurfaceView_AppKit.swift calls the two-argument form in 8 places — each call will pass the surface pointer as ptr and invoke ptr.deinit() on it, corrupting memory. The header declaration and all Swift call sites must be updated atomically.
  • working-directory = ~/path silently regresses: the WorkingDirectory union (with tilde expansion) was replaced by a plain ?[]const u8 with no migration, so existing user configs using ~/ will receive a literal unexpanded path passed to the shell.

Confidence Score: 4/5

  • Not safe to merge until the ghostty_surface_free_text ABI mismatch is resolved; all other findings are P2.
  • The renderer, focus-leak fix, and manual-IO plumbing look correct, but the ghostty_surface_free_text parameter-count mismatch between the Zig export and the public C header (and 8 active Swift call sites in SurfaceView_AppKit.swift) is a definite crash on any code path that frees text selection results.
  • include/ghostty.h (free_text declaration) and src/apprt/embedded.zig (free_text export) must be reconciled; SurfaceView_AppKit.swift call sites must match whichever signature is chosen.

Important Files Changed

Filename Overview
include/ghostty.h Removed GHOSTTY_API visibility macro and added new types/fields; ghostty_surface_free_text still declares 2 params despite the Zig implementation changing to 1 param — an ABI mismatch that will crash 8 Swift call sites in SurfaceView_AppKit.swift.
src/apprt/embedded.zig Added IoMode/manual-IO support, focused seeding, and io_write_cb plumbing. The ghostty_surface_free_text export was silently reduced to one param while the header (and all Swift callers) still use two params.
src/Surface.zig Focus-state seeding before IO thread start looks correct; manual IO mode branching is clean; mouseReport refactored to be fallible and inline-encoded. Zero-size resize guard and balancePaddingIfNeeded simplification are safe.
src/config/Config.zig New macos-background-from-layer flag is straightforward. working-directory type change silently drops tilde expansion, and removal of the same-path guard may cause config double-loading.
src/renderer/generic.zig Layer-background mode skips fullscreen fill and zeros bg_color alpha correctly. Resize logic with presentLastTarget guard and has_presented flag is sound. Display-link restart after setCurrentCGDisplay is a good defensive fix.
src/termio/stream_handler.zig Removing the synthetic focus sequence on DECSET 1004 enable is correct; requestMode/requestModeUnknown inline formatting is equivalent to the old path. New unit test validates the non-emission behavior. Minor typos introduced.

Sequence Diagram

sequenceDiagram
    participant Host as Host (cmux/SwiftUI)
    participant Embedded as embedded.zig Surface
    participant Core as Surface.zig (Core)
    participant IO as termio / IO thread
    participant Terminal as terminal.flags

    Host->>Embedded: "ghostty_surface_new(config{focused=true})"
    Embedded->>Embedded: "Surface.init(opts{focused=true})"
    Embedded->>Core: CoreSurface.init()
    Core->>Core: "initial_focused = rt_surface.initialFocused() → true"
    Core->>IO: "render_thread.flags.focused = true"
    Core->>Terminal: "terminal.flags.focused = true (seeded before IO starts)"
    Note over Terminal: DECSET 1004 can now be set without<br/>emitting a spurious CSI I/O
    Core->>IO: io_thread.start()
    IO-->>Terminal: App enables DECSET 1004 (.focus_event)
    Note over IO,Terminal: setMode(.focus_event) — no synthetic report emitted<br/>(fix for issue #2446)

    Host->>Embedded: ghostty_surface_set_focus(surface, false)
    Embedded->>Core: focusCallback(false)
    Core->>Terminal: "focused = false"
    Core->>IO: "sends .focus = false message"
    IO-->>Host: CSI O (focus-out) emitted only here
Loading

Comments Outside Diff (6)

  1. src/apprt/embedded.zig, line 1738-1741 (link)

    P1 ghostty_surface_free_text ABI mismatch causes crashes in existing call sites

    The Zig export was reduced to one parameter (ptr: *Text), but the header still declares void ghostty_surface_free_text(ghostty_surface_t, ghostty_text_s*) — and SurfaceView_AppKit.swift calls the two-argument form in at least 8 places (lines 288, 308, 1392, 1786, 1837, 1885, 2052, 2196). On all calling conventions the first register/slot holds ghostty_surface_t, so ptr.deinit() will be invoked on the surface pointer rather than the text struct, corrupting or crashing.

    Either update the header (and all Swift call sites) to the new one-argument form:

    Or, until the Swift callers are updated, keep the unused surface parameter to preserve ABI compatibility:

    export fn ghostty_surface_free_text(_: *Surface, ptr: *Text) void {
        ptr.deinit();
    }
    

    The header declaration and every Swift defer { ghostty_surface_free_text(surface, &text) } must be updated atomically with this change.

  2. include/ghostty.h, line 1137 (link)

    P1 Header still declares two-argument form

    SurfaceView_AppKit.swift has 8 call sites using the two-argument form ghostty_surface_free_text(surface, &text), but the Zig export now only accepts one argument. The header must match the implementation or the Swift callers must be updated — see the companion comment in embedded.zig.

  3. include/ghostty.h, line 1-30 (link)

    P2 GHOSTTY_API symbol-visibility macro removed

    The GHOSTTY_API block handled Windows DLL exports/imports and GCC/Clang -fvisibility=hidden for non-macOS shared-library builds. Removing it silently breaks any future or existing shared-library consumers on Linux/Windows who relied on __attribute__((visibility("default"))) or __declspec(dllexport/dllimport). If this is intentionally a macOS-static-library-only fork, a comment or guard to that effect (#if !defined(__APPLE__) error) would prevent the confusion.

  4. src/config/Config.zig, line 1572 (link)

    P2 working-directory = ~/foo silently no longer expands tilde

    Changing the field from ?WorkingDirectory to ?[]const u8 drops the finalize() tilde-expansion pass that converted ~/projects to the absolute home-relative path. The doc comment was updated to remove mention of ~/, but any user config with working-directory = ~/path will now receive a literal ~/path string passed to posix_spawn/execve, which will fail. A migration note (or re-implementing tilde expansion in finalize) would prevent a silent regression for existing users.

  5. src/config/Config.zig, line 4689-4691 (link)

    P2 Config may be loaded twice when legacy and preferred paths are the same

    The old code guarded against this with if (!std.mem.eql(u8, legacy_app_support_path, app_support_path)) self.loadOptionalFile(...). The new code unconditionally calls loadOptionalFile for both paths. On systems where preferredAppSupportPath returns the same path as legacy_app_support_path, the file is loaded twice: both legacy_app_support_action and app_support_action will be .loaded, the "loading them both in that order" warning fires, and every config key in that file takes effect twice (potentially doubling keybinds, palette entries, etc.).

  6. src/termio/stream_handler.zig, line 36-38 (link)

    P2 Duplicated word in comment

    "hints to the renderer that that a repaint" — extra "that".

    A second duplicate appears at line 1036: "Avoid changing the shape it it is already set" → "if it is already set".

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

okuuva added a commit to okuuva/ghostty that referenced this pull request Apr 19, 2026
DECSET 2031 (ColorPaletteUpdates) should notify applications only when
the color scheme changes, not immediately on enable. The unsolicited
initial report leaks into applications that enable the mode without
arranging to consume a response, surfacing as literal `?997;1n` text
in the shell input buffer.

Mirrors the fix applied to DECSET 1004 focus reporting in manaflow-ai#40.

Fixes manaflow-ai/cmux#2636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants