feat(coderd): accept parameters in start_workspace tool#24434
feat(coderd): accept parameters in start_workspace tool#24434ethanndickson merged 4 commits intomainfrom
Conversation
13da2f0 to
feda075
Compare
mafredri
left a comment
There was a problem hiding this comment.
The approach is well-reasoned. Pariston investigated four framings of the problem (missing capability, unhelpful format, eager auto-upgrade, missing orchestration) and concluded the chosen solution is the simplest one that works. The parameter forwarding mirrors create_workspace exactly, ElementsMatch correctly handles map iteration order, and the IsError: false + structured JSON pattern is consistent with the existing buildToolResponse convention. The ClientType backfill on two straggler tests is a nice cleanup.
Severity count: 2 P2, 2 P3, 1 P4, 3 Nit.
The main concern is DEREM-3: template_id is injected into every Responder error, not just parameter validation failures. Five reviewers converged on this independently. The fix is a one-line scope guard.
The two CI title failures are from the scope coderd/x/chatd/chattool not covering coderd/exp_chats.go. P2 [DEREM-6] Use coderd or omit the scope.
"This test proves the tool layer correctly unpacks a structured Responder error into JSON. It does not prove the error gets structured correctly in the first place." -- Bisky
🤖 This review was automatically generated with Coder Agents.
|
Also addressed DEREM-6 by retitling the PR to |
The start_workspace chat tool now accepts an optional parameters map, matching the existing create_workspace pattern. When an active-version upgrade introduces new required parameters, the tool returns structured JSON with the error details, validation failures, and template_id so the model can call read_template, discover what changed, and retry start_workspace with the correct values. Changes: - Add startWorkspaceArgs with Parameters map[string]string to start_workspace, forwarded as RichParameterValues. - Add responseErrorResult helper in chattool.go to convert codersdk.Response into structured tool JSON (preserving validations that IsError content would drop). - Update exp_chats.go to replace the UI-only guidance message with model-actionable retry instructions and preserve the original responder's detail/validations. - Add PassesParameters and update ManualUpdateRequired tests.
fe24ae4 to
1e75493
Compare
ibetitsmike
left a comment
There was a problem hiding this comment.
worth making sure all edge cases are covered - especially if the agent hallucinates param values
The retry instructions for start_workspace's manual-update-required path told the model to read_template and retry with parameters, but didn't guide it on what to do when the right value for a parameter isn't obvious from its description or defaults. Add one sentence telling the model to ask the user in that case rather than guessing.
…es in start_workspace "Optionally provide parameter values" told the model it *can* pass parameters but not *when* to. Rewrite to 'only if necessary or explicitly requested by the user' so the model doesn't preemptively call read_template and pass params on every start.
When the chat
start_workspacetool triggers an active-version upgrade that introduces new required parameters, the build fails with a parameter validation error. Previously this returned a message telling the user to update from the UI — a dead end for the model.This PR lets the model recover inside the chat by:
parametersmap onstart_workspace(same schema ascreate_workspace), forwarded asRichParameterValues.template_id, so the model can callread_templateto discover what changed.exp_chats.gowith model-actionable retry instructions.The expected model flow on an active-version parameter failure is now: