feat: remove viper dependency from pkg/kit SDK

[ezynda3]

Feature Description

The public SDK package pkg/kit directly imports github.com/spf13/viper and uses viper's process-global config store throughout its implementation. The SDK should be decoupled from viper entirely — viper is a CLI concern, not an SDK concern.

The CLI (cmd/) can still use viper to source configuration from flags/env/config files, but it should pass those values into the SDK as plain Options struct fields. The SDK itself should have zero import of viper.

Current State (verified)

pkg/kit/kit.go line 28 imports viper directly, and there are ~92 references to viper.* calls inside the package. Examples:

// pkg/kit/kit.go
import (
    ...
    "github.com/spf13/viper"
)

// Reads from global viper state:
systemPrompt, _ := config.LoadSystemPrompt(viper.GetString("system-prompt"))
thinkingLevel := models.ParseThinkingLevel(viper.GetString("thinking-level"))
extraPaths := viper.GetStringSlice("extension")
modelString = viper.GetString("model")
debug = viper.GetBool("debug")

// Writes to global viper state from New():
viper.Set("model", opts.Model)
viper.Set("system-prompt", opts.SystemPrompt)
viper.Set("max-steps", opts.MaxSteps)
viper.Set("stream", opts.Streaming)

pkg/kit/kit_test.go also imports viper and relies on a resetViper() helper between tests. The package even has self-aware comments acknowledging the problem:

// Global viper state warning:
// Options are applied by [New] via [viper.Set] calls against viper's
// process-global store...
//
// 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

Motivation / Use Case

1. SDK contract hygiene — per AGENTS.md, pkg/kit/ is the public API surface for external Go developers embedding Kit as a library. Internal dependencies (viper, fantasy, etc.) should not leak into the SDK's import graph. Forcing SDK consumers to transitively depend on viper is exactly the kind of leakage the SDK rules are meant to prevent.
2. Global state hazards — viper's global store means two kit.New() calls in the same process race each other. The existing viperInitMu mutex only serializes writes during construction; it does not prevent later reads from seeing clobbered state. Subagent spawning, tests, and any multi-Kit embedding scenario are all affected. The TODO comments in kit.go already flag this.
3. Testability — kit_test.go has to import viper and use a resetViper() helper between tests. An SDK consumer should never need to know viper exists.
4. Embedding friction — anyone embedding Kit must reason about viper's global state even if they have their own config system.

Proposed Implementation

1. Define a self-contained config struct inside pkg/kit (expand Options or introduce an internal resolvedConfig struct) that holds every value currently read via viper.Get*.
2. In cmd/ (CLI layer), build that struct from viper and pass it to kit.New() via Options. This is the "viper → CLI → SDK" boundary described above — passing options through the CLI is fine; importing viper in the SDK is not.
3. Replace every viper.Get* call inside pkg/kit with a field access on the resolved config.
4. Replace every viper.Set call inside pkg/kit with a struct field write (many exist only so a later viper.Get in the same package can read it back).
5. For state that needs to be mutable at runtime (e.g. SetThinkingLevel at lines ~2626/2635), store it on the Kit instance, not viper.
6. Remove the github.com/spf13/viper import from pkg/kit/kit.go and verify with go list -deps ./pkg/kit | grep viper (should be empty).
7. Drop viperInitMu — no longer needed once global state is gone.

Acceptance: grep -r viper pkg/kit/ returns nothing (other than possibly historical comments), and pkg/kit no longer transitively depends on viper.

---

Handling Defaults

Removing viper means we lose viper.IsSet(), which the SDK currently uses as a tri-state signal. pkg/kit/config.go deliberately does not call viper.SetDefault for certain keys precisely because doing so would make IsSet() return true and suppress downstream defaults. We must preserve this tri-state explicitly in Go.

Each setting falls into one of three precedence tiers:

1. Explicitly set by the caller (flag / env / config / Options) → use that value.
2. Unset, but has a package default (model, system-prompt, stream, num-gpu-layers, main-gpu) → currently registered via viper.SetDefault. These become plain Go constants applied in a resolver.
3. Unset, and must stay unset (max-tokens, temperature, top-p, top-k, frequency/presence-penalty, thinking-level) → deliberately given no default so downstream logic still fires:
   • max-tokens: applied as a last-resort struct-level floor (sdkDefaultMaxTokens = 8192) on *models.ProviderConfig after BuildProviderConfig, so per-model defaults (ApplyModelSettings) and auto right-sizing (rightSizeMaxTokens) still run.
   • thinking-level: handled by models.ParseThinkingLevel("") → ThinkingOff.
   • sampling params: left as nil pointers so provider libraries apply their own defaults.

