Skip to content

feat: add unload on_agent_switch builtin hook#2684

Merged
dgageot merged 2 commits intodocker:mainfrom
dgageot:feat/unload-builtin-hook
May 7, 2026
Merged

feat: add unload on_agent_switch builtin hook#2684
dgageot merged 2 commits intodocker:mainfrom
dgageot:feat/unload-builtin-hook

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 7, 2026

Re-implementation of #2660 as a builtin hook instead of explicit runtime calls. Opened to compare the two designs side by side.

What

Adds an unload on_agent_switch builtin hook that asks every model on the previous agent to release its resources when the runtime hands control to another agent. Today only Docker Model Runner ships a provider.Unloader; everything else is silently skipped, so wiring the hook on a cross-provider chain is harmless.

Opt in via the standard hook YAML — no new flag, no auto-injection:

agents:
  coder:
    model: qwen3-large
    handoffs: [reviewer]
    hooks:
      on_agent_switch:
        - type: builtin
          command: unload
  reviewer:
    model: qwen3-coder
    handoffs: [coder]
    hooks:
      on_agent_switch:
        - type: builtin
          command: unload

DMR's _unload URL is derived from base_url (replacing the trailing /v1); a per-provider unload_api field overrides it for non-standard deployments. Errors are logged and swallowed, each call is bounded to 10 s — agent switching never blocks on a slow or unreachable engine.

How it compares to #2660

#2660 this PR
Trigger sites 3 explicit r.unloadOnSwitch(...) calls in agent_delegation.go and runtime.go 1 hook entry, dispatched by the existing on_agent_switch machinery
Discoverability private runtime method shows up next to cache_response, redact_secrets, … in the schema and docs
Composability hard-coded users can interleave their own on_agent_switch hooks
Runtime LOC ~40 ~25
Provider packages touched provider, dmr, openai, base provider, dmr

The single trade-off vs. #2660: LocalRuntime.SetCurrentAgent (the TUI agent picker) doesn't currently fire on_agent_switch hooks, so the hook-based version doesn't cover that path. Closing the gap is a one-line change in SetCurrentAgent that benefits every other on-switch hook too — happy to do it as a follow-up if we go with this approach.

Files

Feature

  • pkg/model/provider/provider.goUnloader interface
  • pkg/model/provider/dmr/unload.go (+ tests) — DMR Unload, default URL derivation, override resolution, HTTP POST
  • pkg/model/provider/defaults.go — plumb unload_api from ProviderConfig into the model's provider_opts
  • pkg/config/latest/types.go (+ tests) — ProviderConfig.UnloadAPI field, ModelConfig.UnloadAPI() accessor
  • pkg/runtime/unload.go (+ tests) — BuiltinUnload const + the closure registered on LocalRuntime
  • pkg/runtime/runtime.go — registration in NewLocalRuntime

Schema, docs, example

  • agent-schema.jsonunload_api field, unload builtin in the command-list blurb
  • docs/configuration/hooks/index.md — built-ins table row + on_agent_switch section
  • docs/providers/dmr/index.md — "Unloading models on agent switch" section, single-tenant warning callout
  • docs/providers/custom/index.mdunload_api row in the provider properties table
  • examples/unload_on_switch.yaml (+ examples/README.md row)

Other things in this branch

The third commit (fix lint: …) cleans up 29 pre-existing lint offences that were sitting in the tree (3 govet/inline, 20 modernize/slicesbackward, 6 Lint/SlogContextual). I noticed them while running task lint against my changes and fixed them in a separate commit — happy to split into a separate PR if preferred.

One thing worth flagging: golangci-lint --fix rewrote truncateOldToolContent in pkg/session/session.go from for i := ...; i-- to for _, v := range slices.Backward(...) and changed msg := &result[i] to msg := &v. The latter points to the loop-local copy, so msg.Content = … was silently mutating the copy and the function no longer truncated anything. I corrected it to for i := range slices.Backward(result) { msg := &result[i] }. Worth being aware of when running --fix elsewhere.

Validation

  • task build
  • task test ✅ (full suite)
  • task lint ✅ — 0 offences (tree was at 29 before)
  • New tests cover: builtin behaviour (happy path, error swallowing, same-agent no-op, registry lookup), DMR URL derivation/override, response body draining for connection reuse, unload_api plumbing through provider-config merge.

@dgageot dgageot requested a review from a team as a code owner May 7, 2026 08:53
@rumpl
Copy link
Copy Markdown
Member

rumpl commented May 7, 2026

There are a lot of unrelated changes in this PR and a lot of conflicts

@dgageot dgageot force-pushed the feat/unload-builtin-hook branch 2 times, most recently from 74a73aa to e1322e2 Compare May 7, 2026 13:11
@dgageot dgageot force-pushed the feat/unload-builtin-hook branch from e1322e2 to 09de0a9 Compare May 7, 2026 13:23
@dgageot dgageot merged commit cb208e4 into docker:main May 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants