Skip to content

refactor(coderd/x/chatd)!: retire the close_agent tool alias - #31003

Closed
ibetitsmike wants to merge 1 commit into
mike/chatd-retire-summary-limit-fallbackfrom
mike/chatd-retire-close-agent-alias
Closed

ibetitsmike wants to merge 1 commit into
mike/chatd-retire-summary-limit-fallbackfrom
mike/chatd-retire-close-agent-alias

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

This PR breaks compatibility with chat histories that contain close_agent tool calls, which chats from v2.34 and earlier have. #26673 (v2.35) renamed close_agent to interrupt_agent and 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.ToolNameAliases with its five resolution sites in chatloop, the alias step in builtin input validation, and the close_agent entries 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:

  • Replay does not change. chatprompt converts tool calls and results without looking up tool names, so the model still sees the historical close_agent call and its stored result, and the UI renders them as before.
  • If the model calls close_agent again, nothing runs and the child keeps running. The tool result is the error Tool not active in this turn: close_agent, and the model can retry with interrupt_agent.

Why it is safe

  • Deleted alias-only tests: TestExecuteSingleTool_ResolvesToolNameAlias, the AliasNamesClassifyAsCalled billing row, the alias subtest of TestPartitionAmbiguousToolCallsGatesOnBuiltins, and the alias loop in TestUnbilledSubagentToolNamesMatchCatalog.
  • TestExecuteSingleTool_UnknownAliasFallsThrough is now TestExecuteSingleTool_RejectsUnknownTool and asserts the exact error above. The observer tests call the tool by its real name.
  • Mutants of the active tool check, step observer notification, serial dispatch, the unbilled set, the plan policy and builtin input validation each fail a remaining test. Only the deleted alias test caught an empty name passed to the tool handler; no chatd tool reads that name (MCP tools use their original name).
  • git grep finds no backend use of the removed names. The frontend keeps its close_agent display mapping, which old histories still use.

Review focus

  • A rejected close_agent call is now billed like any rejected tool call; the rejection is immediate.
  • ARCHITECTURE.md, "Generation goroutine": this PR deletes the sub-bullet that called close_agent a deprecated alias of interrupt_agent.
  • The resolved_tool_name debug log field is gone; it always equalled tool_name.

Validation

  • go test ./coderd/x/chatd/... -count=1 -p 4 -parallel 4: ok
  • go vet and golangci-lint run --fix=false ./coderd/x/chatd/...: clean
  • pre-commit hook: passed

Stack

This stack retires chatd compatibility paths for chat data that older Coder versions stored.

  1. test(coderd): stamp chat message fixtures with the current content version #30996 stamp chat message fixtures with the current content version
  2. test(coderd/x/chatd/chatprompt): drop tests of the legacy content codec #30997 drop tests of the legacy content codec
  3. refactor(coderd)!: retire the V0 chat message content decoder #30998 retire the V0 chat message content decoder
  4. refactor(coderd/x/chatd): inline the model config provider identity #31000 inline the model config provider identity
  5. refactor(coderd/x/chatd)!: drop the provenance fallback for unstamped OpenAI reasoning #31001 drop the provenance fallback for unstamped OpenAI reasoning
  6. refactor(coderd/x/chatd)!: drop the context-limit fallback for old compaction summaries #31002 drop the context-limit fallback for old compaction summaries
  7. refactor(coderd/x/chatd)!: retire the close_agent tool alias #31003 retire the close_agent tool alias (this PR)

Xum, an AI agent, opened this PR for @ibetitsmike and wrote this description.

@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

Xum, an AI agent, posted this comment for @ibetitsmike.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T19:01:46.019780Z 96a2fe2 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 96a2fe22ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

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.

Xum, an AI agent, closed this PR for @ibetitsmike.

@github-actions github-actions Bot locked and limited conversation to collaborators Oct 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release/breaking This label is applied to PRs to detect breaking changes as part of the release process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant