From 213b5159b4cceb567f0eac7ec9cfcfe0f1e90513 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 21 May 2026 00:28:38 +0100 Subject: [PATCH 1/3] fix(legacy): handle null input in interactive prompt validators Several interactive validator closures crashed with a TypeError when the user pressed Enter to accept a null default: - Selector::offerProjectChoice: explode(' - ', null) on the project-by-ID prompt - Selector::offerEnvironmentChoice: closure typed (string $value) with nullable default $defaultEnvironmentId - OrganizationCommandBase user picker: closure typed (string $email) with null default - TeamUserAddCommand: closure typed (string $value) with null default - OrganizationUserAddCommand: optional permissions prompt calling ArrayArgument::split([null]) -> preg_split(..., null) Root cause: QuestionHelper::askInput returns mixed, and Symfony passes null to the validator when the question's default is null. Type each closure parameter as ?string and guard null explicitly. In Selector::offerProjectChoice, also switch to [$id] = explode(' - ', ..., 2) to avoid an "undefined array key 1" warning when a project has no title (autocomplete value has no ' - '). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Command/Organization/OrganizationCommandBase.php | 5 ++++- .../Organization/User/OrganizationUserAddCommand.php | 7 +++++-- legacy/src/Command/Team/User/TeamUserAddCommand.php | 6 +++--- legacy/src/Selector/Selector.php | 10 +++++----- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/legacy/src/Command/Organization/OrganizationCommandBase.php b/legacy/src/Command/Organization/OrganizationCommandBase.php index e07317de8..275eefaf6 100644 --- a/legacy/src/Command/Organization/OrganizationCommandBase.php +++ b/legacy/src/Command/Organization/OrganizationCommandBase.php @@ -117,7 +117,10 @@ protected function chooseMember(Organization $organization): Member } $userId = $this->questionHelper->choose($choices, 'Enter a number to choose a user:', $default); } else { - $userId = $this->questionHelper->askInput('Enter an email address to choose a user', null, array_values($emailAddresses), function (string $email) use ($emailAddresses): string { + $userId = $this->questionHelper->askInput('Enter an email address to choose a user', null, array_values($emailAddresses), function (?string $email) use ($emailAddresses): string { + if ($email === null) { + throw new InvalidArgumentException('An email address is required'); + } if (($key = array_search($email, $emailAddresses)) === false) { throw new InvalidArgumentException('User not found: ' . $email); } diff --git a/legacy/src/Command/Organization/User/OrganizationUserAddCommand.php b/legacy/src/Command/Organization/User/OrganizationUserAddCommand.php index 9098a13e9..262f0a107 100644 --- a/legacy/src/Command/Organization/User/OrganizationUserAddCommand.php +++ b/legacy/src/Command/Organization/User/OrganizationUserAddCommand.php @@ -81,7 +81,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int } else { $questionText = 'Optionally, enter a list of permissions to add (separated by commas)'; } - $response = $this->questionHelper->askInput($questionText, null, [], function ($value) { + $response = $this->questionHelper->askInput($questionText, null, [], function (?string $value): ?string { + if ($value === null) { + return null; + } foreach (ArrayArgument::split([$value]) as $permission) { if (!\in_array($permission, self::$allPermissions)) { throw new InvalidArgumentException('Unrecognized permission: ' . $permission); @@ -89,7 +92,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } return $value; }); - $permissions = ArrayArgument::split([$response]); + $permissions = $response === null ? [] : ArrayArgument::split([$response]); $this->stdErr->writeln(''); } diff --git a/legacy/src/Command/Team/User/TeamUserAddCommand.php b/legacy/src/Command/Team/User/TeamUserAddCommand.php index b205879c3..729c9ffbb 100644 --- a/legacy/src/Command/Team/User/TeamUserAddCommand.php +++ b/legacy/src/Command/Team/User/TeamUserAddCommand.php @@ -56,9 +56,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int $emails[] = $info->email; } } - $identifier = $this->questionHelper->askInput('Enter an email address to add a user', null, $emails, function (string $value): string { - if (!filter_var($value, FILTER_VALIDATE_EMAIL)) { - throw new InvalidArgumentException('Invalid email address:' . $value); + $identifier = $this->questionHelper->askInput('Enter an email address to add a user', null, $emails, function (?string $value): string { + if ($value === null || !filter_var($value, FILTER_VALIDATE_EMAIL)) { + throw new InvalidArgumentException('Invalid email address:' . ($value ?? '')); } return $value; }); diff --git a/legacy/src/Selector/Selector.php b/legacy/src/Selector/Selector.php index 1daa00f82..3ae1f0611 100644 --- a/legacy/src/Selector/Selector.php +++ b/legacy/src/Selector/Selector.php @@ -453,8 +453,8 @@ private function offerProjectChoice(array $projectInfos, SelectorConfig $config) } } asort($autocomplete, SORT_NATURAL | SORT_FLAG_CASE); - return $this->questionHelper->askInput($config->enterProjectText, null, array_values($autocomplete), function ($value) use ($autocomplete): string { - [$id, ] = explode(' - ', $value); + return $this->questionHelper->askInput($config->enterProjectText, null, array_values($autocomplete), function (?string $value) use ($autocomplete): string { + [$id] = explode(' - ', $value ?? '', 2); if (empty(trim($id))) { throw new InvalidArgumentException('A project ID is required'); } @@ -495,9 +495,9 @@ private function offerEnvironmentChoice(InputInterface $input, Project $project, $ids = array_keys($environments); sort($ids, SORT_NATURAL | SORT_FLAG_CASE); - $id = $this->questionHelper->askInput($config->enterEnvText, $defaultEnvironmentId, array_keys($environments), function (string $value) use ($environments): string { - if (!isset($environments[$value])) { - throw new \RuntimeException('Environment not found: ' . $value); + $id = $this->questionHelper->askInput($config->enterEnvText, $defaultEnvironmentId, array_keys($environments), function (?string $value) use ($environments): string { + if ($value === null || !isset($environments[$value])) { + throw new \RuntimeException('Environment not found: ' . ($value ?? '')); } return $value; From 74a13a2961064db1cf1afe7b0bcc6b271501d765 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 21 May 2026 00:40:46 +0100 Subject: [PATCH 2/3] fix(legacy): clearer messages for empty input in interactive validators Distinguish "no input" from "invalid input" in two validators flagged by review on the parent PR: - TeamUserAddCommand: when the user presses Enter (null), report "An email address is required" instead of "Invalid email address:" (the original message also had no space after the colon). - Selector::offerEnvironmentChoice: when the user presses Enter and the default is null, report "An environment ID is required" instead of "Environment not found: " (empty). Co-Authored-By: Claude Opus 4.7 (1M context) --- legacy/src/Command/Team/User/TeamUserAddCommand.php | 7 +++++-- legacy/src/Selector/Selector.php | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/legacy/src/Command/Team/User/TeamUserAddCommand.php b/legacy/src/Command/Team/User/TeamUserAddCommand.php index 729c9ffbb..1c65f54e7 100644 --- a/legacy/src/Command/Team/User/TeamUserAddCommand.php +++ b/legacy/src/Command/Team/User/TeamUserAddCommand.php @@ -57,8 +57,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } $identifier = $this->questionHelper->askInput('Enter an email address to add a user', null, $emails, function (?string $value): string { - if ($value === null || !filter_var($value, FILTER_VALIDATE_EMAIL)) { - throw new InvalidArgumentException('Invalid email address:' . ($value ?? '')); + if ($value === null) { + throw new InvalidArgumentException('An email address is required'); + } + if (!filter_var($value, FILTER_VALIDATE_EMAIL)) { + throw new InvalidArgumentException('Invalid email address: ' . $value); } return $value; }); diff --git a/legacy/src/Selector/Selector.php b/legacy/src/Selector/Selector.php index 3ae1f0611..b8dbe834f 100644 --- a/legacy/src/Selector/Selector.php +++ b/legacy/src/Selector/Selector.php @@ -496,8 +496,11 @@ private function offerEnvironmentChoice(InputInterface $input, Project $project, sort($ids, SORT_NATURAL | SORT_FLAG_CASE); $id = $this->questionHelper->askInput($config->enterEnvText, $defaultEnvironmentId, array_keys($environments), function (?string $value) use ($environments): string { - if ($value === null || !isset($environments[$value])) { - throw new \RuntimeException('Environment not found: ' . ($value ?? '')); + if ($value === null || $value === '') { + throw new \RuntimeException('An environment ID is required'); + } + if (!isset($environments[$value])) { + throw new \RuntimeException('Environment not found: ' . $value); } return $value; From 321c54c7198c7f6f4541b25ae7965c49181b4c36 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 22 May 2026 23:42:56 +0100 Subject: [PATCH 3/3] test(integration): cover accepting defaults at interactive prompts Add a RunInteractive helper that pipes stdin to a command and keeps the legacy CLI in interactive mode by setting SHELL_INTERACTIVE and stripping NO_INTERACTION. Use it in a new project:create test that pipes two newlines to accept the Default branch default ("main") and the cost-confirmation [Y/n] default, then asserts both prompts appeared and the project was created. Co-Authored-By: Claude Opus 4.7 --- .../project_create_interactive_test.go | 44 +++++++++++++++++++ integration-tests/tests.go | 27 ++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 integration-tests/project_create_interactive_test.go diff --git a/integration-tests/project_create_interactive_test.go b/integration-tests/project_create_interactive_test.go new file mode 100644 index 000000000..2b91e859f --- /dev/null +++ b/integration-tests/project_create_interactive_test.go @@ -0,0 +1,44 @@ +package tests + +import ( + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestProjectCreate_AcceptDefaults sends two newlines to accept the +// "Default branch" prompt and the cost-confirmation [Y/n] prompt. +func TestProjectCreate_AcceptDefaults(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + apiHandler.SetOrgs([]*mockapi.Org{ + makeOrg("cli-test-id", "cli-tests", "CLI Test Org", "my-user-id", "flexible"), + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdOut, stdErr, err := f.RunInteractive( + "\n\n", + "project:create", + "--title", "Interactive Defaults Test", + "--region", "test-region", + "--org", "cli-tests", + "--no-set-remote", + ) + require.NoError(t, err, "stdout: %s\nstderr: %s", stdOut, stdErr) + + assert.Contains(t, stdErr, "Default branch") + assert.Contains(t, stdErr, "Are you sure you want to continue?") + assert.Contains(t, stdErr, "[Y/n]") + assert.NotEmpty(t, strings.TrimSpace(stdOut)) +} diff --git a/integration-tests/tests.go b/integration-tests/tests.go index 2bbd4a526..9cbb3bc67 100644 --- a/integration-tests/tests.go +++ b/integration-tests/tests.go @@ -92,6 +92,33 @@ func (f *cmdFactory) RunCombinedOutput(args ...string) (stdOut, stdErr string, e return stdOutBuffer.String(), stdErrBuffer.String(), err } +// RunInteractive runs a command with stdin piped from stdinInput. It keeps +// the legacy CLI in interactive mode under a pipe by setting SHELL_INTERACTIVE +// and stripping NO_INTERACTION. +func (f *cmdFactory) RunInteractive(stdinInput string, args ...string) (stdOut, stdErr string, err error) { + cmd := f.buildCommand(args...) + newEnv := make([]string, 0, len(cmd.Env)+1) + for _, e := range cmd.Env { + if strings.HasPrefix(e, EnvPrefix+"NO_INTERACTION=") { + continue + } + newEnv = append(newEnv, e) + } + newEnv = append(newEnv, "SHELL_INTERACTIVE=1") + cmd.Env = newEnv + cmd.Stdin = strings.NewReader(stdinInput) + var stdOutBuffer bytes.Buffer + var stdErrBuffer bytes.Buffer + cmd.Stdout = &stdOutBuffer + cmd.Stderr = &stdErrBuffer + if testing.Verbose() { + cmd.Stderr = io.MultiWriter(&stdErrBuffer, os.Stderr) + } + f.t.Log("Running (interactive):", cmd) + err = cmd.Run() + return stdOutBuffer.String(), stdErrBuffer.String(), err +} + func (f *cmdFactory) buildCommand(args ...string) *exec.Cmd { cmd := exec.Command(getCommandName(f.t), args...) cmd.Env = testEnv()