diff --git a/apps/cli/commands/_events.ts b/apps/cli/commands/_events.ts index b66f2c6589..8ee43e0a6c 100644 --- a/apps/cli/commands/_events.ts +++ b/apps/cli/commands/_events.ts @@ -29,6 +29,7 @@ import { SITE_EVENTS_SOCKET_PATH, getDaemonBus, } from 'cli/lib/daemon-client'; +import { getLiveSiteOperation } from 'cli/lib/site-operations'; import { isSiteRunning } from 'cli/lib/site-utils'; import { SocketServer } from 'cli/lib/socket'; import { SITE_PROCESS_PREFIX } from 'cli/lib/wordpress-server-manager'; @@ -39,6 +40,8 @@ const logger = new Logger< LoggerAction >(); function toSiteDetails( site: SiteData ) { return siteDetailsSchema.parse( { ...site, + // Overrides rather than augments `...site` — see the note in `site list`. + operation: getLiveSiteOperation( site ), url: getSiteUrl( site ), } ); } @@ -138,6 +141,7 @@ export async function runCommand(): Promise< void > { case SITE_EVENTS.CREATED: case SITE_EVENTS.UPDATED: case SITE_EVENTS.DELETED: + case SITE_EVENTS.OPERATIONS_CHANGED: void emitSiteEvent( parsed.event, parsed.data.siteId ); break; } diff --git a/apps/cli/commands/config/set.ts b/apps/cli/commands/config/set.ts index 18a42cd0e3..e07df34860 100644 --- a/apps/cli/commands/config/set.ts +++ b/apps/cli/commands/config/set.ts @@ -44,6 +44,7 @@ import { connectToDaemon, disconnectFromDaemon, emitCliEvent } from 'cli/lib/dae import { updateDomainInHosts } from 'cli/lib/hosts-file'; import { validateSupportedPhpVersion } from 'cli/lib/php-versions'; import { runWpCliCommand } from 'cli/lib/run-wp-cli-command'; +import { withSiteOperation } from 'cli/lib/site-operations'; import { setupCustomDomain } from 'cli/lib/site-utils'; import { ValidationError } from 'cli/lib/validation-error'; import { @@ -73,6 +74,14 @@ export interface SetCommandOptions { } export async function runCommand( sitePath: string, options: SetCommandOptions ): Promise< void > { + const validated = validateSetOptions( options ); + return withSiteOperation( sitePath, 'settings', () => setSiteConfig( sitePath, validated ) ); +} + +// Runs before the operation is recorded, so an invalid edit fails without +// touching the config file or briefly blocking the site. Returns the +// options with `adminEmail` normalized (blank means "leave it alone"). +function validateSetOptions( options: SetCommandOptions ): SetCommandOptions { const { name, domain, @@ -126,6 +135,12 @@ export async function runCommand( sitePath: string, options: SetCommandOptions ) throw new LoggerError( __( 'Admin password cannot be empty.' ) ); } + // Static check, so it belongs out here with the rest. The runtime-specific + // PHP check further down needs the site record and has to stay inside. + if ( options.php !== undefined ) { + validateSupportedPhpVersion( options.php ); + } + if ( adminEmail !== undefined ) { if ( ! adminEmail.trim() ) { adminEmail = undefined; @@ -137,6 +152,26 @@ export async function runCommand( sitePath: string, options: SetCommandOptions ) } } + return { ...options, adminEmail }; +} + +async function setSiteConfig( sitePath: string, options: SetCommandOptions ): Promise< void > { + const { + name, + domain, + https, + php, + wp, + runtime, + fileAccess, + xdebug, + adminUsername, + adminPassword, + adminEmail, + debugLog, + debugDisplay, + } = options; + try { logger.reportStart( LoggerAction.LOAD_SITES, __( 'Loading site…' ) ); let site = await getSiteByFolder( sitePath ); diff --git a/apps/cli/commands/config/tests/set.test.ts b/apps/cli/commands/config/tests/set.test.ts index 2b90619629..d3da3044cd 100644 --- a/apps/cli/commands/config/tests/set.test.ts +++ b/apps/cli/commands/config/tests/set.test.ts @@ -51,6 +51,13 @@ vi.mock( 'cli/lib/cli-config/sites', async () => { vi.mock( 'cli/lib/certificate-manager' ); vi.mock( 'cli/lib/hosts-file' ); vi.mock( 'cli/lib/daemon-client' ); +// Run the command body directly: this suite covers the command, not the +// operation guard (lib/tests/site-operations.test.ts does that). Spreading the real module keeps +// any other export real rather than silently stubbing it. +vi.mock( 'cli/lib/site-operations', async ( importOriginal ) => ( { + ...( await importOriginal< typeof import('cli/lib/site-operations') >() ), + withSiteOperation: vi.fn( ( _folder: string, _kind: string, fn: () => unknown ) => fn() ), +} ) ); vi.mock( 'cli/lib/run-wp-cli-command' ); vi.mock( 'cli/lib/site-utils' ); vi.mock( 'cli/lib/wordpress-server-manager' ); @@ -160,6 +167,16 @@ describe( 'CLI: studio config set', () => { ); } ); + // Validation runs before the operation is recorded, so a rejected + // edit neither writes the config nor briefly blocks the site. + it( 'should reject an invalid edit without claiming the site', async () => { + const { withSiteOperation } = await import( 'cli/lib/site-operations' ); + + await expect( runCommand( testSitePath, { php: '8.1' } ) ).rejects.toThrow(); + + expect( withSiteOperation ).not.toHaveBeenCalled(); + } ); + it( 'should throw when PHP version is not supported', async () => { await expect( runCommand( testSitePath, { php: '8.1' } ) ).rejects.toThrow( 'PHP 8.1 is not supported. Supported versions: 8.5, 8.4, 8.3, 8.2.' diff --git a/apps/cli/commands/site/delete.ts b/apps/cli/commands/site/delete.ts index 62837d9ed5..28fe96499a 100644 --- a/apps/cli/commands/site/delete.ts +++ b/apps/cli/commands/site/delete.ts @@ -19,6 +19,7 @@ import { import { getSiteByFolder } from 'cli/lib/cli-config/sites'; import { connectToDaemon, disconnectFromDaemon, emitCliEvent } from 'cli/lib/daemon-client'; import { removeDomainFromHosts } from 'cli/lib/hosts-file'; +import { withSiteOperation } from 'cli/lib/site-operations'; import { stopProxyIfNoSitesNeedIt } from 'cli/lib/site-utils'; import { getSnapshotsFromConfig, deleteSnapshotFromConfig } from 'cli/lib/snapshots'; import { getTracksOrigin, recordTracksEvent, TRACKS_EVENTS } from 'cli/lib/tracks'; @@ -70,6 +71,10 @@ export async function runCommand( siteFolder: string, deleteFiles: boolean = true ): Promise< void > { + return withSiteOperation( siteFolder, 'delete', () => deleteSite( siteFolder, deleteFiles ) ); +} + +async function deleteSite( siteFolder: string, deleteFiles: boolean ): Promise< void > { try { logger.reportStart( LoggerAction.START_DAEMON, __( 'Starting process daemon…' ) ); await connectToDaemon(); diff --git a/apps/cli/commands/site/list.ts b/apps/cli/commands/site/list.ts index 38df4b631d..35ba9ebb09 100644 --- a/apps/cli/commands/site/list.ts +++ b/apps/cli/commands/site/list.ts @@ -5,6 +5,7 @@ import CliTable3 from 'cli-table3'; import { readCliConfig, type SiteData } from 'cli/lib/cli-config/core'; import { getSiteUrl } from 'cli/lib/cli-config/sites'; import { connectToDaemon, disconnectFromDaemon } from 'cli/lib/daemon-client'; +import { getLiveSiteOperation } from 'cli/lib/site-operations'; import { isSiteRunning } from 'cli/lib/site-utils'; import { getColumnWidths, getPrettyPath } from 'cli/lib/utils'; import { Logger, LoggerError } from 'cli/logger'; @@ -44,6 +45,10 @@ async function getSiteListData( sites: SiteData[] ): Promise< { jsonEntries.push( { ...site, + // Overrides the stored value from `...site`: it can still name a + // process that has died, and both front ends decide which site + // actions to disable from what this reports. + operation: getLiveSiteOperation( site ), url, running, } ); diff --git a/apps/cli/commands/site/start.ts b/apps/cli/commands/site/start.ts index 78542c78aa..890fcf1c1f 100644 --- a/apps/cli/commands/site/start.ts +++ b/apps/cli/commands/site/start.ts @@ -5,6 +5,7 @@ import { __, sprintf } from '@wordpress/i18n'; import { getSiteByFolder, updateSiteLatestCliPid } from 'cli/lib/cli-config/sites'; import { connectToDaemon, disconnectFromDaemon } from 'cli/lib/daemon-client'; import { getAiInstructionsPath } from 'cli/lib/dependency-management/paths'; +import { withSiteOperation } from 'cli/lib/site-operations'; import { logSiteDetails, openSiteInBrowser, setupCustomDomain } from 'cli/lib/site-utils'; import { keepSqliteIntegrationUpdated } from 'cli/lib/sqlite-integration'; import { isServerRunning, startWordPressServer } from 'cli/lib/wordpress-server-manager'; @@ -17,6 +18,16 @@ export async function runCommand( sitePath: string, skipBrowser = false, skipLogDetails = false +): Promise< void > { + return withSiteOperation( sitePath, 'start', () => + startSite( sitePath, skipBrowser, skipLogDetails ) + ); +} + +async function startSite( + sitePath: string, + skipBrowser: boolean, + skipLogDetails: boolean ): Promise< void > { try { logger.reportStart( LoggerAction.START_DAEMON, __( 'Starting process daemon…' ) ); diff --git a/apps/cli/commands/site/stop.ts b/apps/cli/commands/site/stop.ts index 1605695b01..58d7a9c960 100644 --- a/apps/cli/commands/site/stop.ts +++ b/apps/cli/commands/site/stop.ts @@ -13,6 +13,7 @@ import { disconnectFromDaemon, killDaemonAndChildren, } from 'cli/lib/daemon-client'; +import { withSiteOperation } from 'cli/lib/site-operations'; import { stopProxyIfNoSitesNeedIt } from 'cli/lib/site-utils'; import { getTracksOrigin, recordTracksEvent, TRACKS_EVENTS } from 'cli/lib/tracks'; import { @@ -53,6 +54,16 @@ export async function runCommand( siteFolder: undefined ): Promise< void >; export async function runCommand( target: Mode, siteFolder: string | undefined ): Promise< void > { + // Stopping everything is the quit path — it kills the daemon outright, so + // there is no per-site operation to take (and taking one for every site could + // block on an operation this is about to terminate anyway). + if ( target === Mode.STOP_SINGLE_SITE && siteFolder ) { + return withSiteOperation( siteFolder, 'stop', () => stopSites( target, siteFolder ) ); + } + return stopSites( target, siteFolder ); +} + +async function stopSites( target: Mode, siteFolder: string | undefined ): Promise< void > { try { await connectToDaemon(); diff --git a/apps/cli/commands/site/tests/delete.test.ts b/apps/cli/commands/site/tests/delete.test.ts index f889806248..830effc63c 100644 --- a/apps/cli/commands/site/tests/delete.test.ts +++ b/apps/cli/commands/site/tests/delete.test.ts @@ -55,6 +55,13 @@ vi.mock( 'cli/lib/cli-config/sites', async () => { vi.mock( 'cli/lib/certificate-manager' ); vi.mock( 'cli/lib/hosts-file' ); vi.mock( 'cli/lib/daemon-client' ); +// Run the command body directly: these suites cover the command, not the +// operation guard (lib/tests/site-operations.test.ts does that). Spreading the real module keeps +// any other export real rather than silently stubbing it. +vi.mock( 'cli/lib/site-operations', async ( importOriginal ) => ( { + ...( await importOriginal< typeof import('cli/lib/site-operations') >() ), + withSiteOperation: ( _folder: string, _kind: string, fn: () => unknown ) => fn(), +} ) ); vi.mock( 'cli/lib/site-utils' ); vi.mock( 'cli/lib/snapshots' ); vi.mock( 'cli/lib/wordpress-server-manager' ); diff --git a/apps/cli/commands/site/tests/list.test.ts b/apps/cli/commands/site/tests/list.test.ts index c22652fcef..c8ad19cd20 100644 --- a/apps/cli/commands/site/tests/list.test.ts +++ b/apps/cli/commands/site/tests/list.test.ts @@ -122,6 +122,25 @@ describe( 'CLI: studio site list', () => { expect( disconnectFromDaemon ).toHaveBeenCalled(); } ); + // Both front ends disable a site's actions on what this reports, so an + // entry left behind by a crashed process must not survive into the payload. + it( 'should omit an operation whose owning process is gone', async () => { + vi.mocked( readCliConfig ).mockResolvedValue( { + ...testCliConfig, + sites: [ + { + ...testCliConfig.sites[ 0 ], + operation: { pid: 0x7ffffffe, kind: 'delete' as const }, + }, + ], + } ); + + await runCommand( 'json' ); + + const [ , json ] = mockReportKeyValuePair.mock.calls[ 0 ]; + expect( JSON.parse( json )[ 0 ] ).not.toHaveProperty( 'operation' ); + } ); + it( 'should handle no sites found', async () => { vi.mocked( readCliConfig ).mockResolvedValue( emptyCliConfig ); diff --git a/apps/cli/commands/site/tests/start.test.ts b/apps/cli/commands/site/tests/start.test.ts index dcc2d0ee1a..a9a9c1b9ab 100644 --- a/apps/cli/commands/site/tests/start.test.ts +++ b/apps/cli/commands/site/tests/start.test.ts @@ -16,6 +16,13 @@ vi.mock( 'cli/lib/cli-config/sites', async () => ( { updateSiteLatestCliPid: vi.fn(), } ) ); vi.mock( 'cli/lib/daemon-client' ); +// Run the command body directly: these suites cover the command, not the +// operation guard (lib/tests/site-operations.test.ts does that). Spreading the real module keeps +// any other export real rather than silently stubbing it. +vi.mock( 'cli/lib/site-operations', async ( importOriginal ) => ( { + ...( await importOriginal< typeof import('cli/lib/site-operations') >() ), + withSiteOperation: ( _folder: string, _kind: string, fn: () => unknown ) => fn(), +} ) ); vi.mock( 'cli/lib/site-utils' ); vi.mock( 'cli/lib/wordpress-server-manager' ); vi.mock( 'cli/lib/sqlite-integration' ); diff --git a/apps/cli/commands/site/tests/stop.test.ts b/apps/cli/commands/site/tests/stop.test.ts index 351721cbfd..14323403b2 100644 --- a/apps/cli/commands/site/tests/stop.test.ts +++ b/apps/cli/commands/site/tests/stop.test.ts @@ -34,6 +34,13 @@ vi.mock( 'cli/lib/cli-config/sites', async () => { }; } ); vi.mock( 'cli/lib/daemon-client' ); +// Run the command body directly: these suites cover the command, not the +// operation guard (lib/tests/site-operations.test.ts does that). Spreading the real module keeps +// any other export real rather than silently stubbing it. +vi.mock( 'cli/lib/site-operations', async ( importOriginal ) => ( { + ...( await importOriginal< typeof import('cli/lib/site-operations') >() ), + withSiteOperation: ( _folder: string, _kind: string, fn: () => unknown ) => fn(), +} ) ); vi.mock( 'cli/lib/site-utils' ); vi.mock( 'cli/lib/wordpress-server-manager' ); vi.mock( 'cli/lib/tracks', async ( importActual ) => { diff --git a/apps/cli/lib/cli-config/core.ts b/apps/cli/lib/cli-config/core.ts index acf96b638e..05e02d2da6 100644 --- a/apps/cli/lib/cli-config/core.ts +++ b/apps/cli/lib/cli-config/core.ts @@ -8,6 +8,7 @@ import { import { siteDetailsSchema } from '@studio/common/lib/cli-events'; import { hideDirectoryOnWindows } from '@studio/common/lib/hide-dir-windows'; import { lockFileAsync, unlockFileAsync } from '@studio/common/lib/lockfile'; +import { siteOperationSchema } from '@studio/common/lib/site-operation'; import { getCliConfigPath, getConfigDirectory } from '@studio/common/lib/well-known-paths'; import { snapshotSchema } from '@studio/common/types/snapshot'; import { __ } from '@wordpress/i18n'; @@ -55,6 +56,10 @@ const siteSchema = siteDetailsSchema // first-full-pull vs. delta. Durable on the site record. importComplete: z.boolean().optional(), status: siteStatusSchema.default( 'ready' ).optional(), + // The in-flight Studio operation holding this site. Unlike `status`, it's + // transient: once its owning process is gone it's reclaimed on the next + // acquire. See `cli/lib/site-operations`. + operation: siteOperationSchema.optional(), } ) .loose(); @@ -166,18 +171,27 @@ export async function saveCliConfig( config: CliConfig ): Promise< void > { } } -const LOCKFILE_PATH = path.join( getConfigDirectory(), CLI_CONFIG_LOCKFILE_NAME ); +// Resolved per call, not at module load: `getConfigDirectory()` reads +// DEV_CONFIG_DIR, so pinning it at import time would bake in whatever the +// environment looked like when this module was first pulled in — and would +// throw outright for any importer that loads before the home path resolves. +function getLockfilePath(): string { + return path.join( getConfigDirectory(), CLI_CONFIG_LOCKFILE_NAME ); +} export async function lockCliConfig(): Promise< void > { // The lockfile lives inside the config directory. On a first run that directory may not exist // yet (e.g. telemetry bumps fire before `setupServerFiles()` creates it), and `lockfile.lock` // would reject with ENOENT instead of waiting. Ensure the directory exists before locking. await ensureConfigDirectory(); - await lockFileAsync( LOCKFILE_PATH, { wait: LOCKFILE_WAIT_TIME, stale: LOCKFILE_STALE_TIME } ); + await lockFileAsync( getLockfilePath(), { + wait: LOCKFILE_WAIT_TIME, + stale: LOCKFILE_STALE_TIME, + } ); } export async function unlockCliConfig(): Promise< void > { - await unlockFileAsync( LOCKFILE_PATH ); + await unlockFileAsync( getLockfilePath() ); } export async function updateCliConfigWithPartial( diff --git a/apps/cli/lib/site-operations.ts b/apps/cli/lib/site-operations.ts new file mode 100644 index 0000000000..3d99d15277 --- /dev/null +++ b/apps/cli/lib/site-operations.ts @@ -0,0 +1,138 @@ +import { SITE_EVENTS } from '@studio/common/lib/cli-events'; +import { getSiteOperationNoun } from '@studio/common/lib/site-operation-labels'; +import { __, sprintf } from '@wordpress/i18n'; +import { + lockCliConfig, + readCliConfig, + saveCliConfig, + unlockCliConfig, + type SiteData, +} from 'cli/lib/cli-config/core'; +import { getSiteByFolder } from 'cli/lib/cli-config/sites'; +import { emitCliEvent } from 'cli/lib/daemon-client'; +import { LoggerError } from 'cli/logger'; +import type { SiteOperation, SiteOperationKind } from '@studio/common/lib/site-operation'; + +function siteBusyError( requested: SiteOperationKind, blockedBy: SiteOperationKind ): LoggerError { + return new LoggerError( + sprintf( + /* translators: 1: operation the user asked for, e.g. "a site start". 2: operation already running, e.g. "a settings change". */ + __( + 'Cannot run %1$s: %2$s is already in progress for this site. Wait for it to finish and try again.' + ), + getSiteOperationNoun( requested ), + getSiteOperationNoun( blockedBy ) + ) + ); +} + +// Signal 0 only runs the existence and permission checks. EPERM means the +// process exists but belongs to another user, which still counts as alive. +function isProcessAlive( pid: number ): boolean { + try { + process.kill( pid, 0 ); + return true; + } catch ( error ) { + return ( error as NodeJS.ErrnoException ).code === 'EPERM'; + } +} + +/** + * The site's operation, or undefined once its owning process has died. Every + * path that reports a site to a client must go through this — an entry left + * behind by a crashed process would otherwise keep the site's actions disabled + * in the UI. + */ +export function getLiveSiteOperation( site: SiteData ): SiteOperation | undefined { + return site.operation && isProcessAlive( site.operation.pid ) ? site.operation : undefined; +} + +/** + * Records the operation against the site, throwing when another one already + * holds it. Runs inside the config lock so the read-check-write is atomic + * across CLI processes — the agent, the desktop app and a terminal all reach + * this through the same commands. + */ +async function acquire( siteId: string, kind: SiteOperationKind ): Promise< SiteOperation > { + const operation: SiteOperation = { pid: process.pid, kind }; + + try { + await lockCliConfig(); + const config = await readCliConfig(); + const site = config.sites.find( ( s ) => s.id === siteId ); + + if ( ! site ) { + throw new LoggerError( __( 'Site not found' ) ); + } + + const blocking = getLiveSiteOperation( site ); + if ( blocking ) { + throw siteBusyError( kind, blocking.kind ); + } + + site.operation = operation; + await saveCliConfig( config ); + } finally { + await unlockCliConfig(); + } + + return operation; +} + +async function release( siteId: string, operation: SiteOperation ): Promise< void > { + try { + await lockCliConfig(); + const config = await readCliConfig(); + const site = config.sites.find( ( s ) => s.id === siteId ); + + // A completed `site delete` removes the record entirely; nothing to release. + if ( ! site ) { + return; + } + + // Only clear our own: a reclaimed-then-reacquired site belongs to whoever + // holds it now. + if ( site.operation?.pid === operation.pid ) { + delete site.operation; + } + await saveCliConfig( config ); + } finally { + await unlockCliConfig(); + } +} + +// Both edges matter: without the release, an indicator raised by the daemon's +// own event mid-operation would never clear. +function emitOperationsChanged( siteId: string ): Promise< void > { + return emitCliEvent( { event: SITE_EVENTS.OPERATIONS_CHANGED, data: { siteId } } ); +} + +/** + * Runs `fn` while holding the site, so no other Studio operation can touch it + * concurrently. The operation is persisted on the site record, letting the UI + * disable the actions it blocks and the agent read back why one was refused. + * + * Addressed by folder because that's how every command receives its site. + */ +export async function withSiteOperation< T >( + siteFolder: string, + kind: SiteOperationKind, + fn: () => Promise< T > +): Promise< T > { + const { id: siteId } = await getSiteByFolder( siteFolder ); + const operation = await acquire( siteId, kind ); + await emitOperationsChanged( siteId ); + + try { + return await fn(); + } finally { + // Releasing must never replace the operation's own failure: if the config + // lock times out here, `fn`'s error is the one worth seeing. + try { + await release( siteId, operation ); + } catch ( error ) { + console.error( 'Failed to release the site operation:', error ); + } + await emitOperationsChanged( siteId ); + } +} diff --git a/apps/cli/lib/tests/site-operations.test.ts b/apps/cli/lib/tests/site-operations.test.ts new file mode 100644 index 0000000000..6c7fc76f81 --- /dev/null +++ b/apps/cli/lib/tests/site-operations.test.ts @@ -0,0 +1,145 @@ +import { DEFAULT_PHP_VERSION } from '@studio/common/constants'; +import { SITE_EVENTS } from '@studio/common/lib/cli-events'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { + lockCliConfig, + readCliConfig, + saveCliConfig, + SiteData, + unlockCliConfig, +} from 'cli/lib/cli-config/core'; +import { emitCliEvent } from 'cli/lib/daemon-client'; +import { LoggerError } from 'cli/logger'; +import { getLiveSiteOperation, withSiteOperation } from '../site-operations'; + +vi.mock( 'cli/lib/cli-config/core', () => ( { + lockCliConfig: vi.fn(), + unlockCliConfig: vi.fn(), + readCliConfig: vi.fn(), + saveCliConfig: vi.fn(), +} ) ); +vi.mock( 'cli/lib/daemon-client', () => ( { emitCliEvent: vi.fn() } ) ); +vi.mock( 'cli/lib/cli-config/sites', () => ( { + getSiteByFolder: vi.fn( async ( folder: string ) => { + if ( folder !== site.path ) { + throw new LoggerError( 'Site not found' ); + } + return site; + } ), +} ) ); + +const site: SiteData = { + id: 'site-1', + name: 'My WordPress Site', + path: '/home/user/Studio/my-wordpress-site', + port: 8888, + phpVersion: DEFAULT_PHP_VERSION, +} as const; + +let mockConfig: Awaited< ReturnType< typeof readCliConfig > >; + +function storedOperation() { + return mockConfig.sites[ 0 ].operation; +} + +// A pid that is guaranteed not to be running, standing in for a client that +// crashed mid-operation. +const DEAD_PID = 0x7ffffffe; + +beforeEach( () => { + vi.clearAllMocks(); + mockConfig = { version: 1, sites: [ structuredClone( site ) ], snapshots: [] }; + vi.mocked( lockCliConfig ).mockResolvedValue( undefined ); + vi.mocked( unlockCliConfig ).mockResolvedValue( undefined ); + vi.mocked( readCliConfig ).mockImplementation( async () => structuredClone( mockConfig ) ); + vi.mocked( saveCliConfig ).mockImplementation( async ( config ) => { + mockConfig = structuredClone( config ); + } ); +} ); + +describe( 'getLiveSiteOperation', () => { + it( 'drops an operation whose owning process has died', () => { + expect( + getLiveSiteOperation( { ...site, operation: { pid: DEAD_PID, kind: 'delete' } } ) + ).toBeUndefined(); + } ); + + it( 'keeps an operation whose owning process is alive', () => { + expect( + getLiveSiteOperation( { ...site, operation: { pid: process.pid, kind: 'settings' } } ) + ).toEqual( { pid: process.pid, kind: 'settings' } ); + } ); + + it( 'returns undefined for an idle site', () => { + expect( getLiveSiteOperation( site ) ).toBeUndefined(); + } ); +} ); + +describe( 'withSiteOperation', () => { + it( 'records the operation while it runs and clears it afterwards', async () => { + await withSiteOperation( site.path, 'settings', async () => { + expect( storedOperation() ).toEqual( { pid: process.pid, kind: 'settings' } ); + } ); + + expect( storedOperation() ).toBeUndefined(); + } ); + + it( 'releases the operation when the work throws', async () => { + await expect( + withSiteOperation( site.path, 'settings', async () => { + throw new Error( 'settings blew up' ); + } ) + ).rejects.toThrow( 'settings blew up' ); + + expect( storedOperation() ).toBeUndefined(); + } ); + + it( 'refuses a second operation while one is held', async () => { + await withSiteOperation( site.path, 'settings', async () => { + await expect( + withSiteOperation( site.path, 'start', async () => 'started' ) + ).rejects.toThrow( /already in progress/ ); + } ); + } ); + + it( 'refuses an operation while another holds the site', async () => { + await withSiteOperation( site.path, 'delete', async () => { + await expect( withSiteOperation( site.path, 'stop', async () => undefined ) ).rejects.toThrow( + /already in progress/ + ); + } ); + } ); + + it( 'names both operations in the error so the agent can act on it', async () => { + await withSiteOperation( site.path, 'settings', async () => { + await expect( + withSiteOperation( site.path, 'start', async () => undefined ) + ).rejects.toThrow( /site start.*settings change/ ); + } ); + } ); + + it( 'reclaims an operation whose owning process is gone', async () => { + mockConfig.sites[ 0 ].operation = { pid: DEAD_PID, kind: 'delete' }; + + const result = await withSiteOperation( site.path, 'start', async () => 'started' ); + + expect( result ).toBe( 'started' ); + expect( storedOperation() ).toBeUndefined(); + } ); + + // Reusing SITE_EVENTS.UPDATED here made every acquire assert a running state + // and clear the desktop renderer's loading flag mid-operation, which broke + // stop/start badly enough to fail the startup performance metric. + it( 'announces operation changes without claiming to know the running state', async () => { + await withSiteOperation( site.path, 'settings', async () => undefined ); + + const events = vi.mocked( emitCliEvent ).mock.calls.map( ( [ payload ] ) => payload.event ); + expect( events ).toEqual( [ SITE_EVENTS.OPERATIONS_CHANGED, SITE_EVENTS.OPERATIONS_CHANGED ] ); + } ); + + it( 'does not lock a site that is not in the config', async () => { + await expect( + withSiteOperation( '/no/such/site', 'start', async () => undefined ) + ).rejects.toThrow( 'Site not found' ); + } ); +} ); diff --git a/apps/local/src/index.ts b/apps/local/src/index.ts index 7bae6c090e..8fd80fc01f 100644 --- a/apps/local/src/index.ts +++ b/apps/local/src/index.ts @@ -163,6 +163,7 @@ function toSiteDetails( site: SiteListItem ) { enableXdebug: site.enableXdebug, enableDebugLog: site.enableDebugLog, enableDebugDisplay: site.enableDebugDisplay, + operation: site.operation, siteIcon: null, }; } diff --git a/apps/studio/src/hooks/use-site-details.tsx b/apps/studio/src/hooks/use-site-details.tsx index 0addf90a4e..1acd699cd5 100644 --- a/apps/studio/src/hooks/use-site-details.tsx +++ b/apps/studio/src/hooks/use-site-details.tsx @@ -172,6 +172,19 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) { return prevSites; } + // This event carries no verdict on whether the site is running, so it + // must not go through the merge below — that adopts the event's + // `running` wholesale, which would overwrite the real state mid-stop. + if ( eventType === SITE_EVENTS.OPERATIONS_CHANGED ) { + const index = prevSites.findIndex( ( s ) => s.id === siteId ); + if ( index < 0 ) { + return prevSites; + } + const nextSites = [ ...prevSites ]; + nextSites[ index ] = { ...nextSites[ index ], operation: site.operation }; + return nextSites; + } + const siteDetails: SiteDetails = { ...site, running, diff --git a/apps/studio/src/ipc-types.d.ts b/apps/studio/src/ipc-types.d.ts index a3be22ba6b..fe81a8cebc 100644 --- a/apps/studio/src/ipc-types.d.ts +++ b/apps/studio/src/ipc-types.d.ts @@ -8,6 +8,12 @@ interface ShowNotificationOptions extends Electron.NotificationConstructorOption type SiteRuntime = 'playground' | 'native-php'; type SiteFileAccess = 'site-directory' | 'all-files'; +// Inline import type, not a top-level `import`: this file is an ambient +// declaration, and a real import would turn it into a module and take every +// global declaration in here with it. Hand-mirroring the shape instead drifts +// the moment an operation is added. +type SiteOperation = import('@studio/common/lib/site-operation').SiteOperation; + interface StoppedSiteDetails { running: false; @@ -49,6 +55,7 @@ interface StoppedSiteDetails { landingPage?: string; runtime?: SiteRuntime; fileAccess?: SiteFileAccess; + operation?: SiteOperation; } interface StartedSiteDetails extends StoppedSiteDetails { diff --git a/apps/studio/src/modules/cli/lib/cli-events-subscriber.ts b/apps/studio/src/modules/cli/lib/cli-events-subscriber.ts index 2f05967522..9b7a23c9c2 100644 --- a/apps/studio/src/modules/cli/lib/cli-events-subscriber.ts +++ b/apps/studio/src/modules/cli/lib/cli-events-subscriber.ts @@ -73,6 +73,18 @@ const handleSiteEvent = sequential( async ( event: SiteEvent ): Promise< void > return; } + // This event says nothing about whether the site is running, so it must not + // go through the merge below — that one adopts the event's `running` as + // authoritative and fires the start/stop side effects off the transition. + if ( eventType === SITE_EVENTS.OPERATIONS_CHANGED ) { + const server = SiteServer.get( siteId ) ?? SiteServer.getByPath( site.path ); + if ( server ) { + server.details = { ...server.details, operation: site.operation }; + } + void sendIpcEventToRenderer( 'site-event', event ); + return; + } + if ( eventType === SITE_EVENTS.CREATED ) { const existingServer = SiteServer.get( siteId ) ?? SiteServer.getByPath( site.path ); if ( ! existingServer ) { diff --git a/apps/ui/src/components/site-dropdown/index.tsx b/apps/ui/src/components/site-dropdown/index.tsx index 96a5795446..236bc0e19a 100644 --- a/apps/ui/src/components/site-dropdown/index.tsx +++ b/apps/ui/src/components/site-dropdown/index.tsx @@ -9,7 +9,7 @@ import { SyncDialog } from '@/components/selective-sync/sync-dialog'; import '@/components/selective-sync/selective-sync.css'; import { useConnector } from '@/data/core'; import { useConnectedWpcomSites } from '@/data/queries/use-connected-wpcom-sites'; -import { useIsSiteStarting, useIsSiteStopping } from '@/data/queries/use-sites'; +import { useIsSiteStarting, useIsSiteStopping, useSiteOperation } from '@/data/queries/use-sites'; import { useSnapshots } from '@/data/queries/use-snapshots'; import { usePullSiteFromLive, usePushSiteToLive } from '@/data/queries/use-sync-site'; import { useSiteSyncActivity } from '@/data/sync-activity'; @@ -65,7 +65,8 @@ export function SiteDropdown( { // dot — everything else about status lives inside MainView. const isStarting = useIsSiteStarting( site.id ); const isStopping = useIsSiteStopping( site.id ); - const { status, statusLabel } = deriveSiteStatus( site, isStarting, isStopping ); + const operation = useSiteOperation( site ); + const { status, statusLabel } = deriveSiteStatus( site, isStarting, isStopping, operation ); // Only needed here so the disconnect dialog can reference the current live // site. MainView fetches the same data independently for its action row. diff --git a/apps/ui/src/components/site-dropdown/main-view.test.tsx b/apps/ui/src/components/site-dropdown/main-view.test.tsx index bd83f6fc03..e634b9f2b9 100644 --- a/apps/ui/src/components/site-dropdown/main-view.test.tsx +++ b/apps/ui/src/components/site-dropdown/main-view.test.tsx @@ -56,8 +56,10 @@ vi.mock( '@/data/queries/use-preview-site', () => ( { } ) ); vi.mock( '@/data/queries/use-sites', () => ( { + useIsSiteBusy: () => transitions.starting || transitions.stopping, useIsSiteStarting: () => transitions.starting, useIsSiteStopping: () => transitions.stopping, + useSiteOperation: () => null, useStartSite: () => ( { mutate: startSiteMutate } ), useStopSite: () => ( { mutate: stopSiteMutate } ), } ) ); diff --git a/apps/ui/src/components/site-dropdown/main-view.tsx b/apps/ui/src/components/site-dropdown/main-view.tsx index 4df3e56491..425a1ca2b9 100644 --- a/apps/ui/src/components/site-dropdown/main-view.tsx +++ b/apps/ui/src/components/site-dropdown/main-view.tsx @@ -1,4 +1,6 @@ import { TRACKS_EVENTS } from '@studio/common/lib/record-tracks-event'; +import { type SiteOperationKind } from '@studio/common/lib/site-operation'; +import { getSiteOperationLabel } from '@studio/common/lib/site-operation-labels'; import { isSnapshotExpired } from '@studio/common/lib/snapshots'; import { useIsMutating } from '@tanstack/react-query'; import { __, sprintf } from '@wordpress/i18n'; @@ -14,8 +16,10 @@ import { useLogin } from '@/data/queries/use-auth-user'; import { useConnectedWpcomSites } from '@/data/queries/use-connected-wpcom-sites'; import { usePublishPreviewSite } from '@/data/queries/use-preview-site'; import { + useIsSiteBusy, useIsSiteStarting, useIsSiteStopping, + useSiteOperation, useStartSite, useStopSite, } from '@/data/queries/use-sites'; @@ -30,6 +34,7 @@ import { PopoverRow } from './popover-row'; import { getSyncActivityLabel } from './trigger-secondary'; import { deriveSiteStatus, + getSiteStatusName, ensureProtocol, getSnapshotHostname, pickLatestSnapshot, @@ -131,14 +136,19 @@ export function MainView( { const isStarting = useIsSiteStarting( site.id ); const isStopping = useIsSiteStopping( site.id ); - const isLocalTransitioning = isStarting || isStopping; + const isOperationInProgress = useIsSiteBusy( site ); + const operation = useSiteOperation( site ); const { push: isPushPending, pull: isPullPending } = useIsSiteSyncing( site.id ); const isPreviewPending = publishPreviewSite.isPending; // Preview / push / pull all mutate the same local site; running them // concurrently would wedge the site runtime. const isSyncing = isPreviewPending || isPushPending || isPullPending; + // …and none of them can run while the CLI holds the site either. Gate the + // controls on both, so an operation the agent took disables them visibly rather + // than leaving buttons that swallow the click. + const isSiteBusy = isSyncing || isOperationInProgress; - const { localSublabel } = deriveSiteStatus( site, isStarting, isStopping ); + const { localSublabel } = deriveSiteStatus( site, isStarting, isStopping, operation ); const localSiteUrl = getSiteUrl( site ); const canOpenLocalSite = site.running && ! isStopping; @@ -150,6 +160,14 @@ export function MainView( { if ( isPending ) { return pending; } + if ( operation ) { + return sprintf( + /* translators: 1: a sync action, e.g. "Pull from live". 2: an operation in progress, e.g. "Saving settings". */ + __( '%1$s (%2$s)' ), + idle, + getSiteOperationLabel( operation ) + ); + } if ( isSyncing ) { // translators: %s: a sync action, e.g. "Pull from live". return sprintf( __( '%s (sync in progress)' ), idle ); @@ -186,22 +204,22 @@ export function MainView( { }; const handleStartLocalClick = () => { - if ( isLocalTransitioning || isSyncing || site.running ) return; + if ( isOperationInProgress || isSyncing || site.running ) return; startSite.mutate( site.id ); }; const handleStopLocalClick = () => { - if ( isLocalTransitioning || isSyncing || ! site.running ) return; + if ( isOperationInProgress || isSyncing || ! site.running ) return; stopSite.mutate( site.id ); }; const handlePullClick = () => { - if ( ! liveSite || isSyncing ) return; + if ( ! liveSite || isSyncing || isOperationInProgress ) return; onPullClick(); }; const handlePushClick = () => { - if ( ! liveSite || isSyncing ) return; + if ( ! liveSite || isSyncing || isOperationInProgress ) return; onPushClick(); }; @@ -283,6 +301,7 @@ export function MainView( { running={ site.running } starting={ isStarting } stopping={ isStopping } + operation={ operation } disabled={ isSyncing } onStart={ handleStartLocalClick } onStop={ handleStopLocalClick } @@ -327,7 +346,7 @@ export function MainView( { className={ styles.rowActionButton } loading={ isPreviewPending } loadingAnnouncement={ __( 'Updating preview' ) } - disabled={ isSyncing || ! agenticEnabled } + disabled={ isSiteBusy || ! agenticEnabled } focusableWhenDisabled onClick={ handlePreviewClick } /> @@ -343,7 +362,7 @@ export function MainView( { tone="neutral" loading={ isPreviewPending } loadingAnnouncement={ __( 'Creating preview' ) } - disabled={ isSyncing || ! agenticEnabled } + disabled={ isSiteBusy || ! agenticEnabled } onClick={ handlePreviewClick } /> ) } @@ -371,7 +390,7 @@ export function MainView( { className={ styles.rowActionButton } loading={ isPullPending } loadingAnnouncement={ __( 'Pulling from live' ) } - disabled={ isSyncing || ! agenticEnabled } + disabled={ isSiteBusy || ! agenticEnabled } focusableWhenDisabled onClick={ handlePullClick } /> @@ -388,21 +407,21 @@ export function MainView( { className={ styles.rowActionButton } loading={ isPushPending } loadingAnnouncement={ __( 'Pushing to live' ) } - disabled={ isSyncing || ! agenticEnabled } + disabled={ isSiteBusy || ! agenticEnabled } focusableWhenDisabled onClick={ handlePushClick } /> { __( 'Disconnect' ) } @@ -421,7 +440,7 @@ export function MainView( { tone="brand" loading={ ! agenticEnabled && login.isPending } loadingAnnouncement={ __( 'Opening login page' ) } - disabled={ isSyncing || isOffline } + disabled={ isSiteBusy || isOffline } onClick={ agenticEnabled ? onSetupClick : () => login.mutate() } /> ) } @@ -472,28 +491,23 @@ function SyncActivityDetails( { ); } -function getLocalServerStatusName( { - running, - starting, - stopping, -}: { - running: boolean; - starting: boolean; - stopping: boolean; -} ) { - if ( stopping ) { - return __( 'Stopping' ); - } +// The toggle tracks where the site is heading, not where it is, so an in-flight +// start reads as running before the server is actually up. +function getTargetRunning( running: boolean, starting: boolean, stopping: boolean ): boolean { if ( starting ) { - return __( 'Starting' ); + return true; + } + if ( stopping ) { + return false; } - return running ? __( 'Running' ) : __( 'Stopped' ); + return running; } function LocalServerControl( { running, starting, stopping, + operation, disabled, onStart, onStop, @@ -501,19 +515,22 @@ function LocalServerControl( { running: boolean; starting: boolean; stopping: boolean; + // A CLI operation (an agent settings change, another window's delete). Blocks + // the toggle and names itself in the tooltip, so a dead control explains why. + operation: SiteOperationKind | null; disabled: boolean; onStart: () => void; onStop: () => void; } ) { - const pending = starting || stopping; - const targetRunning = starting ? true : stopping ? false : running; + const pending = starting || stopping || operation !== null; + const targetRunning = getTargetRunning( running, starting, stopping ); // aria-disabled rather than disabled: a natively disabled button suppresses // the pointer events the tooltip listens for, hiding the status exactly // while the site is transitioning. const inert = disabled || pending; const statusLabel = sprintf( __( 'Site status: %s' ), - getLocalServerStatusName( { running, starting, stopping } ) + getSiteStatusName( { running, starting, stopping, operation } ) ); const actionLabel = running ? __( 'Stop site' ) : __( 'Start site' ); diff --git a/apps/ui/src/components/site-dropdown/utils.test.ts b/apps/ui/src/components/site-dropdown/utils.test.ts new file mode 100644 index 0000000000..ca71c84296 --- /dev/null +++ b/apps/ui/src/components/site-dropdown/utils.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from 'vitest'; +import { deriveSiteStatus, getSiteStatusName } from './utils'; +import type { SiteDetails } from '@/data/core'; + +function createSite( overrides: Partial< SiteDetails > = {} ): SiteDetails { + return { + id: 'site-1', + name: 'Site', + path: '/sites/site', + port: 8881, + running: false, + phpVersion: '8.2', + ...overrides, + } as SiteDetails; +} + +describe( 'deriveSiteStatus', () => { + it( 'reports a stopped site', () => { + const { status, statusLabel } = deriveSiteStatus( createSite(), false, false, null ); + + expect( status ).toBe( 'stopped' ); + expect( statusLabel ).toBe( 'Site is stopped' ); + } ); + + it( 'reports this window’s own start', () => { + const { status, localSublabel } = deriveSiteStatus( createSite(), true, false, null ); + + expect( status ).toBe( 'transitioning' ); + expect( localSublabel ).toBe( 'Starting…' ); + } ); + + // An operation covers work this window didn't start — an agent settings + // change, another Studio window. Without it a running site reads as plain + // "running" while every control beside it is disabled. + it( 'names an operation over the running state', () => { + const { status, statusLabel, localSublabel } = deriveSiteStatus( + createSite( { running: true } ), + false, + false, + 'settings' + ); + + expect( status ).toBe( 'transitioning' ); + expect( statusLabel ).toBe( 'Saving settings' ); + expect( localSublabel ).toBe( 'Saving settings…' ); + } ); + + it( 'names a duplicate, which has no CLI operation behind it', () => { + const { localSublabel } = deriveSiteStatus( createSite(), false, false, 'duplicate' ); + + expect( localSublabel ).toBe( 'Duplicating…' ); + } ); +} ); + +// Shared by the site dropdown's toggle and the sidebar's status button, so the +// two can't drift on how a busy site is described. +describe( 'getSiteStatusName', () => { + const base = { running: false, starting: false, stopping: false, operation: null }; + + it( 'reports the plain running state', () => { + expect( getSiteStatusName( { ...base, running: true } ) ).toBe( 'Running' ); + expect( getSiteStatusName( base ) ).toBe( 'Stopped' ); + } ); + + it( 'reports this window’s own transition', () => { + expect( getSiteStatusName( { ...base, starting: true } ) ).toBe( 'Starting' ); + expect( getSiteStatusName( { ...base, stopping: true } ) ).toBe( 'Stopping' ); + } ); + + it( 'names an operation over everything else', () => { + expect( getSiteStatusName( { ...base, running: true, operation: 'settings' } ) ).toBe( + 'Saving settings' + ); + } ); +} ); diff --git a/apps/ui/src/components/site-dropdown/utils.ts b/apps/ui/src/components/site-dropdown/utils.ts index 26d8f5e710..ad0bc046d9 100644 --- a/apps/ui/src/components/site-dropdown/utils.ts +++ b/apps/ui/src/components/site-dropdown/utils.ts @@ -1,4 +1,6 @@ -import { __ } from '@wordpress/i18n'; +import { type SiteOperationKind } from '@studio/common/lib/site-operation'; +import { getSiteOperationLabel } from '@studio/common/lib/site-operation-labels'; +import { __, sprintf } from '@wordpress/i18n'; import { getSiteDisplayUrl } from '@/lib/get-site-url'; import type { SiteStatus } from './dropdown-trigger'; import type { SiteDetails, Snapshot, SyncSite } from '@/data/core'; @@ -46,32 +48,100 @@ export function getSnapshotHostname( snapshot: Snapshot ): string { return stripProtocol( snapshot.url ); } +// Short status name for a site's toggle/tooltip: "Running", "Stopping", +// "Saving settings". Shared with the sidebar so the two can't word it +// differently. +// +// `starting`/`stopping` and `operation` overlap but neither covers the other: +// the first two are this window's in-flight mutations, which land the moment +// the user clicks, while `operation` is what the CLI recorded — a round-trip +// later, but the only one that sees work the agent or another window started. +export function getSiteStatusName( { + running, + starting, + stopping, + operation, +}: { + running: boolean; + starting: boolean; + stopping: boolean; + operation: SiteOperationKind | null; +} ): string { + if ( operation ) { + return getSiteOperationLabel( operation ); + } + if ( stopping ) { + return __( 'Stopping' ); + } + if ( starting ) { + return __( 'Starting' ); + } + return running ? __( 'Running' ) : __( 'Stopped' ); +} + +function getStatus( + site: SiteDetails, + isStarting: boolean, + isStopping: boolean, + operation: SiteOperationKind | null +): SiteStatus { + if ( operation || isStarting || isStopping ) { + return 'transitioning'; + } + return site.running ? 'running' : 'stopped'; +} + +// Sentence form, read out by the status dot's aria-label. +function getStatusLabel( + status: SiteStatus, + isStopping: boolean, + operation: SiteOperationKind | null +): string { + if ( operation ) { + return getSiteOperationLabel( operation ); + } + if ( status === 'running' ) { + return __( 'Site is running' ); + } + if ( status === 'stopped' ) { + return __( 'Site is stopped' ); + } + return isStopping ? __( 'Site is stopping' ) : __( 'Site is starting' ); +} + +// The local-site row's second line: what's happening, or where the site lives. +function getLocalSublabel( + site: SiteDetails, + status: SiteStatus, + isStopping: boolean, + operation: SiteOperationKind | null +): string { + if ( operation ) { + // translators: %s: an operation in progress, e.g. "Saving settings". + return sprintf( __( '%s…' ), getSiteOperationLabel( operation ) ); + } + if ( status !== 'transitioning' ) { + return getSiteDisplayUrl( site ); + } + return isStopping ? __( 'Stopping…' ) : __( 'Starting…' ); +} + // Derives the running/transitioning/stopped status plus the user-visible -// labels for the local-site row. Collapses three related but noisy branches -// into a single helper the dropdown can consume in one line. +// labels for the local-site row, so the dropdown consumes it in one line. export function deriveSiteStatus( site: SiteDetails, isStarting: boolean, - isStopping: boolean + isStopping: boolean, + // From `useSiteOperation`; see `getSiteStatusName` for why this doesn't + // replace the two flags above. Passed in rather than derived here because + // it's react-query state and this stays a pure function. + operation: SiteOperationKind | null ): { status: SiteStatus; statusLabel: string; localSublabel: string } { - const status: SiteStatus = - isStarting || isStopping ? 'transitioning' : site.running ? 'running' : 'stopped'; - - const statusLabel = - status === 'running' - ? __( 'Site is running' ) - : status === 'transitioning' - ? isStopping - ? __( 'Site is stopping' ) - : __( 'Site is starting' ) - : __( 'Site is stopped' ); - - const localSublabel = - status === 'transitioning' - ? isStopping - ? __( 'Stopping…' ) - : __( 'Starting…' ) - : getSiteDisplayUrl( site ); + const status = getStatus( site, isStarting, isStopping, operation ); - return { status, statusLabel, localSublabel }; + return { + status, + statusLabel: getStatusLabel( status, isStopping, operation ), + localSublabel: getLocalSublabel( site, status, isStopping, operation ), + }; } diff --git a/apps/ui/src/components/site-list/index.test.tsx b/apps/ui/src/components/site-list/index.test.tsx index 6a8c57bc57..6017249cae 100644 --- a/apps/ui/src/components/site-list/index.test.tsx +++ b/apps/ui/src/components/site-list/index.test.tsx @@ -9,6 +9,7 @@ import { useDeleteSite, useExportDatabase, useExportFullSite, + useIsSiteBusy, useIsSiteStarting, useIsSiteStopping, useSites, @@ -49,10 +50,16 @@ vi.mock( '@/data/queries/use-agent-run', () => ( { } ) ); vi.mock( '@/data/queries/use-sites', () => ( { + COPY_SITE_MUTATION_KEY: [ 'copySite' ], + EXPORT_DATABASE_MUTATION_KEY: [ 'exportDatabase' ], + EXPORT_FULL_SITE_MUTATION_KEY: [ 'exportFullSite' ], useCopySite: vi.fn(), useDeleteSite: vi.fn(), useExportDatabase: vi.fn(), useExportFullSite: vi.fn(), + useIsSiteBusy: vi.fn(), + useIsSiteMutating: vi.fn(), + useSiteOperation: vi.fn(), useIsSiteStarting: vi.fn(), useIsSiteStopping: vi.fn(), useSites: vi.fn(), @@ -83,6 +90,7 @@ const useCopySiteMock = vi.mocked( useCopySite, { partial: true } ); const useDeleteSiteMock = vi.mocked( useDeleteSite, { partial: true } ); const useExportDatabaseMock = vi.mocked( useExportDatabase, { partial: true } ); const useExportFullSiteMock = vi.mocked( useExportFullSite, { partial: true } ); +const useIsSiteBusyMock = vi.mocked( useIsSiteBusy ); const useIsSiteStartingMock = vi.mocked( useIsSiteStarting ); const useIsSiteStoppingMock = vi.mocked( useIsSiteStopping ); const useSiteAgentActivityMock = vi.mocked( useSiteAgentActivity ); @@ -109,6 +117,7 @@ describe( 'SiteList', () => { reason: null, isReady: true, } ); + useIsSiteBusyMock.mockReturnValue( false ); useIsSiteStartingMock.mockReturnValue( false ); useIsSiteStoppingMock.mockReturnValue( false ); useSiteAgentActivityMock.mockReturnValue( 'idle' ); diff --git a/apps/ui/src/components/site-list/index.tsx b/apps/ui/src/components/site-list/index.tsx index 85d53b68b0..ac19f7762c 100644 --- a/apps/ui/src/components/site-list/index.tsx +++ b/apps/ui/src/components/site-list/index.tsx @@ -23,18 +23,17 @@ import { DeleteSiteDialog } from '@/components/delete-site-dialog'; import * as Menu from '@/components/menu'; import { ReorderableList } from '@/components/reorderable-list'; import { SidebarButton } from '@/components/sidebar-button'; -import { deriveSiteStatus } from '@/components/site-dropdown/utils'; +import { deriveSiteStatus, getSiteStatusName } from '@/components/site-dropdown/utils'; import { XdebugIcon } from '@/components/xdebug-icon'; import { useConnector } from '@/data/core'; import { useSiteAgentActivity, type SiteAgentActivity } from '@/data/queries/use-agent-run'; import { useAgenticFeatures } from '@/data/queries/use-agentic-features'; import { useSessions } from '@/data/queries/use-sessions'; import { - useCopySite, - useExportDatabase, - useExportFullSite, + useIsSiteBusy, useIsSiteStarting, useIsSiteStopping, + useSiteOperation, useSites, useStartSite, useStopSite, @@ -42,6 +41,11 @@ import { } from '@/data/queries/use-sites'; import { useUserPreferences } from '@/data/queries/use-user-preferences'; import { useSiteSyncActivity } from '@/data/sync-activity'; +import { + useSiteManagementActions, + type SiteManagementAction, + type SiteManagementActionId, +} from '@/hooks/use-site-management-actions'; import { getSiteUrl } from '@/lib/get-site-url'; import styles from './style.module.css'; import type { AiSessionSummary, SiteDetails } from '@/data/core'; @@ -239,16 +243,17 @@ function SiteStatusButton( { } ) { const startSite = useStartSite(); const stopSite = useStopSite(); - const { status } = deriveSiteStatus( site, isStarting, isStopping ); - const busy = isStarting || isStopping; - const statusName = - status === 'running' - ? __( 'Running' ) - : status === 'transitioning' - ? isStopping - ? __( 'Stopping' ) - : __( 'Starting' ) - : __( 'Stopped' ); + const busy = useIsSiteBusy( site ); + const operation = useSiteOperation( site ); + const { status } = deriveSiteStatus( site, isStarting, isStopping, operation ); + // The recorded operation wins: it names work this window didn't start (an + // agent restart, another Studio window) that local start/stop state can't see. + const statusName = getSiteStatusName( { + running: site.running, + starting: isStarting, + stopping: isStopping, + operation, + } ); const xdebug = Boolean( site.enableXdebug ); const tooltipLabel = xdebug ? sprintf( __( 'Site status: %s. Xdebug enabled' ), statusName ) @@ -333,13 +338,11 @@ function SiteActionsMenu( { site, sessionIds, isStarting, - isStopping, trigger, }: { site: SiteDetails; sessionIds: string[]; isStarting: boolean; - isStopping: boolean; trigger: ReactElement; } ) { const navigate = useNavigate(); @@ -348,12 +351,14 @@ function SiteActionsMenu( { const { data: userPreferences } = useUserPreferences(); const startSite = useStartSite(); const stopSite = useStopSite(); - const copySite = useCopySite(); - const exportFullSite = useExportFullSite(); - const exportDatabase = useExportDatabase(); - const busy = isStarting || isStopping; - const isExporting = exportFullSite.isPending || exportDatabase.isPending; + const busy = useIsSiteBusy( site ); const [ deleteOpen, setDeleteOpen ] = useState( false ); + // Same source as the overview screen's Manage section, so the two can't drift + // on what's blocked or what's in flight. Only the labels differ here. + const manage = useSiteManagementActions( site, { onDelete: () => setDeleteOpen( true ) } ); + const manageById = Object.fromEntries( + manage.map( ( action ) => [ action.id, action ] ) + ) as Record< SiteManagementActionId, SiteManagementAction >; const stopMenuEventPropagation = ( event: MouseEvent< HTMLElement > | ReactPointerEvent< HTMLElement > @@ -440,8 +445,11 @@ function SiteActionsMenu( { > { __( 'Site settings' ) } - copySite.mutate( site.id ) }> - { copySite.isPending ? __( 'Duplicating…' ) : __( 'Duplicate site' ) } + + { manageById.duplicate.loading ? __( 'Duplicating…' ) : __( 'Duplicate site' ) } { __( 'Open folder' ) } @@ -470,17 +478,20 @@ function SiteActionsMenu( { { __( 'Open WP admin' ) } - exportFullSite.mutate( site.id ) }> - { exportFullSite.isPending ? __( 'Exporting…' ) : __( 'Export entire site' ) } + + { manageById.export.loading ? __( 'Exporting…' ) : __( 'Export entire site' ) } - exportDatabase.mutate( site.id ) }> - { exportDatabase.isPending ? __( 'Exporting…' ) : __( 'Export database' ) } + + { manageById[ 'export-db' ].loading ? __( 'Exporting…' ) : __( 'Export database' ) } setDeleteOpen( true ) } - disabled={ busy || copySite.isPending || isExporting } + destructive={ manageById.delete.destructive } + onClick={ manageById.delete.run } + disabled={ manageById.delete.disabled } > { __( 'Delete site' ) } @@ -528,7 +539,7 @@ function SiteSection( { }, [ isActive ] ); const isStarting = useIsSiteStarting( site.id ); const isStopping = useIsSiteStopping( site.id ); - const { status } = deriveSiteStatus( site, isStarting, isStopping ); + const { status } = deriveSiteStatus( site, isStarting, isStopping, useSiteOperation( site ) ); const agentActivity = useSiteAgentActivity( row.sessionIds ); const syncActivity = useSiteSyncActivity( site.id ); const isLiveSyncPending = @@ -579,7 +590,6 @@ function SiteSection( { site={ site } sessionIds={ row.sessionIds } isStarting={ isStarting } - isStopping={ isStopping } trigger={
diff --git a/apps/ui/src/components/site-overview-view/index.test.tsx b/apps/ui/src/components/site-overview-view/index.test.tsx index a960dfd0a5..4b2c011477 100644 --- a/apps/ui/src/components/site-overview-view/index.test.tsx +++ b/apps/ui/src/components/site-overview-view/index.test.tsx @@ -11,6 +11,7 @@ import { useCopySite, useExportDatabase, useExportFullSite, + useIsSiteBusy, useIsSiteStarting, useIsSiteStopping, useSites, @@ -85,9 +86,14 @@ vi.mock( '@/data/queries/use-create-site-helpers', () => ( { } ) ); vi.mock( '@/data/queries/use-sites', () => ( { + COPY_SITE_MUTATION_KEY: [ 'copySite' ], + EXPORT_DATABASE_MUTATION_KEY: [ 'exportDatabase' ], + EXPORT_FULL_SITE_MUTATION_KEY: [ 'exportFullSite' ], useCopySite: vi.fn(), useExportDatabase: vi.fn(), useExportFullSite: vi.fn(), + useIsSiteBusy: vi.fn(), + useIsSiteMutating: vi.fn(), useIsSiteStarting: vi.fn(), useIsSiteStopping: vi.fn(), useSites: vi.fn(), @@ -132,6 +138,7 @@ const useExistingCustomDomainsMock = vi.mocked( useExistingCustomDomains, { part const useCopySiteMock = vi.mocked( useCopySite, { partial: true } ); const useExportDatabaseMock = vi.mocked( useExportDatabase, { partial: true } ); const useExportFullSiteMock = vi.mocked( useExportFullSite, { partial: true } ); +const useIsSiteBusyMock = vi.mocked( useIsSiteBusy ); const useIsSiteStartingMock = vi.mocked( useIsSiteStarting ); const useIsSiteStoppingMock = vi.mocked( useIsSiteStopping ); const useSiteThumbnailMock = vi.mocked( useSiteThumbnail, { partial: true } ); @@ -202,6 +209,7 @@ describe( 'SiteOverviewView', () => { data: [ createSite( { running: true } ) ], isLoading: false, } ); + useIsSiteBusyMock.mockReturnValue( false ); useSiteThumbnailMock.mockReturnValue( { data: 'data:image/png;base64,site-thumbnail', } ); diff --git a/apps/ui/src/components/site-overview-view/index.tsx b/apps/ui/src/components/site-overview-view/index.tsx index f20200d1f9..cc15b51280 100644 --- a/apps/ui/src/components/site-overview-view/index.tsx +++ b/apps/ui/src/components/site-overview-view/index.tsx @@ -29,7 +29,7 @@ import { DATABASE_HOME_PATH } from '@/components/site-preview/address-bar'; import { isSiteSettingsTab, SiteSettingsForm } from '@/components/site-settings-view'; import * as Tabs from '@/components/tabs'; import { useConnector } from '@/data/core'; -import { useIsSiteStarting, useIsSiteStopping, useSites } from '@/data/queries/use-sites'; +import { useIsSiteBusy, useSites } from '@/data/queries/use-sites'; import { useUserPreferences } from '@/data/queries/use-user-preferences'; import { useWpVersion } from '@/data/queries/use-wordpress-versions'; import { useOpenSiteUrl } from '@/hooks/use-open-site-url'; @@ -230,14 +230,12 @@ function SiteOverviewBody( { } ) { const navigate = useNavigate(); const connector = useConnector(); - const isStarting = useIsSiteStarting( site.id ); - const isStopping = useIsSiteStopping( site.id ); const [ deleteOpen, setDeleteOpen ] = useState( false ); const managementActions = useSiteManagementActions( site, { onDelete: () => setDeleteOpen( true ), } ); - const busy = isStarting || isStopping; + const busy = useIsSiteBusy( site ); const themeDetails = site.themeDetails; const isBlockTheme = themeDetails?.isBlockTheme === true; const { data: wpVersion } = useWpVersion( site.id ); diff --git a/apps/ui/src/components/site-overview-view/style.module.css b/apps/ui/src/components/site-overview-view/style.module.css index 1eafd5f4cf..5c3f11af30 100644 --- a/apps/ui/src/components/site-overview-view/style.module.css +++ b/apps/ui/src/components/site-overview-view/style.module.css @@ -262,6 +262,15 @@ background: var(--wpds-color-bg-surface-error-weak); } +/* The rules above pin an explicit colour on the icon and label, which would + otherwise keep them at full strength while the button is disabled — the + control looks live and swallows the click. Higher specificity than the + destructive rule, so Delete mutes too. */ +.overviewButton:is([data-disabled], :disabled) .overviewButtonIcon, +.overviewButton:is([data-disabled], :disabled) .overviewButtonLabel { + color: var(--wpds-color-fg-interactive-neutral-disabled); +} + @media (max-width: 760px) { .content { padding: var(--wpds-dimension-padding-xl) var(--wpds-dimension-padding-md) diff --git a/apps/ui/src/components/site-preview/index.tsx b/apps/ui/src/components/site-preview/index.tsx index 919b64ba87..0dc2986072 100644 --- a/apps/ui/src/components/site-preview/index.tsx +++ b/apps/ui/src/components/site-preview/index.tsx @@ -1,3 +1,4 @@ +import { getSiteOperationLabel } from '@studio/common/lib/site-operation-labels'; import { useQuery } from '@tanstack/react-query'; import { __, sprintf } from '@wordpress/i18n'; import { chevronLeft, chevronRight, moreVertical, pencil } from '@wordpress/icons'; @@ -10,7 +11,12 @@ import * as Menu from '@/components/menu'; import { OpenInMenu } from '@/components/open-in-menu'; import { useConnector } from '@/data/core'; import { useAgenticFeatures } from '@/data/queries/use-agentic-features'; -import { useIsSiteStarting, useStartSite } from '@/data/queries/use-sites'; +import { + useIsSiteBusy, + useIsSiteStarting, + useSiteOperation, + useStartSite, +} from '@/data/queries/use-sites'; import { useTrafficLightSpace } from '@/hooks/use-traffic-light-space'; import { useWindowControlsOverlay } from '@/hooks/use-window-controls-overlay'; import { getSiteUrl } from '@/lib/get-site-url'; @@ -549,6 +555,8 @@ export function SitePreview( { const { chatEnabled } = useAgenticFeatures(); const startSite = useStartSite(); const isStarting = useIsSiteStarting( site.id ); + const isBusy = useIsSiteBusy( site ); + const operation = useSiteOperation( site ); const siteUrl = getSiteUrl( site ); const canPreview = site.running; const canUseWebview = isElectron(); @@ -1109,13 +1117,20 @@ export function SitePreview( {
) : null }

- { __( 'Start the site to see a live preview.' ) } + { operation + ? sprintf( + /* translators: %s: an operation in progress, e.g. "Saving settings". */ + __( '%s… the site can start once this finishes.' ), + getSiteOperationLabel( operation ) + ) + : __( 'Start the site to see a live preview.' ) }