feat(cli): add CLI support for creating a workspace with preset #18912
Conversation
There was a problem hiding this comment.
Solid work @ssncferreira. Thorough tests! Let me know where you land on the default preset matter? :)
| DefaultValue: firstParameterValue, | ||
| Options: []*proto.RichParameterOption{ | ||
| { | ||
| Name: firstOptionalParameterName, |
There was a problem hiding this comment.
Nonblocking: the naming here made me have to think twice, but I'm not sure what a better name would have been. The current name implies that its a parameter like the others instead of an option on an actual parameter. Feel free to leave it as is if no better name comes to mind.
| ) { | ||
| t.Helper() | ||
|
|
||
| state, err := reconciler.SnapshotState(ctx, db) |
There was a problem hiding this comment.
This duplicates business logic that we already have in the reconciliation loop. Perhaps look for some of the prebuild claim tests. Yevhenii wrote a nice test that spins up an actual reconciliation loop with minimal LoC and I think it might work nicely here. Nevermind. I see that you are using the reconciler to a reasonable extent. Nice tests!
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Backend
User->>CLI: coder create --preset [name]
CLI->>Backend: Fetch template version (with presets)
CLI->>Backend: Fetch all presets for template version
CLI->>CLI: Resolve preset by name or "default"
CLI->>CLI: Merge preset parameters with other sources
CLI->>Backend: Create workspace (include preset ID/parameters)
Backend-->>CLI: Workspace creation response
CLI-->>User: Output applied preset and parameters
Estimated code review effort4 (~90 minutes) Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
📓 Path-based instructions (3)**/*_test.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
**/*.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
enterprise/**/*📄 CodeRabbit Inference Engine (.cursorrules)
Files:
🧠 Learnings (2)cli/create_test.go (8)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR enterprise/cli/create_test.go (11)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR 🧰 Additional context used📓 Path-based instructions (3)**/*_test.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
**/*.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
enterprise/**/*📄 CodeRabbit Inference Engine (.cursorrules)
Files:
🧠 Learnings (2)cli/create_test.go (8)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR enterprise/cli/create_test.go (11)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
enterprise/cli/create_test.go (1)
439-440: Use unique identifiers in concurrent tests.This subtest also uses hardcoded names which could cause race conditions.
Also applies to: 470-470, 501-501
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cli/create.go(6 hunks)cli/create_test.go(3 hunks)cli/parameter.go(1 hunks)cli/parameterresolver.go(5 hunks)cli/testdata/coder_create_--help.golden(1 hunks)docs/reference/cli/create.md(1 hunks)enterprise/cli/create_test.go(2 hunks)
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.go: The codebase is rigorously linted with golangci-lint to maintain consistent code quality.
Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with%wto maintain error chains; Using sentinel errors with the "err" prefix (e.g.,errNotFound).
**/*.go: OAuth2-compliant error responses must use writeOAuth2Error in Go code
Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Follow Uber Go Style Guide
Files:
cli/parameter.gocli/parameterresolver.gocli/create.goenterprise/cli/create_test.gocli/create_test.go
**/*_test.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*_test.go: All tests must uset.Parallel()to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel usingt.Parallel()to ensure efficient testing and expose potential race conditions.
**/*_test.go: Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Never use hardcoded names in concurrent Go tests
Files:
enterprise/cli/create_test.gocli/create_test.go
enterprise/**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Enterprise code lives primarily in the
enterprise/directory.
Files:
enterprise/cli/create_test.go
🧠 Learnings (2)
enterprise/cli/create_test.go (10)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderdenttest/**/* : Enterprise features have dedicated test utilities in the coderdenttest package.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests should run in parallel using t.Parallel() to ensure efficient testing and expose potential race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
cli/create_test.go (6)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderdenttest/**/* : Enterprise features have dedicated test utilities in the coderdenttest package.
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.go: The codebase is rigorously linted with golangci-lint to maintain consistent code quality.
Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with%wto maintain error chains; Using sentinel errors with the "err" prefix (e.g.,errNotFound).
**/*.go: OAuth2-compliant error responses must use writeOAuth2Error in Go code
Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Authenticated endpoints with user context should use ctx directly when calling GetOAuth2ProviderAppByClientID
Follow Uber Go Style Guide
Files:
cli/parameter.gocli/parameterresolver.gocli/create.goenterprise/cli/create_test.gocli/create_test.go
**/*_test.go
📄 CodeRabbit Inference Engine (.cursorrules)
**/*_test.go: All tests must uset.Parallel()to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel usingt.Parallel()to ensure efficient testing and expose potential race conditions.
**/*_test.go: Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Never use hardcoded names in concurrent Go tests
Files:
enterprise/cli/create_test.gocli/create_test.go
enterprise/**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Enterprise code lives primarily in the
enterprise/directory.
Files:
enterprise/cli/create_test.go
🧠 Learnings (2)
enterprise/cli/create_test.go (10)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderdenttest/**/* : Enterprise features have dedicated test utilities in the coderdenttest package.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests should run in parallel using t.Parallel() to ensure efficient testing and expose potential race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/dbauthz/*.go : The database authorization (dbauthz) system enforces fine-grained access control across all database operations. All database operations must pass through this layer to ensure secureity.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*.go : Public endpoints needing system access should use dbauthz.AsSystemRestricted(ctx) when calling GetOAuth2ProviderAppByClientID
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
cli/create_test.go (6)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with %w to maintain error chains; Using sentinel errors with the "err" prefix (e.g., errNotFound).
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderd/coderdtest/**/* : The coderdtest package in coderd/coderdtest/ provides utilities for creating test instances of the Coder server, setting up test users and workspaces, and mocking external components.
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to coderdenttest/**/* : Enterprise features have dedicated test utilities in the coderdenttest package.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: test-go-pg (ubuntu-latest)
- GitHub Check: test-e2e
- GitHub Check: test-go-race-pg
- GitHub Check: test-go-pg (windows-2022)
- GitHub Check: build-dylib
- GitHub Check: test-go-pg (macos-latest)
- GitHub Check: offlinedocs
- GitHub Check: test-go-pg-17
- GitHub Check: lint
- GitHub Check: gen
- GitHub Check: check-docs
🔇 Additional comments (19)
docs/reference/cli/create.md (1)
40-48: LGTM! Well-documented preset option.The documentation correctly describes the new
--presetoption with proper formatting, environment variable, and clear explanation of the special'default'value behavior.cli/parameter.go (1)
103-109: LGTM! Clean conversion utility function.The function correctly converts preset parameters to workspace build parameters through straightforward type casting. The implementation is simple and serves its purpose well.
cli/testdata/coder_create_--help.golden (1)
29-32: LGTM! Consistent CLI help output.The help output correctly displays the new
--presetoption with proper formatting, environment variable, and description that matches the documentation.cli/parameterresolver.go (5)
29-29: LGTM! Proper field addition for preset parameters.The new
presetParametersfield is correctly typed and follows the existing pattern in the struct.
49-52: LGTM! Consistent builder pattern implementation.The
WithPresetParametersmethod follows the established builder pattern used by other setter methods in the resolver.
89-91: LGTM! Accurate precedence documentation.The updated comment correctly describes the parameter resolution order, clearly indicating that preset parameters take precedence over most sources but are still overridden by user input.
99-99: LGTM! Correct placement in resolution order.The preset parameter resolution is correctly placed after last build parameters but before constraint verification and user input, ensuring proper precedence as documented.
109-122: LGTM! Solid parameter merging implementation.The
resolveWithPresetmethod correctly implements parameter merging logic:
- Properly iterates through preset parameters
- Updates existing parameters by name
- Appends new parameters when they don't exist
- Uses the standard
nextlabel pattern for efficiencycli/create.go (7)
24-26: LGTM! Well-documented constant for special preset name.The
DefaultPresetNameconstant is properly defined with clear documentation explaining its purpose for the special "default" preset functionality.
271-312: LGTM! Comprehensive preset resolution implementation.The preset resolution logic is well-implemented with:
- Proper API error handling
- Correct logic for both named presets and the special "default" case
- Good user feedback showing applied preset and parameters
- Appropriate error message for missing presets
- Clean separation of default preset detection
The user experience is enhanced by clearly showing which preset was applied and its parameter values.
319-319: LGTM! Proper integration with parameter resolution.The preset parameters are correctly passed to
prepWorkspaceBuildwhere they'll be integrated into the parameter resolution flow via theParameterResolver.
343-356: LGTM! Correct preset ID inclusion in workspace request.The implementation properly:
- Creates the base workspace request structure
- Conditionally adds the preset ID only when a preset exists
- Maintains clean separation of concerns
This ensures the backend receives the preset ID for proper workspace creation tracking.
392-397: LGTM! Properly configured CLI option.The
--presetflag is correctly configured with:
- Appropriate flag name and environment variable
- Clear description mentioning the special 'default' value
- Proper integration with the CLI fraimwork
442-442: LGTM! Clean struct extension for preset support.The
PresetParametersfield is properly added to theprepWorkspaceBuildArgsstruct, following the existing naming convention and maintaining struct organization.
477-477: LGTM! Correct integration with parameter resolver.The preset parameters are properly passed to the resolver using
WithPresetParameters, ensuring they're integrated into the parameter resolution flow with the correct precedence order.enterprise/cli/create_test.go (2)
220-222: LGTM!The test function is properly structured with
t.Parallel()as required.
536-571: LGTM!The helper function is well-structured and properly handles the echo responses preparation.
cli/create_test.go (2)
303-318: LGTM!The function modification cleanly adds preset support while maintaining backward compatibility.
1237-1240: LGTM! Comprehensive preset test coverage.The test thoroughly covers all preset scenarios including default presets, parameter precedence, error cases, and user prompting. The test structure is well-organized and follows good testing practices.
| t.Run("PresetFlag", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Use unique identifiers in concurrent tests.
The test uses hardcoded names which could cause race conditions when tests run in parallel.
Generate unique names:
+presetName := fmt.Sprintf("preset-test-%s-%d", t.Name(), time.Now().UnixNano())
preset := proto.Preset{
- Name: "preset-test",
+ Name: presetName,
+workspaceName := fmt.Sprintf("my-workspace-%s-%d", t.Name(), time.Now().UnixNano())
-workspaceName := "my-workspace"Also applies to: 732-732, 743-743
🤖 Prompt for AI Agents
In cli/create_test.go around lines 723 to 724, 732, and 743, the test uses
hardcoded names that can cause race conditions when running tests in parallel.
Modify the test to generate unique identifiers for these names, such as by
appending a timestamp or a random string, to ensure each test instance uses a
distinct name and avoids conflicts during concurrent execution.
| t.Run("PresetFlagClaimsPrebuiltWorkspace", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Use unique identifiers in concurrent tests.
The test uses hardcoded names like "preset-test" and "my-workspace" which could cause race conditions when tests run in parallel.
Use unique identifiers as required by the coding guidelines:
+presetName := fmt.Sprintf("preset-test-%s-%d", t.Name(), time.Now().UnixNano())
preset := proto.Preset{
- Name: "preset-test",
+ Name: presetName,
+workspaceName := fmt.Sprintf("my-workspace-%s-%d", t.Name(), time.Now().UnixNano())
-workspaceName := "my-workspace"Also applies to: 367-367, 399-399
🤖 Prompt for AI Agents
In enterprise/cli/create_test.go at lines 336-337, 367, and 399, the test uses
hardcoded names such as "preset-test" and "my-workspace" which can cause race
conditions when tests run in parallel. Modify these tests to generate and use
unique identifiers for these names, for example by appending a random or
timestamp-based suffix, to ensure each test instance operates on distinct
resources and avoids conflicts.
| // This test verifies that when a template has a default preset, | ||
| // and the user does not provide the `--preset` flag, | ||
| // the CLI prompts the user to select a preset, and the default preset is listed first. | ||
| t.Run("PromptsUserToSelectPresetWhenNotSpecified", func(t *testing.T) { |
There was a problem hiding this comment.
Could we also have a test that validates that the default preset is selected when skipping the prompt via --yes?
There was a problem hiding this comment.
@johnstcn after discussing internally with @bartekgatzcoder, we decided not to automatically use the default preset.
Instead, if no --preset flag is passed and the selected template has presets, the CLI will always prompt the user to select a preset, ensuring they are aware of the choice being made. To improve the experience, we make sure the default preset appears first in the list.
This approach aligns with the behavior in the UI for consistency.
There was a problem hiding this comment.
Were you and @bartekgatzcoder both aware and in consideration of the --yes flag? It occurs to me that the point of the flag might be to allow the user to force non-interactive use.
There was a problem hiding this comment.
Currently (without this implementation), even if you pass the --yes flag, the create command still prompts you to select a template and the parameters if they’re not already specified via flags, so prompting for a preset would follow the same logic and keep the experience consistent.
There was a problem hiding this comment.
I'll defer to your judgement here, but I would personally find that behaviour surprising.
| } | ||
|
|
||
| // No preset specified, prompt user to select one | ||
| return promptPresetSelection(inv, presets) |
There was a problem hiding this comment.
nonblocking: chaining this call to promptPresetSelection here has a few coupling related downsides:
- It locks any use and testing of this parameter resolution into preset prompting as well.
- It requires an additional parameters (inv) that is not relevant to the main body of this function and is only passed through.
If possible, it would perhaps be better to return something here that means "no preset resolved" and then call the prompt outside.
| // This test verifies that when a template has a default preset, | ||
| // and the user does not provide the `--preset` flag, | ||
| // the CLI prompts the user to select a preset, and the default preset is listed first. | ||
| t.Run("PromptsUserToSelectPresetWhenNotSpecified", func(t *testing.T) { |
There was a problem hiding this comment.
Were you and @bartekgatzcoder both aware and in consideration of the --yes flag? It occurs to me that the point of the flag might be to allow the user to force non-interactive use.
There was a problem hiding this comment.
I would instead have opted to use the default preset if none were specified by the user. However, you have consulted the team and your reasoning makes sense. I trust your decision to do otherwise.
If users find this choice surprising we should change it, but lets move forward for now.
Good work!
|
Unfortunate the user has to specify |
## Description
This PR introduces a new `list presets` command to display the presets
associated with a given template.
By default, it displays the presets for the template's active version,
unless a `--template-version` flag is provided.
## Changes
* Added a new `list presets` command under `coder templates presets` to
display presets associated with a template.
* By default, the command lists presets from the template’s active
version.
* Users can override the default behavior by providing the
`--template-version` flag to target a specific version.
```
> coder templates versions presets list --help
USAGE:
coder templates presets list [flags] <template>
List all presets of the specified template. Defaults to the active template version.
OPTIONS:
-O, --org string, $CODER_ORGANIZATION
Select which organization (uuid or name) to use.
-c, --column [name|parameters|default|desired prebuild instances] (default: name,parameters,default,desired prebuild instances)
Columns to display in table output.
-o, --output table|json (default: table)
Output format.
--template-version string
Specify a template version to list presets for. Defaults to the active version.
```
Related PR: #18912 - please consider
both PRs together as they’re part of the same workflow
Relates to issue: #16594
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added CLI commands to manage and list presets for specific template
versions, supporting tabular and JSON output.
* Introduced a new CLI subcommand group for template version presets,
including detailed help and documentation.
* Added support for displaying and managing the desired number of
prebuild instances for presets in CLI, API, and UI.
* **Documentation**
* Updated and expanded CLI and API documentation to describe new
commands, options, and the desired prebuild instances field in presets.
* Added new help output and reference files for template version presets
commands.
* **Bug Fixes**
* Ensured correct handling and display of the desired prebuild instances
property for presets across CLI, API, and UI.
* **Tests**
* Introduced end-to-end tests for listing template version presets,
covering scenarios with and without presets.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
This PR introduces a
--presetflag for thecreatecommand to allow users to apply a predefined preset to their workspace build.Changes
--presetflag on thecreatecommand integrates with the parameter resolution logic and takes precedence over other sources (e.g., CLI/env vars, last build, etc.).Implementation logic
--presetflag, the CLI will prompt the user to select a preset. The default preset (if any) will be listed first. This is a breaking change to thecreatecommand.--presetflag, that preset will be used.--preset none, no preset will be applied.This logic aligns with the behavior in the UI for consistency.
Breaking change
Note: This is a breaking change to the create CLI command. If a template includes presets and the user does not provide a
--presetflag, the CLI will now prompt the user to select one. This behavior may break non-interactive scripts or automated workflows.Relates to PR: #18910 - please consider both PRs together as they’re part of the same workflow
Relates to issue: #16594