Update ghostty to v1.3.0 - #1142
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds macOS AppleScript scripting support and resources, extensive localization strings and sdef, Xcode project and Info.plist wiring, zsh integration gating flag and tests, terminal bell/audio handling, scriptable AppDelegate APIs, a ghostty submodule pointer bump and new C header type, docs updates documenting upstream rebase and fork changes. Changes
Sequence Diagram(s)sequenceDiagram
participant AppleScript as "AppleScript Client"
participant NSApp as "NSApplication"
participant AppDel as "AppDelegate"
participant ScriptObj as "ScriptWindow/Tab/Terminal"
participant Terminal as "Terminal Surface"
AppleScript->>NSApp: send AppleEvent (e.g., new tab / input text)
NSApp->>AppDel: route script command
AppDel->>ScriptObj: resolve target (window/tab/terminal)
ScriptObj->>Terminal: invoke operation (split / input / focus / close)
Terminal-->>ScriptObj: result/status
ScriptObj-->>AppDel: return success/failure
AppDel-->>NSApp: construct AppleEvent reply
NSApp-->>AppleScript: reply/result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR advances the Key changes:
One documentation issue found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["upstream Ghostty v1.3.0\n+ newer main commits"] --> B["patch 1: OSC 99 notification parser\na2252e7a9"]
B --> C["patch 2: macOS display link restart\nc07e6c5a5"]
C --> D["patch 4 (docs): resize stale-frame mitigation\n769bbf7a9 → 9efcdfdf8"]
D --> E["patch 3 (docs): keyboard copy mode C API\na50579bd5 ← submodule HEAD"]
E --> F["manaflow-ai/cmux submodule\npinned to a50579bd5"]
style F fill:#2d6a4f,color:#fff
style A fill:#1d3557,color:#fff
Last reviewed commit: 99818ff |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5be151d4df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmuxTests/GhosttyConfigTests.swift">
<violation number="1" location="cmuxTests/GhosttyConfigTests.swift:1525">
P2: `waitUntilExit()` is unbounded here; add a timeout so this test fails fast instead of potentially hanging the suite.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Resources/shell-integration/.zshenv (1)
16-18: Consider removing unused variable_cmux_had_ghostty_zdotdir.The variable is still assigned on line 16-18 but is no longer used since the gating condition at line 39 now uses
CMUX_LOAD_GHOSTTY_ZSH_INTEGRATIONinstead. The assignment and cleanup at line 51 can be removed.♻️ Suggested cleanup
-builtin typeset _cmux_had_ghostty_zdotdir=0 if [[ -n "${GHOSTTY_ZSH_ZDOTDIR+X}" ]]; then - _cmux_had_ghostty_zdotdir=1 builtin export ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" builtin unset GHOSTTY_ZSH_ZDOTDIRAnd at line 51:
- builtin unset _cmux_file _cmux_ghostty _cmux_integ _cmux_had_ghostty_zdotdir + builtin unset _cmux_file _cmux_ghostty _cmux_integ🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/shell-integration/.zshenv` around lines 16 - 18, Remove the now-unused _cmux_had_ghostty_zdotdir variable and its related setup/cleanup: delete the declaration/assignment of _cmux_had_ghostty_zdotdir (the builtin typeset and the block that sets it when GHOSTTY_ZSH_ZDOTDIR is present) and remove the corresponding cleanup/unset later, since the gating now uses CMUX_LOAD_GHOSTTY_ZSH_INTEGRATION; ensure no other code references _cmux_had_ghostty_zdotdir before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 3489-3500: The current
addWorkspace(windowId:workingDirectory:bringToFront:) unconditionally calls
setActiveMainWindow(...) and creates the workspace with select: true even when
bringToFront is false; change it so that focus/selection only happens when
bringToFront is true: only call setActiveMainWindow(...) and bringToFront(...)
when shouldBringToFront is true, and pass select: shouldBringToFront (or
otherwise false) to state.tabManager.addWorkspace(...) so the non-focus path
does not mutate the app's active window/workspace (references:
addWorkspace(windowId:workingDirectory:bringToFront:), setActiveMainWindow,
bringToFront, state.tabManager.addWorkspace).
In `@Sources/GhosttyTerminalView.swift`:
- Around line 1528-1534: The function appleScriptAutomationEnabled currently
returns true when config is nil which opens the gate on init/error; change it to
fail-closed by returning false when config is nil, i.e. in
appleScriptAutomationEnabled guard let config else { return false }, and keep
the existing ghostty_config_get call using the key "macos-applescript" and the
local enabled var so missing-key still yields the intended false default.
---
Nitpick comments:
In `@Resources/shell-integration/.zshenv`:
- Around line 16-18: Remove the now-unused _cmux_had_ghostty_zdotdir variable
and its related setup/cleanup: delete the declaration/assignment of
_cmux_had_ghostty_zdotdir (the builtin typeset and the block that sets it when
GHOSTTY_ZSH_ZDOTDIR is present) and remove the corresponding cleanup/unset
later, since the gating now uses CMUX_LOAD_GHOSTTY_ZSH_INTEGRATION; ensure no
other code references _cmux_had_ghostty_zdotdir before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8b837db-5f9f-4833-999f-70bfa3f05697
📒 Files selected for processing (10)
GhosttyTabs.xcodeproj/project.pbxprojResources/Info.plistResources/Localizable.xcstringsResources/cmux.sdefResources/shell-integration/.zshenvSources/AppDelegate.swiftSources/AppleScriptSupport.swiftSources/GhosttyTerminalView.swiftcmuxTests/GhosttyConfigTests.swiftghostty.h
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae9ddcf148
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/GhosttyTerminalView.swift (1)
1528-1534:⚠️ Potential issue | 🟠 MajorFail closed when the AppleScript gate cannot read config.
Line 1529 still returns
truewhenconfigis unavailable, so init/error paths bypass themacos-applescriptgate. This should returnfalse.🔒 Proposed fix
func appleScriptAutomationEnabled() -> Bool { - guard let config else { return true } + guard let config else { return false } var enabled = false let key = "macos-applescript" _ = ghostty_config_get(config, &enabled, key, UInt(key.lengthOfBytes(using: .utf8))) return enabled }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 1528 - 1534, The appleScriptAutomationEnabled() function currently returns true when config is nil, bypassing the macos-applescript gate; change the guard to return false when config is unavailable and ensure the retrieved value from ghostty_config_get (called with key "macos-applescript") is used as the result (defaulting to false on any read failure) so the feature fails closed instead of open.
🧹 Nitpick comments (1)
Sources/GhosttyTerminalView.swift (1)
711-711: Cache custom bell audio by path to avoid repeated file I/O on the main thread.
NSSound(contentsOfFile:byReference:)is called on every bell ring at line 1584, causing file I/O and decoding each time. Instead, cache the sound by resolvedbell-audio-pathand only rebuild when the path changes. Store the cached path alongsidebellAudioSoundto detect when invalidation is needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` at line 711, The NSSound is being recreated on every bell ring; add a cachedPath String? alongside the existing private var bellAudioSound: NSSound? and update the bell ring logic (where NSSound(contentsOfFile:byReference:) is currently called) to compare the resolved bell-audio-path to cachedPath and only create a new NSSound and assign cachedPath when the path differs or is nil; reuse bellAudioSound for subsequent rings to avoid repeated file I/O and decoding on the main thread, and ensure invalidation occurs when settings change the bell-audio-path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift`:
- Around line 7638-7639: The test currently sends a real printable character
("a") via makeKeyEvent and surfaceView.keyDown(with:), which forwards into
Ghostty and can cause flakiness; change the event so it triggers
GhosttyNSView.keyDown(with:) dismissal logic without injecting terminal input —
e.g., call makeKeyEvent with a non-printable character or empty characters
(replace "a" with "" or a NUL/non-printable code) or use a non-printable keyCode
(arrow/function key) so surfaceView.keyDown(with: event) exercises the
unread-state dismissal branch but does not forward a printable character into
the Ghostty surface. Ensure the change targets the makeKeyEvent(...) call used
before surfaceView.keyDown(with:) and preserves any required modifier/flags for
the dismissal path.
---
Duplicate comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 1528-1534: The appleScriptAutomationEnabled() function currently
returns true when config is nil, bypassing the macos-applescript gate; change
the guard to return false when config is unavailable and ensure the retrieved
value from ghostty_config_get (called with key "macos-applescript") is used as
the result (defaulting to false on any read failure) so the feature fails closed
instead of open.
---
Nitpick comments:
In `@Sources/GhosttyTerminalView.swift`:
- Line 711: The NSSound is being recreated on every bell ring; add a cachedPath
String? alongside the existing private var bellAudioSound: NSSound? and update
the bell ring logic (where NSSound(contentsOfFile:byReference:) is currently
called) to compare the resolved bell-audio-path to cachedPath and only create a
new NSSound and assign cachedPath when the path differs or is nil; reuse
bellAudioSound for subsequent rings to avoid repeated file I/O and decoding on
the main thread, and ensure invalidation occurs when settings change the
bell-audio-path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a16d1d4-7654-4c41-8596-f25e933f79d3
📒 Files selected for processing (2)
Sources/GhosttyTerminalView.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swift
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
* Update ghostty to v1.3.0 * Add bell handling and AppleScript support * Add zsh shell integration handoff test * Fix Ghostty zsh integration handoff in cmux * Add terminal keypress notification dismissal test * Dismiss terminal notifications on keypress * Address PR review feedback * Tighten notification dismissal regression test * Pin GhosttyKit checksum for latest ghostty
Summary
ghosttysubmodule to upstream Ghosttyv1.3.0plus newermainfixesdocs/ghostty-fork.mdto document the rebased fork delta and note thatcursor-click-to-movelanded upstreamTesting
cd /Users/lawrencechen/fun/cmuxterm-hq/worktrees/task-update-ghostty-latest && ./scripts/reload.sh --tag task-update-ghostty-latest(build succeeded)Issues
Summary by cubic
Updated
ghosttysubmodule to v1.3.0 (+ recentmain), added macOS AppleScript automation and configurable bell features, refined tab creation to avoid stealing focus, and fixed zsh shell‑integration handoff and notification dismissal. PinnedGhosttyKitchecksum for the new submodule.New Features
cmux.sdef,AppleScriptSupport.swift) gated bymacos-applescript; supports querying windows/tabs/terminals, performing actions, creating windows, creating tabs without bringing to front by default, splitting, focusing/closing, and inputting text.bell-audio-path/bell-audio-volume, and Dock attention; wired upGHOSTTY_ACTION_RING_BELL.Bug Fixes
CMUX_LOAD_GHOSTTY_ZSH_INTEGRATIONfrom Ghostty and update.zshenvto use it; add tests so Ghostty prompt hooks load only when requested.Written for commit b172394. Summary will update on new commits.
Summary by CodeRabbit