From 04b2af25ca30cef6cae7000fec0af099020523eb Mon Sep 17 00:00:00 2001 From: Nick Vigilante Date: Wed, 30 Sep 2026 14:18:26 -0400 Subject: [PATCH] fix(cli): offer a None option in the coder create preset prompt (#30068) When a template has presets but no default preset, `coder create` prompts for a preset, but the prompt listed only the template's presets. The only way to skip a preset was `--preset none`, while the dashboard lists **None** first. This adds a **None** entry at the top of the prompt. Selecting it behaves like `--preset none`: the CLI sends no preset ID and prints `No preset applied.` If a preset's label is also `None`, the no-preset entry is labeled `None (no preset)` so both stay selectable. Default preset auto-selection is unchanged. > [!NOTE] > "No preset ID sent" doesn't always mean the build records no preset. > When a create request has no preset ID, coderd matches the parameter values against the template's presets (`prebuilds.FindMatchingPresetID`) and records any exact match, as `--preset none` and the dashboard's None option already do. Fixes ENG-3407
Validation and decision log ### Validation Tested at e5f52ffe4b against Postgres. - `TestCreate`, `TestCreateWithPreset`, and the new `TestPresetSelectOptions` pass against Postgres 13. - Ran a dev server from this branch (`./scripts/develop.sh --port-offset`) and drove `coder create` through a real PTY against a template with two presets and no default: - The menu lists `None`, `eu-preset: Deploys to EU`, `us-preset`. - `None` prints `No preset applied.`, then prompts for parameters. - Selecting either preset applies its parameters. - `--preset none` and the prompt's `None` produce identical `workspace_builds.template_version_preset_id` values. With `region=us`, both record the matching `us-preset` because of server-side preset matching. - Template with a preset literally named `None`: - The dashboard shows `None` twice. Each entry has its own internal value, so both work, but they look identical. - The first commit on this branch showed `None` twice in the CLI too, and both entries meant "no preset". That made the `None` preset unreachable from the prompt, which is a regression from `main`. - The second commit fixes that: the menu shows `None (no preset)`, `None`, `us-preset`, and selecting `None` applies the preset (`region=eu`). ### Decisions - **Label `None` to match the dashboard.** Switch to `None (no preset)` only when a preset's label collides, so the common case stays identical to the dashboard. - **Out of scope: `--preset None` flag.** The flag matches `none` case-insensitively before any preset name, so a preset named `None` or `none` can't be selected by flag. That's unchanged from `main`. - **Out of scope: default presets.** The dashboard preselects the default preset but still lets you pick another preset or None. The CLI applies the default without prompting. That behavior needs a separate decision.
--- _Generated by Coder Agents on behalf of @nickvigilante._ --- cli/create.go | 40 ++++++++++++++++++---- cli/create_internal_test.go | 67 +++++++++++++++++++++++++++++++++++++ cli/create_test.go | 19 +++++------ 3 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 cli/create_internal_test.go diff --git a/cli/create.go b/cli/create.go index d9abbfc9c7..a4d3e1b388 100644 --- a/cli/create.go +++ b/cli/create.go @@ -302,12 +302,15 @@ func (r *RootCmd) Create(opts CreateOptions) *serpent.Command { if !errors.Is(err, ErrNoPresetFound) { return xerrors.Errorf("unable to resolve preset: %w", err) } - // If no preset found, prompt the user to choose a preset + // If no preset found, prompt the user to choose a preset. + // A nil preset means the user chose "None". if preset, err = promptPresetSelection(inv, tvPresets); err != nil { return xerrors.Errorf("unable to prompt user for preset: %w", err) } } + } + if preset != nil { // Convert preset parameters into workspace build parameters presetParameters = presetParameterAsWorkspaceBuildParameters(preset.Parameters) // Inform the user which preset was applied and its parameters @@ -516,12 +519,21 @@ func resolvePreset(presets []codersdk.Preset, presetName string) (*codersdk.Pres return nil, ErrNoPresetFound } -// promptPresetSelection shows a CLI selection menu of the presets defined in the template version. -// Returns the selected preset -func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (*codersdk.Preset, error) { - presetMap := make(map[string]*codersdk.Preset) - var presetOptions []string +const ( + // presetNoneOption is the selection menu label for creating a workspace + // without a preset, matching the dashboard's "None" option. + presetNoneOption = "None" + // presetNoneFallbackOption replaces presetNoneOption when a preset's + // label is also "None", so the menu never shows two identical entries. + presetNoneFallbackOption = "None (no preset)" +) +// presetSelectOptions builds the preset selection menu entries. The first +// entry is the label for applying no preset, which is also returned as +// noneOption. presetMap maps every other entry to its preset. +func presetSelectOptions(presets []codersdk.Preset) (options []string, noneOption string, presetMap map[string]*codersdk.Preset) { + presetMap = make(map[string]*codersdk.Preset, len(presets)) + presetOptions := make([]string, 0, len(presets)) for _, preset := range presets { var option string if preset.Description == "" { @@ -533,6 +545,19 @@ func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) ( presetMap[option] = &preset } + noneOption = presetNoneOption + if _, ok := presetMap[noneOption]; ok { + noneOption = presetNoneFallbackOption + } + return append([]string{noneOption}, presetOptions...), noneOption, presetMap +} + +// promptPresetSelection shows a CLI selection menu of the presets defined in the template version, +// preceded by an option to apply no preset. +// Returns the selected preset, or nil if the user chose to apply no preset. +func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (*codersdk.Preset, error) { + presetOptions, noneOption, presetMap := presetSelectOptions(presets) + // Show selection UI _, _ = fmt.Fprintln(inv.Stdout, pretty.Sprint(cliui.DefaultStyles.Wrap, "Select a preset below:")) selected, err := cliui.Select(inv, cliui.SelectOptions{ @@ -543,6 +568,9 @@ func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) ( return nil, xerrors.Errorf("failed to select preset: %w", err) } + if selected == noneOption { + return nil, nil //nolint:nilnil // A nil preset means no preset is applied. + } return presetMap[selected], nil } diff --git a/cli/create_internal_test.go b/cli/create_internal_test.go new file mode 100644 index 0000000000..56ff181ed6 --- /dev/null +++ b/cli/create_internal_test.go @@ -0,0 +1,67 @@ +package cli + +import ( + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/codersdk" +) + +func TestPresetSelectOptions(t *testing.T) { + t.Parallel() + + eu := codersdk.Preset{ID: uuid.New(), Name: "eu", Description: "Deploys to EU"} + us := codersdk.Preset{ID: uuid.New(), Name: "us"} + namedNone := codersdk.Preset{ID: uuid.New(), Name: "None"} + namedNoneWithDescription := codersdk.Preset{ID: uuid.New(), Name: "None", Description: "Minimal"} + + tests := []struct { + name string + presets []codersdk.Preset + wantOptions []string + wantNone string + wantPresets map[string]uuid.UUID + }{ + { + name: "NoneFirst", + presets: []codersdk.Preset{eu, us}, + wantOptions: []string{"None", "eu: Deploys to EU", "us"}, + wantNone: "None", + wantPresets: map[string]uuid.UUID{"eu: Deploys to EU": eu.ID, "us": us.ID}, + }, + { + // A preset whose label is "None" must stay selectable, so the + // option for applying no preset gets a distinct label. + name: "PresetNamedNone", + presets: []codersdk.Preset{namedNone, us}, + wantOptions: []string{"None (no preset)", "None", "us"}, + wantNone: "None (no preset)", + wantPresets: map[string]uuid.UUID{"None": namedNone.ID, "us": us.ID}, + }, + { + name: "PresetNamedNoneWithDescription", + presets: []codersdk.Preset{namedNoneWithDescription}, + wantOptions: []string{"None", "None: Minimal"}, + wantNone: "None", + wantPresets: map[string]uuid.UUID{"None: Minimal": namedNoneWithDescription.ID}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + options, noneOption, presetMap := presetSelectOptions(tt.presets) + require.Equal(t, tt.wantOptions, options) + require.Equal(t, tt.wantNone, noneOption) + require.NotContains(t, presetMap, noneOption) + require.Len(t, presetMap, len(tt.wantPresets)) + for option, id := range tt.wantPresets { + require.Contains(t, presetMap, option) + require.Equal(t, id, presetMap[option].ID) + } + }) + } +} diff --git a/cli/create_test.go b/cli/create_test.go index f1622b6cfa..c1165f49a2 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -1395,7 +1395,8 @@ func TestCreateWithPreset(t *testing.T) { // This test verifies that when a template has presets but no default preset, // and the user does not provide the `--preset` flag, - // the CLI prompts the user to select a preset. + // the CLI prompts the user to select a preset, offering "None" first. + // Selecting "None" behaves like `--preset none`. t.Run("NoDefaultPresetPromptUser", func(t *testing.T) { t.Parallel() logger := testutil.Logger(t) @@ -1436,32 +1437,28 @@ func TestCreateWithPreset(t *testing.T) { // Should: prompt the user for the preset stdout.ExpectMatch(ctx, "Select a preset below:") // We don't actually have to respond to the selector, since we hardcode the cliui.Select to return the - // first option in test scenarios (c.f. cliui/select.go) - stdout.ExpectMatch(ctx, "Preset 'preset-test' applied") + // first option in test scenarios (c.f. cliui/select.go). The first option is "None". + stdout.ExpectMatch(ctx, "No preset applied.") stdout.ExpectMatch(ctx, "Confirm create?") stdin.WriteLine("yes") <-doneChan - // Verify if the new workspace uses expected parameters. - tvPresets, err := client.TemplateVersionPresets(ctx, version.ID) - require.NoError(t, err) - require.Len(t, tvPresets, 1) - workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ Name: workspaceName, }) require.NoError(t, err) require.Len(t, workspaces.Workspaces, 1) - // Should: create a workspace using the expected template version and the preset-defined parameters + // Should: create a workspace using the expected template version, no preset, + // and the parameters from the command line flags workspaceLatestBuild := workspaces.Workspaces[0].LatestBuild require.Equal(t, version.ID, workspaceLatestBuild.TemplateVersionID) - require.Equal(t, tvPresets[0].ID, *workspaceLatestBuild.TemplateVersionPresetID) + require.Nil(t, workspaceLatestBuild.TemplateVersionPresetID) buildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceLatestBuild.ID) require.NoError(t, err) require.Len(t, buildParameters, 2) - require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: firstParameterName, Value: secondOptionalParameterValue}) + require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: firstParameterName, Value: firstOptionalParameterValue}) require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: thirdParameterName, Value: thirdParameterValue}) })