Guard close-confirmation alert against re-entrant presentation - #667
Merged
Conversation
Close requests reach presentCloseConfirmation from independent paths (TCA effects, Ghostty callbacks, the tab bar, the CLI socket) while NSAlert.runModal() keeps draining main-actor jobs, so a late request could present a second alert nested inside the first one's run loop. On macOS 27 beta AppKit answers that with an NSException rethrown out of runModal, which Swift cannot catch (SIGABRT, Sentry PROWL-MACOS-FP). Add TerminalCloseConfirmationGate: while a confirmation is on screen, later ones are dropped and treated as cancelled. Claude-Session: https://claude.ai/code/session_01TWSNesKV4dk8HLKmcNEJjS
There was a problem hiding this comment.
🟢 Ready to approve
The change is narrowly scoped, the re-entrancy guard is applied at the shared confirmation entry point, and tests cover the intended gating behavior.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR introduces an app-global main-actor “gate” to prevent re-entrant NSAlert.runModal() close-confirmation prompts from being presented while an existing confirmation is already on screen, addressing a SIGABRT crash caused by nested modal alerts.
Changes:
- Added
TerminalCloseConfirmationGateto drop nested close-confirmation presentations and treat them as cancelled. - Routed
WorktreeTerminalState.presentCloseConfirmationthrough the gate and mapped “dropped” tofalse. - Added Swift Testing coverage for the gate’s basic behavior and re-entrancy protection.
File summaries
| File | Description |
|---|---|
| supacodeTests/TerminalCloseConfirmationGateTests.swift | Adds unit tests validating gating behavior, nested-call dropping, and post-run release. |
| supacode/Features/Terminal/Models/WorktreeTerminalState+Surfaces.swift | Wraps the NSAlert.runModal() confirmation prompt with the new gate and treats drops as cancelled. |
| supacode/Features/Terminal/Models/TerminalCloseConfirmationGate.swift | Introduces the global @MainActor guard that prevents re-entrant close-confirmation presentation. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| @@ -0,0 +1,24 @@ | |||
| import Foundation | |||
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.
Summary
Fixes the SIGABRT crash in Sentry issue PROWL-MACOS-FP (
SIGABRT: Close Terminal Tab? > terminating due to uncaught exception of type NSException).Root cause: close requests reach
presentCloseConfirmationfrom independent paths (TCA effects, Ghostty callbacks, the tab bar, the CLI socket).NSAlert.runModal()keeps draining main-actor jobs while it spins (the modal loop runs in common modes), so a late close request could present a second confirmation nested inside the first one's run loop. On macOS 27 beta, AppKit answers that nested modal with an NSException that is rethrown out ofrunModalinto Swift frames, which cannot catch ObjC exceptions →std::terminate→ SIGABRT.The crash report's OS crash-info showed the on-screen alert was "Close Terminal Tab?" (
.tabtarget) while the crashing stack was presenting the.panealert fromhandleCloseRequest— two different alerts, confirming the nesting.Fix
Add
TerminalCloseConfirmationGate, an app-global@MainActorguard: while a close confirmation is on screen, laterpresentCloseConfirmationcalls are dropped and treated as cancelled (returnfalse). All close paths funnel throughpresentCloseConfirmation, so the gate covers TCA, Ghostty-callback, tab-bar, and CLI triggers alike.Testing
TerminalCloseConfirmationGateTests(3 tests, passing): free-gate execution, nested run dropped, gate released after run.make checkpasses.make build-appsucceeds.https://claude.ai/code/session_01TWSNesKV4dk8HLKmcNEJjS