From 722028e1c3a9978cc7eb4016cd9fa24aad1a5996 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 23:24:15 +0100 Subject: [PATCH 1/3] fix(legacy): handle a null exit code in Shell::runProcess() If a process fails to start, Symfony throws ProcessStartFailedException (a ProcessFailedException subclass) and getExitCode() returns null. With $mustRun disabled, runProcess() returned that null from an int method, causing a TypeError. This happened in Config::getVersion(), which runs `git describe` with CLI_ROOT as the working directory. Inside a phar, CLI_ROOT is a phar:// path: it passes is_dir() but proc_open() cannot use it. - Return 1 when the exit code is null. - Skip the git version lookup when CLI_ROOT is a phar:// path. - Remove the now-fixed PHPStan baseline entry. Co-Authored-By: Claude Opus 5.5 --- legacy/phpstan-baseline.neon | 6 ------ legacy/src/Service/Config.php | 2 +- legacy/src/Service/Shell.php | 3 ++- legacy/tests/Service/ShellServiceTest.php | 20 ++++++++++++++++++++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/legacy/phpstan-baseline.neon b/legacy/phpstan-baseline.neon index 246d95cc6..46e66db0c 100644 --- a/legacy/phpstan-baseline.neon +++ b/legacy/phpstan-baseline.neon @@ -7122,12 +7122,6 @@ parameters: count: 1 path: src/Service/Shell.php - - - message: '#^Method Platformsh\\Cli\\Service\\Shell\:\:runProcess\(\) should return int but returns int\|null\.$#' - identifier: return.type - count: 1 - path: src/Service/Shell.php - - message: '#^Parameter \#1 \$messages of method Symfony\\Component\\Console\\Output\\OutputInterface\:\:write\(\) expects iterable\|string, string\|null given\.$#' identifier: argument.type diff --git a/legacy/src/Service/Config.php b/legacy/src/Service/Config.php index fb33936ba..38b4ed046 100644 --- a/legacy/src/Service/Config.php +++ b/legacy/src/Service/Config.php @@ -547,7 +547,7 @@ public function getVersion(): string return $this->version; } $version = $this->getWithDefault('application.version', '@version-placeholder@'); - if (str_starts_with((string) $version, '@') && str_ends_with((string) $version, '@')) { + if (str_starts_with((string) $version, '@') && str_ends_with((string) $version, '@') && !str_starts_with(CLI_ROOT, 'phar://')) { // Silently try getting the version from Git. $tag = (new Shell())->execute(['git', 'describe', '--tags'], CLI_ROOT); if (is_string($tag) && str_starts_with($tag, 'v')) { diff --git a/legacy/src/Service/Shell.php b/legacy/src/Service/Shell.php index 69ad5c87c..cd28aada1 100644 --- a/legacy/src/Service/Shell.php +++ b/legacy/src/Service/Shell.php @@ -215,7 +215,8 @@ private function runProcess(Process $process, bool $mustRun = false, bool $quiet }); } catch (ProcessFailedException) { if (!$mustRun) { - return $process->getExitCode(); + // The exit code is null if the process failed to start. + return $process->getExitCode() ?? 1; } // The default for Symfony's ProcessFailedException is to print the // entire STDOUT and STDERR. But if $quiet is disabled, then the user diff --git a/legacy/tests/Service/ShellServiceTest.php b/legacy/tests/Service/ShellServiceTest.php index 5fe54cff5..37ac13dbe 100644 --- a/legacy/tests/Service/ShellServiceTest.php +++ b/legacy/tests/Service/ShellServiceTest.php @@ -7,9 +7,12 @@ use PHPUnit\Framework\TestCase; use Platformsh\Cli\Exception\ProcessFailedException; use Platformsh\Cli\Service\Shell; +use Platformsh\Cli\Tests\HasTempDirTrait; class ShellServiceTest extends TestCase { + use HasTempDirTrait; + /** * Test Shell::execute(). */ @@ -47,4 +50,21 @@ public function testMustExecute(): void $this->expectException(ProcessFailedException::class); $shell->mustExecute(['which', 'nonexistent']); } + + /** + * Test Shell::execute() when the process cannot start, so has no exit code. + */ + public function testExecuteStartFailure(): void + { + $this->tempDirSetUp(); + assert($this->tempDir !== null); + + // A phar:// directory passes is_dir() but cannot be used by proc_open(). + $archive = new \PharData($this->tempDir . '/test.tar'); + $archive->addFromString('dir/file.txt', 'test'); + $dir = 'phar://' . $this->tempDir . '/test.tar/dir'; + + $shell = new Shell(); + $this->assertFalse($shell->execute(['pwd'], $dir)); + } } From 745b6f4e93c98d399d1b49fb260f8fb5c9fc174e Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 23:31:58 +0100 Subject: [PATCH 2/3] fix(legacy): rethrow start failures in Shell::runProcess() with mustRun When a process failed to start and $mustRun was true, runProcess() wrapped it in the CLI's ProcessFailedException, whose constructor reads the process output. For a process that never started, that throws a LogicException and loses the original error. Rethrow Symfony's exception instead. Co-Authored-By: Claude Opus 5.5 --- legacy/src/Service/Shell.php | 6 +++++- legacy/tests/Service/ShellServiceTest.php | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/legacy/src/Service/Shell.php b/legacy/src/Service/Shell.php index cd28aada1..93d6d4ead 100644 --- a/legacy/src/Service/Shell.php +++ b/legacy/src/Service/Shell.php @@ -213,11 +213,15 @@ private function runProcess(Process $process, bool $mustRun = false, bool $quiet $output->write(preg_replace('/(^|[\n\r]+)(.)/', '$1 $2', $buffer)); } }); - } catch (ProcessFailedException) { + } catch (ProcessFailedException $e) { if (!$mustRun) { // The exit code is null if the process failed to start. return $process->getExitCode() ?? 1; } + // A process that failed to start has no output to report. + if (!$process->isStarted()) { + throw $e; + } // The default for Symfony's ProcessFailedException is to print the // entire STDOUT and STDERR. But if $quiet is disabled, then the user // will have already seen the command's output. So we need to diff --git a/legacy/tests/Service/ShellServiceTest.php b/legacy/tests/Service/ShellServiceTest.php index 37ac13dbe..7b8432cac 100644 --- a/legacy/tests/Service/ShellServiceTest.php +++ b/legacy/tests/Service/ShellServiceTest.php @@ -8,6 +8,7 @@ use Platformsh\Cli\Exception\ProcessFailedException; use Platformsh\Cli\Service\Shell; use Platformsh\Cli\Tests\HasTempDirTrait; +use Symfony\Component\Process\Exception\ProcessStartFailedException; class ShellServiceTest extends TestCase { @@ -66,5 +67,7 @@ public function testExecuteStartFailure(): void $shell = new Shell(); $this->assertFalse($shell->execute(['pwd'], $dir)); + $this->expectException(ProcessStartFailedException::class); + $shell->mustExecute(['pwd'], $dir); } } From 65bbefeaffaa59de0cb99eb221a39dbfec624724 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 23:36:07 +0100 Subject: [PATCH 3/3] fix(legacy): treat start failures as not "command does not exist" Shell::exceptionMeansCommandDoesNotExist() read the process's error output on Windows, which throws a LogicException for a process that never started. Return false in that case: a start failure (e.g. an unusable working directory) does not show that the command is missing. Co-Authored-By: Claude Opus 5.5 --- legacy/src/Service/Shell.php | 4 ++++ legacy/tests/Service/ShellServiceTest.php | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/legacy/src/Service/Shell.php b/legacy/src/Service/Shell.php index 93d6d4ead..dc9d0d3d9 100644 --- a/legacy/src/Service/Shell.php +++ b/legacy/src/Service/Shell.php @@ -283,6 +283,10 @@ protected function findWhere(string $command, bool $noticeOnError = true): strin public function exceptionMeansCommandDoesNotExist(ProcessFailedException $e): bool { $process = $e->getProcess(); + // A process that failed to start has no exit code or output to check. + if (!$process->isStarted()) { + return false; + } if ($process->getExitCode() === 127) { return true; } diff --git a/legacy/tests/Service/ShellServiceTest.php b/legacy/tests/Service/ShellServiceTest.php index 7b8432cac..c33e38827 100644 --- a/legacy/tests/Service/ShellServiceTest.php +++ b/legacy/tests/Service/ShellServiceTest.php @@ -67,7 +67,11 @@ public function testExecuteStartFailure(): void $shell = new Shell(); $this->assertFalse($shell->execute(['pwd'], $dir)); - $this->expectException(ProcessStartFailedException::class); - $shell->mustExecute(['pwd'], $dir); + try { + $shell->mustExecute(['pwd'], $dir); + $this->fail('Expected a ProcessStartFailedException'); + } catch (ProcessStartFailedException $e) { + $this->assertFalse($shell->exceptionMeansCommandDoesNotExist($e)); + } } }