Repository navigation
Fix #3998: discard input for dead terminal panes - #4045
Closed
austinywang wants to merge 43 commits into
Closed
austinywang wants to merge 43 commits into
austinywang wants to merge 43 commits into
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 implements a terminal input blocking state machine that prevents input forwarding after a child process exits. ChangesChild-exited input blocking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (11 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 |
Contributor
Greptile SummaryThis PR fixes #3998 by introducing a
Confidence Score: 5/5Safe to merge — the input-gating logic is consistent across all paths, the lock is correctly scoped to protect cross-thread access without holding it during Ghostty FFI calls, and the child-exit transition is idempotent. The lifecycle gate, lock usage, and queue-drain shape are sound. Previous thread findings have all been addressed. The two remaining items are style-level observations that do not affect current behavior. GhosttyTerminalView.swift — string-prefix dispatch in inputSendResult/namedKeySendResult couples result types to free-form log strings, and the .sent arm in enqueuePendingSocketKeyIfAllowed is unreachable dead code. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
GhosttyCallback["Ghostty callback thread"]
MarkInputExited["markChildProcessInputExited()\nunder inputLifecycleLock"]
MainQueue["DispatchQueue.main.async\nmarkChildProcessExited()"]
GhosttyCallback --> MarkInputExited
GhosttyCallback --> MainQueue
TabManager["TabManager.closePanelAfterChildExited()"]
TabManager --> MarkInputExited
subgraph InputPaths["All input paths"]
KeyDown["keyDown / keyUp / flagsChanged"]
SendText["sendText / sendInput / sendNamedKey"]
Flush["flushPendingSocketInput"]
end
MarkInputExited --> |"childExited state set"| InputPaths
KeyDown --> |"SurfaceInputReadiness.blocked"| Discard["Discarded"]
SendText --> |"blockReason != nil"| Discard
Flush --> |"drain under lock"| Discard
Reviews (26): 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}
![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
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)
4433-4774:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSynchronize the child-exit gate with queued socket input.
markChildProcessExited(reason:)now flipsinputLifecycleStateand clearspendingSocketInputQueue, but the socket entry points still read/mutate both without any shared isolation. A concurrentsendText/sendNamedKeycan race the exit transition and enqueue or flush one more payload after the pane is already marked dead, which breaks the new discard-after-exit contract.🤖 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 4433 - 4774, The markChildProcessExited race happens because inputLifecycleState and pendingSocketInputQueue/pendingSocketInputBytes are accessed without synchronization; fix by protecting all reads/writes of inputLifecycleState and pendingSocketInputQueue/pendingSocketInputBytes with a single lock (either reuse debugMetadataLock or add a dedicated NSLock), updating markChildProcessExited(reason:), terminalInputBlockReason(), shouldForwardTerminalInput(reason:), discardPendingSocketInput(reason:) and any socket-entry points (e.g. sendText/sendNamedKey equivalents) to acquire/release that lock around checks and mutations so the transition to .childExited and the clearing of the queues is atomic and prevents post-exit enqueues.
🤖 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 6868-6869: The guard treating
terminalSurface?.acceptsTerminalInput == false the same as a missing surface
must return an explicit "blocked" state instead of nil so callers can silently
consume input; update the code around the guard (the check on
terminalSurface?.acceptsTerminalInput and the subsequent if let surface =
surface branch) to return a distinct .blocked enum/case (or a distinct value)
when the surface exists but acceptsTerminalInput is false, leave nil only for
the truly missing/unattached surface, and adjust callers like keyDown(with:) and
requestInputRecoveryAfterSurfaceMiss(reason:) to consume .blocked without
calling requestInputRecoveryAfterSurfaceMiss or super.keyDown(with:). Ensure you
reference the enum/case you introduce (e.g., .blocked) in the same type
currently returned by the function so downstream logic can branch on .blocked vs
nil.
In `@Sources/TerminalController.swift`:
- Around line 15226-15243: Extract the repeated attached-vs-unattached send
routing into a single `@MainActor` helper that takes the unescaped text and a
TerminalPanel (e.g., routeUnescapedInput(_ text: String, to terminalPanel:
TerminalPanel)); replace each verbatim block in sendInputToWorkspace,
sendInputToSurface, and the current site with calls to this helper; inside the
helper perform the conditional on terminalPanel.surface.surface != nil and call
terminalPanel.surface.sendInput(text) when attached, otherwise call
terminalPanel.sendText(text) and
terminalPanel.surface.requestBackgroundSurfaceStartIfNeeded() so all routing
logic is centralized for future lifecycle/gating changes.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4433-4774: The markChildProcessExited race happens because
inputLifecycleState and pendingSocketInputQueue/pendingSocketInputBytes are
accessed without synchronization; fix by protecting all reads/writes of
inputLifecycleState and pendingSocketInputQueue/pendingSocketInputBytes with a
single lock (either reuse debugMetadataLock or add a dedicated NSLock), updating
markChildProcessExited(reason:), terminalInputBlockReason(),
shouldForwardTerminalInput(reason:), discardPendingSocketInput(reason:) and any
socket-entry points (e.g. sendText/sendNamedKey equivalents) to acquire/release
that lock around checks and mutations so the transition to .childExited and the
clearing of the queues is atomic and prevents post-exit enqueues.
🪄 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: 7c2176be-b72b-4c62-836f-3a8745b3a857
📒 Files selected for processing (4)
Sources/GhosttyTerminalView.swiftSources/TabManager.swiftSources/TerminalController.swiftcmuxTests/GhosttyCommandShiftForwardingTests.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 1624-1654: The helper cmuxZshTerminalKeyboardResetSequence
currently swallows sourcing errors (the "|| true") and never validates the
subprocess exit or output, so it can return empty data; remove the "|| true"
that masks source failures, capture standardError into a Pipe variable, call try
process.run(), waitUntilExit(), then validate process.terminationStatus (or
process.terminationReason) and the output payload and if the process failed or
produced empty output throw an error that includes the captured stderr and/or
stdout to fail fast and surface the integration failure.
🪄 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: b9876028-3d34-4f29-bcdb-bae61df8831f
📒 Files selected for processing (3)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshcmuxTests/CJKIMEInputTests.swift
There was a problem hiding this comment.
Actionable comments posted: 4
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)
4427-4822: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract the terminal-input lifecycle into its own helper/type.
This PR adds several hundred more lines of lifecycle, queueing, and routing logic to a production Swift file that is already well past the repo budget. Please move the new input-state machine/pending-socket logic out of
GhosttyTerminalView.swiftbefore merge.As per coding guidelines, "do not accept more than 250 lines added to an existing production Swift file that is already over 800 lines, unless an extraction exception is met by removing/moving mixed responsibilities and decreasing total line count by more than 200 lines."
Also applies to: 5985-6070, 6944-7009
🤖 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 4427 - 4822, The new terminal-input lifecycle, queueing and routing logic (types PendingSocketInputDiscard, PendingSocketInputQueueSnapshot, PendingSocketInputDrain, TerminalInputLifecycleState, the pendingSocketInputQueue/Bytes/maxPendingSocketInputBytes state, inputLifecycleLock, and all helper methods terminalInputBlockReasonLocked, terminalInputBlockReason, clearPendingSocketInputLocked, logPendingSocketInputDiscard, consumeTerminalInputIfAllowed, markChildProcessExited, withInputLifecycleLock, etc.) should be extracted into a dedicated helper type (e.g. TerminalInputLifecycleManager) in its own Swift file; move the enums/structs and the queue + lock internals into that type, keep synchronization inside it, and expose a narrow API (methods like consumeTerminalInputIfAllowed(reason:), markChildProcessExited(reason:), terminalInputBlockReason(), clearPendingSocketInputIfAny(), and portal/attachment-related setters/getters) so GhosttyTerminalView holds a single instance and delegates calls to it—update all references in GhosttyTerminalView to use the new instance and preserve any `@MainActor` or debug logging calls by forwarding context/IDs as parameters.
🤖 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 `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 1883-1888: Reorder the test so the injected keyboard-reset bytes
from cmuxZshTerminalKeyboardResetSequence() are processed after the
clear-history path completes: call
hostedTerminal.surface.performBindingAction("clear_screen") and
hostedTerminal.surface.forceRefresh(reason: "unit.clearHistory") (and the
subsequent RunLoop.current.run(...)) first, then invoke try
processTerminalOutput(cmuxZshTerminalKeyboardResetSequence(), in:
hostedTerminal) so the reset happens at the fresh prompt and avoids leaving the
runtime surface in stale Kitty mode.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 6976-6987: flagsChanged(with:) currently bypasses the readiness
checks and calls ghostty_surface_key directly; change flagsChanged(with:) to
call ensureSurfaceReadyForInput() and handle the returned SurfaceInputReadiness
the same way as keyDown/keyUp/performKeyEquivalent (i.e., do nothing on
.blocked/.unavailable and only call ghostty_surface_key when the readiness is
.ready(surface)), so modifier-only events are gated behind the same
blocked/unavailable logic as other input paths.
- Around line 4743-4758: The new reader terminalInputBlockReasonLocked() reads
portalLifecycleState under inputLifecycleLock but writers
(beginPortalCloseLifecycle, markPortalLifecycleClosed) still mutate
portalLifecycleState without that lock, causing a race; fix by ensuring all
accesses and mutations of portalLifecycleState are protected by the same
inputLifecycleLock (wrap reads in
terminalInputBlockReasonLocked/terminalInputBlockReason and wrap mutations in
beginPortalCloseLifecycle and markPortalLifecycleClosed, or move those mutations
onto the same synchronization context) so portalLifecycleState is only
read/written while holding inputLifecycleLock.
In `@Sources/TerminalController.swift`:
- Around line 15487-15488: The remaining send paths (e.g., the call to
Self.routeUnescapedInput(unescaped, to: terminalPanel) and the other similar
send exits) currently ignore the helper result so they skip the attached-surface
redraw; change these to capture the helper return value (the object/tuple that
exposes shouldForceRefresh), set success accordingly, and propagate its
shouldForceRefresh into the redraw path so the same refresh preserved by the
logic around lines 6818-6822 is applied; in short, replace the discarded-result
calls to routeUnescapedInput (and the other send paths noted) with assignments
that read .shouldForceRefresh and use that flag to trigger the redraw.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4427-4822: The new terminal-input lifecycle, queueing and routing
logic (types PendingSocketInputDiscard, PendingSocketInputQueueSnapshot,
PendingSocketInputDrain, TerminalInputLifecycleState, the
pendingSocketInputQueue/Bytes/maxPendingSocketInputBytes state,
inputLifecycleLock, and all helper methods terminalInputBlockReasonLocked,
terminalInputBlockReason, clearPendingSocketInputLocked,
logPendingSocketInputDiscard, consumeTerminalInputIfAllowed,
markChildProcessExited, withInputLifecycleLock, etc.) should be extracted into a
dedicated helper type (e.g. TerminalInputLifecycleManager) in its own Swift
file; move the enums/structs and the queue + lock internals into that type, keep
synchronization inside it, and expose a narrow API (methods like
consumeTerminalInputIfAllowed(reason:), markChildProcessExited(reason:),
terminalInputBlockReason(), clearPendingSocketInputIfAny(), and
portal/attachment-related setters/getters) so GhosttyTerminalView holds a single
instance and delegates calls to it—update all references in GhosttyTerminalView
to use the new instance and preserve any `@MainActor` or debug logging calls by
forwarding context/IDs as parameters.
🪄 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: 18460a3f-0ae6-4203-83eb-6eb6af2feba0
📒 Files selected for processing (3)
Sources/GhosttyTerminalView.swiftSources/TerminalController.swiftcmuxTests/CJKIMEInputTests.swift
austinywang
commented
May 13, 2026
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)
5725-5786:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe lifecycle guard still races with the actual Ghostty write.
These paths check
consumeTerminalInputIfAllowed()/ drain the queue underinputLifecycleLock, then release the lock before callingghostty_surface_textorghostty_surface_key. A socket thread that passes the guard just beforemarkChildProcessExited(reason:)flips the state can still deliver a final chunk/key after exit. To fully close this regression, the lifecycle transition and the C write need a single serialization point.Also applies to: 6056-6097
🤖 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 5725 - 5786, The race happens because consumeTerminalInputIfAllowed() (the inputLifecycleLock) is released before the actual C surface writes (ghostty_surface_text / ghostty_surface_key) performed by writeTextData(_:to:) and sendKeyEvent(surface:keycode:mods:), so a thread can pass the guard then lose the race with markChildProcessExited(reason:). Fix by serializing the lifecycle transition and the C write on the same lock: ensure sendText(_:), sendNamedKey(_:), and sendInput(_:) perform the actual calls into writeTextData and sendKeyEvent while still holding the inputLifecycleLock (or refactor writeTextData/sendKeyEvent to acquire that same lock internally), so the guard and the native write are atomic with respect to markChildProcessExited(reason:).
🤖 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 2072-2075: The code currently calls
callbackTerminalSurface?.markChildProcessExited(reason:) inside
DispatchQueue.main.async, leaving inputLifecycleState as .acceptingInput until
the async block runs and allowing a queued keyDown to reach a dead pane; change
this so that the lifecycle mutation is done synchronously: call
markChildProcessExited(reason:) (or otherwise set inputLifecycleState to the
child-exited state) immediately on the current thread before enqueueing the
DispatchQueue.main.async UI-close work, and leave only UI-only operations in the
async block; apply the same synchronous flip in the other identical site
referenced (the block around lines 3764-3766) and ensure
ensureSurfaceReadyForInput() will see the updated state.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 5725-5786: The race happens because
consumeTerminalInputIfAllowed() (the inputLifecycleLock) is released before the
actual C surface writes (ghostty_surface_text / ghostty_surface_key) performed
by writeTextData(_:to:) and sendKeyEvent(surface:keycode:mods:), so a thread can
pass the guard then lose the race with markChildProcessExited(reason:). Fix by
serializing the lifecycle transition and the C write on the same lock: ensure
sendText(_:), sendNamedKey(_:), and sendInput(_:) perform the actual calls into
writeTextData and sendKeyEvent while still holding the inputLifecycleLock (or
refactor writeTextData/sendKeyEvent to acquire that same lock internally), so
the guard and the native write are atomic with respect to
markChildProcessExited(reason:).
🪄 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: 895ebf22-367a-4b8c-860a-e2d4a947a466
📒 Files selected for processing (3)
Sources/GhosttyTerminalView.swiftSources/TerminalController.swiftcmuxTests/CJKIMEInputTests.swift
…arbled-input # Conflicts: # Sources/GhosttyTerminalView.swift
…arbled-input # Conflicts: # cmuxTests/CJKIMEInputTests.swift
There was a problem hiding this comment.
Actionable comments posted: 2
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)
4418-4830: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract the input lifecycle/queue state machine out of this file.
This PR adds a large chunk of non-view lifecycle and socket-queue logic to an already massive production file. Pulling
TerminalInputLifecycleStateplus the pending-socket queue/drain helpers into a dedicated type or companion file would keep the portal/view code reviewable and make this state machine much easier to test in isolation.As per coding guidelines, "Flag Swift changes that add too much unrelated responsibility to one file: do not accept more than 250 lines added to an existing production Swift file that is already over 800 lines..."
Also applies to: 6000-6090
🤖 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 4418 - 4830, Extract the input-lifecycle and pending-socket queue logic into a new type (e.g. TerminalInputLifecycle or TerminalInputQueue) in its own file: move the enums/structs TerminalInputLifecycleState, PendingSocketInputDiscard, PendingSocketInputQueueSnapshot, PendingSocketInputDrain and all related stored state (pendingSocketInputQueue, pendingSocketInputBytes, maxPendingSocketInputBytes, inputLifecycleLock, inputLifecycleState) plus the methods terminalInputBlockReasonLocked, terminalInputBlockReason, clearPendingSocketInputLocked, logPendingSocketInputDiscard, consumeTerminalInputIfAllowed, markChildProcessInputExited, markChildProcessExited and portalBinding helpers into that new type; preserve the locking semantics by making the lock private to the new type and expose thread-safe APIs used by the view (e.g. canAcceptInput(), consumeIfAllowed(reason:), markChildExited(reason:), snapshotQueue()) so call sites in GhosttyTerminalView swap to those APIs and no external code references the moved private symbols or the private vars directly.
🤖 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 6997-7008: NSTextInputClient.insertText currently calls
sendTextToSurface() directly and bypasses the readiness gate; update insertText
to call ensureSurfaceReadyForInput() and only forward text when it returns
.ready(surface) (use the returned surface), otherwise drop or handle the input
consistently with key-event handlers (e.g., treat .blocked/.unavailable as
no-op). Specifically change NSTexInputClient.insertText to consult
ensureSurfaceReadyForInput() before invoking sendTextToSurface(), matching the
behavior around markChildProcessInputExited and preventing ghostty_surface_key
calls when the surface isn't ready.
- Around line 7603-7609: The switch in ensureSurfaceReadyForInput() currently
returns true for the .blocked case which incorrectly consumes AppKit key
equivalents; change the .blocked branch to return false so blocked panes reject
terminal input instead of hijacking menu shortcuts (leave .ready handling as-is
and keep .unavailable returning false). Update the switch in
GhosttyTerminalView.swift (the ensureSurfaceReadyForInput() call/case handling)
so .blocked falls through to allow main menu key equivalents.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4418-4830: Extract the input-lifecycle and pending-socket queue
logic into a new type (e.g. TerminalInputLifecycle or TerminalInputQueue) in its
own file: move the enums/structs TerminalInputLifecycleState,
PendingSocketInputDiscard, PendingSocketInputQueueSnapshot,
PendingSocketInputDrain and all related stored state (pendingSocketInputQueue,
pendingSocketInputBytes, maxPendingSocketInputBytes, inputLifecycleLock,
inputLifecycleState) plus the methods terminalInputBlockReasonLocked,
terminalInputBlockReason, clearPendingSocketInputLocked,
logPendingSocketInputDiscard, consumeTerminalInputIfAllowed,
markChildProcessInputExited, markChildProcessExited and portalBinding helpers
into that new type; preserve the locking semantics by making the lock private to
the new type and expose thread-safe APIs used by the view (e.g.
canAcceptInput(), consumeIfAllowed(reason:), markChildExited(reason:),
snapshotQueue()) so call sites in GhosttyTerminalView swap to those APIs and no
external code references the moved private symbols or the private vars directly.
🪄 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: 6c1ae43f-ef1e-4daa-8a72-7c33ed8c3057
📒 Files selected for processing (3)
Sources/GhosttyTerminalView.swiftSources/TabManager.swiftcmuxTests/CJKIMEInputTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
austinywang
changed the title
…-3998-dead-pty-garbled-input
…-3998-dead-pty-garbled-input # Conflicts: # Sources/GhosttyTerminalView.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bfe100c. Configure here.
lawrencecchen
added
the
stale-revisit
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
3 participants
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
Local reproduction
exitand externalkill -9of the pane shell both auto-closed the workspace/pane on the current debug build before it could be typed into.debug-terminalsreports the focused surface withtty=nil; observed input was still accepted/rendered into the terminal grid instead of being dropped until a live PTY consumer exists.Testing
Note
Medium Risk
Touches terminal input routing, lifecycle, and concurrency around Ghostty surface callbacks; regressions could drop valid input or affect panel close/restore behavior.
Overview
Fixes dead-pane input by introducing an explicit terminal input lifecycle in
GhosttyTerminalView(locked) that flips to child-exited from Ghostty callbacks/close paths, clears queued socket input, and blocks furthersendText/sendInput/sendNamedKeyand view key handling withprocess_exitedsemantics.Also forces Ghostty
wait_after_commandoff (no inheritance and overridden at runtime surface creation) so exited PTYs aren’t kept focusable, and updates bash/zsh shell integration to reset keyboard protocols at every prompt (with a Kitty alias).Adds/updates regression tests covering dead-pane socket + key rejection, suppression of key forwarding to Ghostty, keyboard reset emission correctness, and ensuring startup-command tabs/splits don’t request/enable
wait_after_command.Reviewed by Cursor Bugbot for commit 53bb46d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Stops all input to dead terminal panes and returns
process_exitedfor control‑socket and v2 API commands. Also disableswait_after_commandeverywhere and resets terminal keyboard protocols at each shell prompt to prevent garbled input; aligns with issue-3998.sendText/sendInput/sendNamedKey/paste and viewkeyDown/keyUp/flagsChanged/paste readiness respect ready/blocked/unavailable; dead panes swallow events before Ghostty; skip readiness waits when blocked.surface.send_text/surface.send_keyand legacysendreturnprocess_exitedfor dead panes andsurface_unavailablewhen closed; refresh only when input applies; fix queued vs blocked reporting._cmux_reset_kitty_keyboard_protocolalias; remove duplicate helpers. Tests verify the reset sequence and that typing “chart” emits plain bytes (no Kitty CSI‑u).wait_after_commandglobally: never inherit it and force false at runtime. Tests cover startup‑command tabs/splits, initial workspaces, remote splits, and templates that request it; runtime override verified.Written for commit 53bb46d. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
New Features
Tests