Consolidate studio site set-* commands into single command by bcotrim · Pull Request #2355 · Automattic/studio

bcotrim · 2026-01-06T17:35:03Z

Related issues

Fixes STU-1079

Proposed Changes

Replace four separate studio site set-* commands (set-domain, set-https, set-php-version, set-wp-version) with a single studio site set command
Support multiple options at once: --name, --domain, --https, --php, --wp
Add new --name option to change site name (no restart needed, metadata only)
Server restarts only once even when multiple options change
Detect "no changes" scenario and show error instead of doing nothing
Pass correct site URL to WP-CLI to avoid corrupting WordPress URL options when using custom domains with HTTPS

Testing Instructions

Create a test site: studio site create --path /tmp/test-site
Start the site: studio site start --path /tmp/test-site

Test single options:

studio site set --name "New Name" --path /tmp/test-site - should update name without restart
studio site set --php 8.2 --path /tmp/test-site - should update PHP and restart

Test multiple options:

studio site set --name "Test" --php 8.3 --path /tmp/test-site - should apply both changes with single restart

Test edge cases:

studio site set --path /tmp/test-site - should show error "At least one option required"
studio site set --name "Test" --path /tmp/test-site (when name is already "Test") - should show "No changes to apply"
studio site set --php invalid --path /tmp/test-site - should show valid PHP version choices

Test HTTPS with custom domain:

studio site set --domain test.local --https true --path /tmp/test-site - should work correctly with SSL

Pre-merge Checklist

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

fredrikekelund · 2026-01-07T09:49:17Z

Reviewing this now

fredrikekelund

LGTM 👍 This turned out great. I left a couple of small comments on the test file. My only real concern is about cli/lib/run-wp-cli-command.ts, where I'm not sure why we need that change. It would be good to straighten that out before landing this. I'm still approving now to unblock you, though.

fredrikekelund · 2026-01-07T13:02:02Z

+		if ( wpChanged ) {
+			logger.reportStart( LoggerAction.SET_WP_VERSION, __( 'Changing WordPress version…' ) );
+			const phpVersion = validatePhpVersion( site.phpVersion );
+			const zipUrl = getWordPressVersionUrl( wp! );


Suggested change

const zipUrl = getWordPressVersionUrl( wp! );

const zipUrl = getWordPressVersionUrl( wp );

Nit, but we don't need to override TS here.

fredrikekelund · 2026-01-07T13:05:39Z

+		it( 'should update site name without restart', async () => {
+			await runCommand( testSitePath, { name: 'New Name' } );
+
+			const savedAppdata = ( saveAppdata as jest.Mock ).mock.calls[ 0 ][ 0 ];
+			expect( savedAppdata.sites[ 0 ].name ).toBe( 'New Name' );
+			expect( stopWordPressServer ).not.toHaveBeenCalled();
+			expect( startWordPressServer ).not.toHaveBeenCalled();
+		} );
+
+		it( 'should not restart running site when only name changes', async () => {
+			( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription );
+
+			await runCommand( testSitePath, { name: 'New Name' } );
+
+			expect( stopWordPressServer ).not.toHaveBeenCalled();
+			expect( startWordPressServer ).not.toHaveBeenCalled();
+		} );


These two tests are almost identical. We could move the isServerRunning mock to the first test case and remove the second one.

fredrikekelund · 2026-01-07T13:07:57Z

+	const createTestSite = (): SiteData => ( {
+		id: 'site-1',
+		name: 'Test Site',
+		path: testSitePath,
+		port: 8080,
+		phpVersion: '8.0',
+		adminUsername: 'admin',
+		adminPassword: 'password123',
+	} );
+
+	const createTestSiteWithDomain = (): SiteData => ( {


Suggested change

const createTestSite = (): SiteData => ( {

id: 'site-1',

name: 'Test Site',

path: testSitePath,

port: 8080,

phpVersion: '8.0',

adminUsername: 'admin',

adminPassword: 'password123',

} );

const createTestSiteWithDomain = (): SiteData => ( {

const getTestSite = (): SiteData => ( {

id: 'site-1',

name: 'Test Site',

path: testSitePath,

port: 8080,

phpVersion: '8.0',

adminUsername: 'admin',

adminPassword: 'password123',

} );

const getTestSiteWithDomain = (): SiteData => ( {

Truly nitpicking, but create seems to potentially imply that this function has side effects, whereas get implies that it doesn't. Up to you, though.

fredrikekelund · 2026-01-07T13:09:13Z

+		it( 'should update HTTPS setting', async () => {
+			const siteWithDomain = createTestSiteWithDomain();
+			( getSiteByFolder as jest.Mock ).mockResolvedValue( siteWithDomain );
+			( readAppdata as jest.Mock ).mockResolvedValue( {
+				sites: [ siteWithDomain ],
+				snapshots: [],
+			} );
+
+			await runCommand( testSitePath, { https: true } );
+
+			const savedAppdata = ( saveAppdata as jest.Mock ).mock.calls[ 0 ][ 0 ];
+			expect( savedAppdata.sites[ 0 ].enableHttps ).toBe( true );
+		} );
+
+		it( 'should restart running site when HTTPS changes', async () => {
+			const siteWithDomain = createTestSiteWithDomain();
+			( getSiteByFolder as jest.Mock ).mockResolvedValue( siteWithDomain );
+			( readAppdata as jest.Mock ).mockResolvedValue( {
+				sites: [ siteWithDomain ],
+				snapshots: [],
+			} );
+			( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription );
+
+			await runCommand( testSitePath, { https: true } );
+
+			expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' );
+			expect( startWordPressServer ).toHaveBeenCalled();


Let's differentiate these tests by asserting that stopWordPressServer and startWordPressServer weren't called in the first test.

fredrikekelund · 2026-01-07T13:09:34Z

+		it( 'should update PHP version', async () => {
+			await runCommand( testSitePath, { php: '8.2' } );
+
+			const savedAppdata = ( saveAppdata as jest.Mock ).mock.calls[ 0 ][ 0 ];
+			expect( savedAppdata.sites[ 0 ].phpVersion ).toBe( '8.2' );
+		} );
+
+		it( 'should restart running site when PHP version changes', async () => {
+			( isServerRunning as jest.Mock ).mockResolvedValue( testProcessDescription );
+
+			await runCommand( testSitePath, { php: '8.2' } );
+
+			expect( stopWordPressServer ).toHaveBeenCalledWith( 'site-1' );
+			expect( startWordPressServer ).toHaveBeenCalled();


Same thing here: let's differentiate the tests by asserting that stopWordPressServer and startWordPressServer weren't called in the first test.

fredrikekelund · 2026-01-07T13:11:35Z

+		const mockWpCliResponse = {
+			exitCode: Promise.resolve( 0 ),
+		};
+
+		beforeEach( () => {
+			( runWpCliCommand as jest.Mock ).mockResolvedValue( [
+				mockWpCliResponse,
+				jest.fn().mockResolvedValue( undefined ),
+			] );
+		} );


Suggested change

const mockWpCliResponse = {

exitCode: Promise.resolve( 0 ),

};

beforeEach( () => {

( runWpCliCommand as jest.Mock ).mockResolvedValue( [

mockWpCliResponse,

jest.fn().mockResolvedValue( undefined ),

] );

} );

beforeEach( () => {

( runWpCliCommand as jest.Mock ).mockResolvedValue( [

{ exitCode: Promise.resolve( 0 ) },

jest.fn().mockResolvedValue( undefined ),

] );

} );

Another true nit, but we might as well make this a bit more compact.

fredrikekelund · 2026-01-07T13:15:31Z

+		it( 'should update isWpAutoUpdating when setting to latest', async () => {
+			await runCommand( testSitePath, { wp: 'latest' } );
+
+			// Second saveAppdata call updates isWpAutoUpdating
+			expect( saveAppdata ).toHaveBeenCalledTimes( 2 );
+		} );


Suggested change

it( 'should update isWpAutoUpdating when setting to latest', async () => {

await runCommand( testSitePath, { wp: 'latest' } );

// Second saveAppdata call updates isWpAutoUpdating

expect( saveAppdata ).toHaveBeenCalledTimes( 2 );

} );

it( 'should update isWpAutoUpdating to false when using specific version', async () => {

await runCommand( testSitePath, { wp: '6.8' } );

expect( saveAppdata ).toHaveBeenCalledWith(

expect.objectContaining( {

sites: expect.arrayContaining( [

expect.objectContaining( { isWpAutoUpdating: false } ),

] ),

} )

);

} );

Since isWpAutoUpdating defaults to true, I think it's better to test that we set it to false. Also, let's actually test that and not just check the number of times saveAppdata is called.

fredrikekelund · 2026-01-07T13:18:10Z

+			expect( startWordPressServer ).toHaveBeenCalledTimes( 1 );
+		} );
+
+		it( 'should handle name + domain change (only domain triggers restart)', async () => {


Suggested change

it( 'should handle name + domain change (only domain triggers restart)', async () => {

it( 'should restart if options contain changes that require restart and ones that don\'t', async () => {

fredrikekelund · 2026-01-07T13:19:03Z

Could you elaborate on why we need this change, @bcotrim? I seem to remember testing the current setup and not having any trouble with it.

Changing WP version on a site with a custom domain and https, would make the site no longer accessible.
My assumption was that the command to change wordpress version somehow uses the site url.
Using the correct site URL when running the wp upgrade command succeeded and the site was accessible.

OK, cool. Let's

Consolidate studio site set-* commands into single command

37554f0

bcotrim self-assigned this Jan 6, 2026

bcotrim requested review from a team and fredrikekelund January 6, 2026 17:36

fredrikekelund approved these changes Jan 7, 2026

View reviewed changes

address pr feedback

1eca302

bcotrim merged commit 3be67b8 into dev/studio-cli-i2 Jan 7, 2026
3 of 4 checks passed

bcotrim deleted the stu-1079-studio-consolidate-studio-site-set-commands-into-a-single branch January 7, 2026 15:27

bcotrim mentioned this pull request Jan 7, 2026

Studio CLI: Add support for xdebug beta flag #2316

Merged

1 task

bcotrim added a commit that referenced this pull request Jan 15, 2026

Consolidate studio site set-* commands into single command (#2355)

ae79eee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Consolidate studio site set-* commands into single command - #2355

Consolidate studio site set-* commands into single command#2355
bcotrim merged 2 commits into
dev/studio-cli-i2from
stu-1079-studio-consolidate-studio-site-set-commands-into-a-single

bcotrim commented Jan 6, 2026 •

edited

Loading

fredrikekelund commented Jan 7, 2026

fredrikekelund left a comment

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

fredrikekelund Jan 7, 2026

bcotrim Jan 7, 2026

fredrikekelund Jan 7, 2026

Uh oh!

Labels

2 participants

	const zipUrl = getWordPressVersionUrl( wp! );
	const zipUrl = getWordPressVersionUrl( wp );

-		it( 'should update isWpAutoUpdating when setting to latest', async () => {
-			await runCommand( testSitePath, { wp: 'latest' } );
-			// Second saveAppdata call updates isWpAutoUpdating
-			expect( saveAppdata ).toHaveBeenCalledTimes( 2 );
-		} );
+		it( 'should update isWpAutoUpdating to false when using specific version', async () => {
+			await runCommand( testSitePath, { wp: '6.8' } );
+			expect( saveAppdata ).toHaveBeenCalledWith(
+				expect.objectContaining( {
+					sites: expect.arrayContaining( [
+						expect.objectContaining( { isWpAutoUpdating: false } ),
+					] ),
+				} )
+			);
+		} );

	it( 'should handle name + domain change (only domain triggers restart)', async () => {
	it( 'should restart if options contain changes that require restart and ones that don\'t', async () => {

Uh oh!

Conversation

bcotrim commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

fredrikekelund commented Jan 7, 2026

fredrikekelund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Labels

2 participants

bcotrim commented Jan 6, 2026 •

edited

Loading