Add Claude Desktop as an LLM gateway client#5712
Conversation
thv llm setup can route Claude Code and several editors through an OIDC-protected LLM gateway, but not Claude Desktop. Claude Desktop exposes a different configuration surface — its "third-party inference" (Cowork 3P) model — which uses a configLibrary document plus a _meta.json selector and a credential-helper executable, none of which fit the existing JSON-key-patching path. Add a credential-helper client mode that reuses "thv llm token": - New ClaudeDesktop client with LLMGatewayMode "credential-helper". Writes a <uuid>.json config document (inferenceProvider/gateway URL/ helper-script kind/models) and merges a _meta.json selector, preserving any user-owned entries. Generates a no-arg shim that execs "thv llm token" (browser only in the interactive helper context). - Detect Claude Desktop by its app-support directory (a GUI app, not on $PATH); warn when a managed-preferences profile would override the local config; tell the user to relaunch (config is read at launch). - Teardown removes the entry, config document, and shim, and clears appliedId only when it pointed at ToolHive's config. - New --models flag feeds inferenceModels; the key is omitted when empty so the app can fall back to gateway model discovery once available. Verified end-to-end against a real Claude Desktop install (config applied, live inference on all models) plus unit and e2e coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jerm-dro
requested review from
ChrisJBurns,
JAORMX,
amirejaz,
aponcedeleonch,
jhrozek,
lujunsan,
rdimitrov and
reyortiz3
as code owners
There was a problem hiding this comment.
Pull request overview
This PR extends thv llm setup to support configuring Claude Desktop as an LLM gateway client using Claude Desktop’s third-party inference credential-helper configuration model (configLibrary <id>.json + _meta.json selector + helper executable), rather than the existing JSON-pointer patching approach used by other tools. It introduces a dedicated credential-helper writer, adds a --models flag to optionally populate inferenceModels, and updates docs/tests to cover setup/teardown and detection.
Changes:
- Add a new Claude Desktop LLM gateway client mode (
credential-helper) with dedicated configLibrary writer, shim generation, detection, and teardown. - Thread a new
--modelsflag through CLI → setup pipeline and persist it into credential-helper configs (omitted when empty). - Add unit + E2E coverage for Claude Desktop setup/verify/teardown and update docs/CLI help text.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/cli_llm_all_clients_test.go | Adds an E2E scenario covering Claude Desktop credential-helper setup, verification, and teardown. |
| pkg/llmgateway/config.go | Introduces Claude Desktop helper TTL/timeout constants and adds Models to ApplyConfig. |
| pkg/llm/setup.go | Threads models into setup, applies Anthropic base URL logic to credential-helper mode, and adds credential-helper warnings. |
| pkg/llm/setup_test.go | Updates setup tests for the new Setup signature and GatewayManager interface. |
| pkg/client/llm_gateway.go | Dispatches Configure/Revert to the credential-helper writer, adds managed-profile detection, and improves GUI app detection paths. |
| pkg/client/llm_gateway_credential_helper.go | New credential-helper implementation: writes shim, config doc, and _meta.json selector; supports teardown and managed-profile detection. |
| pkg/client/llm_gateway_credential_helper_test.go | Unit tests for config doc writing, _meta.json merge behavior, shim creation, idempotency, and teardown behavior. |
| pkg/client/config.go | Registers claude-desktop client integration metadata (paths, detection, managed profile domain, credential-helper capability). |
| docs/cli/thv_llm_setup.md | Documents Claude Desktop support and the new --models flag semantics. |
| cmd/thv/app/llm.go | Adds --models flag and passes it into llm.Setup; updates command long description to include Claude Desktop. |
| cmd/thv/app/llm_test.go | Updates CLI tests for the new runLLMSetup signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| shimPath, err := cm.writeCredentialHelperShim(cfg.TokenHelperCommand) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| baseURL := cfg.AnthropicBaseURL | ||
| if baseURL == "" { | ||
| baseURL = cfg.GatewayURL | ||
| } |
| // Always attempt shim removal — it is ToolHive-owned and lives at a fixed path. | ||
| shimPath := cm.credentialHelperShimPath() | ||
| if err := os.Remove(shimPath); err != nil && !os.IsNotExist(err) { | ||
| return fmt.Errorf("removing credential helper shim %s: %w", shimPath, err) | ||
| } | ||
|
|
||
| if configPath == "" { | ||
| return nil |
| _, _ = fmt.Fprintf(out, | ||
| "%s reads its configuration only at launch — fully quit (Cmd-Q) and reopen it "+ | ||
| "for the gateway change to take effect.\n", tc.Tool) |
|
Developer Mode (open): Setup writes the configLibrary files directly, no UI. Confirmed live inference works with the app's |
- revertCredentialHelper: de-reference _meta.json and delete the config document before removing the shim, so a mid-revert failure never leaves Claude Desktop pointing at a config that references a missing helper. Treat an empty configPath as a no-op (do not touch the shim when we cannot confirm the selector no longer references it). - configureCredentialHelper: best-effort cleanup of the config document and a freshly-created shim when setup fails, so a failure leaves no partial state. Skip shim removal when an earlier setup already created it (setup is idempotent). - Make the relaunch note platform-neutral (drop macOS-specific "Cmd-Q"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size/L
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5712 +/- ##
==========================================
- Coverage 70.63% 70.59% -0.04%
==========================================
Files 667 668 +1
Lines 67607 67773 +166
==========================================
+ Hits 47752 47843 +91
- Misses 16399 16456 +57
- Partials 3456 3474 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
A few things I ran into going through the Claude Desktop credential-helper code — mostly small, one worth a closer look before merging.
| if err != nil { | ||
| return fmt.Errorf("encoding Claude Desktop config: %w", err) | ||
| } | ||
| configPath = filepath.Join(dir, id+".json") |
There was a problem hiding this comment.
id here comes straight from _meta.json via metaEntryID with no validation before it's joined into a path (filepath.Join(dir, id+".json")). _meta.json lives in a directory Claude Desktop also writes to, so a corrupted or hand-edited entry like {"id": "../../../../.ssh/authorized_keys", "name": "ToolHive Gateway"} would make this write land outside configLibrary. The same untrusted value flows into revertCredentialHelper's metaPath derivation too.
Probably worth rejecting anything that isn't a bare filename in metaEntryID before returning it:
if id == "" || filepath.Base(id) != id || strings.Contains(id, "..") {
return "" // treat as absent; caller mints a fresh UUID
}|
|
||
| // Track whether the shim already existed so failure cleanup does not delete a | ||
| // shim an earlier successful setup still depends on (setup is idempotent). | ||
| shimExisted := fileExistsAt(cm.credentialHelperShimPath()) |
There was a problem hiding this comment.
The shim (shimExisted / writeCredentialHelperShim) is created before WithFileLock(metaPath, ...) is acquired, and removed after the lock is released in revertCredentialHelper (around line 193). So the shim isn't actually covered by the lock that's supposed to serialize concurrent thv llm setup/teardown runs. Two overlapping invocations could interleave such that one process's failure-cleanup removes a shim the other's now-committed _meta.json entry depends on. Might be worth moving the shim create/remove inside the same lock scope as the _meta.json update.
| return "", err | ||
| } | ||
|
|
||
| baseURL := cfg.AnthropicBaseURL |
There was a problem hiding this comment.
This reimplements the same "AnthropicBaseURL else GatewayURL" fallback that resolveApplyConfigField already computes in llm_gateway.go (case "AnthropicBaseURL"). Might be simpler to call that instead of duplicating the logic here, so the two don't drift if the fallback rule ever changes.
| meta["entries"] = removeMetaEntry(metaEntries(meta), id) | ||
| // Only clear appliedId if it still points at our config; leave a | ||
| // user's own active config selection alone. | ||
| if applied, _ := meta["appliedId"].(string); applied == id { |
There was a problem hiding this comment.
applied, _ := meta["appliedId"].(string) treats a present-but-non-string appliedId the same as absent, and leaves it untouched with no signal that the file was malformed. Not a big deal in practice, but checking ok here (and maybe logging) would make a corrupted _meta.json easier to diagnose later.
| Description: "Claude Desktop", | ||
| LLMGatewayOnly: true, | ||
| LLMGatewayMode: "credential-helper", | ||
| LLMCredentialHelper: true, |
There was a problem hiding this comment.
This entry sets both LLMGatewayMode: "credential-helper" and LLMCredentialHelper: true to encode the same fact. llm_gateway.go dispatches on the bool, while pkg/llm/setup.go's usesAnthropicBaseURL/warnCredentialHelperTools dispatch on the string — for the same client. Nothing keeps these in sync, so a future credential-helper-style client could set one without the other. Could probably drop LLMCredentialHelper and dispatch everywhere on LLMGatewayMode == "credential-helper".
| // local config, so "thv llm setup" warns when one is detected (the local config | ||
| // it writes would be ignored). macOS only; returns false elsewhere or when the | ||
| // client declares no managed-profile domain. | ||
| func managedProfilePresent(domain string) bool { |
There was a problem hiding this comment.
managedProfilePresent has no test coverage — the darwin gate, the direct-path check, and the per-user glob fallback are all unexercised. Since this drives the MDM-override warning in warnCredentialHelperTools, a regression here (e.g. an inverted check or a broken glob) would silently disable that warning. Might be worth a small overridable root path (e.g. var managedPreferencesRoot = "/Library/Managed Preferences") so this can be unit tested against a temp dir.
| // (Claude Desktop) need: they read config only at launch, so the user must fully | ||
| // quit and relaunch; and a managed-preferences profile, if present, overrides | ||
| // the local config setup just wrote. | ||
| func warnCredentialHelperTools(out, errOut io.Writer, gm GatewayManager, configured []ToolConfig) { |
There was a problem hiding this comment.
warnCredentialHelperTools isn't covered by any test — all three stub GatewayManagers in setup_test.go hardcode IsManaged to false, so the managed-profile warning branch is never exercised. Worth a test with a stub returning true for one tool to confirm the warning lands on errOut for that tool only, and the relaunch note prints on out for all credential-helper tools.
| // The _meta.json update is done under a file lock and preserves every entry and | ||
| // key ToolHive does not own, so a user's other saved configurations are left | ||
| // intact. Writes are crash-safe via AtomicWriteFile. | ||
| func (cm *ClientManager) configureCredentialHelper(appCfg *clientAppConfig, cfg llmgateway.ApplyConfig) (string, error) { |
There was a problem hiding this comment.
None of the tests in llm_gateway_credential_helper_test.go inject a failure inside the locked section of configureCredentialHelper, so the rollback/cleanup logic (conditional shim removal, conditional config removal) is only exercised by inspection. A test that makes the configLibrary dir read-only before calling ConfigureLLMGateway (so AtomicWriteFile fails) would confirm the cleanup actually behaves as the comments claim, and that a retry afterward succeeds cleanly.
aponcedeleonch
left a comment
aponcedeleonch
left a comment
There was a problem hiding this comment.
If _meta.json is valid JSON but entries isn't an array (say an object, or absent-and-later-set-wrong), configure then does meta["entries"] = append(nil, ourEntry) and silently drops whatever was there. That quietly breaks the "preserve every entry ToolHive does not own" contract the writer is built around. Fully-malformed JSON already aborts safely on the parse error, so this is the narrow "valid JSON, wrong shape" case.
Same input class shows up in metaEntryID: if a name-matching entry has a non-string id, it returns "", and configure then generates a fresh UUID and appends a second "ToolHive Gateway" entry, which defeats the idempotency the stable name is there to provide.
Both are low-likelihood, but since the whole point of reading into a generic map is to not stomp on user data, I'd lean toward bailing with an error when entries is present and not a []any, rather than overwriting. A small test with a non-array entries would lock it in.
Summary
thv llm setuproutes Claude Code and several editors through an OIDC-protected LLM gateway, but not Claude Desktop. Claude Desktop exposes a different configuration surface — its "third-party inference" (Cowork 3P) model — which uses aconfigLibraryconfig document plus a_meta.jsonselector and a credential-helper executable, none of which fit the existing JSON-key-patching path.This adds a credential-helper client mode that reuses
thv llm token:ClaudeDesktopclient (LLMGatewayMode: "credential-helper"). Writes a<uuid>.jsonconfig document (inferenceProvider: gateway, base URL,inferenceCredentialKind: helper-script, optionalinferenceModels) and merges a_meta.jsonselector, preserving any user-owned entries. Generates a no-arg shim that execsthv llm token(browser only in theinteractivehelper context; silent otherwise).$PATH); warns when a managed-preferences profile would override the local config; tells the user to relaunch (config is read only at launch).appliedIdonly when it pointed at ToolHive's config.--modelsflag feedsinferenceModels. The key is omitted when empty so Claude Desktop can fall back to gateway-side model discovery once the gateway serves it (tracked instacklok/stacklok-enterprise-platform#2077— the gateway's/anthropic/v1/modelscurrently 404s, so--modelsis the interim path).Type of change
Test plan
task test) — new writer/_metamerge/shim/teardown/detection tests inpkg/clienttask test-e2e) — newclaude-desktopsetup→verify→teardown spec in the all-client matrixtask lint-fix)Manual testing: Ran
thv llm setup --client claude-desktop --models …against the staging gateway on a real macOS Claude Desktop install. Verified the writtenconfigLibrary/<uuid>.json,_meta.jsonselector, and generated shim match Claude Desktop's own schema; relaunched the app and confirmed the "ToolHive Gateway" config applied and live inference worked on all models. Ranthv llm teardown claude-desktopand confirmed the entry, config document, and shim were removed andappliedIdcleared.Special notes for reviewers
configLibraryfiles directly (no in-app UI), so setup itself doesn't need Claude Desktop's Developer Mode. Whether the app applies the local config at launch with Developer Mode off is not yet confirmed — the live test above was run with Developer Mode on. Worth a quick verification before we document "no Developer Mode required."/bin/shscript, consistent with the rest of the LLM-gateway token-helper feature (buildTokenHelperCommandis already POSIX-only). Setup returns a clear error on Windows.LLMGatewayKeysJSON-pointer machinery — it's a separate writer (pkg/client/llm_gateway_credential_helper.go) dispatched on a newLLMCredentialHelpercapability flag.🤖 Generated with Claude Code