feat(cli): add CLI support for creating a workspace with preset #18912
Conversation
cli/create.go
Outdated
| @@ -21,10 +21,15 @@ import ( | |||
| "github.com/coder/serpent" | |||
| ) | |||
|
|
|||
| // DefaultPresetName is used when a user runs `create --preset default`. | |||
| // It instructs the CLI to use the default preset defined for the template version, if one exists. | |||
| const DefaultPresetName = "default" | |||
There was a problem hiding this comment.
To preserve current behavior, if no --preset is provided, the CLI does not apply any preset. I added support for a special default value to allow users to explicitly use the default preset (if one exists). However, this approach might not be ideal, as it introduces ambiguity, e.g., what if a preset is literally named "default"?
Another approach I considered was assigning a default value (like default) to the --preset flag itself. But that could lead to unexpected behavior: the user may not realize a preset is being applied at all.
A potentially better UX would be:
- If the template has presets and none is specified via
--preset, prompt the user to select one. - If there’s a default preset, it should appear as the first option (as in the UI).
- Add a
noneoption to explicitly skip using a preset (as in the UI). This means that instead of having a default value for the preset, we can have a none value that explicitly indicates no preset should be used. - This would bring the CLI experience closer to the web interface and make it more intuitive.
However, the downside is that this would break existing automation/scripts that rely on creating workspaces silently when presets exist. Those scripts would now hang, waiting for input.
Let me know what you think
There was a problem hiding this comment.
You've summarised the trade-offs well. My assessment of them is that without a --preset flag specified, it should use the default as defined in the template. This use case is what we allow a default to be defined for.
I see you've already added a line to the command output to inform the user which preset was chosen, so that should help make it more visible.
Like with the frontend, we can allow for a "None" preset. In fact it might be a good idea to move that from the frontend to the API so that we have it here without any additional trouble.
There was a problem hiding this comment.
As discussed internally, we’ve decided on the following behavior for preset selection in the CLI:
- If a template has presets and the user does not specify the
--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. - If a template does not have presets, the CLI will not prompt the user.
- If the user specifies a preset using the
--presetflag, that preset will be used. - If the user passes
--preset none, no preset will be applied.
This logic aligns with the behavior in the UI for consistency.
There was a problem hiding this comment.
Solid work @ssncferreira. Thorough tests! Let me know where you land on the default preset matter? :)
cli/create.go
Outdated
| @@ -21,10 +21,15 @@ import ( | |||
| "github.com/coder/serpent" | |||
| ) | |||
|
|
|||
| // DefaultPresetName is used when a user runs `create --preset default`. | |||
| // It instructs the CLI to use the default preset defined for the template version, if one exists. | |||
| const DefaultPresetName = "default" | |||
There was a problem hiding this comment.
You've summarised the trade-offs well. My assessment of them is that without a --preset flag specified, it should use the default as defined in the template. This use case is what we allow a default to be defined for.
I see you've already added a line to the command output to inform the user which preset was chosen, so that should help make it more visible.
Like with the frontend, we can allow for a "None" preset. In fact it might be a good idea to move that from the frontend to the API so that we have it here without any additional trouble.
| @@ -411,6 +477,7 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p | |||
| WithSourceWorkspaceParameters(args.SourceWorkspaceParameters). | |||
| WithPromptEphemeralParameters(args.PromptEphemeralParameters). | |||
| WithEphemeralParameters(args.EphemeralParameters). | |||
| WithPresetParameters(args.PresetParameters). | |||
There was a problem hiding this comment.
Should this not be last in the list? Otherwise we allow preset parameters to be overwritten.
There was a problem hiding this comment.
This step does not overwrite the preset parameters. It simply adds the preset parameters to the ParameterResolver:
coder/cli/parameterresolver.go
Lines 49 to 52 in ae16155
The actual overwrite logic happens later in the Resolve method. But importantly, resolveWithPreset is added as the last step in the resolution process:
coder/cli/parameterresolver.go
Lines 95 to 103 in ae16155
After that, the code verifies if there are any constraints and prompts the user for any missing parameters.
So the order ensures preset parameters are only applied after everything else, preventing unwanted overwrites.
| 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 security.
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 security.
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 framework
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.
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.).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
Summary by CodeRabbit
New Features
--presetCLI option (and$CODER_PRESET_NAMEenv variable) to specify a template version preset when creating workspaces, including support for a special"default"preset.Bug Fixes
Documentation
--presetoption and its usage.Tests