feat: add enable/disable toggle for Claude Desktop routing - #1291
Conversation
Adds an on/off toggle to the Claude Desktop tab, mirroring the Claude Code connection toggle. When off, the 3P config files are removed from Claude Desktop's config library; when on, they are written back from the saved profile. Server: - src/types.ts: add desktopEnabled flag to OcxClaudeCodeConfig - src/claude/desktop-3p.ts: new clearDesktop3pConfig() to remove the opencodex entry from Claude Desktop's _meta.json and delete the config JSON file - src/server/management/native-integration-routes.ts: add claude-desktop to native integration client IDs, desktopStatus() probe, and handleDesktopToggle() PUT route with single-flight guard - src/server/management/agent-settings-routes.ts: include enabled field in GET /api/claude-desktop and /status responses GUI: - gui/src/pages/ClaudeDesktop.tsx: Switch toggle above the toolbar; when OFF, hides Save & Apply (only Save remains) and shows a disabled notice - gui/src/pages/integrations/native-api.ts: add claude-desktop to valid client IDs - gui/src/pages/integrations/integration-api.ts: extract enabled from the status payload - gui/src/pages/integrations/overview-clients.ts: add enabled? to ClaudeDesktopPayload i18n: 4 new keys (toggleFailed, enabledLabel, toggleAria, disabledNotice) in all 6 locales
📝 WalkthroughWalkthroughAdds persisted Claude Desktop routing enablement, native 3P configuration management, API status fields, GUI toggle behavior, disabled-state save handling, and localized messages. ChangesClaude Desktop routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeDesktop as ClaudeDesktop.tsx
participant NativeRoutes as native-integration-routes.ts
participant Desktop3P as desktop-3p.ts
ClaudeDesktop->>NativeRoutes: PUT /api/native-integrations/claude-desktop
NativeRoutes->>Desktop3P: write or clear 3P configuration
Desktop3P-->>NativeRoutes: return configuration result
NativeRoutes-->>ClaudeDesktop: return enabled status
ClaudeDesktop->>NativeRoutes: refresh profile and status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/pages/integrations/overview-clients.ts (1)
94-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winOverview grid ignores Claude Desktop's new
enabledfield.
ClaudeDesktopPayload.enabledis added at Line 99, andloadClaudeDesktopStatus(gui/src/pages/integrations/integration-api.ts) already populates it end to end. ButclaudeDesktopRow(Lines 274-311) never readspayload.enabled. It only branches onpayload.applied,payload.stale, andpayload.activeProfile.Consequence: when a user turns off "Claude Desktop routing" on the Desktop page, but a prior apply left
applied: truein the backend state, the Integrations overview row still reportsstate: "current"/applied: true. The overview card and the "Configured clients" summary count Claude Desktop as applied even though routing is off — the file's own docstring calls this exact miscount pattern out for the Claude Code / Grok cases it fixed (isAppliedState,claudeDetailKey), but Claude Desktop was not given equivalent treatment here.Compare with
claudeRow(Lines 225-262), which derivesclaudeDetailKeyfrompayload.enabled !== trueand surfacesintegrations.detail.claudeOff. Add an analogous branch toclaudeDesktopRow, for example:🛠️ Proposed fix
function claudeDesktopRow(payload: ClaudeDesktopPayload | null): OverviewRow { const base = { id: "claudeDesktop" as const, hash: "integrations/claude/desktop", labelKey: "claudeDesktop.title" as TKey, toggle: null, toggleBlocked: null, togglePath: null, status: null, detail: null, detailVars: null, }; if (!payload) return { ...base, state: "unknown", installed: false, applied: false, detailKey: null }; + if (payload.enabled === false) { + return { + ...base, + state: "absent", + installed: true, + applied: false, + detailKey: "integrations.detail.claudeOff", + }; + } if (payload.applied !== true) {Verify with the maintainer whether the overview page is intentionally out of scope for this PR; if so, at minimum leave a comment explaining why
enabledis unused here, since an unused payload field on a freshly extended type reads as an oversight.Also applies to: 274-311
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gui/src/pages/integrations/overview-clients.ts` around lines 94 - 104, Update claudeDesktopRow to account for payload.enabled before deriving its state from applied, stale, or activeProfile, treating any value other than true as disabled and surfacing the Claude Desktop equivalent of integrations.detail.claudeOff. Preserve the existing applied/current behavior only when enabled is true, and ensure overview applied status and configured-client counts no longer report disabled routing as applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/pages/ClaudeDesktop.tsx`:
- Around line 317-329: Update toggleDesktop to call
toggleNativeIntegration(apiBase, "claude-desktop", next, signal) instead of
using raw fetch and readJsonOrThrow. Preserve the existing pending-state and
success handling, and catch NativeApiError so its refusal message and
reason-specific data are mapped into the existing setMessage/setAnnouncement
flow.
- Around line 317-336: Guard the optional result from readJsonOrThrow in
toggleDesktop before accessing body.message, while preserving the successful
setDesktopEnabled flow for valid responses. Only display the server message and
announcement when body exists and contains a message, so an empty or invalid
successful response cannot throw after the local state is updated.
In `@src/claude/desktop-3p.ts`:
- Around line 399-406: Validate the selected opencodex entry ID with the
project’s canonical UUID validation in a shared metadata helper before
constructing any path. Update both the deletion flow around parseMetadata and
writeDesktop3pConfig to reject non-UUID IDs, preventing join-based path
traversal while preserving the existing invalid-entry handling.
In `@src/server/management/agent-settings-routes.ts`:
- Line 665: Gate every Claude Desktop configuration writer on
config.claudeCode?.desktopEnabled: return before writeDesktop3pConfig in
autoApplyDesktopBestEffort and reject POST /api/claude-desktop/apply while
disabled. Preserve restoration only through the native toggle-enable path.
Ensure tokens and OAuth material, including the API key passed to the writer,
are never logged or serialized in responses.
In `@src/server/management/native-integration-routes.ts`:
- Around line 558-564: Update src/server/management/native-integration-routes.ts
lines 558-564 in the idempotent toggle path to verify the on-disk 3P entry and
retry writeDesktop3pConfig or clearDesktop3pConfig before returning changed:
false. Update lines 121-127 to report the observed _meta.json entry state, or
expose a separate configured-enable value, so failed cleanup is not reported as
absent.
---
Outside diff comments:
In `@gui/src/pages/integrations/overview-clients.ts`:
- Around line 94-104: Update claudeDesktopRow to account for payload.enabled
before deriving its state from applied, stale, or activeProfile, treating any
value other than true as disabled and surfacing the Claude Desktop equivalent of
integrations.detail.claudeOff. Preserve the existing applied/current behavior
only when enabled is true, and ensure overview applied status and
configured-client counts no longer report disabled routing as applied.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a4943e6a-9b67-4a27-9e53-f2562351ff81
📒 Files selected for processing (14)
gui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/ClaudeDesktop.tsxgui/src/pages/integrations/integration-api.tsgui/src/pages/integrations/native-api.tsgui/src/pages/integrations/overview-clients.tssrc/claude/desktop-3p.tssrc/server/management/agent-settings-routes.tssrc/server/management/native-integration-routes.tssrc/types.ts
| const toggleDesktop = async () => { | ||
| if (connectionInFlight.current) return; | ||
| connectionInFlight.current = true; | ||
| setConnectionPending(true); | ||
| setMessage(null); | ||
| const next = !desktopEnabled; | ||
| try { | ||
| const response = await fetch(`${apiBase}/api/native-integrations/claude-desktop`, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ enabled: next }), | ||
| }); | ||
| const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed")); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Reuse toggleNativeIntegration instead of a raw fetch for the Desktop toggle.
gui/src/pages/integrations/native-api.ts already exports toggleNativeIntegration(apiBase, client, enabled, signal), which PUTs the same endpoint shape (/api/native-integrations/${client}), returns a typed NativeToggleEnvelope (ok, clientId, changed, state, message, reason), and throws a typed NativeApiError with a parsed .refusal (including disableBlocked reason codes like not_installed or config_busy) on failure. NativeIntegrationClientId now includes "claude-desktop" (native-api.ts Line 11), so this helper is directly usable here.
The raw fetch + readJsonOrThrow path in toggleDesktop (Lines 317-329) bypasses that refusal-aware parsing. A native refusal from the server (for example, the config file busy or Desktop not installed) will still throw via readJsonOrThrow's error path, but only as a flat string pulled from error/message, losing the reason code that the rest of the native-integration surface (claudeRow, grokRow, toggleBlocked: native?.disableBlocked) uses to drive localized, reason-specific messaging.
Do you want me to generate a version of toggleDesktop that calls toggleNativeIntegration and maps NativeApiError.refusal?.message into the existing setMessage/setAnnouncement flow?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/pages/ClaudeDesktop.tsx` around lines 317 - 329, Update toggleDesktop
to call toggleNativeIntegration(apiBase, "claude-desktop", next, signal) instead
of using raw fetch and readJsonOrThrow. Preserve the existing pending-state and
success handling, and catch NativeApiError so its refusal message and
reason-specific data are mapped into the existing setMessage/setAnnouncement
flow.
| const toggleDesktop = async () => { | ||
| if (connectionInFlight.current) return; | ||
| connectionInFlight.current = true; | ||
| setConnectionPending(true); | ||
| setMessage(null); | ||
| const next = !desktopEnabled; | ||
| try { | ||
| const response = await fetch(`${apiBase}/api/native-integrations/claude-desktop`, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ enabled: next }), | ||
| }); | ||
| const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed")); | ||
| setDesktopEnabled(next); | ||
| if (body.message) { | ||
| setMessage({ tone: "ok", text: body.message }); | ||
| setAnnouncement(body.message); | ||
| } | ||
| void desktopResource.refresh(); | ||
| void statusResource.refresh(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Unguarded body.message access on a possibly-undefined value.
readJsonOrThrow is declared as Promise<T | undefined> (see its definition in gui/src/fetch-json.ts, also surfaced in this PR's own graph context). At Line 329, body therefore has type { ok?: boolean; message?: string; state?: string } | undefined. Line 331 then reads body.message with no null check.
Every other call site of readJsonOrThrow in this same file (Lines 189, 355, 361) either discards the return value or only relies on the throw-on-!res.ok side effect. This is the only place the return value is stored and its property accessed directly, and the only place missing a guard.
Failure mode: if the PUT response is res.ok but the body doesn't parse into JSON (empty body, wrong Content-Type, truncated response), readJsonOrThrow returns undefined, and body.message throws TypeError: Cannot read properties of undefined (reading 'message') inside the try block's own success path — this crash is not caught by the surrounding catch's intended "toggle failed" messaging path in a meaningful way (it is caught, but reports a generic JS error instead of the real failure), and it also means setDesktopEnabled(next) on Line 330 has already run, leaving the toggle state locally flipped even though the "success" handling then throws.
🐛 Proposed fix
- const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed"));
+ const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed")) ?? {};
setDesktopEnabled(next);
if (body.message) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const toggleDesktop = async () => { | |
| if (connectionInFlight.current) return; | |
| connectionInFlight.current = true; | |
| setConnectionPending(true); | |
| setMessage(null); | |
| const next = !desktopEnabled; | |
| try { | |
| const response = await fetch(`${apiBase}/api/native-integrations/claude-desktop`, { | |
| method: "PUT", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ enabled: next }), | |
| }); | |
| const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed")); | |
| setDesktopEnabled(next); | |
| if (body.message) { | |
| setMessage({ tone: "ok", text: body.message }); | |
| setAnnouncement(body.message); | |
| } | |
| void desktopResource.refresh(); | |
| void statusResource.refresh(); | |
| const toggleDesktop = async () => { | |
| if (connectionInFlight.current) return; | |
| connectionInFlight.current = true; | |
| setConnectionPending(true); | |
| setMessage(null); | |
| const next = !desktopEnabled; | |
| try { | |
| const response = await fetch(`${apiBase}/api/native-integrations/claude-desktop`, { | |
| method: "PUT", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ enabled: next }), | |
| }); | |
| const body = await readJsonOrThrow<{ ok?: boolean; message?: string; state?: string }>(response, t("claudeDesktop.toggleFailed")) ?? {}; | |
| setDesktopEnabled(next); | |
| if (body.message) { | |
| setMessage({ tone: "ok", text: body.message }); | |
| setAnnouncement(body.message); | |
| } | |
| void desktopResource.refresh(); | |
| void statusResource.refresh(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/pages/ClaudeDesktop.tsx` around lines 317 - 336, Guard the optional
result from readJsonOrThrow in toggleDesktop before accessing body.message,
while preserving the successful setDesktopEnabled flow for valid responses. Only
display the server message and announcement when body exists and contains a
message, so an empty or invalid successful response cannot throw after the local
state is updated.
| const entry = metadata.entries.find(e => e?.name === "opencodex" && typeof e.id === "string"); | ||
| if (!entry || typeof entry.id !== "string") { | ||
| return { cleared: false, path: sentinel, reason: "no opencodex entry" }; | ||
| } | ||
| const configPath = join(libraryPath, `${entry.id}.json`); | ||
| // Remove the config file — best-effort; a missing file is not a failure. | ||
| if (existsSync(configPath)) { | ||
| unlinkSync(configPath); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the metadata entry ID before constructing the deletion path.
parseMetadata accepts arbitrary entry objects. Lines 399-406 only require entry.id to be a string. A crafted value such as "../../target" escapes libraryPath through join(...) and lets unlinkSync remove a reachable target.json file.
Require a canonical UUID before using an entry ID. Apply the validation in a shared metadata helper so writeDesktop3pConfig also cannot read or write through a malicious existing ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/claude/desktop-3p.ts` around lines 399 - 406, Validate the selected
opencodex entry ID with the project’s canonical UUID validation in a shared
metadata helper before constructing any path. Update both the deletion flow
around parseMetadata and writeDesktop3pConfig to reject non-UUID IDs, preventing
join-based path traversal while preserving the existing invalid-entry handling.
| const state = await buildClaudeDesktopState(config); | ||
| const runtimePort = Number(url.port) || config.port; | ||
| return jsonResponse({ ...state, port: runtimePort }); | ||
| return jsonResponse({ ...state, port: runtimePort, enabled: config.claudeCode?.desktopEnabled !== false }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Gate every Claude Desktop configuration writer with desktopEnabled.
The response at Line 665 reports that routing is disabled, but autoApplyDesktopBestEffort at Lines 130-150 and POST /api/claude-desktop/apply at Lines 701-754 still call writeDesktop3pConfig.
After a user disables routing, a catalog update or direct apply request can recreate the Desktop 3P entry. Both paths also pass config.apiKeys?.[0]?.key to the writer.
Return before auto-apply when config.claudeCode?.desktopEnabled === false. Reject apply requests while disabled. Allow only the native toggle enable path to restore the 3P configuration.
As per path instructions, “tokens and OAuth material must never be logged or serialized into responses.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/management/agent-settings-routes.ts` at line 665, Gate every
Claude Desktop configuration writer on config.claudeCode?.desktopEnabled: return
before writeDesktop3pConfig in autoApplyDesktopBestEffort and reject POST
/api/claude-desktop/apply while disabled. Preserve restoration only through the
native toggle-enable path. Ensure tokens and OAuth material, including the API
key passed to the writer, are never logged or serialized in responses.
Source: Path instructions
| if (desktopEnabled(config) === enabled) { | ||
| return jsonResponse({ | ||
| ok: true, clientId: "claude-desktop", changed: false, | ||
| state: enabled ? "current" : "absent", | ||
| message: enabled ? "Claude Desktop routing is already on" : "Claude Desktop routing is already off", | ||
| } satisfies NativeToggleEnvelope); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reconcile failed Desktop file operations before returning an idempotent result.
The toggle persists desktopEnabled before file work. If writeDesktop3pConfig or clearDesktop3pConfig fails, the next identical PUT exits at Line 558 and never retries the incomplete operation. When disable cleanup fails, Lines 121-127 then report absent even if _meta.json still contains the opencodex entry.
src/server/management/native-integration-routes.ts#L558-L564: verify and reconcile the 3P entry before returningchanged: false. Do not claim that a failed write “will be applied on retry” unless the same request performs that retry.src/server/management/native-integration-routes.ts#L121-L127: report the observed on-disk entry state, or add a separate configured-enable field. Do not forceabsentwhen cleanup failed.
📍 Affects 1 file
src/server/management/native-integration-routes.ts#L558-L564(this comment)src/server/management/native-integration-routes.ts#L121-L127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/management/native-integration-routes.ts` around lines 558 - 564,
Update src/server/management/native-integration-routes.ts lines 558-564 in the
idempotent toggle path to verify the on-disk 3P entry and retry
writeDesktop3pConfig or clearDesktop3pConfig before returning changed: false.
Update lines 121-127 to report the observed _meta.json entry state, or expose a
separate configured-enable value, so failed cleanup is not reported as absent.
|
Thanks for taking the time to build this, and for the note that it was meant as inspiration rather than a final shape. This capability already shipped on Where the feature lives todayUsing it: the toggle is on the Integrations overview page, on the Claude Desktop row — not inside the Claude Desktop tab itself. Turning it off pops a confirmation dialog that names the exact config path being changed, then reverts Claude Desktop to standard mode and clears the opencodex entry. Turning it back on regenerates the config from the saved profile and the current visible model list. Server:
GUI:
Tests: Why this branch cannot be merged as-isThe design differs in one way that matters. This PR introduces On the problem that prompted thisYou mentioned you could not remove the third-party gateway from Claude Desktop and had to delete the files by hand. If that happened on |
|
Thank you for taking the time to look into. I'll upgrade to v2.11.0 and test again 🔥 |
2 participants
Adds an on/off toggle to the Claude Desktop tab, mirroring the Claude Code connection toggle. When off, the 3P config files are removed from Claude Desktop's config library; when on, they are written back from the saved profile.
Server:
GUI:
i18n: 4 new keys (toggleFailed, enabledLabel, toggleAria, disabledNotice) in all 6 locales
Summary
I had big issues when trying to things, and couldn't remove the 3rd party / gateway in Claude Desktop. Had to locate the files, and simply delete them. This is to make it more user friendly.
Verification
Take it as inspiration - and reshape it until it fits with what you want backed into the solution
Summary by CodeRabbit