Restart the last terminal when quit is cancelled by lawrencecchen · Pull Request #9492 · manaflow-ai/cmux

lawrencecchen · 2026-08-04T00:14:25Z

Summary

Ctrl+D on the sole terminal always opens the existing “Quit cmux?” decision
Cancel atomically replaces the exited surface with a focused default shell and no prior scrollback
Quit follows complete app termination, including remote-session and notification cleanup
concurrent quit requests join the active decision instead of treating contention as cancellation

Testing

final tagged macOS build: https://github.com/manaflow-ai/cmux/actions/runs/30883506942
terminal recovery unit suite: https://github.com/manaflow-ai/cmux/actions/runs/30882600423
active quit presenter unit suite: https://github.com/manaflow-ai/cmux/actions/runs/30882600357
exact UI regression added at CloseWorkspaceCmdDUITests/testCtrlDOnOnlyTerminalCancelRespawnsThenQuitExitsApp
hosted UI attempts were blocked before test assertions by the runner leaving cmux Running Background: https://github.com/manaflow-ai/cmux/actions/runs/30882992643 and https://github.com/manaflow-ai/cmux/actions/runs/30884258994
Computer Use on final tag cxdq9: Ctrl+D showed the quit sheet; Cancel removed seeded scrollback and old shell environment; a second Ctrl+D plus Quit removed the process and socket

Checklist

prompt is mandatory for sole-terminal Ctrl+D
Cancel creates a clean, focused shell
Quit terminates the app completely
recovery is identity-validated and scoped to the active decision
notifications survive Cancel and clear only after committed close or Quit
no new user-facing strings

coderabbitai · 2026-08-04T00:14:52Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The change restores a fresh terminal when closing the last terminal window is cancelled after a child exit. It propagates a cancellation callback through close handling, validates terminal state before recovery, supports default-shell respawn, and adds regression coverage.

Changes

Terminal child-exit recovery

Layer / File(s)	Summary
Optional terminal respawn command `Sources/Workspace.swift`	`respawnTerminalSurface` accepts an optional command. It launches the default shell for `nil` and rejects explicit blank commands.
Recovery state and close-flow wiring `Sources/TabManager.swift`, `Sources/App/MainWindowController.swift`, `Sources/AppDelegate.swift`	Last-terminal child exits arm a validated recovery callback. Close handling stores, consumes, and invokes the callback when quit confirmation is cancelled.
Cancelled-close regression coverage `cmuxTests/LastTerminalChildExitRecoveryTests.swift`, `cmux.xcodeproj/project.pbxproj`	The test verifies terminal replacement, replay-state cleanup, and preserved workspace and panel counts. The test is added to the Xcode test target.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TabManager
  participant MainWindowController
  participant AppDelegate
  participant Workspace
  TabManager->>AppDelegate: Pass recovery callback to window-close flow
  AppDelegate->>MainWindowController: Store cancellation callback
  MainWindowController->>AppDelegate: Provide callback during close
  AppDelegate->>TabManager: Invoke callback when close is cancelled
  TabManager->>Workspace: Respawn validated terminal surface
  Workspace-->>TabManager: Return replacement surface

Possibly related PRs

manaflow-ai/cmux#8681: Both PRs modify child-exit handling, but this PR adds close-cancellation recovery while #8681 handles abnormal child-exit retention and shell resolution.
manaflow-ai/cmux#9107: Both PRs modify close handling in MainWindowController and AppDelegate, but address different behaviors.

