Re-enable WP-CLI execution in the child process #2261

bcotrim · 2025-12-16T11:32:39Z

Related issues

Fixes STU-1138

Proposed Changes

Re-enable WP-CLI execution in child process (cli/commands/wp.ts)
- Set IS_WP_CLI_CHILD_PROCESS_EXECUTION_ENABLED = true
- WP-CLI commands now use the running site's child process instead of spawning new Playground instances
Add WP-CLI command queue (cli/wordpress-server-child.ts)
- WpCliCommandQueue class serializes concurrent WP-CLI commands
- MAX_CONCURRENT_WP_CLI_COMMANDS = 3 - limits concurrent playground.cli() calls to prevent MaxPhpInstancesError
- MAX_WP_CLI_QUEUE_SIZE = 10 - prevents unbounded queue growth, rejects new commands when full
Fix messageId collision in IPC (cli/lib/wordpress-server-manager.ts)
- Replace sequential nextMessageId++ with generateMessageId() using timestamp + random
- Prevents concurrent CLI processes from receiving each other's responses

Testing Instructions

Setup

npm run cli:build
node dist/cli/main.js site create --path /tmp/test-site --name "Test" --skip-browser

Test 1: Concurrent commands return correct results

# Each command should return its own correct result (not mixed up)
node dist/cli/main.js wp --path /tmp/test-site option get blogname &
node dist/cli/main.js wp --path /tmp/test-site option get siteurl &
node dist/cli/main.js wp --path /tmp/test-site option get admin_email &
node dist/cli/main.js wp --path /tmp/test-site plugin list --format=count &
wait

Expected: Each returns different, correct values (e.g., "My WordPress Website", "http://localhost:XXXX", "[email protected]", "3")

Test 2: Site doesn't crash with concurrent commands

curl -s -o /dev/null -w "Before: %{http_code}" http://localhost:<PORT>
for i in {1..10}; do
  node dist/cli/main.js wp --path /tmp/test-site option get blogname &
done
wait
curl -s -o /dev/null -w "After: %{http_code}" http://localhost:<PORT>

Expected: Site returns HTTP 200 both before and after (no crash)

Test 3: Failed commands don't crash the server

node dist/cli/main.js wp --path /tmp/test-site nonexistent-command
curl -s -o /dev/null -w "Site status: %{http_code}" http://localhost:<PORT>

Expected: Command shows error message, site still returns HTTP 200

Test 4: Queue rejects commands when full

# Send 15 slow commands (3 active + 10 queue = 13 max, so 2 should fail)
for i in {1..15}; do
  node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1 &
done
wait

Expected: 13 commands succeed, 2 commands fail with "WP-CLI command queue is full (10 pending commands). Please try again later."

Cleanup

node dist/cli/main.js site stop --path /tmp/test-site
rm -rf /tmp/test-site

Pre-merge Checklist

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

fredrikekelund

Seeing your solution in this PR, I realized that we already have a sequential helper function in common/lib/sequential.ts that can accomplish this a little more succinctly. If we apply the following diff, implement my inline suggestion and restore the function body of runWpCliCommand, I believe we should be good.

