Repository navigation
refactor(coderd/x/chatd)!: retire the close_agent tool alias - #31003
ibetitsmike wants to merge 1 commit into
Conversation
|
@codex review
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Closing this PR in favor of #31005, which carries the same patch (identical patch-id) on top of #31000. This PR sat on #31001 and #31002, which left the stack when Mike chose to keep those two fallbacks for now (decision D6). Review continues on #31005. The branch stays in place.
|
This PR breaks compatibility with chat histories that contain
close_agenttool calls, which chats from v2.34 and earlier have. #26673 (v2.35) renamedclose_agenttointerrupt_agentand kept a hidden alias, so a model that copied the old name from such a history still interrupted the child. This PR deletes the alias:subagentToolNameAliases,ExecuteLocalToolsOptions.ToolNameAliaseswith its five resolution sites inchatloop, the alias step in builtin input validation, and theclose_agententries in the unbilled tool set, the plan and explore policies, and the chatprompt lifecycle list. Coder Agents is experimental (coderd/x), and our rule for experimental persisted data is to drop the legacy shape outright and delete the code that existed only to support it, instead of carrying it forward.What happens to an old chat:
close_agentcall and its stored result, and the UI renders them as before.close_agentagain, nothing runs and the child keeps running. The tool result is the errorTool not active in this turn: close_agent, and the model can retry withinterrupt_agent.Why it is safe
TestExecuteSingleTool_ResolvesToolNameAlias, theAliasNamesClassifyAsCalledbilling row, the alias subtest ofTestPartitionAmbiguousToolCallsGatesOnBuiltins, and the alias loop inTestUnbilledSubagentToolNamesMatchCatalog.TestExecuteSingleTool_UnknownAliasFallsThroughis nowTestExecuteSingleTool_RejectsUnknownTooland asserts the exact error above. The observer tests call the tool by its real name.git grepfinds no backend use of the removed names. The frontend keeps itsclose_agentdisplay mapping, which old histories still use.Review focus
close_agentcall is now billed like any rejected tool call; the rejection is immediate.ARCHITECTURE.md, "Generation goroutine": this PR deletes the sub-bullet that calledclose_agenta deprecated alias ofinterrupt_agent.resolved_tool_namedebug log field is gone; it always equalledtool_name.Validation
go test ./coderd/x/chatd/... -count=1 -p 4 -parallel 4: okgo vetandgolangci-lint run --fix=false ./coderd/x/chatd/...: cleanStack
This stack retires chatd compatibility paths for chat data that older Coder versions stored.
close_agenttool alias (this PR)