diff --git a/cmd/root/run.go b/cmd/root/run.go index 02ef8cdc3f..b5efa8bd5d 100644 --- a/cmd/root/run.go +++ b/cmd/root/run.go @@ -20,6 +20,7 @@ import ( "github.com/docker/docker-agent/pkg/agent" "github.com/docker/docker-agent/pkg/app" boardtui "github.com/docker/docker-agent/pkg/board/tui" + "github.com/docker/docker-agent/pkg/bootstrap" "github.com/docker/docker-agent/pkg/cli" "github.com/docker/docker-agent/pkg/config" latestcfg "github.com/docker/docker-agent/pkg/config/latest" @@ -928,34 +929,14 @@ func (f *runExecFlags) loadAgentFrom(ctx context.Context, req runtime.LoadTeamRe // current agent name are passed in because they're resolved by callers from // different sources (e.g. the spawner uses the same store as the parent). func (f *runExecFlags) runtimeOpts(loadResult *teamloader.LoadResult, runConfig *config.RuntimeConfig, sessStore session.Store, agentName string) []runtime.Opt { - modelSwitcherCfg := &runtime.ModelSwitcherConfig{ - Models: loadResult.Models, - Providers: loadResult.Providers, - ModelsGateway: runConfig.ModelsGateway, - EncryptedConfig: loadResult.EncryptedConfig, - EnvProvider: runConfig.EnvProvider(), - ProviderRegistry: loadResult.ProviderRegistry, - AgentDefaultModels: loadResult.AgentDefaultModels, - } - // Share the models.dev store the team loader already warmed (parsing the - // multi-MB catalog once) so the runtime doesn't build its own cold store - // and re-pay the parse on the first /model open. On error we leave it unset - // and the runtime falls back to its lazy default. - if store, err := runConfig.ModelsDevStore(); err == nil { - modelSwitcherCfg.ModelsStore = store - } else { - slog.Warn("Failed to obtain shared models.dev store; runtime will use its own", "error", err) - } + loadedOpts := bootstrap.RuntimeOpts(loadResult, runConfig) opts := []runtime.Opt{ runtime.WithSessionStore(sessStore), runtime.WithCurrentAgent(agentName), runtime.WithWorkingDir(runConfig.WorkingDir), runtime.WithTracer(otel.Tracer(AppName)), - runtime.WithModelSwitcherConfig(modelSwitcherCfg), - runtime.WithBudget(loadResult.Budget), - runtime.WithNamedBudgets(loadResult.Budgets, loadResult.AgentBudgets), } - return opts + return append(opts, loadedOpts...) } // snapshotRuntimeOpts wires the snapshot builtin into a runtime. diff --git a/docs/guides/go-sdk/index.md b/docs/guides/go-sdk/index.md index c6bb6a0b09..533ec493a8 100644 --- a/docs/guides/go-sdk/index.md +++ b/docs/guides/go-sdk/index.md @@ -93,6 +93,47 @@ These entry points exclude the application, CLI, MCP toolset implementation, and JavaScript engine. Runtime event types still retain MCP protocol support and the runtime's own builtin tools; this is not a runtime or remote-session refactor. +## Explicit Loader Defaults + +`pkg/teamloader/defaults.LoaderOpts()` supplies the full provider, toolset and +source registries without registering runtime factories. Enable optional +execution features on each runtime with +`runtime.WithHarnessFactory(codingharness.Factory)` and +`runtime.WithCommandEvaluatorFactory(jscommands.Factory)`. + +The existing `loaderdefaults.Opts()` remains a compatibility helper: it returns +those loader options and also registers the harness and JavaScript factories +globally. Both helpers link the full defaults; dependency-light embedders should +continue supplying their own registries. + +## Loaded Configuration to Runtime + +`pkg/bootstrap.RuntimeOpts(loaded, runConfig)` translates a +`teamloader.LoadResult` into model-switching and budget options. It reuses the +shared model catalog and the encrypted configuration resolved during loading. +It does not load a team, create a session, register factories or own resources. + +```go +opts := bootstrap.RuntimeOpts(loaded, runConfig) +opts = append(opts, + runtime.WithSessionStore(store), + runtime.WithCurrentAgent(agentName), + runtime.WithWorkingDir(workingDir), + runtime.WithHarnessFactory(codingharness.Factory), + runtime.WithCommandEvaluatorFactory(jscommands.Factory), +) +rt, err := runtime.New(ctx, loaded.Team, opts...) +``` + +Environment and catalog initialization happen when `RuntimeOpts` is called. +Catalog initialization failures retain the runtime's lazy fallback. Later +options can override the assembly defaults; callers still select interaction +policy, tracing, storage and cleanup. In particular, callers own the supplied +session store, and a shared team's toolsets must outlive all its runtimes. + +The CLI uses this helper for both its initial runtime and TUI-spawned runtimes. +Serving adapters retain their existing wiring and budget behavior. + ## Headless Embedded Chat (`pkg/embeddedchat`) `pkg/embeddedchat` is a thin wrapper around the Docker Agent runtime that lets you drive an agent from your own UI instead of running Docker Agent's Bubble Tea application. It handles runtime construction, event projection, and conversation state, exposing a simple `Send` / `Confirm` / `Restart` / `Close` API. diff --git a/e2e/dependencies_test.go b/e2e/dependencies_test.go index 51c60cebc6..35ee0f960e 100644 --- a/e2e/dependencies_test.go +++ b/e2e/dependencies_test.go @@ -94,6 +94,7 @@ func TestDependencies(t *testing.T) { deps := listTransitiveDeps(t, "./pkg/config", "./pkg/teamloader", + "./pkg/bootstrap", "./pkg/embeddedchat", "./pkg/tools/builtin/api/client", "./examples/golibrary/leanapi", diff --git a/pkg/bootstrap/bootstrap.go b/pkg/bootstrap/bootstrap.go new file mode 100644 index 0000000000..1c4c00b21c --- /dev/null +++ b/pkg/bootstrap/bootstrap.go @@ -0,0 +1,36 @@ +// Package bootstrap translates loaded agent configuration into runtime options. +// Callers retain ownership of session stores, interaction policy and lifetimes. +package bootstrap + +import ( + "log/slog" + + "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/teamloader" +) + +// RuntimeOpts carries model-switching configuration and budgets into a runtime. +// It resolves the shared environment and model store when called, not when applied. +func RuntimeOpts(loaded *teamloader.LoadResult, runConfig *config.RuntimeConfig) []runtime.Opt { + modelSwitcherCfg := &runtime.ModelSwitcherConfig{ + Models: loaded.Models, + Providers: loaded.Providers, + ModelsGateway: runConfig.ModelsGateway, + EncryptedConfig: loaded.EncryptedConfig, + EnvProvider: runConfig.EnvProvider(), + ProviderRegistry: loaded.ProviderRegistry, + AgentDefaultModels: loaded.AgentDefaultModels, + } + // Reuse the loader's warmed catalog; a failed lookup keeps the runtime's lazy fallback. + if store, err := runConfig.ModelsDevStore(); err == nil { + modelSwitcherCfg.ModelsStore = store + } else { + slog.Warn("Failed to obtain shared models.dev store; runtime will use its own", "error", err) + } + return []runtime.Opt{ + runtime.WithModelSwitcherConfig(modelSwitcherCfg), + runtime.WithBudget(loaded.Budget), + runtime.WithNamedBudgets(loaded.Budgets, loaded.AgentBudgets), + } +} diff --git a/pkg/bootstrap/bootstrap_test.go b/pkg/bootstrap/bootstrap_test.go new file mode 100644 index 0000000000..4abaf1eb3f --- /dev/null +++ b/pkg/bootstrap/bootstrap_test.go @@ -0,0 +1,213 @@ +package bootstrap + +import ( + "context" + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/environment" + "github.com/docker/docker-agent/pkg/model/provider" + "github.com/docker/docker-agent/pkg/model/provider/base" + "github.com/docker/docker-agent/pkg/model/provider/options" + "github.com/docker/docker-agent/pkg/modelsdev" + "github.com/docker/docker-agent/pkg/paths" + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/team" + "github.com/docker/docker-agent/pkg/teamloader" + "github.com/docker/docker-agent/pkg/tools" +) + +type testProvider struct { + base.Config +} + +func (testProvider) CreateChatCompletionStream(context.Context, []chat.Message, []tools.Tool) (chat.MessageStream, error) { + return &testStream{}, nil +} + +func (testProvider) MaxTokens() int { return 0 } + +type testStream struct { + done bool +} + +func (s *testStream) Recv() (chat.MessageStreamResponse, error) { + if s.done { + return chat.MessageStreamResponse{}, io.EOF + } + s.done = true + return chat.MessageStreamResponse{ + Choices: []chat.MessageStreamChoice{{Delta: chat.MessageDelta{Content: "hello"}}}, + Usage: &chat.Usage{InputTokens: 3, OutputTokens: 2}, + }, nil +} + +func (*testStream) Close() {} + +func testLoaded() *teamloader.LoadResult { + p := &testProvider{Config: base.Config{ModelConfig: latest.ModelConfig{Provider: "test", Model: "model"}}} + return &teamloader.LoadResult{ + Team: team.New(team.WithAgents(agent.New("root", "", agent.WithModel(p)))), + } +} + +func TestRuntimeOptsModelConfiguration(t *testing.T) { + t.Parallel() + + loaded := testLoaded() + loaded.Models = map[string]latest.ModelConfig{"alternate": {Provider: "test", Model: "alternate"}} + loaded.Providers = map[string]latest.ProviderConfig{"custom": {}} + loaded.AgentDefaultModels = map[string]string{"root": "alternate"} + loaded.EncryptedConfig = "loaded-encrypted-config" + env := environment.NewMapEnvProvider(map[string]string{"TEST_KEY": "value"}) + store := modelsdev.NewDatabaseStore(&modelsdev.Database{}) + runConfig := &config.RuntimeConfig{ + Config: config.Config{ModelsGateway: "https://gateway.example", EncryptedConfig: "unresolved-config"}, + EnvProviderOverride: env, + ModelsDevStoreOverride: store, + } + calls := 0 + loaded.ProviderRegistry = provider.NewRegistry(map[string]provider.Factory{ + "test": func(_ context.Context, cfg *latest.ModelConfig, gotEnv environment.Provider, opts ...options.Opt) (provider.Provider, error) { + calls++ + assert.Equal(t, loaded.Models["alternate"].Model, cfg.Model) + assert.Same(t, env, gotEnv) + gotOpts := options.Apply(opts...) + assert.Equal(t, runConfig.ModelsGateway, gotOpts.Gateway()) + assert.Equal(t, loaded.EncryptedConfig, gotOpts.EncryptedConfig()) + assert.Equal(t, loaded.Providers, gotOpts.Providers()) + assert.Same(t, store, gotOpts.ModelsDevStore()) + return &testProvider{Config: base.Config{ModelConfig: *cfg}}, nil + }, + }) + + opts := RuntimeOpts(loaded, runConfig) + assert.Zero(t, calls, "assembly must not construct model clients") + rt, err := runtime.New(t.Context(), loaded.Team, opts...) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + assert.True(t, rt.SupportsModelSwitching()) + require.NoError(t, rt.SetAgentModel(t.Context(), "root", "alternate")) + assert.Equal(t, 1, calls) + require.NoError(t, rt.SetAgentModel(t.Context(), "root", "")) + assert.Equal(t, 1, calls, "clearing an override must reuse the original model") + ag, err := loaded.Team.DefaultAgent() + require.NoError(t, err) + assert.Equal(t, "test/model", ag.Model(t.Context()).ID().String()) +} + +func TestRuntimeOptsBudgets(t *testing.T) { + t.Parallel() + for _, name := range []string{"run", "shared", "unbudgeted"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + loaded := testLoaded() + switch name { + case "run": + loaded.Budget = &latest.BudgetConfig{MaxTokens: 10} + case "shared": + loaded.Budgets = map[string]latest.BudgetConfig{"shared": {MaxTokens: 10}, "unused": {MaxTokens: 1}} + loaded.AgentBudgets = map[string][]string{"root": {"shared"}} + } + runConfig := &config.RuntimeConfig{ + EnvProviderOverride: environment.NewMapEnvProvider(nil), + ModelsDevStoreOverride: modelsdev.NewDatabaseStore(&modelsdev.Database{}), + } + rt, err := runtime.New(t.Context(), loaded.Team, RuntimeOpts(loaded, runConfig)...) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + + sess := session.New(session.WithUserMessage("hello")) + var statuses []runtime.BudgetStatus + for event := range rt.RunStream(t.Context(), sess) { + if usage, ok := event.(*runtime.BudgetUsageEvent); ok { + statuses = usage.Budgets + } + } + assert.Equal(t, "hello", sess.GetLastAssistantMessageContent()) + if name == "unbudgeted" { + assert.Empty(t, statuses) + return + } + require.Len(t, statuses, 1) + assert.Equal(t, name, statuses[0].Name) + assert.Equal(t, int64(10), statuses[0].MaxTokens) + assert.Equal(t, int64(5), statuses[0].Tokens) + }) + } +} + +func TestRuntimeOptsCallerOverrides(t *testing.T) { + t.Parallel() + loaded := testLoaded() + loaded.Models = map[string]latest.ModelConfig{"alternate": {Provider: "test", Model: "alternate"}} + overrideStore := modelsdev.NewDatabaseStore(&modelsdev.Database{}) + called := false + loaded.ProviderRegistry = provider.NewRegistry(map[string]provider.Factory{ + "test": func(_ context.Context, cfg *latest.ModelConfig, _ environment.Provider, opts ...options.Opt) (provider.Provider, error) { + called = true + gotOpts := options.Apply(opts...) + assert.Same(t, overrideStore, gotOpts.ModelsDevStore()) + return &testProvider{Config: base.Config{ModelConfig: *cfg}}, nil + }, + }) + runConfig := &config.RuntimeConfig{ + EnvProviderOverride: environment.NewMapEnvProvider(nil), + ModelsDevStoreOverride: modelsdev.NewDatabaseStore(&modelsdev.Database{}), + } + opts := RuntimeOpts(loaded, runConfig) + store := session.NewInMemorySessionStore() + opts = append(opts, runtime.WithModelStore(overrideStore), runtime.WithSessionStore(store)) + rt, err := runtime.New(t.Context(), loaded.Team, opts...) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + assert.Same(t, store, rt.SessionStore()) + require.NoError(t, rt.SetAgentModel(t.Context(), "root", "alternate")) + assert.True(t, called) +} + +func TestRuntimeOptsResolvesEnvironmentWhenAssembled(t *testing.T) { + t.Parallel() + workingDir := t.TempDir() + file := filepath.Join(workingDir, "test.env") + require.NoError(t, os.WriteFile(file, []byte("TEST_KEY=before\n"), 0o600)) + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: workingDir, EnvFiles: []string{"test.env"}}, + ModelsDevStoreOverride: modelsdev.NewDatabaseStore(&modelsdev.Database{}), + } + loaded := testLoaded() + opts := RuntimeOpts(loaded, runConfig) + require.NoError(t, os.WriteFile(file, []byte("TEST_KEY=after\n"), 0o600)) + rt, err := runtime.New(t.Context(), loaded.Team, opts...) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + value, ok := runConfig.EnvProvider().Get(t.Context(), "TEST_KEY") + require.True(t, ok) + assert.Equal(t, "before", value) +} + +func TestRuntimeOptsModelStoreFailureFallsBack(t *testing.T) { + // Cache-dir overrides are process-global. + original := paths.GetCacheDir() + paths.SetCacheDir(filepath.Join(t.TempDir(), "file")) + t.Cleanup(func() { paths.SetCacheDir(original) }) + require.NoError(t, os.WriteFile(paths.GetCacheDir(), nil, 0o600)) + loaded := testLoaded() + runConfig := &config.RuntimeConfig{EnvProviderOverride: environment.NewMapEnvProvider(nil)} + opts := RuntimeOpts(loaded, runConfig) + _, err := runConfig.ModelsDevStore() + require.Error(t, err) + rt, err := runtime.New(t.Context(), loaded.Team, opts...) + require.NoError(t, err, "failed catalog initialization must not prevent runtime construction") + t.Cleanup(func() { require.NoError(t, rt.Close()) }) +} diff --git a/pkg/chatserver/agent.go b/pkg/chatserver/agent.go index 2172abccf9..e9f597f46c 100644 --- a/pkg/chatserver/agent.go +++ b/pkg/chatserver/agent.go @@ -188,6 +188,13 @@ func appendLatestUser(sess *session.Session, msgs []ChatCompletionMessage) bool return false } +// runtimeRunner is the execution and interaction contract the chat adapter needs. +type runtimeRunner interface { + RunStream(ctx context.Context, sess *session.Session) <-chan runtime.Event + Resume(ctx context.Context, req runtime.ResumeRequest) + ResumeElicitation(ctx context.Context, action tools.ElicitationAction, content map[string]any, elicitationID ...string) error +} + // agentEmit collects the side-effect callbacks invoked by runAgentLoop as // it drives the runtime. All callbacks are optional; nil means "ignore // this kind of event". @@ -211,7 +218,7 @@ type agentEmit struct { // All ErrorEvents seen in the run are joined into the returned error so // callers can see the full picture; the loop keeps draining until the // stream closes so the runtime can shut down cleanly. -func runAgentLoop(ctx context.Context, rt runtime.Runtime, sess *session.Session, emit agentEmit) error { +func runAgentLoop(ctx context.Context, rt runtimeRunner, sess *session.Session, emit agentEmit) error { var runErrs []error toolIndex := 0 for ev := range rt.RunStream(ctx, sess) { diff --git a/pkg/chatserver/agent_test.go b/pkg/chatserver/agent_test.go new file mode 100644 index 0000000000..6b98fdb4e2 --- /dev/null +++ b/pkg/chatserver/agent_test.go @@ -0,0 +1,111 @@ +package chatserver + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/tools" +) + +type stubRunner struct { + events []runtime.Event + onRun func(context.Context, *session.Session) + resumes []runtime.ResumeRequest + elicitationIDs []string + elicitationReply tools.ElicitationAction + elicitationData map[string]any + elicitationErr error +} + +func (r *stubRunner) RunStream(ctx context.Context, sess *session.Session) <-chan runtime.Event { + if r.onRun != nil { + r.onRun(ctx, sess) + } + events := make(chan runtime.Event, len(r.events)) + for _, event := range r.events { + events <- event + } + close(events) + return events +} + +func (r *stubRunner) Resume(_ context.Context, req runtime.ResumeRequest) { + r.resumes = append(r.resumes, req) +} + +func (r *stubRunner) ResumeElicitation(_ context.Context, action tools.ElicitationAction, content map[string]any, ids ...string) error { + r.elicitationReply = action + r.elicitationData = content + r.elicitationIDs = append(r.elicitationIDs, ids...) + return r.elicitationErr +} + +func TestRunAgentLoopForwardsEvents(t *testing.T) { + t.Parallel() + first := tools.ToolCall{ID: "first", Type: "function", Function: tools.FunctionCall{Name: "read_file", Arguments: `{"path":"test.txt"}`}} + second := tools.ToolCall{ID: "second", Type: "function", Function: tools.FunctionCall{Name: "think", Arguments: `{}`}} + runner := &stubRunner{events: []runtime.Event{ + runtime.AgentChoice("root", "session", "hello"), + runtime.ToolCall(first, tools.Tool{}, "root"), + runtime.AgentChoice("root", "session", " world"), + runtime.ToolCall(second, tools.Tool{}, "root"), + }} + sess := session.New() + runner.onRun = func(ctx context.Context, got *session.Session) { + assert.Equal(t, t.Context(), ctx) + assert.Same(t, sess, got) + } + var content []string + var calls []ToolCallReference + err := runAgentLoop(t.Context(), runner, sess, agentEmit{ + onContent: func(delta string) { content = append(content, delta) }, + onToolCall: func(call ToolCallReference) { calls = append(calls, call) }, + }) + require.NoError(t, err) + assert.Equal(t, []string{"hello", " world"}, content) + assert.Equal(t, []ToolCallReference{ + {Index: 0, ID: "first", Type: "function", Function: ToolCallFunction{Name: "read_file", Arguments: first.Function.Arguments}}, + {Index: 1, ID: "second", Type: "function", Function: ToolCallFunction{Name: "think", Arguments: second.Function.Arguments}}, + }, calls) +} + +func TestRunAgentLoopRespondsToInteraction(t *testing.T) { + t.Parallel() + runner := &stubRunner{events: []runtime.Event{ + &runtime.ToolCallConfirmationEvent{}, + &runtime.ElicitationRequestEvent{ElicitationID: "request-id", ServerElicitationID: "server-id"}, + &runtime.MaxIterationsReachedEvent{}, + }, elicitationErr: errors.New("response failed")} + + require.NoError(t, runAgentLoop(t.Context(), runner, session.New(), agentEmit{})) + assert.Equal(t, []runtime.ResumeRequest{runtime.ResumeApprove(), runtime.ResumeReject("")}, runner.resumes) + assert.Equal(t, tools.ElicitationActionDecline, runner.elicitationReply) + assert.Nil(t, runner.elicitationData) + assert.Equal(t, []string{"request-id"}, runner.elicitationIDs) +} + +func TestRunAgentLoopDrainsAfterErrorsAndCancellation(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + runner := &stubRunner{events: []runtime.Event{ + &runtime.ErrorEvent{Error: "first failure"}, + runtime.AgentChoice("root", "session", "cancel"), + &runtime.ErrorEvent{Error: "second failure"}, + runtime.AgentChoice("root", "session", "drained"), + }} + var content []string + err := runAgentLoop(ctx, runner, session.New(), agentEmit{onContent: func(delta string) { + content = append(content, delta) + cancel() + }}) + require.EqualError(t, err, "first failure\nsecond failure") + assert.Equal(t, []string{"cancel", "drained"}, content) + assert.ErrorIs(t, ctx.Err(), context.Canceled) +} diff --git a/pkg/chatserver/runtime_pool.go b/pkg/chatserver/runtime_pool.go index 7fb0f0b874..e2a8a1949a 100644 --- a/pkg/chatserver/runtime_pool.go +++ b/pkg/chatserver/runtime_pool.go @@ -12,7 +12,7 @@ import ( "github.com/docker/docker-agent/pkg/version" ) -// runtimePool keeps a small set of `runtime.Runtime` instances ready for +// runtimePool keeps a small set of `runtimeRunner` instances ready for // reuse, keyed by agent name. Building a runtime is non-trivial (it // resolves the agent's tools, creates per-agent hook executors, sets up // channels for resume/elicitation), so reusing the work across requests @@ -32,7 +32,7 @@ type runtimePool struct { maxIdle int mu sync.Mutex - idle map[string]chan runtime.Runtime + idle map[string]chan runtimeRunner } // errInvalidRuntime is returned when a caller asks for a runtime for an @@ -49,13 +49,13 @@ func newRuntimePool(ctx context.Context, t *team.Team, maxIdle int) *runtimePool team: t, ctx: func() context.Context { return context.WithoutCancel(ctx) }, maxIdle: maxIdle, - idle: make(map[string]chan runtime.Runtime), + idle: make(map[string]chan runtimeRunner), } } // Get returns a ready-to-use runtime for the given agent, either // recycled from the pool or freshly created. -func (p *runtimePool) Get(agent string) (runtime.Runtime, error) { +func (p *runtimePool) Get(agent string) (runtimeRunner, error) { if p == nil { return nil, errInvalidRuntime } @@ -79,7 +79,7 @@ func (p *runtimePool) Get(agent string) (runtime.Runtime, error) { // slot is full the runtime is discarded (not closed: the team owns the // underlying toolsets). The runtime must not be used by the caller // after Put returns. -func (p *runtimePool) Put(agent string, rt runtime.Runtime) { +func (p *runtimePool) Put(agent string, rt runtimeRunner) { if p == nil || rt == nil || p.maxIdle == 0 { return } @@ -92,7 +92,7 @@ func (p *runtimePool) Put(agent string, rt runtime.Runtime) { } } -func (p *runtimePool) takeIdle(agent string) runtime.Runtime { +func (p *runtimePool) takeIdle(agent string) runtimeRunner { p.mu.Lock() ch, ok := p.idle[agent] p.mu.Unlock() @@ -107,12 +107,12 @@ func (p *runtimePool) takeIdle(agent string) runtime.Runtime { } } -func (p *runtimePool) channelFor(agent string) chan runtime.Runtime { +func (p *runtimePool) channelFor(agent string) chan runtimeRunner { p.mu.Lock() defer p.mu.Unlock() ch, ok := p.idle[agent] if !ok { - ch = make(chan runtime.Runtime, p.maxIdle) + ch = make(chan runtimeRunner, p.maxIdle) p.idle[agent] = ch } return ch diff --git a/pkg/chatserver/runtime_pool_test.go b/pkg/chatserver/runtime_pool_test.go index a144a7bd2e..f79b54724a 100644 --- a/pkg/chatserver/runtime_pool_test.go +++ b/pkg/chatserver/runtime_pool_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRuntimePool_DisabledIsNotCached(t *testing.T) { @@ -22,6 +23,20 @@ func TestRuntimePool_NegativeCapTreatedAsZero(t *testing.T) { assert.Equal(t, 0, p.maxIdle) } +func TestRuntimePoolReusesRunner(t *testing.T) { + t.Parallel() + p := newRuntimePool(t.Context(), nil, 1) + first, second := &stubRunner{}, &stubRunner{} + p.Put("root", first) + p.Put("root", second) + + runner, err := p.Get("root") + require.NoError(t, err) + assert.Same(t, first, runner, "a full pool must retain the first runner") + assert.Nil(t, p.takeIdle("root"), "acquired runners must leave the pool") + assert.Nil(t, p.takeIdle("other"), "runners must remain agent-scoped") +} + func TestRuntimePool_takeIdleNoChannel(t *testing.T) { t.Parallel() p := newRuntimePool(t.Context(), nil, 4) diff --git a/pkg/chatserver/server.go b/pkg/chatserver/server.go index 2f7e8ab6c3..91aa6a3419 100644 --- a/pkg/chatserver/server.go +++ b/pkg/chatserver/server.go @@ -40,7 +40,6 @@ import ( "github.com/docker/docker-agent/pkg/config/sources" "github.com/docker/docker-agent/pkg/echolog" "github.com/docker/docker-agent/pkg/httpsec" - "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/servesafety" "github.com/docker/docker-agent/pkg/session" "github.com/docker/docker-agent/pkg/team" @@ -459,7 +458,7 @@ func (s *server) commitConversation(id string, sess *session.Session, runErr err // error (nil on success) so the caller can decide whether to commit the // conversation; the HTTP response — success or error envelope — is always // written here. -func (s *server) chatCompletion(c echo.Context, rt runtime.Runtime, sess *session.Session, model string) error { +func (s *server) chatCompletion(c echo.Context, rt runtimeRunner, sess *session.Session, model string) error { var toolCalls []ToolCallReference emit := agentEmit{ onToolCall: func(tc ToolCallReference) { @@ -496,7 +495,7 @@ func (s *server) chatCompletion(c echo.Context, rt runtime.Runtime, sess *sessio // whether to commit the conversation. The error is *also* reported in-band // as an SSE error event, so the HTTP handler itself still returns nil; the // return value here exists purely to drive the commit decision. -func (s *server) streamChatCompletion(c echo.Context, rt runtime.Runtime, sess *session.Session, model string, includeUsage bool) error { +func (s *server) streamChatCompletion(c echo.Context, rt runtimeRunner, sess *session.Session, model string, includeUsage bool) error { stream := newSSEStream(c.Response(), newChatID(), model) // Initial "role: assistant" delta so clients can start rendering. diff --git a/pkg/teamloader/defaults/defaults.go b/pkg/teamloader/defaults/defaults.go index 5ed11a387e..5555e9bd94 100644 --- a/pkg/teamloader/defaults/defaults.go +++ b/pkg/teamloader/defaults/defaults.go @@ -20,12 +20,16 @@ import ( "github.com/docker/docker-agent/pkg/tools/toon" ) +// Opts returns loader defaults and registers global runtime factories for compatibility. +// Use LoaderOpts with explicit runtime factories for independent runtimes. func Opts() []teamloader.Opt { - // ${...} JavaScript expressions in slash-command instructions and - // `harness:` agents are runtime features; enable them alongside the - // loader's own. jscommands.Register() runtime.RegisterHarness(codingharness.Factory) + return LoaderOpts() +} + +// LoaderOpts returns loader defaults without registering runtime factories. +func LoaderOpts() []teamloader.Opt { return []teamloader.Opt{ teamloader.WithToolsetRegistry(toolsets.NewDefaultToolsetRegistry()), teamloader.WithProviderRegistry(providers.NewDefaultRegistry()), diff --git a/pkg/teamloader/defaults/defaults_test.go b/pkg/teamloader/defaults/defaults_test.go new file mode 100644 index 0000000000..9ca561d364 --- /dev/null +++ b/pkg/teamloader/defaults/defaults_test.go @@ -0,0 +1,107 @@ +package defaults + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/config/types" + "github.com/docker/docker-agent/pkg/environment" + "github.com/docker/docker-agent/pkg/harness" + "github.com/docker/docker-agent/pkg/modelsdev" + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/team" + "github.com/docker/docker-agent/pkg/teamloader" + "github.com/docker/docker-agent/pkg/tools" +) + +type commandEvaluator struct{} + +func (commandEvaluator) Evaluate(context.Context, string, []string) string { + return "custom evaluator" +} + +func TestLoaderOptsPreservesGlobalFactories(t *testing.T) { + // Global factories are restored before parallel tests run. + t.Cleanup(func() { Opts() }) + runtime.RegisterCommandEvaluator(func([]tools.Tool) runtime.CommandEvaluator { + return commandEvaluator{} + }) + harnessCalls := 0 + runtime.RegisterHarness(func(*latest.HarnessConfig) (harness.Provider, error) { + harnessCalls++ + return nil, runtime.ErrHarnessNotRegistered + }) + + ag := agent.New("root", "", agent.WithHarness(&latest.HarnessConfig{Type: "codex"}), agent.WithCommands(types.Commands{ + "test": {Instruction: "${args[0]}"}, + })) + rt, err := runtime.New(t.Context(), team.New(team.WithAgents(ag))) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + + LoaderOpts() + assert.Zero(t, harnessCalls, "loader options must not invoke runtime factories") + assert.Equal(t, "custom evaluator", runtime.ResolveCommand(t.Context(), rt, "/test hello")) + + _, err = rt.Run(t.Context(), session.New(session.WithUserMessage("hello"))) + require.Error(t, err) + assert.Equal(t, 1, harnessCalls, "loader defaults must not replace the harness factory") +} + +func TestOptsRegistersCommandEvaluator(t *testing.T) { + t.Cleanup(func() { Opts() }) + runtime.RegisterCommandEvaluator(func([]tools.Tool) runtime.CommandEvaluator { + return commandEvaluator{} + }) + + harnessCalls := 0 + runtime.RegisterHarness(func(*latest.HarnessConfig) (harness.Provider, error) { + harnessCalls++ + return nil, runtime.ErrHarnessNotRegistered + }) + ag := agent.New("root", "", agent.WithHarness(&latest.HarnessConfig{Type: "unknown"}), agent.WithCommands(types.Commands{ + "test": {Instruction: "${args[0]}"}, + })) + rt, err := runtime.New(t.Context(), team.New(team.WithAgents(ag))) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, rt.Close()) }) + + Opts() + assert.Equal(t, "hello", runtime.ResolveCommand(t.Context(), rt, "/test hello")) + _, err = rt.Run(t.Context(), session.New(session.WithUserMessage("hello"))) + require.ErrorContains(t, err, "unknown") + assert.Zero(t, harnessCalls, "legacy options must reinstall the default harness factory") +} + +func TestLoaderOptsLoadsDefaultCapabilities(t *testing.T) { + t.Parallel() + + source := config.NewBytesSource("test", []byte(`agents: + root: + model: openai/gpt-5-mini + instruction: A test agent + toolsets: + - type: think +`)) + runConfig := &config.RuntimeConfig{ + EnvProviderOverride: environment.NewMapEnvProvider(map[string]string{"OPENAI_API_KEY": "test-key"}), + ModelsDevStoreOverride: modelsdev.NewDatabaseStore(&modelsdev.Database{}), + } + loaded, err := teamloader.LoadWithConfig(t.Context(), source, runConfig, LoaderOpts()...) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, loaded.Team.StopToolSets(t.Context())) }) + + ag, err := loaded.Team.DefaultAgent() + require.NoError(t, err) + assert.Equal(t, "root", ag.Name()) + agentTools, err := ag.Tools(t.Context()) + require.NoError(t, err) + require.NotEmpty(t, agentTools) +}