Recover iOS terminal render pipeline stalls - #7098
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:
📝 WalkthroughWalkthroughAdds generation-aware render recovery and replay-barrier handling for terminal output, updates UI callbacks to report resets and request replay, switches visible snapshot reads to async, and expands tests and liveness support for replay recovery scenarios. ChangesRender recovery and replay barrier
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swift`:
- Around line 426-430: The render-pipeline reset callback in
ghosttySurfaceViewDidResetRenderPipeline is forwarding replay based only on
surfaceID, which can let a late callback from an old GhosttySurfaceView trigger
recovery for the current owner. Add a guard at the start of the callback to
verify self.surfaceView is still the same instance as the passed surfaceView
before creating the Task and calling terminalOutputNeedsReplay(surfaceID:). Keep
the replay request tied to the active authoritative surface view in
GhosttySurfaceRepresentable.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 2277-2282: Replace the `Task.sleep`-based recovery in
`GhosttySurfaceView` with a real, cancellation-aware deadline owned by the
current surface generation. Update the timeout logic around
`outputApplyTimeoutNanoseconds` and the related `Task`/deadline handling so
slow-but-valid applies do not tear down the surface prematurely; use the
existing generation/state coordination points in `GhosttySurfaceView` and the
apply/rebuild path instead of a sleeping task on the main actor. Also remove the
matching sleep-based stall recovery near the other referenced timeout block so
both paths share the same scheduler/signal-driven mechanism.
- Around line 2678-2679: The reset flow is requesting terminal replay twice
because `initializeSurface()` is followed by
`ghosttySurfaceViewDidResetRenderPipeline(_:)`, while
`GhosttySurfaceRepresentable.attach(...)` already handles `false` from the
`*AndWait` recovery path via `terminalOutputDidReset(...)` and
`requestTerminalReplay(...)`. Update `GhosttySurfaceView` so only one recovery
path is authoritative: either keep the delegate callback or the `false`-return
handling, but not both, and gate `ghosttySurfaceViewDidResetRenderPipeline(_:)`
so it does not fire for recoveries that already bubble back through
`attach(...)` and `terminalOutputDidReset(...)`.
- Around line 3783-3798: The visible snapshot loop in GhosttySurfaceView uses a
single shared DispatchTime deadline, so later items are measured against an
already-expired timeout. Move the timeout calculation inside the pending/item
loop in the visible snapshot path so each queue wait gets its own fresh 600 ms
budget, and keep the rest of the done/holder handling unchanged.
🪄 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: f1bc0d35-1893-4ee8-90ad-82f49e6a1a31
📒 Files selected for processing (9)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileTerminalOutputSinking.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceRegistry.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceWorkQueue.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/SurfaceOperationWaiter.swift
Contributor
Greptile SummaryThis PR adds a bounded, rate-limited self-healing layer around the iOS terminal render pipeline. When libghostty's output or geometry application stalls (detected via display-link deadline), the view tears down the wedged surface, creates a fresh per-surface
Confidence Score: 5/5The recovery path is carefully bounded: one pending surface free at a time, two RPC retries per barrier, one follow-up replay per surface — no unbounded loops or silent discards. The core invariants (generation checks guard stale queue callbacks, GhosttySurfaceView.swift and MobileShellComposite.swift are the two largest files and carry the most new state; reviewers should pay particular attention to recoverRenderPipeline and the ten new barrier dictionaries when returning to maintain this code. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant DL as DisplayLink (handleDisplayLinkFire)
participant GSV as GhosttySurfaceView (@MainActor)
participant OQ as GhosttySurfaceWorkQueue (serial BG)
participant REP as GhosttySurfaceRepresentable
participant MSC as MobileShellComposite (@MainActor)
participant MAC as Mac RPC
Note over GSV: renderInFlight = true (render_now dispatched)
DL->>GSV: handleDisplayLinkFire()
GSV->>GSV: checkSurfaceOperationDeadlines(now)
GSV->>GSV: "renderInFlightSince stall detected (>=2s)"
GSV->>GSV: recoverRenderPipeline(replay:.delegateWhenNoCaller)
GSV->>OQ: enqueueSurfaceFree(oldSurface) [old queue]
GSV->>GSV: "surfaceGeneration += 1, new outputQueue, initializeSurface()"
GSV->>REP: ghosttySurfaceViewDidResetRenderPipeline()
REP->>MSC: terminalOutputNeedsReplay(surfaceID)
MSC->>MSC: beginTerminalReplayBarrier → drops live output
MSC->>MAC: mobile.terminal.replay RPC
MAC-->>MSC: replay response (renderGrid or bytes)
MSC->>MSC: deliverTerminalRenderGrid(bypassReplayBarrier:true)
MSC-->>REP: AsyncStream yields replay chunk
REP->>GSV: processOutputAndWait(chunk.data) → true
REP->>MSC: terminalOutputDidProcess → barrier cleared
Note over MSC: Live output resumes
Note over GSV,OQ: Old queue render_now completes
OQ-->>GSV: "ghostty_surface_free(oldSurface) → pendingSurfaceFreeCount -= 1"
%%{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 DL as DisplayLink (handleDisplayLinkFire)
participant GSV as GhosttySurfaceView (@MainActor)
participant OQ as GhosttySurfaceWorkQueue (serial BG)
participant REP as GhosttySurfaceRepresentable
participant MSC as MobileShellComposite (@MainActor)
participant MAC as Mac RPC
Note over GSV: renderInFlight = true (render_now dispatched)
DL->>GSV: handleDisplayLinkFire()
GSV->>GSV: checkSurfaceOperationDeadlines(now)
GSV->>GSV: "renderInFlightSince stall detected (>=2s)"
GSV->>GSV: recoverRenderPipeline(replay:.delegateWhenNoCaller)
GSV->>OQ: enqueueSurfaceFree(oldSurface) [old queue]
GSV->>GSV: "surfaceGeneration += 1, new outputQueue, initializeSurface()"
GSV->>REP: ghosttySurfaceViewDidResetRenderPipeline()
REP->>MSC: terminalOutputNeedsReplay(surfaceID)
MSC->>MSC: beginTerminalReplayBarrier → drops live output
MSC->>MAC: mobile.terminal.replay RPC
MAC-->>MSC: replay response (renderGrid or bytes)
MSC->>MSC: deliverTerminalRenderGrid(bypassReplayBarrier:true)
MSC-->>REP: AsyncStream yields replay chunk
REP->>GSV: processOutputAndWait(chunk.data) → true
REP->>MSC: terminalOutputDidProcess → barrier cleared
Note over MSC: Live output resumes
Note over GSV,OQ: Old queue render_now completes
OQ-->>GSV: "ghostty_surface_free(oldSurface) → pendingSurfaceFreeCount -= 1"
Reviews (13): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swift`:
- Around line 138-152: The new replay assertions are relying on pollUntil, but
that helper still uses Task.sleep and makes the test depend on real time. Update
the test support in MobileShellRenderGridLivenessTestSupport and the
TerminalOutputDeliveryQueueTests flow to wait on a real signal instead, such as
a router event/continuation for the replay request or advancing the existing
fake clock, so the assertions in terminal output replay/reset paths no longer
use wall-clock polling.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 3878-3893: `VisibleSnapshotRequest` and `visibleSnapshotSection`
are using a stale raw surface pointer without validating it against the current
surface generation. Capture `surfaceGeneration` when building each request in
`visibleTerminalSnapshot`, then in `visibleSnapshotSection` verify `view.surface
=== surface` and that the generation still matches before enqueuing the read on
`outputQueue`; if either check fails, return nil/fail closed so an outdated
surface is never read after an await.
- Around line 3992-4029: Mark the helper payload types in
GhosttySurfaceView.swift as nonisolated so they are not implicitly tied to
`@MainActor` isolation. Update the declarations of RenderPipelineRecoveryReplay,
PendingSurfaceOperation, PendingVisibleSnapshot, VisibleSnapshotRequest, and
VisibleSnapshotRead to be explicitly nonisolated while keeping their stored
properties unchanged. This should ensure these coordination values can safely
cross main-actor and work-queue closures without pulling UI isolation into the
payload types.
🪄 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: c0046640-d73a-4425-870f-aa032426b58d
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (7)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellRenderGridLivenessTestSupport.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Line 6727: The replay failure log in MobileShellComposite.swift is exposing
dynamic error details as public text. Update the CMUX_REPLAY logging around the
mobileShellLog.error call to avoid String(describing: error) being marked
.public; instead mark the error value private or replace it with a stable
non-sensitive error category while keeping surfaceID public. Keep the change
localized to the replay failure path so production logs redact sensitive
upstream/auth context.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TerminalOutputDelivery.swift:
- Around line 140-149: terminalOutputDidReset currently bails out when
terminalReplayBarrierTokensBySurfaceID already has a token, which leaves the
in-flight queue item and ack token stuck. Update
MobileShellComposite+TerminalOutputDelivery’s terminalOutputDidReset flow to
always restart the reset path: clear or replace the existing replay barrier
state, then call beginTerminalReplayBarrier and requestTerminalReplay even when
a barrier is already active. Keep the existing surface/token guards, and ensure
the reset logic drains the queued item so terminalOutputDidProcess can resume
normally.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 2777-2784: The recovery path in recoverRenderPipeline is dropping
the rebuild when pendingSurfaceFreeCount hits Self.maxPendingSurfaceFrees, so a
wedged surface can stay unrecovered; keep a pending recovery request instead of
returning false immediately, and have the free-drain completion trigger the
rebuild for the current surface. Update the logic around renderInFlight,
needsAnotherRender, and the drain callback so the caller is not told replay is
needed until a rebuild is actually scheduled, and make the same change in the
related code path near the reuse/recover handling.
🪄 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: 6e2c1175-c716-4f32-9aea-dd30e9a4a625
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellRenderGridLivenessTestSupport.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalOutputDeliveryQueueTests.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
austinywang
added a commit
that referenced
this pull request
Jul 2, 2026
…values-for Resolved conflicts: - .github/swift-file-length-budget.tsv: regenerated via swift_file_length_budget.py --write-budget - ChatScrollEdgeCoordinator.swift, ChatTranscriptTableView.swift: took origin/main (main superseded this branch's inline #if compiler(>=6.2) glass-API guards with the applyScrollEdgeEffects helper + scroll-momentum work in #7072/#7109; branch predated it) - GhosttySurfaceView.swift: took origin/main and removed the now-orphaned GhosttySurfaceHandle.swift; main's GhosttySurfaceWorkQueue redesign (#7098) supersedes this branch's GhosttySurfaceHandle Sendable-wrapper approach for the same surface-pointer safety concern. Codex transcript payload (resolver realpath fix, service shutdown()/race guard, MobileShellComposite isolated deinit) preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 2, 2026
Merged
This was referenced Jul 8, 2026
This was referenced Jul 15, 2026
4 tasks done
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
Fixes #7093
Root Cause
The iOS input path and render path are separate. Keystrokes travel as input RPCs, but terminal output is delivered through a render/output stream that waits for
GhosttySurfaceView.processOutputAndWaitto finish beforeMobileShellCompositeacknowledges the chunk.If libghostty's local iOS render/output path wedges,
renderInFlightcan stay true orprocessOutputAndWaitcan stop returning. The host session continues to receive input, but the shell delivery queue remainsinFlightand pending output accumulates behind it. That matches the reported behavior: echo gets progressively laggier, the phone stops repainting, and only an app restart clears the local renderer state.Changes
GhosttySurfaceWorkQueueso recovery can abandon a stuck queue and continue on a fresh queue.render_nowself-heals instead of leaving frame production disabled.TerminalOutputDeliveryQueue, invalidate stale stream tokens, and request a replay.terminal.output.pending) next to the existingoq.render.LAGsignal.Tests
arch -arm64 swift test --filter terminalOutputResetDropsStalledBacklogAndInvalidatesOldAcksinPackages/iOS/CmuxMobileShellarch -arm64 swift testinPackages/iOS/CmuxMobileShell(295 tests)scripts/lint-ios-package-conventions.shgit diff --checkI also tried SwiftPM builds for
Packages/iOS/CmuxMobileTerminalandPackages/iOS/CmuxMobileShellUI, but this clone does not currently have a usable rootGhosttyKit.xcframework, so SwiftPM stops before compiling those targets withlocal binary target 'GhosttyKit' ... does not contain a binary artifact. I did not run the macOS DEV build or any reload command per the issue instructions; CI is the iOS compile/build gate for this PR.Localization
No new user-facing UI strings were added. New strings are debug diagnostics / log messages only; no localization catalog changes required.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Stops iOS terminal freezes by detecting stalled Ghostty render/output and self-healing via render‑pipeline rebuilds plus authoritative replay (fixes #7093). Adds bounded, rate‑limited recoveries, caps replay follow‑ups to prevent loops, defers replay until recovery can run, and localizes fallback labels and the debug‑logs menu.
Written for commit 3638376. Summary will update on new commits.
Summary by CodeRabbit