Show quit confirmation dialog when sites are running #2314

bcotrim · 2025-12-24T15:34:58Z

Related issues

Fixes STU-1112

Proposed Changes

Add quit confirmation dialog when user quits Studio with running sites
Dialog offers two options: "Stop sites" or "Leave running"
"Remember my choice" checkbox saves preference to skip dialog on future quits
Store preference as stopSitesOnQuit boolean in user data (undefined = show dialog, true = stop, false = leave running)
Add getRunningSiteCount() helper function to track running sites
Conditionally stop servers on quit based on user preference

Testing Instructions

Happy Path

Start one or more sites, then quit Studio (Cmd+Q or close window)
Verify modal appears with "Stop sites" and "Leave running" buttons
Click "Stop sites" → verify sites stop, then app quits
Click "Leave running" → verify app quits but sites continue running (check with studio site list)

Remember Choice

Start a site, quit Studio, check "Remember my choice", click "Stop sites"
Start a site again, quit Studio → verify no dialog appears and sites stop
To reset: manually delete stopSitesOnQuit from ~/Library/Application Support/WordPress Studio/appdata-v1.json

Edge Cases

Quit with no running sites → no dialog should appear
CMD+Q keyboard shortcut respects modal behavior

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

…quit

katinthehatsite · 2025-12-29T09:52:19Z

src/index.ts

+						runningSiteCount
+					),
+					buttons: [ __( 'Stop sites' ), __( 'Leave running' ) ],
+					checkboxLabel: __( 'Remember my choice' ),


Should we use something lile "Don't ask again" to be consistent with other checkboxes around the app?

I see that actually we have default fallback as Don't show this warning again. I would propose change teh default value to Don't ask again and remove checkboxLabel here and in src/components/content-tab-import-export.tsx

I think I am generally okay to keep Don't show this warning again as default as well. I think the default wording could be either of those but I don't think we should introduce another option such as Remember my choice. I think we should stick to the same wording everywhere across the app.

I changed it to Don't ask again for consistency with other dialogs.
My original reasoning for Remember my choice was to make it explicit that we'd save and use that choice in the future.

I see that actually we have default fallback as Don't show this warning again. I would propose change teh default value to Don't ask again and remove checkboxLabel here and in src/components/content-tab-import-export.tsx

I agree we should standardize the checkbox across the app. To avoid increasing the scope of this PR, I propose we do that refactor in a follow-up after this one is merged.

katinthehatsite · 2025-12-29T10:15:08Z

src/index.ts

+					checkboxLabel: __( 'Remember my choice' ),
+					cancelId: LEAVE_RUNNING_BUTTON_INDEX,
+					defaultId: STOP_SITES_BUTTON_INDEX,
+				} );


I think it looks fine but I am wondering about the scenario where the user might change their mind and for example, might want to stop some sites manually. Should not they have an option to close this disalog without picking anything?

Also, if we go with adding this option, let's ensure that the Esc button just closes the popup, since now it applies "closing app w/o turning off sites", which is IMO a bit unexpected.

+1 to this one, I think we should provide some way for the user to get out of this screen.

Thanks for the suggestions, I added a Cancel option, that is also triggered by Esc

fredrikekelund

I agree with the points made by @katinthehatsite. Functionality-wise, this is good, though 👍

E2E tests are currently broken for other reasons, too, but I believe we'll need to add some explicit handling for E2E test cases here, too. I.e., fall back to the previous logic in the E2E test context and don't display this modal.

fredrikekelund · 2026-01-05T10:13:12Z

src/index.ts

+let isQuittingConfirmed = false;
+let shouldStopSitesOnQuit = true;


Optional, but I would consider moving these variable declarations inside appBoot, closer to where they're actually used.

fredrikekelund · 2026-01-05T10:18:11Z

src/tests/site-server.test.ts

This is fine, but I'm assuming the AI wrote these tests, and I'm starting to question the value of tests written entirely by AI. This is a very testable function, so the tests are easy to read, but I know I'll be trying the inverse approach soon: TDD and letting the AI write the implementation based on my tests.

nightnei

Thanks for addressing the changes

