Skip to content

Purge LLM tokens on full teardown - #6733

Open
danbarr wants to merge 1 commit into
mainfrom
issue-6732
Open

danbarr wants to merge 1 commit into
mainfrom
issue-6732

Conversation

@danbarr

@danbarr danbarr commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • A complete LLM teardown could leave reusable cached credentials behind, making a later setup silently reuse stale authentication instead of providing a reliable fresh-auth boundary.
  • Purge cached LLM tokens by default when teardown removes the final configured tool, while adding --keep-tokens for intentional session retention and preserving tokens by default during partial targeted teardown.
  • Make provider, listing, and deletion failures fatal and perform cleanup before configuration mutation so a failed targeted purge can be retried with the same command.
  • Document that cleanup is local-only and that running proxies or token helpers can recreate cached credentials.

Fixes #6732

Type of change

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

Test plan

  • Unit tests (task test) — all affected LLM and CLI tests passed; the full suite remains nonzero on pre-existing pkg/plugins/pluginsvc SSRF and git:// parsing failures
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Build (task build)
  • CLI documentation generation (task docs)
  • Diff validation (git diff --check)

Changes

File Change
cmd/thv/app/llm.go Add conditional token-cleanup defaults, --keep-tokens, fatal cleanup errors, and updated help text
cmd/thv/app/llm_test.go Cover flag exclusivity and user-facing cleanup guidance
pkg/llm/setup.go Purge before config mutation, support zero-tool cleanup, and preserve partial-teardown sessions by default
pkg/llm/setup_test.go Cover full, partial, zero-tool, failure, ordering, and retry behavior
docs/cli/thv_llm_teardown.md Regenerate teardown command documentation

Does this introduce a user-facing change?

Yes. Full and last-tool teardown now purge cached LLM tokens by default. Users can pass --keep-tokens to retain the session. Partial targeted teardown still retains shared tokens unless --purge-tokens is explicit.

Implementation plan

Approved implementation plan
  1. Determine the teardown targets and whether any configured tools remain.
  2. Default to purging for full, last-tool, and zero-tool cleanup; retain tokens for partial teardown unless explicitly purged.
  3. Purge credentials before config or tool mutation so cleanup failures leave the exact command retryable.
  4. Persist the updated config, then revert the selected tool files.
  5. Add CLI guidance and tests for defaults, opt-outs, partial teardown, cleanup failures, and retry behavior.

Special notes for reviewers

Token deletion is intentionally local: it does not revoke credentials at the identity provider or clear browser SSO. This PR warns users that a running LLM proxy or token helper can recreate credentials; automatically stopping those processes is deferred.

@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Oct 1, 2026
@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.35%. Comparing base (47dbcab) to head (6249747).

Files with missing lines Patch % Lines
pkg/llm/setup.go 94.11% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6733   +/-   ##
=======================================
  Coverage   79.34%   79.35%           
=======================================
  Files         802      802           
  Lines       81218    81245   +27     
=======================================
+ Hits        64446    64471   +25     
- Misses      16767    16769    +2     
  Partials        5        5           

☔ 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.

@Sanskarzz

Copy link
Copy Markdown
Collaborator

Thanks, @danbarr, for opening the PR. I’ll review the changes and check the cleanup and retry cases.

@Sanskarzz
Sanskarzz self-requested a review October 1, 2026 12:05

@Sanskarzz Sanskarzz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Approved.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make LLM teardown purge cached credentials reliably by default

2 participants