mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-13 19:20:06 +00:00
fix(sdk): stop leaking fantasy types through pkg/kit.AgentConfig (#30)
Replace the alias-based AgentConfig and handler types with SDK-owned structs and function types. CoreTools / ExtraTools / ToolWrapper now accept []kit.Tool, and the handler types (ToolCallHandler, ToolExecutionHandler, ToolResultHandler, ResponseHandler, StreamingResponseHandler, ToolCallContentHandler) plus SpinnerFunc are declared in pkg/kit/ with signatures that reference only SDK types. Consumers no longer need to import charm.land/fantasy to populate an AgentConfig or assign a handler. go doc pkg/kit AgentConfig output no longer mentions fantasy.*. - Add unexported (*AgentConfig).toInternal() to convert at the SDK boundary; Tool is still an alias for the underlying tool type, so slice and function fields convert without allocation. - Add agent_config_internal_test.go covering nil receiver, scalar fields, tool slices, ToolWrapper invocation, OnMCPServerLoaded, and auth/token-factory wiring. - Add types_test.go cases that populate AgentConfig and SpinnerFunc without importing fantasy -- the file compiling is the regression proof for the leak. - Update pkg/kit/README.md Re-exported Types section to record that AgentConfig and the handler types are now Kit-owned. Fixes #30
This commit is contained in:
+15
-2
@@ -243,7 +243,7 @@ host.ClearSession()
|
||||
|
||||
## Re-exported Types
|
||||
|
||||
The SDK re-exports types so you don't need direct internal imports:
|
||||
The SDK re-exports message/session/MCP types so you don't need direct internal imports. Agent-configuration types are Kit-owned (not aliases) and use only SDK types in their signatures, so consumers never need to import the underlying LLM-provider package.
|
||||
|
||||
```go
|
||||
// Message types
|
||||
@@ -251,13 +251,26 @@ kit.Message, kit.MessageRole, kit.ContentPart
|
||||
kit.TextContent, kit.ReasoningContent, kit.ToolCall, kit.ToolResult, kit.Finish
|
||||
kit.RoleUser, kit.RoleAssistant, kit.RoleTool, kit.RoleSystem
|
||||
|
||||
// LLM types — concrete Kit-owned structs, no external library dependency
|
||||
// LLM types — Kit-owned `LLM*` aliases over the underlying provider types,
|
||||
// so consumers never import the provider package directly
|
||||
kit.LLMMessage // {Role LLMMessageRole, Content string}
|
||||
kit.LLMMessageRole // "user" | "assistant" | "system" | "tool"
|
||||
kit.LLMUsage // {InputTokens, OutputTokens, TotalTokens, ...}
|
||||
kit.LLMResponse // {Content, FinishReason, Usage}
|
||||
kit.LLMFilePart // {Filename, Data []byte, MediaType}
|
||||
|
||||
// Agent configuration — concrete Kit-owned structs and function types.
|
||||
// All fields use SDK types (e.g. `[]kit.Tool`), so consumers can construct
|
||||
// these without importing any LLM-provider package.
|
||||
kit.AgentConfig // Lower-level agent config — prefer Options unless you need direct control
|
||||
kit.ToolCallHandler // func(toolCallID, toolName, toolArgs string)
|
||||
kit.ToolExecutionHandler // func(toolCallID, toolName, toolArgs string, isStarting bool)
|
||||
kit.ToolResultHandler // func(toolCallID, toolName, toolArgs, result, metadata string, isError bool)
|
||||
kit.ResponseHandler // func(content string)
|
||||
kit.StreamingResponseHandler // func(content string)
|
||||
kit.ToolCallContentHandler // func(content string)
|
||||
kit.SpinnerFunc // func(fn func() error) error
|
||||
|
||||
// MCP OAuth types
|
||||
kit.MCPServer // *server.MCPServer for in-process MCP transport
|
||||
kit.MCPServerConfig // Configuration for an MCP server (stdio, SSE, or in-process)
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
package kit
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/mark3labs/kit/internal/agent"
|
||||
)
|
||||
|
||||
// TestAgentConfigToInternal verifies that the SDK-side AgentConfig converts
|
||||
// faithfully to the internal agent.AgentConfig representation, preserving
|
||||
// every field consumed by the internal agent layer.
|
||||
//
|
||||
// Regression test for https://github.com/mark3labs/kit/issues/30.
|
||||
func TestAgentConfigToInternal(t *testing.T) {
|
||||
t.Run("nil receiver returns nil", func(t *testing.T) {
|
||||
var c *AgentConfig
|
||||
if got := c.toInternal(); got != nil {
|
||||
t.Errorf("nil.toInternal() = %v, want nil", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("scalar fields round-trip", func(t *testing.T) {
|
||||
c := &AgentConfig{
|
||||
SystemPrompt: "sys",
|
||||
MaxSteps: 7,
|
||||
StreamingEnabled: true,
|
||||
DisableCoreTools: true,
|
||||
}
|
||||
got := c.toInternal()
|
||||
if got == nil {
|
||||
t.Fatal("toInternal() = nil")
|
||||
}
|
||||
if got.SystemPrompt != "sys" {
|
||||
t.Errorf("SystemPrompt = %q, want %q", got.SystemPrompt, "sys")
|
||||
}
|
||||
if got.MaxSteps != 7 {
|
||||
t.Errorf("MaxSteps = %d, want 7", got.MaxSteps)
|
||||
}
|
||||
if !got.StreamingEnabled {
|
||||
t.Error("StreamingEnabled = false, want true")
|
||||
}
|
||||
if !got.DisableCoreTools {
|
||||
t.Error("DisableCoreTools = false, want true")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("tool slices propagate without conversion", func(t *testing.T) {
|
||||
// Tool is a type alias for the underlying LLM-tool type, so the
|
||||
// SDK []Tool and internal []fantasy.AgentTool slices share the
|
||||
// same backing array after conversion.
|
||||
tool := NewTool[struct{}]("noop", "noop", nil)
|
||||
c := &AgentConfig{
|
||||
CoreTools: []Tool{tool},
|
||||
ExtraTools: []Tool{tool, tool},
|
||||
}
|
||||
got := c.toInternal()
|
||||
if len(got.CoreTools) != 1 {
|
||||
t.Errorf("CoreTools len = %d, want 1", len(got.CoreTools))
|
||||
}
|
||||
if len(got.ExtraTools) != 2 {
|
||||
t.Errorf("ExtraTools len = %d, want 2", len(got.ExtraTools))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("tool wrapper is invoked through internal config", func(t *testing.T) {
|
||||
called := false
|
||||
c := &AgentConfig{
|
||||
ToolWrapper: func(in []Tool) []Tool {
|
||||
called = true
|
||||
return in
|
||||
},
|
||||
}
|
||||
got := c.toInternal()
|
||||
if got.ToolWrapper == nil {
|
||||
t.Fatal("internal ToolWrapper is nil")
|
||||
}
|
||||
_ = got.ToolWrapper(nil)
|
||||
if !called {
|
||||
t.Error("SDK ToolWrapper was not invoked through the internal config")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("OnMCPServerLoaded propagates", func(t *testing.T) {
|
||||
var captured string
|
||||
wantErr := errors.New("boom")
|
||||
c := &AgentConfig{
|
||||
OnMCPServerLoaded: func(name string, _ int, _ error) {
|
||||
captured = name
|
||||
},
|
||||
}
|
||||
got := c.toInternal()
|
||||
got.OnMCPServerLoaded("svr", 3, wantErr)
|
||||
if captured != "svr" {
|
||||
t.Errorf("OnMCPServerLoaded captured = %q, want %q", captured, "svr")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("auth and token store factories are wired", func(t *testing.T) {
|
||||
auth := &fakeAuthHandler{}
|
||||
tokenCalls := 0
|
||||
var tokenServer string
|
||||
factory := MCPTokenStoreFactory(func(server string) (MCPTokenStore, error) {
|
||||
tokenCalls++
|
||||
tokenServer = server
|
||||
return nil, nil
|
||||
})
|
||||
c := &AgentConfig{
|
||||
AuthHandler: auth,
|
||||
TokenStoreFactory: factory,
|
||||
}
|
||||
got := c.toInternal()
|
||||
if got.AuthHandler == nil {
|
||||
t.Fatal("internal AuthHandler is nil")
|
||||
}
|
||||
if got.TokenStoreFactory == nil {
|
||||
t.Fatal("internal TokenStoreFactory is nil")
|
||||
}
|
||||
_, _ = got.TokenStoreFactory("https://example.test")
|
||||
if tokenCalls != 1 {
|
||||
t.Errorf("token factory call count = %d, want 1", tokenCalls)
|
||||
}
|
||||
if tokenServer != "https://example.test" {
|
||||
t.Errorf("token factory server arg = %q", tokenServer)
|
||||
}
|
||||
if got.AuthHandler.RedirectURI() != "redirect" {
|
||||
t.Errorf("RedirectURI = %q, want %q", got.AuthHandler.RedirectURI(), "redirect")
|
||||
}
|
||||
})
|
||||
|
||||
// Compile-time check that the internal type is what we expect.
|
||||
//nolint:staticcheck // QF1011: explicit type asserts the conversion target.
|
||||
var _ *agent.AgentConfig = (&AgentConfig{}).toInternal()
|
||||
}
|
||||
|
||||
// fakeAuthHandler implements both kit.MCPAuthHandler and the structurally
|
||||
// identical tools.MCPAuthHandler used by the internal layer.
|
||||
type fakeAuthHandler struct{}
|
||||
|
||||
func (f *fakeAuthHandler) RedirectURI() string { return "redirect" }
|
||||
func (f *fakeAuthHandler) HandleAuth(_ context.Context, _ string, _ string) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
+1
-1
@@ -1489,7 +1489,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) {
|
||||
|
||||
if opts.CLI != nil {
|
||||
setupOpts.ShowSpinner = opts.CLI.ShowSpinner
|
||||
setupOpts.SpinnerFunc = opts.CLI.SpinnerFunc
|
||||
setupOpts.SpinnerFunc = agent.SpinnerFunc(opts.CLI.SpinnerFunc)
|
||||
setupOpts.UseBufferedLogger = opts.CLI.UseBufferedLogger
|
||||
if opts.CLI.ProgressReaderFunc != nil {
|
||||
providerConfig.ProgressReaderFunc = opts.CLI.ProgressReaderFunc
|
||||
|
||||
+111
-18
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/mark3labs/kit/internal/message"
|
||||
"github.com/mark3labs/kit/internal/models"
|
||||
"github.com/mark3labs/kit/internal/session"
|
||||
"github.com/mark3labs/kit/internal/tools"
|
||||
"github.com/mark3labs/mcp-go/client/transport"
|
||||
"github.com/mark3labs/mcp-go/server"
|
||||
)
|
||||
@@ -75,25 +76,117 @@ type Config = config.Config
|
||||
// local (stdio) and remote (StreamableHTTP/SSE) server types.
|
||||
type MCPServerConfig = config.MCPServerConfig
|
||||
|
||||
// ==== Agent Types (internal/agent/) ====
|
||||
// ==== Agent Types ====
|
||||
|
||||
// AgentConfig holds configuration options for creating a new Agent.
|
||||
type AgentConfig = agent.AgentConfig
|
||||
// AgentConfig holds configuration options for constructing an agent at the
|
||||
// SDK boundary. All fields use SDK-owned types, so consumers can populate
|
||||
// this struct without importing any underlying LLM-provider package.
|
||||
//
|
||||
// For most use cases, prefer the high-level [New] entry point with
|
||||
// [Options]. AgentConfig is exposed for advanced consumers that need
|
||||
// direct access to the lower-level agent configuration shape.
|
||||
type AgentConfig struct {
|
||||
// ModelConfig holds the LLM provider configuration. A nil value means
|
||||
// that the default provider/model resolution will be used.
|
||||
ModelConfig *ProviderConfig
|
||||
|
||||
type (
|
||||
// ToolCallHandler is a function type for handling tool calls as they happen.
|
||||
ToolCallHandler = agent.ToolCallHandler
|
||||
// ToolExecutionHandler is a function type for handling tool execution start/end events.
|
||||
ToolExecutionHandler = agent.ToolExecutionHandler
|
||||
// ToolResultHandler is a function type for handling tool results.
|
||||
ToolResultHandler = agent.ToolResultHandler
|
||||
// ResponseHandler is a function type for handling LLM responses.
|
||||
ResponseHandler = agent.ResponseHandler
|
||||
// StreamingResponseHandler is a function type for handling streaming LLM responses.
|
||||
StreamingResponseHandler = agent.StreamingResponseHandler
|
||||
// ToolCallContentHandler is a function type for handling content that accompanies tool calls.
|
||||
ToolCallContentHandler = agent.ToolCallContentHandler
|
||||
)
|
||||
// MCPConfig describes any MCP servers whose tools should be loaded
|
||||
// alongside core tools.
|
||||
MCPConfig *Config
|
||||
|
||||
// SystemPrompt is the system prompt sent to the LLM.
|
||||
SystemPrompt string
|
||||
|
||||
// MaxSteps caps the number of LLM iterations per turn. A value of
|
||||
// zero means no cap is applied at this layer.
|
||||
MaxSteps int
|
||||
|
||||
// StreamingEnabled controls whether the agent streams responses.
|
||||
StreamingEnabled bool
|
||||
|
||||
// AuthHandler handles OAuth authorization for remote MCP servers.
|
||||
// When nil, remote MCP servers requiring OAuth will fail to connect.
|
||||
AuthHandler MCPAuthHandler
|
||||
|
||||
// TokenStoreFactory, if non-nil, creates a custom token store for each
|
||||
// remote MCP server's OAuth tokens. When nil, the default file-based
|
||||
// token store is used.
|
||||
TokenStoreFactory MCPTokenStoreFactory
|
||||
|
||||
// CoreTools overrides the default core tool set. If empty, [AllTools]
|
||||
// is used. Provide a custom tool set (e.g. [CodingTools] or tools
|
||||
// built with a custom WorkDir) to scope agent capabilities.
|
||||
CoreTools []Tool
|
||||
|
||||
// DisableCoreTools, when true, prevents loading any core tools.
|
||||
// Combined with empty CoreTools this yields a chat-only agent with
|
||||
// no built-in tools.
|
||||
DisableCoreTools bool
|
||||
|
||||
// ExtraTools are additional tools loaded alongside core and MCP tools.
|
||||
ExtraTools []Tool
|
||||
|
||||
// ToolWrapper, if non-nil, wraps the combined tool list before it is
|
||||
// handed to the LLM. Used to intercept tool calls or results.
|
||||
ToolWrapper func([]Tool) []Tool
|
||||
|
||||
// OnMCPServerLoaded, if non-nil, is invoked once for each MCP server
|
||||
// when its tools have finished loading (or failed). Called from a
|
||||
// background goroutine.
|
||||
OnMCPServerLoaded func(serverName string, toolCount int, err error)
|
||||
}
|
||||
|
||||
// toInternal converts an AgentConfig to its internal representation.
|
||||
// Slice and function fields convert without allocation because [Tool]
|
||||
// is a type alias for the underlying LLM-tool type.
|
||||
func (c *AgentConfig) toInternal() *agent.AgentConfig {
|
||||
if c == nil {
|
||||
return nil
|
||||
}
|
||||
out := &agent.AgentConfig{
|
||||
ModelConfig: c.ModelConfig,
|
||||
MCPConfig: c.MCPConfig,
|
||||
SystemPrompt: c.SystemPrompt,
|
||||
MaxSteps: c.MaxSteps,
|
||||
StreamingEnabled: c.StreamingEnabled,
|
||||
CoreTools: c.CoreTools,
|
||||
DisableCoreTools: c.DisableCoreTools,
|
||||
ExtraTools: c.ExtraTools,
|
||||
ToolWrapper: c.ToolWrapper,
|
||||
OnMCPServerLoaded: c.OnMCPServerLoaded,
|
||||
}
|
||||
if c.AuthHandler != nil {
|
||||
out.AuthHandler = c.AuthHandler
|
||||
}
|
||||
if c.TokenStoreFactory != nil {
|
||||
out.TokenStoreFactory = tools.TokenStoreFactory(c.TokenStoreFactory)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// ToolCallHandler is invoked when the LLM produces a tool call. It receives
|
||||
// the call ID, tool name, and the JSON-encoded input arguments.
|
||||
type ToolCallHandler func(toolCallID, toolName, toolArgs string)
|
||||
|
||||
// ToolExecutionHandler is invoked at the start and end of tool execution.
|
||||
// The isStarting flag distinguishes the two phases.
|
||||
type ToolExecutionHandler func(toolCallID, toolName, toolArgs string, isStarting bool)
|
||||
|
||||
// ToolResultHandler is invoked after a tool finishes executing. The metadata
|
||||
// parameter carries optional structured data (e.g. file-diff info) from the
|
||||
// tool execution, JSON-encoded; it may be empty.
|
||||
type ToolResultHandler func(toolCallID, toolName, toolArgs, result, metadata string, isError bool)
|
||||
|
||||
// ResponseHandler is invoked with the final assistant text for each turn.
|
||||
type ResponseHandler func(content string)
|
||||
|
||||
// StreamingResponseHandler is invoked with each streamed text delta as it
|
||||
// arrives from the LLM.
|
||||
type StreamingResponseHandler func(content string)
|
||||
|
||||
// ToolCallContentHandler is invoked with any assistant text that accompanies
|
||||
// a tool call within the same step.
|
||||
type ToolCallContentHandler func(content string)
|
||||
|
||||
// ==== Provider & Model Types (internal/models/) ====
|
||||
|
||||
@@ -126,7 +219,7 @@ type ModelsRegistry = models.ModelsRegistry
|
||||
|
||||
// SpinnerFunc wraps a function in a loading spinner animation. Used for
|
||||
// Ollama model loading. Signature: func(fn func() error) error.
|
||||
type SpinnerFunc = agent.SpinnerFunc
|
||||
type SpinnerFunc func(fn func() error) error
|
||||
|
||||
// ==== LLM Types ====
|
||||
//
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package kit_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
@@ -263,6 +264,101 @@ func TestConvertFromLLMMessage(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentConfigNoFantasyImport verifies AgentConfig can be populated with
|
||||
// every field — including CoreTools, ExtraTools, and ToolWrapper — using
|
||||
// only SDK-owned types. This test deliberately does not import
|
||||
// "charm.land/fantasy"; the package compiling at all is the proof that the
|
||||
// SDK no longer leaks the dependency name through AgentConfig.
|
||||
//
|
||||
// Regression test for https://github.com/mark3labs/kit/issues/30.
|
||||
func TestAgentConfigNoFantasyImport(t *testing.T) {
|
||||
myTool := kit.NewTool[struct{}]("noop", "does nothing", func(_ context.Context, _ struct{}) (kit.ToolOutput, error) {
|
||||
return kit.TextResult("ok"), nil
|
||||
})
|
||||
|
||||
wrapperCalled := false
|
||||
cfg := kit.AgentConfig{
|
||||
SystemPrompt: "you are a tester",
|
||||
MaxSteps: 5,
|
||||
StreamingEnabled: true,
|
||||
CoreTools: []kit.Tool{myTool},
|
||||
ExtraTools: []kit.Tool{myTool},
|
||||
DisableCoreTools: false,
|
||||
ToolWrapper: func(in []kit.Tool) []kit.Tool {
|
||||
wrapperCalled = true
|
||||
return in
|
||||
},
|
||||
OnMCPServerLoaded: func(_ string, _ int, _ error) {},
|
||||
}
|
||||
|
||||
if cfg.SystemPrompt != "you are a tester" {
|
||||
t.Errorf("SystemPrompt = %q, want %q", cfg.SystemPrompt, "you are a tester")
|
||||
}
|
||||
if cfg.MaxSteps != 5 {
|
||||
t.Errorf("MaxSteps = %d, want 5", cfg.MaxSteps)
|
||||
}
|
||||
if !cfg.StreamingEnabled {
|
||||
t.Error("StreamingEnabled = false, want true")
|
||||
}
|
||||
if len(cfg.CoreTools) != 1 {
|
||||
t.Errorf("CoreTools len = %d, want 1", len(cfg.CoreTools))
|
||||
}
|
||||
if len(cfg.ExtraTools) != 1 {
|
||||
t.Errorf("ExtraTools len = %d, want 1", len(cfg.ExtraTools))
|
||||
}
|
||||
|
||||
// Exercise the wrapper to confirm the func type is usable.
|
||||
out := cfg.ToolWrapper(cfg.CoreTools)
|
||||
if !wrapperCalled {
|
||||
t.Error("ToolWrapper was not invoked")
|
||||
}
|
||||
if len(out) != 1 {
|
||||
t.Errorf("wrapped tool list len = %d, want 1", len(out))
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentConfigToolWrapperSignature documents that AgentConfig.ToolWrapper
|
||||
// uses kit.Tool (not the underlying provider type) in its signature.
|
||||
func TestAgentConfigToolWrapperSignature(t *testing.T) {
|
||||
//nolint:staticcheck // QF1011: explicit type asserts the SDK-side func signature.
|
||||
var _ func([]kit.Tool) []kit.Tool = func(in []kit.Tool) []kit.Tool { return in }
|
||||
cfg := kit.AgentConfig{
|
||||
ToolWrapper: func(in []kit.Tool) []kit.Tool { return in },
|
||||
}
|
||||
if cfg.ToolWrapper == nil {
|
||||
t.Fatal("ToolWrapper assignment failed")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSpinnerFuncSignature verifies SpinnerFunc has the documented signature
|
||||
// and can be constructed without importing any provider package.
|
||||
func TestSpinnerFuncSignature(t *testing.T) {
|
||||
called := false
|
||||
var sp kit.SpinnerFunc = func(fn func() error) error {
|
||||
called = true
|
||||
return fn()
|
||||
}
|
||||
err := sp(func() error { return nil })
|
||||
if err != nil {
|
||||
t.Errorf("SpinnerFunc returned err: %v", err)
|
||||
}
|
||||
if !called {
|
||||
t.Error("SpinnerFunc did not invoke fn")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandlerTypesSignatures verifies the SDK-owned handler function types
|
||||
// can be assigned from plain function literals using only standard library
|
||||
// types in their signatures (no provider-package import required).
|
||||
func TestHandlerTypesSignatures(t *testing.T) {
|
||||
var _ kit.ToolCallHandler = func(_, _, _ string) {}
|
||||
var _ kit.ToolExecutionHandler = func(_, _, _ string, _ bool) {}
|
||||
var _ kit.ToolResultHandler = func(_, _, _, _, _ string, _ bool) {}
|
||||
var _ kit.ResponseHandler = func(_ string) {}
|
||||
var _ kit.StreamingResponseHandler = func(_ string) {}
|
||||
var _ kit.ToolCallContentHandler = func(_ string) {}
|
||||
}
|
||||
|
||||
// containsStr is a tiny helper to avoid importing strings in test.
|
||||
func containsStr(s, substr string) bool {
|
||||
return len(s) >= len(substr) && (s == substr || len(s) > 0 && indexStr(s, substr) >= 0)
|
||||
|
||||
Reference in New Issue
Block a user