Suggested reviewers: austinywang

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Cmux Architecture Rethink	❌ Error	MainWindowController adds a mutable pendingCloseCancellationAction side channel; if performClose does not reach windowShouldClose, a later close can consume stale recovery state.	Use an AppDelegate-owned close transaction keyed by window and surface identity. Let the AppKit delegate consume that transaction and clear it on every close-attempt outcome.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	All new production callbacks and methods remain inside existing `@MainActor` AppDelegate, TabManager, Workspace, and MainWindowController types; the diff adds no Sendable models, service protocols, o...
Cmux Swift Blocking Runtime	✅ Passed	The production diff adds callback state and identity checks only; no new semaphore, wait, sleep, delayed dispatch, polling, main sync, or manual lock appears in added Swift lines.
Cmux Browser Automation Off-Main	✅ Passed	The feature diff changes only terminal close/recovery files and tests; it does not modify TerminalController, the worker policy/router, or add browser automation code.
Cmux Expensive Synchronous Load	✅ Passed	The PR adds no production expensive loader or large-file parsing. The new close-cancel path only uses cached state and existing respawn logic; existing load fallbacks are unchanged.
Cmux Cache Substitution Correctness	✅ Passed	The diff adds transient close-rollback state, not a persistence/history/snapshot cache substitution; the callback revalidates workspace, panel, and surface identity before respawn.
Cmux No Hacky Sleeps	✅ Passed	The diff changes only Swift files and Xcode test registration; no non-Swift runtime timing, sleep, timer, polling, or wall-clock wait was added.
Cmux Algorithmic Complexity	✅ Passed	The diff adds no loops, sorting, filtering, or batch rescans. Recovery explicitly bounds work to one workspace and one panel, with dictionary panel lookup; the controller uses one non-nested window...
Cmux Swift Concurrency	✅ Passed	The diff adds no new Dispatch, Task, Combine, or internal completion-handler async machinery; its cancellation closures bridge AppKit window-close and alert callbacks.
Cmux Swift `@Concurrent`	✅ Passed	The PR adds or changes only synchronous, `@MainActor-isolated` functions; the patch adds no `@concurrent/nonisolated` async work or UI calls to heavy async helpers.
Cmux Swift Package Boundaries	✅ Passed	The diff adds AppKit/app-lifecycle close wiring and Ghostty/Workspace state recovery; it introduces no reusable domain logic or new package-boundary candidate.
Cmux Swiftpm Lockfiles	✅ Passed	The PR changes no Package.swift, Package.resolved, or .gitignore files, and its project-file edit only registers a test; no SwiftPM package-reference change requires a lockfile diff.
Cmux Swift Logging	✅ Passed	The diff adds no print, debugPrint, dump, NSLog, Logger, stdout, or diagnostic file logging; the only file write is test fixture setup.
Cmux User-Facing Error Privacy	✅ Passed	The PR adds no user-facing error or diagnostic text. It only wires cancellation and recovery callbacks; the existing “Quit cmux?” alert remains generic, and test strings are non-user-facing.
Cmux Full Internationalization	✅ Passed	The production diff adds no user-facing text or localization resources. It reuses the existing localized quit alert, whose five keys have translated entries for all 19 supported locales.
Cmux Swiftui State Layout	✅ Passed	The PR diff adds AppKit close callbacks, terminal recovery, and tests; it adds no SwiftUI views, state wrappers, layout readers, lazy rows, or render-time mutations.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The PR changes only main workspace-window close handling and test wiring; it adds no standalone auxiliary window, and scripts/lint_auxiliary_window_close_shortcuts.py passes.
Cmux Source Artifacts	✅ Passed	The PR changes only Swift source/tests and the Xcode project file; no logs, media, caches, build output, scratch directories, or other artifact paths enter the diff.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The production diff adds no new test/debug-named members or test-build guards; new accessors have production callers, and the recovery method is exercised by product flow and `@testable` tests.
Cmux No Ambient Global State	✅ Passed	The production diff adds scoped instance methods and a private callback property on MainWindowController; it adds no file-scope function, global mutable state, static namespace, or singleton.
Title check	✅ Passed	The title clearly describes the primary change: restarting the last terminal when quit is cancelled.
Description check	✅ Passed	The description includes a clear summary, testing evidence, and verification details, but omits the template's demo video, review trigger, and standard checklist sections.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat-cancel-last-terminal-restart

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

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/TabManagerUnitTests.swift`:
- Around line 823-826: In the test setup around replayFile, add deferred cleanup
immediately after creating the temporary replay file so the file is removed
regardless of setup or assertion failures. Keep the existing write and
ownedSessionScrollbackReplayFileURL assignment 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 Plus

Run ID: fb51c448-43f3-4b0d-8e2a-cd86c60f8a1b

📥 Commits

Reviewing files that changed from the base of the PR and between d35187e and 2b2385c.

📒 Files selected for processing (4)

Sources/AppDelegate.swift
Sources/TabManager.swift
Sources/Workspace.swift
cmuxTests/TabManagerUnitTests.swift

coderabbitai

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/TabManagerUnitTests.swift`:
- Around line 820-825: Update the test around
lastTerminalChildExitRecoveryAction to exercise cancellation through
closeMainWindowContainingTabId, takePendingCloseCancellationAction(), and
handleQuitShortcutWarning(onCancel:) instead of invoking the recovery closure
directly. Trigger the cancelled close transaction through those production
boundaries, then assert that the replacement terminal state is restored
correctly.

