Skip to content

Reset the LLM config when the last tool is torn down - #6295

Merged
jerm-dro merged 3 commits into
mainfrom
jerm-dro/01KZT0PZF6Z1V1YVWFEJJA8E9X
Aug 13, 2026
Merged

Reset the LLM config when the last tool is torn down#6295
jerm-dro merged 3 commits into
mainfrom
jerm-dro/01KZT0PZF6Z1V1YVWFEJJA8E9X

Conversation

@jerm-dro

@jerm-dro jerm-dro commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

thv llm setup re-applied Bedrock settings that the user had already torn down. Settings like Bedrock and Models are persisted so a plain setup re-run keeps them (#5832), but nothing ever ended that stickiness — a value outlived every tool that read it and got re-applied by the next setup, even against a gateway since repointed elsewhere. Models survived a full teardown; Bedrock could only be cleared with an explicit --bedrock-compat=false.

Teardown now resets the LLM config once no configured tool remains, the same thing thv llm config reset already does. A teardown that leaves other tools configured is unchanged — they still need the gateway settings to work.

Cached OIDC tokens survive the reset unless --purge-tokens is passed, so the keyring secret is never left orphaned.

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e) — not run locally; CI's E2E Tests Core (llm) passed
  • Linting (task lint-fix)
  • Manual testing (describe below)

Three teardown tests cover the rule: last tool reverted resets the config, a targeted teardown preserves it for the tools that remain, and --purge-tokens still clears the token refs. Also drove setup → teardown → setup end-to-end to confirm the second setup no longer re-pins the Bedrock model IDs.

Does this introduce a user-facing change?

Yes. After tearing down every tool, thv llm setup needs the connection settings supplied again — the same state as a fresh install. Cached tokens are untouched unless --purge-tokens is passed.

Special notes for reviewers

Reset-when-empty is deliberately coarser than tracking which client reads which setting, so one case remains: tearing down claude-code while claude-desktop is still configured leaves Bedrock set with no consumer. Nothing reads it until Claude Code is set up again, and the next full teardown clears it.

Not fixed here — changing --gateway-url without a teardown still re-applies Bedrock keys. Same root cause, worth its own PR.

Best reviewed squashed; the branch history walks through three earlier approaches.

Generated with Claude Code

Bedrock compat is persisted so a later plain "thv llm setup" keeps it,
but it applies to no client other than Claude Code. Tearing Claude Code
down therefore left the setting with no consumer, and only an explicit
--bedrock-compat=false could clear it. A subsequent setup — possibly
against a gateway that no longer forwards to Bedrock — silently re-pinned
the Bedrock model IDs and CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS the user
had just removed, which reads as setup forcing Bedrock unconditionally.

Clear the persisted Bedrock config when Claude Code is among the reverted
tools. Stickiness across ordinary re-runs is unchanged.
@jerm-dro
jerm-dro requested a review from JAORMX as a code owner August 13, 2026 00:13
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.90%. Comparing base (3c4dec3) to head (2575cf5).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
pkg/llm/setup.go 82.35% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6295      +/-   ##
==========================================
+ Coverage   72.85%   72.90%   +0.04%     
==========================================
  Files         743      742       -1     
  Lines       77681    77835     +154     
==========================================
+ Hits        56596    56742     +146     
+ Misses      17118    17102      -16     
- Partials     3967     3991      +24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Bedrock compat was not the only persisted setting that could outlive the
client it configures. Models has the same shape: it is read only by Claude
Desktop and, under Bedrock compat, Claude Code, yet it survived a full
teardown and was silently re-applied by the next setup.

Replace the Bedrock-specific check with a registry of client-scoped
settings, each naming the clients that consume it. Teardown clears any
entry left with no consumer among the still-configured tools and reports
what it removed, so a later setup no longer re-applies settings the user
tore down. A setting is preserved while any of its consumers remains, so
Models outlives a Claude Code teardown when Claude Desktop is still set up.

Settings that apply to every client, such as TLSSkipVerify, are not
registered and are unaffected.
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 13, 2026
@jerm-dro jerm-dro changed the title Clear Bedrock compat when tearing down Claude Code Clear client-scoped LLM settings stranded by teardown Aug 13, 2026
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 13, 2026
@jerm-dro
jerm-dro marked this pull request as draft August 13, 2026 17:21
Replace the per-setting stranded-value pruning with a single rule:
teardown resets the whole LLM config once no configured tool remains.

Settings like Bedrock compat and the model list are deliberately sticky so
an ordinary "thv llm setup" re-run keeps them. Nothing ended that
stickiness, so a value could outlive every tool that read it and be
silently re-applied by the next setup, possibly against a gateway the user
had since repointed elsewhere.

Resetting wholesale drops the need to track which client consumes which
setting, and matches what "thv llm config reset" already does. A targeted
teardown that leaves other tools configured keeps the config, which those
tools still need to reach the gateway.

Cached token state survives the reset unless --purge-tokens is passed: the
secret lives in the keyring, and dropping the only reference without
deleting it would strand it once the gateway URL changes.
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 13, 2026
@jerm-dro jerm-dro changed the title Clear client-scoped LLM settings stranded by teardown Reset the LLM config when the last tool is torn down Aug 13, 2026
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 13, 2026
@jerm-dro
jerm-dro marked this pull request as ready for review August 13, 2026 18:17
@jerm-dro
jerm-dro merged commit 374c306 into main Aug 13, 2026
51 checks passed
@jerm-dro
jerm-dro deleted the jerm-dro/01KZT0PZF6Z1V1YVWFEJJA8E9X branch August 13, 2026 18:21
@github-actions github-actions Bot mentioned this pull request Aug 14, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants