Autocomplete consistency for app/job/secret/valkey/opensearch commands#668
Draft
rbjornstad wants to merge 10 commits intomainfrom
Draft
Autocomplete consistency for app/job/secret/valkey/opensearch commands#668rbjornstad wants to merge 10 commits intomainfrom
rbjornstad wants to merge 10 commits intomainfrom
Conversation
Contributor
📝 Changelog previewBelow is a preview of the Changelog that will be added to the next release. Only commit messages that follow the Conventional Commits specification will be included in the Changelog. v5.14.0 - 2026-03-13Full Changelog: v5.13.1...v5.14.0 🚀 Features
🐛 Bug Fixes
⚙️ Miscellaneous Changes |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and standardizes CLI auto-completion for several resource commands (Valkey, OpenSearch, Secrets, Apps, Jobs), and adds more explicit handling of environment flag usage—particularly for secrets where the environment can be inferred at runtime.
Changes:
- Extracted shared auto-completion helpers for Valkey and OpenSearch instance names (dedupe + sort + optional environment filtering).
- Reworked secret name auto-completion to support environment filtering, and added secret environment resolution logic for
secrets get. - Added CLI-arg parsing helpers to infer
--environmentvalues for auto-completion in some commands, plus added “missing team” feedback in list commands.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/valkey/command/update.go | Uses shared Valkey name auto-completion helper. |
| internal/valkey/command/get.go | Uses shared Valkey name auto-completion helper. |
| internal/valkey/command/delete.go | Uses shared Valkey name auto-completion helper. |
| internal/valkey/command/command.go | Adds autoCompleteValkeyNames (dedupe/sort/filter). |
| internal/opensearch/command/update.go | Uses shared OpenSearch name auto-completion helper. |
| internal/opensearch/command/get.go | Uses shared OpenSearch name auto-completion helper. |
| internal/opensearch/command/delete.go | Uses shared OpenSearch name auto-completion helper. |
| internal/opensearch/command/command.go | Adds autoCompleteOpenSearchNames + CLI arg environment parsing helper. |
| internal/secret/command/command.go | Refactors secret name auto-completion with environment filtering and sorting. |
| internal/secret/command/environment.go | Adds secret environment resolution + environment flag counting/parsing helpers. |
| internal/secret/command/get.go | Updates validation flow and runtime environment resolution for secrets get; refactors run logic into helper. |
| internal/secret/command/set.go | Adds single-environment-flag usage validation; updates secret-name auto-completion call. |
| internal/secret/command/unset.go | Adds single-environment-flag usage validation; updates secret-name auto-completion call. |
| internal/secret/command/delete.go | Adds single-environment-flag usage validation; updates secret-name auto-completion call. |
| internal/secret/command/activity.go | Uses parsed CLI environments for secret-name auto-completion when flags aren’t populated. |
| internal/app/command/list.go | Adds AutoCompleteFunc to surface missing-team guidance. |
| internal/app/command/activity.go | Adds CLI parsing fallback to require environment(s) for activity name completion. |
| internal/job/command/list.go | Adds AutoCompleteFunc to surface missing-team guidance. |
| internal/job/command/activity.go | Adds CLI parsing fallback to require environment(s) for activity name completion. |
Comments suppressed due to low confidence (1)
internal/secret/command/get.go:45
secrets getno longer validates the value of--environment. If the user provides an invalid environment string,resolveSecretEnvironmentreturns it as-is and the command proceeds to call the API, which can lead to confusing downstream errors. Consider reintroducingvalidation.CheckEnvironment(string(f.Environment))when the flag is set, or validatingprovidedinsideresolveSecretEnvironmentbefore returning it.
ValidateFunc: func(_ context.Context, args *naistrix.Arguments) error {
if err := validateSingleEnvironmentFlagUsage(); err != nil {
return err
}
if err := validateArgs(args); err != nil {
return err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
This reverts commit b8a6bbe.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors several CLI commands to centralize and improve shell auto-completion behavior (especially around filtering by environment), and enhances secret handling by resolving the secret environment when not explicitly provided.
Changes:
- Centralize name auto-completion for Valkey and OpenSearch instances (dedupe + sort + optional environment filtering).
- Improve secret commands by adding environment resolution logic and stricter detection of multiple
--environment/-eflag usages. - Introduce a small
internal/cliflagshelper for extracting/counting flag values directly from raw CLI args (used primarily in completion paths).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/valkey/command/update.go | Switch Valkey name completion to shared helper. |
| internal/valkey/command/get.go | Switch Valkey name completion to shared helper. |
| internal/valkey/command/delete.go | Switch Valkey name completion to shared helper. |
| internal/valkey/command/command.go | Add autoCompleteValkeyNames helper (dedupe/sort/filter by environment). |
| internal/secret/command/unset.go | Enforce single environment flag usage; update completion call signature. |
| internal/secret/command/set.go | Enforce single environment flag usage; update completion call signature. |
| internal/secret/command/get.go | Resolve environment dynamically; refactor run logic into runGetCommand. |
| internal/secret/command/environment.go | New helpers for resolving secret environment and inspecting raw --environment/-e usage. |
| internal/secret/command/delete.go | Enforce single environment flag usage; update completion call signature. |
| internal/secret/command/command.go | Refactor/extend secret name completion with environment filtering + sorting. |
| internal/secret/command/activity.go | Allow completion across multiple environments (including from raw CLI args). |
| internal/opensearch/command/update.go | Switch OpenSearch name completion to shared helper. |
| internal/opensearch/command/get.go | Switch OpenSearch name completion to shared helper. |
| internal/opensearch/command/delete.go | Switch OpenSearch name completion to shared helper. |
| internal/opensearch/command/command.go | Add autoCompleteOpenSearchNames helper + raw env parsing fallback. |
| internal/job/command/list.go | Add completion-time hint when team is missing. |
| internal/job/command/activity.go | Completion now derives envs from raw args when needed. |
| internal/cliflags/flags.go | New utilities for parsing unique flag values / counting occurrences from args. |
| internal/app/command/list.go | Add completion-time hint when team is missing. |
| internal/app/command/activity.go | Completion now derives envs from raw args when needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| func validateSingleEnvironmentFlagUsage() error { | ||
| if countEnvironmentFlagsInCLIArgs() > 1 { | ||
| return fmt.Errorf("exactly one environment must be specified") |
| AutoCompleteFunc: func(ctx context.Context, args *naistrix.Arguments, _ string) ([]string, string) { | ||
| if args.Len() == 0 { | ||
| return autoCompleteSecretNames(ctx, parentFlags) | ||
| return autoCompleteSecretNames(ctx, f.Team, string(f.Environment), true) |
Comment on lines
+101
to
+103
| result, _ := pterm.DefaultInteractiveTextInput. | ||
| WithDefaultText("Reason for accessing secret values (min 10 chars)"). | ||
| Show() |
Comment on lines
+15
to
+42
| func resolveSecretEnvironment(ctx context.Context, team, name, provided string) (string, error) { | ||
| envs, err := secret.SecretEnvironments(ctx, team, name) | ||
| if err != nil { | ||
| return "", fmt.Errorf("fetching environments for secret %q: %w", name, err) | ||
| } | ||
|
|
||
| if provided != "" { | ||
| if slices.Contains(envs, provided) { | ||
| return provided, nil | ||
| } | ||
|
|
||
| if len(envs) == 0 { | ||
| return "", fmt.Errorf("secret %q not found in team %q", name, team) | ||
| } | ||
|
|
||
| sort.Strings(envs) | ||
| return "", fmt.Errorf("secret %q does not exist in environment %q; available environments: %s", name, provided, strings.Join(envs, ", ")) | ||
| } | ||
|
|
||
| switch len(envs) { | ||
| case 0: | ||
| return "", fmt.Errorf("secret %q not found in team %q", name, team) | ||
| case 1: | ||
| return envs[0], nil | ||
| default: | ||
| sort.Strings(envs) | ||
| return "", fmt.Errorf("secret %q exists in multiple environments (%s); specify --environment/-e", name, strings.Join(envs, ", ")) | ||
| } |
Comment on lines
+5
to
+26
| // UniqueFlagValues returns unique values for a short/long CLI flag from args. | ||
| // It supports forms: -e value, --environment value, -e=value, --environment=value. | ||
| func UniqueFlagValues(args []string, shortFlag, longFlag string) []string { | ||
| seen := map[string]struct{}{} | ||
| values := make([]string, 0) | ||
|
|
||
| for i := 0; i < len(args); i++ { | ||
| arg := args[i] | ||
| switch { | ||
| case arg == shortFlag || arg == longFlag: | ||
| if i+1 >= len(args) { | ||
| continue | ||
| } | ||
| next := args[i+1] | ||
| if strings.HasPrefix(next, "-") || next == "" { | ||
| continue | ||
| } | ||
| if _, ok := seen[next]; !ok { | ||
| seen[next] = struct{}{} | ||
| values = append(values, next) | ||
| } | ||
| i++ |
Comment on lines
+63
to
+65
| if len(environments) == 0 { | ||
| return nil, "Please provide environment to auto-complete application names. '--environment <environment>' flag." | ||
| } |
Comment on lines
+53
to
+55
| if len(environments) == 0 { | ||
| return nil, "Please provide environment to auto-complete job names. '--environment <environment>' flag." | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves autocomplete consistency and environment handling across app, job, secret, valkey, and opensearch commands.
Main goals:
Changes
Secrets
secrets getsecrets deletesecrets setsecrets unsetsecrets activitywhen multiple-eare providedApp / Job
app activityandjob activityautocomplete to better respect environment in completion context.job list.app listautocomplete UX:Valkey (alpha)
alpha valkey getalpha valkey deletealpha valkey updateOpenSearch (alpha)
alpha opensearch getalpha opensearch deletealpha opensearch updateWhy
Autocomplete behavior was inconsistent across command groups and could:
-e/--environmentin completion contextThis PR makes behavior consistent and safer for environment-scoped commands.
Validation
go test ./internal/secret/command ./internal/app/command ./internal/job/command ./internal/opensearch/commandgo build ./...go build -o nais . && source <(./nais completion zsh)Manual smoke checks:
app list,app activityjob list,job activitysecrets get,secrets activityalpha valkey getalpha opensearch getNotes
valkeyandopensearchare currently underalpha.