Repository navigation
Fix terminal focus retry after tiny responder handoff - #6359
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR addresses a regression where terminal panes intermittently stop responding to keyboard input when running many agents. The fix implements deferred first-responder focus recovery when a Ghostty surface frame becomes hidden or tiny. When ChangesDeferred First-Responder Focus Recovery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 21 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (21 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR fixes a latent AppKit/Ghostty focus-state divergence: when
Confidence Score: 5/5The change is a targeted retry mechanism using AppKit layout signals — no timers, no polling. The logic is correct in all traced paths, tests cover the key recovery scenarios, and the existing focus machinery is not otherwise restructured. All cancel-sites for the new flag were verified against every focus-yield and resign path visible in the diff; the retry loop is bounded by real AppKit layout events; no regression was found in the coordinator-suppress, reparent-suppress, Find-bar restore, or right-sidebar dock paths. Sources/GhosttyTerminalView.swift — the new pendingSuppressedFirstResponderFocusReapply flag now participates in 9+ focus code paths; any future focus-yield entrypoint that forgets to call cancelSuppressedFirstResponderFocusReapply will leave the flag stuck, causing ordinary focus applies to gate on surface geometry. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant AppKit
participant GhosttyNSView as GhosttyNSView (surfaceView)
participant ScrollView as GhosttySurfaceScrollView (hostedView)
participant Ghostty
AppKit->>GhosttyNSView: becomeFirstResponder()
GhosttyNSView->>GhosttyNSView: "desiredFocus = true"
Note over GhosttyNSView: geometry is hidden/tiny
GhosttyNSView->>ScrollView: scheduleSuppressedFirstResponderFocusReapply()
ScrollView->>ScrollView: "pendingSuppressedFirstResponderFocusReapply = true"
ScrollView->>ScrollView: scheduleAutomaticFirstResponderApply()
Note over ScrollView: async apply fires (geometry still tiny)
ScrollView->>ScrollView: applyFirstResponderIfNeeded()
ScrollView-->>ScrollView: return (geometry guard fails, flag stays true)
AppKit->>GhosttyNSView: layout() — frame grows to usable size
GhosttyNSView->>ScrollView: scheduleSuppressedFirstResponderFocusReapplyIfReady()
ScrollView->>ScrollView: all guards pass → scheduleAutomaticFirstResponderApply()
Note over ScrollView: deferred apply fires
ScrollView->>ScrollView: applyFirstResponderIfNeeded()
ScrollView->>ScrollView: reassertTerminalSurfaceFocus()
ScrollView->>Ghostty: setFocus(true)
ScrollView->>ScrollView: "pendingSuppressedFirstResponderFocusReapply = false"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant AppKit
participant GhosttyNSView as GhosttyNSView (surfaceView)
participant ScrollView as GhosttySurfaceScrollView (hostedView)
participant Ghostty
AppKit->>GhosttyNSView: becomeFirstResponder()
GhosttyNSView->>GhosttyNSView: "desiredFocus = true"
Note over GhosttyNSView: geometry is hidden/tiny
GhosttyNSView->>ScrollView: scheduleSuppressedFirstResponderFocusReapply()
ScrollView->>ScrollView: "pendingSuppressedFirstResponderFocusReapply = true"
ScrollView->>ScrollView: scheduleAutomaticFirstResponderApply()
Note over ScrollView: async apply fires (geometry still tiny)
ScrollView->>ScrollView: applyFirstResponderIfNeeded()
ScrollView-->>ScrollView: return (geometry guard fails, flag stays true)
AppKit->>GhosttyNSView: layout() — frame grows to usable size
GhosttyNSView->>ScrollView: scheduleSuppressedFirstResponderFocusReapplyIfReady()
ScrollView->>ScrollView: all guards pass → scheduleAutomaticFirstResponderApply()
Note over ScrollView: deferred apply fires
ScrollView->>ScrollView: applyFirstResponderIfNeeded()
ScrollView->>ScrollView: reassertTerminalSurfaceFocus()
ScrollView->>Ghostty: setFocus(true)
ScrollView->>ScrollView: "pendingSuppressedFirstResponderFocusReapply = false"
Reviews (13): Last reviewed commit: "chore: clarify focus reassertion retry h..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
4922-4926:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel stale suppressed retries before returning for reparent suppression.
This branch intentionally prevents the old reparented view from stealing focus, but a previously queued hidden/tiny retry can survive and later re-apply focus during layout. Clear it before the early return.
🐛 Proposed fix
if suppressingReparentFocus { + terminalSurface?.hostedView.cancelSuppressedFirstResponderFocusReapply() `#if` DEBUG cmuxDebugLog("focus.firstResponder SUPPRESSED (reparent) surface=\(terminalSurface?.id.uuidString.prefix(5) ?? "nil")") `#endif` return result }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/GhosttyTerminalView.swift` around lines 4922 - 4926, Before the early return statement in the suppressingReparentFocus block, add code to cancel any stale queued hidden or tiny retry that might re-apply focus later during layout. This prevents the old reparented view from stealing focus when the layout is processed. Identify the retry mechanism being used in the codebase (likely a scheduled task or timer) and ensure it is cleared when suppressing reparent focus, so the cancellation happens before the function returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9795-9809: In the
scheduleSuppressedFirstResponderFocusReapplyIfReady function, add an early guard
check for pendingAutomaticFirstResponderApply at the beginning of the function
to return early if it is already true. This prevents unnecessary re-execution of
the matchesCurrentTerminalFocusTarget pane scan when an automatic first
responder apply is already queued, since the queued apply will re-check geometry
and ownership conditions anyway on the same layout turn, avoiding redundant
collection scans.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4922-4926: Before the early return statement in the
suppressingReparentFocus block, add code to cancel any stale queued hidden or
tiny retry that might re-apply focus later during layout. This prevents the old
reparented view from stealing focus when the layout is processed. Identify the
retry mechanism being used in the codebase (likely a scheduled task or timer)
and ensure it is cleared when suppressing reparent focus, so the cancellation
happens before the function returns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: da92da8b-888b-4402-b4e3-4b1d155dae23
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Sources/GhosttyTerminalView.swiftcmuxTests/WorkspaceUnitTests.swift
This was referenced Jun 20, 2026
6 tasks done
1 of 2 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GhosttyNSView.becomeFirstResponder()suppresses focus because geometry is hidden or tinyRoot cause
Code inspection confirmed a reachable AppKit-focused/Ghostty-unfocused divergence:
becomeFirstResponder()sets the view-leveldesiredFocusbefore geometry gating, then the hidden/tiny branch logs and returns without callingghostty_surface_set_focusor scheduling the normal focus reconciliation path. If AppKit makes the Ghostty view first responder during transient bonsplit/window portal layout churn, subsequent plain key input can route to the view while the runtime surface still hasdesiredFocusState == false.The branch dates to February 2026 (
5070b137a4), so this looks like a latent race exposed by later timing changes rather than code introduced directly by the June refactors. The June candidates mostly changed surrounding timing: #6147 moved portal installation through window-chrome overlay resolution, #6278 moved terminal surface lifecycle into the macOS package and added restore/input-demand paths, and #6227/#6267 did not show a direct focus-state change in this path.Tests
Closes #6342
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Retries terminal focus after AppKit hands first responder to a hidden or tiny terminal, deferring apply until the portal and inner surface are usable. Forced reparent focus uses the same geometry guard and queues a retry on layout/visibility; dock retries only run when the dock owns first responder (fixes #6342).
Bug Fixes
Tests
Written for commit 9df36b5. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests