diff --git a/pkg/llm/config.go b/pkg/llm/config.go index 18b789466c..8be3928bfc 100644 --- a/pkg/llm/config.go +++ b/pkg/llm/config.go @@ -43,6 +43,8 @@ type Config struct { // BedrockConfig holds settings for configuring Claude Code to reach an LLM // gateway that forwards to AWS Bedrock. It is persisted so that a later plain // "thv llm setup" re-applies these settings rather than silently clearing them. +// That stickiness ends at teardown: reverting the last configured tool resets the +// whole LLM config (see Teardown), so it cannot outlive the tools that used it. type BedrockConfig struct { // Compat, when true, writes CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS=1 and the // per-tier Bedrock model IDs into Claude Code's settings.json. Bedrock rejects diff --git a/pkg/llm/setup.go b/pkg/llm/setup.go index d4516e29b3..8bd8ab9a7c 100644 --- a/pkg/llm/setup.go +++ b/pkg/llm/setup.go @@ -191,6 +191,12 @@ func Setup( // configured tools. An error is returned when targetTool is non-empty but not // found in the configured tool list. // +// Reverting the last configured tool also resets the persisted LLM config to its +// zero value, so settings that are deliberately sticky across "thv llm setup" +// re-runs (e.g. Bedrock compat) are not silently re-applied by a later setup. +// A targeted teardown that leaves other tools configured keeps the config, which +// those tools still need to reach the gateway. +// // If secretsProvider is non-nil and purgeTokens is true, cached OIDC tokens // are deleted after the config update succeeds. func Teardown( @@ -239,17 +245,19 @@ func Teardown( // Persist the updated tool list (and clear token metadata if purging) in a // single write before mutating any tool config files. If this fails, // nothing on disk has changed and the caller can retry. + lastTool := len(remaining) == 0 if err := provider.UpdateLLMConfig(func(c *Config) error { - c.ConfiguredTools = remaining - if purgeTokens { - c.OIDC.CachedRefreshTokenRef = "" - c.OIDC.CachedTokenExpiry = time.Time{} - } + applyTeardownToConfig(c, remaining, purgeTokens) return nil }); err != nil { return fmt.Errorf("persisting tool configuration: %w", err) } + if lastTool { + _, _ = fmt.Fprintln(out, + "Cleared the LLM gateway configuration: no tools are configured anymore.") + } + // Revert tool config files best-effort; warn on failure but do not undo // the config update above (the user can re-run setup+teardown to reconcile). for _, tc := range toRevert { @@ -265,6 +273,40 @@ func Teardown( return nil } +// applyTeardownToConfig updates the persisted LLM config for a teardown that +// leaves remaining configured. purgeTokens reports whether the caller also asked +// to drop cached OIDC token state. +// +// When no tool remains, the config is reset wholesale rather than pruned field by +// field. Settings such as Bedrock compat are deliberately sticky across "thv llm +// setup" re-runs, so keeping them past the last teardown would let the next setup +// silently re-apply settings the user just removed — possibly against a gateway +// they have since repointed elsewhere. While any tool remains the config is left +// intact, since those tools still need it to reach the gateway. +// +// Cached token state survives a reset unless purgeTokens is set: the secret lives +// in the keyring, and dropping the only reference to it without deleting it would +// strand it there once the user points at a different gateway (the fallback key is +// derived from the gateway URL and issuer). Token lifetime stays the exclusive +// business of --purge-tokens. +func applyTeardownToConfig(c *Config, remaining []ToolConfig, purgeTokens bool) { + if len(remaining) > 0 { + c.ConfiguredTools = remaining + if purgeTokens { + c.OIDC.CachedRefreshTokenRef = "" + c.OIDC.CachedTokenExpiry = time.Time{} + } + return + } + + tokenState := c.OIDC + *c = Config{} + if !purgeTokens { + c.OIDC.CachedRefreshTokenRef = tokenState.CachedRefreshTokenRef + c.OIDC.CachedTokenExpiry = tokenState.CachedTokenExpiry + } +} + // PurgeTokens deletes all cached OIDC tokens from the provided secrets // provider. Errors are logged as warnings rather than returned. func PurgeTokens(ctx context.Context, errOut io.Writer, provider pkgsecrets.Provider) { diff --git a/pkg/llm/setup_test.go b/pkg/llm/setup_test.go index aa0ae2c841..141d3a5f4b 100644 --- a/pkg/llm/setup_test.go +++ b/pkg/llm/setup_test.go @@ -355,6 +355,118 @@ func TestTeardown_PurgeTokens_ClearsConfigRefsAndDeletesSecrets(t *testing.T) { assert.Equal(t, []string{"cursor"}, gm.reverted) } +// fullSetupConfig returns a config as "thv llm setup" would leave it, with the +// given tools configured and every persisted setting populated. +func fullSetupConfig(tools ...string) Config { + configured := make([]ToolConfig, len(tools)) + for i, tool := range tools { + configured[i] = ToolConfig{Tool: tool, ConfigPath: "/tmp/" + tool + ".json"} + } + return Config{ + GatewayURL: "https://gw.example.com", + TLSSkipVerify: true, + OIDC: OIDCConfig{ + Issuer: "https://auth.example.com", + ClientID: "cid", + CachedRefreshTokenRef: "secret-ref", + }, + Proxy: ProxyConfig{ListenPort: 14001}, + Bedrock: BedrockConfig{Compat: true, Enable1M: true}, + Models: []string{"us.anthropic.claude-opus-4-8"}, + ConfiguredTools: configured, + } +} + +// TestTeardown_ResetsConfigWhenLastToolReverted verifies that reverting the last +// configured tool resets the whole LLM config. Settings like Bedrock compat are +// deliberately sticky across "thv llm setup" re-runs, so leaving them behind +// would let a later setup silently re-apply settings the user just tore down. +// +// Cached token state is the one exception: it is carried over so the keyring +// secret it points at is not stranded, and stays the business of --purge-tokens. +func TestTeardown_ResetsConfigWhenLastToolReverted(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + configured []string + targetTool string + }{ + { + name: "targeted teardown of the only tool", + configured: []string{"claude-code"}, + targetTool: "claude-code", + }, + { + name: "untargeted teardown of every tool", + configured: []string{"claude-code", "claude-desktop", "cursor"}, + targetTool: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + provider := &stubConfigUpdater{cfg: fullSetupConfig(tt.configured...)} + + var stdout, stderr bytes.Buffer + err := Teardown(context.Background(), &stdout, &stderr, + &stubGatewayManager{}, tt.targetTool, false, provider, nil) + require.NoError(t, err) + + want := Config{OIDC: OIDCConfig{CachedRefreshTokenRef: "secret-ref"}} + assert.Equal(t, want, provider.cfg, + "no tools remain, so every persisted setting except cached token state must be reset") + assert.Contains(t, stdout.String(), "Cleared the LLM gateway configuration") + }) + } +} + +// TestTeardown_ResetsCachedTokenStateWhenPurging verifies that --purge-tokens +// still clears the cached token refs on a full teardown, so the config keeps no +// pointer to secrets that PurgeTokens deletes. +func TestTeardown_ResetsCachedTokenStateWhenPurging(t *testing.T) { + t.Parallel() + + provider := &stubConfigUpdater{cfg: fullSetupConfig("claude-code")} + + var stdout, stderr bytes.Buffer + err := Teardown(context.Background(), &stdout, &stderr, + &stubGatewayManager{}, "", true, provider, nil) + require.NoError(t, err) + + assert.Equal(t, Config{}, provider.cfg) +} + +// TestTeardown_KeepsConfigWhileToolsRemain verifies that a targeted teardown +// preserves the gateway configuration the still-configured tools depend on. +// Zeroing it here would break "thv llm token" and "thv llm proxy start" for a +// tool the user never asked to touch, since both gate on IsConfigured(). +func TestTeardown_KeepsConfigWhileToolsRemain(t *testing.T) { + t.Parallel() + + provider := &stubConfigUpdater{cfg: fullSetupConfig("claude-code", "cursor")} + + var stdout, stderr bytes.Buffer + err := Teardown(context.Background(), &stdout, &stderr, + &stubGatewayManager{}, "claude-code", false, provider, nil) + require.NoError(t, err) + + assert.True(t, provider.cfg.IsConfigured(), + "cursor still routes through the gateway, so its config must survive") + assert.Equal(t, "secret-ref", provider.cfg.OIDC.CachedRefreshTokenRef, + "an unrelated teardown must not force a fresh login") + assert.Equal(t, 14001, provider.cfg.EffectiveProxyPort()) + assert.NotContains(t, stdout.String(), "Cleared the LLM gateway configuration") + + var remaining []string + for _, tc := range provider.cfg.ConfiguredTools { + remaining = append(remaining, tc.Tool) + } + assert.Equal(t, []string{"cursor"}, remaining) +} + func TestTeardown_NoPurge_LeavesTokenRefsIntact(t *testing.T) { t.Parallel()