fix: use dynamic parameter resolution in the cli#21734
Merged
Conversation
This was referenced Jan 28, 2026
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
9bfa44b to
c98542a
Compare
f7c8acd to
da22b9c
Compare
c8bc7fb to
374aa14
Compare
526c413 to
52b13b6
Compare
|
All contributors have signed the CLA ✍️ ✅ |
52b13b6 to
f7ccc47
Compare
da22b9c to
0d75983
Compare
838493d to
9ba2198
Compare
0d75983 to
36f1eb9
Compare
johnstcn
reviewed
Feb 3, 2026
9ba2198 to
33ab544
Compare
36f1eb9 to
ab7a276
Compare
0de394a to
2b9bd55
Compare
johnstcn
approved these changes
Feb 3, 2026
|
|
||
| func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter { | ||
| func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { | ||
| if pr.promptRichParameters { |
Member
There was a problem hiding this comment.
Calling resolveWithLastBuildParameters in this case feels like a logic error? Nevertheless I'm OK with short-circuiting here.
Member
Author
There was a problem hiding this comment.
I don't follow what you are saying 🤔
Member
There was a problem hiding this comment.
Why call this function at all if we know ahead of time that we don't want to fetch parameters from the last build?
Member
Author
There was a problem hiding this comment.
Ah, I see. Do you mean instead to write it as:
if !pr.promptRichParameters {
staged = pr.resolveWithLastBuildParameters(staged)
}instead?
If so, I'm not changing to code pattern, I just moved some existing code around
Member
There was a problem hiding this comment.
Perhaps; the flow just seemed weird to me. Not a blocker in any case.
Member
Author
There was a problem hiding this comment.
The parameter resolution code is a bit tangled imo. I wanted to avoid trying to untangle it, and just update it to use the dynamic param resolver instead of the static.
| "workspace_folder": "/workspace1", | ||
| "name": "dev1", | ||
| "id": "eb9b7f18-c277-48af-af7c-2a8e5fb42bab", | ||
| "subagent_id": "72d17819-ea3b-450a-a502-175886583ecf" |
Member
There was a problem hiding this comment.
I don't think this change is relevant, see also L35 below.
Member
Author
There was a problem hiding this comment.
This is strange. I just rebased it, and make gen did this. I assumed I missed something on main. Looking into it
ab7a276 to
38ce00f
Compare
10bb903 to
1841432
Compare
1841432 to
7afdcf3
Compare
38ce00f to
6021e9b
Compare
7afdcf3 to
92f45cc
Compare
7d2d291 to
dabb755
Compare
92f45cc to
9d448e6
Compare
When using dynamic parameters, parameters without template defaults are marked as required. However, users can still provide defaults via CLI flags (--parameter-default). This change ensures: 1. The default value is shown in the prompt text whenever one is provided (via CLI flags), regardless of whether the parameter is marked as required. 2. Validation accepts empty input when a CLI default is available, substituting the default value before validation runs.
dabb755 to
6759b51
Compare
9d448e6 to
1025e2c
Compare
1025e2c to
d535f5b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #19879
What this does
Uses dynamic parameters
EvaluateTemplateVersionvsTemplateVersionRichParametersto determine initial parameter state.This is still not perfect, as dynamic templates are reactive. So if the cli prompts the user for a value, that value can change another parameter. So to fix this holistically, we need fundamentally redesign the parameter logic in the cli to be in a loop. The ux for that is not obvious though, as you might have to re-prompt the user for things they already decided?
For now, this is a stop gap. This handles if the initial parameters are dynamic based on initial inputs (initial args & user context)
Changes Summary (AI generated)
Core Logic (
cli/create.go)Ownerfield toprepWorkspaceBuildArgsto support dynamic parameter evaluationprepWorkspaceBuildto checktemplate.UseClassicParameterFlowand conditionally use either:client.TemplateVersionRichParameters()(existing behavior)client.EvaluateTemplateVersion()with initial parameter values from CLI inputsParameter Resolution (
cli/parameterresolver.go)InitialValues()method to gather CLI-provided parameter values (from flags, files, presets, last build, etc.) without validating against template parameters. This is required to initialize any dynamic elements of a template off the original input.