From 99fa2d05c3e031b2d76bc534bff4b6192ec32a2b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 25 Sep 2026 12:43:23 +0100 Subject: [PATCH 1/3] fix(environment:delete): handle a single wildcard or missing environment The project selector was given the original input instead of the copy with the environment argument and option cleared, so a single environment argument (or -e) was resolved as an exact ID and failed with "Specified environment not found" before wildcards were applied. Exact IDs that don't match any environment were also dropped by Wildcard::select(), so the command's own "not found" refresh and error never ran. Keep exact IDs so they are reported. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_delete_test.go | 82 +++++++++++++++++++ .../Environment/EnvironmentDeleteCommand.php | 6 +- 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 integration-tests/environment_delete_test.go diff --git a/integration-tests/environment_delete_test.go b/integration-tests/environment_delete_test.go new file mode 100644 index 00000000..1277c083 --- /dev/null +++ b/integration-tests/environment_delete_test.go @@ -0,0 +1,82 @@ +package tests + +import ( + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/pkg/mockapi" +) + +func TestEnvironmentDelete(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + projectID := mockapi.ProjectID() + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + apiHandler.SetEnvironments([]*mockapi.Environment{ + makeEnv(projectID, "main", "production", "active", nil), + makeEnv(projectID, "test-1", "development", "active", "main"), + makeEnv(projectID, "test-2", "development", "active", "main"), + makeEnv(projectID, "dev", "development", "active", "main"), + }) + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + f.Run("cc") + + cases := []struct { + name string + args []string + wantErr bool + wantStdErr []string + wantMissing []string + }{ + { + name: "single wildcard argument", + args: []string{"test-*"}, + wantStdErr: []string{"2 environments found by ID.", "Selected environments: test-1, test-2"}, + wantMissing: []string{"Specified environment not found"}, + }, + { + name: "wildcard option", + args: []string{"-e", "test-*"}, + wantStdErr: []string{"2 environments found by ID.", "Selected environments: test-1, test-2"}, + wantMissing: []string{"Specified environment not found"}, + }, + { + name: "missing environment", + args: []string{"missing"}, + wantErr: true, + wantStdErr: []string{"Environment not found: missing", "0 environments found by ID."}, + wantMissing: []string{"Specified environment not found"}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + args := append([]string{"environment:delete", "-p", projectID}, c.args...) + // Decline any confirmation, so nothing is deleted. + _, stdErr, err := f.RunInteractive("n\nn\n", args...) + if c.wantErr { + assert.Error(t, err) + } + for _, s := range c.wantStdErr { + assert.Contains(t, stdErr, s) + } + for _, s := range c.wantMissing { + assert.NotContains(t, stdErr, s) + } + }) + } +} diff --git a/legacy/src/Command/Environment/EnvironmentDeleteCommand.php b/legacy/src/Command/Environment/EnvironmentDeleteCommand.php index d78aabde..8258f65b 100644 --- a/legacy/src/Command/Environment/EnvironmentDeleteCommand.php +++ b/legacy/src/Command/Environment/EnvironmentDeleteCommand.php @@ -82,7 +82,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $inputCopy = clone $input; $inputCopy->setArgument('environment', null); $inputCopy->setOption('environment', null); - $selection = $this->selector->getSelection($input, new SelectorConfig(envRequired: false)); + $selection = $this->selector->getSelection($inputCopy, new SelectorConfig(envRequired: false)); $environments = $this->api->getEnvironments($selection->getProject()); @@ -103,7 +103,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($specifiedEnvironmentIds) { $anythingSpecified = true; $allIds = \array_map(fn(Environment $e) => $e->id, $environments); - $specifiedEnvironmentIds = Wildcard::select($allIds, $specifiedEnvironmentIds); + // Keep exact IDs even if they don't match, so they can be reported as not found. + $exactIds = array_filter($specifiedEnvironmentIds, fn(string $id): bool => !str_contains($id, '%') && !str_contains($id, '*')); + $specifiedEnvironmentIds = array_values(array_unique(array_merge(Wildcard::select($allIds, $specifiedEnvironmentIds), $exactIds))); $notFound = array_diff($specifiedEnvironmentIds, array_keys($environments)); if (!empty($notFound)) { // Refresh the environments list if any environment is not found. From 4dc71f1e74448fd4f77041846e97bb7bf12bf81a Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 25 Sep 2026 12:48:09 +0100 Subject: [PATCH 2/3] fix(environment:delete): select only the project With the environment argument and option cleared, the selector still fell back to the BRANCH environment variable and failed if it named an unknown environment, blocking explicit deletions. Add a selectEnv option to SelectorConfig to skip environment selection, and use it instead of clearing the input. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_delete_test.go | 9 +++++++++ .../src/Command/Environment/EnvironmentDeleteCommand.php | 8 ++------ legacy/src/Selector/Selector.php | 4 +++- legacy/src/Selector/SelectorConfig.php | 2 ++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/integration-tests/environment_delete_test.go b/integration-tests/environment_delete_test.go index 1277c083..6126fc21 100644 --- a/integration-tests/environment_delete_test.go +++ b/integration-tests/environment_delete_test.go @@ -39,6 +39,7 @@ func TestEnvironmentDelete(t *testing.T) { cases := []struct { name string args []string + extraEnv []string wantErr bool wantStdErr []string wantMissing []string @@ -55,6 +56,13 @@ func TestEnvironmentDelete(t *testing.T) { wantStdErr: []string{"2 environments found by ID.", "Selected environments: test-1, test-2"}, wantMissing: []string{"Specified environment not found"}, }, + { + name: "ignores an unknown branch variable", + args: []string{"test-1"}, + extraEnv: []string{"PLATFORM_BRANCH=missing"}, + wantStdErr: []string{"1 environment found by ID.", "Selected environment: test-1"}, + wantMissing: []string{"Specified environment not found"}, + }, { name: "missing environment", args: []string{"missing"}, @@ -65,6 +73,7 @@ func TestEnvironmentDelete(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { + f.extraEnv = c.extraEnv args := append([]string{"environment:delete", "-p", projectID}, c.args...) // Decline any confirmation, so nothing is deleted. _, stdErr, err := f.RunInteractive("n\nn\n", args...) diff --git a/legacy/src/Command/Environment/EnvironmentDeleteCommand.php b/legacy/src/Command/Environment/EnvironmentDeleteCommand.php index 8258f65b..76e168d8 100644 --- a/legacy/src/Command/Environment/EnvironmentDeleteCommand.php +++ b/legacy/src/Command/Environment/EnvironmentDeleteCommand.php @@ -77,12 +77,8 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { - // Select the current project, deliberately ignoring the 'environment' - // argument and option, as those will be processed separately. - $inputCopy = clone $input; - $inputCopy->setArgument('environment', null); - $inputCopy->setOption('environment', null); - $selection = $this->selector->getSelection($inputCopy, new SelectorConfig(envRequired: false)); + // Select only the project: the 'environment' argument and option are processed separately. + $selection = $this->selector->getSelection($input, new SelectorConfig(selectEnv: false)); $environments = $this->api->getEnvironments($selection->getProject()); diff --git a/legacy/src/Selector/Selector.php b/legacy/src/Selector/Selector.php index 76179a63..9950fe27 100644 --- a/legacy/src/Selector/Selector.php +++ b/legacy/src/Selector/Selector.php @@ -144,7 +144,9 @@ public function getSelection(InputInterface $input, ?SelectorConfig $config = nu $environment = null; $envArgName = $config->envArgName; - if ($input->hasArgument($envArgName) + if (!$config->selectEnv) { + $this->debug('Skipping environment selection'); + } elseif ($input->hasArgument($envArgName) && $input->getArgument($envArgName) !== null && $input->getArgument($envArgName) !== []) { if ($input->hasOption($envOptionName) && Option::stringOrNull($input, $envOptionName)) { diff --git a/legacy/src/Selector/SelectorConfig.php b/legacy/src/Selector/SelectorConfig.php index 378aebe1..80373739 100644 --- a/legacy/src/Selector/SelectorConfig.php +++ b/legacy/src/Selector/SelectorConfig.php @@ -10,6 +10,8 @@ class SelectorConfig { public function __construct( public bool $envRequired = true, + // Set to false to select only the project, e.g. when the command handles environments itself. + public bool $selectEnv = true, public string $envArgName = 'environment', public string $chooseProjectText = 'Enter a number to choose a project:', public string $chooseEnvText = 'Enter a number to choose an environment:', From 12aac2627329b0048e01fd74b64cd9eec250a671 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 25 Sep 2026 13:22:18 +0100 Subject: [PATCH 3/3] test(environment:delete): cover an unknown branch variable with only flags The previous case passed an environment argument, so the selector never read BRANCH and the case passed without the fix. Use --type instead, which reached the fallback before selectEnv was added. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_delete_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/environment_delete_test.go b/integration-tests/environment_delete_test.go index 6126fc21..a6530106 100644 --- a/integration-tests/environment_delete_test.go +++ b/integration-tests/environment_delete_test.go @@ -58,9 +58,9 @@ func TestEnvironmentDelete(t *testing.T) { }, { name: "ignores an unknown branch variable", - args: []string{"test-1"}, + args: []string{"--type", "development"}, extraEnv: []string{"PLATFORM_BRANCH=missing"}, - wantStdErr: []string{"1 environment found by ID.", "Selected environment: test-1"}, + wantStdErr: []string{"3 environments found matching type(s): development"}, wantMissing: []string{"Specified environment not found"}, }, {