Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 1, 2026

On systems like NixOS, bash is not located at /bin/bash. The wp shell command was hardcoded to this path, causing failures despite the shell being available at $SHELL.

Changes

  • Shell detection priority in REPL.php:

    1. WP_CLI_CUSTOM_SHELL (explicit override)
    2. SHELL (system default) ← new
    3. /bin/bash (fallback)
  • Test coverage: Added scenario verifying SHELL environment variable is respected

// Before
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
    $shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} else {
    $shell_binary = '/bin/bash';
}

// After
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
    $shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} elseif ( getenv( 'SHELL' ) ) {
    $shell_binary = getenv( 'SHELL' );
} else {
    $shell_binary = '/bin/bash';
}

Aligns with how wp --info detects the shell.

Original prompt

This section details on the original issue you should resolve

<issue_title>Shell binary not getting detected correctly?</issue_title>
<issue_description>## Bug Report

The shell binary '/bin/bash' is not valid.

The wp --info command shows the correct Shell, but wp shell does not seem to find it. Since it's not in /bin/bash

[sergio@samara:/var/www/nerdpress]$ wp --info
OS:     Linux 6.6.32 wp-cli/shell-command#1-NixOS SMP PREEMPT_DYNAMIC Sat May 25 14:22:56 UTC 2024 x86_64
Shell:  /run/current-system/sw/bin/bash
PHP binary:     /nix/store/swkmyv7mplz46vlr9jyk7qlp2lxv19z0-php-with-extensions-8.2.19/bin/php
PHP version:    8.2.19
php.ini used:   /nix/store/6g69kphwfkdx4p2qn1fgwm888l15cpz7-wp-cli-2.10.0/etc/php.ini
MySQL binary:   /run/current-system/sw/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.6-MariaDB, for Linux (x86_64) using readline 5.1
SQL modes:      STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /var/www/nerdpress
WP-CLI packages dir:
WP-CLI cache dir:       $XDG_CACHE_HOME/wp-cli
WP-CLI global config:
WP-CLI project config:
WP-CLI version: 2.10.0

[sergio@samara:/var/www/nerdpress]$ wp shell
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.

[sergio@samara:/var/www/nerdpress]$ php -a
Interactive shell

php > var_dump( is_file( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > var_dump( is_readable( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > exit

Describe how other contributors can replicate this bug

  • Have get( 'SHELL' ); return something other than /bin/bash
  • Run wp shell

Provide a possible solution

Seems that the wp cli info command gets the shell binary using:
getenv( 'SHELL' )

Where wp shell uses getenv( 'WP_CLI_CUSTOM_SHELL' ) to check for a "custom shell", then assumes /bin/bash is it isn't set.

Why not use $shell_binary = getenv( 'SHELL' ); instead of $shell_binary = '/bin/bash';

</issue_description>

Comments on the Issue (you are @copilot in this section)

@swissspidy `WP_CLI_CUSTOM_SHELL` is an environment variable accessed via `getenv`. It is not a PHP constant. So you would do this instead:
WP_CLI_CUSTOM_SHELL=/run/current-system/sw/bin/bash wp shell
```</body></comment_new>
<comment_new><author>@swissspidy</author><body>
Good question. Maybe @schlessera or @danielbachhuber have some context about why the current default is a hardcoded `/bin/bash` and not `getenv( 'SHELL' )`. It sounds like a reasonable enhancement at first glance.</body></comment_new>
<comment_new><author>@danielbachhuber</author><body>
I don't recall a specific reason off the top of my head. For a more definitive answer, I'd look at the original pull request(s) to see if there was a discussion around it.</body></comment_new>
</comments>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix shell binary detection for wp shell command Use SHELL environment variable before falling back to /bin/bash Feb 1, 2026
Copilot AI requested a review from swissspidy February 1, 2026 19:35
@codecov
Copy link

codecov bot commented Feb 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@swissspidy swissspidy added this to the 2.0.17 milestone Feb 2, 2026
@swissspidy swissspidy requested a review from Copilot February 2, 2026 02:01
@swissspidy
Copy link
Member

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly implements the fallback to the SHELL environment variable before using the hardcoded /bin/bash path. The changes are logical and the new test case in features/shell.feature provides good coverage for the new behavior. I have one minor suggestion in src/WP_CLI/Shell/REPL.php to make the code more concise and efficient.

Comment on lines 123 to 129
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
$shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} elseif ( getenv( 'SHELL' ) ) {
$shell_binary = getenv( 'SHELL' );
} else {
$shell_binary = '/bin/bash';
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This if/elseif/else block can be simplified and made more efficient by avoiding repeated calls to getenv(). You can use the ternary operator shorthand (?:) to achieve the same logic in a single, more concise line. This avoids calling getenv() multiple times for the same variable.

		$shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' ) ?: getenv( 'SHELL' ) ?: '/bin/bash';

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts wp shell’s shell binary selection to work on systems where bash isn’t located at /bin/bash (e.g. NixOS), by considering environment variables before falling back.

Changes:

  • Update REPL shell selection to prefer WP_CLI_CUSTOM_SHELL, then SHELL, then /bin/bash.
  • Add a Behat scenario intended to verify SHELL is respected.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/WP_CLI/Shell/REPL.php Changes shell binary detection order to include $SHELL before /bin/bash.
features/shell.feature Adds a feature scenario covering $SHELL behavior and invalid $SHELL erroring.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 123 to 128
if ( getenv( 'WP_CLI_CUSTOM_SHELL' ) ) {
$shell_binary = getenv( 'WP_CLI_CUSTOM_SHELL' );
} elseif ( getenv( 'SHELL' ) ) {
$shell_binary = getenv( 'SHELL' );
} else {
$shell_binary = '/bin/bash';
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the SHELL environment variable as the default shell can break wp shell for users whose login shell is not bash (e.g. zsh/fish). create_prompt_cmd() builds a bash-specific command (history -r/-s/-w, read -re), so selecting a non-bash SHELL will pass the is_file/is_readable check but then fail at runtime. Consider preferring a known bash binary first (e.g. /bin/bash when it exists), and only falling back to SHELL when /bin/bash is unavailable or when SHELL points to a bash binary (e.g. basename is bash), or probe for bash on PATH.

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +77
Scenario: Use SHELL environment variable as fallback
Given a WP install

And a session file:
"""
return true;
"""

When I try `SHELL=/bin/bash wp shell --basic < session`
Then STDOUT should contain:
"""
bool(true)
"""
And STDERR should be empty
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scenario doesn't clearly validate the new behavior of preferring $SHELL, because setting SHELL=/bin/bash matches the existing hardcoded fallback and would succeed even without the code change. Consider asserting a bash path that differs from /bin/bash (e.g. via a resolved bash path) or otherwise structuring the test so it fails on the old behavior but passes on the new one.

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +84
Then STDOUT should be empty
And STDERR should contain:
"""
Error: The shell binary '/nonsense/path' is not valid.
"""
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new precedence order, an invalid SHELL now causes wp shell to error even if /bin/bash is present and usable (previous behavior would have ignored SHELL and worked). The second half of this scenario locks in that behavior; please confirm this breaking change is intended. If not intended, adjust the selection logic to fall back to /bin/bash when SHELL is unset/invalid/non-bash, and update the test accordingly.

Suggested change
Then STDOUT should be empty
And STDERR should contain:
"""
Error: The shell binary '/nonsense/path' is not valid.
"""
Then STDOUT should contain:
"""
bool(true)
"""
And STDERR should be empty

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell binary not getting detected correctly?

2 participants