Fix WP-CLI tool calls from Studio Code when using the native PHP runtime #3847
Conversation
|
Going to give this a try. I wanted to note something as well in our skills and system prompt we have some guidelines in places to tell the agent exactly how to use wp cli (avoid file paths...) in order to make it work with playground. I believe that part is probably not necessary with the php runtime and we could have some condition to drop these entirely from the system prompt and just let the agent do what it thinks is best. |
| const result = await sendWpCliCommand( siteId, [ 'theme', 'activate', slug ] ); | ||
| if ( result.exitCode !== 0 ) { | ||
| const detail = ( result.stderr || result.stdout || '' ).trim(); | ||
| await using command = await runWpCliCommandWithMessaging( site, [ |
There was a problem hiding this comment.
If you're not familiar with the using keyword, I recommend a quick read-up on JavaScript resource management.
In short, it saves us from having to call a cleanup function because the JS runtime automatically calls the method identified by Symbol.dispose when exiting the scope.
Why do we need a cleanup function, you ask? Because when the site is using the Playground runtime but isn't running, we have to instantiate a new PHP-WASM instance that needs to be killed when we're done. We could kill the PHP-WASM instance once all stdout/stderr content has been buffered, but because PHP-WASM also provides streams for stdout/stderr, and we sometimes want to use those streams instead of the buffered output, we need to do it this way.
sejas
left a comment
There was a problem hiding this comment.
I tested it and everything worked well. I didn't catch any regression.
I was able to create a new site which showed Runtime Native, then I created a new post and installed a plugin which executed wp-cli for both actions. Everything worked as expected.
Riad makes a great point. I'll create a separate issue to tackle it separately, and load Playground differences conditionally.
Will the PHP runtime be managed per site? That could complicate things a bit.
Create site
Create post
Install plugin
|
Works well in my testing as well. |
|
I created STU-1840 to address Studio Code Playground paths, and other specifics in a separate PR. |
bcotrim
left a comment
There was a problem hiding this comment.
I found a regression on a edge case, but still worth a fix.
Changing the php version (to a never used one) on a stopped sites, makes all studio wp commands fail since we no longer do ensurePhpBinaryAvailable
npm run cli:build
# Site must be stopped
studio site set --runtime native --php ${VERSION} --path ${SITE_PATH}
studio wp --path ${SITE_PATH} option get blognameWill result in
❯ sdev wp --path ~/Studio/my-calm-website option get blogname
✖ Failed to run WP-CLI command: spawn /Users/bcotrim/.studio/php-bin/8.2.31/php ENOENT
|
Thanks for staying alert, @bcotrim! I've added a |
Yes, it will be. I'm happy to discuss this further. Might be helpful with a bit of handover here. cc @sejas |
Related issues
How AI was used in this PR
Claude was used iteratively to review and implement individual pieces of this. I leaned on it more for review than implementation in this case, but the
stripLeadingShebangis one case where the implementation is basically all Claude.Proposed Changes
tl;dr: Studio Code was using the
sendWpCliCommandfunction to run thewp_clitool. This function doesn't work with the native PHP runtime. This PR fixes the issue by replacingsendWpCliCommandwithrunWpCliCommandWithMessaging(a wrapper aroundrunWpCliCommand).gs;wm:
The new
runWpCliCommandWithMessagingfunction checks if the target site uses the Playground runtime and if the server is running. If so, it'll forward the WP-CLI command as an IPC message to the child process. This is the same behavior that we had withsendWpCliCommand, except the new function falls back torunWpCliCommand, meaning the server doesn't need to be running (or using the native PHP runtime).So, aside from making Studio Code work with the native PHP runtime, we also gain more flexibility with the
wp_clitool.Moreover, I've streamlined
apps/cli/commands/wp.tsto also use therunWpCliCommandfunction. To do this, I had to add anstdiooption to that function, to allow the native PHP child process to be spawned withstdio: 'inherit'(which is what setsapps/cli/commands/wp.tsapart from otherrunWpCliCommandcallers).Testing Instructions
npm run cli:buildSTUDIO_RUNTIME=native-php node apps/cli/dist/cli/main.mjs codeCreate siteoutput includesChecking PHP N.N binary…WP-CLItool calls don't stallPre-merge Checklist