Skip to content

feat(cli): add CLI support for creating a workspace with preset #18912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ssncferreira
Copy link
Contributor

@ssncferreira ssncferreira commented Jul 17, 2025

Description

This PR introduces a --preset flag for the create command to allow users to apply a predefined preset to their workspace build.

Changes

  • The --preset flag on the create command integrates with the parameter resolution logic and takes precedence over other sources (e.g., CLI/env vars, last build, etc.).
  • Added internal logic to ensure that preset parameters override parameters values during resolution.
  • Updated tests and added new ones to cover these flows.
> coder create --help

USAGE:
  coder create [flags] [workspace]

  Create a workspace

    - Create a workspace for another user (if you have permission):

        $ coder create <username>/<workspace_name>

OPTIONS:
      (...)

      --preset string, $CODER_PRESET_NAME
          Specify the name of a template version preset. Use 'none' to explicitly indicate that no preset should be used.

      (...)

  -y, --yes bool
          Bypass prompts.

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 --preset flag, 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

    • Added a --preset CLI option (and $CODER_PRESET_NAME env variable) to specify a template version preset when creating workspaces, including support for a special "default" preset.
    • Preset parameters are merged into workspace creation, overriding other parameter sources as needed.
    • CLI output now displays the applied preset and its parameters.
  • Bug Fixes

    • Improved handling and error messaging when a specified preset does not exist or is incomplete.
  • Documentation

    • Updated CLI help and documentation to describe the new --preset option and its usage.
  • Tests

    • Introduced comprehensive tests covering preset selection, default preset logic, parameter precedence, error cases, user prompting for missing parameters, and integration with prebuilt workspaces.

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"
Copy link
Contributor Author

@ssncferreira ssncferreira Jul 17, 2025

Choose a reason for hiding this comment

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

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 none option 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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 --preset flag, 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 the create command.
  • If a template does not have presets, the CLI will not prompt the user.
  • If the user specifies a preset using the --preset flag, 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.

@ssncferreira ssncferreira marked this pull request as ready for review July 17, 2025 09:29
@ssncferreira ssncferreira requested review from Emyrk and SasSwart July 17, 2025 09:30
Copy link
Contributor

@SasSwart SasSwart left a comment

Choose a reason for hiding this comment

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

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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).
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not be last in the list? Otherwise we allow preset parameters to be overwritten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step does not overwrite the preset parameters. It simply adds the preset parameters to the ParameterResolver:

func (pr *ParameterResolver) WithPresetParameters(params []codersdk.WorkspaceBuildParameter) *ParameterResolver {
pr.presetParameters = params
return pr
}

The actual overwrite logic happens later in the Resolve method. But importantly, resolveWithPreset is added as the last step in the resolution process:

