feat(terminal): add Recover Terminal Display command (#1472) - #1473
Merged
Conversation
A terminal pane can go black while its shell keeps running, and no input brings it back. SwiftTerm's Metal renderer refuses a frame when the frame semaphore is held or no drawable exists; both paths set `pendingRedraw` without submitting a command buffer, yet only that command buffer's completion handler consumes the flag. The view then accepts input and PTY output forever without presenting anything. Pine cannot detect this from the outside — with the semaphore held, both `setNeedsDisplay` and `drawMetalFrameNow()` re-enter the same refusal, and there is no public "frame presented" signal. Rebuilding the renderer is the only escape: it installs a fresh MTKView, semaphore, and drawable chain while Terminal, the PTY, and the scrollback stay untouched. Terminal ▸ Recover Terminal Display (⌘⌥R) rebuilds the renderer for the active tab of every visible terminal pane, plus the quick terminal when it is on screen. Background tabs are skipped — they are detached, so a rebuild would cost a GPU round-trip for pixels nobody sees, and their own re-attach already repaints them. The item is never disabled: the quick terminal lives in its own window where `focusedProject` is nil, and gating on project panes would leave exactly that surface unfixable. Existing coverage missed this class entirely — `TerminalMetalRendererTests` exercises only attachment-shaped events, and the UI-test harness launches with `--disable-metal`. The durable fix belongs upstream in SwiftTerm; it will be filed separately, after which this command stays useful but no longer load-bearing.
Owner
Author
|
Upstream fix filed: migueldeicaza/SwiftTerm#642 — retries frames refused by Once that lands and Pine bumps SwiftTerm, this command stays useful as a manual escape hatch but is no longer load-bearing. |
Contributor
✅ Code Coverage: 79.2424%Threshold: 70% Logic-only lines: 78656/99260 Coverage is at or above the required threshold. Generated by CI — see job summary for detailed file-level breakdown. |
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
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.
Closes #1472
Problem
A terminal pane goes black while its shell keeps running, and nothing brings it back — typing included. Recreating the tab or relaunching Pine is the only way out.
Root cause is in SwiftTerm 1.18.0's
MetalTerminalRenderer.draw(in:). Two paths refuse a frame:currentDrawable/currentRenderPassDescriptorisnil.Both call
markPendingRedraw()and return without submitting a command buffer — whilependingRedrawis consumed only insidecommandBuffer.addCompletedHandler, i.e. only when a frame was submitted. The pending request ends up with no consumer, and the renderer goes quiet for good. WithisPaused = true/enableSetNeedsDisplay = true/autoResizeDrawable = false, nothing re-drives it.Input does not help: key events land on
metalView.setNeedsDisplay, which re-enters the same refusal. Pine's ownrequestRendererDisplay()is reachable only from attachment-shaped events (viewDidMoveToWindow, host rebind, occlusion, backing properties) and a one-shot first-visible-content hook, so ordinary PTY output after that first chunk never routes through recovery.Verified on a live session: every child shell was alive on its own tty while the pane showed nothing — the PTY had started fine and only presentation was lost.
Why a command and not automatic recovery
Pine cannot detect the stuck state from outside. With the semaphore held, both
setNeedsDisplayanddrawMetalFrameNow()hit the same refusal, and SwiftTerm exposes no "frame presented" signal to poll. Rebuilding the renderer is the only escape — a freshMTKView, semaphore, and drawable chain, whileTerminal, the PTY, and the scrollback stay untouched.The durable fix belongs upstream (re-request the dropped frame from the refusal paths themselves instead of from a completion handler that will never run) and will be filed separately. This command stays useful afterwards, just no longer load-bearing.
Change
Terminal ▸ Recover Terminal Display (⌘⌥R)
PineTerminalView.recoverRendererNow()— rebuilds the Metal renderer via the supportedsetUseMetal(false/true)pair and re-arms the bounded first-frame retry batch, since a freshCAMetalLayercan miss its first drawable exactly like one created on attach. Under CoreGraphics (no such trap) it is a single repaint through the existing backend-aware bridge. No-op while detached.TerminalTab.recoverDisplay()— rebuild, thenforceFullRedraw(), then SIGWINCH. The signal is raised unconditionally rather than only for the alternate screen: a stuck renderer leaves Pine unable to tell whether the primary buffer still matches what the child last drew, and an ordinary shell merely reprints its prompt.TerminalManager.recoverVisibleTerminalDisplays()— the active tab of every terminal pane. Background tabs are skipped: they are detached, so a rebuild would cost a GPU round-trip for pixels nobody sees, and their own re-attach already repaints them. All panes rather than just the focused one — the stuck pane is frequently not the focused one.QuickTerminalController.recoverDisplay()— the panel is a separate window wherefocusedProjectis nil, so the menu item is deliberately never disabled; gating on project panes would leave exactly that surface unfixable.Tests
13 new tests (
TerminalDisplayRecoveryTests, plus two inQuickTerminalTests), covering the negative and edge paths as well as the happy one:MTKViewreally appears (asserted as a new identity, since SwiftTerm defers removing the outgoing view until the replacement has drawn) and the buffer survives; five consecutive recoveries stay stable and losslessExisting coverage missed this class entirely:
TerminalMetalRendererTestsexercises only attachment-shaped events, and the UI-test harness launches with--disable-metal, so the Metal path has no end-to-end coverage.Verification
xcodebuild build— succeedsTerminalDisplayRecoveryTests,TerminalMetalRendererTests,QuickTerminalTests,PineAppMenuCommandsTests— all pass, with Metal genuinely available on the dev machine (the renderer-rebuild assertions really ran rather than being skipped)Not yet done: the in-window visual check of the menu item and shortcut. It needs a foreground app instance, and a working Pine with live agent sessions was running on the machine — UI tests launch with
--reset-state, which clears saved project sessions in the standard domain, so running them was not worth the risk. Worth a quick manual pass before merge.