feat: Invoke `wp-cli` from terminal sessions originating from Studio #213

dcalhoun · 2024-06-06T01:32:50Z

Relates to https://github.com/Automattic/dotcom-forge/issues/7327.

Proposed Changes

Expose the bundled wp-cli for terminal sessions originating from Studio.

Testing Instructions

1. Generic commands succeed

Check out the proposed changes.
npm run package
Launch the app found in the out directory.
Click "Open in Terminal" for a given site.
In the terminal session, wp help or wp --version.
Verify the executed wp-cli subcommand outputs the expected information.

2. Site-specific commands succeed

Check out the proposed changes.
npm run package
Launch the app found in the out directory.
Click "Open in Terminal" for a given site.
In the terminal session, wp user list.
Verify the executed wp-cli subcommand outputs the expected information for
the targeted site.

3. Studio's bundled wp-cli is preferred over global wp-cli install

Install wp-cli on your system.
Check out the proposed changes.
npm run package
Launch the app found in the out directory.
Click "Open in Terminal" for a given site.
In the terminal session, which wp (macOS) or where wp (Windows).
Verify the outputted location is Studio's bundled wp-cli.
Open a new terminal session outside of Studio, which wp (macOS) or where wp (Windows).
Verify the logged location is not Studio's bundled wp-cli, but the globally installed wp-cli instead.

Pre-merge Checklist

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

src/ipc-handlers.ts

src/lib/cli.ts

dcalhoun · 2024-06-07T19:40:52Z

vendor/wp-now/src/execute-wp-cli.ts

