fix(kit): correct config loading and isolate ACP sessions

- Isolate each ACP session's config store instead of sharing the global
  viper, preventing per-session SetModel/SetThinkingLevel races; seed the
  root-command flag values (model, thinking-level, provider URL/key) so
  `kit acp -m <model>` is still honored
- Run initConfig for isolated SDK stores by gating on opts.CLI instead of
  v.GetString("model"), which setSDKDefaults always populates and thus
  skipped .kit.yml / KIT_* loading for SDK callers
- Configure KIT_* env overrides unconditionally in initConfig so passing an
  explicit config file no longer disables environment variable support
- Wrap config unmarshal/validate errors with %w to preserve the error chain
This commit is contained in:
Ed Zynda
2026-06-02 13:42:55 +03:00
parent 3254045fae
commit f38b4b1734
4 changed files with 33 additions and 18 deletions
+14 -7
View File
@@ -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,14 +39,20 @@ 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 <model> [--thinking-level ...] [--provider-url ...]` is still
// honored; .kit.yml and KIT_* env vars are loaded per session by kit.New.
kitInstance, err := kit.New(ctx, &kit.Options{
SessionDir: cwd,
Quiet: true,
Streaming: true,
// Share the process-global config store so cobra flag bindings and
// config loaded by the CLI's cobra.OnInitialize remain in effect for
// the ACP server. SDK consumers leave CLI nil to get isolated stores.
CLI: &kit.CLIOptions{},
SessionDir: cwd,
Quiet: true,
Streaming: true,
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
+2 -2
View File
@@ -31,7 +31,7 @@ func LoadAndValidateConfigFrom(v *viper.Viper) (*Config, error) {
MCPServers: make(map[string]MCPServerConfig),
}
if err := v.Unmarshal(config); err != nil {
return nil, fmt.Errorf("failed to unmarshal config: %v", err)
return nil, fmt.Errorf("failed to unmarshal config: %w", err)
}
// Fix environment variable case sensitivity issue
@@ -39,7 +39,7 @@ func LoadAndValidateConfigFrom(v *viper.Viper) (*Config, error) {
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
+11 -6
View File
@@ -92,6 +92,17 @@ 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(v, configFile)
}
@@ -130,12 +141,6 @@ func initConfig(v *viper.Viper, configFile string, debug bool) error {
fmt.Fprintf(os.Stderr, "No config file found in current directory or home directory\n")
}
v.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.
v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
v.AutomaticEnv()
return nil
}
+6 -3
View File
@@ -1223,10 +1223,13 @@ func New(ctx context.Context, opts *Options) (*Kit, error) {
setSDKDefaults(v)
// Initialize config (loads config files and env vars) into the instance
// store. Only initialize if not already done (e.g., by CLI's
// cobra.OnInitialize, which populates the shared global store).
// 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 && v.GetString("model") == "" {
if !opts.SkipConfig && opts.CLI == nil {
if err := initConfig(v, opts.ConfigFile, false); err != nil {
return fmt.Errorf("failed to initialize config: %w", err)
}