Mapping viper.IsSet() → dependency-free equivalent

Replace !viper.IsSet(key) with a nil pointer (or a small Option[T]{ v, set } generic). A nil pointer = "unset, let downstream default win"; non-nil = "explicit." This is the same pattern the sampling params (*Temperature, *TopP, ...) already use — extend it to every tier-3 setting. This also kills the global-state race that viperInitMu exists to paper over, since state becomes per-instance.

⚠️ Do NOT collapse the tri-state

stream, num-gpu-layers, main-gpu, max-tokens, and the sampling params must not be given collapsing zero-value defaults. MaxTokens: 0 / Streaming: false currently mean "resolve via chain," not "force this value." Naively moving these to plain struct fields would silently break per-model defaults and right-sizing.

Note: Streaming bool is an existing latent edge case — New() unconditionally does viper.Set("stream", opts.Streaming), so an SDK caller cannot currently leave it "unset" (false always wins). The refactor should make a deliberate choice here (honor the bool as-is, or switch to *bool) and document it.

Boundary

All flag/env/config-file precedence resolution stays in cmd/ (where viper lives legitimately). The CLI translates resolved values into Options; the SDK consumes a fully-resolved struct.

---

Ergonomic Constructor & Backward Compatibility

Does this break the API contract?

No, if done carefully. viper never appears in an exported signature — it's purely an implementation detail of kit.go function bodies. The public contract is New(ctx, *Options) (*Kit, error) plus the Options fields, both of which stay unchanged. New(ctx, *Options) is used throughout the README and examples/sdk/*, so it must keep working and should not be deprecated (it's the escape hatch for programmatic config, MCPConfig, InProcessMCPServers).

The only behavioral risk is the tier-3 tri-state described above — preserve the documented "0 = resolve via chain" conventions and it remains non-breaking (except the Streaming nuance noted above).

Add a functional-options front door

Layer an ergonomic constructor on top, purely additively:

// Existing low-level constructor stays as-is (backward compatible).
func New(ctx context.Context, opts *Options) (*Kit, error)

// New ergonomic variadic constructor.
func NewAgent(ctx context.Context, opts ...Option) (*Kit, error) {
    o := &Options{}
    for _, fn := range opts { fn(o) }
    return New(ctx, o)
}

type Option func(*Options)

func WithModel(m string) Option        { return func(o *Options) { o.Model = m } }
func WithSystemPrompt(p string) Option { return func(o *Options) { o.SystemPrompt = p } }
func WithStreaming(b bool) Option      { return func(o *Options) { o.Streaming = b } }
func WithMaxTokens(n int) Option       { return func(o *Options) { o.MaxTokens = n } }
func WithTools(t ...Tool) Option       { return func(o *Options) { o.Tools = t } }
func Ephemeral() Option                { return func(o *Options) { o.NoSession = true } }

Usage:

agent, err := kit.NewAgent(ctx,
    kit.WithModel("anthropic/claude-sonnet-4-5"),
    kit.WithStreaming(true),
    kit.Ephemeral(),
)

Why this also solves the defaults problem

With functional options, "option not provided" IS the unset signal — there is no zero-vs-unset ambiguity at the struct level, because absent With* calls simply never touch the field. This is also the clean CLI→SDK bridge: the CLI reads viper and appends an option only when the key is set:

// in cmd/ — viper lives here, legitimately
var opts []kit.Option
if viper.IsSet("model")      { opts = append(opts, kit.WithModel(viper.GetString("model"))) }
if viper.IsSet("max-tokens") { opts = append(opts, kit.WithMaxTokens(viper.GetInt("max-tokens"))) }
agent, _ := kit.NewAgent(ctx, opts...)

viper.IsSet() → "append option or not" is the natural tri-state bridge across the boundary, with zero viper in the SDK.

Recommendation

1. Keep New(ctx, *Options) (do not deprecate).
2. Add NewAgent(ctx, ...Option) as the ergonomic entry point with With* / Ephemeral() helpers.
3. Option is func(*Options) — no dependency leakage.

[mark-iii-labs-huly]

Connected to Huly®: KIT-39

[ezynda3]

