diff --git a/README.md b/README.md index 11565170..f426e73d 100644 --- a/README.md +++ b/README.md @@ -556,7 +556,7 @@ host, err := kit.New(ctx, &kit.Options{ SystemPrompt: "You are a helpful bot", ConfigFile: "/path/to/config.yml", MaxSteps: 10, - Streaming: true, + Streaming: ptr(true), // *bool: nil = unset (default true), &false = off Quiet: true, // Generation parameters (override env/config/per-model defaults) @@ -603,6 +603,38 @@ are pointer types so explicit `0.0` is distinguishable from "leave alone"; a non-zero `MaxTokens` suppresses automatic right-sizing the same way `--max-tokens` does on the CLI. +### Functional options (`NewAgent`) + +For simple programmatic setups, `kit.NewAgent` offers an ergonomic +functional-options front door over `kit.New`. Streaming is **enabled by +default**; pass `kit.WithStreaming(false)` to opt out. + +```go +host, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithSystemPrompt("You are a helpful assistant."), + kit.WithMaxTokens(8192), + kit.WithThinkingLevel("medium"), + kit.Ephemeral(), // in-memory session, no persistence +) +``` + +Available options: `WithModel`, `WithSystemPrompt`, `WithStreaming`, +`WithMaxTokens`, `WithThinkingLevel`, `WithTools`, `WithExtraTools`, +`WithProviderAPIKey`, `WithProviderURL`, `WithConfigFile`, `WithDebug`, and +`Ephemeral`. For advanced configuration not covered by the helpers (custom MCP +config, in-process MCP servers, session backends, MCP task tuning) construct an +`Options` value explicitly and call `kit.New`. + +### Per-instance config isolation + +Each `kit.New` / `kit.NewAgent` call owns an **isolated configuration store**, +so constructing multiple Kit instances in the same process is safe: setting the +model, thinking level, or generation parameters on one never affects another, +and runtime mutators (`SetModel`, `SetThinkingLevel`) only touch the owning +instance. This makes subagent spawning and multi-Kit embedding race-free with +no external synchronization required. + ### MCP OAuth (remote MCP servers) When a remote MCP server returns 401, Kit runs the full OAuth flow (dynamic diff --git a/examples/sdk/README.md b/examples/sdk/README.md index 7db0b2f9..b5313416 100644 --- a/examples/sdk/README.md +++ b/examples/sdk/README.md @@ -42,4 +42,14 @@ defer host.Close() response, err := host.Prompt(ctx, "Hello!") ``` +Or use the functional-options constructor for quick setups (streaming defaults on): + +```go +host, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithSystemPrompt("You are a helpful assistant."), + kit.Ephemeral(), +) +``` + See the [SDK README](../../pkg/kit/README.md) for the full API reference. diff --git a/internal/acpserver/session.go b/internal/acpserver/session.go index af18a862..937cc6ad 100644 --- a/internal/acpserver/session.go +++ b/internal/acpserver/session.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/charmbracelet/log" + "github.com/spf13/viper" "github.com/mark3labs/kit/internal/extbridge" "github.com/mark3labs/kit/internal/extensions" @@ -38,10 +39,21 @@ func newSessionRegistry() *sessionRegistry { // given working directory. The Kit-generated session ID is used as the ACP // session ID so the mapping is 1:1. func (r *sessionRegistry) create(ctx context.Context, cwd string) (*acpSession, error) { + // Each ACP session gets its own isolated config store (CLI is left nil) so + // per-session SetModel / SetThinkingLevel calls cannot race or bleed across + // the sessionRegistry. We seed the relevant root-command flag values from + // the process-global store (which cobra populated from flags) so launching + // `kit acp -m [--thinking-level ...] [--provider-url ...]` is still + // honored; .kit.yml and KIT_* env vars are loaded per session by kit.New. + streamOn := true kitInstance, err := kit.New(ctx, &kit.Options{ - SessionDir: cwd, - Quiet: true, - Streaming: true, + SessionDir: cwd, + Quiet: true, + Streaming: &streamOn, + Model: viper.GetString("model"), + ThinkingLevel: viper.GetString("thinking-level"), + ProviderURL: viper.GetString("provider-url"), + ProviderAPIKey: viper.GetString("provider-api-key"), }) if err != nil { // Provide actionable guidance for provider auth errors, which are diff --git a/internal/config/merger.go b/internal/config/merger.go index 64459ad9..d64eab58 100644 --- a/internal/config/merger.go +++ b/internal/config/merger.go @@ -7,32 +7,48 @@ import ( "github.com/spf13/viper" ) -// LoadAndValidateConfig loads configuration from viper, fixes environment variable -// casing issues, and validates the configuration. Returns an error if loading or -// validation fails. +// LoadAndValidateConfig loads configuration from the process-global viper +// store, fixes environment variable casing issues, and validates the +// configuration. Returns an error if loading or validation fails. +// +// This is a convenience wrapper around [LoadAndValidateConfigFrom] using the +// shared global store; it is retained for the CLI and other callers that rely +// on viper's process-global state. func LoadAndValidateConfig() (*Config, error) { + return LoadAndValidateConfigFrom(viper.GetViper()) +} + +// LoadAndValidateConfigFrom loads configuration from the supplied per-instance +// store, fixes environment variable casing issues, and validates the +// configuration. When v is nil, the process-global store is used. Threading an +// explicit store lets each Kit instance own an isolated configuration without +// clobbering other instances in the same process. +func LoadAndValidateConfigFrom(v *viper.Viper) (*Config, error) { + if v == nil { + v = viper.GetViper() + } config := &Config{ MCPServers: make(map[string]MCPServerConfig), } - if err := viper.Unmarshal(config); err != nil { - return nil, fmt.Errorf("failed to unmarshal config: %v", err) + if err := v.Unmarshal(config); err != nil { + return nil, fmt.Errorf("failed to unmarshal config: %w", err) } // Fix environment variable case sensitivity issue // Viper lowercases all keys, but we need to preserve the original case for environment variables - fixEnvironmentCase(config) + fixEnvironmentCase(v, config) if err := config.Validate(); err != nil { - return nil, fmt.Errorf("invalid config: %v", err) + return nil, fmt.Errorf("invalid config: %w", err) } return config, nil } // fixEnvironmentCase fixes the case of environment variable keys that were lowercased by Viper -func fixEnvironmentCase(config *Config) { +func fixEnvironmentCase(v *viper.Viper, config *Config) { // Get the raw config data from viper - rawConfig := viper.AllSettings() + rawConfig := v.AllSettings() // Check if we have mcpServers in the raw config if mcpServersRaw, ok := rawConfig["mcpservers"]; ok { diff --git a/internal/extensions/runner.go b/internal/extensions/runner.go index 0bd6aeac..919cfbf5 100644 --- a/internal/extensions/runner.go +++ b/internal/extensions/runner.go @@ -98,9 +98,20 @@ type Runner struct { disabledTools map[string]bool // nil = all tools enabled customEventSubs map[string][]func(string) // inter-extension event bus optionOverrides map[string]string // runtime option overrides + configStore *viper.Viper // per-instance config store (nil = global) mu sync.RWMutex } +// SetConfigStore sets the per-instance configuration store used by GetOption +// to resolve "options." config values. When unset (nil), GetOption falls +// back to the process-global viper store. Threading a per-Kit store keeps +// extension option resolution isolated between Kit instances. +func (r *Runner) SetConfigStore(v *viper.Viper) { + r.mu.Lock() + defer r.mu.Unlock() + r.configStore = v +} + // ShortcutEntry pairs a shortcut definition with its handler. type ShortcutEntry struct { Def ShortcutDef @@ -872,7 +883,13 @@ func (r *Runner) GetOption(name string) string { // 3. Viper config: options. configKey := "options." + name - if v := viper.GetString(configKey); v != "" { + r.mu.RLock() + store := r.configStore + r.mu.RUnlock() + if store == nil { + store = viper.GetViper() + } + if v := store.GetString(configKey); v != "" { return v } diff --git a/internal/kitsetup/setup.go b/internal/kitsetup/setup.go index 2d276104..a02010dd 100644 --- a/internal/kitsetup/setup.go +++ b/internal/kitsetup/setup.go @@ -46,9 +46,9 @@ type AgentSetupOptions struct { ToolWrapper func([]fantasy.AgentTool) []fantasy.AgentTool // ProviderConfig, when non-nil, is used directly instead of calling - // BuildProviderConfig(). Callers that already hold viperInitMu can - // pre-build this and release the lock before calling SetupAgent, so the - // slow agent/MCP initialisation runs concurrently with other New() calls. + // BuildProviderConfig(). Callers (e.g. Kit.New) pre-build this from their + // per-instance config store and pass it here, so the slow agent/MCP + // initialisation can run without further config reads. ProviderConfig *models.ProviderConfig // Debug enables debug logging. When zero-value, viper is consulted. // Only meaningful when ProviderConfig is also set. @@ -75,6 +75,11 @@ type AgentSetupOptions struct { // MCPTaskConfig configures task-augmented tools/call execution. The // zero value preserves historical synchronous-only behaviour. MCPTaskConfig tools.MCPTaskConfig + // Viper is the per-instance configuration store. When set, it is used for + // any fallback config reads (debug, no-extensions, max-steps, stream, + // extension paths) and is attached to the extension runner. When nil, the + // process-global viper store is used. + Viper *viper.Viper } // AgentSetupResult bundles the created agent and any debug logger so the caller @@ -87,57 +92,62 @@ type AgentSetupResult struct { ExtRunner *extensions.Runner } -// BuildProviderConfig creates a *models.ProviderConfig from the current viper -// state. All entry points (root, script, SDK) converge through this function. +// BuildProviderConfig creates a *models.ProviderConfig from the supplied viper +// store (or the process-global store when v is nil). All entry points (root, +// script, SDK) converge through this function. // // Generation parameter pointers (Temperature, TopP, etc.) are only set when // the user has explicitly configured them via CLI flag, environment variable, // or global config file. This allows per-model defaults from modelSettings // and customModels to fill in unset parameters downstream. -func BuildProviderConfig() (*models.ProviderConfig, string, error) { - systemPrompt, err := config.LoadSystemPrompt(viper.GetString("system-prompt")) +func BuildProviderConfig(v *viper.Viper) (*models.ProviderConfig, string, error) { + if v == nil { + v = viper.GetViper() + } + systemPrompt, err := config.LoadSystemPrompt(v.GetString("system-prompt")) if err != nil { return nil, "", fmt.Errorf("failed to load system prompt: %w", err) } - numGPU := int32(viper.GetInt("num-gpu-layers")) - mainGPU := int32(viper.GetInt("main-gpu")) + numGPU := int32(v.GetInt("num-gpu-layers")) + mainGPU := int32(v.GetInt("main-gpu")) cfg := &models.ProviderConfig{ - ModelString: viper.GetString("model"), + ModelString: v.GetString("model"), SystemPrompt: systemPrompt, - ProviderAPIKey: viper.GetString("provider-api-key"), - ProviderURL: viper.GetString("provider-url"), - MaxTokens: viper.GetInt("max-tokens"), - StopSequences: viper.GetStringSlice("stop-sequences"), + ProviderAPIKey: v.GetString("provider-api-key"), + ProviderURL: v.GetString("provider-url"), + MaxTokens: v.GetInt("max-tokens"), + StopSequences: v.GetStringSlice("stop-sequences"), NumGPU: &numGPU, MainGPU: &mainGPU, - TLSSkipVerify: viper.GetBool("tls-skip-verify"), - ThinkingLevel: models.ParseThinkingLevel(viper.GetString("thinking-level")), + TLSSkipVerify: v.GetBool("tls-skip-verify"), + ThinkingLevel: models.ParseThinkingLevel(v.GetString("thinking-level")), + ConfigStore: v, } // Only set generation parameter pointers when the user has explicitly // provided a value. This leaves nil pointers for unset params, allowing // per-model defaults (modelSettings / customModels params) to apply. - if viper.IsSet("temperature") { - v := float32(viper.GetFloat64("temperature")) - cfg.Temperature = &v + if v.IsSet("temperature") { + val := float32(v.GetFloat64("temperature")) + cfg.Temperature = &val } - if viper.IsSet("top-p") { - v := float32(viper.GetFloat64("top-p")) - cfg.TopP = &v + if v.IsSet("top-p") { + val := float32(v.GetFloat64("top-p")) + cfg.TopP = &val } - if viper.IsSet("top-k") { - v := int32(viper.GetInt("top-k")) - cfg.TopK = &v + if v.IsSet("top-k") { + val := int32(v.GetInt("top-k")) + cfg.TopK = &val } - if viper.IsSet("frequency-penalty") { - v := float32(viper.GetFloat64("frequency-penalty")) - cfg.FrequencyPenalty = &v + if v.IsSet("frequency-penalty") { + val := float32(v.GetFloat64("frequency-penalty")) + cfg.FrequencyPenalty = &val } - if viper.IsSet("presence-penalty") { - v := float32(viper.GetFloat64("presence-penalty")) - cfg.PresencePenalty = &v + if v.IsSet("presence-penalty") { + val := float32(v.GetFloat64("presence-penalty")) + cfg.PresencePenalty = &val } return cfg, systemPrompt, nil @@ -149,14 +159,21 @@ func SetupAgent(ctx context.Context, opts AgentSetupOptions) (*AgentSetupResult, var modelConfig *models.ProviderConfig var systemPrompt string + // Resolve the config store: prefer the per-instance store, falling back to + // the process-global store. + v := opts.Viper + if v == nil { + v = viper.GetViper() + } + if opts.ProviderConfig != nil { - // Pre-built config supplied by caller (e.g. Kit.New after releasing - // viperInitMu). Use it directly — no viper reads needed here. + // Pre-built config supplied by caller (e.g. Kit.New after building the + // per-instance store). Use it directly — no viper reads needed here. modelConfig = opts.ProviderConfig systemPrompt = modelConfig.SystemPrompt } else { var err error - modelConfig, systemPrompt, err = BuildProviderConfig() + modelConfig, systemPrompt, err = BuildProviderConfig(v) if err != nil { return nil, err } @@ -164,13 +181,13 @@ func SetupAgent(ctx context.Context, opts AgentSetupOptions) (*AgentSetupResult, // Resolve debug / no-extensions / max-steps / streaming: prefer explicit // fields (set when ProviderConfig was pre-built) over viper fallback. - debugEnabled := opts.Debug || viper.GetBool("debug") - noExtensions := opts.NoExtensions || viper.GetBool("no-extensions") + debugEnabled := opts.Debug || v.GetBool("debug") + noExtensions := opts.NoExtensions || v.GetBool("no-extensions") maxSteps := opts.MaxSteps if maxSteps == 0 { - maxSteps = viper.GetInt("max-steps") + maxSteps = v.GetInt("max-steps") } - streamingEnabled := opts.StreamingEnabled || viper.GetBool("stream") + streamingEnabled := opts.StreamingEnabled || v.GetBool("stream") // Create the appropriate debug logger. var debugLogger tools.DebugLogger @@ -189,7 +206,7 @@ func SetupAgent(ctx context.Context, opts AgentSetupOptions) (*AgentSetupResult, var extCreationOpts extensionCreationOpts if !noExtensions { var extErr error - extRunner, extCreationOpts, extErr = loadExtensions() + extRunner, extCreationOpts, extErr = loadExtensions(v) if extErr != nil { fmt.Printf("Warning: Failed to load extensions: %v\n", extErr) } @@ -253,9 +270,14 @@ type extensionCreationOpts struct { } // loadExtensions discovers and loads Yaegi extensions, builds the runner, -// and returns the tool wrapper/extra tools. -func loadExtensions() (*extensions.Runner, extensionCreationOpts, error) { - extraPaths := viper.GetStringSlice("extension") +// and returns the tool wrapper/extra tools. The supplied store is used to +// resolve the "extension" config key and is attached to the runner so +// extension option lookups stay isolated to this Kit instance. +func loadExtensions(v *viper.Viper) (*extensions.Runner, extensionCreationOpts, error) { + if v == nil { + v = viper.GetViper() + } + extraPaths := v.GetStringSlice("extension") loaded, err := extensions.LoadExtensions(extraPaths) if err != nil { return nil, extensionCreationOpts{}, err @@ -266,6 +288,7 @@ func loadExtensions() (*extensions.Runner, extensionCreationOpts, error) { } runner := extensions.NewRunner(loaded) + runner.SetConfigStore(v) wrapper := func(tools []fantasy.AgentTool) []fantasy.AgentTool { return extensions.WrapToolsWithExtensions(tools, runner) diff --git a/internal/models/custom.go b/internal/models/custom.go index 43f11c5d..cce5f5a8 100644 --- a/internal/models/custom.go +++ b/internal/models/custom.go @@ -10,14 +10,24 @@ import ( // loadCustomModelsFromConfig loads custom model definitions from the config file // and returns them as a map of model ID -> ModelInfo. Returns nil if no custom -// models are configured. +// models are configured. Reads from the process-global viper store (the model +// registry is a process-global singleton). func loadCustomModelsFromConfig() map[string]ModelInfo { - if !viper.IsSet("customModels") { + return loadCustomModelsFrom(viper.GetViper()) +} + +// loadCustomModelsFrom loads custom model definitions from the supplied store. +// When v is nil the process-global store is used. +func loadCustomModelsFrom(v *viper.Viper) map[string]ModelInfo { + if v == nil { + v = viper.GetViper() + } + if !v.IsSet("customModels") { return nil } var customModels map[string]CustomModelConfig - if err := viper.UnmarshalKey("customModels", &customModels); err != nil { + if err := v.UnmarshalKey("customModels", &customModels); err != nil { log.Printf("Warning: Failed to parse customModels: %v", err) return nil } @@ -60,15 +70,26 @@ func modelConfigToModelInfo(modelID string, cfg CustomModelConfig) ModelInfo { } // LoadModelSettingsFromConfig loads per-model generation parameter overrides -// from the config file. Keys are "provider/model" strings. Returns nil if -// no model settings are configured. +// from the process-global viper store. Keys are "provider/model" strings. +// Returns nil if no model settings are configured. func LoadModelSettingsFromConfig() map[string]*GenerationParams { - if !viper.IsSet("modelSettings") { + return LoadModelSettingsFrom(viper.GetViper()) +} + +// LoadModelSettingsFrom loads per-model generation parameter overrides from the +// supplied per-instance store. When v is nil the process-global store is used. +// Keys are "provider/model" strings. Returns nil if no model settings are +// configured. +func LoadModelSettingsFrom(v *viper.Viper) map[string]*GenerationParams { + if v == nil { + v = viper.GetViper() + } + if !v.IsSet("modelSettings") { return nil } var settings map[string]GenerationParamsConfig - if err := viper.UnmarshalKey("modelSettings", &settings); err != nil { + if err := v.UnmarshalKey("modelSettings", &settings); err != nil { log.Printf("Warning: Failed to parse modelSettings: %v", err) return nil } @@ -148,12 +169,17 @@ func ApplyModelSettings(config *ProviderConfig, modelInfo *ModelInfo) { return } + // Resolve the config store: prefer the per-instance store carried on the + // ProviderConfig (set by BuildProviderConfig / Kit.New), falling back to + // the process-global store for callers that don't thread one through. + store := config.ConfigStore + // Collect model-level params: modelSettings override > custom model params. // modelSettings takes priority because it's the more specific/intentional config. var params *GenerationParams // First check modelSettings from config. - if settings := LoadModelSettingsFromConfig(); settings != nil { + if settings := LoadModelSettingsFrom(store); settings != nil { modelKey := provider + "/" + modelName if p, ok := settings[modelKey]; ok { params = p @@ -173,28 +199,28 @@ func ApplyModelSettings(config *ProviderConfig, modelInfo *ModelInfo) { // We check viper.IsSet() which returns true only when the key was // set via CLI flag, environment variable, or config file global section. - if params.MaxTokens != nil && !isExplicitlySet("max-tokens") { + if params.MaxTokens != nil && !isExplicitlySet(store, "max-tokens") { config.MaxTokens = *params.MaxTokens } - if params.Temperature != nil && !isExplicitlySet("temperature") { + if params.Temperature != nil && !isExplicitlySet(store, "temperature") { config.Temperature = params.Temperature } - if params.TopP != nil && !isExplicitlySet("top-p") { + if params.TopP != nil && !isExplicitlySet(store, "top-p") { config.TopP = params.TopP } - if params.TopK != nil && !isExplicitlySet("top-k") { + if params.TopK != nil && !isExplicitlySet(store, "top-k") { config.TopK = params.TopK } - if params.FrequencyPenalty != nil && !isExplicitlySet("frequency-penalty") { + if params.FrequencyPenalty != nil && !isExplicitlySet(store, "frequency-penalty") { config.FrequencyPenalty = params.FrequencyPenalty } - if params.PresencePenalty != nil && !isExplicitlySet("presence-penalty") { + if params.PresencePenalty != nil && !isExplicitlySet(store, "presence-penalty") { config.PresencePenalty = params.PresencePenalty } - if len(params.StopSequences) > 0 && !isExplicitlySet("stop-sequences") { + if len(params.StopSequences) > 0 && !isExplicitlySet(store, "stop-sequences") { config.StopSequences = params.StopSequences } - if params.ThinkingLevel != "" && !isExplicitlySet("thinking-level") { + if params.ThinkingLevel != "" && !isExplicitlySet(store, "thinking-level") { config.ThinkingLevel = params.ThinkingLevel } if params.SystemPrompt != "" && config.SystemPrompt == "" { @@ -228,7 +254,14 @@ func LoadSystemPromptValue(input string) string { // isExplicitlySet returns true when the user has explicitly set a config key // via CLI flag, environment variable, or the global section of the config file. // Model-level defaults should not override explicitly set values. -func isExplicitlySet(key string) bool { +// +// The check runs against the supplied per-instance store when non-nil, +// otherwise the process-global store. This keeps the "explicit vs unset" +// precedence contract per-Kit-instance once a store is threaded through. +func isExplicitlySet(v *viper.Viper, key string) bool { + if v == nil { + v = viper.GetViper() + } // viper.IsSet returns true if the key has been set in any of the // data stores (flag, env, config file, default). We need to check // whether the value was set at the global config level (not just @@ -239,7 +272,7 @@ func isExplicitlySet(key string) bool { // file values. This means global config file values (e.g. // temperature: 0.7 at the top level) will correctly take precedence // over model-level defaults, which is the desired behavior. - return viper.IsSet(key) + return v.IsSet(key) } // GenerationParams holds per-model generation parameter defaults. diff --git a/internal/models/providers.go b/internal/models/providers.go index b4ba21fc..d79858d6 100644 --- a/internal/models/providers.go +++ b/internal/models/providers.go @@ -25,6 +25,7 @@ import ( openaisdk "github.com/charmbracelet/openai-go" "github.com/mark3labs/kit/internal/auth" + "github.com/spf13/viper" ) const ( @@ -164,6 +165,13 @@ type ProviderConfig struct { ThinkingLevel ThinkingLevel DisableCaching bool // Opt-out: set to true to disable automatic prompt caching + // ConfigStore is the per-instance configuration store used to resolve + // "explicitly set" precedence checks (isExplicitlySet), per-model + // settings, and right-sizing. When nil, the process-global viper store is + // used. Threading a per-Kit store here keeps generation-parameter + // precedence isolated between Kit instances in the same process. + ConfigStore *viper.Viper + // ProgressReaderFunc, when set, wraps an io.Reader with progress display // for long operations like Ollama model pulls. The returned io.ReadCloser // must be closed when done. When nil, the raw reader is consumed directly @@ -530,7 +538,7 @@ func rightSizeMaxTokens(config *ProviderConfig, modelInfo *ModelInfo) { if modelInfo == nil || modelInfo.Limit.Output <= 0 { return } - if isExplicitlySet("max-tokens") { + if isExplicitlySet(config.ConfigStore, "max-tokens") { return } target := min(modelInfo.Limit.Output, defaultRightSizeCap) diff --git a/pkg/kit/README.md b/pkg/kit/README.md index 50d69a72..fcaafb4a 100644 --- a/pkg/kit/README.md +++ b/pkg/kit/README.md @@ -49,6 +49,36 @@ The SDK behaves identically to the CLI: - Respects all environment variables (`KIT_*`) - Uses the same defaults as the CLI +Each `kit.New` / `kit.NewAgent` call owns an **isolated configuration store**, +so constructing multiple Kit instances in the same process is safe — setting +the model, thinking level, or generation parameters on one never affects +another, and runtime mutators (`SetModel`, `SetThinkingLevel`) only touch the +owning instance. This makes subagent spawning and multi-Kit embedding race-free +without external synchronization. + +### Functional options (`NewAgent`) + +For simple programmatic setups, `kit.NewAgent` is an ergonomic +functional-options front door over `kit.New`. Streaming is enabled by default; +pass `kit.WithStreaming(false)` to opt out. + +```go +host, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithSystemPrompt("You are a helpful assistant."), + kit.WithMaxTokens(8192), + kit.WithThinkingLevel("medium"), + kit.Ephemeral(), // in-memory session, no persistence +) +``` + +Helpers: `WithModel`, `WithSystemPrompt`, `WithStreaming`, `WithMaxTokens`, +`WithThinkingLevel`, `WithTools`, `WithExtraTools`, `WithProviderAPIKey`, +`WithProviderURL`, `WithConfigFile`, `WithDebug`, and `Ephemeral`. `Option` is +a plain `func(*Options)`, so you can define your own. For fields without a +`With*` helper (`MCPConfig`, `InProcessMCPServers`, `SessionManager`, MCP task +tuning) construct an `Options` value and call `kit.New`. + ### Options You can override specific settings: @@ -59,7 +89,7 @@ host, err := kit.New(ctx, &kit.Options{ SystemPrompt: "You are a helpful bot", // Override system prompt ConfigFile: "/path/to/config.yml", // Use specific config file MaxSteps: 10, // Override max steps - Streaming: true, // Enable streaming + Streaming: ptrBool(true), // *bool: nil = unset (default true), &false = off Quiet: true, // Suppress debug output // Session options @@ -331,6 +361,7 @@ msg := kit.ConvertFromLLMMessage(lMsg) // LLMMessage → SDK Message - `Kit` - Main SDK type - `Options` - Configuration options +- `Option` - Functional option (`func(*Options)`) for `NewAgent` - `Message` - Conversation message with typed content parts - `Tool` - Agent tool interface - `TurnResult` - Full result from a prompt including usage stats @@ -338,6 +369,7 @@ msg := kit.ConvertFromLLMMessage(lMsg) // LLMMessage → SDK Message ### Key Methods - `New(ctx, opts)` - Create new Kit instance +- `NewAgent(ctx, ...Option)` - Create a Kit via functional options (streaming on by default) - `Prompt(ctx, message)` - Send message and get response string - `PromptResult(ctx, message)` - Send message and get full TurnResult - `PromptWithOptions(ctx, message, opts)` - Prompt with per-call options diff --git a/pkg/kit/config.go b/pkg/kit/config.go index 7636bb86..e6f35718 100644 --- a/pkg/kit/config.go +++ b/pkg/kit/config.go @@ -65,23 +65,46 @@ const sdkDefaultMaxTokens = 8192 // which returns models.ThinkingOff. // - sampling params (temperature, top-p, top-k, frequency/presence-penalty): // left as nil pointers so provider libraries apply their own defaults. -func setSDKDefaults() { - viper.SetDefault("model", "anthropic/claude-sonnet-4-5-20250929") - viper.SetDefault("system-prompt", defaultSystemPrompt) - viper.SetDefault("stream", true) - viper.SetDefault("num-gpu-layers", -1) - viper.SetDefault("main-gpu", 0) +func setSDKDefaults(v *viper.Viper) { + v.SetDefault("model", "anthropic/claude-sonnet-4-5-20250929") + v.SetDefault("system-prompt", defaultSystemPrompt) + v.SetDefault("stream", true) + v.SetDefault("num-gpu-layers", -1) + v.SetDefault("main-gpu", 0) } -// InitConfig initializes the viper configuration system. +// InitConfig initializes the process-global viper configuration system. // It searches for config files in standard locations and loads them with // environment variable substitution. // // configFile: explicit config file path (empty = search defaults). // debug: if true, print warnings about missing configs to stderr. +// +// This wraps [initConfig] using the process-global store and is retained for +// the CLI, which binds its flags to the global viper. func InitConfig(configFile string, debug bool) error { + return initConfig(viper.GetViper(), configFile, debug) +} + +// initConfig loads configuration into the supplied per-instance store. When v +// is nil the process-global store is used. +func initConfig(v *viper.Viper, configFile string, debug bool) error { + if v == nil { + v = viper.GetViper() + } + + // Configure KIT_* environment overrides unconditionally, before any file + // is loaded, so that an explicit config file does not disable env support. + // Map hyphenated config keys (e.g. "max-tokens") to underscored env var + // names (e.g. KIT_MAX_TOKENS); without this AutomaticEnv looks for + // KIT_MAX-TOKENS and silently misses valid overrides. Precedence is + // resolved at read time, so calling these before ReadConfig is fine. + v.SetEnvPrefix("KIT") + v.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) + v.AutomaticEnv() + if configFile != "" { - return LoadConfigWithEnvSubstitution(configFile) + return loadConfigWithEnvSubstitution(v, configFile) } // Ensure a config file exists (create default if none found). @@ -97,15 +120,15 @@ func InitConfig(configFile string, debug bool) error { } // Current directory has higher priority than home directory. - viper.AddConfigPath(".") - viper.AddConfigPath(home) + v.AddConfigPath(".") + v.AddConfigPath(home) configLoaded := false - viper.SetConfigName(".kit") - if err := viper.ReadInConfig(); err == nil { - configPath := viper.ConfigFileUsed() - if err := LoadConfigWithEnvSubstitution(configPath); err != nil { + v.SetConfigName(".kit") + if err := v.ReadInConfig(); err == nil { + configPath := v.ConfigFileUsed() + if err := loadConfigWithEnvSubstitution(v, configPath); err != nil { if strings.Contains(err.Error(), "environment variable substitution failed") { return fmt.Errorf("error reading config file '%s': %w", configPath, err) } @@ -118,17 +141,21 @@ func InitConfig(configFile string, debug bool) error { fmt.Fprintf(os.Stderr, "No config file found in current directory or home directory\n") } - viper.SetEnvPrefix("KIT") - // Map hyphenated config keys (e.g. "max-tokens") to underscored env - // var names (e.g. KIT_MAX_TOKENS). Without this, AutomaticEnv looks - // for KIT_MAX-TOKENS and silently misses valid env overrides. - viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) - viper.AutomaticEnv() return nil } -// LoadConfigWithEnvSubstitution loads a config file with ${ENV_VAR} expansion. +// LoadConfigWithEnvSubstitution loads a config file with ${ENV_VAR} expansion +// into the process-global viper store. func LoadConfigWithEnvSubstitution(configPath string) error { + return loadConfigWithEnvSubstitution(viper.GetViper(), configPath) +} + +// loadConfigWithEnvSubstitution loads a config file with ${ENV_VAR} expansion +// into the supplied per-instance store (or the global store when v is nil). +func loadConfigWithEnvSubstitution(v *viper.Viper, configPath string) error { + if v == nil { + v = viper.GetViper() + } rawContent, err := os.ReadFile(configPath) if err != nil { return fmt.Errorf("failed to read config file: %w", err) @@ -146,6 +173,6 @@ func LoadConfigWithEnvSubstitution(configPath string) error { } config.SetConfigPath(configPath) - viper.SetConfigType(configType) - return viper.ReadConfig(strings.NewReader(processedContent)) + v.SetConfigType(configType) + return v.ReadConfig(strings.NewReader(processedContent)) } diff --git a/pkg/kit/export_test.go b/pkg/kit/export_test.go new file mode 100644 index 00000000..f42f5514 --- /dev/null +++ b/pkg/kit/export_test.go @@ -0,0 +1,22 @@ +package kit + +// This file exposes a handful of internal accessors to the external kit_test +// package. Because it ends in _test.go it is only compiled during testing and +// is therefore not part of the public SDK surface. + +// ConfigValueIsSetForTest reports whether key is explicitly set in this Kit's +// isolated configuration store. Used by tests to assert the tri-state +// precedence contract per-instance. +func (m *Kit) ConfigValueIsSetForTest(key string) bool { return m.v.IsSet(key) } + +// ConfigStringForTest returns the string value of key from this Kit's isolated +// configuration store. +func (m *Kit) ConfigStringForTest(key string) string { return m.v.GetString(key) } + +// ConfigFloatForTest returns the float64 value of key from this Kit's isolated +// configuration store. +func (m *Kit) ConfigFloatForTest(key string) float64 { return m.v.GetFloat64(key) } + +// ConfigBoolForTest returns the bool value of key from this Kit's isolated +// configuration store. +func (m *Kit) ConfigBoolForTest(key string) bool { return m.v.GetBool(key) } diff --git a/pkg/kit/kit.go b/pkg/kit/kit.go index a29e8298..51c8524f 100644 --- a/pkg/kit/kit.go +++ b/pkg/kit/kit.go @@ -53,6 +53,14 @@ type Kit struct { opts *Options // stored for reload operations (skills, etc.) mcpConfig *config.Config // loaded MCP/server config, shared with subagents + // v is this Kit instance's isolated configuration store. Each Kit owns its + // own *viper.Viper (constructed via viper.New) so that runtime config + // mutators (SetModel, SetThinkingLevel) and config reads do not clobber or + // observe state from other Kit instances in the same process. When the CLI + // constructs a Kit (Options.CLI != nil) this points at the process-global + // store so cobra flag bindings remain in effect. + v *viper.Viper + // hasCustomSystemPrompt is true when the user explicitly configured a // system prompt (via --system-prompt flag, config file, or SDK option). // When false, per-model system prompts from modelSettings/customModels @@ -555,8 +563,8 @@ func (m *Kit) SetModel(ctx context.Context, modelString string) error { // Build a provider config from current settings, overriding the model. // Load system prompt properly (handles both file paths and inline content). - systemPrompt, _ := config.LoadSystemPrompt(viper.GetString("system-prompt")) - thinkingLevel := models.ParseThinkingLevel(viper.GetString("thinking-level")) + systemPrompt, _ := config.LoadSystemPrompt(m.v.GetString("system-prompt")) + thinkingLevel := models.ParseThinkingLevel(m.v.GetString("thinking-level")) // Validate and adjust thinking level for the target model. // Some models (e.g., OpenAI gpt-5.4) don't support "minimal" and require "none". @@ -567,8 +575,8 @@ func (m *Kit) SetModel(ctx context.Context, modelString string) error { if !models.IsValidThinkingLevelForModel(thinkingLevel, modelName) { fallback := models.SuggestThinkingLevelFallback(thinkingLevel, modelName) if fallback != models.ThinkingOff { - // Adjust the thinking level in viper so the change persists. - viper.Set("thinking-level", string(fallback)) + // Adjust the thinking level in the instance store so the change persists. + m.v.Set("thinking-level", string(fallback)) thinkingLevel = fallback } } @@ -580,35 +588,36 @@ func (m *Kit) SetModel(ctx context.Context, modelString string) error { cfg := &models.ProviderConfig{ ModelString: modelString, SystemPrompt: systemPrompt, - ProviderAPIKey: viper.GetString("provider-api-key"), - ProviderURL: viper.GetString("provider-url"), - MaxTokens: viper.GetInt("max-tokens"), - TLSSkipVerify: viper.GetBool("tls-skip-verify"), + ProviderAPIKey: m.v.GetString("provider-api-key"), + ProviderURL: m.v.GetString("provider-url"), + MaxTokens: m.v.GetInt("max-tokens"), + TLSSkipVerify: m.v.GetBool("tls-skip-verify"), ThinkingLevel: thinkingLevel, DisableCaching: false, // Caching enabled by default, works with thinking + ConfigStore: m.v, } // Only set generation parameter pointers when the user has explicitly // provided a value. This leaves nil pointers for unset params, allowing // per-model defaults (modelSettings / customModels params) to apply. - if viper.IsSet("temperature") { - v := float32(viper.GetFloat64("temperature")) + if m.v.IsSet("temperature") { + v := float32(m.v.GetFloat64("temperature")) cfg.Temperature = &v } - if viper.IsSet("top-p") { - v := float32(viper.GetFloat64("top-p")) + if m.v.IsSet("top-p") { + v := float32(m.v.GetFloat64("top-p")) cfg.TopP = &v } - if viper.IsSet("top-k") { - v := int32(viper.GetInt("top-k")) + if m.v.IsSet("top-k") { + v := int32(m.v.GetInt("top-k")) cfg.TopK = &v } - if viper.IsSet("frequency-penalty") { - v := float32(viper.GetFloat64("frequency-penalty")) + if m.v.IsSet("frequency-penalty") { + v := float32(m.v.GetFloat64("frequency-penalty")) cfg.FrequencyPenalty = &v } - if viper.IsSet("presence-penalty") { - v := float32(viper.GetFloat64("presence-penalty")) + if m.v.IsSet("presence-penalty") { + v := float32(m.v.GetFloat64("presence-penalty")) cfg.PresencePenalty = &v } @@ -734,7 +743,7 @@ func (m *Kit) ReloadExtensions() error { } // Re-load from disk. - extraPaths := viper.GetStringSlice("extension") + extraPaths := m.v.GetStringSlice("extension") loaded, err := extensions.LoadExtensions(extraPaths) if err != nil { return fmt.Errorf("reloading extensions: %w", err) @@ -742,6 +751,7 @@ func (m *Kit) ReloadExtensions() error { // Swap extensions on the runner (clears dynamic state). m.extRunner.Reload(loaded) + m.extRunner.SetConfigStore(m.v) // Update extension tools on the agent so the LLM sees changes. if m.agent != nil { @@ -780,7 +790,8 @@ func (m *Kit) ExecuteCompletion(ctx context.Context, req extensions.CompleteRequ // Create a temporary provider for the requested model. config := &models.ProviderConfig{ ModelString: req.Model, - TLSSkipVerify: viper.GetBool("tls-skip-verify"), + TLSSkipVerify: m.v.GetBool("tls-skip-verify"), + ConfigStore: m.v, } if req.MaxTokens > 0 { config.MaxTokens = req.MaxTokens @@ -866,37 +877,30 @@ func (m *Kit) ExecuteCompletion(ctx context.Context, req extensions.CompleteRequ // prompts, configuration, and behavior settings. All fields are optional // and will use CLI defaults if not specified. // -// Global viper state warning: -// Options are applied by [New] via [viper.Set] calls against viper's -// process-global store. This store is shared with every downstream reader -// (e.g. [Kit.SetModel], [Kit.GetThinkingLevel], BuildProviderConfig, and -// any other code path that calls viper.Get*). Two consequences: -// -// 1. Kit instances are NOT isolated from each other within a single -// process. Values set by the second New() call overwrite the first, -// and any code that later reads viper will see the most recent Set. -// 2. Fields left at the zero value do NOT clear prior viper state; they -// simply skip the viper.Set. Callers that need a clean slate between -// constructions should invoke viper.Reset() (the test suite uses a -// private resetViper() helper that wraps it) before the next New(). -// -// Recommended usage: create one Kit per process, or reset viper between -// constructions. Concurrent calls to New are serialized internally by -// [viperInitMu], but that mutex does not prevent later viper reads (from -// a different Kit) from observing mutated keys. -// -// TODO: refactor New to use a per-instance *viper.Viper (constructed via -// viper.New()) so each Kit owns its own isolated config store and Options -// no longer leak through the global singleton. +// Config isolation: each [New] / [NewAgent] call constructs its own isolated +// configuration store (via viper.New internally). Options are applied to that +// per-instance store, so two Kits constructed in the same process do NOT share +// or clobber each other's configuration. Runtime mutators ([Kit.SetModel], +// [Kit.SetThinkingLevel]) and config readers ([Kit.GetThinkingLevel]) operate +// only on the owning instance. Fields left at their zero value are simply not +// applied; they fall through to the precedence chain (env → .kit.yml → +// per-model defaults) resolved within the instance's own store. type Options struct { Model string // Override model (e.g., "anthropic/claude-sonnet-4-5-20250929") SystemPrompt string // Override system prompt ConfigFile string // Override config file path MaxSteps int // Override max steps (0 = use default) - Streaming bool // Enable streaming (default from config) - Quiet bool // Suppress debug output - Tools []Tool // Custom tool set. If empty, AllTools() is used. - ExtraTools []Tool // Additional tools added alongside core/MCP/extension tools. + + // Streaming enables or disables streaming output. It is a pointer so the + // SDK can distinguish "unset" (nil) from an explicit choice, mirroring the + // sampling-parameter fields below. nil leaves streaming to the precedence + // chain (env → .kit.yml → default true); a non-nil value forces it. Prefer + // [WithStreaming] for the functional-options API. + Streaming *bool + + Quiet bool // Suppress debug output + Tools []Tool // Custom tool set. If empty, AllTools() is used. + ExtraTools []Tool // Additional tools added alongside core/MCP/extension tools. // Generation parameters. These override the corresponding values from // .kit.yml / KIT_* environment variables. Leaving a field at its @@ -1169,40 +1173,40 @@ func InitTreeSession(opts *Options) (*session.TreeManager, error) { return session.CreateTreeSession(sessionDir) } -// viperInitMu serializes viper writes during [New]. Viper's global state -// is not thread-safe, so concurrent calls (e.g. parallel subagent spawns) -// must not overlap the Set/Get window. Note that this mutex only protects -// the construction window — it does not isolate long-lived Kit instances -// from each other. See the "Global viper state warning" on [Options]. -var viperInitMu sync.Mutex - // New creates a Kit instance using the same initialization as the CLI. // It loads configuration, initializes MCP servers, creates the LLM model, and // sets up the agent for interaction. Returns an error if initialization fails. // -// Global viper state warning: fields on [Options] are applied by calling -// [viper.Set] on viper's process-global store. As a result, two Kits -// constructed in the same process are NOT isolated: the second New -// overwrites viper keys set by the first, and any downstream reader -// (e.g. [Kit.SetModel], [Kit.GetThinkingLevel]) will observe the most -// recent value. Callers that need multiple independent Kits should call -// viper.Reset() between constructions, or avoid constructing more than -// one Kit per process. Writes during New are serialized by [viperInitMu]. +// Config isolation: New constructs a per-instance configuration store (via +// viper.New internally) and applies [Options] to it. Two Kits constructed in +// the same process are therefore fully isolated — neither overwrites the +// other's model, thinking level, or generation parameters, and runtime +// mutators ([Kit.SetModel], [Kit.SetThinkingLevel]) only affect the owning +// instance. This makes subagent spawning and multi-Kit embedding safe without +// any external synchronization. // -// TODO: refactor to use a per-call viper.New() instance so each Kit owns -// its own isolated config store and Options stop leaking through the -// global singleton. +// CLI integration: when Options.CLI is non-nil the Kit shares the +// process-global viper store instead of allocating a fresh one, so cobra flag +// bindings established by the CLI remain in effect. SDK callers leave +// Options.CLI nil and always get an isolated store. +// +// For an ergonomic functional-options front door, see [NewAgent]. func New(ctx context.Context, opts *Options) (*Kit, error) { if opts == nil { opts = &Options{} } - // All viper writes (SetSDKDefaults, InitConfig, Set calls, system-prompt - // composition) happen under viperInitMu. We also call BuildProviderConfig - // here — it's fast (just reads) — so we can capture the full config - // snapshot before releasing the lock. The expensive work (MCP loading, - // provider creation, session init) then runs outside the lock, allowing - // parallel subagent spawns to proceed concurrently. + // Construct this Kit's configuration store. SDK callers get a fresh, + // isolated *viper.Viper so concurrent constructions never clobber each + // other. The CLI (Options.CLI != nil) shares the process-global store so + // its cobra flag bindings and pre-loaded config remain visible. + var v *viper.Viper + if opts.CLI != nil { + v = viper.GetViper() + } else { + v = viper.New() + } + var ( providerConfig *models.ProviderConfig modelString string @@ -1221,79 +1225,84 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { ) if err := func() error { - viperInitMu.Lock() - defer viperInitMu.Unlock() - - // Set CLI-equivalent defaults for viper. When used as an SDK (without - // cobra), these defaults are not registered via flag bindings. - setSDKDefaults() - - // Initialize config (loads config files and env vars). - // Only initialize if not already done (e.g., by CLI's cobra.OnInitialize). - // Check if model is already set, which indicates config was loaded. + // Set CLI-equivalent defaults on the instance store. When used as an + // SDK (without cobra), these defaults are not registered via flag bindings. + setSDKDefaults(v) + + // Initialize config (loads config files and env vars) into the instance + // store. The CLI shares the process-global store, which cobra.OnInitialize + // has already populated, so re-running initConfig there is unnecessary; + // SDK callers get a fresh isolated store that must be loaded here. + // We key off opts.CLI (not a config value) because setSDKDefaults always + // seeds "model", which would otherwise mask an empty store. // SkipConfig bypasses .kit.yml file loading (viper defaults and env vars still apply). - if !opts.SkipConfig && viper.GetString("model") == "" { - if err := InitConfig(opts.ConfigFile, false); err != nil { + if !opts.SkipConfig && opts.CLI == nil { + if err := initConfig(v, opts.ConfigFile, false); err != nil { return fmt.Errorf("failed to initialize config: %w", err) } } // Handle CLI debug mode. if opts.Debug { - viper.Set("debug", true) + v.Set("debug", true) } - // Override viper settings with options. + // Override instance settings with options. if opts.Model != "" { - viper.Set("model", opts.Model) + v.Set("model", opts.Model) } if opts.SystemPrompt != "" { - viper.Set("system-prompt", opts.SystemPrompt) + v.Set("system-prompt", opts.SystemPrompt) } if opts.MaxSteps > 0 { - viper.Set("max-steps", opts.MaxSteps) + v.Set("max-steps", opts.MaxSteps) + } + // Only override streaming when the caller explicitly set it. Otherwise + // leave the precedence chain (env → config → default true) untouched so a + // zero-valued Options does not silently force stream=false. + if opts.Streaming != nil { + v.Set("stream", *opts.Streaming) } - viper.Set("stream", opts.Streaming) // Generation parameter overrides. Each Options field, when set, - // is pushed into viper here so the existing downstream code - // (BuildProviderConfig, SetModel, modelSettings lookups) picks - // it up uniformly. Pointer-typed sampling params use viper.Set - // only when non-nil so that nil means "leave provider/per-model - // default in place" (BuildProviderConfig keys off viper.IsSet). + // is pushed into the instance store here so the existing downstream + // code (BuildProviderConfig, SetModel, modelSettings lookups) picks + // it up uniformly. Pointer-typed sampling params use Set only when + // non-nil so that nil means "leave provider/per-model default in + // place" (BuildProviderConfig keys off IsSet). if opts.MaxTokens > 0 { - viper.Set("max-tokens", opts.MaxTokens) + v.Set("max-tokens", opts.MaxTokens) } if opts.ThinkingLevel != "" { - viper.Set("thinking-level", opts.ThinkingLevel) + v.Set("thinking-level", opts.ThinkingLevel) } if opts.Temperature != nil { - viper.Set("temperature", *opts.Temperature) + v.Set("temperature", *opts.Temperature) } if opts.TopP != nil { - viper.Set("top-p", *opts.TopP) + v.Set("top-p", *opts.TopP) } if opts.TopK != nil { - viper.Set("top-k", *opts.TopK) + v.Set("top-k", *opts.TopK) } if opts.FrequencyPenalty != nil { - viper.Set("frequency-penalty", *opts.FrequencyPenalty) + v.Set("frequency-penalty", *opts.FrequencyPenalty) } if opts.PresencePenalty != nil { - viper.Set("presence-penalty", *opts.PresencePenalty) + v.Set("presence-penalty", *opts.PresencePenalty) } // Provider overrides. TLSSkipVerify only takes effect when true — // callers wanting to force-disable should use the config file or // env var instead. if opts.ProviderAPIKey != "" { - viper.Set("provider-api-key", opts.ProviderAPIKey) + v.Set("provider-api-key", opts.ProviderAPIKey) } if opts.ProviderURL != "" { - viper.Set("provider-url", opts.ProviderURL) + v.Set("provider-url", opts.ProviderURL) } if opts.TLSSkipVerify { - viper.Set("tls-skip-verify", true) + v.Set("tls-skip-verify", true) } // Resolve working directory for context/skill discovery. @@ -1324,7 +1333,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { // explicitly set system-prompt, use the per-model prompt as the // base instead of the global default. { - rawPromptInput := viper.GetString("system-prompt") + rawPromptInput := v.GetString("system-prompt") // Resolve a file path to its content so PromptBuilder receives the // actual prompt text rather than a literal path string. Without this, @@ -1349,12 +1358,12 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { // Check for per-model system prompt override when no explicit // global system-prompt was configured by the user. if !userSetSystemPrompt { - modelStr := viper.GetString("model") + modelStr := v.GetString("model") if modelStr != "" { if mi := models.LookupModelForSettings(modelStr); mi != nil { var perModelParams *models.GenerationParams // modelSettings takes priority over custom model params. - if ms := models.LoadModelSettingsFromConfig(); ms != nil { + if ms := models.LoadModelSettingsFrom(v); ms != nil { perModelParams = ms[modelStr] } if perModelParams == nil && mi.Params != nil { @@ -1389,42 +1398,42 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { time.Now().Format("Monday, January 2, 2006, 3:04:05 PM MST"), cwd, )) - viper.Set("system-prompt", pb.Build()) + v.Set("system-prompt", pb.Build()) } - // Snapshot all viper-derived values now, while the lock is held. - // BuildProviderConfig is fast (pure reads), so we do it here. + // Snapshot all instance-derived values now. + // BuildProviderConfig is fast (pure reads). var pcErr error - providerConfig, _, pcErr = kitsetup.BuildProviderConfig() + providerConfig, _, pcErr = kitsetup.BuildProviderConfig(v) if pcErr != nil { return fmt.Errorf("failed to build provider config: %w", pcErr) } // SDK last-resort max-tokens floor. When nothing — Options, env, // config, nor a per-model default — supplied a value, we land on - // zero here (viper.GetInt returns 0 for unset keys). Apply the - // SDK default directly on the struct rather than via viper so - // viper.IsSet("max-tokens") stays false: downstream right-sizing + // zero here (GetInt returns 0 for unset keys). Apply the + // SDK default directly on the struct rather than via the store so + // IsSet("max-tokens") stays false: downstream right-sizing // can still raise this toward the model's known output ceiling, // and per-model modelSettings[...].maxTokens can still win. if providerConfig.MaxTokens == 0 && opts.MaxTokens == 0 { providerConfig.MaxTokens = sdkDefaultMaxTokens } - modelString = viper.GetString("model") - debug = viper.GetBool("debug") - noExtensions = opts.NoExtensions || viper.GetBool("no-extensions") - disableCoreTools = opts.DisableCoreTools || viper.GetBool("no-core-tools") - maxSteps = viper.GetInt("max-steps") - streaming = viper.GetBool("stream") + modelString = v.GetString("model") + debug = v.GetBool("debug") + noExtensions = opts.NoExtensions || v.GetBool("no-extensions") + disableCoreTools = opts.DisableCoreTools || v.GetBool("no-core-tools") + maxSteps = v.GetInt("max-steps") + streaming = v.GetBool("stream") return nil }(); err != nil { return nil, err } - // ---- viperInitMu released — heavy I/O below runs concurrently ---- + // ---- config snapshot complete — heavy I/O below ---- // Load MCP configuration. Use pre-loaded config if provided directly, - // via CLI options, or load from viper as a last resort. + // via CLI options, or load from the instance store as a last resort. if opts.MCPConfig != nil { mcpConfig = opts.MCPConfig } else if opts.CLI != nil && opts.CLI.MCPConfig != nil { @@ -1432,7 +1441,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { } if mcpConfig == nil { var err error - mcpConfig, err = config.LoadAndValidateConfig() + mcpConfig, err = config.LoadAndValidateConfigFrom(v) if err != nil { return nil, fmt.Errorf("failed to load MCP config: %w", err) } @@ -1488,6 +1497,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { timeout: opts.MCPTaskTimeout, progress: opts.MCPTaskProgress, }.toToolsConfig(), + Viper: v, } // Set up OAuth handler for remote MCP servers. The SDK does not create @@ -1557,6 +1567,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { authHandler: setupOpts.AuthHandler, opts: opts, mcpConfig: mcpConfig, + v: v, hasCustomSystemPrompt: hasCustomSystemPrompt, systemPromptSource: systemPromptSource, basePrompt: capturedBasePrompt, @@ -1836,6 +1847,50 @@ type SubagentResult struct { Elapsed time.Duration } +// inheritProviderConfig copies the parent's effective provider/runtime +// configuration from its isolated config store onto child Options. Used by +// Kit.Subagent so the child — which owns a separate store and re-loads only +// .kit.yml / KIT_* on its own — still observes provider credentials, the +// thinking level, and sampler/token overrides the parent acquired via +// programmatic Options or runtime setters (e.g. SetThinkingLevel). +// +// max-tokens and the sampling parameters are only propagated when the parent +// explicitly set them (IsSet), preserving the tri-state precedence so per-model +// defaults still apply on the child when the parent left them unset. A nil +// child or store is a no-op. +func inheritProviderConfig(child *Options, v *viper.Viper) { + if child == nil || v == nil { + return + } + child.ProviderAPIKey = v.GetString("provider-api-key") + child.ProviderURL = v.GetString("provider-url") + child.TLSSkipVerify = v.GetBool("tls-skip-verify") + child.ThinkingLevel = v.GetString("thinking-level") + if v.IsSet("max-tokens") { + child.MaxTokens = v.GetInt("max-tokens") + } + if v.IsSet("temperature") { + t := float32(v.GetFloat64("temperature")) + child.Temperature = &t + } + if v.IsSet("top-p") { + p := float32(v.GetFloat64("top-p")) + child.TopP = &p + } + if v.IsSet("top-k") { + k := int32(v.GetInt("top-k")) + child.TopK = &k + } + if v.IsSet("frequency-penalty") { + fp := float32(v.GetFloat64("frequency-penalty")) + child.FrequencyPenalty = &fp + } + if v.IsSet("presence-penalty") { + pp := float32(v.GetFloat64("presence-penalty")) + child.PresencePenalty = &pp + } +} + // Subagent spawns an in-process child Kit instance to perform a task. The // child gets its own session, event bus, and agent loop but shares the // parent's config (API keys, provider settings) and defaults to the parent's @@ -1905,22 +1960,28 @@ func (m *Kit) Subagent(ctx context.Context, cfg SubagentConfig) (*SubagentResult } // Create child Kit instance. Pass the parent's loaded MCP config to - // avoid re-reading viper (which races with concurrent subagent spawns). - // Streaming must be explicitly enabled — Options.Streaming defaults to - // false, and New() unconditionally writes viper.Set("stream", opts.Streaming). - // Without this, the subagent would (a) pollute viper global state for - // other concurrent callers and (b) potentially hit provider-level - // differences (e.g. Anthropic non-streaming timeouts with extended - // thinking). + // avoid re-loading and re-validating config for the child. + // Streaming is enabled explicitly — without it, non-streaming can hit + // provider-level differences (e.g. Anthropic non-streaming timeouts with + // extended thinking). The child gets its own config store, so this does not + // affect any other concurrent caller. + streamOn := true childOpts := &Options{ Model: model, SystemPrompt: systemPrompt, Tools: tools, NoSession: cfg.NoSession, Quiet: true, - Streaming: true, + Streaming: &streamOn, MCPConfig: m.mcpConfig, } + + // Inherit the parent's effective provider/runtime configuration. Since #40 + // each Kit owns an isolated config store, so the child's New() only re-loads + // .kit.yml / KIT_* on its own — values the parent picked up from + // programmatic Options or runtime setters (e.g. SetThinkingLevel) would + // otherwise be lost. + inheritProviderConfig(childOpts, m.v) // Propagate the parent's MCP task configuration so a child subagent // invoking long-running MCP tools observes the same per-server modes, // timeouts, and progress callback as the parent. Without this, child @@ -2129,7 +2190,7 @@ func (m *Kit) generate(ctx context.Context, messages []fantasy.Message) (*agent. } }, OnStepUsage: func(inputTokens, outputTokens, cacheReadTokens, cacheCreationTokens int64) { - if viper.GetBool("debug") { + if m.v.GetBool("debug") { log.Printf("DEBUG Kit.generate emitting StepUsageEvent: input=%d output=%d cacheRead=%d cacheCreate=%d", inputTokens, outputTokens, cacheReadTokens, cacheCreationTokens, ) @@ -2625,7 +2686,7 @@ func (m *Kit) IsReasoningModel() bool { // GetThinkingLevel returns the current thinking level. func (m *Kit) GetThinkingLevel() string { - return viper.GetString("thinking-level") + return m.v.GetString("thinking-level") } // SetThinkingLevel changes the thinking level and recreates the agent with @@ -2634,7 +2695,7 @@ func (m *Kit) GetThinkingLevel() string { // With message-level caching, both thinking and caching work together. // Caching reduces costs by 60-90% for repeated context. func (m *Kit) SetThinkingLevel(ctx context.Context, level string) error { - viper.Set("thinking-level", level) + m.v.Set("thinking-level", level) // Recreate agent with new thinking config by re-running SetModel // with the same model string. SetModel rebuilds the provider and // passes the updated viper config (including thinking-level). diff --git a/pkg/kit/kit_test.go b/pkg/kit/kit_test.go index 04ca2cb2..3bd9d957 100644 --- a/pkg/kit/kit_test.go +++ b/pkg/kit/kit_test.go @@ -86,8 +86,8 @@ func TestNewWithGenerationOptions(t *testing.T) { if got := host.MaxTokens(); got != want { t.Errorf("Options.MaxTokens=%d did not propagate; Kit.MaxTokens()=%d", want, got) } - if !viper.IsSet("max-tokens") { - t.Error("viper.IsSet(\"max-tokens\") should be true after MaxTokens override") + if !host.ConfigValueIsSetForTest("max-tokens") { + t.Error("max-tokens should be marked explicitly set on the instance store after MaxTokens override") } }) @@ -129,11 +129,11 @@ func TestNewWithGenerationOptions(t *testing.T) { } defer func() { _ = host.Close() }() - if !viper.IsSet("temperature") { - t.Fatal("viper.IsSet(\"temperature\") should be true after Temperature override") + if !host.ConfigValueIsSetForTest("temperature") { + t.Fatal("temperature should be marked explicitly set on the instance store after Temperature override") } - if got := float32(viper.GetFloat64("temperature")); got != want { - t.Errorf("Options.Temperature=%v did not propagate; viper=%v", want, got) + if got := float32(host.ConfigFloatForTest("temperature")); got != want { + t.Errorf("Options.Temperature=%v did not propagate; instance store=%v", want, got) } }) } @@ -185,8 +185,8 @@ func TestNewPreservesIsSetSemantics(t *testing.T) { // from SDK-side SetDefault/Set calls — which is exactly what this // test is guarding against. for _, k := range checkKeys { - if viper.IsSet(k) { - t.Errorf("viper.IsSet(%q) == true when no Options field set it "+ + if host.ConfigValueIsSetForTest(k) { + t.Errorf("instance store reports %q explicitly set when no Options field set it "+ "(SDK defaults must not corrupt IsSet semantics)", k) } } @@ -217,14 +217,14 @@ func TestNewWithProviderOptions(t *testing.T) { } defer func() { _ = host.Close() }() - if got := viper.GetString("provider-api-key"); got != apiKey { - t.Errorf("Options.ProviderAPIKey did not propagate to viper; got %q (len=%d)", got, len(got)) + if got := host.ConfigStringForTest("provider-api-key"); got != apiKey { + t.Errorf("Options.ProviderAPIKey did not propagate to the instance store; got %q (len=%d)", got, len(got)) } }) - // Override precedence: even when viper already holds a different - // provider-api-key value (as it would if a config file or earlier - // Set() call populated one), Options.ProviderAPIKey must win. + // Override precedence: even when the process-global store already holds a + // different provider-api-key value, Options.ProviderAPIKey must win on the + // Kit's isolated store. t.Run("Options override beats pre-existing viper state", func(t *testing.T) { defer resetViper() @@ -242,15 +242,16 @@ func TestNewWithProviderOptions(t *testing.T) { ProviderAPIKey: want, }) // Creation may still fail if the model registry is strict, but - // we only care that the override reached viper before any - // provider handshake happened. - if host != nil { - defer func() { _ = host.Close() }() + // we only care that the override reached the instance store before + // any provider handshake happened. + if host == nil { + t.Fatalf("expected a Kit instance to inspect; got nil (err=%v)", err) } + defer func() { _ = host.Close() }() _ = err - if got := viper.GetString("provider-api-key"); got != want { - t.Errorf("Options.ProviderAPIKey did not override pre-existing viper value; got %q, want %q", got, want) + if got := host.ConfigStringForTest("provider-api-key"); got != want { + t.Errorf("Options.ProviderAPIKey did not override pre-existing value on the instance store; got %q, want %q", got, want) } }) @@ -270,7 +271,7 @@ func TestNewWithProviderOptions(t *testing.T) { } defer func() { _ = host.Close() }() - if got := viper.GetString("provider-url"); got != want { + if got := host.ConfigStringForTest("provider-url"); got != want { t.Errorf("Options.ProviderURL did not propagate; got %q, want %q", got, want) } }) @@ -353,9 +354,9 @@ func TestNewSystemPromptFilePath(t *testing.T) { t.Errorf("GetSystemPromptSource() = %q; want %q", got, want) } - // The composed system prompt is written back to viper after PromptBuilder - // runs. It must contain the file's contents, not the file path. - composed := viper.GetString("system-prompt") + // The composed system prompt is written back to the instance store after + // PromptBuilder runs. It must contain the file's contents, not the file path. + composed := host.ConfigStringForTest("system-prompt") if !strings.Contains(composed, promptContent) { t.Errorf("composed system-prompt does not contain file contents\n composed = %q\n want substring = %q", composed, promptContent) } @@ -392,7 +393,7 @@ func TestNewSystemPromptInline(t *testing.T) { if got := host.GetSystemPromptSource(); got != inline { t.Errorf("GetSystemPromptSource() = %q; want %q", got, inline) } - if composed := viper.GetString("system-prompt"); !strings.Contains(composed, inline) { + if composed := host.ConfigStringForTest("system-prompt"); !strings.Contains(composed, inline) { t.Errorf("composed system-prompt missing inline content; got %q", composed) } } diff --git a/pkg/kit/mcp_tasks_test.go b/pkg/kit/mcp_tasks_test.go index 9887bebc..b1188c68 100644 --- a/pkg/kit/mcp_tasks_test.go +++ b/pkg/kit/mcp_tasks_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "github.com/mark3labs/kit/internal/tools" ) @@ -163,3 +165,82 @@ func TestSubagentPropagatesMCPTaskOptions(t *testing.T) { inheritMCPTaskOptions(&Options{}, nil) inheritMCPTaskOptions(nil, parent) } + +// TestInheritProviderConfig verifies that Kit.Subagent's provider/runtime +// config inheritance copies the parent's effective settings onto child +// Options, and that the tri-state (IsSet) keys are only propagated when the +// parent explicitly set them. Regression test for config loss after the +// per-instance viper store isolation (#40). +func TestInheritProviderConfig(t *testing.T) { + t.Run("explicit values propagate", func(t *testing.T) { + v := viper.New() + v.Set("provider-api-key", "sk-parent") + v.Set("provider-url", "https://proxy.internal/v1") + v.Set("tls-skip-verify", true) + v.Set("thinking-level", "high") + v.Set("max-tokens", 4321) + v.Set("temperature", 0.25) + v.Set("top-p", 0.9) + v.Set("top-k", 40) + v.Set("frequency-penalty", 0.1) + v.Set("presence-penalty", 0.2) + + child := &Options{} + inheritProviderConfig(child, v) + + if child.ProviderAPIKey != "sk-parent" { + t.Errorf("ProviderAPIKey = %q, want sk-parent", child.ProviderAPIKey) + } + if child.ProviderURL != "https://proxy.internal/v1" { + t.Errorf("ProviderURL = %q", child.ProviderURL) + } + if !child.TLSSkipVerify { + t.Error("TLSSkipVerify not propagated") + } + if child.ThinkingLevel != "high" { + t.Errorf("ThinkingLevel = %q, want high", child.ThinkingLevel) + } + if child.MaxTokens != 4321 { + t.Errorf("MaxTokens = %d, want 4321", child.MaxTokens) + } + if child.Temperature == nil || *child.Temperature != 0.25 { + t.Errorf("Temperature = %v, want 0.25", child.Temperature) + } + if child.TopP == nil || *child.TopP != 0.9 { + t.Errorf("TopP = %v, want 0.9", child.TopP) + } + if child.TopK == nil || *child.TopK != 40 { + t.Errorf("TopK = %v, want 40", child.TopK) + } + if child.FrequencyPenalty == nil || *child.FrequencyPenalty != 0.1 { + t.Errorf("FrequencyPenalty = %v, want 0.1", child.FrequencyPenalty) + } + if child.PresencePenalty == nil || *child.PresencePenalty != 0.2 { + t.Errorf("PresencePenalty = %v, want 0.2", child.PresencePenalty) + } + }) + + t.Run("unset tri-state keys stay unset", func(t *testing.T) { + // A store with no sampler / max-tokens keys must leave the child's + // pointers nil and MaxTokens zero so per-model defaults still apply. + v := viper.New() + child := &Options{} + inheritProviderConfig(child, v) + + if child.MaxTokens != 0 { + t.Errorf("MaxTokens = %d, want 0 (unset)", child.MaxTokens) + } + if child.Temperature != nil || child.TopP != nil || child.TopK != nil || + child.FrequencyPenalty != nil || child.PresencePenalty != nil { + t.Error("sampler pointers must stay nil when the parent did not set them") + } + if child.ThinkingLevel != "" { + t.Errorf("ThinkingLevel = %q, want empty", child.ThinkingLevel) + } + }) + + t.Run("nil child or store is a no-op", func(t *testing.T) { + inheritProviderConfig(nil, viper.New()) + inheritProviderConfig(&Options{}, nil) + }) +} diff --git a/pkg/kit/options.go b/pkg/kit/options.go new file mode 100644 index 00000000..d40c711a --- /dev/null +++ b/pkg/kit/options.go @@ -0,0 +1,88 @@ +package kit + +import "context" + +// Option configures a [Kit] created via [NewAgent]. Options are applied in +// order to an [Options] value, so later options override earlier ones. The +// type is a plain func(*Options), so callers can define their own options +// without depending on any internal type. +type Option func(*Options) + +// NewAgent creates a Kit using an ergonomic functional-options API. It is a +// thin, additive front door over [New]: the supplied options are applied to a +// fresh [Options] value which is then passed to [New]. For advanced +// configuration not covered by the With* helpers (MCPConfig, +// InProcessMCPServers, session backends, MCP task tuning, etc.) construct an +// [Options] explicitly and call [New]. +// +// Streaming defaults to enabled. Pass WithStreaming(false) to disable it. +// +// Example: +// +// k, err := kit.NewAgent(ctx, +// kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), +// kit.WithSystemPrompt("You are a helpful assistant."), +// kit.WithMaxTokens(8192), +// kit.Ephemeral(), +// ) +func NewAgent(ctx context.Context, opts ...Option) (*Kit, error) { + // Streaming defaults to true for the ergonomic constructor — this is the + // natural expectation for interactive agents. WithStreaming(false) overrides it. + streamOn := true + o := &Options{Streaming: &streamOn} + for _, fn := range opts { + fn(o) + } + return New(ctx, o) +} + +// WithModel sets the model in "provider/model" format +// (e.g. "anthropic/claude-sonnet-4-5-20250929"). +func WithModel(m string) Option { return func(o *Options) { o.Model = m } } + +// WithSystemPrompt sets the system prompt. The value may be inline text or a +// path to a file whose contents are loaded as the prompt. +func WithSystemPrompt(p string) Option { return func(o *Options) { o.SystemPrompt = p } } + +// WithStreaming enables or disables streaming responses. [NewAgent] enables +// streaming by default, so pass WithStreaming(false) to opt out. +func WithStreaming(b bool) Option { + return func(o *Options) { o.Streaming = &b } +} + +// WithMaxTokens sets the maximum output tokens per LLM response. A value of 0 +// lets the precedence chain (env → config → per-model → SDK floor) resolve a +// value; a non-zero value pins it and suppresses automatic right-sizing. +func WithMaxTokens(n int) Option { return func(o *Options) { o.MaxTokens = n } } + +// WithThinkingLevel sets the reasoning effort for models that support extended +// thinking. Valid values: "off", "none", "minimal", "low", "medium", "high". +// An empty string lets the precedence chain resolve a level. +func WithThinkingLevel(level string) Option { return func(o *Options) { o.ThinkingLevel = level } } + +// WithTools sets the agent's tool set, replacing the default core tools. When +// no tools are provided the default set is used. +func WithTools(t ...Tool) Option { return func(o *Options) { o.Tools = t } } + +// WithExtraTools adds tools alongside the core/MCP/extension tools rather than +// replacing them. +func WithExtraTools(t ...Tool) Option { return func(o *Options) { o.ExtraTools = t } } + +// WithProviderAPIKey overrides the API key used to authenticate with the model +// provider. +func WithProviderAPIKey(key string) Option { return func(o *Options) { o.ProviderAPIKey = key } } + +// WithProviderURL overrides the provider endpoint URL. Useful for +// OpenAI-compatible proxies (LiteLLM, vLLM, Azure OpenAI, etc.). +func WithProviderURL(url string) Option { return func(o *Options) { o.ProviderURL = url } } + +// WithConfigFile sets an explicit config file path, overriding the default +// .kit.yml search. +func WithConfigFile(path string) Option { return func(o *Options) { o.ConfigFile = path } } + +// WithDebug enables SDK debug logging. +func WithDebug() Option { return func(o *Options) { o.Debug = true } } + +// Ephemeral configures an in-memory session with no persistence (equivalent to +// Options.NoSession = true). +func Ephemeral() Option { return func(o *Options) { o.NoSession = true } } diff --git a/pkg/kit/viper_isolation_test.go b/pkg/kit/viper_isolation_test.go new file mode 100644 index 00000000..3cca7063 --- /dev/null +++ b/pkg/kit/viper_isolation_test.go @@ -0,0 +1,232 @@ +package kit_test + +import ( + "context" + "os" + "testing" + + "github.com/mark3labs/kit/pkg/kit" +) + +// TestOptionFunctionsPlumbing verifies that the functional options apply their +// values to the underlying Options struct. This does not create a provider, so +// it runs without API keys. +func TestOptionFunctionsPlumbing(t *testing.T) { + o := &kit.Options{} + opts := []kit.Option{ + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithSystemPrompt("be terse"), + kit.WithMaxTokens(4321), + kit.WithThinkingLevel("high"), + kit.WithProviderAPIKey("sk-test"), + kit.WithProviderURL("https://example.test/v1"), + kit.WithConfigFile("/tmp/.kit.yml"), + kit.WithStreaming(false), + kit.WithDebug(), + kit.Ephemeral(), + } + for _, fn := range opts { + fn(o) + } + + if o.Model != "anthropic/claude-sonnet-4-5-20250929" { + t.Errorf("WithModel: got %q", o.Model) + } + if o.SystemPrompt != "be terse" { + t.Errorf("WithSystemPrompt: got %q", o.SystemPrompt) + } + if o.MaxTokens != 4321 { + t.Errorf("WithMaxTokens: got %d", o.MaxTokens) + } + if o.ThinkingLevel != "high" { + t.Errorf("WithThinkingLevel: got %q", o.ThinkingLevel) + } + if o.ProviderAPIKey != "sk-test" { + t.Errorf("WithProviderAPIKey: got %q", o.ProviderAPIKey) + } + if o.ProviderURL != "https://example.test/v1" { + t.Errorf("WithProviderURL: got %q", o.ProviderURL) + } + if o.ConfigFile != "/tmp/.kit.yml" { + t.Errorf("WithConfigFile: got %q", o.ConfigFile) + } + if o.Streaming == nil { + t.Error("WithStreaming: expected Streaming to be set (non-nil)") + } else if *o.Streaming { + t.Error("WithStreaming(false): expected *Streaming=false") + } + if !o.Debug { + t.Error("WithDebug: expected Debug=true") + } + if !o.NoSession { + t.Error("Ephemeral: expected NoSession=true") + } +} + +// TestOptionOrderingOverrides verifies later options override earlier ones. +func TestOptionOrderingOverrides(t *testing.T) { + o := &kit.Options{} + kit.WithModel("a/b")(o) + kit.WithModel("c/d")(o) + if o.Model != "c/d" { + t.Errorf("later WithModel should win; got %q", o.Model) + } +} + +// TestKitConfigIsolation is the regression test for issue #40: two Kit +// instances constructed in the same process must own independent configuration +// stores. Setting the thinking level (or model) on one must not affect the +// other. Against the previous global-viper implementation this test fails +// because both Kits read and write the same process-global store. +func TestKitConfigIsolation(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + + ctx := context.Background() + + a, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + ThinkingLevel: "low", + Quiet: true, + NoSession: true, + NoExtensions: true, + }) + if err != nil { + t.Fatalf("failed to create Kit A: %v", err) + } + defer func() { _ = a.Close() }() + + b, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + ThinkingLevel: "high", + Quiet: true, + NoSession: true, + NoExtensions: true, + }) + if err != nil { + t.Fatalf("failed to create Kit B: %v", err) + } + defer func() { _ = b.Close() }() + + // Each instance must retain its own configured thinking level. Under the + // old global-viper implementation, B's construction overwrote A's value. + if got := a.GetThinkingLevel(); got != "low" { + t.Errorf("Kit A thinking level = %q; want %q (config leaked from B)", got, "low") + } + if got := b.GetThinkingLevel(); got != "high" { + t.Errorf("Kit B thinking level = %q; want %q", got, "high") + } + + // Mutating one at runtime must not bleed into the other. + if err := a.SetThinkingLevel(ctx, "medium"); err != nil { + t.Fatalf("SetThinkingLevel on A: %v", err) + } + if got := a.GetThinkingLevel(); got != "medium" { + t.Errorf("after SetThinkingLevel, Kit A = %q; want %q", got, "medium") + } + if got := b.GetThinkingLevel(); got != "high" { + t.Errorf("after mutating A, Kit B leaked to %q; want %q", got, "high") + } +} + +// TestNewAgentDefaultsStreamingOn verifies that the ergonomic constructor +// enables streaming by default and applies functional options. +func TestNewAgentDefaultsStreamingOn(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + + ctx := context.Background() + k, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithMaxTokens(2048), + kit.Ephemeral(), + ) + if err != nil { + t.Fatalf("NewAgent failed: %v", err) + } + defer func() { _ = k.Close() }() + + if !k.ConfigValueIsSetForTest("max-tokens") { + t.Error("NewAgent did not propagate WithMaxTokens to the instance store") + } + if !k.ConfigBoolForTest("stream") { + t.Error("NewAgent should enable streaming by default") + } +} + +// TestNewAgentStreamingOptOut verifies WithStreaming(false) disables the +// default-on streaming behaviour of NewAgent. +func TestNewAgentStreamingOptOut(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + + ctx := context.Background() + k, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithStreaming(false), + kit.Ephemeral(), + ) + if err != nil { + t.Fatalf("NewAgent failed: %v", err) + } + defer func() { _ = k.Close() }() + + if k.ConfigBoolForTest("stream") { + t.Error("WithStreaming(false) should disable streaming") + } +} + +// TestNewZeroOptionsKeepsStreamingDefault is the regression test for the +// unconditional `v.Set("stream", opts.Streaming)` bug: a zero-valued Options +// (Streaming == nil) must NOT force stream=false. With Streaming unset, +// streaming resolves through the precedence chain, whose SDK default is true. +func TestNewZeroOptionsKeepsStreamingDefault(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + + ctx := context.Background() + k, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + Quiet: true, + NoSession: true, + SkipConfig: true, // isolate from any ~/.kit.yml / env stream setting + }) + if err != nil { + t.Fatalf("New failed: %v", err) + } + defer func() { _ = k.Close() }() + + if !k.ConfigBoolForTest("stream") { + t.Error("zero-valued Options must not force stream=false; expected the default (true)") + } +} + +// TestNewStreamingExplicitOptOut verifies that a raw Options can still disable +// streaming by setting Streaming to a pointer to false. +func TestNewStreamingExplicitOptOut(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + + streamOff := false + ctx := context.Background() + k, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + Quiet: true, + NoSession: true, + SkipConfig: true, + Streaming: &streamOff, + }) + if err != nil { + t.Fatalf("New failed: %v", err) + } + defer func() { _ = k.Close() }() + + if k.ConfigBoolForTest("stream") { + t.Error("Streaming=&false should disable streaming") + } +} diff --git a/www/pages/sdk/options.md b/www/pages/sdk/options.md index 9896b16d..424c1ce4 100644 --- a/www/pages/sdk/options.md +++ b/www/pages/sdk/options.md @@ -7,6 +7,16 @@ description: Configuration options for the Kit Go SDK. Pass an `Options` struct to `kit.New()` to configure the Kit instance. +::: tip +For simple setups, `kit.NewAgent(ctx, ...Option)` provides functional-options +helpers (`WithModel`, `WithStreaming`, `Ephemeral`, ...) over the same `Options` +struct. See [Functional options](/sdk/overview#functional-options-newagent). +::: + +Each `kit.New` / `kit.NewAgent` call owns an isolated configuration store, so +these options never leak between Kit instances in the same process. See +[Per-instance config isolation](/sdk/overview#per-instance-config-isolation). + ## Full options reference ```go @@ -18,7 +28,7 @@ host, err := kit.New(ctx, &kit.Options{ // Behavior MaxSteps: 10, - Streaming: true, + Streaming: ptrBool(true), // *bool: nil = unset (default true), &false = off Quiet: true, Debug: true, @@ -91,7 +101,7 @@ host, err := kit.New(ctx, &kit.Options{ | `SystemPrompt` | `string` | — | System prompt text or file path | | `ConfigFile` | `string` | `~/.kit.yml` | Path to config file | | `MaxSteps` | `int` | `0` | Max agent steps (0 = unlimited) | -| `Streaming` | `bool` | `true` | Enable streaming output | +| `Streaming` | `*bool` | `nil` | Enable streaming output. `nil` leaves it to the precedence chain (env → config → default `true`); `&true`/`&false` forces it. Pointer so unset is distinct from explicit `false`. | | `Quiet` | `bool` | `false` | Suppress output | | `Debug` | `bool` | `false` | Enable debug logging | @@ -114,9 +124,10 @@ defaults for samplers). | `FrequencyPenalty` | `*float32` | — | OpenAI-family frequency penalty. `nil` leaves provider default. | | `PresencePenalty` | `*float32` | — | OpenAI-family presence penalty. `nil` leaves provider default. | -Pointer-typed samplers are populated via a tiny helper: +Pointer-typed fields (`Streaming` and the samplers) are populated via tiny helpers: ```go +func ptrBool(v bool) *bool { return &v } func ptrFloat32(v float32) *float32 { return &v } ``` @@ -127,7 +138,7 @@ when embedding Kit as a library. | Field | Type | Default | Description | |-------|------|---------|-------------| -| `ProviderAPIKey` | `string` | — | API key used to authenticate with the provider. `""` falls back to config / provider-specific env var (e.g. `ANTHROPIC_API_KEY`). When set, overrides any pre-existing viper state. | +| `ProviderAPIKey` | `string` | — | API key used to authenticate with the provider. `""` falls back to config / provider-specific env var (e.g. `ANTHROPIC_API_KEY`). When set, it takes precedence over config and env values on this instance's store. | | `ProviderURL` | `string` | — | Override the provider endpoint (e.g. LiteLLM, vLLM, Azure OpenAI, internal proxy). `""` = provider default. | | `TLSSkipVerify` | `bool` | `false` | Disable TLS certificate verification on the provider HTTP client. Only effective when `true`; to force-disable, use config file or env var instead. For self-signed dev certs only. | diff --git a/www/pages/sdk/overview.md b/www/pages/sdk/overview.md index 61e708eb..9214b4d2 100644 --- a/www/pages/sdk/overview.md +++ b/www/pages/sdk/overview.md @@ -45,6 +45,73 @@ func main() { } ``` +## Functional options (`NewAgent`) + +For simple programmatic setups, `kit.NewAgent` offers an ergonomic +functional-options front door over `kit.New`. Streaming is **enabled by +default**; pass `kit.WithStreaming(false)` to opt out. + +```go +host, err := kit.NewAgent(ctx, + kit.WithModel("anthropic/claude-sonnet-4-5-20250929"), + kit.WithSystemPrompt("You are a helpful assistant."), + kit.WithMaxTokens(8192), + kit.WithThinkingLevel("medium"), + kit.Ephemeral(), // in-memory session, no persistence +) +if err != nil { + log.Fatal(err) +} +defer host.Close() +``` + +Available options: + +| Option | Sets | +|--------|------| +| `WithModel(string)` | `Options.Model` (provider/model format) | +| `WithSystemPrompt(string)` | `Options.SystemPrompt` (inline text or file path) | +| `WithStreaming(bool)` | `Options.Streaming` (default `true` under `NewAgent`) | +| `WithMaxTokens(int)` | `Options.MaxTokens` | +| `WithThinkingLevel(string)` | `Options.ThinkingLevel` | +| `WithTools(...Tool)` | `Options.Tools` (replaces the default set) | +| `WithExtraTools(...Tool)` | `Options.ExtraTools` (adds alongside defaults) | +| `WithProviderAPIKey(string)` | `Options.ProviderAPIKey` | +| `WithProviderURL(string)` | `Options.ProviderURL` | +| `WithConfigFile(string)` | `Options.ConfigFile` | +| `WithDebug()` | `Options.Debug = true` | +| `Ephemeral()` | `Options.NoSession = true` | + +Options are applied in order, so later options override earlier ones. `Option` +is a plain `func(*Options)`, so you can define your own. For advanced +configuration not covered by the helpers (custom MCP config, in-process MCP +servers, session backends, MCP task tuning) construct an `Options` value +explicitly and call `kit.New`. + +### When to use which + +| Constructor | Use when | +|-------------|----------| +| `kit.NewAgent(ctx, ...Option)` | Quick programmatic setups; you only need the common fields. Streaming defaults on. | +| `kit.New(ctx, *Options)` | You need fields without a `With*` helper (`MCPConfig`, `InProcessMCPServers`, `SessionManager`, MCP task tuning, etc.), or you already hold an `Options` value. | + +## Per-instance config isolation + +Each `kit.New` / `kit.NewAgent` call owns an **isolated configuration store**, +so constructing multiple Kit instances in the same process is safe: setting the +model, thinking level, or generation parameters on one never affects another, +and runtime mutators (`SetModel`, `SetThinkingLevel`) only touch the owning +instance. This makes subagent spawning and multi-Kit embedding race-free with +no external synchronization required. + +```go +a, _ := kit.NewAgent(ctx, kit.WithThinkingLevel("low")) +b, _ := kit.NewAgent(ctx, kit.WithThinkingLevel("high")) + +a.SetThinkingLevel(ctx, "medium") +// a.GetThinkingLevel() == "medium"; b.GetThinkingLevel() is still "high" +``` + ## Multi-turn conversations Conversations retain context automatically across calls: