diff --git a/pkg/app/app.go b/pkg/app/app.go index a5aa07a614..4fc895c97e 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -36,6 +36,10 @@ import ( "github.com/docker/docker-agent/pkg/tui/messages" ) +type titleGenerator interface { + Generate(ctx context.Context, sessionID string, userMessages []string) (string, error) +} + type App struct { operation atomic.Uint64 ctx func() context.Context @@ -54,7 +58,7 @@ type App struct { exitAfterFirstResponse bool // Exit TUI after first assistant response completes readOnly bool // When true, no new messages can be sent to the LLM titleGenerating atomic.Bool // True when title generation is in progress - titleGen *sessiontitle.Generator // Title generator for local runtime (nil for remote) + titleGen titleGenerator // Title generator for local runtime (nil for remote) snapshotController builtins.SnapshotController // Drives /undo, /snapshots, /reset; nil for runtimes that don't capture snapshots streamGuard sync.Locker // Held for the duration of every direct RunStream call; nil when not attached to a SessionManager (see WithStreamGuard) @@ -100,7 +104,10 @@ func WithQueuedMessages(msgs []string) Opt { // WithTitleGenerator sets the title generator for local title generation. // If not set, title generation will be handled by the runtime (for remote) or skipped. -func WithTitleGenerator(gen *sessiontitle.Generator) Opt { +func WithTitleGenerator(gen titleGenerator) Opt { + if concrete, ok := gen.(*sessiontitle.Generator); ok && concrete == nil { + gen = nil + } return func(a *App) { a.titleGen = gen } diff --git a/pkg/app/session_lifecycle_test.go b/pkg/app/session_lifecycle_test.go index 60ef40d1ef..584478e2ff 100644 --- a/pkg/app/session_lifecycle_test.go +++ b/pkg/app/session_lifecycle_test.go @@ -2,7 +2,6 @@ package app import ( "context" - "io" "testing" "testing/synctest" @@ -10,12 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/docker/docker-agent/pkg/chat" - "github.com/docker/docker-agent/pkg/model/provider/base" - "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/sessiontitle" "github.com/docker/docker-agent/pkg/tools" skillstool "github.com/docker/docker-agent/pkg/tools/builtin/skills" ) @@ -215,42 +210,20 @@ func TestAppRunDropsCanceledWorkWaitingForStreamGuard(t *testing.T) { } } -type blockingTitleProvider struct { +type blockingTitleGenerator struct { started chan struct{} release chan struct{} } -func (p *blockingTitleProvider) ID() modelsdev.ID { - return modelsdev.NewID("test", "title") +func (g *blockingTitleGenerator) Generate(context.Context, string, []string) (string, error) { + close(g.started) + <-g.release + return "Original title", nil } -func (p *blockingTitleProvider) BaseConfig() base.Config { return base.Config{} } - -func (p *blockingTitleProvider) CreateChatCompletionStream(context.Context, []chat.Message, []tools.Tool) (chat.MessageStream, error) { - close(p.started) - <-p.release - return &singleTitleStream{}, nil -} - -type singleTitleStream struct { - done bool -} - -func (s *singleTitleStream) 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: "Original title"}}}, - }, nil -} - -func (*singleTitleStream) Close() {} - func TestGenerateTitleKeepsOriginalSession(t *testing.T) { synctest.Test(t, func(t *testing.T) { - provider := &blockingTitleProvider{ + generator := &blockingTitleGenerator{ started: make(chan struct{}), release: make(chan struct{}), } @@ -259,7 +232,7 @@ func TestGenerateTitleKeepsOriginalSession(t *testing.T) { runtime: &mockRuntime{}, session: oldSession, events: make(chan tea.Msg, 1), - titleGen: sessiontitle.New(provider), + titleGen: generator, } done := make(chan struct{}) @@ -267,10 +240,10 @@ func TestGenerateTitleKeepsOriginalSession(t *testing.T) { defer close(done) app.generateTitle(t.Context(), oldSession, []string{"hello"}) }() - <-provider.started + <-generator.started newSession := session.New() app.ReplaceSession(t.Context(), newSession) - close(provider.release) + close(generator.release) <-done assert.Equal(t, "Original title", oldSession.TitleSnapshot()) diff --git a/pkg/app/title_generator_test.go b/pkg/app/title_generator_test.go new file mode 100644 index 0000000000..17fd96a021 --- /dev/null +++ b/pkg/app/title_generator_test.go @@ -0,0 +1,87 @@ +package app + +import ( + "context" + "errors" + "testing" + "testing/synctest" + + "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/sessiontitle" +) + +type recordingTitleGenerator struct { + calls int + id string + messages []string + title string + err error +} + +func (g *recordingTitleGenerator) Generate(_ context.Context, id string, messages []string) (string, error) { + g.calls++ + g.id = id + g.messages = messages + return g.title, g.err +} + +func TestTitleGeneratorContract(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + title string + err error + }{ + {name: "success", title: "Generated title"}, + {name: "empty"}, + {name: "failure", err: errors.New("generation failed")}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + sess := session.New() + sess.AddMessage(session.UserMessage("hello")) + gen := &recordingTitleGenerator{title: tc.title, err: tc.err} + app := New(t.Context(), &mockRuntime{}, sess, WithTitleGenerator(gen)) + assert.Zero(t, gen.calls) + require.NoError(t, app.RegenerateSessionTitle(t.Context())) + synctest.Wait() + assert.Equal(t, 1, gen.calls) + assert.Equal(t, sess.ID, gen.id) + assert.Equal(t, []string{"hello"}, gen.messages) + assert.False(t, app.IsTitleGenerating()) + event, ok := unwrappedTestEvent(<-app.events).(*runtime.SessionTitleEvent) + require.True(t, ok) + assert.Equal(t, sess.ID, event.SessionID) + want := tc.title + if tc.err != nil { + want = "" + } + assert.Equal(t, want, sess.TitleSnapshot()) + assert.Equal(t, want, event.Title) + }) + }) + } +} + +func TestNilTitleGenerator(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + gen titleGenerator + }{ + {"nil", nil}, {"typed nil", (*sessiontitle.Generator)(nil)}, {"no providers", sessiontitle.New()}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + app := New(t.Context(), &mockRuntime{}, session.New(), WithTitleGenerator(tc.gen)) + assert.Nil(t, app.titleGen) + require.EqualError(t, app.RegenerateSessionTitle(t.Context()), "title regeneration not available") + assert.False(t, app.IsTitleGenerating()) + }) + } +} diff --git a/pkg/runtime/command_source_test.go b/pkg/runtime/command_source_test.go new file mode 100644 index 0000000000..a296cb4f8c --- /dev/null +++ b/pkg/runtime/command_source_test.go @@ -0,0 +1,97 @@ +package runtime + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/docker/docker-agent/pkg/config/types" + "github.com/docker/docker-agent/pkg/tools" +) + +type lookupOnlySource struct { + calls int +} + +func (s *lookupOnlySource) CurrentAgentInfo(context.Context) CurrentAgentInfo { + s.calls++ + return CurrentAgentInfo{Commands: types.Commands{"test": {Instruction: "instruction"}}} +} + +func TestCommandLookupNeedsOnlyAgentInfo(t *testing.T) { + t.Parallel() + source := &lookupOnlySource{} + _, _, ok := LookupCommand(t.Context(), source, "ordinary message") + assert.False(t, ok) + assert.Zero(t, source.calls) + command, rest, ok := LookupCommand(t.Context(), source, "/test arguments") + assert.True(t, ok) + assert.Equal(t, "instruction", command.Instruction) + assert.Equal(t, "arguments", rest) + assert.Equal(t, 1, source.calls) +} + +type recordingCommandSource struct { + commands types.Commands + calls []string +} + +func (s *recordingCommandSource) CurrentAgentInfo(context.Context) CurrentAgentInfo { + s.calls = append(s.calls, "info") + return CurrentAgentInfo{Commands: s.commands} +} + +func (s *recordingCommandSource) CurrentAgentTools(context.Context) ([]tools.Tool, error) { + s.calls = append(s.calls, "tools") + return []tools.Tool{{Name: "echo", Handler: func(context.Context, tools.ToolCall, tools.Runtime) (*tools.ToolCallResult, error) { + s.calls = append(s.calls, "handler") + return tools.ResultSuccess("${untrusted}"), nil + }}}, nil +} + +func (s *recordingCommandSource) CommandEvaluatorFactory() CommandEvaluatorFactory { + s.calls = append(s.calls, "factory") + return func([]tools.Tool) CommandEvaluator { + return commandEvaluatorFunc(func(_ context.Context, instruction string, _ []string) string { + s.calls = append(s.calls, "evaluate") + return instruction + }) + } +} + +func TestCommandSourcePreservesLazyDiscovery(t *testing.T) { + t.Parallel() + source := &recordingCommandSource{commands: types.Commands{ + "agent": {Agent: "worker"}, + "test": {Instruction: "!echo()"}, + }} + assert.Equal(t, "message", ResolveCommand(t.Context(), source, "message")) + assert.Empty(t, source.calls) + assert.Equal(t, "/unknown", ResolveCommand(t.Context(), source, "/unknown")) + assert.Equal(t, []string{"info"}, source.calls) + source.calls = nil + assert.Equal(t, "task", ResolveCommand(t.Context(), source, "/agent task")) + assert.Equal(t, []string{"info"}, source.calls) + source.calls = nil + assert.Equal(t, "${untrusted}", ResolveCommand(t.Context(), source, "/test")) + assert.Equal(t, []string{"info", "factory", "tools", "evaluate", "tools", "handler"}, source.calls) +} + +type toolsOnlySource struct { + calls int +} + +func (s *toolsOnlySource) CurrentAgentTools(context.Context) ([]tools.Tool, error) { + s.calls++ + return nil, nil +} + +func TestToolCommandsNeedOnlyTools(t *testing.T) { + t.Parallel() + source := &toolsOnlySource{} + assert.Equal(t, "ordinary text", executeToolCommands(t.Context(), source, "ordinary text")) + assert.Zero(t, source.calls) + assert.Contains(t, executeToolCommands(t.Context(), source, "!missing()"), "not found") + assert.Equal(t, 1, source.calls) +} diff --git a/pkg/runtime/commands.go b/pkg/runtime/commands.go index 86760a0add..7c4aba45fb 100644 --- a/pkg/runtime/commands.go +++ b/pkg/runtime/commands.go @@ -69,13 +69,26 @@ func globalCommandFactory() CommandEvaluatorFactory { // This includes ${args}, ${args[N]}, ${args.join(...)}, ${args.length}, etc. var argsPlaceholderRegex = regexp.MustCompile(`\$\{args[^}]*\}`) +type commandLookup interface { + CurrentAgentInfo(ctx context.Context) CurrentAgentInfo +} + +type commandTools interface { + CurrentAgentTools(ctx context.Context) ([]tools.Tool, error) +} + +type commandSource interface { + commandLookup + commandTools +} + // LookupCommand parses userInput as a /command invocation and returns the // matching command along with its trailing arguments. The boolean is false // when userInput doesn't start with '/' or doesn't match a configured // command. Callers that need both the resolved instruction and the original // command metadata (e.g. its target agent) typically call LookupCommand to // inspect the command before calling ResolveCommand. -func LookupCommand(ctx context.Context, rt Runtime, userInput string) (cmd types.Command, rest string, ok bool) { +func LookupCommand(ctx context.Context, rt commandLookup, userInput string) (cmd types.Command, rest string, ok bool) { if !strings.HasPrefix(userInput, "/") { return types.Command{}, "", false } @@ -104,7 +117,7 @@ func LookupCommand(ctx context.Context, rt Runtime, userInput string) (cmd types // caller can forward them to the target sub-agent after switching. When the // command has no instruction and no arguments, the result is the empty // string, signalling "no message to send". -func ResolveCommand(ctx context.Context, rt Runtime, userInput string) string { +func ResolveCommand(ctx context.Context, rt commandSource, userInput string) string { command, rest, ok := LookupCommand(ctx, rt, userInput) if !ok { return userInput @@ -270,7 +283,7 @@ func isWordChar(b byte) bool { } // executeToolCommands executes !tool_name(arg=value) patterns and replaces them with output. -func executeToolCommands(ctx context.Context, rt Runtime, instruction string) string { +func executeToolCommands(ctx context.Context, rt commandTools, instruction string) string { commands := parseToolCommands(instruction) if len(commands) == 0 { return instruction diff --git a/pkg/runtime/commands_test.go b/pkg/runtime/commands_test.go index 0a05c255bb..13b0c739ae 100644 --- a/pkg/runtime/commands_test.go +++ b/pkg/runtime/commands_test.go @@ -7,15 +7,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/docker/docker-agent/pkg/config/types" - "github.com/docker/docker-agent/pkg/effort" - "github.com/docker/docker-agent/pkg/session" - "github.com/docker/docker-agent/pkg/sessiontitle" "github.com/docker/docker-agent/pkg/tools" - skillstool "github.com/docker/docker-agent/pkg/tools/builtin/skills" - mcptools "github.com/docker/docker-agent/pkg/tools/mcp" ) -// mockRuntime implements Runtime interface for testing +// mockRuntime supplies the command resolver's agent information and tools. type mockRuntime struct { commands types.Commands tools []tools.Tool @@ -24,9 +19,7 @@ type mockRuntime struct { func (m *mockRuntime) CurrentAgentTools(context.Context) ([]tools.Tool, error) { return m.tools, nil } -func (m *mockRuntime) CurrentAgentName(context.Context) string { return "test" } -func (m *mockRuntime) CurrentAgentToolsetStatuses() []tools.ToolsetStatus { return nil } -func (m *mockRuntime) RestartToolset(context.Context, string) error { return nil } + func (m *mockRuntime) CurrentAgentInfo(context.Context) CurrentAgentInfo { return CurrentAgentInfo{ Name: "test", @@ -35,69 +28,6 @@ func (m *mockRuntime) CurrentAgentInfo(context.Context) CurrentAgentInfo { } } -func (m *mockRuntime) SetCurrentAgent(context.Context, string) error { - return nil -} -func (m *mockRuntime) EmitStartupInfo(context.Context, *session.Session, EventSink) {} -func (m *mockRuntime) EmitAgentInfo(context.Context, EventSink) {} -func (m *mockRuntime) ResetStartupInfo() {} -func (m *mockRuntime) RunStream(context.Context, *session.Session) <-chan Event { - return nil -} - -func (m *mockRuntime) Run(context.Context, *session.Session) ([]session.Message, error) { - return nil, nil -} -func (m *mockRuntime) Resume(context.Context, ResumeRequest) {} -func (m *mockRuntime) ResumeElicitation(context.Context, tools.ElicitationAction, map[string]any, ...string) error { - return nil -} -func (m *mockRuntime) SessionStore() session.Store { return nil } -func (m *mockRuntime) Summarize(context.Context, *session.Session, string, EventSink) { -} -func (m *mockRuntime) PermissionsInfo() *PermissionsInfo { return nil } -func (m *mockRuntime) CurrentAgentSkillsToolset() *skillstool.ToolSet { - return nil -} - -func (m *mockRuntime) RunSkillFork(context.Context, *session.Session, skillstool.RunSkillArgs, EventSink) (*tools.ToolCallResult, error) { - return nil, nil -} - -func (m *mockRuntime) CurrentMCPPrompts(context.Context) map[string]mcptools.PromptInfo { - return make(map[string]mcptools.PromptInfo) -} - -func (m *mockRuntime) ExecuteMCPPrompt(context.Context, string, map[string]string) (string, error) { - return "", nil -} - -func (m *mockRuntime) UpdateSessionTitle(context.Context, *session.Session, string) error { - return nil -} -func (m *mockRuntime) TitleGenerator(context.Context) *sessiontitle.Generator { return nil } -func (m *mockRuntime) Close() error { return nil } -func (m *mockRuntime) Steer(context.Context, QueuedMessage) error { return nil } -func (m *mockRuntime) FollowUp(context.Context, QueuedMessage) error { return nil } -func (m *mockRuntime) QueueStatus() QueueStatus { return QueueStatus{} } -func (m *mockRuntime) TogglePause(context.Context) (bool, error) { return false, nil } -func (m *mockRuntime) SetAgentModel(context.Context, string, string) error { return nil } -func (m *mockRuntime) CycleAgentThinkingLevel(context.Context, string) (effort.Level, error) { - return "", ErrUnsupported -} - -func (m *mockRuntime) SetAgentThinkingLevel(context.Context, string, effort.Level) (effort.Level, error) { - return "", ErrUnsupported -} -func (m *mockRuntime) AvailableModels(context.Context) []ModelChoice { return nil } -func (m *mockRuntime) SupportsModelSwitching() bool { return false } -func (m *mockRuntime) OnToolsChanged(func(Event)) {} -func (m *mockRuntime) OnBackgroundEvent(func(Event)) {} -func (m *mockRuntime) OnElicitationRequest(func(Event)) {} - -func (m *mockRuntime) RegenerateTitle(context.Context, *session.Session, chan Event) { -} - func TestResolveCommand_SimpleCommand(t *testing.T) { t.Parallel() @@ -745,7 +675,3 @@ func TestResolveCommand_AgentWithInstruction(t *testing.T) { result := ResolveCommand(t.Context(), rt, "/plan add login") assert.Equal(t, "Plan the work for: add login", result) } - -func (m *mockRuntime) ReadSkillContent(context.Context, *session.Session, string) (string, error) { - return "", nil -} diff --git a/pkg/tools/a2a/a2a.go b/pkg/tools/a2a/a2a.go index 9d21f2e314..c3b8123904 100644 --- a/pkg/tools/a2a/a2a.go +++ b/pkg/tools/a2a/a2a.go @@ -6,6 +6,7 @@ import ( "context" "errors" "fmt" + "iter" "log/slog" "net/http" neturl "net/url" @@ -25,6 +26,14 @@ import ( "github.com/docker/docker-agent/pkg/upstream" ) +type streamingSender interface { + SendStreamingMessage(ctx context.Context, params *a2a.MessageSendParams) iter.Seq2[a2a.Event, error] +} + +type headerExpander interface { + ExpandMap(ctx context.Context, values map[string]string) map[string]string +} + // Toolset implements tools.ToolSet for A2A remote agents. type Toolset struct { name string @@ -32,8 +41,8 @@ type Toolset struct { headers map[string]string timeout time.Duration allowPrivateIPs bool - expander *js.Expander - client *a2aclient.Client + expander headerExpander + client streamingSender card *a2a.AgentCard mu sync.RWMutex } @@ -55,7 +64,10 @@ func WithAllowPrivateIPs(allow bool) Option { return func(t *Toolset) { t.allowPrivateIPs = allow } } -func WithExpander(expander *js.Expander) Option { +func WithExpander(expander headerExpander) Option { + if expander == nil { + expander = (*js.Expander)(nil) + } return func(t *Toolset) { t.expander = expander } } @@ -85,10 +97,11 @@ func CreateToolSet(ctx context.Context, toolset latest.Toolset, runConfig *confi // NewToolset creates a new A2A toolset for the given URL. func NewToolset(name, url string, headers map[string]string, opts ...Option) *Toolset { t := &Toolset{ - name: name, - url: url, - headers: headers, - timeout: httpclient.DefaultToolHTTPTimeout, + name: name, + url: url, + headers: headers, + timeout: httpclient.DefaultToolHTTPTimeout, + expander: (*js.Expander)(nil), } for _, opt := range opts { opt(t) diff --git a/pkg/tools/a2a/expander_test.go b/pkg/tools/a2a/expander_test.go new file mode 100644 index 0000000000..f981549f41 --- /dev/null +++ b/pkg/tools/a2a/expander_test.go @@ -0,0 +1,62 @@ +package a2a + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + goa2a "github.com/a2aproject/a2a-go/a2a" + "github.com/a2aproject/a2a-go/a2asrv" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/js" +) + +type recordingHeaderExpander struct{ calls int } + +func (e *recordingHeaderExpander) ExpandMap(ctx context.Context, values map[string]string) map[string]string { + e.calls++ + return map[string]string{"Authorization": "resolved"} +} + +func TestNilExpander(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + expander headerExpander + }{ + {"nil", nil}, {"typed nil", (*js.Expander)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + expander := NewToolset("test", "https://example.com", nil, WithExpander(tc.expander)).expander + assert.Nil(t, expander.ExpandMap(t.Context(), nil)) + assert.Panics(t, func() { expander.ExpandMap(t.Context(), map[string]string{}) }) + }) + } +} + +func TestHeaderExpanderAtStart(t *testing.T) { + t.Parallel() + server := httptest.NewServer(a2asrv.NewJSONRPCHandler(a2asrv.NewHandler(testA2AHandler{}))) + t.Cleanup(server.Close) + expander := &recordingHeaderExpander{} + cardServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Empty(t, r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(goa2a.AgentCard{ + Name: "test", URL: server.URL, Version: "1.0.0", ProtocolVersion: string(goa2a.Version), + PreferredTransport: goa2a.TransportProtocolJSONRPC, Capabilities: goa2a.AgentCapabilities{Streaming: true}, + DefaultInputModes: []string{"text/plain"}, DefaultOutputModes: []string{"text/plain"}, + }) + })) + t.Cleanup(cardServer.Close) + tool := NewToolset("test", cardServer.URL, map[string]string{"Authorization": "template"}, WithExpander(expander), WithAllowPrivateIPs(true)) + assert.Zero(t, expander.calls) + require.NoError(t, tool.Start(t.Context())) + assert.Equal(t, 1, expander.calls) + require.NoError(t, tool.Stop(t.Context())) +} diff --git a/pkg/tools/a2a/streaming_sender_test.go b/pkg/tools/a2a/streaming_sender_test.go new file mode 100644 index 0000000000..aa67b4ab1a --- /dev/null +++ b/pkg/tools/a2a/streaming_sender_test.go @@ -0,0 +1,84 @@ +package a2a + +import ( + "context" + "errors" + "iter" + "testing" + + goa2a "github.com/a2aproject/a2a-go/a2a" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/tools" +) + +type streamingSenderFunc func(context.Context, *goa2a.MessageSendParams) iter.Seq2[goa2a.Event, error] + +func (f streamingSenderFunc) SendStreamingMessage(ctx context.Context, params *goa2a.MessageSendParams) iter.Seq2[goa2a.Event, error] { + return f(ctx, params) +} + +func TestStreamingSenderContract(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + events []goa2a.Event + want string + isError bool + }{ + {name: "empty", want: "No response from agent", isError: true}, + {name: "message", events: []goa2a.Event{goa2a.NewMessage(goa2a.MessageRoleAgent, &goa2a.TextPart{Text: "hello"})}, want: "hello"}, + {name: "mixed", events: []goa2a.Event{ + &goa2a.TaskStatusUpdateEvent{Status: goa2a.TaskStatus{Message: goa2a.NewMessage(goa2a.MessageRoleAgent, &goa2a.TextPart{Text: "one"})}}, + &goa2a.TaskArtifactUpdateEvent{Artifact: &goa2a.Artifact{Parts: goa2a.ContentParts{&goa2a.TextPart{Text: "two"}}}}, + &goa2a.Task{Status: goa2a.TaskStatus{Message: goa2a.NewMessage(goa2a.MessageRoleAgent, &goa2a.TextPart{Text: "three"})}}, + }, want: "onetwothree"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + calls := 0 + tool := NewToolset("test", "https://example.com", nil) + tool.client = streamingSenderFunc(func(ctx context.Context, params *goa2a.MessageSendParams) iter.Seq2[goa2a.Event, error] { + calls++ + assert.Equal(t, t.Context(), ctx) + assert.Equal(t, goa2a.MessageRoleUser, params.Message.Role) + assert.Equal(t, "hello", extractText(params.Message)) + return func(yield func(goa2a.Event, error) bool) { + for _, event := range tc.events { + if !yield(event, nil) { + return + } + } + } + }) + result, err := tool.createHandler()(t.Context(), tools.ToolCall{Function: tools.FunctionCall{Arguments: `{"message":"hello"}`}}, tools.NopRuntime{}) + require.NoError(t, err) + assert.Equal(t, tc.want, result.Output) + assert.Equal(t, tc.isError, result.IsError) + assert.Equal(t, 1, calls) + }) + } +} + +func TestStreamingSenderStopsOnError(t *testing.T) { + t.Parallel() + sentinel := errors.New("stream failed") + stopped := false + tool := NewToolset("test", "https://example.com", nil) + tool.client = streamingSenderFunc(func(context.Context, *goa2a.MessageSendParams) iter.Seq2[goa2a.Event, error] { + return func(yield func(goa2a.Event, error) bool) { + if !yield(goa2a.NewMessage(goa2a.MessageRoleAgent, &goa2a.TextPart{Text: "partial"}), nil) { + return + } + stopped = !yield(nil, sentinel) + } + }) + result, err := tool.createHandler()(t.Context(), tools.ToolCall{Function: tools.FunctionCall{Arguments: `{"message":"hello"}`}}, tools.NopRuntime{}) + require.ErrorIs(t, err, sentinel) + assert.Nil(t, result) + assert.True(t, stopped) + require.NoError(t, tool.Stop(t.Context())) + _, err = tool.createHandler()(t.Context(), tools.ToolCall{Function: tools.FunctionCall{Arguments: `{"message":"hello"}`}}, tools.NopRuntime{}) + require.EqualError(t, err, "A2A client not initialized") +} diff --git a/pkg/tools/builtin/fetch/expander_test.go b/pkg/tools/builtin/fetch/expander_test.go new file mode 100644 index 0000000000..6372ad0406 --- /dev/null +++ b/pkg/tools/builtin/fetch/expander_test.go @@ -0,0 +1,62 @@ +package fetch + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/js" + "github.com/docker/docker-agent/pkg/tools" +) + +type recordingHeaderExpander struct{ calls int } + +func (e *recordingHeaderExpander) ExpandMap(ctx context.Context, values map[string]string) map[string]string { + e.calls++ + return map[string]string{"Authorization": "resolved"} +} + +func TestNilExpander(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + expander headerExpander + }{ + {"nil", nil}, {"typed nil", (*js.Expander)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + expander := New(WithExpander(tc.expander)).handler.expander + assert.Nil(t, expander.ExpandMap(t.Context(), nil)) + assert.Panics(t, func() { expander.ExpandMap(t.Context(), map[string]string{}) }) + }) + } +} + +func TestHeaderExpanderAtCallSite(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "resolved", r.Header.Get("Authorization")) + if r.URL.Path == "/robots.txt" { + w.WriteHeader(http.StatusNotFound) + return + } + _, _ = w.Write([]byte("content")) + })) + t.Cleanup(server.Close) + expander := &recordingHeaderExpander{} + tool := New(WithExpander(expander), WithHeaders(map[string]string{"Authorization": "template"}), WithAllowPrivateIPs(true)) + list, err := tool.Tools(t.Context()) + require.NoError(t, err) + assert.Zero(t, expander.calls) + for range 2 { + result, err := list[0].Handler(t.Context(), tools.ToolCall{Function: tools.FunctionCall{Arguments: `{"urls":["` + server.URL + `"],"format":"text"}`}}, tools.NopRuntime{}) + require.NoError(t, err) + assert.False(t, result.IsError) + } + assert.Equal(t, 2, expander.calls) +} diff --git a/pkg/tools/builtin/fetch/fetch.go b/pkg/tools/builtin/fetch/fetch.go index adae3703a7..dbd62bc2d8 100644 --- a/pkg/tools/builtin/fetch/fetch.go +++ b/pkg/tools/builtin/fetch/fetch.go @@ -28,6 +28,10 @@ import ( "github.com/docker/docker-agent/pkg/useragent" ) +type headerExpander interface { + ExpandMap(ctx context.Context, values map[string]string) map[string]string +} + const ( ToolNameFetch = types.ToolNameFetch ) @@ -49,7 +53,7 @@ type fetchHandler struct { headers map[string]string allowPrivateIPs bool escapeHTML bool - expander *js.Expander + expander headerExpander } type ToolArgs struct { @@ -530,7 +534,8 @@ func CreateToolSet(toolset latest.Toolset, runConfig *config.RuntimeConfig) (too func New(options ...ToolOption) *ToolSet { tool := &ToolSet{ handler: &fetchHandler{ - timeout: httpclient.DefaultToolHTTPTimeout, + timeout: httpclient.DefaultToolHTTPTimeout, + expander: (*js.Expander)(nil), }, } @@ -597,7 +602,10 @@ func WithEscapeHTML(escape bool) ToolOption { } } -func WithExpander(expander *js.Expander) ToolOption { +func WithExpander(expander headerExpander) ToolOption { + if expander == nil { + expander = (*js.Expander)(nil) + } return func(t *ToolSet) { t.handler.expander = expander } diff --git a/pkg/tools/builtin/openapi/expander_test.go b/pkg/tools/builtin/openapi/expander_test.go new file mode 100644 index 0000000000..20634c203e --- /dev/null +++ b/pkg/tools/builtin/openapi/expander_test.go @@ -0,0 +1,61 @@ +package openapi + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/js" +) + +type recordingHeaderExpander struct{ calls int } + +func (e *recordingHeaderExpander) ExpandMap(ctx context.Context, values map[string]string) map[string]string { + e.calls++ + return map[string]string{"Authorization": "resolved"} +} + +func TestNilExpander(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + expander headerExpander + }{ + {"nil", nil}, {"typed nil", (*js.Expander)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + expander := New("https://example.com/spec", nil, WithExpander(tc.expander)).expander + assert.Nil(t, expander.ExpandMap(t.Context(), nil)) + assert.Panics(t, func() { expander.ExpandMap(t.Context(), map[string]string{}) }) + }) + } +} + +func TestHeaderExpanderAtCallSite(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/spec" { + assert.Equal(t, "template", r.Header.Get("Authorization")) + _, _ = w.Write([]byte(petStoreSpec)) + return + } + assert.Equal(t, "resolved", r.Header.Get("Authorization")) + _, _ = w.Write([]byte("content")) + })) + t.Cleanup(server.Close) + expander := &recordingHeaderExpander{} + tool := New(server.URL+"/spec", map[string]string{"Authorization": "template"}, WithExpander(expander), WithAllowPrivateIPs(true)) + list, err := tool.Tools(t.Context()) + require.NoError(t, err) + assert.Zero(t, expander.calls) + for range 2 { + result := callTool(t, toolByName(t, list, "listPets"), `{}`) + assert.False(t, result.IsError) + } + assert.Equal(t, 2, expander.calls) +} diff --git a/pkg/tools/builtin/openapi/openapi.go b/pkg/tools/builtin/openapi/openapi.go index 6de38eaa77..373b0c178d 100644 --- a/pkg/tools/builtin/openapi/openapi.go +++ b/pkg/tools/builtin/openapi/openapi.go @@ -27,6 +27,10 @@ import ( "github.com/docker/docker-agent/pkg/useragent" ) +type headerExpander interface { + ExpandMap(ctx context.Context, values map[string]string) map[string]string +} + // CreateToolSet is used by the tools registry. func CreateToolSet(ctx context.Context, toolset latest.Toolset, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { expander := js.NewJsExpander(runConfig.EnvProvider()) @@ -55,7 +59,7 @@ type ToolSet struct { timeout time.Duration maxOutputBytes int allowPrivateIPs bool - expander *js.Expander + expander headerExpander } // Verify interface compliance. @@ -88,7 +92,10 @@ func WithAllowPrivateIPs(allow bool) Option { return func(t *ToolSet) { t.allowPrivateIPs = allow } } -func WithExpander(expander *js.Expander) Option { +func WithExpander(expander headerExpander) Option { + if expander == nil { + expander = (*js.Expander)(nil) + } return func(t *ToolSet) { t.expander = expander } } @@ -99,6 +106,7 @@ func New(specURL string, headers map[string]string, opts ...Option) *ToolSet { headers: headers, timeout: httpclient.DefaultToolHTTPTimeout, maxOutputBytes: maxOutputSize, + expander: (*js.Expander)(nil), } for _, opt := range opts { opt(t) @@ -463,7 +471,7 @@ type openAPIHandler struct { timeout time.Duration maxOutputBytes int allowPrivateIPs bool - expander *js.Expander + expander headerExpander } type openAPICallArgs map[string]any @@ -495,7 +503,11 @@ func (h *openAPIHandler) callTool(ctx context.Context, params openAPICallArgs) ( } req.Header.Set("Accept", "application/json") - headers := h.expander.ExpandMap(ctx, h.headers) + expander := h.expander + if expander == nil { + expander = (*js.Expander)(nil) + } + headers := expander.ExpandMap(ctx, h.headers) setHeaders(req, headers) client := httpclient.ClientForAllowPrivateIPs(h.timeout, h.allowPrivateIPs) diff --git a/pkg/tools/builtin/openurl/expander_test.go b/pkg/tools/builtin/openurl/expander_test.go new file mode 100644 index 0000000000..0f3e6489f2 --- /dev/null +++ b/pkg/tools/builtin/openurl/expander_test.go @@ -0,0 +1,54 @@ +package openurl + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/js" +) + +type recordingURLExpander struct{ calls int } + +func (e *recordingURLExpander) Expand(ctx context.Context, text string, values map[string]string) string { + e.calls++ + return "https://example.com/resolved" +} + +func TestURLExpanderAtCallSite(t *testing.T) { + t.Parallel() + expander := &recordingURLExpander{} + var opened string + tool := New("https://example.com/${env.VALUE}", WithExpander(expander), WithOpener(func(_ context.Context, url string) error { opened = url; return nil })) + _, err := tool.Tools(t.Context()) + require.NoError(t, err) + assert.Zero(t, expander.calls) + for range 2 { + _, err = tool.callTool(t.Context(), struct{}{}) + require.NoError(t, err) + } + assert.Equal(t, 2, expander.calls) + assert.Equal(t, "https://example.com/resolved", opened) +} + +func TestNilURLExpanderSkipsExpansion(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + expander urlExpander + }{ + {"nil", nil}, {"typed nil", (*js.Expander)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var opened string + tool := New("https://example.com/${env.VALUE}", WithExpander(tc.expander), WithOpener(func(_ context.Context, url string) error { opened = url; return nil })) + _, err := tool.callTool(t.Context(), struct{}{}) + require.NoError(t, err) + assert.Nil(t, tool.expander) + assert.Equal(t, "https://example.com/${env.VALUE}", opened) + }) + } +} diff --git a/pkg/tools/builtin/openurl/openurl.go b/pkg/tools/builtin/openurl/openurl.go index 70d3172c17..9ba55ff83f 100644 --- a/pkg/tools/builtin/openurl/openurl.go +++ b/pkg/tools/builtin/openurl/openurl.go @@ -21,12 +21,16 @@ import ( "github.com/docker/docker-agent/pkg/tools" ) +type urlExpander interface { + Expand(ctx context.Context, text string, values map[string]string) string +} + const ToolNameOpenURL = "open_url" type ToolSet struct { url string name string - expander *js.Expander + expander urlExpander open func(context.Context, string) error } @@ -65,7 +69,10 @@ func WithName(name string) Option { } // WithExpander enables ${env.X} interpolation of the configured URL. -func WithExpander(expander *js.Expander) Option { +func WithExpander(expander urlExpander) Option { + if concrete, ok := expander.(*js.Expander); ok && concrete == nil { + expander = nil + } return func(t *ToolSet) { t.expander = expander } } diff --git a/pkg/tools/builtin/rag/backend_test.go b/pkg/tools/builtin/rag/backend_test.go new file mode 100644 index 0000000000..4eec9f0cfb --- /dev/null +++ b/pkg/tools/builtin/rag/backend_test.go @@ -0,0 +1,147 @@ +package rag + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "testing" + "testing/synctest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/rag" + "github.com/docker/docker-agent/pkg/rag/database" + ragtypes "github.com/docker/docker-agent/pkg/rag/types" +) + +type stubBackend struct { + results []database.SearchResult + usage ragtypes.Usage + query string + err error + events chan ragtypes.Event + initialize func(context.Context) error + watch func(context.Context) error + close func() error +} + +func (b *stubBackend) Initialize(ctx context.Context) error { + if b.initialize != nil { + return b.initialize(ctx) + } + return nil +} + +func (b *stubBackend) StartFileWatcher(ctx context.Context) error { + if b.watch != nil { + return b.watch(ctx) + } + return nil +} +func (b *stubBackend) Events() <-chan ragtypes.Event { return b.events } +func (b *stubBackend) Query(_ context.Context, query string) ([]database.SearchResult, ragtypes.Usage, error) { + b.query = query + return b.results, b.usage, b.err +} +func (*stubBackend) Description() string { return "backend description" } +func (*stubBackend) ToolInstruction() string { return "backend instruction" } +func (b *stubBackend) Close() error { + if b.close != nil { + return b.close() + } + return nil +} + +func TestBackendQuery(t *testing.T) { + t.Parallel() + b := &stubBackend{} + for i := range 12 { + b.results = append(b.results, database.SearchResult{Document: database.Document{SourcePath: fmt.Sprintf("%d.txt", i), Content: fmt.Sprintf("content %d", i), ChunkIndex: i}, Similarity: float64(i) / 12}) + } + tool := New(b, "search") + list, err := tool.Tools(t.Context()) + require.NoError(t, err) + assert.Equal(t, "backend description", list[0].Description) + assert.Equal(t, "backend instruction", tool.Instructions()) + result, err := tool.handleQueryRAG(t.Context(), queryRAGArgs{Query: "needle"}) + require.NoError(t, err) + assert.Equal(t, "needle", b.query) + var results []queryResult + require.NoError(t, json.Unmarshal([]byte(result.Output), &results)) + require.Len(t, results, 10) + assert.Equal(t, "11.txt", results[0].SourcePath) + assert.Equal(t, "content 11", results[0].Content) + assert.Equal(t, 11, results[0].ChunkIndex) + assert.Equal(t, "2.txt", results[9].SourcePath) + sentinel := errors.New("query failed") + b.err = sentinel + _, err = tool.handleQueryRAG(t.Context(), queryRAGArgs{Query: "needle"}) + require.ErrorIs(t, err, sentinel) + b.query = "" + _, err = tool.handleQueryRAG(t.Context(), queryRAGArgs{}) + require.EqualError(t, err, "query cannot be empty") + assert.Empty(t, b.query) +} + +func TestBackendLifecycle(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + watchContexts := make(chan context.Context, 1) + watcherDone := false + closed := false + b := &stubBackend{events: make(chan ragtypes.Event, 1)} + b.initialize = func(ctx context.Context) error { + require.NoError(t, ctx.Err()) + b.events <- ragtypes.Event{Type: ragtypes.EventTypeIndexingStarted} + return nil + } + b.watch = func(ctx context.Context) error { + watchContexts <- ctx + <-ctx.Done() + watcherDone = true + return ctx.Err() + } + b.close = func() error { + assert.True(t, watcherDone) + closed = true + return nil + } + tool := New(b, "search") + received := make(chan ragtypes.Event, 1) + unsubscribe := tool.SubscribeEvents(func(event ragtypes.Event) { received <- event }) + defer unsubscribe() + ctx, cancel := context.WithCancel(t.Context()) + require.NoError(t, tool.Start(ctx)) + cancel() + synctest.Wait() + watchCtx := <-watchContexts + require.NotNil(t, watchCtx) + require.NoError(t, watchCtx.Err()) + assert.Equal(t, ragtypes.EventTypeIndexingStarted, (<-received).Type) + require.NoError(t, tool.Stop(t.Context())) + assert.True(t, closed) + assert.ErrorIs(t, watchCtx.Err(), context.Canceled) + }) +} + +func TestNilBackend(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + manager backend + }{{"nil", nil}, {"typed nil", (*rag.Manager)(nil)}} { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tool := New(tc.manager, "search") + assert.Nil(t, tool.manager) + require.NoError(t, tool.Start(t.Context())) + require.NoError(t, tool.Stop(t.Context())) + list, err := tool.Tools(t.Context()) + require.NoError(t, err) + assert.Contains(t, list[0].Description, "search") + assert.Contains(t, tool.Instructions(), "search") + }) + } +} diff --git a/pkg/tools/builtin/rag/rag.go b/pkg/tools/builtin/rag/rag.go index c3b46e3e81..8da4657a18 100644 --- a/pkg/tools/builtin/rag/rag.go +++ b/pkg/tools/builtin/rag/rag.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker-agent/pkg/config" "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/rag" + "github.com/docker/docker-agent/pkg/rag/database" ragtypes "github.com/docker/docker-agent/pkg/rag/types" "github.com/docker/docker-agent/pkg/telemetry" "github.com/docker/docker-agent/pkg/tools" @@ -46,9 +47,19 @@ func CreateToolSet(ctx context.Context, toolset latest.Toolset, parentDir string // EventCallback is called to forward RAG manager events during initialization. type EventCallback = ragtypes.EventCallback +type backend interface { + Initialize(ctx context.Context) error + StartFileWatcher(ctx context.Context) error + Events() <-chan ragtypes.Event + Query(ctx context.Context, query string) ([]database.SearchResult, ragtypes.Usage, error) + Description() string + ToolInstruction() string + Close() error +} + // ToolSet provides document querying capabilities for a single RAG source. type ToolSet struct { - manager *rag.Manager + manager backend toolName string subscribers tools.Subscribers[ragtypes.Event] cancelWatcher context.CancelFunc @@ -86,7 +97,10 @@ func WithIndexingTimeout(d time.Duration) Option { } // New creates a new RAG toolset for a single RAG manager. -func New(manager *rag.Manager, toolName string, opts ...Option) *ToolSet { +func New(manager backend, toolName string, opts ...Option) *ToolSet { + if concrete, ok := manager.(*rag.Manager); ok && concrete == nil { + manager = nil + } t := &ToolSet{ manager: manager, toolName: toolName, diff --git a/pkg/tools/builtin/rag/rag_test.go b/pkg/tools/builtin/rag/rag_test.go index 6d2af9caaf..9db39b2fa9 100644 --- a/pkg/tools/builtin/rag/rag_test.go +++ b/pkg/tools/builtin/rag/rag_test.go @@ -109,38 +109,8 @@ func (m *mockStrategy) Close() error { } func TestRAGTool_HandleQuery_Telemetry(t *testing.T) { - // This test asserts handleQueryRAG runs and doesn't panic. - // Since telemetry is global, we can't easily assert on the emitted event here without - // exposing internal test utilities, but we can verify it doesn't crash on non-zero usage. - - events := make(chan ragtypes.Event) - defer close(events) - - stratA := &mockStrategy{ - usage: ragtypes.Usage{ - TotalTokens: 10, - Cost: 0.1, - ModelID: "test-model", - }, - } - - cfg := rag.Config{ - StrategyConfigs: []strategy.Config{ - {Name: "stratA", Strategy: stratA}, - }, - Results: rag.ResultsConfig{ - Limit: 10, - }, - } - - mgr, err := rag.New(t.Context(), "test-rag", cfg, events) - require.NoError(t, err) - - tool := &ToolSet{ - manager: mgr, - toolName: "test-rag", - } - + t.Parallel() + tool := New(&stubBackend{usage: ragtypes.Usage{TotalTokens: 10, Cost: 0.1, ModelID: "test-model"}}, "test-rag") res, err := tool.handleQueryRAG(t.Context(), queryRAGArgs{Query: "test"}) require.NoError(t, err) assert.NotNil(t, res) diff --git a/pkg/tools/builtin/webhook/expander_test.go b/pkg/tools/builtin/webhook/expander_test.go new file mode 100644 index 0000000000..b598a014d3 --- /dev/null +++ b/pkg/tools/builtin/webhook/expander_test.go @@ -0,0 +1,61 @@ +package webhook + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/js" +) + +type recordingTemplateExpander struct{ calls []string } + +func (e *recordingTemplateExpander) Expand(ctx context.Context, text string, values map[string]string) string { + e.calls = append(e.calls, "url") + return "https://example.com/webhook" +} + +func (e *recordingTemplateExpander) ExpandMap(ctx context.Context, values map[string]string) map[string]string { + e.calls = append(e.calls, "headers") + return map[string]string{"Authorization": "resolved"} +} + +func TestTemplateExpanderAtEachAttempt(t *testing.T) { + t.Parallel() + expander := &recordingTemplateExpander{} + tool := New(latest.WebhookToolConfig{URL: "template"}, expander, time.Second) + client := &fakeDoer{} + tool.client = client + assert.Empty(t, expander.calls) + for range 2 { + verdict, _, _ := tool.attempt(t.Context(), SendArgs{Message: "hello"}) + assert.Equal(t, delivered, verdict) + } + assert.Equal(t, []string{"url", "headers", "url", "headers"}, expander.calls) + require.Len(t, client.reqs, 2) + assert.Equal(t, "resolved", client.reqs[0].Header.Get("Authorization")) +} + +func TestNilTemplateExpander(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + expander templateExpander + }{ + {"nil", nil}, {"typed nil", (*js.Expander)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tool := New(latest.WebhookToolConfig{URL: "https://example.com"}, tc.expander, time.Second) + tool.client = &fakeDoer{} + verdict, _, _ := tool.attempt(t.Context(), SendArgs{Message: "hello"}) + assert.Equal(t, delivered, verdict) + assert.Panics(t, func() { tool.expander.ExpandMap(t.Context(), map[string]string{}) }) + assert.Panics(t, func() { tool.expander.Expand(t.Context(), "${env.VALUE}", nil) }) + }) + } +} diff --git a/pkg/tools/builtin/webhook/webhook.go b/pkg/tools/builtin/webhook/webhook.go index 100f73dff8..11d0b7685a 100644 --- a/pkg/tools/builtin/webhook/webhook.go +++ b/pkg/tools/builtin/webhook/webhook.go @@ -26,6 +26,11 @@ import ( "github.com/docker/docker-agent/pkg/useragent" ) +type templateExpander interface { + Expand(ctx context.Context, text string, values map[string]string) string + ExpandMap(ctx context.Context, values map[string]string) map[string]string +} + const ( ToolNameSendWebhook = "send_webhook" category = "webhook" @@ -106,7 +111,7 @@ const ( type ToolSet struct { cfg latest.WebhookToolConfig - expander *js.Expander + expander templateExpander client httpDoer maxAttempts int @@ -132,7 +137,10 @@ var ( _ tools.Instructable = (*ToolSet)(nil) ) -func New(cfg latest.WebhookToolConfig, expander *js.Expander, timeout time.Duration) *ToolSet { +func New(cfg latest.WebhookToolConfig, expander templateExpander, timeout time.Duration) *ToolSet { + if expander == nil { + expander = (*js.Expander)(nil) + } return &ToolSet{ cfg: cfg, expander: expander, diff --git a/pkg/tui/dialog/attention.go b/pkg/tui/dialog/attention.go index 9b41752c5c..79da5c2baa 100644 --- a/pkg/tui/dialog/attention.go +++ b/pkg/tui/dialog/attention.go @@ -5,7 +5,6 @@ import ( tea "charm.land/bubbletea/v2" - "github.com/docker/docker-agent/pkg/app" "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/tui/animation" "github.com/docker/docker-agent/pkg/tui/components/tool" @@ -13,12 +12,12 @@ import ( ) // NewAttentionDialog builds a prompt without opening it or running its Init command. -func NewAttentionDialog(ctx context.Context, ar *animation.Runtime, application *app.App, state *service.SessionState, event tea.Msg, registries ...*tool.Registry) Dialog { +func NewAttentionDialog(ctx context.Context, ar *animation.Runtime, application elicitationResponder, state *service.SessionState, event tea.Msg, registries ...*tool.Registry) Dialog { switch ev := event.(type) { case *runtime.ToolCallConfirmationEvent: return NewToolConfirmationDialog(ar, ev, state, registries...) case *runtime.MaxIterationsReachedEvent: - return NewMaxIterationsDialog(ev.MaxIterations, application) + return NewMaxIterationsDialog(ev.MaxIterations) case *runtime.ElicitationRequestEvent: if ev.Meta["docker-agent/type"] == "oauth_flow" { serverURL, _ := ev.Meta["docker-agent/server_url"].(string) diff --git a/pkg/tui/dialog/max_iterations.go b/pkg/tui/dialog/max_iterations.go index 513fc46c7d..1b273dd2de 100644 --- a/pkg/tui/dialog/max_iterations.go +++ b/pkg/tui/dialog/max_iterations.go @@ -7,7 +7,6 @@ import ( tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" - "github.com/docker/docker-agent/pkg/app" "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/tui/core" "github.com/docker/docker-agent/pkg/tui/core/layout" @@ -25,15 +24,13 @@ type maxIterationsDialog struct { BaseDialog maxIterations int - app *app.App keyMap ConfirmKeyMap } // NewMaxIterationsDialog creates a new max iterations confirmation dialog -func NewMaxIterationsDialog(maxIterations int, appInstance *app.App) Dialog { +func NewMaxIterationsDialog(maxIterations int) Dialog { return &maxIterationsDialog{ maxIterations: maxIterations, - app: appInstance, keyMap: DefaultConfirmKeyMap(), } } diff --git a/pkg/tui/dialog/oauth_authorization.go b/pkg/tui/dialog/oauth_authorization.go index b4e4641daa..e656cb0bd2 100644 --- a/pkg/tui/dialog/oauth_authorization.go +++ b/pkg/tui/dialog/oauth_authorization.go @@ -6,13 +6,16 @@ import ( tea "charm.land/bubbletea/v2" - "github.com/docker/docker-agent/pkg/app" "github.com/docker/docker-agent/pkg/tools" "github.com/docker/docker-agent/pkg/tui/core" "github.com/docker/docker-agent/pkg/tui/core/layout" "github.com/docker/docker-agent/pkg/tui/styles" ) +type elicitationResponder interface { + ResumeElicitation(ctx context.Context, action tools.ElicitationAction, content map[string]any, elicitationID ...string) error +} + type oauthAuthorizationDialog struct { BaseDialog @@ -20,7 +23,7 @@ type oauthAuthorizationDialog struct { serverURL string elicitationID string - app *app.App + app elicitationResponder keyMap ConfirmKeyMap } @@ -28,7 +31,7 @@ type oauthAuthorizationDialog struct { // elicitationID is variadic for the same backward-compatibility reason as // NewElicitationDialog (see firstElicitationID); at most the first value is // meaningful. -func NewOAuthAuthorizationDialog(ctx context.Context, serverURL string, appInstance *app.App, elicitationID ...string) Dialog { +func NewOAuthAuthorizationDialog(ctx context.Context, serverURL string, appInstance elicitationResponder, elicitationID ...string) Dialog { return &oauthAuthorizationDialog{ ctx: func() context.Context { return context.WithoutCancel(ctx) }, serverURL: serverURL, diff --git a/pkg/tui/dialog/oauth_authorization_test.go b/pkg/tui/dialog/oauth_authorization_test.go new file mode 100644 index 0000000000..05130e3ca8 --- /dev/null +++ b/pkg/tui/dialog/oauth_authorization_test.go @@ -0,0 +1,73 @@ +package dialog + +import ( + "context" + "errors" + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/tools" +) + +type recordingElicitationResponder struct { + calls int + action tools.ElicitationAction + id string + contextErr error + value any +} + +func (r *recordingElicitationResponder) ResumeElicitation(ctx context.Context, action tools.ElicitationAction, content map[string]any, ids ...string) error { + r.calls++ + r.action = action + r.id = firstElicitationID(ids) + r.contextErr = ctx.Err() + r.value = ctx.Value(oauthContextKey{}) + return errors.New("response failed") +} + +type oauthContextKey struct{} + +func TestOAuthAuthorizationResponder(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + key rune + action tools.ElicitationAction + }{ + {'y', tools.ElicitationActionAccept}, + {'n', tools.ElicitationActionDecline}, + } { + t.Run(string(tc.key), func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.WithValue(t.Context(), oauthContextKey{}, "retained")) + cancel() + responder := &recordingElicitationResponder{} + d := NewAttentionDialog(ctx, nil, responder, nil, &runtime.ElicitationRequestEvent{ + ElicitationID: "request-1", + Meta: map[string]any{"docker-agent/type": "oauth_flow", "docker-agent/server_url": "https://example.com"}, + }) + require.NotNil(t, d) + _, cmd := d.Update(tea.KeyPressMsg{Code: tc.key, Text: string(tc.key)}) + require.NotNil(t, cmd) + assert.Equal(t, 1, responder.calls) + assert.Equal(t, tc.action, responder.action) + assert.Equal(t, "request-1", responder.id) + require.NoError(t, responder.contextErr) + assert.Equal(t, "retained", responder.value) + assert.Equal(t, CloseDialogMsg{ElicitationID: "request-1"}, cmd()) + }) + } +} + +func TestMaxIterationsNeedsNoApplication(t *testing.T) { + t.Parallel() + d := NewAttentionDialog(t.Context(), nil, nil, nil, &runtime.MaxIterationsReachedEvent{MaxIterations: 10}) + require.IsType(t, &maxIterationsDialog{}, d) + assert.Equal(t, 10, d.(*maxIterationsDialog).maxIterations) + _, cmd := d.Update(tea.KeyPressMsg{Code: 'y', Text: "y"}) + require.NotNil(t, cmd) +}