Design proposal — scoping the transitive viper removal

I've mapped the full reach of viper from pkg/kit. The pkg/kit-local design in this issue (resolved-config struct, viper.Set→struct fields, tri-state via nil pointers, NewAgent/With* front door) is clear and I'm ready to implement it. Before I do, I want sign-off on the internal-package mechanism, because the acceptance criterion "pkg/kit no longer transitively depends on viper" (go list -deps ./pkg/kit | grep viper empty) reaches well beyond pkg/kit/:

Non-test viper call sites reachable from pkg/kit (~38 total):

• internal/kitsetup/setup.go — BuildProviderConfig() + SetupAgent() read viper directly (called by both pkg/kit and cmd)
• internal/config/merger.go — LoadAndValidateConfig() (MCP config) via viper.Unmarshal / viper.AllSettings
• internal/config/config.go — GetField helpers via viper.UnmarshalKey
• internal/models/custom.go + registry.go — the global ModelsRegistry singleton lazily loads customModels / modelSettings from viper's global store
• internal/extensions/runner.go — reads one config key from viper

Why there's no safe partial. If I only strip viper from pkg/kit's own source (drop the viper.Set calls), the downstream readers above stop observing those values and behavior silently breaks (model selection, per-model settings, custom models, MCP config). So the change is effectively all-or-nothing across the config subsystem.

Proposed architecture

1. Introduce a viper-free resolved-config type (e.g. internal/config.Settings) holding every value currently read via viper.Get*, using the tri-state convention from the issue (*bool/*int/nil pointers = "unset, let downstream default win").
2. Add a viper-free .kit.yml loader in internal/config (read file → env-substitution → YAML/JSON unmarshal into Settings + the Config struct). This replaces the SDK's reliance on viper.ReadInConfig/AutomaticEnv.
3. Thread Settings explicitly:
   • kitsetup.BuildProviderConfig(s *config.Settings) and SetupAgent(... ) take resolved values instead of reading viper.
   • internal/models custom-model/model-settings loading takes Settings (inject into the registry rather than reading the global viper store).
   • internal/extensions runner takes its one key explicitly.
4. Confine viper to cmd/: the CLI keeps viper for flags/env/config-file precedence, builds Settings (using viper.IsSet → "set field or leave nil" as the tri-state bridge), and passes it down — exactly the "viper → CLI → SDK" boundary described in the issue.
5. pkg/kit builds Settings from Options (+ the new loader for SDK consumers) — zero viper import.

Phasing question

This is a sizable, high-regression-risk change to config precedence + the global model registry. Two options:

• (A) One PR doing the whole transitive removal (achieves go list -deps clean).
• (B) Two PRs: (1) land the internal/config.Settings + viper-free loader + thread through kitsetup/models/extensions with cmd still using viper to populate Settings; (2) flip pkg/kit to Settings + add NewAgent/With*.

I'd lean toward (A) for a single coherent diff, but it touches the global ModelsRegistry singleton, so I want your nod on (a) the Settings-injection approach for the registry and (b) whether New(ctx, *Options) keeping its current config-file auto-loading behavior for SDK callers is required (vs. SDK callers being expected to pass config explicitly).

Once you confirm the internal mechanism + phasing, I'll implement with full tri-state preservation and regression tests.

[ezynda3]

Closing in favor of a re-scoped issue.

After mapping the full transitive reach of viper from pkg/kit (~38 call sites across 5 internal packages plus the global ModelsRegistry singleton), the "remove viper entirely from the SDK's import graph" framing turns out to be a high-risk, low-payoff trade:

• viper does not leak per AGENTS.md's actual definition — it never appears in any exported symbol, type, godoc, or struct field. It's purely an implementation detail of function bodies, which the SDK rules explicitly permit.
• The genuine defect is viper's process-global singleton (the subagent-spawn race that viperInitMu papers over), not viper-the-dependency. That has a far cheaper fix: a per-instance viper.New() store per Kit.
• A full removal would force re-implementing viper's layered precedence, IsSet() tri-state, AutomaticEnv, and env-substituting config loader by hand across kitsetup/config/models/extensions — net-new bespoke config plumbing in the most behavior-sensitive part of the system, for the sole benefit of a cosmetic go list -deps metric.

The valuable parts — config isolation (kills the race, drops viperInitMu) and ergonomics (NewAgent + With* functional options) — are achievable without the sprawling purge.

Re-scoped follow-up: #40