staged = pr.resolveWithParametersMapFile(staged)
staged = pr.resolveWithCommandLineOrEnv(staged)
staged = pr.resolveWithSourceBuildParameters(staged, templateVersionParameters)
staged = pr.resolveWithLastBuildParameters(staged, templateVersionParameters)
staged = pr.resolveWithPreset(staged) // Preset parameters take precedence from all other parameters
if err = pr.verifyConstraints(staged, action, templateVersionParameters); err != nil {
return nil, err
}
if staged, err = pr.resolveWithInput(staged, inv, action, templateVersionParameters); err != nil {

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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!

Copy link

coderabbitai bot commented Jul 23, 2025

Walkthrough

A new --preset CLI option and corresponding environment variable were added to the workspace creation command, allowing users to specify a template version preset by name—including a special "default" preset. The parameter resolution and workspace build flows were updated to support presets. Extensive tests and documentation updates accompany the feature.

Changes

File(s) Change Summary
cli/create.go, cli/parameter.go, cli/parameterresolver.go Added support for specifying and resolving template version presets in workspace creation, including parameter precedence logic and helpers.
cli/create_test.go, enterprise/cli/create_test.go Introduced comprehensive tests covering preset application, error cases, parameter precedence, user prompting, and integration with prebuilt workspaces.
cli/testdata/coder_create_--help.golden, docs/reference/cli/create.md Updated CLI help and documentation to describe the new --preset option and its usage.

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
Loading

Estimated code review effort

4 (~90 minutes)

Possibly related issues

Poem

A hop, a skip, a preset new—
Now workspaces bloom in every hue!
With flags and tests, the code’s robust,
Preset or default, in Coder we trust.
🐇✨
“Choose your flavor, set your scene,
The CLI’s now a preset machine!”


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae16155 and 3d7b40e.

📒 Files selected for processing (2)
  • cli/create_test.go (3 hunks)
  • enterprise/cli/create_test.go (2 hunks)
📓 Path-based instructions (3)
**/*_test.go

📄 CodeRabbit Inference Engine (.cursorrules)

**/*_test.go: All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel using t.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:

  • cli/create_test.go
  • enterprise/cli/create_test.go
**/*.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 %w to 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/create_test.go
  • enterprise/cli/create_test.go
enterprise/**/*

📄 CodeRabbit Inference Engine (.cursorrules)

Enterprise code lives primarily in the enterprise/ directory.

Files:

  • enterprise/cli/create_test.go
🧠 Learnings (2)
cli/create_test.go (8)

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: 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 **/*_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 **/*.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: .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.

enterprise/cli/create_test.go (11)

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: 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 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 **/*_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: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go

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: .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.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.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.

🧰 Additional context used
📓 Path-based instructions (3)
**/*_test.go

📄 CodeRabbit Inference Engine (.cursorrules)

**/*_test.go: All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel using t.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:

  • cli/create_test.go
  • enterprise/cli/create_test.go
**/*.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 %w to 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/create_test.go
  • enterprise/cli/create_test.go
enterprise/**/*

📄 CodeRabbit Inference Engine (.cursorrules)

Enterprise code lives primarily in the enterprise/ directory.

Files:

  • enterprise/cli/create_test.go
🧠 Learnings (2)
cli/create_test.go (8)

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: 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 **/*_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 **/*.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: .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.

enterprise/cli/create_test.go (11)

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: 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 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 **/*_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: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to enterprise/audit/table.go : If audit errors occur after database changes, update enterprise/audit/table.go

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: .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.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.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined in coderd/coderd.go and uses Chi for HTTP routing.

⏰ 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: offlinedocs
  • GitHub Check: test-e2e
  • GitHub Check: test-go-pg (windows-2022)
  • GitHub Check: build-dylib
  • GitHub Check: test-go-pg (macos-latest)
  • GitHub Check: test-go-pg (ubuntu-latest)
  • GitHub Check: test-go-race-pg
  • GitHub Check: test-go-pg-17
  • GitHub Check: lint
  • GitHub Check: gen
  • GitHub Check: check-docs
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between aae5fc2 and ae16155.

📒 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 %w to 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.go
  • cli/parameterresolver.go
  • cli/create.go
  • enterprise/cli/create_test.go
  • cli/create_test.go
**/*_test.go

📄 CodeRabbit Inference Engine (.cursorrules)

**/*_test.go: All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel using t.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.go
  • cli/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 %w to 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.go
  • cli/parameterresolver.go
  • cli/create.go
  • enterprise/cli/create_test.go
  • cli/create_test.go
**/*_test.go

📄 CodeRabbit Inference Engine (.cursorrules)

**/*_test.go: All tests must use t.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
All tests should run in parallel using t.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.go
  • cli/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 --preset option 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 --preset option 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 presetParameters field is correctly typed and follows the existing pattern in the struct.


49-52: LGTM! Consistent builder pattern implementation.

The WithPresetParameters method 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 resolveWithPreset method 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 next label pattern for efficiency
cli/create.go (7)

24-26: LGTM! Well-documented constant for special preset name.

The DefaultPresetName constant 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 prepWorkspaceBuild where they'll be integrated into the parameter resolution flow via the ParameterResolver.


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 --preset flag 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 PresetParameters field is properly added to the prepWorkspaceBuildArgs struct, 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.

Comment on lines +723 to +724
t.Run("PresetFlag", func(t *testing.T) {
t.Parallel()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

Comment on lines +336 to +337
t.Run("PresetFlagClaimsPrebuiltWorkspace", func(t *testing.T) {
t.Parallel()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

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.

2 participants