🪄 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 Plus

Run ID: 7ec1e891-a07e-48df-8fcb-63dbf0f1c9c4

📥 Commits

Reviewing files that changed from the base of the PR and between 15c2db7 and fab63c8.

📒 Files selected for processing (4)

Sources/App/MainWindowController.swift
Sources/AppDelegate.swift
Sources/TabManager.swift
cmuxTests/TabManagerUnitTests.swift

coderabbitai

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/LastTerminalChildExitRecoveryTests.swift`:
- Around line 26-31: Add a behavior-level regression test alongside the existing
test that invokes the real close flow through closeMainWindowContainingTabId,
including window.performClose(nil) and cancellation of quit confirmation. After
close handling completes, assert the replacement surface, restored focus, and
history-free state; retain the existing direct recovery-closure unit test.

🪄 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 Plus

Run ID: 013199d6-ac3d-485f-8559-346de2d4dbaf

📥 Commits

Reviewing files that changed from the base of the PR and between d156dfc and 120b077.

📒 Files selected for processing (3)

Sources/AppDelegate.swift
cmux.xcodeproj/project.pbxproj
cmuxTests/LastTerminalChildExitRecoveryTests.swift

lawrencecchen added 2 commits August 3, 2026 17:08

Add regression test for cancelled last-terminal close

ef70dff

Restart last terminal when close is cancelled

2b2385c

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread cmuxTests/TabManagerUnitTests.swift Outdated

lawrencecchen added 2 commits August 3, 2026 17:33

Allow quit-cancel flow in debug UI tests

15c2db7

Keep terminal recovery in the close transaction

fab63c8

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread cmuxTests/TabManagerUnitTests.swift Outdated

lawrencecchen added 2 commits August 3, 2026 19:34

Always confirm quit after the last terminal exits

d156dfc

Preserve terminal recovery with window docks

120b077

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread cmuxTests/LastTerminalChildExitRecoveryTests.swift

lawrencecchen added 6 commits August 3, 2026 21:57

Keep quit recovery inside the close request

94b2e71

Document last-terminal quit confirmation invariant

cc3fbbe

Clear close state only after cancellation

54e1b27

Join terminal recovery to active quit decisions

b16764c

Test the last-terminal quit decision end to end

8199964

Preserve notifications until quit commits

6b0a397

lawrencecchen merged commit 7dcb2bf into main Aug 4, 2026
18 checks passed

lawrencecchen deleted the feat-cancel-last-terminal-restart branch August 4, 2026 06:45

austinywang mentioned this pull request Aug 4, 2026

Quit confirmation dialog renders as a washed-out, mispositioned 'ghost' window (regressed reachability from #9492) #9595

Closed

Navigation Menu

Repository navigation

Restart the last terminal when quit is cancelled - #9492

Restart the last terminal when quit is cancelled#9492
lawrencecchen merged 12 commits into
mainfrom
feat-cancel-last-terminal-restart

lawrencecchen commented Aug 4, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Pre-merge checks failed

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lawrencecchen commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Checklist

Uh oh!

coderabbitai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Pre-merge checks failed

❌ Failed checks (1 error)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lawrencecchen commented Aug 4, 2026 •

edited

Loading

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading