Skip to content

fix(environment:delete): handle a single wildcard or missing environment - #179

Merged
pjcdawkins merged 3 commits into
mainfrom
cli-183-environment-delete-wildcard
Sep 25, 2026
Merged

pjcdawkins merged 3 commits into
mainfrom
cli-183-environment-delete-wildcard

Conversation

@pjcdawkins

Copy link
Copy Markdown
Contributor

environment:delete failed with "Specified environment not found" when given a single wildcard ('test-*' or -e 'test-*'), because the project selector received the original input and tried to resolve the argument as an exact environment ID.

Changes:

  • Add a selectEnv option to SelectorConfig to select only the project, and use it in environment:delete. This also stops an unknown <prefix>BRANCH value from blocking explicit deletions.
  • Keep exact IDs that match no environment, so the command's own "Environment not found" refresh and error run (previously they were silently dropped by Wildcard::select()).
  • Add integration tests for these cases.

🤖 Generated with Claude Code

pjcdawkins and others added 2 commits September 25, 2026 12:43
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 <noreply@anthropic.com>
With the environment argument and option cleared, the selector still
fell back to the <prefix>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 <noreply@anthropic.com>

@upsun-dispatch upsun-dispatch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔍 Full review · 4 files reviewed

🔵 Minor point

  • integration-tests/environment_delete_test.go:61 — The test case "ignores an unknown branch variable" passes an environment argument (test-1) along with PLATFORM_BRANCH=missing. It would have passed before this fix too. When the environment argument is not empty, the old Selector::getSelection() took the argument branch and called selectEnvironment($input, $project, $config, 'test-1'). That call gets a non-null $environmentId, so selectEnvironment never reads &lt;prefix>BRANCH; the variable is only read when $environmentId === null. &lt;prefix>BRANCH blocked deletion only when no argument and no -e were given, for example environment:delete --inactive or --merged or --type .... In that case the selector's elseif ($input->hasOption('environment')) branch passed null, fell back to PLATFORM_BRANCH, and threw "Specified environment not found". Nothing covers that case, so a regression in the selectEnv: false path for flag-only deletions would go unnoticed.
Verification
  • All ~50 new SelectorConfig(...) call sites in legacy/src pass named arguments, so adding selectEnv as the second constructor parameter moves no one's positional arguments.
  • envRequired is read only inside selectEnvironment, which selectEnv: false now skips, so dropping envRequired: false from the delete command changes nothing.
  • The delete command never used $selection->getEnvironment(); its no-argument fallback calls getCurrentEnvironment(), which is Git-based. Skipping environment selection loses no behaviour.
  • The exact-ID merge keeps wildcard matches first, then adds exact IDs that match nothing, so they reach the refresh and the "Environment not found" error; array_unique removes the duplicates.

The PR adds integration-tests/environment_delete_test.go with four cases (wildcard argument, wildcard -e option, unknown PLATFORM_BRANCH, missing environment), run against the mock API. These tests need a built binary under dist/ and skip themselves if there isn't one. The PLATFORM_BRANCH case doesn't exercise the path that was actually broken, and nothing tests flag-only deletions such as --inactive with a branch variable set.

Review details
  • Commit: 4dc71f1
  • Model: claude-opus-5-5

Review 1 of 10 for this pull request · View the full run

…flags

The previous case passed an environment argument, so the selector never
read <prefix>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 <noreply@anthropic.com>
@pjcdawkins
pjcdawkins merged commit 185da06 into main Sep 25, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant