Repository navigation
Fix iOS 27 keyboard re-open animation - #10006
Merged
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughThe PR adds initial keyboard-frame overlap support and lifecycle tracking for overlapping iOS 27 keyboard notifications. ChangesKeyboard transition handling
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to The PR improves keyboard animation handling during rapid toggles, but a legitimate repeated transition can still be ignored when it returns to earlier geometry, causing incorrect or missing animation in that sequence. This should be addressed before merge. Sequence Diagram(s)sequenceDiagram
participant UIKit
participant GhosttySurfaceView
participant KeyboardNotificationTransitionLifecycle
participant DockGeometry
UIKit->>GhosttySurfaceView: Post keyboard frame notification
GhosttySurfaceView->>KeyboardNotificationTransitionLifecycle: Resolve begin/end overlap
KeyboardNotificationTransitionLifecycle-->>GhosttySurfaceView: Return decision and generation
GhosttySurfaceView->>DockGeometry: Animate, converge, settle, or ignore dock update
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (21 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 131-140: In GhosttySurfaceView.swift lines 131-140, update the
keyboard completion handling to avoid recording or resolving transition legs
while the geometry owner has no window, allowing the first post-attach did
notification to use the converge path. In
KeyboardNotificationTransitionLifecycleTests.swift lines 54-78, add coverage
that posts will while detached, attaches the view, then posts did and asserts
convergence to the settled target.
In
`@Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift`:
- Around line 98-104: Update probeValues(_:) to construct the dictionary with a
uniquing initializer instead of Dictionary(uniqueKeysWithValues:), preserving
the parsed key/value pairs while allowing duplicate keys to be handled without a
runtime trap so the downstream test assertion can fail normally.
🪄 Autofix
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 Plus
Run ID: bbe051a6-a709-4bd3-8265-e44f47038e6e
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/MobileKeyboardTransition.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 77-89: Use MobileKeyboardTransition.beginFrame and endFrame,
rather than owner-dependent overlaps, as the identity stored and compared by
Leg.matches in GhosttySurfaceView.swift:77-89. At
GhosttySurfaceView.swift:1127-1135, pass raw notification frames to resolve
while retaining overlap calculations only for dock layout. In
KeyboardNotificationTransitionLifecycleTests.swift:25-78, add a
will/owner-resize/did scenario asserting the final dock target uses the
post-resize overlap.
🪄 Autofix
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 Plus
Run ID: c07768e1-1a0f-4b13-8a29-fc8d5d7a2c19
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/MobileKeyboardTransition.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift`:
- Around line 177-188: Update
KeyboardNotificationTransitionLifecycle.resolve(phase: .will, ...) to reject
duplicate or superseded will notifications instead of creating a new generation
during an active reversal. Preserve the authoritative typed lifecycle state and
keep the existing reversal target and keyboardUp values unchanged when the stale
hide leg arrives.
🪄 Autofix
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 Plus
Run ID: d17d62c1-e954-4a2a-98ea-6844b8df3c74
📒 Files selected for processing (1)
Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift
Comment on lines
+177
to
+188
| // UIKit can deliver a duplicate of the superseded hide leg after the | ||
| // reversal starts. That older will must not reclaim the dock target. | ||
| postKeyboardFrameChange( | ||
| UIResponder.keyboardWillChangeFrameNotification, | ||
| beginFrame: shownFrame, | ||
| endFrame: hiddenFrame | ||
| ) | ||
|
|
||
| let probe = probeValues(view.composerDockProbeValue) | ||
| #expect(probe["keyboardDockSource"] == "notification") | ||
| #expect(probe["keyboardTransitionTarget"] == "300.000") | ||
| #expect(probe["keyboardUp"] == "1") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject a superseded duplicate will notification before this test can pass.
KeyboardNotificationTransitionLifecycle.resolve(phase: .will, ...) currently records every will leg. The notification at Lines 179-183 creates a new hide generation. GhosttySurfaceView then applies a zero dock target and sets keyboardUp to 0.
Add stale-leg detection for duplicate will notifications, or otherwise preserve the active reversal generation. Do not change these assertions to match the stale hide target.
As per path instructions, keyboard lifecycle tracking must use one authoritative typed source and ignore stale notifications.
🤖 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
`@Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/KeyboardNotificationTransitionLifecycleTests.swift`
around lines 177 - 188, Update
KeyboardNotificationTransitionLifecycle.resolve(phase: .will, ...) to reject
duplicate or superseded will notifications instead of creating a new generation
during an active reversal. Preserve the authoritative typed lifecycle state and
keep the existing reversal target and keyboardUp values unchanged when the stale
hide leg arrives.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 133-138: Update the stale-transition check in GhosttySurfaceView’s
transition handling to use an authoritative transition/event identity, not only
beginFrame/endFrame geometry, so a repeated A → B → A `will` event is accepted
rather than returned as `.ignoreStale`. Ensure the identity is available before
assigning the local generation, and add a regression test covering the A → B → A
sequence.
🪄 Autofix
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 Plus
Run ID: 3ef7cbc0-ec5c-4cd2-8c3d-a7bb3b918fee
📒 Files selected for processing (1)
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
…ggle-animation # Conflicts: # .github/workflows/reload-build.yml
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
azooz2003-bit
added a commit
that referenced
this pull request
Aug 14, 2026
azooz2003-bit
added a commit
that referenced
this pull request
Aug 14, 2026
azooz2003-bit
added a commit
that referenced
this pull request
Aug 26, 2026
The guide-locked rewrite (#10594/#10687) left iOS 27 on a notification seat that consumes the full keyboard notification stream: did-frame disagreement reseats and steady-state tracker re-derivations. iOS 27 misreports keyboard frames outside the will transaction (#10518 recorded this when it quarantined the rebuilt path away from that OS), so those corrections moved a perfectly settled composer bar after toggles - the regression against the #9958/#10006 path that shipped will-only and was rated perfect in dogfood. Restore that contract, scoped so iOS <= 26 behavior is untouched: - iOS 27 seats ignore keyboardDidChangeFrame entirely and skip the steady-state tracker heal in layoutSubviews (attach recovery in didMoveToWindow stays - it fixes real workspace-switch wedges and did not exist on the old path). - Interrupted legs rebase from live presentation frames before the next will leg (the #10006 reversal contract), folding the live dock bottom into the seat constraint and re-deriving clip and wrapper models from that edge before stripping animations. - TerminalKeyboardSeatSelection (replacing the stale, unused TerminalKeyboardDockPathSelection) owns the seat decision table: guide seat on iOS <= 26, will-only notification seat on iOS 27+, full-stream notification seat behind the iOS <= 26 kill switch. - CMUX_UITEST_FORCE_IOS27_KEYBOARD_SEAT (DEBUG) runs the exact iOS 27 path on any simulator OS; the dock probe reports keyboardSeatWillOnly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Aug 26, 2026
The guide-locked rewrite (#10594/#10687) left iOS 27 on a notification seat that consumes the full keyboard notification stream: did-frame disagreement reseats and steady-state tracker re-derivations. iOS 27 misreports keyboard frames outside the will transaction (#10518 recorded this when it quarantined the rebuilt path away from that OS), so those corrections moved a perfectly settled composer bar after toggles - the regression against the #9958/#10006 path that shipped will-only and was rated perfect in dogfood. Restore that contract, scoped so iOS <= 26 behavior is untouched: - iOS 27 seats ignore keyboardDidChangeFrame entirely and skip the steady-state tracker heal in layoutSubviews (attach recovery in didMoveToWindow stays - it fixes real workspace-switch wedges and did not exist on the old path). - Interrupted legs rebase from live presentation frames before the next will leg (the #10006 reversal contract), folding the live dock bottom into the seat constraint and re-deriving clip and wrapper models from that edge before stripping animations. - TerminalKeyboardSeatSelection (replacing the stale, unused TerminalKeyboardDockPathSelection) owns the seat decision table: guide seat on iOS <= 26, will-only notification seat on iOS 27+, full-stream notification seat behind the iOS <= 26 kill switch. - CMUX_UITEST_FORCE_IOS27_KEYBOARD_SEAT (DEBUG) runs the exact iOS 27 path on any simulator OS; the dock probe reports keyboardSeatWillOnly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Aug 26, 2026
…10810) * fix(ios): unbreak the iOS build after the workspace-groups merge Two iOS-only compile errors from #10662 (the Mac target builds; both break every iOS build, so main's iOS lane is red): - MobileWorkspaceAggregation assigns a MobileWorkspaceGroupPreview.ID to anchorWorkspaceID, which is typed MobileWorkspacePreview.ID. Convert through rawValue, the same mapping the type's own initializer uses for exactly this empty-header fallback. - WorkspaceListTableCoordinator.groupActionCapabilities(for:) is fileprivate but called from the context-menu actions extension in WorkspaceListTableCoordinator+Actions.swift, a different file. Make it internal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ios): iOS 27 will-only keyboard seat must keep a settled dock still Red half of the regression pair. Forces the iOS 27 keyboard seat via CMUX_UITEST_FORCE_IOS27_KEYBOARD_SEAT and asserts the seat reports the will-only contract, a settled composer bar does not move without a will notification, and rapid reversals keep dock and render pinned. Red today because the force env and the will-only seat do not exist: iOS 27 hosts consume the full notification stream, whose non-will frames misreport on that OS and hop the settled dock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS 27: restore the will-only keyboard seat contract The guide-locked rewrite (#10594/#10687) left iOS 27 on a notification seat that consumes the full keyboard notification stream: did-frame disagreement reseats and steady-state tracker re-derivations. iOS 27 misreports keyboard frames outside the will transaction (#10518 recorded this when it quarantined the rebuilt path away from that OS), so those corrections moved a perfectly settled composer bar after toggles - the regression against the #9958/#10006 path that shipped will-only and was rated perfect in dogfood. Restore that contract, scoped so iOS <= 26 behavior is untouched: - iOS 27 seats ignore keyboardDidChangeFrame entirely and skip the steady-state tracker heal in layoutSubviews (attach recovery in didMoveToWindow stays - it fixes real workspace-switch wedges and did not exist on the old path). - Interrupted legs rebase from live presentation frames before the next will leg (the #10006 reversal contract), folding the live dock bottom into the seat constraint and re-deriving clip and wrapper models from that edge before stripping animations. - TerminalKeyboardSeatSelection (replacing the stale, unused TerminalKeyboardDockPathSelection) owns the seat decision table: guide seat on iOS <= 26, will-only notification seat on iOS 27+, full-stream notification seat behind the iOS <= 26 kill switch. - CMUX_UITEST_FORCE_IOS27_KEYBOARD_SEAT (DEBUG) runs the exact iOS 27 path on any simulator OS; the dock probe reports keyboardSeatWillOnly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review: poll the settled dock over a window; document the attach-heal trust tradeoff CodeRabbit findings on #10810: the post-settle stability check now samples the dock edge five times over 1.5s and requires every sample at the settled edge (a transient hop between two single samples could previously pass), and healKeyboardModelFromTracker documents why the will-only seat keeps ATTACH recovery despite the tracker recording did frames iOS 27 can misreport (settled end frame at attach, next will leg corrects a bad record, and skipping recovery wedges the dock after workspace switches mid-transition). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Merged
1 task
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.
Fixes the keyboard snapping upward after successive toggle taps on iOS 27.
The notification-driven dock now pairs keyboard frame transition legs by geometry and generation. A matching
didChangeFramesettles the animation without replacing it with a zero-duration layout, while stale completions cannot overwrite the newest target. An unmatched completion can converge only before this observer has seen a transition.Regression coverage posts an interrupted hide/show notification sequence against the real terminal surface.
Verification in progress:
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stops iOS 27 keyboard re-open snaps by animating the dock and terminal together in a host view and freezing renderer geometry until settle. Previously
keyboardLayoutGuidecould lag and reversals replaced in‑flight animations; now iOS 27 uses notification-driven transitions bound to a generation, rebases only when a prior leg is active, and keeps the rendered terminal edge pinned to the dock.CmuxMobileTerminal: AddsGhosttySurfaceHostViewto own terminal clipping, dock placement, and keyboard motion; uses notifications on iOS 27 and the system guide elsewhere; freezes the renderer during animation and folds the settled edge into the model; rebases from live frames before reversals; exposes host hooks (beginHostedKeyboardTransition,finishHostedKeyboardTransition,updateHostedKeyboardLayoutGuide); samples terminal–dock presentation gap; defers geometry sync while keyboard presentation is active; propagates theme background to the host.CmuxMobileSupport: ExtendsMobileKeyboardTransitionwithbeginFrameandbeginOverlap(...)to pair legs and reject superseded completions.CmuxMobileTerminal: Fences verified replay by draining pending native/local scroll and waiting for local apply; updates scroll paths to carry an interaction generation.CmuxMobileTerminalKit: Adds.requestVisibleFocusto reassert UIKit focus without reloading input views, fixing the iOS 27 toggle focus handoff.CmuxMobileShellUI: Defers verified replay reveal until restore/scroll require re-fencing; binds toast settings state.ONLY_ACTIVE_ARCH=YES.Written for commit 76728bd. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests