From de0f379ae56774f557356eb4816ba69c667d19b1 Mon Sep 17 00:00:00 2001 From: bcotrim Date: Thu, 23 Jul 2026 16:02:33 +0100 Subject: [PATCH 1/3] Fix changing the WordPress version on a site that has never been started --- apps/cli/commands/config/set.ts | 61 +++++++++++++----- apps/cli/commands/config/tests/set.test.ts | 73 +++++++++++++++++----- 2 files changed, 103 insertions(+), 31 deletions(-) diff --git a/apps/cli/commands/config/set.ts b/apps/cli/commands/config/set.ts index 18a42cd0e3..6948d36501 100644 --- a/apps/cli/commands/config/set.ts +++ b/apps/cli/commands/config/set.ts @@ -1,7 +1,8 @@ +import path from 'node:path'; import { DEFAULT_WORDPRESS_VERSION, MINIMUM_WORDPRESS_VERSION } from '@studio/common/constants'; import { SITE_EVENTS } from '@studio/common/lib/cli-events'; import { getDomainNameValidationError } from '@studio/common/lib/domains'; -import { arePathsEqual } from '@studio/common/lib/fs-utils'; +import { arePathsEqual, pathExists, recursiveCopyDirectory } from '@studio/common/lib/fs-utils'; import { encodePassword, validateAdminEmail, @@ -38,9 +39,12 @@ import { readCliConfig, saveCliConfig, unlockCliConfig, + type SiteData, } from 'cli/lib/cli-config/core'; import { getSiteByFolder } from 'cli/lib/cli-config/sites'; import { connectToDaemon, disconnectFromDaemon, emitCliEvent } from 'cli/lib/daemon-client'; +import { getWordPressVersionPath } from 'cli/lib/dependency-management/paths'; +import { downloadWordPress } from 'cli/lib/dependency-management/wordpress'; import { updateDomainInHosts } from 'cli/lib/hosts-file'; import { validateSupportedPhpVersion } from 'cli/lib/php-versions'; import { runWpCliCommand } from 'cli/lib/run-wp-cli-command'; @@ -72,6 +76,45 @@ export interface SetCommandOptions { debugDisplay?: boolean; } +// A site that has never been started has no wp-config.php and no database, so WP-CLI can't +// boot it ("The site you have requested is not installed") and `core update` always fails. +// Nothing but the core files on disk decides such a site's version, so swap those in the way +// `site create --wp` does; the WordPress installer runs against them on the first start. +async function setWordPressVersion( site: SiteData, wp: string ): Promise< void > { + if ( ! ( await pathExists( path.join( site.path, 'wp-config.php' ) ) ) ) { + await downloadWordPress( wp ); + await recursiveCopyDirectory( getWordPressVersionPath( wp ), site.path ); + return; + } + + await using command = await runWpCliCommand( site, [ + 'core', + 'update', + getWordPressVersionUrl( wp ), + '--force', + '--skip-plugins', + '--skip-themes', + ] ); + + if ( ( await command.response.exitCode ) === 0 ) { + return; + } + + const [ stdout, stderr ] = await Promise.all( [ + command.response.stdoutText, + command.response.stderrText, + ] ); + throw new LoggerError( + [ + sprintf( __( 'Failed to update WordPress version to %s' ), wp ), + stderr.trim(), + stdout.trim(), + ] + .filter( Boolean ) + .join( '\n' ) + ); +} + export async function runCommand( sitePath: string, options: SetCommandOptions ): Promise< void > { const { name, @@ -314,21 +357,7 @@ export async function runCommand( sitePath: string, options: SetCommandOptions ) if ( wpChanged ) { logger.reportStart( LoggerAction.SET_WP_VERSION, __( 'Updating WordPress version…' ) ); - const zipUrl = getWordPressVersionUrl( wp ); - - await using command = await runWpCliCommand( site, [ - 'core', - 'update', - zipUrl, - '--force', - '--skip-plugins', - '--skip-themes', - ] ); - - const exitCode = await command.response.exitCode; - if ( exitCode !== 0 ) { - throw new LoggerError( sprintf( __( 'Failed to update WordPress version to %s' ), wp ) ); - } + await setWordPressVersion( site, wp ); logger.reportSuccess( __( 'WordPress version updated' ) ); try { diff --git a/apps/cli/commands/config/tests/set.test.ts b/apps/cli/commands/config/tests/set.test.ts index 2b90619629..53c24c6ad4 100644 --- a/apps/cli/commands/config/tests/set.test.ts +++ b/apps/cli/commands/config/tests/set.test.ts @@ -1,5 +1,6 @@ +import { Readable } from 'node:stream'; import { getDomainNameValidationError } from '@studio/common/lib/domains'; -import { arePathsEqual } from '@studio/common/lib/fs-utils'; +import { arePathsEqual, pathExists, recursiveCopyDirectory } from '@studio/common/lib/fs-utils'; import { encodePassword } from '@studio/common/lib/passwords'; import { SITE_MODE_NATIVE, @@ -11,6 +12,8 @@ import { vi } from 'vitest'; import { readCliConfig, saveCliConfig, unlockCliConfig, SiteData } from 'cli/lib/cli-config/core'; import { getSiteByFolder } from 'cli/lib/cli-config/sites'; import { connectToDaemon, disconnectFromDaemon } from 'cli/lib/daemon-client'; +import { getWordPressVersionPath } from 'cli/lib/dependency-management/paths'; +import { downloadWordPress } from 'cli/lib/dependency-management/wordpress'; import { updateDomainInHosts } from 'cli/lib/hosts-file'; import { runWpCliCommand, WpCliResponse } from 'cli/lib/run-wp-cli-command'; import { setupCustomDomain } from 'cli/lib/site-utils'; @@ -28,6 +31,8 @@ vi.mock( '@studio/common/lib/fs-utils', async () => { return { ...actual, arePathsEqual: vi.fn(), + pathExists: vi.fn(), + recursiveCopyDirectory: vi.fn(), }; } ); vi.mock( 'cli/lib/cli-config/core', async () => { @@ -49,9 +54,17 @@ vi.mock( 'cli/lib/cli-config/sites', async () => { }; } ); vi.mock( 'cli/lib/certificate-manager' ); +vi.mock( 'cli/lib/dependency-management/paths' ); +vi.mock( 'cli/lib/dependency-management/wordpress' ); vi.mock( 'cli/lib/hosts-file' ); vi.mock( 'cli/lib/daemon-client' ); -vi.mock( 'cli/lib/run-wp-cli-command' ); +vi.mock( 'cli/lib/run-wp-cli-command', async () => { + const actual = await vi.importActual( 'cli/lib/run-wp-cli-command' ); + return { + ...actual, + runWpCliCommand: vi.fn(), + }; +} ); vi.mock( 'cli/lib/site-utils' ); vi.mock( 'cli/lib/wordpress-server-manager' ); @@ -89,6 +102,12 @@ describe( 'CLI: studio config set', () => { const testCliConfig = { version: 1 as const, sites: [ testSite ], snapshots: [] }; vi.mocked( arePathsEqual ).mockReturnValue( true ); + vi.mocked( pathExists ).mockResolvedValue( true ); + vi.mocked( recursiveCopyDirectory ).mockResolvedValue( undefined ); + vi.mocked( downloadWordPress ).mockResolvedValue( undefined ); + vi.mocked( getWordPressVersionPath ).mockImplementation( + ( version ) => `/wp-versions/${ version }` + ); vi.mocked( getSiteByFolder ).mockResolvedValue( getTestSite() ); vi.mocked( readCliConfig ).mockResolvedValue( testCliConfig ); vi.mocked( connectToDaemon ).mockResolvedValue( undefined ); @@ -287,14 +306,19 @@ describe( 'CLI: studio config set', () => { } ); describe( 'WordPress version changes', () => { - beforeEach( () => { - const mockResponse: Partial< WpCliResponse > = { - exitCode: Promise.resolve( 0 ), - }; + const mockWpCliResult = ( exitCode: number, stdout = '', stderr = '' ) => { vi.mocked( runWpCliCommand ).mockResolvedValue( { - response: mockResponse as WpCliResponse, - [ Symbol.dispose ]: vi.fn().mockResolvedValue( undefined ), + response: new WpCliResponse( + Readable.from( [ stdout ] ), + Readable.from( [ stderr ] ), + Promise.resolve( exitCode ) + ), + [ Symbol.dispose ]: vi.fn(), } ); + }; + + beforeEach( () => { + mockWpCliResult( 0 ); } ); it( 'should run WP-CLI to update WordPress version', async () => { @@ -317,19 +341,38 @@ describe( 'CLI: studio config set', () => { } ); it( 'should throw when WP-CLI fails', async () => { - const mockResponse: Partial< WpCliResponse > = { - exitCode: Promise.resolve( 1 ), - }; - vi.mocked( runWpCliCommand ).mockResolvedValue( { - response: mockResponse as WpCliResponse, - [ Symbol.dispose ]: vi.fn().mockResolvedValue( undefined ), - } ); + mockWpCliResult( 1 ); await expect( runCommand( testSitePath, { wp: '6.7' } ) ).rejects.toThrow( 'Failed to update WordPress version to 6.7' ); } ); + it( 'should surface the WP-CLI output when it fails', async () => { + mockWpCliResult( 1, 'some stdout noise', "Error: 'wp-config.php' not found." ); + + await expect( runCommand( testSitePath, { wp: '6.7' } ) ).rejects.toThrow( + /Failed to update WordPress version to 6\.7\nError: 'wp-config\.php' not found\.\nsome stdout noise/ + ); + } ); + + it( 'should copy the WordPress files instead of running WP-CLI on a never-started site', async () => { + vi.mocked( pathExists ).mockResolvedValue( false ); + + await runCommand( testSitePath, { wp: '6.7' } ); + + expect( runWpCliCommand ).not.toHaveBeenCalled(); + expect( downloadWordPress ).toHaveBeenCalledWith( '6.7' ); + expect( recursiveCopyDirectory ).toHaveBeenCalledWith( '/wp-versions/6.7', testSitePath ); + expect( saveCliConfig ).toHaveBeenCalledWith( + expect.objectContaining( { + sites: expect.arrayContaining( [ + expect.objectContaining( { isWpAutoUpdating: false } ), + ] ), + } ) + ); + } ); + it( 'should update isWpAutoUpdating to false when using specific version', async () => { await runCommand( testSitePath, { wp: '6.8' } ); From 5a90d98b055962b0bb39e66944f1afabde215ea8 Mon Sep 17 00:00:00 2001 From: bcotrim Date: Thu, 23 Jul 2026 16:17:00 +0100 Subject: [PATCH 2/3] Include WP-CLI output in errors so failures are not silently swallowed --- apps/cli/commands/config/set.ts | 7 +-- .../import-export/export/export-database.ts | 17 +++++-- .../export/exporters/default-exporter.ts | 8 ++- .../export/tests/export-database.test.ts | 51 +++++++++++++++++++ .../import/importers/importer.ts | 4 +- .../import/importers/wxr-importer.ts | 4 +- apps/cli/lib/run-wp-cli-command.ts | 9 ++++ 7 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 apps/cli/lib/import-export/export/tests/export-database.test.ts diff --git a/apps/cli/commands/config/set.ts b/apps/cli/commands/config/set.ts index 6948d36501..4e97184eb6 100644 --- a/apps/cli/commands/config/set.ts +++ b/apps/cli/commands/config/set.ts @@ -100,15 +100,10 @@ async function setWordPressVersion( site: SiteData, wp: string ): Promise< void return; } - const [ stdout, stderr ] = await Promise.all( [ - command.response.stdoutText, - command.response.stderrText, - ] ); throw new LoggerError( [ sprintf( __( 'Failed to update WordPress version to %s' ), wp ), - stderr.trim(), - stdout.trim(), + await command.response.outputText(), ] .filter( Boolean ) .join( '\n' ) diff --git a/apps/cli/lib/import-export/export/export-database.ts b/apps/cli/lib/import-export/export/export-database.ts index fe1244880b..cb34e4aeba 100644 --- a/apps/cli/lib/import-export/export/export-database.ts +++ b/apps/cli/lib/import-export/export/export-database.ts @@ -26,7 +26,9 @@ export async function exportDatabaseToFile( const exitCode = await command.response.exitCode; if ( exitCode !== 0 ) { - throw new Error( __( 'Database export failed' ) ); + throw new Error( + sprintf( __( 'Database export failed: %s' ), await command.response.outputText() ) + ); } // Move the file to its final destination @@ -57,7 +59,9 @@ export async function exportDatabaseToMultipleFiles( const tablesStdout = await command.response.stdoutText; const exitCode = await command.response.exitCode; if ( exitCode !== 0 ) { - throw new Error( __( 'Database export failed' ) ); + throw new Error( + sprintf( __( 'Database export failed: %s' ), await command.response.outputText() ) + ); } let tables; @@ -101,7 +105,14 @@ export async function exportDatabaseToMultipleFiles( const exitCode = await command.response.exitCode; if ( exitCode !== 0 ) { - throw new Error( sprintf( __( 'Database export failed for table %s' ), table ) ); + throw new Error( + sprintf( + /* translators: 1: database table name, 2: WP-CLI output */ + __( 'Database export failed for table %1$s: %2$s' ), + table, + await command.response.outputText() + ) + ); } // Move the file to its final destination diff --git a/apps/cli/lib/import-export/export/exporters/default-exporter.ts b/apps/cli/lib/import-export/export/exporters/default-exporter.ts index 505af90857..2112f4437c 100644 --- a/apps/cli/lib/import-export/export/exporters/default-exporter.ts +++ b/apps/cli/lib/import-export/export/exporters/default-exporter.ts @@ -374,7 +374,9 @@ export class DefaultExporter extends ImportExportEventEmitter implements Exporte const stdout = await command.response.stdoutText; if ( exitCode !== 0 ) { - throw new Error( sprintf( __( 'Failed to get site plugins: %s' ), stderr ) ); + throw new Error( + sprintf( __( 'Failed to get site plugins: %s' ), await command.response.outputText() ) + ); } try { @@ -413,7 +415,9 @@ export class DefaultExporter extends ImportExportEventEmitter implements Exporte const stdout = await command.response.stdoutText; if ( exitCode !== 0 ) { - throw new Error( sprintf( __( 'Failed to get site themes: %s' ), stderr ) ); + throw new Error( + sprintf( __( 'Failed to get site themes: %s' ), await command.response.outputText() ) + ); } try { diff --git a/apps/cli/lib/import-export/export/tests/export-database.test.ts b/apps/cli/lib/import-export/export/tests/export-database.test.ts new file mode 100644 index 0000000000..4ef90e475d --- /dev/null +++ b/apps/cli/lib/import-export/export/tests/export-database.test.ts @@ -0,0 +1,51 @@ +import { Readable } from 'node:stream'; +import { vi } from 'vitest'; +import { runWpCliCommand, WpCliResponse } from 'cli/lib/run-wp-cli-command'; +import { exportDatabaseToFile, exportDatabaseToMultipleFiles } from '../export-database'; +import type { SiteData } from 'cli/lib/cli-config/core'; + +vi.mock( 'cli/lib/run-wp-cli-command', async () => { + const actual = await vi.importActual( 'cli/lib/run-wp-cli-command' ); + return { + ...actual, + runWpCliCommand: vi.fn(), + }; +} ); + +describe( 'export-database', () => { + const site = { id: 'site-1', name: 'Test Site', path: '/test/site', port: 8080 } as SiteData; + + const mockWpCliFailure = ( stdout: string, stderr: string ) => { + vi.mocked( runWpCliCommand ).mockResolvedValue( { + response: new WpCliResponse( + Readable.from( [ stdout ] ), + Readable.from( [ stderr ] ), + Promise.resolve( 1 ) + ), + [ Symbol.dispose ]: vi.fn(), + } ); + }; + + beforeEach( () => { + vi.clearAllMocks(); + } ); + + // A site that has never been started has no database, so WP-CLI can't boot it. The + // reason has to reach the user instead of a bare "Database export failed". + it( 'surfaces the WP-CLI error when exporting a site that was never started', async () => { + mockWpCliFailure( '', 'Error: The site you have requested is not installed.' ); + + await expect( exportDatabaseToFile( site, '/tmp/out.sql' ) ).rejects.toThrow( + 'Database export failed: Error: The site you have requested is not installed.' + ); + } ); + + // Which stream carries the message depends on the runtime, so both are reported. + it( 'surfaces WP-CLI output sent to stdout', async () => { + mockWpCliFailure( 'Error: could not read the database.', '' ); + + await expect( exportDatabaseToMultipleFiles( site, '/tmp/out' ) ).rejects.toThrow( + 'Database export failed: Error: could not read the database.' + ); + } ); +} ); diff --git a/apps/cli/lib/import-export/import/importers/importer.ts b/apps/cli/lib/import-export/import/importers/importer.ts index 16b8e2d94b..abead839ce 100644 --- a/apps/cli/lib/import-export/import/importers/importer.ts +++ b/apps/cli/lib/import-export/import/importers/importer.ts @@ -116,7 +116,9 @@ abstract class BaseImporter extends ImportExportEventEmitter implements Importer } if ( exitCode !== 0 ) { - throw new Error( sprintf( __( 'Database import failed: %s' ), stderr ) ); + throw new Error( + sprintf( __( 'Database import failed: %s' ), await command.response.outputText() ) + ); } } finally { await this.safelyDeletePath( tmpPath ); diff --git a/apps/cli/lib/import-export/import/importers/wxr-importer.ts b/apps/cli/lib/import-export/import/importers/wxr-importer.ts index dbda6952d5..b3ea0057a1 100644 --- a/apps/cli/lib/import-export/import/importers/wxr-importer.ts +++ b/apps/cli/lib/import-export/import/importers/wxr-importer.ts @@ -82,7 +82,9 @@ export class WxrImporter extends ImportExportEventEmitter implements Importer { console.error( __( 'Error during WordPress export import:' ), stderr ); } if ( exitCode !== 0 ) { - throw new Error( sprintf( __( 'WordPress export import failed: %s' ), stderr ) ); + throw new Error( + sprintf( __( 'WordPress export import failed: %s' ), await command.response.outputText() ) + ); } this.emit( ImportEvents.IMPORT_DATABASE_COMPLETE ); diff --git a/apps/cli/lib/run-wp-cli-command.ts b/apps/cli/lib/run-wp-cli-command.ts index 92f15a1cfa..23a93e493b 100644 --- a/apps/cli/lib/run-wp-cli-command.ts +++ b/apps/cli/lib/run-wp-cli-command.ts @@ -85,6 +85,15 @@ export class WpCliResponse { this.#stderrText ??= text( this.stderr ); return this.#stderrText; } + + // What WP-CLI actually said, for error messages. Both streams, because which one carries + // a given message depends on the runtime — WP-CLI writes errors to stderr natively, but + // under Playground they can land on stdout, and a message on the wrong stream is a message + // the user never sees. + async outputText(): Promise< string > { + const [ stdout, stderr ] = await Promise.all( [ this.stdoutText, this.stderrText ] ); + return [ stderr.trim(), stdout.trim() ].filter( Boolean ).join( '\n' ); + } } /** From 52bbb9c96507f5e9b7d16b2437b6de23eb77604e Mon Sep 17 00:00:00 2001 From: bcotrim Date: Thu, 23 Jul 2026 17:09:17 +0100 Subject: [PATCH 3/3] Require an absent database too before treating a site as unprovisioned --- apps/cli/commands/config/set.ts | 29 ++++++++++++--- apps/cli/commands/config/tests/set.test.ts | 43 +++++++++++++++++++++- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/apps/cli/commands/config/set.ts b/apps/cli/commands/config/set.ts index 4e97184eb6..8ed145dd6a 100644 --- a/apps/cli/commands/config/set.ts +++ b/apps/cli/commands/config/set.ts @@ -76,12 +76,31 @@ export interface SetCommandOptions { debugDisplay?: boolean; } -// A site that has never been started has no wp-config.php and no database, so WP-CLI can't -// boot it ("The site you have requested is not installed") and `core update` always fails. -// Nothing but the core files on disk decides such a site's version, so swap those in the way -// `site create --wp` does; the WordPress installer runs against them on the first start. +// `site create` puts the WordPress files in place but doesn't install WordPress — that happens +// on the first start, which is what writes wp-config.php and creates the database. Both being +// absent is what makes WP-CLI unable to boot the site at all. +// +// A missing wp-config.php alone isn't enough to conclude that: an imported site's config can +// live outside the site directory (its constants come from the runtime prepend instead, which +// is why the server start skips wp-config handling for it), and a site can simply have lost +// its config. Those have real content, so they must keep taking the WP-CLI path rather than +// having core files copied over them. +async function isUnprovisionedSite( site: SiteData ): Promise< boolean > { + if ( site.runtimeBlueprintPath ) { + return false; + } + + const [ hasConfig, hasDatabase ] = await Promise.all( [ + pathExists( path.join( site.path, 'wp-config.php' ) ), + pathExists( path.join( site.path, 'wp-content', 'database', '.ht.sqlite' ) ), + ] ); + return ! hasConfig && ! hasDatabase; +} + +// Nothing but the core files on disk decides an unprovisioned site's version, so swap those in +// the way `site create --wp` does; the WordPress installer runs against them on the first start. async function setWordPressVersion( site: SiteData, wp: string ): Promise< void > { - if ( ! ( await pathExists( path.join( site.path, 'wp-config.php' ) ) ) ) { + if ( await isUnprovisionedSite( site ) ) { await downloadWordPress( wp ); await recursiveCopyDirectory( getWordPressVersionPath( wp ), site.path ); return; diff --git a/apps/cli/commands/config/tests/set.test.ts b/apps/cli/commands/config/tests/set.test.ts index 53c24c6ad4..7ee09cad0c 100644 --- a/apps/cli/commands/config/tests/set.test.ts +++ b/apps/cli/commands/config/tests/set.test.ts @@ -87,6 +87,13 @@ describe( 'CLI: studio config set', () => { enableHttps: false, } ); + // Defaults to a provisioned site: WordPress has been installed, so both exist. + const mockSiteFiles = ( { wpConfig = true, database = true } = {} ) => { + vi.mocked( pathExists ).mockImplementation( async ( filePath: string ) => + filePath.endsWith( 'wp-config.php' ) ? wpConfig : database + ); + }; + const testProcessDescription: ProcessDescription = { name: 'test-site', pmId: 0, @@ -102,7 +109,7 @@ describe( 'CLI: studio config set', () => { const testCliConfig = { version: 1 as const, sites: [ testSite ], snapshots: [] }; vi.mocked( arePathsEqual ).mockReturnValue( true ); - vi.mocked( pathExists ).mockResolvedValue( true ); + mockSiteFiles(); vi.mocked( recursiveCopyDirectory ).mockResolvedValue( undefined ); vi.mocked( downloadWordPress ).mockResolvedValue( undefined ); vi.mocked( getWordPressVersionPath ).mockImplementation( @@ -357,7 +364,7 @@ describe( 'CLI: studio config set', () => { } ); it( 'should copy the WordPress files instead of running WP-CLI on a never-started site', async () => { - vi.mocked( pathExists ).mockResolvedValue( false ); + mockSiteFiles( { wpConfig: false, database: false } ); await runCommand( testSitePath, { wp: '6.7' } ); @@ -373,6 +380,38 @@ describe( 'CLI: studio config set', () => { ); } ); + // A site can lose its wp-config.php and still have all of its content, so the database + // is what decides whether copying core files over it would be destructive. + it( 'should still run WP-CLI when wp-config.php is missing but a database exists', async () => { + mockSiteFiles( { wpConfig: false, database: true } ); + + await runCommand( testSitePath, { wp: '6.7' } ); + + expect( runWpCliCommand ).toHaveBeenCalled(); + expect( recursiveCopyDirectory ).not.toHaveBeenCalled(); + } ); + + // An imported site keeps its config outside the site directory, so a missing + // wp-config.php there says nothing about whether WordPress is installed. + it( 'should still run WP-CLI for an imported site with no wp-config.php', async () => { + const importedSite = { + ...getTestSite(), + runtimeBlueprintPath: '/pulls/site-1/blueprint.json', + }; + vi.mocked( getSiteByFolder ).mockResolvedValue( importedSite ); + vi.mocked( readCliConfig ).mockResolvedValue( { + sites: [ importedSite ], + version: 1, + snapshots: [], + } ); + mockSiteFiles( { wpConfig: false, database: false } ); + + await runCommand( testSitePath, { wp: '6.7' } ); + + expect( runWpCliCommand ).toHaveBeenCalled(); + expect( recursiveCopyDirectory ).not.toHaveBeenCalled(); + } ); + it( 'should update isWpAutoUpdating to false when using specific version', async () => { await runCommand( testSitePath, { wp: '6.8' } );