@@ -66,7 +66,7 @@ export async function executeWPCli ( projectPath: string, args: string[] ): Prom

 		return { stdout: result.text.replace('#!/usr/bin/env php', '').trim(), stderr: result.errors };
 	} catch (error) {
-		const errorContent = php.readFileAsText(stderrPath);
+		const errorContent = php.readFileAsText(stderrPath).replace('PHP.run() output was: #!/usr/bin/env php', '').trim();


Updated to match the stdout changes above and avoid unnecessary noise in the logs.

bin/wp.bat

src/lib/cli.ts

dcalhoun · 2024-06-07T19:56:10Z

src/lib/cli.ts

Most of this logic was inspired by a previous exploration in https://github.com/Automattic/local-environment/pull/174. Some of the logic was removed to simplify the source for the current feature scope.

dcalhoun · 2024-06-07T20:22:55Z

forge.config.ts

@@ -21,7 +21,7 @@ import type { ForgeConfig } from '@electron-forge/shared-types';
 const config: ForgeConfig = {
 	packagerConfig: {
 		asar: true,
-		extraResource: [ './wp-files', './assets' ],
+		extraResource: [ './wp-files', './assets', './bin' ],


This copies both bin/wp (macOS) and bin/wp.bat (Windows). Ideally, we copy only the relevant file for the given platform.

fluiddot · 2024-06-10T11:34:17Z

When I was testing https://github.com/Automattic/local-environment/pull/174, I recall that I managed to use the development server by doing the following steps:

Run npm start to launch the first instance of the app.
In the Electron Forge configuration, change the ports used for the second instance:
- Change the value of the development port to 3457.
- Add the loggerPort property with the value 9001 (9000 is the default value).
Run npm start -- -- --cli"=<CLI-COMMAND>" in another terminal instance.

dcalhoun · 2024-06-10T21:42:46Z

vendor/wp-now/src/execute-wp-cli.ts

+	const php: NodePHP = await NodePHP.load(options.phpVersion,{
+		requestHandler: {
+			documentRoot: options.documentRoot,
+			absoluteUrl: options.absoluteUrl,
+		}
 	});
-	const [, php] = phpInstances;
+	php.mount(projectPath, options.documentRoot);


Improve wp-cli performance by avoiding starting the site. In theory, we only need a PHP instance.

Nice! Love it!

We cannot modify the search path with environment variables alone, we must execute an arbitrary script.

For shell sessions originating from Studio, the bundled `wp-cli` should take precedence over a user's existing `wp-cli` installation.

Previously, apparent windows from previous Terminal sessions would linger and appear when launching a new Terminal.

The preceding `C:` was duplicated in the CLI path.

The script must contain the `.bat` file ending in order to run the script on Windows.

The internals now return a result rather than logging the result.

Match the approach for `stdout` and avoid displaying unnecessary noise.

The `executeWpCli` command relies upon `console.warn`, which resulted in unnecessary noise when invoking the `wp-cli` in the console.

This logic is likely unnecessary for the current usage with `wp-cli`.

…stance

Mimic core `wp-cli`'s behavior for the `help` command.

fluiddot

Testing Commands

I tested different commands on macOS and found that some don't work as expected:

wp cli info: The execution freezes and doesn't respond.
wp cron test: Only works when the site is running.
wp db create: The execution freezes and doesn't respond.
wp db check: The execution freezes and doesn't respond.
wp db cli: The execution freezes and doesn't respond.
wp db query < debug.sql: Produces error env: mysql: No such file or directory.
wp eval 'echo WP_CONTENT_DIR;': Produces error: Error: Too many positional arguments: WP_CONTENT_DIR;.
wp eval-file eval.php: Produces error: Error: 'eval.php' does not exist.
- It works when pointing to the server path: wp eval-file /var/www/html/eval.php
wp export: Export process succeed but the file can't be located.
- It works when pointing to the server path: wp export --dir=/var/www/html
wp i18n make-pot . languages/hello-dolly.pot: Succeeds but the file can't be located.
- It works when pointing to the server path: wp i18n make-pot . /var/www/html/languages/hello-dolly.pot
wp import --authors=create export.xml: Can't find the file.
- It works when pointing to the server path: wp import --authors=create /var/www/html/export.xml
wp menu create "My Menu": Produces error Error: Too many positional arguments: Menu".
wp package install wp-cli/server-command: Produces error: Error: The "https://wp-cli.org/package-index/packages.json" file could not be downloaded: allow_url_fopen must be enabled in php.ini (https:// wrapper is disabled in the server configuration by allow_url_fopen=0.
wp server: The execution freezes and doesn't respond.
wp shell: Produces error Error: The shell binary '/bin/bash' is not valid. You can override the shell to be used through the WP_CLI_CUSTOM_SHELL environment variable..

I also noticed that commands that prompt user's input are quitted, not allowing the user to input anything.

Based on the above, I propose three items to investigate:

For frozen commands, we need to figure out how to debug them. It's likely that something is failing and we don't see the error.
String arguments (wrapped with ") are not processed properly. We'd need to review how they are passed to wp-cli.
File paths need to be prefixed with the document root (e.g. /var/www/html). The fact that you are in the site folder but can't point to files will be confusing to users. Besides, we need to warn users that only files within the site folder can be accessed (this is a limitation as we only mount this path).

UPDATE: I saw in the Playground documentation that it doesn't support the full list of wp-cli commands. Probably some of the ones mentioned above (especially the ones that freeze) are likely not working due to this limitation.

Other Tests

Additionally, I tested changes introduced in #203 (using the test button) and confirmed WP-CLI commands can be executed and display the excepted output.

fluiddot · 2024-06-12T15:08:25Z

vendor/wp-now/src/execute-wp-cli.ts

+	const php: NodePHP = await NodePHP.load(options.phpVersion,{
+		requestHandler: {
+			documentRoot: options.documentRoot,
+			absoluteUrl: options.absoluteUrl,
+		}
 	});
-	const [, php] = phpInstances;
+	php.mount(projectPath, options.documentRoot);


For reference, when we bump the @php-wasm/node version, we'll need to update this part to conform this breaking change.

fluiddot · 2024-06-12T15:10:20Z

vendor/wp-now/src/execute-wp-cli.ts

+	const php: NodePHP = await NodePHP.load(options.phpVersion,{
+		requestHandler: {
+			documentRoot: options.documentRoot,
+			absoluteUrl: options.absoluteUrl,
+		}
 	});
-	const [, php] = phpInstances;
+	php.mount(projectPath, options.documentRoot);


For reference, the project path is mounted following the current logic when running the site in WordPress mode:

studio/vendor/wp-now/src/wp-now.ts

Lines 186 to 190 in 4320082

async function runWordPressMode(

php: NodePHP,

{ documentRoot, projectPath, absoluteUrl }: WPNowOptions

) {

php.mount(projectPath, documentRoot);

fluiddot · 2024-06-12T15:38:05Z

src/index.ts

-		process.env.NODE_ENV !== 'development' && process.env.NODE_ENV !== 'test' && ! process.env.E2E,
-	release: `${ app.getVersion() ? app.getVersion() : COMMIT_HASH }-${ getPlatformName() }`,
-} );
+if ( ! isCLI() ) {


Maybe we should enable Sentry in second instances (like when invoking the CLI) to be able to collect exceptions produced when executing CLI commands. I originally disabled this in the PoC because it was interfering with the testing, but might be interesting to consider it.

Let's explore this option in a separate PR.

fluiddot · 2024-06-12T16:05:23Z

src/lib/cli.ts

+
+const commands = {
+	wp: async () => {
+		disableConsole();


It might be interesting to rewire console logs to the logging system instead of disabling it. This will facilitate debugging potential issues on CLI commands.

Let's explore this option in a separate PR.

fluiddot · 2024-06-13T11:14:40Z

vendor/wp-now/src/execute-wp-cli.ts

+		// When running PHP on Playground, the value of constant PHP_OS is set to Linux.
+		// This implies that platform-specific logic won't work as expected. To solve this,
+		// we use an environment variable to ensure WP-CLI runs the Windows-specific logic.
+		putenv( 'WP_CLI_TEST_IS_WINDOWS=${isWindows ? 1 : 0}' );


This environment variable is mostly used by unit tests in wp-cli. As described in the inline comment, it allows us to tell wp-cli when the command is being executed in Windows. It should be enough as a temporary workaround but in the future, we should plan a more robust solution to match the value provided in PHP_OS with the actual OS.

This library is used to parse the wp-cli arguments.

fluiddot · 2024-06-13T15:20:14Z

bin/wp.bat

+)
+
+SET COMMAND=%*
+SET CLI=wp %COMMAND%


I noticed that running commands that include quotes in the arguments don't work on Windows. I tried different approaches but nonce seemed to work.

E.g.: wp eval 'echo WP_CONTENT_DIR;'

# Conflicts: # package-lock.json # package.json # src/__mocks__/electron.ts # src/index.ts

fluiddot · 2024-06-13T15:55:35Z

package.json

@@ -115,6 +116,7 @@
 		"hpagent": "1.2.0",
 		"react-markdown": "^9.0.1",
 		"semver": "^7.6.0",
+		"shell-quote": "^1.8.1",


This package is used to parse the CLI arguments passed as a single string to an array of strings, as need to invoke wp-cli.

fluiddot · 2024-06-13T16:00:26Z

After the fixes 8b9c2aa, 97a6a4d, and aa9d943, I reviewed the failed commands and some of them now work 🎊.

Here's the list of pending commands to be addressed:

wp cli info: The execution freezes and doesn't respond.
wp cron test: Only works when the site is running.
wp db create: The execution freezes and doesn't respond.
wp db check: The execution freezes and doesn't respond.
wp db cli: The execution freezes and doesn't respond.
wp db query < debug.sql: Produces error `env: mysql: No such file or directory
wp package install wp-cli/server-command: Produces error: Error: The "https://wp-cli.org/package-index/packages.json" file could not be downloaded: allow_url_fopen must be enabled in php.ini (https:// wrapper is disabled in the server configuration by allow_url_fopen=0.
wp server: The execution freezes and doesn't respond.
wp shell: Produces error Error: The shell binary '/bin/bash' is not valid. You can override the shell to be used through the WP_CLI_CUSTOM_SHELL environment variable..

For some of the above errors, the logs are showing an error related to the File System but couldn't pinpoint the code that produces it.

kozer

@dcalhoun and @fluiddot I created a PR that fixes terminal issues on linux: #257

Note that this fix the execution on development as well by using the local running app.

However, I didn't made any fixes for MacOS and Windows, as I couldn't test it.

Other than that, it works as expected, besides the wp shell command ( at least for me ). Ignore the warnings in the image, it's my problem

kozer · 2024-06-17T13:18:42Z

Updated the PR, as discussed @fluiddot

sejas

I tested it on Mac and it works as expected.

From my understanding, this PR is adding to the terminal PATH a bash script that calls Studio app with the --cli flag which calls executeWPCli that runs php-wasm importing the wp-cli.phar that lives in server-files library directory.

wp-cli-success.mp4

I also tested using the parameter --path and it correctly detected the site.

I also tested executing a non existing command and it display the error correctly. Although the text was returned as stdout and not stderr. Left a comment to fix it.

Congrats for this piece of engineering 🥳

sejas · 2024-06-17T22:19:12Z

bin/wp

@@ -0,0 +1,11 @@
+if [ -z "$APP_PATH" ]; then


Should we add STUDIO as a prefix to reduce the possibility of conflicting with other env variables?

Suggested change

if [ -z "$APP_PATH" ]; then

if [ -z "$STUDIO_APP_PATH" ]; then

Yeah, good idea. Better to use a unique name to avoid conflicts.

Suggestion applied in 9a4253a.

sejas · 2024-06-17T22:43:02Z

vendor/wp-now/src/execute-wp-cli.ts

+	const php: NodePHP = await NodePHP.load(options.phpVersion,{
+		requestHandler: {
+			documentRoot: options.documentRoot,
+			absoluteUrl: options.absoluteUrl,
+		}
 	});
-	const [, php] = phpInstances;
+	php.mount(projectPath, options.documentRoot);


Nice! Love it!

sejas · 2024-06-17T22:49:47Z

src/index.ts

+		// Handle CLI commands
+		listenCLICommands();
+		executeCLICommand();


Should we wrap these calls inside if ( isCLI() ) ?

isCLI function only checks if the current instance was launched with a CLI command. listenCLICommands needs to be run on the main instance to set up the second-instance event, which is invoked when a second instance is launched. Hence, it needs to be executed independently if a CLI command is passed in the main instance. I'd like to note that this scenario doesn't apply to wp-cli commands as they are meant to be executed in the same instance they are triggered. However, in the future, it might be interesting to run commands on the first instance to manage sites.

Here's a diagram that explains the process:

Note
In the diagram, wp-cli commands are considered standalone commands.

executeCLICommand could be wrapped with if ( isCLI() ) but the function already checks if a CLI command exists.

sejas · 2024-06-18T00:13:19Z

src/lib/cli.ts

+		try {
+			const result = await executeWPCli( projectPath, argsSansPath );
+			if ( result.stderr ) {
+				systemLog( result.stderr );


Lets declare const systemError = console.error; and call it on error to produce the exit in the right "file descriptor" 2.

Suggested change

systemLog( result.stderr );

systemError( result.stderr );

I'm not sure if using console.error is actually pointing to the stderr file descriptor. In any case, I'm ok using the error function to log errors.

Suggestion applied in e20496f.

* Studio: Fix wp-cli command for linux * Update: fix variables per environment * Update: Fix wp script to handle commands on both MacOS and linux

kozer · 2024-06-18T12:39:24Z

I checked it again, after merging my linux fix and works as expected! Amazing work!

# Conflicts: # src/ipc-handlers.ts

fluiddot · 2024-06-18T13:45:36Z

src/ipc-handlers.ts

+		const exePath = app.getPath( 'exe' );
+		const appDirectory = app.getAppPath();
+		const appPath =
+			process.env.NODE_ENV === 'development' ? `${ exePath } ${ appDirectory }` : exePath;


I spotted that this condition should be ! app.isPackaged because the Electron shell app is invoked when the app is not packaged, i.e. produces its own executable. I'll address it in a separate PR.

This is addressed in #268.

dcalhoun added the [Type] Enhancement Improvement upon an existing feature label Jun 6, 2024

dcalhoun self-assigned this Jun 6, 2024

dcalhoun commented Jun 6, 2024

View reviewed changes

src/ipc-handlers.ts Outdated Show resolved Hide resolved

src/lib/cli.ts Show resolved Hide resolved

dcalhoun force-pushed the feat/execute-studio-bundled-wp-cli-electron branch 2 times, most recently from b55df25 to dd6d29f Compare June 7, 2024 17:46

dcalhoun changed the title ~~feat: Invoke wp-cli from shell sessions originating from Studio~~ feat: Invoke wp-cli from terminal sessions originating from Studio Jun 7, 2024

dcalhoun commented Jun 7, 2024

View reviewed changes

dcalhoun marked this pull request as ready for review June 10, 2024 21:40

dcalhoun requested review from a team June 10, 2024 21:41

dcalhoun commented Jun 10, 2024

View reviewed changes

dcalhoun and others added 18 commits June 11, 2024 08:06

feat: Invoke wp-cli from shell sessions originating from Studio

c9f4abd

feat: Add bundled wp-cli to macOS search $PATH

2a64efe

We cannot modify the search path with environment variables alone, we must execute an arbitrary script.

fix: Handle errors from wp-cli execution failures

a57f94d

fix: Prefer bundled wp-cli over global wp-cli

cbc9988

For shell sessions originating from Studio, the bundled `wp-cli` should take precedence over a user's existing `wp-cli` installation.

fix: Prevent launching duplicative Terminal windows

0ac1341

Previously, apparent windows from previous Terminal sessions would linger and appear when launching a new Terminal.

feat: Expose bundled wp-cli for Windows shell sessions

03079e5

fix: Correct Windows wp-cli path

dc32f9d

The preceding `C:` was duplicated in the CLI path.

fix: Add .bat file ending for Windows

8975452

The script must contain the `.bat` file ending in order to run the script on Windows.

fix: Update logging of modified new executeWpCli method

69bca19

The internals now return a result rather than logging the result.

fix: Remove prefix from wp-cli error out

18038bb

Match the approach for `stdout` and avoid displaying unnecessary noise.

feat: Silence additional console commands

f88c157

The `executeWpCli` command relies upon `console.warn`, which resulted in unnecessary noise when invoking the `wp-cli` in the console.

fix: Remove window reload after CLI command

22ae0ba

This logic is likely unnecessary for the current usage with `wp-cli`.

test: Mock Electron command line methods

b6f0e0c

Avoid focusing app when launching second instance with CLI commands

dbc3721

Prevent unhandled promise when not executing a CLI command on main in…

e4d0ec5

…stance

Ensure WP-CLI runs the Windows-specific logic

27b356b

Add server script path argument to avoid errors in WP-CLI

876521c

Run WP-CLI commands without starting a site

1c3ef82

dcalhoun added 2 commits June 11, 2024 08:08

feat: Conditionally pipe wp-cli output through a pager

5106f98

Mimic core `wp-cli`'s behavior for the `help` command.

refactor: Remove unnecessary quotation

4320082

dcalhoun force-pushed the feat/execute-studio-bundled-wp-cli-electron branch from 9e12aaf to 4320082 Compare June 11, 2024 12:10

fluiddot self-requested a review June 12, 2024 14:46

fluiddot reviewed Jun 12, 2024

View reviewed changes

fluiddot reviewed Jun 13, 2024

View reviewed changes

fluiddot added 4 commits June 13, 2024 13:23

Fix working directory when executing wp-cli commands

8b9c2aa

Add shell-quote package

639c5ef

This library is used to parse the wp-cli arguments.

Parse CLI arguments using shell-quote

97a6a4d

Fix missing quotes when invoking wp-cli commands on macOS

aa9d943

fluiddot reviewed Jun 13, 2024

View reviewed changes

Merge branch 'trunk' into feat/execute-studio-bundled-wp-cli-electron

dffb805

# Conflicts: # package-lock.json # package.json # src/__mocks__/electron.ts # src/index.ts

fluiddot reviewed Jun 13, 2024

View reviewed changes

fluiddot approved these changes Jun 13, 2024

View reviewed changes

fluiddot self-assigned this Jun 13, 2024

kozer mentioned this pull request Jun 17, 2024

Studio: Add chat context #252

Merged

1 task

kozer reviewed Jun 17, 2024

View reviewed changes

sejas approved these changes Jun 18, 2024

View reviewed changes

kozer and others added 3 commits June 18, 2024 13:21

Studio: Fix wp-cli command for linux (#257)

e4cefe1

* Studio: Fix wp-cli command for linux * Update: fix variables per environment * Update: Fix wp script to handle commands on both MacOS and linux

Rename CLI env vars with unique names

9a4253a

Fix paging for command wp help

7a5eef0

kozer approved these changes Jun 18, 2024

View reviewed changes

fluiddot added 2 commits June 18, 2024 15:08

Log wp-cli command errors using console.error

e20496f

Merge branch 'trunk' into feat/execute-studio-bundled-wp-cli-electron

e2c9beb

# Conflicts: # src/ipc-handlers.ts

fluiddot merged commit e428f51 into trunk Jun 18, 2024
10 checks passed

fluiddot deleted the feat/execute-studio-bundled-wp-cli-electron branch June 18, 2024 13:43

fluiddot reviewed Jun 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Invoke `wp-cli` from terminal sessions originating from Studio #213

feat: Invoke `wp-cli` from terminal sessions originating from Studio #213

dcalhoun commented Jun 6, 2024 •

edited

Loading

dcalhoun Jun 7, 2024

dcalhoun Jun 7, 2024

dcalhoun Jun 7, 2024

fluiddot commented Jun 10, 2024

dcalhoun Jun 10, 2024

sejas Jun 17, 2024

fluiddot left a comment •

edited

Loading

fluiddot Jun 12, 2024

fluiddot Jun 12, 2024

fluiddot Jun 12, 2024

fluiddot Jun 13, 2024

fluiddot Jun 12, 2024

fluiddot Jun 13, 2024

fluiddot Jun 13, 2024

fluiddot Jun 13, 2024 •

edited

Loading

fluiddot Jun 13, 2024

fluiddot commented Jun 13, 2024 •

edited

Loading

kozer left a comment

kozer commented Jun 17, 2024

sejas left a comment •

edited

Loading

sejas Jun 17, 2024

fluiddot Jun 18, 2024

fluiddot Jun 18, 2024

sejas Jun 17, 2024

sejas Jun 17, 2024

fluiddot Jun 18, 2024 •

edited

Loading

sejas Jun 18, 2024

fluiddot Jun 18, 2024

fluiddot Jun 18, 2024

kozer commented Jun 18, 2024

fluiddot Jun 18, 2024

fluiddot Jun 18, 2024

	async function runWordPressMode(
	php: NodePHP,
	{ documentRoot, projectPath, absoluteUrl }: WPNowOptions
	) {
	php.mount(projectPath, documentRoot);

	if [ -z "$APP_PATH" ]; then
	if [ -z "$STUDIO_APP_PATH" ]; then

feat: Invoke wp-cli from terminal sessions originating from Studio #213

feat: Invoke wp-cli from terminal sessions originating from Studio #213

Conversation

dcalhoun commented Jun 6, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot commented Jun 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot left a comment • edited Loading

Choose a reason for hiding this comment

Testing Commands

Other Tests

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

fluiddot Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot commented Jun 13, 2024 • edited Loading

kozer left a comment

Choose a reason for hiding this comment

kozer commented Jun 17, 2024

sejas left a comment • edited Loading

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

fluiddot Jun 18, 2024 • edited Loading

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

kozer commented Jun 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: Invoke `wp-cli` from terminal sessions originating from Studio #213

feat: Invoke `wp-cli` from terminal sessions originating from Studio #213

dcalhoun commented Jun 6, 2024 •

edited

Loading

fluiddot left a comment •

edited

Loading

fluiddot Jun 13, 2024 •

edited

Loading

fluiddot commented Jun 13, 2024 •

edited

Loading

sejas left a comment •

edited

Loading

fluiddot Jun 18, 2024 •

edited

Loading