diff --git a/cli/commands/site/create.ts b/cli/commands/site/create.ts index 0360af7f6c..523e343f12 100644 --- a/cli/commands/site/create.ts +++ b/cli/commands/site/create.ts @@ -331,7 +331,7 @@ export async function runCommand( logger.reportKeyValuePair( 'id', siteDetails.id ); logger.reportKeyValuePair( 'running', String( siteDetails.running ) ); } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/delete.ts b/cli/commands/site/delete.ts index 89d1361587..a70f49eb14 100644 --- a/cli/commands/site/delete.ts +++ b/cli/commands/site/delete.ts @@ -65,14 +65,14 @@ export async function runCommand( deleteFiles: boolean = false ): Promise< void > { try { - logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); - const site = await getSiteByFolder( siteFolder ); - logger.reportSuccess( __( 'Site loaded' ) ); - logger.reportStart( LoggerAction.START_DAEMON, __( 'Starting process daemon…' ) ); await connect(); logger.reportSuccess( __( 'Process daemon started' ) ); + logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); + const site = await getSiteByFolder( siteFolder ); + logger.reportSuccess( __( 'Site loaded' ) ); + const runningProcess = await isServerRunning( site.id ); if ( runningProcess ) { logger.reportStart( LoggerAction.STOP_SITE, __( 'Stopping WordPress server…' ) ); @@ -126,7 +126,7 @@ export async function runCommand( logger.reportSuccess( __( 'Site files deleted' ) ); } } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/list.ts b/cli/commands/site/list.ts index 23f7a95c98..396911a4d0 100644 --- a/cli/commands/site/list.ts +++ b/cli/commands/site/list.ts @@ -2,7 +2,7 @@ import { __, _n, sprintf } from '@wordpress/i18n'; import Table from 'cli-table3'; import { SiteCommandLoggerAction as LoggerAction } from 'common/logger-actions'; import { getSiteUrl, readAppdata, type SiteData } from 'cli/lib/appdata'; -import { connect, disconnect } from 'cli/lib/pm2-manager'; +import { connect, disconnect, subscribePm2KillEvent } from 'cli/lib/pm2-manager'; import { getColumnWidths, getPrettyPath } from 'cli/lib/utils'; import { isServerRunning, subscribeSiteEvents } from 'cli/lib/wordpress-server-manager'; import { Logger, LoggerError } from 'cli/logger'; @@ -126,10 +126,24 @@ export async function runCommand( format: 'table' | 'json', watch: boolean ): Pr }, { debounceMs: 500 } ); + + await subscribePm2KillEvent( () => { + for ( const site of sitesData ) { + const payload = { + siteId: site.id, + status: 'stopped', + url: site.url, + }; + logger.reportKeyValuePair( 'site-status', JSON.stringify( payload ) ); + } + } ); + + process.on( 'SIGINT', disconnect ); + process.on( 'SIGTERM', disconnect ); } } finally { if ( ! watch ) { - disconnect(); + await disconnect(); } } } diff --git a/cli/commands/site/set.ts b/cli/commands/site/set.ts index 3d5c4cef91..4cd3a5fa93 100644 --- a/cli/commands/site/set.ts +++ b/cli/commands/site/set.ts @@ -246,7 +246,7 @@ export async function runCommand( return { usedWpCli: wpChanged }; } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/start.ts b/cli/commands/site/start.ts index 2bdd2001d5..7dbcc5ee68 100644 --- a/cli/commands/site/start.ts +++ b/cli/commands/site/start.ts @@ -12,14 +12,14 @@ const logger = new Logger< LoggerAction >(); export async function runCommand( sitePath: string, skipBrowser = false ): Promise< void > { try { - logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); - const site = await getSiteByFolder( sitePath ); - logger.reportSuccess( __( 'Site loaded' ) ); - logger.reportStart( LoggerAction.START_DAEMON, __( 'Starting process daemon…' ) ); await connect(); logger.reportSuccess( __( 'Process daemon started' ) ); + logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); + const site = await getSiteByFolder( sitePath ); + logger.reportSuccess( __( 'Site loaded' ) ); + const runningProcess = await isServerRunning( site.id ); if ( runningProcess ) { logger.reportSuccess( __( 'WordPress server is already running' ) ); @@ -60,7 +60,7 @@ export async function runCommand( sitePath: string, skipBrowser = false ): Promi throw new LoggerError( __( 'Failed to start WordPress server' ), error ); } } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/status.ts b/cli/commands/site/status.ts index c2f87bf8a1..894f3a0fcb 100644 --- a/cli/commands/site/status.ts +++ b/cli/commands/site/status.ts @@ -13,12 +13,14 @@ const logger = new Logger< LoggerAction >(); export async function runCommand( siteFolder: string, format: 'table' | 'json' ): Promise< void > { try { + logger.reportStart( LoggerAction.START_DAEMON, __( 'Starting process daemon…' ) ); + await connect(); + logger.reportSuccess( __( 'Process daemon started' ) ); + logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); const site = await getSiteByFolder( siteFolder ); logger.reportSuccess( __( 'Site loaded' ) ); - await connect(); - const isOnline = Boolean( await isServerRunning( site.id ) ); const status = isOnline ? `🟢 ${ __( 'Online' ) }` : `🔴 ${ __( 'Offline' ) }`; const siteUrl = getSiteUrl( site ); @@ -74,7 +76,7 @@ export async function runCommand( siteFolder: string, format: 'table' | 'json' ) console.log( JSON.stringify( logData, null, 2 ) ); } } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/stop.ts b/cli/commands/site/stop.ts index 53f4db74fe..528e9e162c 100644 --- a/cli/commands/site/stop.ts +++ b/cli/commands/site/stop.ts @@ -3,11 +3,14 @@ import { SiteCommandLoggerAction as LoggerAction } from 'common/logger-actions'; import { clearSiteLatestCliPid, getSiteByFolder, + lockAppdata, readAppdata, + saveAppdata, + unlockAppdata, updateSiteAutoStart, type SiteData, } from 'cli/lib/appdata'; -import { connect, disconnect } from 'cli/lib/pm2-manager'; +import { connect, disconnect, killDaemonAndAllChildren } from 'cli/lib/pm2-manager'; import { stopProxyIfNoSitesNeedIt } from 'cli/lib/site-utils'; import { isServerRunning, stopWordPressServer } from 'cli/lib/wordpress-server-manager'; import { Logger, LoggerError } from 'cli/logger'; @@ -38,92 +41,76 @@ export async function runCommand( try { await connect(); - const sitesToTarget: SiteData[] = []; - if ( target === Mode.STOP_SINGLE_SITE && siteFolder ) { const site = await getSiteByFolder( siteFolder ); - sitesToTarget.push( site ); - const runningProcess = await isServerRunning( site.id ); if ( ! runningProcess ) { logger.reportSuccess( __( 'WordPress server is not running' ) ); return; } - logger.reportStart( LoggerAction.STOP_SITE, __( 'Stopping WordPress server…' ) ); + logger.reportStart( LoggerAction.STOP_SITE, __( 'Stopping WordPress servers…' ) ); + + try { + await stopWordPressServer( site.id ); + await clearSiteLatestCliPid( site.id ); + await updateSiteAutoStart( site.id, autoStart ); + logger.reportSuccess( __( 'WordPress server stopped' ) ); + await stopProxyIfNoSitesNeedIt( site.id, logger ); + } catch ( error ) { + throw new LoggerError( __( 'Failed to stop WordPress server' ), error ); + } } else { const appdata = await readAppdata(); + const runningSites: SiteData[] = []; for ( const site of appdata.sites ) { const runningProcess = await isServerRunning( site.id ); if ( runningProcess ) { - sitesToTarget.push( site ); + runningSites.push( site ); } } - if ( ! sitesToTarget.length ) { + if ( ! runningSites.length ) { logger.reportSuccess( __( 'No sites are currently running' ) ); return; } - logger.reportStart( LoggerAction.STOP_ALL_SITES, __( 'Stopping all WordPress sites...' ) ); - } - - const stoppedSiteIds: string[] = []; + logger.reportStart( LoggerAction.STOP_ALL_SITES, __( 'Stopping all WordPress servers…' ) ); + await killDaemonAndAllChildren(); - for ( const site of sitesToTarget ) { try { - logger.reportProgress( - sprintf( - __( 'Stopping site "%s" (%d/%d)…' ), - site.name, - stoppedSiteIds.length + 1, - sitesToTarget.length - ) - ); - await stopWordPressServer( site.id ); - await clearSiteLatestCliPid( site.id ); - await updateSiteAutoStart( site.id, autoStart ); - - stoppedSiteIds.push( site.id ); - } catch ( error ) { - logger.reportError( - new LoggerError( sprintf( __( 'Failed to stop site %s' ), site.name ) ) - ); + await lockAppdata(); + const appdata = await readAppdata(); + for ( const site of appdata.sites ) { + if ( runningSites.find( ( r ) => r.id === site.id ) ) { + delete site.latestCliPid; + site.autoStart = autoStart; + } + } + await saveAppdata( appdata ); + } finally { + await unlockAppdata(); } - } - - try { - await stopProxyIfNoSitesNeedIt( stoppedSiteIds, logger ); - } catch ( error ) { - throw new LoggerError( __( 'Failed to stop proxy server' ), error ); - } - if ( stoppedSiteIds.length === sitesToTarget.length ) { logger.reportSuccess( sprintf( _n( 'Successfully stopped %d site', 'Successfully stopped %d sites', - sitesToTarget.length + runningSites.length ), - sitesToTarget.length - ) - ); - } else if ( stoppedSiteIds.length === 0 && sitesToTarget.length === 0 ) { - throw new LoggerError( __( 'Failed to stop site' ) ); - } else { - throw new LoggerError( - sprintf( - _n( 'Stopped %d site out of %d', 'Stopped %d sites out of %d', stoppedSiteIds.length ), - stoppedSiteIds.length, - sitesToTarget.length + runningSites.length ) ); + + // Calling `pm2.killDaemon` requires us to forcefully exit the process. pm2 does the same + // thing internally in its CLI. + process.exit( 0 ); } } finally { - disconnect(); + await disconnect(); } } diff --git a/cli/commands/site/tests/stop.test.ts b/cli/commands/site/tests/stop.test.ts index 30e4ccb5b4..763a2ff6e6 100644 --- a/cli/commands/site/tests/stop.test.ts +++ b/cli/commands/site/tests/stop.test.ts @@ -3,9 +3,10 @@ import { clearSiteLatestCliPid, getSiteByFolder, readAppdata, + saveAppdata, updateSiteAutoStart, } from 'cli/lib/appdata'; -import { connect, disconnect } from 'cli/lib/pm2-manager'; +import { connect, disconnect, killDaemonAndAllChildren } from 'cli/lib/pm2-manager'; import { stopProxyIfNoSitesNeedIt } from 'cli/lib/site-utils'; import { isServerRunning, stopWordPressServer } from 'cli/lib/wordpress-server-manager'; import { Mode, runCommand } from '../stop'; @@ -17,6 +18,7 @@ jest.mock( 'cli/lib/appdata', () => ( { clearSiteLatestCliPid: jest.fn(), updateSiteAutoStart: jest.fn().mockResolvedValue( undefined ), getAppdataDirectory: jest.fn().mockReturnValue( '/test/appdata' ), + saveAppdata: jest.fn().mockResolvedValue( undefined ), } ) ); jest.mock( 'cli/lib/pm2-manager' ); jest.mock( 'cli/lib/site-utils' ); @@ -49,6 +51,7 @@ describe( 'CLI: studio site stop', () => { ( stopWordPressServer as jest.Mock ).mockResolvedValue( undefined ); ( clearSiteLatestCliPid as jest.Mock ).mockResolvedValue( undefined ); ( stopProxyIfNoSitesNeedIt as jest.Mock ).mockResolvedValue( undefined ); + ( killDaemonAndAllChildren as jest.Mock ).mockResolvedValue( undefined ); } ); afterEach( () => { @@ -79,7 +82,7 @@ describe( 'CLI: studio site stop', () => { ( stopWordPressServer as jest.Mock ).mockRejectedValue( new Error( 'Server stop failed' ) ); await expect( runCommand( Mode.STOP_SINGLE_SITE, '/test/site', false ) ).rejects.toThrow( - 'Stopped 0 sites out of 1' + 'Failed to stop WordPress server' ); expect( disconnect ).toHaveBeenCalled(); } ); @@ -105,10 +108,7 @@ describe( 'CLI: studio site stop', () => { expect( isServerRunning ).toHaveBeenCalledWith( testSite.id ); expect( stopWordPressServer ).toHaveBeenCalledWith( testSite.id ); expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( testSite.id ); - expect( stopProxyIfNoSitesNeedIt ).toHaveBeenCalledWith( - [ testSite.id ], - expect.any( Object ) - ); + expect( stopProxyIfNoSitesNeedIt ).toHaveBeenCalledWith( testSite.id, expect.any( Object ) ); expect( disconnect ).toHaveBeenCalled(); } ); @@ -202,12 +202,14 @@ describe( 'CLI: studio site stop --all', () => { beforeEach( () => { jest.clearAllMocks(); + // Mock process.exit to prevent tests from exiting + jest.spyOn( process, 'exit' ).mockImplementation( () => undefined as never ); + ( connect as jest.Mock ).mockResolvedValue( undefined ); ( disconnect as jest.Mock ).mockResolvedValue( undefined ); ( isServerRunning as jest.Mock ).mockResolvedValue( undefined ); - ( stopWordPressServer as jest.Mock ).mockResolvedValue( undefined ); + ( killDaemonAndAllChildren as jest.Mock ).mockResolvedValue( undefined ); ( clearSiteLatestCliPid as jest.Mock ).mockResolvedValue( undefined ); - ( stopProxyIfNoSitesNeedIt as jest.Mock ).mockResolvedValue( undefined ); } ); afterEach( () => { @@ -234,31 +236,17 @@ describe( 'CLI: studio site stop --all', () => { expect( disconnect ).toHaveBeenCalled(); } ); - it( 'should throw when all sites fail to stop', async () => { + it( 'should throw when killDaemonAndAllChildren fails', async () => { ( readAppdata as jest.Mock ).mockResolvedValue( { sites: testSites } ); ( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription ); - ( stopWordPressServer as jest.Mock ).mockRejectedValue( new Error( 'Server stop failed' ) ); - - await expect( runCommand( Mode.STOP_ALL_SITES, undefined, false ) ).rejects.toThrow( - 'Stopped 0 sites out of 3' + ( killDaemonAndAllChildren as jest.Mock ).mockRejectedValue( + new Error( 'Failed to kill daemon' ) ); - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should throw when some sites fail to stop', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: testSites } ); - ( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription ); - - ( stopWordPressServer as jest.Mock ) - .mockResolvedValueOnce( undefined ) // site-1 success - .mockRejectedValueOnce( new Error( 'Server stop failed' ) ) // site-2 fails - .mockResolvedValueOnce( undefined ); // site-3 success await expect( runCommand( Mode.STOP_ALL_SITES, undefined, false ) ).rejects.toThrow( - 'Stopped 2 sites out of 3' + 'Failed to kill daemon' ); expect( disconnect ).toHaveBeenCalled(); - expect( stopWordPressServer ).toHaveBeenCalledTimes( 3 ); } ); } ); @@ -269,7 +257,7 @@ describe( 'CLI: studio site stop --all', () => { await runCommand( Mode.STOP_ALL_SITES, undefined, false ); expect( connect ).toHaveBeenCalled(); - expect( stopWordPressServer ).not.toHaveBeenCalled(); + expect( killDaemonAndAllChildren ).not.toHaveBeenCalled(); expect( disconnect ).toHaveBeenCalled(); } ); @@ -282,9 +270,8 @@ describe( 'CLI: studio site stop --all', () => { expect( connect ).toHaveBeenCalled(); expect( isServerRunning ).toHaveBeenCalledTimes( 3 ); - expect( stopWordPressServer ).not.toHaveBeenCalled(); + expect( killDaemonAndAllChildren ).not.toHaveBeenCalled(); expect( clearSiteLatestCliPid ).not.toHaveBeenCalled(); - expect( stopProxyIfNoSitesNeedIt ).not.toHaveBeenCalled(); expect( disconnect ).toHaveBeenCalled(); } ); @@ -294,9 +281,19 @@ describe( 'CLI: studio site stop --all', () => { await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - expect( stopWordPressServer ).toHaveBeenCalledTimes( 1 ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' ); - expect( disconnect ).toHaveBeenCalled(); + expect( killDaemonAndAllChildren ).toHaveBeenCalledTimes( 1 ); + expect( saveAppdata ).toHaveBeenCalledTimes( 1 ); + expect( saveAppdata ).toHaveBeenCalledWith( + expect.objectContaining( { + sites: expect.arrayContaining( [ + expect.objectContaining( { + id: 'site-1', + autoStart: false, + } ), + ] ), + } ) + ); + expect( process.exit ).toHaveBeenCalledWith( 0 ); } ); it( 'should stop all running sites', async () => { @@ -312,21 +309,29 @@ describe( 'CLI: studio site stop --all', () => { expect( isServerRunning ).toHaveBeenCalledWith( 'site-2' ); expect( isServerRunning ).toHaveBeenCalledWith( 'site-3' ); - expect( stopWordPressServer ).toHaveBeenCalledTimes( 3 ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-2' ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-3' ); - - expect( clearSiteLatestCliPid ).toHaveBeenCalledTimes( 3 ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( 'site-1' ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( 'site-2' ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( 'site-3' ); - - expect( stopProxyIfNoSitesNeedIt ).toHaveBeenCalledWith( - [ 'site-1', 'site-2', 'site-3' ], - expect.any( Object ) + expect( killDaemonAndAllChildren ).toHaveBeenCalledTimes( 1 ); + + expect( saveAppdata ).toHaveBeenCalledTimes( 1 ); + expect( saveAppdata ).toHaveBeenCalledWith( + expect.objectContaining( { + sites: expect.arrayContaining( [ + expect.objectContaining( { + id: 'site-1', + autoStart: false, + } ), + expect.objectContaining( { + id: 'site-2', + autoStart: false, + } ), + expect.objectContaining( { + id: 'site-3', + autoStart: false, + } ), + ] ), + } ) ); - expect( disconnect ).toHaveBeenCalled(); + + expect( process.exit ).toHaveBeenCalledWith( 0 ); } ); it( 'should stop only running sites (mixed state)', async () => { @@ -337,96 +342,39 @@ describe( 'CLI: studio site stop --all', () => { .mockResolvedValueOnce( undefined ) // site-2 not running .mockResolvedValueOnce( testProcessDescription ); // site-3 running - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); + await runCommand( Mode.STOP_ALL_SITES, undefined, true ); expect( isServerRunning ).toHaveBeenCalledTimes( 3 ); - expect( stopWordPressServer ).toHaveBeenCalledTimes( 2 ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-3' ); - expect( stopWordPressServer ).not.toHaveBeenCalledWith( 'site-2' ); - - expect( clearSiteLatestCliPid ).toHaveBeenCalledTimes( 2 ); - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should continue stopping other sites even if one fails', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: testSites } ); - ( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription ); - - ( stopWordPressServer as jest.Mock ) - .mockResolvedValueOnce( undefined ) // site-1 success - .mockRejectedValueOnce( new Error( 'Server stop failed' ) ) // site-2 fails - .mockResolvedValueOnce( undefined ); // site-3 success - - try { - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - } catch { - // Expected to throw due to partial failure - } - - expect( stopWordPressServer ).toHaveBeenCalledTimes( 3 ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledTimes( 2 ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( 'site-1' ); - expect( clearSiteLatestCliPid ).toHaveBeenCalledWith( 'site-3' ); - expect( clearSiteLatestCliPid ).not.toHaveBeenCalledWith( 'site-2' ); - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should throw when proxy stop fails', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: [ testSites[ 0 ] ] } ); - ( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription ); - ( stopProxyIfNoSitesNeedIt as jest.Mock ).mockRejectedValue( - new Error( 'Proxy stop failed' ) + expect( killDaemonAndAllChildren ).toHaveBeenCalledTimes( 1 ); + + expect( saveAppdata ).toHaveBeenCalledTimes( 1 ); + expect( saveAppdata ).toHaveBeenCalledWith( + expect.objectContaining( { + sites: expect.arrayContaining( [ + expect.objectContaining( { + id: 'site-1', + autoStart: true, + } ), + expect.objectContaining( { + id: 'site-3', + autoStart: true, + } ), + ] ), + } ) ); - - // Should throw when proxy stop fails - await expect( runCommand( Mode.STOP_ALL_SITES, undefined, false ) ).rejects.toThrow( - 'Failed to stop proxy server' + expect( saveAppdata ).toHaveBeenCalledWith( + expect.objectContaining( { + sites: expect.not.arrayContaining( [ + expect.objectContaining( { + id: 'site-2', + autoStart: true, + } ), + ] ), + } ) ); - expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' ); - expect( disconnect ).toHaveBeenCalled(); - } ); - } ); - - describe( 'Cleanup', () => { - it( 'should always disconnect from PM2 on success', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: testSites } ); - ( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription ); - - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should always disconnect from PM2 on error', async () => { - ( readAppdata as jest.Mock ).mockRejectedValue( new Error( 'Error' ) ); - - try { - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - } catch { - // Expected - } - - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should always disconnect when no sites exist', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: [] } ); - - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - - expect( disconnect ).toHaveBeenCalled(); - } ); - - it( 'should always disconnect when no sites are running', async () => { - ( readAppdata as jest.Mock ).mockResolvedValue( { sites: testSites } ); - ( isServerRunning as jest.Mock ).mockResolvedValue( undefined ); - - await runCommand( Mode.STOP_ALL_SITES, undefined, false ); - - expect( disconnect ).toHaveBeenCalled(); + expect( process.exit ).toHaveBeenCalledWith( 0 ); } ); } ); } ); diff --git a/cli/commands/wp.ts b/cli/commands/wp.ts index d6d11abb8c..2aa9ed7b88 100644 --- a/cli/commands/wp.ts +++ b/cli/commands/wp.ts @@ -26,6 +26,9 @@ export async function runCommand( const useCustomPhpVersion = options.phpVersion && options.phpVersion !== site.phpVersion; if ( ! useCustomPhpVersion ) { + process.on( 'SIGINT', disconnect ); + process.on( 'SIGTERM', disconnect ); + try { await connect(); @@ -36,10 +39,13 @@ export async function runCommand( process.exit( result.exitCode ); } } finally { - disconnect(); + await disconnect(); } } + process.on( 'SIGINT', () => process.exit( 1 ) ); + process.on( 'SIGTERM', () => process.exit( 1 ) ); + // …If not, instantiate a new Playground instance const [ response, closeWpCliServer ] = await runWpCliCommand( siteFolder, diff --git a/cli/lib/pm2-manager.ts b/cli/lib/pm2-manager.ts index 7d80f3d317..c01ab01601 100644 --- a/cli/lib/pm2-manager.ts +++ b/cli/lib/pm2-manager.ts @@ -1,6 +1,8 @@ import os from 'os'; import path from 'path'; +import { LOCKFILE_STALE_TIME, LOCKFILE_WAIT_TIME } from 'common/constants'; import { cacheFunctionTTL } from 'common/lib/cache-function-ttl'; +import { lockFileAsync, unlockFileAsync } from 'common/lib/lockfile'; import { custom as PM2, StartOptions } from 'pm2'; import { getAppdataPath } from 'cli/lib/appdata'; import { ProcessDescription } from 'cli/lib/types/pm2'; @@ -12,11 +14,13 @@ import { const PM2_STATUS_ONLINE = 'online'; const PROXY_PROCESS_NAME = 'studio-proxy'; -const DAEMON_TIMEOUT = 10000; +const CONNECTION_TIMEOUT = 10_000; +const KILL_TIMEOUT = 25_000; // Set consistent PM2 home directory for Studio CLI // This ensures all Studio CLI commands use the same PM2 daemon const STUDIO_PM2_HOME = path.join( os.homedir(), '.studio', 'pm2' ); +const PM2_LOCKFILE_PATH = path.join( STUDIO_PM2_HOME, 'pm2-connection.lock' ); export interface ProcessEventData { processName: string; @@ -32,14 +36,19 @@ export async function connect(): Promise< void > { return; } - return new Promise( ( resolve, reject ) => { + await lockFileAsync( PM2_LOCKFILE_PATH, { + wait: LOCKFILE_WAIT_TIME, + stale: LOCKFILE_STALE_TIME, + } ); + + return new Promise< void >( ( resolve, reject ) => { const timeout = setTimeout( () => { reject( new Error( 'PM2 connection timeout after 10 seconds. Try running: PM2_HOME=~/.studio/pm2 pm2 update' ) ); - }, DAEMON_TIMEOUT ); + }, CONNECTION_TIMEOUT ); pm2.connect( ( error ) => { clearTimeout( timeout ); @@ -50,14 +59,52 @@ export async function connect(): Promise< void > { isConnected = true; resolve(); } ); + } ).finally( () => { + return unlockFileAsync( PM2_LOCKFILE_PATH ); } ); } -export function disconnect(): void { - if ( isConnected ) { - pm2.disconnect(); - isConnected = false; +export async function disconnect(): Promise< void > { + if ( ! isConnected ) { + return; } + + return new Promise< void >( ( resolve, reject ) => { + const timeout = setTimeout( () => { + reject( new Error( 'Timeout after 10 seconds trying to disconnect from PM2' ) ); + }, CONNECTION_TIMEOUT ); + + pm2.disconnect( ( error ) => { + clearTimeout( timeout ); + if ( error ) { + reject( error ); + return; + } + isConnected = false; + resolve(); + } ); + } ); +} + +export async function killDaemonAndAllChildren() { + return new Promise< void >( ( resolve, reject ) => { + const timeout = setTimeout( () => { + reject( + new Error( + 'PM2 kill timeout after 25 seconds. Try running: PM2_HOME=~/.studio/pm2 pm2 kill' + ) + ); + }, KILL_TIMEOUT ); + + pm2.killDaemon( ( error ) => { + clearTimeout( timeout ); + if ( error ) { + reject( error ); + return; + } + resolve(); + } ); + } ); } // Cache the return value of `pm2.list` for a very short time to make multiple calls in quick @@ -283,3 +330,13 @@ export async function subscribeProcessMessages( bus.off( 'process:msg', messageHandler ); }; } + +export async function subscribePm2KillEvent( handler: () => void ) { + const bus = await getPm2Bus(); + + bus.on( 'pm2:kill', handler ); + + return () => { + bus.off( 'pm2:kill', handler ); + }; +} diff --git a/cli/lib/tests/pm2-manager.test.ts b/cli/lib/tests/pm2-manager.test.ts index 7a92873436..9be746300f 100644 --- a/cli/lib/tests/pm2-manager.test.ts +++ b/cli/lib/tests/pm2-manager.test.ts @@ -44,9 +44,12 @@ describe( 'PM2 Manager', () => { const { connect } = await import( '../pm2-manager' ); const connectPromise = connect(); - jest.advanceTimersByTime( 10000 ); - await expect( connectPromise ).rejects.toThrow( 'PM2 connection timeout after 10 seconds' ); + const expectation = expect( connectPromise ).rejects.toThrow( + 'PM2 connection timeout after 10 seconds' + ); + await jest.advanceTimersByTimeAsync( 10000 ); + await expectation; jest.useRealTimers(); } ); @@ -150,8 +153,17 @@ describe( 'PM2 Manager', () => { describe( 'isProcessRunning() - process discovery and filtering', () => { it( 'should return undefined when not connected', async () => { - const { isProcessRunning, disconnect } = await import( '../pm2-manager' ); - disconnect(); + mockPm2Instance.disconnect.mockImplementation( ( callback ) => { + callback( null ); + } ); + mockPm2Instance.connect.mockImplementation( ( callback ) => { + callback( null ); + } ); + + const { isProcessRunning, connect, disconnect } = await import( '../pm2-manager' ); + + await connect(); + await disconnect(); const result = await isProcessRunning( 'app' ); diff --git a/cli/lib/wordpress-server-manager.ts b/cli/lib/wordpress-server-manager.ts index 0b5020d35a..bf13860b34 100644 --- a/cli/lib/wordpress-server-manager.ts +++ b/cli/lib/wordpress-server-manager.ts @@ -35,11 +35,8 @@ const SITE_PROCESS_PREFIX = 'studio-site-'; // Get an abort signal that's triggered on SIGINT/SIGTERM. This is useful for aborting and cleaning // up async operations. const abortController = new AbortController(); -function handleProcessTermination() { - abortController.abort(); -} -process.on( 'SIGINT', handleProcessTermination ); -process.on( 'SIGTERM', handleProcessTermination ); +process.on( 'SIGINT', () => abortController.abort() ); +process.on( 'SIGTERM', () => abortController.abort() ); function getProcessName( siteId: string ): string { return `${ SITE_PROCESS_PREFIX }${ siteId }`; diff --git a/cli/patches/pm2+6.0.13.patch b/cli/patches/pm2+6.0.14.patch similarity index 99% rename from cli/patches/pm2+6.0.13.patch rename to cli/patches/pm2+6.0.14.patch index e2b92a3b1b..af1b739b9f 100644 --- a/cli/patches/pm2+6.0.13.patch +++ b/cli/patches/pm2+6.0.14.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/pm2/types/index.d.ts b/node_modules/pm2/types/index.d.ts -index 77280f5..887f710 100644 +index 77280f5..092273a 100644 --- a/node_modules/pm2/types/index.d.ts +++ b/node_modules/pm2/types/index.d.ts @@ -3,316 +3,384 @@ @@ -131,7 +131,7 @@ index 77280f5..887f710 100644 + /** + * Disconnects from the pm2 daemon. + */ -+ disconnect(): void; ++ disconnect(errback: ErrProcCallback): void; -/** - * Stops and restarts the process. diff --git a/e2e/e2e-helpers.ts b/e2e/e2e-helpers.ts index dcd4bbdedf..687f70e094 100644 --- a/e2e/e2e-helpers.ts +++ b/e2e/e2e-helpers.ts @@ -4,6 +4,7 @@ import path from 'path'; import { findLatestBuild, parseElectronApp } from 'electron-playwright-helpers'; import fs from 'fs-extra'; import { _electron as electron, Page, ElectronApplication } from 'playwright'; +import { rimraf } from 'rimraf'; export class E2ESession { electronApp: ElectronApplication; @@ -14,7 +15,6 @@ export class E2ESession { homePath: string; public constructor() { - // Create temporary folder to hold application data this.sessionPath = path.join( tmpdir(), `studio-app-e2e-session-${ randomUUID() }` ); this.appDataPath = path.join( this.sessionPath, 'appData' ); this.homePath = path.join( this.sessionPath, 'home' ); @@ -28,7 +28,7 @@ export class E2ESession { // Path must include 'Studio' subfolder to match Electron app's path structure const studioAppDataPath = path.join( this.appDataPath, 'Studio' ); await fs.mkdir( studioAppDataPath, { recursive: true } ); - const appdataPath = path.join( studioAppDataPath, 'appdata-v1.json' ); + const initialAppdata = { version: 1, sites: [], @@ -37,42 +37,58 @@ export class E2ESession { studioSitesCli: true, }, }; - await fs.writeFile( appdataPath, JSON.stringify( initialAppdata, null, 2 ) ); - // find the latest build in the out directory - const latestBuild = findLatestBuild(); + await fs.writeFile( + path.join( studioAppDataPath, 'appdata-v1.json' ), + JSON.stringify( initialAppdata, null, 2 ) + ); - // parse the packaged Electron app and find paths and other info - const appInfo = parseElectronApp( latestBuild ); - let executablePath = appInfo.executable; - if ( appInfo.platform === 'win32' ) { - // `parseElectronApp` function obtains the executable path by finding the first executable from the build folder. - // We need to ensure that the executable is the Studio app. - executablePath = executablePath.replace( 'Squirrel.exe', 'Studio.exe' ); - } + await this.launchFirstWindow( testEnv ); + } - this.electronApp = await electron.launch( { - args: [ appInfo.main ], // main file from package.json - executablePath, // path to the Electron executable - env: { - ...process.env, - ...testEnv, - E2E: 'true', // allow app to determine whether it's running as an end-to-end test - E2E_APP_DATA_PATH: this.appDataPath, - E2E_HOME_PATH: this.homePath, - }, - timeout: 60_000, + async closeApp() { + console.log( 'Closing app...' ); + const childProcess = this.electronApp.process(); + + // Playwright's electronApp.close() can hang, especially on Windows. This is likely due to how + // `stopAllServersOnQuit` spawns a child process in the `will-quit` event handler, sidestepping + // Electron's normal close sequence. + const exitPromise = new Promise< void >( ( resolve ) => { + childProcess.once( 'exit', resolve ); } ); - this.mainWindow = await this.electronApp.firstWindow( { timeout: 60_000 } ); + const timeoutPromise = new Promise< void >( ( _, reject ) => { + setTimeout( () => reject( new Error( 'Process exit timeout' ) ), 30_000 ); + } ); + + await this.electronApp.evaluate( ( { app } ) => app.quit() ).catch( () => {} ); + + try { + await Promise.race( [ exitPromise, timeoutPromise ] ); + await new Promise< void >( ( resolve ) => setTimeout( resolve, 2000 ) ); + console.log( 'App closed successfully' ); + } catch ( error ) { + console.log( 'Process exit timeout' ); + } } - // Close the app but keep the data for persistence testing async restart() { - await this.electronApp?.close(); + await this.closeApp(); + await this.launchFirstWindow(); + } + + async cleanup() { + await this.closeApp(); + await rimraf( this.sessionPath ); + } + + private async launchFirstWindow( testEnv: NodeJS.ProcessEnv = {} ) { const latestBuild = findLatestBuild(); const appInfo = parseElectronApp( latestBuild ); let executablePath = appInfo.executable; + if ( appInfo.platform === 'win32' ) { + // `parseElectronApp` function obtains the executable path by finding the first executable from + // the build folder. We need to ensure that the executable is the Studio app. executablePath = executablePath.replace( 'Squirrel.exe', 'Studio.exe' ); } @@ -81,18 +97,14 @@ export class E2ESession { executablePath, env: { ...process.env, + ...testEnv, E2E: 'true', E2E_APP_DATA_PATH: this.appDataPath, E2E_HOME_PATH: this.homePath, }, timeout: 60_000, } ); - this.mainWindow = await this.electronApp.firstWindow( { timeout: 60_000 } ); - } - async cleanup() { - await this.electronApp?.close(); - // Clean up temporary folder to hold application data - fs.rmSync( this.sessionPath, { recursive: true, force: true } ); + this.mainWindow = await this.electronApp.firstWindow( { timeout: 60_000 } ); } } diff --git a/e2e/fixtures/blueprints/activate-plugin.json b/e2e/fixtures/blueprints/activate-plugin.json index 6b4f6b326c..111bd4eefc 100644 --- a/e2e/fixtures/blueprints/activate-plugin.json +++ b/e2e/fixtures/blueprints/activate-plugin.json @@ -4,7 +4,7 @@ "steps": [ { "step": "installPlugin", - "pluginZipFile": { + "pluginData": { "resource": "wordpress.org/plugins", "slug": "hello-dolly" } @@ -14,4 +14,4 @@ "pluginPath": "hello-dolly/hello.php" } ] -} \ No newline at end of file +} diff --git a/e2e/fixtures/blueprints/activate-theme.json b/e2e/fixtures/blueprints/activate-theme.json index fe60126e93..2600eab205 100644 --- a/e2e/fixtures/blueprints/activate-theme.json +++ b/e2e/fixtures/blueprints/activate-theme.json @@ -4,7 +4,7 @@ "steps": [ { "step": "installTheme", - "themeZipFile": { + "themeData": { "resource": "wordpress.org/themes", "slug": "twentytwentyone" } @@ -14,4 +14,4 @@ "themeFolderName": "twentytwentyone" } ] -} \ No newline at end of file +} diff --git a/e2e/fixtures/blueprints/install-plugin.json b/e2e/fixtures/blueprints/install-plugin.json index 76c840c7f5..2dd6e971bf 100644 --- a/e2e/fixtures/blueprints/install-plugin.json +++ b/e2e/fixtures/blueprints/install-plugin.json @@ -4,10 +4,10 @@ "steps": [ { "step": "installPlugin", - "pluginZipFile": { + "pluginData": { "resource": "wordpress.org/plugins", "slug": "akismet" } } ] -} \ No newline at end of file +} diff --git a/e2e/fixtures/blueprints/install-theme.json b/e2e/fixtures/blueprints/install-theme.json index 5bb3492410..d718e55e37 100644 --- a/e2e/fixtures/blueprints/install-theme.json +++ b/e2e/fixtures/blueprints/install-theme.json @@ -4,10 +4,10 @@ "steps": [ { "step": "installTheme", - "themeZipFile": { + "themeData": { "resource": "wordpress.org/themes", "slug": "twentytwentytwo" } } ] -} \ No newline at end of file +} diff --git a/e2e/sites.test.ts b/e2e/sites.test.ts index 1c3e973704..13a8cb4104 100644 --- a/e2e/sites.test.ts +++ b/e2e/sites.test.ts @@ -122,7 +122,7 @@ test.describe( 'Servers', () => { await settingsTab.siteNameInput.fill( newSiteName ); await settingsTab.saveButton.click(); - await expect( settingsTab.editSiteDialog ).not.toBeVisible( { timeout: 10000 } ); + await expect( settingsTab.editSiteDialog ).not.toBeVisible( { timeout: 20_000 } ); // Explicitly wait for the rename to propagate const renamedSiteContent = new SiteContent( session.mainWindow, newSiteName ); diff --git a/package-lock.json b/package-lock.json index 795b6a72c6..5f5e665ac7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -126,6 +126,7 @@ "react": "^18.2.0", "react-dom": "^18.2.0", "resize-observer-polyfill": "^1.5.1", + "rimraf": "^6.1.2", "tailwindcss": "^3.3.6", "ts-jest": "^29.4.6", "typescript": "~5.9.3", @@ -5339,6 +5340,29 @@ } } }, + "node_modules/@isaacs/balanced-match": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@isaacs/balanced-match/-/balanced-match-4.0.1.tgz", + "integrity": "sha512-yzMTt9lEb8Gv7zRioUilSglI0c0smZ9k5D65677DLWLtWJaXIS3CqcGyUFByYKlnUj6TkjLVs54fBl6+TiGQDQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": "20 || >=22" + } + }, + "node_modules/@isaacs/brace-expansion": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@isaacs/brace-expansion/-/brace-expansion-5.0.0.tgz", + "integrity": "sha512-ZT55BDLV0yv0RBm2czMiZ+SqCGO7AvmOM3G/w2xhVPH+te0aKgFjmBvGlL1dH+ql2tgGO3MVrbb3jCKyvpgnxA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@isaacs/balanced-match": "^4.0.1" + }, + "engines": { + "node": "20 || >=22" + } + }, "node_modules/@isaacs/cliui": { "version": "8.0.2", "resolved": "https://registry.npmjs.org/@isaacs/cliui/-/cliui-8.0.2.tgz", @@ -6258,6 +6282,23 @@ "node": "^12.13.0 || ^14.15.0 || >=16.0.0" } }, + "node_modules/@npmcli/move-file/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "deprecated": "Rimraf versions prior to v4 are no longer supported", + "dev": true, + "license": "ISC", + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/@octokit/app": { "version": "14.1.0", "resolved": "https://registry.npmjs.org/@octokit/app/-/app-14.1.0.tgz", @@ -9016,6 +9057,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9032,6 +9074,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9048,6 +9091,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -9064,6 +9108,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9080,6 +9125,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9096,6 +9142,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9112,6 +9159,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9128,6 +9176,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9144,6 +9193,7 @@ "cpu": [ "ia32" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -9160,6 +9210,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -12962,6 +13013,69 @@ "node": ">=10" } }, + "node_modules/cacache/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "deprecated": "Rimraf versions prior to v4 are no longer supported", + "dev": true, + "license": "ISC", + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/cacache/node_modules/rimraf/node_modules/brace-expansion": { + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/cacache/node_modules/rimraf/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "deprecated": "Glob versions prior to v9 are no longer supported", + "dev": true, + "license": "ISC", + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/cacache/node_modules/rimraf/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "license": "ISC", + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, "node_modules/cacache/node_modules/tar": { "version": "6.2.1", "resolved": "https://registry.npmjs.org/tar/-/tar-6.2.1.tgz", @@ -24804,15 +24918,91 @@ "license": "MIT" }, "node_modules/rimraf": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", - "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "version": "6.1.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-6.1.2.tgz", + "integrity": "sha512-cFCkPslJv7BAXJsYlK1dZsbP8/ZNLkCAQ0bi1hf5EKX2QHegmDFEFA6QhuYJlk7UDdc+02JjO80YSOrWPpw06g==", "dev": true, + "license": "BlueOak-1.0.0", "dependencies": { - "glob": "^7.1.3" + "glob": "^13.0.0", + "package-json-from-dist": "^1.0.1" }, "bin": { - "rimraf": "bin.js" + "rimraf": "dist/esm/bin.mjs" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/rimraf/node_modules/glob": { + "version": "13.0.0", + "resolved": "https://registry.npmjs.org/glob/-/glob-13.0.0.tgz", + "integrity": "sha512-tvZgpqk6fz4BaNZ66ZsRaZnbHvP/jG3uKJvAZOwEVUL4RTA5nJeeLYfyN9/VA8NX/V3IBG+hkeuGpKjvELkVhA==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "minimatch": "^10.1.1", + "minipass": "^7.1.2", + "path-scurry": "^2.0.0" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/rimraf/node_modules/lru-cache": { + "version": "11.2.4", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.2.4.tgz", + "integrity": "sha512-B5Y16Jr9LB9dHVkh6ZevG+vAbOsNOYCX+sXvFWFu7B3Iz5mijW3zdbMyhsh8ANd2mSWBYdJgnqi+mL7/LrOPYg==", + "dev": true, + "license": "BlueOak-1.0.0", + "engines": { + "node": "20 || >=22" + } + }, + "node_modules/rimraf/node_modules/minimatch": { + "version": "10.1.1", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.1.1.tgz", + "integrity": "sha512-enIvLvRAFZYXJzkCYG5RKmPfrFArdLv+R+lbQ53BmIMLIry74bjKzX6iHAm8WYamJkhSSEabrWN5D97XnKObjQ==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "@isaacs/brace-expansion": "^5.0.0" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/rimraf/node_modules/minipass": { + "version": "7.1.2", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.2.tgz", + "integrity": "sha512-qOOzS1cBTWYF4BH8fVePDBOO9iptMnGUEZwNc/cMWnTV2nVLZ7VoNWEPHkYczZA0pdoA7dl6e7FL659nX9S2aw==", + "dev": true, + "license": "ISC", + "engines": { + "node": ">=16 || 14 >=14.17" + } + }, + "node_modules/rimraf/node_modules/path-scurry": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/path-scurry/-/path-scurry-2.0.1.tgz", + "integrity": "sha512-oWyT4gICAu+kaA7QWk/jvCHWarMKNs6pXOGWKDTr7cw4IGcUbW+PeTfbaQiLGheFRpjo6O9J0PmyMfQPjH71oA==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "lru-cache": "^11.0.0", + "minipass": "^7.1.2" + }, + "engines": { + "node": "20 || >=22" }, "funding": { "url": "https://github.com/sponsors/isaacs" diff --git a/package.json b/package.json index bb56147f7a..2a2cbd61e3 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,7 @@ "react": "^18.2.0", "react-dom": "^18.2.0", "resize-observer-polyfill": "^1.5.1", + "rimraf": "^6.1.2", "tailwindcss": "^3.3.6", "ts-jest": "^29.4.6", "typescript": "~5.9.3", diff --git a/src/__mocks__/electron.ts b/src/__mocks__/electron.ts index 4fccacca5f..93752dff67 100644 --- a/src/__mocks__/electron.ts +++ b/src/__mocks__/electron.ts @@ -15,6 +15,7 @@ export const app = { getPreferredSystemLanguages: jest.fn( () => [ 'en-US' ] ), requestSingleInstanceLock: jest.fn( () => true ), on: jest.fn(), + off: jest.fn(), setAppLogsPath: jest.fn(), setAsDefaultProtocolClient: jest.fn(), enableSandbox: jest.fn(), diff --git a/src/hooks/use-site-details.tsx b/src/hooks/use-site-details.tsx index 104cd0c426..d2c2bb8052 100644 --- a/src/hooks/use-site-details.tsx +++ b/src/hooks/use-site-details.tsx @@ -188,21 +188,6 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) { } } ); - /* - * Site Update Listeners - * - * Two complementary watchers keep the UI in sync with external changes: - * - * 1. 'site-status-changed' (from Site Status Watcher - execute-site-watch-command.ts): - * - Source: PM2 process events via `studio site list --watch` - * - Detects: Site start/stop/crash events - * - * 2. 'user-data-updated' (from User Data Watcher - user-data-watcher.ts): - * - Source: fs.watch on the appdata file - * - Detects: ALL changes (new sites, edits, deletions) - * - Used for: CLI site creation, property changes, external modifications - * - */ useIpcListener( 'site-status-changed', ( _, { siteId, status, url } ) => { setSites( ( prevSites ) => prevSites.map( ( site ) => @@ -211,16 +196,6 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) { ); } ); - useIpcListener( 'user-data-updated', async () => { - const updatedSites = await getIpcApi().getSiteDetails(); - setSites( updatedSites ); - - // Handle case where selected site was deleted externally - if ( selectedSiteId && ! updatedSites.find( ( site ) => site.id === selectedSiteId ) ) { - setSelectedSiteId( updatedSites.length ? updatedSites[ 0 ].id : '' ); - } - } ); - const toggleLoadingServerForSite = useCallback( ( siteId: string ) => { setLoadingServer( ( currentLoading ) => ( { ...currentLoading, diff --git a/src/index.ts b/src/index.ts index 8e1ad39745..01173c6843 100644 --- a/src/index.ts +++ b/src/index.ts @@ -54,7 +54,7 @@ import { unlockAppdata, updateAppdata, } from 'src/storage/user-data'; -import { setupUpdates } from 'src/updates'; +import { getAutoUpdaterState, setupUpdates } from 'src/updates'; // eslint-disable-next-line import/order import packageJson from '../package.json'; @@ -356,9 +356,11 @@ async function appBoot() { await renameLaunchUniquesStat(); - await createMainWindow(); await startUserDataWatcher(); - startSiteWatcher(); + + await startSiteWatcher(); + + await createMainWindow(); const userData = await loadUserData(); // Bump stats for the first time the app runs - this is when no lastBumpStats are available @@ -398,12 +400,14 @@ async function appBoot() { } } ); - app.on( 'will-quit', () => { - globalShortcut.unregisterAll(); - } ); - - let isQuittingConfirmed = false; + /** + * We want to stop all running sites (including the process daemon) in any of these cases: + * - There's a pending auto-update + * - There are no running sites (in which case we kill just the daemon) + * - There are running sites, and the user has confirmed they want to stop them upon closing the app + */ let shouldStopSitesOnQuit = true; + let isQuittingConfirmed = false; app.on( 'before-quit', ( event ) => { if ( isQuittingConfirmed ) { @@ -447,7 +451,7 @@ async function appBoot() { } const runningSiteCount = getRunningSiteCount(); - if ( runningSiteCount > 0 ) { + if ( getAutoUpdaterState() !== 'waiting-for-restart' && runningSiteCount > 0 ) { event.preventDefault(); void ( async () => { @@ -506,12 +510,21 @@ async function appBoot() { } } ); - app.on( 'quit', () => { - if ( shouldStopSitesOnQuit ) { - void stopAllServersOnQuit(); - } + app.on( 'will-quit', ( event ) => { + globalShortcut.unregisterAll(); stopUserDataWatcher(); stopSiteWatcher(); + + if ( shouldStopSitesOnQuit ) { + event.preventDefault(); + stopAllServersOnQuit() + .then( () => { + app.exit(); + } ) + .catch( () => { + app.exit(); + } ); + } } ); app.on( 'activate', () => { diff --git a/src/ipc-handlers.ts b/src/ipc-handlers.ts index c487e6faf2..18069e41c1 100644 --- a/src/ipc-handlers.ts +++ b/src/ipc-handlers.ts @@ -174,19 +174,7 @@ function mergeSiteDetailsWithRunningDetails( sites: SiteDetails[] ): SiteDetails return sites.map( ( site ) => { const server = SiteServer.get( site.id ); if ( server ) { - // Merge fresh data from disk with running state from server - // This ensures external changes (e.g., from CLI) are reflected - if ( server.details.running ) { - return { - ...site, - running: true as const, - url: server.details.url, - }; - } - return { - ...site, - running: false as const, - }; + return server.details; } return site; } ); @@ -396,14 +384,19 @@ export async function updateSite( const freshSiteData = userData.sites.find( ( s ) => s.id === updatedSite.id ); if ( freshSiteData ) { const wasRunning = server.details.running; - const url = wasRunning ? ( server.details as StartedSiteDetails ).url : undefined; - if ( wasRunning && url ) { + if ( wasRunning ) { + const url = freshSiteData.customDomain + ? `${ freshSiteData.enableHttps ? 'https' : 'http' }://${ freshSiteData.customDomain }` + : `http://localhost:${ freshSiteData.port }`; + server.details = { ...freshSiteData, running: true, url, }; + + server.server.url = url; } else { server.details = { ...freshSiteData, diff --git a/src/modules/cli/lib/execute-command.ts b/src/modules/cli/lib/execute-command.ts index f249ccef08..49cd5e1634 100644 --- a/src/modules/cli/lib/execute-command.ts +++ b/src/modules/cli/lib/execute-command.ts @@ -1,3 +1,4 @@ +import { app } from 'electron'; import { fork, ChildProcess, StdioOptions } from 'node:child_process'; import EventEmitter from 'node:events'; import * as Sentry from '@sentry/electron/main'; @@ -10,8 +11,9 @@ export interface CliCommandResult { } type CliCommandEventMap = { - data: { data: unknown }; + started: void; error: { error: Error }; + data: { data: unknown }; success: { result?: CliCommandResult }; failure: { result?: CliCommandResult }; }; @@ -62,11 +64,23 @@ export function executeCliCommand( } ); const eventEmitter = new CliCommandEventEmitter(); + child.on( 'spawn', () => { + eventEmitter.emit( 'started' ); + } ); + + child.on( 'error', ( error ) => { + console.error( 'Child process error:', error ); + Sentry.captureException( error ); + eventEmitter.emit( 'error', { error } ); + } ); + let stdout = ''; let stderr = ''; if ( options.output === 'capture' ) { - const logPrefix = options.logPrefix ? `[CLI - ${ options.logPrefix }]` : '[CLI]'; + const logPrefix = options.logPrefix + ? `[CLI - pid ${ child.pid } - site ID ${ options.logPrefix }]` + : '[CLI]'; child.stdout?.on( 'data', ( data: Buffer ) => { const text = data.toString(); stdout += text; @@ -84,20 +98,21 @@ export function executeCliCommand( eventEmitter.emit( 'data', { data: message } ); } ); - child.on( 'error', ( error ) => { - console.error( 'Child process error:', error ); - Sentry.captureException( error ); - eventEmitter.emit( 'error', { error } ); - } ); - let capturedExitCode: number | null = null; child.on( 'exit', ( code ) => { capturedExitCode = code; } ); + function appQuitHandler() { + const pid = child.pid; + const result = child.kill(); + console.log( `Child process with pid ${ pid } killed with result: ${ result }` ); + } + child.on( 'close', ( code ) => { child.removeAllListeners(); + app.off( 'will-quit', appQuitHandler ); const exitCode = capturedExitCode ?? code ?? 1; const result: CliCommandResult | undefined = @@ -110,9 +125,7 @@ export function executeCliCommand( } } ); - process.on( 'exit', () => { - child.kill(); - } ); + app.on( 'will-quit', appQuitHandler ); return [ eventEmitter, child ]; } diff --git a/src/modules/cli/lib/execute-site-watch-command.ts b/src/modules/cli/lib/execute-site-watch-command.ts index 4a310de1b7..2b12d4ba50 100644 --- a/src/modules/cli/lib/execute-site-watch-command.ts +++ b/src/modules/cli/lib/execute-site-watch-command.ts @@ -1,31 +1,3 @@ -/** - * Site Status Watcher - * - * This module monitors site running/stopped status changes by subscribing to PM2 process events - * via `studio site list --watch`. It's primarily used to detect status changes that occur outside - * of Studio's direct control, such as: - * - Sites started/stopped via CLI commands - * - Site crashes or unexpected process terminations - * - * IMPORTANT: Architecture Notes - * ----------------------------- - * There are currently TWO separate watchers that update the UI with site changes: - * - * 1. Site Status Watcher (this file): - * - Monitors PM2 process events (start/stop/crash) - * - Only detects running/stopped status changes - * - Sends 'site-status-changed' IPC events to the renderer - * - * 2. User Data Watcher (src/lib/user-data-watcher.ts): - * - Monitors the appdata file directly via fs.watch - * - Detects ALL changes to site data (new sites, edits, deletions) - * - Sends 'user-data-updated' IPC events to the renderer - * - * The renderer (use-site-details.tsx) listens to BOTH: - * - 'site-status-changed': Updates running/stopped status for existing sites - * - 'user-data-updated': Refreshes the entire site list (handles new sites, edits, deletions) - * - */ import { z } from 'zod'; import { sendIpcEventToRenderer } from 'src/ipc-utils'; import { executeCliCommand } from 'src/modules/cli/lib/execute-command'; @@ -92,37 +64,44 @@ async function updateSiteServerStatus( await current; } -export function startSiteWatcher(): void { - if ( watcher ) { - return; - } +export async function startSiteWatcher(): Promise< void > { + return new Promise( ( resolve, reject ) => { + if ( watcher ) { + return resolve(); + } - watcher = executeCliCommand( [ 'site', 'list', '--watch', '--format', 'json' ], { - output: 'ignore', - } ); - const [ eventEmitter ] = watcher; + watcher = executeCliCommand( [ 'site', 'list', '--watch', '--format', 'json' ], { + output: 'ignore', + } ); + const [ eventEmitter ] = watcher; - eventEmitter.on( 'data', ( { data } ) => { - const parsed = siteStatusEventSchema.safeParse( data ); - if ( ! parsed.success ) { - return; - } + eventEmitter.on( 'started', () => { + resolve(); + } ); - const { siteId, status, url } = parsed.data.value; - const isRunning = status === 'running'; + eventEmitter.on( 'error', ( { error } ) => { + reject(); + console.error( 'Site watcher error:', error ); + watcher = null; + } ); - void updateSiteServerStatus( siteId, isRunning, url ); - void sendIpcEventToRenderer( 'site-status-changed', parsed.data.value ); - } ); + eventEmitter.on( 'data', ( { data } ) => { + const parsed = siteStatusEventSchema.safeParse( data ); + if ( ! parsed.success ) { + return; + } - eventEmitter.on( 'error', ( { error } ) => { - console.error( 'Site watcher error:', error ); - watcher = null; - } ); + const { siteId, status, url } = parsed.data.value; + const isRunning = status === 'running'; - eventEmitter.on( 'failure', () => { - console.warn( 'Site watcher exited unexpectedly' ); - watcher = null; + void updateSiteServerStatus( siteId, isRunning, url ); + void sendIpcEventToRenderer( 'site-status-changed', parsed.data.value ); + } ); + + eventEmitter.on( 'failure', () => { + console.warn( 'Site watcher exited unexpectedly' ); + watcher = null; + } ); } ); } diff --git a/src/site-server.ts b/src/site-server.ts index 5fb246a8e8..9078d27b69 100644 --- a/src/site-server.ts +++ b/src/site-server.ts @@ -30,14 +30,13 @@ export async function createSiteWorkingDirectory( } export async function stopAllServersOnQuit() { - // We're quitting so this doesn't have to be tidy, just stop the - // servers as directly as possible. - // Preserve autoStart so sites will restart on next app launch. - // Use silent mode to avoid terminal errors during quit. + // The `--auto-start` option ensures sites will restart on next app launch. return new Promise< void >( ( resolve ) => { - const [ emitter ] = executeCliCommand( [ 'site', 'stop', '--all', '--auto-start' ], { - output: 'ignore', - } ); + const [ emitter, childProcess ] = executeCliCommand( + [ 'site', 'stop', '--all', '--auto-start' ], + { output: 'ignore' } + ); + console.log( `Spawned stop-all child process with pid ${ childProcess.pid }` ); emitter.on( 'success', () => resolve() ); emitter.on( 'failure', () => resolve() ); emitter.on( 'error', () => resolve() ); @@ -156,6 +155,10 @@ export class SiteServer { servers.set( siteDetails.id, server ); server.details = siteDetails; + if ( siteDetails.running && siteDetails.url ) { + server.server.url = siteDetails.url; + } + return { server, details: siteDetails }; } finally { server.hasOngoingOperation = false; @@ -185,6 +188,11 @@ export class SiteServer { const userData = await loadUserData(); const freshSiteData = userData.sites.find( ( s ) => s.id === this.details.id ); + + if ( freshSiteData?.port ) { + this.details.port = freshSiteData.port; + } + const url = getAbsoluteUrl( this.details ); this.details = { @@ -194,6 +202,8 @@ export class SiteServer { autoStart: true, latestCliPid: freshSiteData?.latestCliPid, }; + + this.server.url = url; } finally { this.hasOngoingOperation = false; } diff --git a/src/tests/index.test.ts b/src/tests/index.test.ts index dddb34bae8..3deb0d4b3d 100644 --- a/src/tests/index.test.ts +++ b/src/tests/index.test.ts @@ -128,8 +128,6 @@ describe( 'App initialization', () => { expect( setupSpy.mock.invocationCallOrder[ 0 ] ).toBeLessThan( ( createMainWindow as jest.Mock ).mock.invocationCallOrder[ 0 ] ); - - await mockedEvents.quit(); } ); } ); @@ -146,8 +144,6 @@ describe( 'App initialization', () => { await mockedEvents.ready(); await mockedEvents.activate(); expect( createMainWindow ).toHaveBeenCalled(); - - await mockedEvents.quit(); } ); } ); @@ -179,8 +175,6 @@ describe( 'App initialization', () => { expect( getMainWindow as jest.Mock ).toHaveBeenCalled(); Object.defineProperty( process, 'platform', { value: originalProcessPlatform } ); - - await mockedEvents.quit(); } ); } ); } ); diff --git a/src/tests/ipc-handlers.test.ts b/src/tests/ipc-handlers.test.ts index b95905b902..67c0659484 100644 --- a/src/tests/ipc-handlers.test.ts +++ b/src/tests/ipc-handlers.test.ts @@ -283,11 +283,9 @@ describe( 'getXdebugEnabledSite', () => { const result = await getXdebugEnabledSite( mockIpcMainInvokeEvent ); expect( result ).toEqual( { - autoStart: false, id: 'site-2', name: 'Site 2', path: '/path/to/site-2', - phpVersion: '8.0', running: true, enableXdebug: true, } ); @@ -304,11 +302,9 @@ describe( 'getXdebugEnabledSite', () => { ( fs.existsSync as jest.Mock ).mockReturnValue( true ); ( SiteServer.get as jest.Mock ).mockReturnValue( { details: { - autoStart: false, id: 'site-1', name: 'Site 1', path: '/path/to/site-1', - phpVersion: '8.0', running: false, enableXdebug: true, }, @@ -317,11 +313,9 @@ describe( 'getXdebugEnabledSite', () => { const result = await getXdebugEnabledSite( mockIpcMainInvokeEvent ); expect( result ).toEqual( { - autoStart: false, id: 'site-1', name: 'Site 1', path: '/path/to/site-1', - phpVersion: '8.0', running: false, enableXdebug: true, } ); diff --git a/src/updates.ts b/src/updates.ts index 21c6d1d879..ddc50cf57a 100644 --- a/src/updates.ts +++ b/src/updates.ts @@ -22,6 +22,10 @@ let showManualCheckDialogs = false; const shouldPoll = process.env.NODE_ENV === 'production' && app.isPackaged && ! isDevRelease( app.getVersion() ); +export function getAutoUpdaterState() { + return updaterState; +} + export function setupUpdates() { if ( process.env.E2E ) { console.log( 'Skipping update server setup in E2E tests' );