fix: add preset support to MCP tools#24694
Conversation
|
/coder-agents-review |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean single-concern PR. Preset support is plumbed through all three tools with correct backend delegation; the wsbuilder handles parameter merging and prebuild claiming, and the tools are thin pass-through wrappers. Tests cover both happy and error paths across all three tools. 1 P2, 6 P3, 3 Nit.
The P2 is a cross-version preset staleness gap: when chatStartWorkspace auto-updates to the active template version, the LLM's stale preset_id survives, potentially binding wrong parameter defaults to the new version. The fix is one line in exp_chats.go.
The P3s cluster around two themes: (1) tool descriptions that are stale or narrower than actual capability, and (2) test coverage gaps where new branches are created but never asserted. One convergent finding (7 reviewers) flags the unconditional preset parameter query.
"The preset-id mismatch count tracks the publish cadence." (Hisoka)
coderd/x/chatd/chattool/readtemplate.go:31
P3 [DEREM-8] The tool description says "configurable parameters" but the tool now also returns presets. Both create_workspace and start_workspace say "Obtain available presets from read_template," but read_template's own description does not mention presets. An LLM browsing available tools would not learn that read_template is the source for preset information.
Suggestion: "including its configurable parameters and available presets." (Chopper)
🤖
🤖 This review was automatically generated with Coder Agents.
johnstcn
changed the title
johnstcn
changed the title
|
/coder-agents-review |
There was a problem hiding this comment.
Re-review after addressing 8 of 10 R1 findings. All 8 fixes verified by the panel. Both contested findings (DEREM-7, DEREM-9) closed unanimously (7/7 accept author defense). 2 P2, 1 P3 new.
DEREM-17 is a regression in the DEREM-2 fix: the preset clear runs unconditionally inside the RequireActiveVersion block, silently dropping valid presets even when the template version did not change. Enterprise templates with RequireActiveVersion are common; a user requesting a preset on a workspace already at the active version loses their preset selection every time. The fix is wrapping the clear in if updatedToActiveVersion.
DEREM-16 (P2) flags the same code path from a different angle: the DEREM-2 fix has zero test coverage. A P2 fix that can regress to a P2 consequence without detection.
"The auto-update path itself was untested before this PR, so the gap is partially pre-existing, but the new preset-clearing behavior is this PR's responsibility." (Netero)
coderd/x/chatd/chattool/startworkspace.go:246
P3 [DEREM-18] When chatStartWorkspace auto-updates to the active version and clears the preset, the tool response includes updated_to_active_version: true but never mentions the preset was dropped. The LLM has args.PresetID (what was requested) and startBuild.TemplateVersionPresetID (what was applied). Comparing them would take three lines and would let the response include preset_dropped: true or equivalent, so the LLM does not tell the user their preset was applied when it was not.
(Hisoka P3, Pariston Note)
🤖
🤖 This review was automatically generated with Coder Agents.
Adds a coder_template_version_presets tool so agents can discover presets, and a template_version_preset_id argument on the create_workspace tool so they can use one. The backend and the create_task tool already honored the preset ID; this closes the agent-facing gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 882ed84)
…t_template Folds parameters and presets into a single coder_get_template tool, mirroring the chattool read_template surface added in #24694. Drops the standalone coder_template_version_presets tool, which has not shipped externally yet. External agent flow drops from list_templates -> template_version_parameters -> template_version_presets -> create_workspace (4 calls) to list_templates -> get_template -> create_workspace (3 calls), matching the conceptual shape of the in-product chat tools.
johnstcn
changed the title
|
/coder-agents-review |
There was a problem hiding this comment.
Review blocked. Three open findings from round 2 have no author response or code changes targeting them. The R2-to-R3 diff modifies only codersdk/toolsdk/ files; none of the findings below are addressed.
Unaddressed findings:
- DEREM-17 (P2,
exp_chats.go:3215): Preset clear is unconditional insideRequireActiveVersion, silently drops valid presets when the template version did not change. Fix: wrap inif updatedToActiveVersion { ... }. - DEREM-16 (P2,
exp_chats.go:3215): The DEREM-2 fix has zero test coverage. No test exerciseschatStartWorkspacewith a preset_id underRequireActiveVersion. - DEREM-18 (P3,
startworkspace.go:246): Response does not signal the preset was dropped on auto-update.
Further review is blocked until the author responds to or pushes fixes for these findings.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
There was a problem hiding this comment.
Re-review after all R2 findings addressed. DEREM-16/17/18 verified fixed by the full panel. Scope expanded to include toolsdk GetTemplate, CreateWorkspace preset support, and CreateWorkspaceBuild preset/params support. All prior fixes remain sound. 3 P3 new, no P2+.
The expanded toolsdk surface is well-structured: presetView projects the untagged codersdk.Preset into snake_case, richParametersFromMap deduplicates parameter conversion, errors.Join surfaces preset+params violations in one round-trip for LLM efficiency. Test coverage is thorough (1150 test lines for 340 production lines).
"If parameter names diverge between versions, wrong values are stored." (Hisoka)
🤖 This review was automatically generated with Coder Agents.
Restores symmetry with chatCreateWorkspace. Tests reach the method via an export_test.go alias; CODAGT-292 tracks the proper pure-function extraction.
…workspace_build Presets are scoped to the template version they were created on. The tool defaulted template_version_id to the previous build's version when omitted, so a preset from the active version could be silently applied to an older version with mismatched parameter defaults. Surface the constraint in the description so the LLM pairs them up. Addresses DEREM-21.
…n get_template Set DesiredInstances on the GetTemplate fixture preset and assert DesiredPrebuildInstances flows through toPresetView. Without this, a regression in the projection (e.g. dropping the field) would not be caught and the LLM would silently lose the prebuild hint promised by the tool description. Addresses DEREM-23.
# Conflicts: # coderd/exp_chats_test.go
|
/coder-agents-review |
There was a problem hiding this comment.
R4 findings (DEREM-21/22/23) verified fixed. The export_test.go correction is clean with a tracking ticket for deeper refactoring. The desired_prebuild_instances test catches toPresetView field regressions. The cross-version preset description warns LLMs appropriately. 1 P3 new.
The remaining P3 is an asymmetry between the two tool surfaces: the toolsdk GetTemplate includes desired_prebuild_instances so MCP consumers can prefer prebuilt presets, but the chattool read_template omits it while the create_workspace description promises "potentially claim a prebuilt workspace for faster startup." The LLM navigates blind when multiple presets exist.
"Template with 5 presets, one with prebuilts? The chat LLM has a 1-in-5 chance of finding the fast path." (Hisoka)
🤖 This review was automatically generated with Coder Agents.
…read_template The create_workspace tool description tells the LLM that supplying a preset_id may claim a prebuilt workspace for faster startup, but the read_template response never indicated which presets actually have prebuilds. With multiple presets the LLM had no signal for the fast path. The DB row carries the count already; mirror the toolsdk desired_prebuild_instances key so MCP and chat surfaces stay consistent. Addresses DEREM-25.
|
/coder-agents-review |
There was a problem hiding this comment.
DEREM-25 verified fixed: read_template now includes desired_prebuild_instances on presets, matching the toolsdk surface. Test covers the round-trip and absence on unset presets. 1 P3 new.
DEREM-27 is a structural variant of DEREM-21 on the chattool surface: start_workspace has no template_version_id field, so the DEREM-21 description-warning mitigation is unavailable. When RequireActiveVersion is false and the workspace is on an older version, the LLM sees active-version presets it cannot correctly apply. The root cause is wsbuilder lacking preset-version affinity validation (acknowledged as out of scope in DEREM-21). This needs a human decision: add template_version_id to start_workspace, fix wsbuilder, or accept the gap.
26 findings raised across 6 rounds, 17 fixed, 2 closed by panel, 8 dropped. 1 open (DEREM-27).
"The LLM is shown presets it may not be able to correctly use, and has no escape hatch." (Hisoka)
🤖 This review was automatically generated with Coder Agents.
Presets are a creation-time configuration choice. They drive prebuild claiming and parameter defaults at create; subsequent starts use the workspace's existing version and parameters. Users who need a different preset or version on an existing chat can create the workspace out-of-band (CLI / UI / API) with the desired configuration and attach the chat to it. Addresses DEREM-27.
…reActiveVersion The agents experiment flag was removed in main; the helper that built DeploymentValues for chat tests was updated in the same commit. Mirror that here.
|
/coder-agents-review |
There was a problem hiding this comment.
DEREM-27 verified resolved: preset_id dropped from start_workspace entirely, eliminating the cross-version preset mismatch on the chattool surface. The rationale is clean: prebuild claiming only happens at creation, so presets on start had no prebuild benefit, only risk. 1 P3, 1 Nit new.
27 findings raised across 7 rounds, 18 fixed, 2 closed by panel (7/7), 9 dropped. 2 open (DEREM-28, DEREM-29). Both are minor: missing invalid-UUID tests for three branches in toolsdk CreateWorkspace, and a pre-existing em-dash in a string the PR modifies.
"Seven rounds. Twenty-seven findings, all resolved. I pulled every thread I could see... This one held up." (Hisoka)
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for template version presets across the chat tool surface and the codersdk/toolsdk MCP tools, so agents can discover presets and apply them when creating workspaces/builds (including surfacing prebuild hints).
Changes:
- Extend chat
read_templateto return active-version presets (including preset parameters anddesired_prebuild_instanceswhen set). - Extend chat
create_workspaceto acceptpreset_idand forward it astemplate_version_preset_idduring workspace creation. - Add
coder_get_templatetotoolsdk, plus preset +rich_parameterssupport and transition-scoped validation forcoder_create_workspace_build; updatecoder_create_workspaceto supporttemplate_id/template_version_idexclusivity andtemplate_version_preset_id.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| codersdk/toolsdk/toolsdk.go | Adds coder_get_template, preset plumbing for create/build tools, and request pre-validation helpers. |
| codersdk/toolsdk/toolsdk_test.go | Expands MCP tool integration tests for presets, rich parameters, and new tool annotations. |
| coderd/x/chatd/chattool/readtemplate.go | Includes template version presets (and their parameters) in read_template responses. |
| coderd/x/chatd/chattool/readtemplate_test.go | Adds coverage for read_template returning presets and omitting them when empty. |
| coderd/x/chatd/chattool/createworkspace.go | Accepts preset_id and forwards it into CreateWorkspaceRequest.TemplateVersionPresetID. |
| coderd/x/chatd/chattool/createworkspace_test.go | Adds focused tests to verify preset_id parsing/forwarding and preset + params behavior. |
| coderd/x/chatd/chattool/startworkspace.go | Minor formatting-only change in request assembly block. |
| coderd/export_test.go | Exposes chatStartWorkspace to external tests via ChatStartWorkspace alias. |
| coderd/exp_chats.go | Documents the ChatStartWorkspace test alias and the rationale for it. |
| coderd/exp_chats_test.go | Adds an end-to-end test ensuring RequireActiveVersion auto-update on chat start doesn’t apply presets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // presetView is a tool-local projection of codersdk.Preset with | ||
| // snake_case JSON keys that match the field names referenced in | ||
| // the create_workspace tool description. codersdk.Preset has no | ||
| // JSON tags, so its fields would otherwise serialize as PascalCase | ||
| // and the LLM would look for keys that do not exist on the wire. | ||
| type presetView struct { | ||
| ID uuid.UUID `json:"id"` | ||
| Name string `json:"name"` | ||
| Description string `json:"description,omitempty"` | ||
| Default bool `json:"default"` | ||
| DesiredPrebuildInstances *int `json:"desired_prebuild_instances,omitempty"` | ||
| Parameters []presetParameterView `json:"parameters"` | ||
| } |
There was a problem hiding this comment.
GetTemplate’s presetView drops the preset Icon even though codersdk.Preset includes it (and the chattool read_template response includes it). Consider adding an icon field (omitempty) to presetView and populating it in toPresetView so preset metadata is consistent across surfaces.
The chat tools (
read_template,create_workspace) did not surface or respect template version presets. Presets were invisible to the LLM and preset parameter defaults were never applied at workspace creation. ThetoolsdkMCP surface had the same gap (ref #24695, now subsumed here).What this changes
read_templatereturns presets withid,name,default,description,icon,parameters, anddesired_prebuild_instances(when set), so the LLM can pick the right preset and prefer prebuilt-backed ones.create_workspaceaccepts apreset_id. The wsbuilder applies preset parameter defaults and may claim a prebuilt workspace.start_workspacedoes not accept a preset. Presets are a creation-time choice; subsequent starts use the workspace's existing version and parameters. Users who need a specific preset or version on an existing chat can create the workspace out-of-band (CLI / UI / API) with the desired configuration and attach the chat to it.toolsdkgainsGetTemplate(with presets includingdesired_prebuild_instances), preset support onCreateWorkspace, and preset +rich_parameterssupport onCreateWorkspaceBuild. Thetemplate_version_preset_iddescription warns about preset/version affinity.Scope
coderd/x/chatd/chattool/(chat tools)coderd/exp_chats.go(chat tool wiring + auto-update onRequireActiveVersion)codersdk/toolsdk/(MCP tool surface)Review history
Multiple rounds of automated review covered:
start_workspaceentirely (DEREM-27)desired_prebuild_instanceson both surfaces (DEREM-23, plusread_templatetest)chatStartWorkspaceandchatCreateWorkspace(DEREM-22), resolved with anexport_test.goalias. The deeper refactor to a pure request-builder is tracked in CODAGT-292.start_workspace(DEREM-27), addressed by droppingpreset_idfrom the start tool. Prebuild claiming only happens at workspace creation, so a preset on a start build had no prebuild benefit and risked applying active-version preset values to a stale build's parameter schema.