Address review: shared quit type, simpler choice mapping, marker-free…

… migration

apps/studio/src/index.ts

@@ -64,6 +64,7 @@ import {
 	saveUserData,
 	unlockAppdata,
 	updateAppdata,
+	type QuitSitesBehavior,
 } from 'src/storage/user-data';
 import { getAutoUpdaterState, setupUpdates } from 'src/updates';
 // eslint-disable-next-line import-x/order
@@ -436,7 +437,7 @@ async function appBoot() {
 	let clearAutoStartOnQuit = false;
 	let isQuittingConfirmed = false;
 
-	const applyQuitSitesBehavior = ( behavior: 'stop' | 'stop-and-auto-start' | 'leave-running' ) => {
+	const applyQuitSitesBehavior = ( behavior: QuitSitesBehavior ) => {
 		shouldStopSitesOnQuit = behavior !== 'leave-running';
 		clearAutoStartOnQuit = behavior === 'stop';
 	};
@@ -502,33 +503,33 @@ async function appBoot() {
 					return;
 				}
 
-				const STOP_BUTTON_INDEX = 0;
-				const STOP_AND_AUTO_START_BUTTON_INDEX = 1;
-				const LEAVE_RUNNING_BUTTON_INDEX = 2;
-				const CANCEL_BUTTON_INDEX = 3;
+				const quitChoices: { label: string; behavior: QuitSitesBehavior }[] = [
+					{ label: __( 'Stop' ), behavior: 'stop' },
+					{ label: __( 'Auto-start' ), behavior: 'stop-and-auto-start' },
+					{ label: __( 'Leave running' ), behavior: 'leave-running' },
+				];
+				const cancelButtonIndex = quitChoices.length;
+				const defaultButtonIndex = quitChoices.findIndex(
+					( choice ) => choice.behavior === 'leave-running'
+				);
 
 				const { response, checkboxChecked } = await dialog.showMessageBox( {
 					type: 'question',
 					message: _n( 'You have a running site', 'You have running sites', runningSiteCount ),
 					detail: __(
 						'Choose what to do with your running sites when Studio quits:\n\n• Leave running — they keep running while Studio is closed.\n• Auto-start — they restart when you reopen Studio.\n• Stop — they stay stopped next time you open Studio.'
 					),
-					buttons: [ __( 'Stop' ), __( 'Auto-start' ), __( 'Leave running' ), __( 'Cancel' ) ],
+					buttons: [ ...quitChoices.map( ( choice ) => choice.label ), __( 'Cancel' ) ],
 					checkboxLabel: __( "Don't ask again" ),
-					cancelId: CANCEL_BUTTON_INDEX,
-					defaultId: LEAVE_RUNNING_BUTTON_INDEX,
+					cancelId: cancelButtonIndex,
+					defaultId: defaultButtonIndex,
 				} );
 
-				if ( response === CANCEL_BUTTON_INDEX ) {
+				if ( response === cancelButtonIndex ) {
 					return;
 				}
 
-				const behavior =
-					response === STOP_BUTTON_INDEX
-						? 'stop'
-						: response === STOP_AND_AUTO_START_BUTTON_INDEX
-						? 'stop-and-auto-start'
-						: 'leave-running';
+				const { behavior } = quitChoices[ response ];
 
 				if ( checkboxChecked ) {
 					await updateAppdata( { quitSitesBehavior: behavior } );

apps/studio/src/migrations/06-relocate-autostart-to-app-json.ts

@@ -1,59 +1,74 @@
 /**
- * Relocates the per-site `autoStart` flag out of CLI-owned `cli.json` and into Studio's Desktop-only
- * `app.json` (`siteMetadata[id].autoStart`), and converts the old boolean `stopSitesOnQuit` preference
- * into the new tri-state `quitSitesBehavior`.
+ * Relocates the per-site `autoStart` flag out of CLI-owned `cli.json` into Studio's Desktop-only
+ * `app.json` (`siteMetadata[id].autoStart`), and converts the legacy boolean `stopSitesOnQuit` quit
+ * preference into the tri-state `quitSitesBehavior`.
  *
- * `autoStart` is a desktop-launch concept — only Studio reads it — so it now lives with Studio's other
- * per-site metadata. Runs once, gated by the `autoStartRelocated` marker. Any stale `autoStart` left in
- * `cli.json` is harmless: the CLI no longer reads or writes it.
+ * `autoStart` is a desktop-launch concept — only Studio acts on it — so it now lives with Studio's
+ * other per-site metadata. The migration is self-gating without a stored marker: it runs whenever
+ * `cli.json` still carries an `autoStart` flag (or `app.json` still has the legacy `stopSitesOnQuit`),
+ * and strips those source fields once relocated, so it can't re-run or clobber freshly-tracked values.
+ *
+ * Like the 02/04 migrations it validates with a local, loose zod schema (so unrelated cli.json fields
+ * survive the write-back) and reads/writes the file directly, since migrations run at startup before
+ * the CLI daemon touches it.
  */
 
 import fs from 'node:fs';
 import { getCliConfigPath } from '@studio/common/lib/well-known-paths';
-import { readFile } from 'atomically';
+import { readFile, writeFile } from 'atomically';
 import { z } from 'zod';
 import { loadUserData, lockAppdata, saveUserData, unlockAppdata } from 'src/storage/user-data';
 import type { Migration } from '@studio/common/lib/migration';
 import type { UserData } from 'src/storage/storage-types';
 
-const cliAutoStartSchema = z
+const cliConfigSchema = z
 	.object( {
 		sites: z
 			.array( z.object( { id: z.string(), autoStart: z.boolean().optional() } ).loose() )
 			.optional(),
 	} )
 	.loose();
 
-async function readCliSitesAutoStart(): Promise< { id: string; autoStart?: boolean }[] > {
+type CliConfig = z.infer< typeof cliConfigSchema >;
+
+async function readCliConfig(): Promise< CliConfig | null > {
 	const cliPath = getCliConfigPath();
 	if ( ! fs.existsSync( cliPath ) ) {
-		return [];
+		return null;
 	}
 	try {
-		const raw = await readFile( cliPath, { encoding: 'utf8' } );
-		const parsed = cliAutoStartSchema.safeParse( JSON.parse( raw ) );
-		return parsed.success ? parsed.data.sites ?? [] : [];
+		const parsed = cliConfigSchema.safeParse(
+			JSON.parse( await readFile( cliPath, { encoding: 'utf8' } ) )
+		);
+		return parsed.success ? parsed.data : null;
 	} catch {
-		return [];
+		return null;
 	}
 }
 
+const autoStartSites = ( config: CliConfig | null ) =>
+	( config?.sites ?? [] ).filter( ( site ) => site.autoStart !== undefined );
+
 export const relocateAutostartToAppJson: Migration = {
 	async needsToRun() {
-		const userData = await loadUserData();
-		return ! userData.autoStartRelocated;
+		if ( autoStartSites( await readCliConfig() ).length > 0 ) {
+			return true;
+		}
+		const userData = ( await loadUserData() ) as UserData & { stopSitesOnQuit?: boolean };
+		return userData.stopSitesOnQuit !== undefined;
 	},
 	async run() {
-		const cliSites = await readCliSitesAutoStart();
+		const cliConfig = await readCliConfig();
+		const flagged = autoStartSites( cliConfig );
 
 		try {
 			await lockAppdata();
-			const userData = await loadUserData();
-			const legacy = userData as UserData & { stopSitesOnQuit?: boolean };
+			const userData = ( await loadUserData() ) as UserData & { stopSitesOnQuit?: boolean };
 
-			// Seed per-site autoStart into Studio-owned app.json metadata.
-			for ( const site of cliSites ) {
-				if ( site.autoStart === undefined ) {
+			// Seed per-site autoStart into app.json, skipping anything Studio already tracks so a retry
+			// after a crash can't clobber a fresher value.
+			for ( const site of flagged ) {
+				if ( ! site.id || userData.siteMetadata[ site.id ]?.autoStart !== undefined ) {
 					continue;
 				}
 				userData.siteMetadata[ site.id ] = {
@@ -62,20 +77,28 @@ export const relocateAutostartToAppJson: Migration = {
 				};
 			}
 
-			// Convert the old boolean quit preference. The previous "Stop sites" stopped sites but kept
-			// them flagged to auto-start, so it maps to 'stop-and-auto-start'; the falsey value was
-			// "Leave running".
-			if ( legacy.stopSitesOnQuit !== undefined && userData.quitSitesBehavior === undefined ) {
-				userData.quitSitesBehavior = legacy.stopSitesOnQuit
+			// The old "Stop sites" stopped sites but kept them flagged to auto-start, so a truthy
+			// preference maps to 'stop-and-auto-start'; falsey was "Leave running".
+			if ( userData.stopSitesOnQuit !== undefined && userData.quitSitesBehavior === undefined ) {
+				userData.quitSitesBehavior = userData.stopSitesOnQuit
 					? 'stop-and-auto-start'
 					: 'leave-running';
 			}
-			delete legacy.stopSitesOnQuit;
+			delete userData.stopSitesOnQuit;
 
-			userData.autoStartRelocated = true;
 			await saveUserData( userData );
 		} finally {
 			await unlockAppdata();
 		}
+
+		// Strip the relocated flags from cli.json so the migration stays one-shot.
+		if ( flagged.length > 0 && cliConfig?.sites ) {
+			cliConfig.sites.forEach( ( site ) => {
+				delete site.autoStart;
+			} );
+			await writeFile( getCliConfigPath(), JSON.stringify( cliConfig, null, 2 ) + '\n', {
+				encoding: 'utf8',
+			} );
+		}
 	},
 };

apps/studio/src/storage/storage-types.ts

@@ -47,8 +47,6 @@ export interface UserData {
 	colorScheme?: 'system' | 'light' | 'dark';
 	betaFeatures?: BetaFeatures;
 	quitSitesBehavior?: QuitSitesBehavior;
-	// One-time marker: autoStart relocated from cli.json to per-site app.json metadata (migration 06).
-	autoStartRelocated?: boolean;
 	defaultSiteDirectory?: string;
 	cliAutoInstalled?: boolean;
 	cliUserUninstalled?: boolean;

apps/studio/src/storage/user-data.ts

@@ -8,7 +8,14 @@ import { getAppConfigLockFilePath } from '@studio/common/lib/well-known-paths';
 import { readFile, writeFile } from 'atomically';
 import { sanitizeUnstructuredData, sanitizeUserpath } from 'src/lib/sanitize-for-logging';
 import { getUserDataFilePath } from 'src/storage/paths';
-import { EMPTY_USER_DATA, type UserData, type WindowBounds } from 'src/storage/storage-types';
+import {
+	EMPTY_USER_DATA,
+	type QuitSitesBehavior,
+	type UserData,
+	type WindowBounds,
+} from 'src/storage/storage-types';
+
+export type { QuitSitesBehavior };
 
 export async function loadUserData(): Promise< UserData > {
 	const filePath = getUserDataFilePath();
@@ -66,7 +73,6 @@ type UserDataSafeKeys =
 	| 'onboardingCompleted'
 	| 'promptWindowsSpeedUpResult'
 	| 'quitSitesBehavior'
-	| 'autoStartRelocated'
 	| 'sentryUserId'
 	| 'lastSeenVersion'
 	| 'preferredTerminal'