I agree we should standardize the checkbox across the app. To avoid increasing the scope of this PR, I propose we do that refactor in a follow-up after this one is merged.

Makes sense 👍

Functionality-wise, "Stop sites" doesn't work for me. Could you please double-test it? And in appdata-v1.json I still see "autoStart": true,, maybe it my local issue or maybe it's due to runnign Studiio via npm start instead of builded version 🤔

fredrikekelund · 2026-01-06T08:43:51Z

And in appdata-v1.json I still see "autoStart": true

I believe this is what we want. Whether or not we stop the servers when quitting Studio has more to do with the CLI than with changing Studio's existing behavior. We fully expect that most users won't work with the CLI, so the fact that the new "default" is the same as the old default, just gated by a button in this modal, makes a lot of sense to me.

fredrikekelund · 2026-01-06T08:46:20Z

@bcotrim, something struck me just now: do you think there's a case to be made for not displaying this modal if the user hasn't installed/activated the Studio CLI? The reason for this change is really that users might have started sites using the CLI, and they might not expect them to be stopped just because they close Studio.

katinthehatsite · 2026-01-06T10:00:23Z

Functionality-wise, "Stop sites" doesn't work for me. Could you please double-test it? And in appdata-v1.json I still see "autoStart": true,, maybe it my local issue or maybe it's due to runnign Studiio via npm start instead of builded versio

This seems to be working as expected for me - the sites were stopped one by one when I chose that option.

On my end, things look good 👍 I will approve later once other comments are addressed.

bcotrim · 2026-01-06T10:43:23Z

@bcotrim, something struck me just now: do you think there's a case to be made for not displaying this modal if the user hasn't installed/activated the Studio CLI? The reason for this change is really that users might have started sites using the CLI, and they might not expect them to be stopped just because they close Studio.

That makes sense, added in 35e4b2d

Functionality-wise, "Stop sites" doesn't work for me. Could you please double-test it? And in appdata-v1.json I still see "autoStart": true,, maybe it my local issue or maybe it's due to runnign Studiio via npm start instead of builded version 🤔

I couldn't replicate the issue, with CLI installed or not. All tests were done running npm start. Could you check again and share more details, please? Do you see anything in the logs?

fredrikekelund

LGTM 👍 It takes a while for all sites to stop after quitting the app and choosing Stop sites (maybe that's what @nightnei observed in #2314 (review)). This PR doesn't make a difference on that front, but it's good to be aware of.

#2299 has a small change to how sites are stopped (the site stop-all child process is forked in detached mode). It won't make a difference to performance, but again, good to be aware of.

stop sites on exit dialog

8139cd0

bcotrim self-assigned this Dec 24, 2025

bcotrim requested review from a team and fredrikekelund December 24, 2025 15:37

bcotrim added the Reviewer can merge label Dec 24, 2025

Merge dev/studio-cli-i2 into stu-1112-improve-feedback-running-sites-…

1b74ae7

…quit

katinthehatsite reviewed Dec 29, 2025

View reviewed changes

fredrikekelund reviewed Jan 5, 2026

View reviewed changes

cancel option, consistent message and e2e check

f6cada3

bcotrim requested review from fredrikekelund, katinthehatsite and nightnei January 5, 2026 11:42

nightnei requested changes Jan 5, 2026

View reviewed changes

conditional dialog when studio cli is installed

35e4b2d

fredrikekelund approved these changes Jan 6, 2026

View reviewed changes

		let isQuittingConfirmed = false;
		let shouldStopSitesOnQuit = true;

Show quit confirmation dialog when sites are running #2314

Are you sure you want to change the base?

Show quit confirmation dialog when sites are running #2314

Conversation

bcotrim commented Dec 24, 2025

Related issues

Proposed Changes

Testing Instructions

Happy Path

Remember Choice

Edge Cases

Pre-merge Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nightnei left a comment

Choose a reason for hiding this comment

Uh oh!

fredrikekelund commented Jan 6, 2026

Uh oh!

fredrikekelund commented Jan 6, 2026

Uh oh!

katinthehatsite commented Jan 6, 2026

Uh oh!

bcotrim commented Jan 6, 2026

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants