Files
ghostty-rs/docs/rewrite/notes/review-core-codex.md
T

5.2 KiB
Raw Blame History

Codex (gpt-6-astra) audit: core Surface vs Zig (2026-10-12)

Read-only static audit; I made no changes and ran no tests. Findings below exclude macOS-only paths and the inspector.

Critical

  • Replacing an existing selection panics with copy-on-select enabled. Rust: surface/mod.rs:1337; Zig: Surface.zig:2311. Rust replaces the selection, invalidating prev’s tracked handles, then compares against prev; the lookup’s expect consequently panics—for example, on Select All twice. Zig has the same unsafe ordering with raw pointers, but the port turns it into a deterministic handle-validation failure; snapshot the previous concrete bounds or compare before replacement.

Major

  • Child-environment filtering is undone. Rust: surface/mod.rs:1783; Zig: Surface.zig:623. Rust overlays the runtime’s environment onto the process environment, restoring variables GTK deliberately removed—including GDK_DISABLE, GSK_RENDERER, and NOTIFY_SOCKET—whereas Zig uses the runtime environment wholesale and falls back only on failure. Collect the runtime-provided environment directly, preserving its deletions.

  • Global/all new_window bindings incorrectly inherit a parent surface. Rust: surface/keyboard.rs:734; Zig: App.zig:445, reached through Surface.zig:2941. Rust’s duplicated app dispatcher targets the invoking surface, enabling parent-based directory/font inheritance; Zig explicitly creates the window with parent = null. Use Target::App in this dispatcher, retaining surface targeting for ordinary surface-local bindings.

  • Flatpak resource discovery bypasses the runtime-specific implementation. Rust: global.rs:135; Zig: global.zig:174. Rust calls generic resource discovery, which never populates host_path, whereas Zig’s GTK discovery obtains the host installation path from /.flatpak-info; consequently, host-side shells can receive an unusable sandbox resource path for shell integration. Restore runtime-specific discovery and preserve separate application and host paths.

  • Invalid-locale recovery is missing. Rust: global.rs:130; Zig: global.zig:164. Rust omits ensureLocale, and its GTK substitute ignores setlocale failure; Zig removes an invalid LANG, retries the system locale, then attempts en_US.UTF-8 and updates LANG on success. Port that recovery before locale-dependent initialization and child-environment capture.

  • Failed screen-file opening incorrectly counts as a performed binding. Rust: surface/binding.rs:592; Zig: Surface.zig:4543 and Surface.zig:6106. When the runtime declines opening and the fallback opener fails, Rust logs the error and returns success, so a performable: screen/history/selection-file binding consumes the key despite opening nothing; Zig propagates the error and permits fallback input. Return and propagate a Result from open_url_with_fallback.

Minor

  • OSC 52 Base64 acceptance differs in both directions. Rust: surface/clipboard.rs:49; Zig: Surface.zig:2149 and Surface.zig:2163. Rust accepts unpadded YQ, which Zig rejects during size validation, but rejects nonzero trailing bits in YR==, which Zig decodes to a and accepts by ignoring the decoding-stage InvalidPadding. Match Zig’s separate size-validation and decode-error rules.

  • Oversized CSI/ESC bindings are accepted instead of rejected. Rust: surface/binding.rs:694; Zig: Surface.zig:5155. Rust dynamically constructs and sends arbitrary-length sequences, whereas Zig’s 128-byte buffer rejects CSI payloads exceeding 126 bytes and ESC payloads exceeding 127 bytes before sending anything. Enforce the same limits and propagate failure for strict parity.

  • GHOSTTY_LOG parsing has different boolean and empty-value behavior. Rust: global.rs:33; Zig: global.zig:116, using args.zig:607. GHOSTTY_LOG=false leaves stderr logging enabled in Rust but disables it in Zig; an empty value disables logging in Rust but retains Zig’s default. Implement the packed-struct parser’s boolean aliases, default initialization, and invalid-field handling.