feat(teamloader): add WithModelOptions passthrough for provider-agnostic HTTP transport wrapping#3549
Conversation
…tic HTTP transport wrapping Load/LoadWithConfig had no way to thread options.WithHTTPTransportWrapper (docker#3089) into the model clients teamloader constructs internally (primary, fallback, title, and compaction models, plus models loaded for external OCI/URL-referenced sub-agents). WithHTTPTransportWrapper already lets an embedder inject auth into anthropic/openai/gemini clients built directly (internal/gordon in agentic-platform does this), but any caller going through teamloader.Load — which resolves providers from a cagent.yml, so the caller can't build clients directly — had no equivalent hook. Adds WithModelOptions(...options.Opt), appended after teamloader's own built-in opts at all 4 provider-construction call sites and threaded into loadExternalAgent so external sub-agents inherit it too. Adds an end-to-end regression test: loads a team from an inline cagent config with WithModelOptions(options.WithHTTPTransportWrapper(...)), drives the resulting agent's model through a real (httptest) streaming call, and asserts the wrapper's RoundTrip was invoked — proving the opt reaches the actual HTTP client, not just that it compiles.
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The WithModelOptions passthrough is mechanically correct. The new modelOpts []options.Opt field is safely threaded through all four provider-construction sites (getModelsForAgent, getFallbackModelsForAgent, getTitleModelForAgent, getCompactionModelForAgent) and into loadExternalAgent. Option ordering (caller opts appended after built-ins) is intentional and correctly documented. Nil/empty-slice edge cases are safe — variadic append(opts, modelOpts...) on an empty/nil slice is a no-op in Go.
The regression test (TestWithModelOptions_TransportWrapperReachesAgentModel) correctly validates end-to-end that the wrapper reaches the actual HTTP client: the countingTransport closure is invoked once at provider construction time (the documented semantics of WithHTTPTransportWrapper), so counter.base is always populated before any RoundTrip call. The httptest-driven streaming path and assertion on counter.calls.Load() are sound.
No bugs were found in the changes introduced by this PR.
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The WithModelOptions passthrough is correctly implemented: caller-supplied opts are appended after teamloader's built-in opts at all four provider-construction sites (getModelsForAgent, getFallbackModelsForAgent, getTitleModelForAgent, getCompactionModelForAgent) and correctly threaded through loadExternalAgent for external sub-agents. The append semantics are sound — no aliasing hazards. The end-to-end regression test exercises the full Load → agent → provider → HTTP path and correctly asserts that the transport wrapper is invoked. No bugs introduced by this PR were confirmed.
aheritier
added
area/models
4 participants
🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer
Problem
teamloader.Load/LoadWithConfighas no way for a caller to inject a customoptions.Optinto the model clients it constructs internally. In particular,options.WithHTTPTransportWrapper(#3089) already lets a caller wrap the HTTP transport used by anthropic/openai/gemini provider clients — e.g. to inject a bearer token provider-agnostically, decoupled fromenvironment.IsTrustedDockerURL— but that only works today for callers who build a provider client directly (e.g.agentic-platform'sinternal/gordon). Any caller going throughteamloader.Load(which resolves providers dynamically from a cagent.yml config, so it can't build clients directly) has no equivalent hook.This surfaced while investigating a credential-injection gap in
agentic-platform's autonomous sandbox-runner sessions (tracked there as APT-632): the runner usesteamloader.Load, so it can't currently apply a transport wrapper for gateway auth across arbitrary configured providers.Change
WithModelOptions(opts ...options.Opt) Opt, appended after teamloader's own built-in opts (options.WithGateway,options.WithStructuredOutput, etc.) at all 4 internal provider-construction sites (getModelsForAgent,getFallbackModelsForAgent,getTitleModelForAgent,getCompactionModelForAgent), so caller-supplied opts take precedence for anything both sides set.loadExternalAgentso external (OCI/URL-referenced) sub-agents inherit it too.TestWithModelOptions_TransportWrapperReachesAgentModel): loads a team from an inline cagent config viaWithModelOptions(options.WithHTTPTransportWrapper(...)), drives the resulting agent's model through a realhttpteststreaming call, and asserts the wrapper'sRoundTripwas invoked — proving the opt reaches the actual HTTP client, not just that it compiles. (Confirmed by reverting onlyteamloader.go: the test then fails to compile withundefined: WithModelOptions.)Testing
go build ./pkg/teamloader/...,go vet ./pkg/teamloader/...,go test ./pkg/teamloader/...— all pass.go mod tidy --diff— empty.go run ./lint .— no offenses (1491 files).golangci-lint v2.12.2 run ./pkg/teamloader/...— 0 issues.