diff --git a/common/lib/sequential.ts b/common/lib/sequential.ts
index 593ee246..d7698147 100644
--- a/common/lib/sequential.ts
+++ b/common/lib/sequential.ts
@@ -2,6 +2,7 @@ const sequentialLocks = new Map< () => Promise< unknown >, Set< Promise< unknown
 
 // Ensures that calls to the provided function are executed sequentially
 export function sequential< Args extends unknown[], Return >(
+       concurrentCount: number,
        fn: ( ...args: Args ) => Promise< Return >
 ) {
        return async ( ...args: Args ) => {
@@ -10,7 +11,7 @@ export function sequential< Args extends unknown[], Return >(
                        sequentialLocks.set( fn, locks );
                }
 
-               while ( locks.size ) {
+               while ( locks.size >= concurrentCount ) {
                        await Promise.allSettled( [ ...locks ] );
                }

Let me know your thoughts, @bcotrim

fredrikekelund · 2025-12-16T13:28:54Z

cli/wordpress-server-child.ts

-async function runWpCliCommand(
-	args: string[]
-): Promise< { stdout: string; stderr: string; exitCode: number } > {


const runWpCliCommand = sequential( 3, async ( args: string[] ): Promise< WpCliResult > => {

fredrikekelund

The "Queue rejects commands when full" test case didn't work as expected for me (with or without my proposed changes to common/lib/sequential.ts). This definitely fixes the core issue, though. I've confirmed that I can run WP-CLI commands from the Assistant without any issues.

Great work identifying the messageId issue, too 👍

fredrikekelund · 2025-12-17T10:46:12Z

cli/lib/wordpress-server-manager.ts

+/**
+ * Generate a unique message ID that won't collide across concurrent CLI processes.
+ */
+function generateMessageId(): number {
+	return Date.now() * 1000 + Math.floor( Math.random() * 1000 );
+}
+


Oh man, good catch 👍 This is definitely part of the reason I was seeing jumbled output when running multiple WP-CLI commands in quick succession.

The current implementation is fine, but I guess we could also use crypto.randomUUID(). Any thoughts, @bcotrim?

fredrikekelund · 2025-12-17T10:49:17Z

cli/wordpress-server-child.ts

+/**
+ * Result type for WP-CLI command execution
+ */
+type WpCliResult = { stdout: string; stderr: string; exitCode: number };
+


This is no big deal, but we could easily keep this inlined in the return type for runWpCliCommand, IMO. Since we're not reusing it, I mean.

fredrikekelund · 2025-12-17T11:00:41Z

common/lib/sequential.ts

+		if ( locks.size >= concurrentCount ) {
+			if ( maxQueueSize !== undefined && queueCount >= maxQueueSize ) {
+				throw new Error(
+					`Queue is full (${ maxQueueSize } pending commands). Please try again later.`
+				);
+			}

-		while ( locks.size ) {
-			await Promise.allSettled( [ ...locks ] );
+			queueCount++;
+			try {
+				while ( locks.size >= concurrentCount ) {
+					await Promise.allSettled( [ ...locks ] );
+				}
+			} finally {
+				queueCount--;
+			}
 		}


if ( maxQueueSize !== undefined && queueCount >= maxQueueSize ) { throw new Error( `Queue is full (${ maxQueueSize } pending commands). Please try again later.` ); } while ( locks.size >= concurrentCount ) { queueCount++; await Promise.allSettled( [ ...locks ] ); queueCount--; }

This is my suggestion ⬆️

A couple of comments:

The initial if ( locks.size >= concurrentCount ) doesn't really make sense to me. We should check the queue count before checking the lock count.

There's no need to wrap Promise.allSettled in a try..catch, since it never rejects (unlike Promise.all).

By moving the queueCount increment and decrement operations inside the while loop, we ensure that the queueCount variable isn't touched if the lock count doesn't exceed concurrentCount. This doesn't really matter, though, since we don't run any asynchronous operations if this is case. Consider this an optional nit that maybe just looks cleaner 🙂

fredrikekelund · 2025-12-17T11:02:38Z

common/lib/sequential.ts

+		locks.add( fnPromise );

 		try {
-			locks.add( fnPromise );


This is probably fine, but I guess the locks.add call was previously part of the try clause to clarify its relationship to the finally clause. I don't believe this change makes any functional difference, though, so feel free to implement it whichever way you prefer, @bcotrim

fredrikekelund · 2025-12-17T11:09:31Z

common/lib/sequential.ts

 ) {
+	const concurrentCount = options?.concurrent ?? 1;
+	const maxQueueSize = options?.max;
+	const locks = new Set< Promise< unknown > >();


Suggested change

const locks = new Set< Promise< unknown > >();

const locks = new Set< Promise< Return > >();

Good idea moving locks inside the closure 👍 It definitely simplifies things. We might as well use the correct return type now, though.

bcotrim · 2025-12-17T13:12:05Z

The "Queue rejects commands when full" test case didn't work as expected for me (with or without my proposed changes to common/lib/sequential.ts). This definitely fixes the core issue, though. I've confirmed that I can run WP-CLI commands from the Assistant without any issues.

Great work identifying the messageId issue, too 👍

❯ for i in {1..15}; do
  node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1 &
done
wait
[2] 41596
[3] 41597
[4] 41598
[5] 41599
[6] 41600
[7] 41601
[8] 41602
[9] 41603
[10] 41604
[11] 41605
[12] 41606
[13] 41607
[14] 41608
[15] 41609
[16] 41610
✖ Failed to run WP-CLI command: Queue is full (10 pending commands). Please try again later.
✖ Failed to run WP-CLI command: Queue is full (10 pending commands). Please try again later.
[14]    exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[2]    exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[5]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[9]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[12]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[4]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[16]  + done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[8]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[6]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[15]  + done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[7]    done       node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
✖ Failed to run WP-CLI command: Timeout waiting for response to message 2d2c6a3a-9e83-4114-8bcb-21adbdc8cef9: No activity for 120s
✖ Failed to run WP-CLI command: Timeout waiting for response to message ab1da4ca-bb4e-4971-8f6e-87b0a85c9564: No activity for 120s
✖ Failed to run WP-CLI command: Timeout waiting for response to message 6b933194-c256-4118-bc08-8f4ec6708500: No activity for 120s
✖ Failed to run WP-CLI command: Timeout waiting for response to message 0c5c871d-dff6-4150-9ee9-fda246e3d71c: No activity for 120s
[11]  - exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[3]    exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[10]  - exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1
[13]  + exit 1     node dist/cli/main.js wp --path /tmp/test-site eval "sleep(30);" 2>&1

Were you referencing the timeouts? I feel like that is expected, we could not timeout something while it is in queue, but it might be overthinking at this point.
What do you think?

fredrikekelund · 2025-12-17T13:29:33Z

No, I meant that I didn't get any timeout errors. I didn't get any Playground errors about exceeding the maximum number of request handlers either, though.

I say let's land this PR and revisit this later if needed 👍

fredrikekelund · 2025-12-18T14:55:03Z

Let's change the base branch to dev/studio-cli-i2 and merge this.

bcotrim requested review from a team and fredrikekelund December 16, 2025 11:32

bcotrim self-assigned this Dec 16, 2025

bcotrim marked this pull request as ready for review December 16, 2025 11:44

fredrikekelund reviewed Dec 16, 2025

View reviewed changes

bcotrim changed the title ~~implement wp cli queue and fix message id~~ Re-enable WP-CLI execution in the child process Dec 16, 2025

bcotrim force-pushed the stu-1138-studio-re-enable-wp-cli-execution-in-the-child-process branch from e3fe64e to b536fec Compare December 16, 2025 22:19

bcotrim requested a review from fredrikekelund December 16, 2025 22:19

bcotrim force-pushed the stu-1138-studio-re-enable-wp-cli-execution-in-the-child-process branch from 2881990 to b536fec Compare December 17, 2025 08:23

fredrikekelund force-pushed the dev/studio-cli-i2 branch from 40c541c to bf9c008 Compare December 17, 2025 10:08

Base automatically changed from dev/studio-cli-i2 to trunk December 17, 2025 10:28

fredrikekelund approved these changes Dec 17, 2025

View reviewed changes

fredrikekelund and others added 9 commits December 17, 2025 17:50

Capture more details about failure

3cba6e3

Run a specific E2E test

065ff4a

Fix screenshot path

e5f30c3

implement wp cli queue and fix message id

5dc3b7b

fix lint

8d1033c

use sequential function

1f500e1

Fix merge conflicts

95ac72a

address pr feedback

bfb2c19

fix unit tests

52c93b1

bcotrim force-pushed the stu-1138-studio-re-enable-wp-cli-execution-in-the-child-process branch from 7c11d5c to 52c93b1 Compare December 17, 2025 17:51

fredrikekelund mentioned this pull request Dec 18, 2025

Ensure all CLI commands can be aborted #2282

Open

1 task

bcotrim changed the base branch from trunk to dev/cli-e2e December 18, 2025 15:06

bcotrim changed the base branch from dev/cli-e2e to dev/studio-cli-i2 December 18, 2025 15:07

bcotrim merged commit f83579a into dev/studio-cli-i2 Dec 18, 2025
9 of 11 checks passed

bcotrim deleted the stu-1138-studio-re-enable-wp-cli-execution-in-the-child-process branch December 18, 2025 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Re-enable WP-CLI execution in the child process #2261

Re-enable WP-CLI execution in the child process #2261

bcotrim commented Dec 16, 2025 •

edited

Loading

Uh oh!

fredrikekelund left a comment

Uh oh!

fredrikekelund Dec 16, 2025

Uh oh!

fredrikekelund left a comment

Uh oh!

fredrikekelund Dec 17, 2025

Uh oh!

fredrikekelund Dec 17, 2025

Uh oh!

fredrikekelund Dec 17, 2025

Uh oh!

fredrikekelund Dec 17, 2025

Uh oh!

fredrikekelund Dec 17, 2025

Uh oh!

bcotrim commented Dec 17, 2025

Uh oh!

fredrikekelund commented Dec 17, 2025

Uh oh!

fredrikekelund commented Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const locks = new Set< Promise< unknown > >();
	const locks = new Set< Promise< Return > >();

Re-enable WP-CLI execution in the child process #2261

Re-enable WP-CLI execution in the child process #2261

Conversation

bcotrim commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Setup

Test 1: Concurrent commands return correct results

Test 2: Site doesn't crash with concurrent commands

Test 3: Failed commands don't crash the server

Test 4: Queue rejects commands when full

Cleanup

Pre-merge Checklist

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

fredrikekelund Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

bcotrim commented Dec 17, 2025

Uh oh!

fredrikekelund commented Dec 17, 2025

Uh oh!

fredrikekelund commented Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bcotrim commented Dec 16, 2025 •

edited

Loading