Don't prompt to restart Extension Terminal on shutdown (#4101, #4145)#5520
Don't prompt to restart Extension Terminal on shutdown (#4101, #4145)#5520andyleejordan wants to merge 1 commit into
Conversation
The "PowerShell Extension Terminal has stopped, would you like to restart it?" prompt was firing in two unwanted scenarios: - #4101: When VS Code itself is closing or reloading the window, it disposes our terminal, which fires `onDidCloseTerminal` -> `PowerShellProcess.dispose()` -> `onExited` -> `promptForRestart()`. Because that can happen before the extension's `deactivate()` runs, a disposal/status check alone isn't enough. We now capture the new-ish `TerminalExitReason` (finalized in VS Code 1.71) when the terminal closes and bail out of the prompt when the reason is `Shutdown`. We also set an `isDisposing` flag in `dispose()` as a belt-and-suspenders guard for the deactivation path. - #4145: Nothing stopped the prompt from appearing more than once concurrently. We now guard it behind an `isPromptingForRestart` boolean wrapped in try/finally so at most one is open at a time. The existing `suppressTerminalStoppedNotification` behavior is preserved, and we still prompt on a genuine user-close or process crash. Added tests under `test/core/session.test.ts` covering all five paths. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes two related bugs with the "The PowerShell Extension Terminal has stopped, would you like to restart it?" prompt. It suppresses the prompt during VS Code shutdown/reload (issue #4101) by checking the terminal's TerminalExitReason and an isDisposing flag, and prevents duplicate concurrent prompts (issue #4145) with an isPromptingForRestart guard.
Changes:
- Captures
TerminalExitReasoninPowerShellProcesswhen the Extension Terminal closes and passes it through topromptForRestart, which early-returns onShutdownorisDisposing. - Guards
promptForRestartwithisPromptingForRestart(try/finally) so at most one prompt is shown at a time. - Adds 5 targeted tests covering user-close prompts, shutdown suppression, dispose suppression, concurrent deduplication, and the suppress-notification setting.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/process.ts |
Adds public terminalExitReason property, captured in onTerminalClose before dispose() fires onExited. |
src/session.ts |
Adds isDisposing and isPromptingForRestart guards; promptForRestart now accepts an optional TerminalExitReason and early-returns when the window is shutting down, the manager is disposing, or a prompt is already open. |
test/core/session.test.ts |
Adds a new describe("SessionManager restart prompt") block with 5 tests, a createSessionManager helper, and a promptForRestart test wrapper. |
|
Closing as redundant. On investigation, both #4101 and #4145 were already addressed by #5380 and #5463 (the April termination-experience rework): the double prompt is suppressed by the 🤖 Drafted by Copilot for @andyleejordan to review/edit. |
|
(But I did like the approach with the terminal closed reasons...didn't know that was a thing.) |
Summary
Fixes #4101 and #4145, two related bugs with the "The PowerShell Extension Terminal has stopped, would you like to restart it?" prompt.
onDidCloseTerminal→PowerShellProcess.dispose()→onExited→promptForRestart(). Since that can happen before the extension'sdeactivate()runs, a status/disposal check alone misses it. We now capture the terminal'sTerminalExitReason(finalized in VS Code 1.71) and bail out of the prompt when the reason isShutdown, plus set anisDisposingflag indispose()as a belt-and-suspenders guard.isPromptingForRestartguard (try/finally) so at most one is open at a time.We still prompt on a genuine user-close (
User) or process crash (Process), and the existingpowershell.integratedConsole.suppressTerminalStoppedNotificationbehavior is preserved.Changes
src/process.ts: captureterminal.exitStatus?.reasonon close, exposed asterminalExitReason.src/session.ts: pass the exit reason intopromptForRestart(); early-return onisDisposingorShutdown(During closing VSCode, PowerShell Extension displays popup #4101) and dedupe withisPromptingForRestart(Double prompt when closing powershell extension terminal #4145).test/core/session.test.ts: 5 new tests (user-close prompts; suppressed while disposing; suppressed on shutdown; deduped concurrent prompts; respects the suppress setting).Verification
npm run compile,npm run lint,npm run format— clean.vscode-testsuite: 109 passing, 4 pending, including the 5 new tests.