Target server settings by environment#4559
Conversation
Let the desktop app run as a pure client for remote or independently managed backends, instead of always owning a local backend process. - Add a persisted `managed` / `client-only` backend mode with a launch-only `--backend-mode` override, exposed over IPC. - In client-only mode, bootstrap skips the backend pool entirely and serves the renderer from packaged static assets (or the dev proxy), opening the window on renderer readiness rather than backend readiness, and skipping backend shutdown on quit. - `ElectronProtocol` takes an explicit `source: "proxy" | "static"`. Static serving adds path-safe file resolution, SPA fallback, MIME types, CSP and HEAD support. - Web routing treats client-only like hosted static, and local environment bootstraps are empty, so a client-only desktop is never stranded on a dead primary-backend state. Co-Authored-By: Claude <noreply@anthropic.com>
The sidebar decided a thread or project was remote by comparing its environment against the primary environment id. A client-only desktop (and the hosted static web app) never registers a primary, so that id is always null and every row read as local: no cloud/server icon, and no environment name on the project header. Make the comparison explicit about the two different meanings of a null primary id. `isRemoteEnvironmentId` now takes an `ownsLocalEnvironment` flag: when the app can never serve an environment from its own backend, every environment is remote; when it can, a null primary id still means "the local backend has not registered yet" and nothing is flagged remote. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The diagnostics environment picker made three pieces of panel-local state lie about which machine they belong to: - `openLogsDirectoryError` / `isOpeningLogsDirectory` kept showing an environment A failure (or spinner) after switching to environment B. They are now stored together with the environment they belong to, so they only render for that environment and a late completion can only update its own request. - `signalingPid` was a single shared value, so completing a signal on environment A re-enabled the signal controls for an in-flight signal on environment B and allowed a duplicate SIGKILL. Pending signals are now identified by environment id plus pid. - Diagnostics queries only run while the selected environment's supervisor is connected, so a disconnected machine showed permanent "Loading..." placeholders with the refresh buttons disabled. The panel now derives a connection notice and renders it instead of the loading state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the review feedback on the previous commit: - `DiagnosticsRefreshButton` used a single `isPending` prop for both the spinner and `disabled`, so suppressing the spinner while the selected environment is disconnected also made the button clickable even though the query cannot run. It now takes an explicit `isDisabled` prop. - The open-logs completion matched on environment id alone, so returning to an environment and starting a second open while the first was still in flight let the older completion clear the newer pending state. The completion now matches the request by identity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🟡 Medium
Server-backed SettingResetButton controls remain clickable when the selected environment is disconnected. The adjacent switches are disabled via canConfigureServer, but the reset buttons are not — so clicking Reset calls updateSettings and fires an RPC against an environment whose connection phase is not "connected". This affects Assistant output, provider update checks, new-thread/worktree settings, add-project base directory, and the text-generation model rows. Each reset button should be disabled alongside its paired control when !canConfigureServer.
Also found in 1 other location(s)
apps/web/src/components/settings/DiagnosticsSettings.tsx:1125
ProcessDiagnosticsTableis still rendered with active signal buttons whenisConnectedis false. If cachedprocessDataexists and the selected environment disconnects, the refresh and logs-folder controls are disabled but the INT/KILL controls remain clickable andsignalProcesshas no connection guard, so users can attempt destructive process signals against an offline/reconnecting environment (and get a failed or stuck command) instead of the server-backed control being disabled.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/SettingsPanels.tsx around line 1:
Server-backed `SettingResetButton` controls remain clickable when the selected environment is disconnected. The adjacent switches are disabled via `canConfigureServer`, but the reset buttons are not — so clicking Reset calls `updateSettings` and fires an RPC against an environment whose connection phase is not `"connected"`. This affects Assistant output, provider update checks, new-thread/worktree settings, add-project base directory, and the text-generation model rows. Each reset button should be disabled alongside its paired control when `!canConfigureServer`.
Also found in 1 other location(s):
- apps/web/src/components/settings/DiagnosticsSettings.tsx:1125 -- `ProcessDiagnosticsTable` is still rendered with active signal buttons when `isConnected` is false. If cached `processData` exists and the selected environment disconnects, the refresh and logs-folder controls are disabled but the INT/KILL controls remain clickable and `signalProcess` has no connection guard, so users can attempt destructive process signals against an offline/reconnecting environment (and get a failed or stuck command) instead of the server-backed control being disabled.
There was a problem hiding this comment.
🟡 Medium
When the selected environment changes, isAddingBinding and per-row draft state (key, when clause) are not reset. Because rows are keyed by row.id, which encodes command/key/when rather than environmentId, React reuses the same component instances across environments — an unsaved draft from environment A stays visible and editable after switching to environment B. Pressing Save then sends that stale draft to upsertKeybinding with the new environmentId, modifying environment B instead of A. Consider clearing isAddingBinding and row drafts when environmentId changes, or keying the editing subtree by environmentId so state is discarded on switch.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/KeybindingsSettings.tsx around line 1112:
When the selected environment changes, `isAddingBinding` and per-row draft state (key, when clause) are not reset. Because rows are keyed by `row.id`, which encodes `command`/`key`/`when` rather than `environmentId`, React reuses the same component instances across environments — an unsaved draft from environment A stays visible and editable after switching to environment B. Pressing **Save** then sends that stale draft to `upsertKeybinding` with the new `environmentId`, modifying environment B instead of A. Consider clearing `isAddingBinding` and row drafts when `environmentId` changes, or keying the editing subtree by `environmentId` so state is discarded on switch.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bf1d0ee. Configure here.
|
|
||
| export function GeneralSettingsPanel() { | ||
| const { theme, setTheme } = useTheme(); | ||
| const settings = usePrimarySettings(); |
There was a problem hiding this comment.
Server resets while disconnected
Medium Severity
The "Restore defaults" and individual reset actions in the General settings panel can attempt to update server-backed preferences even when the selected environment is not connected. This can lead to silent failures or partial updates, as only client-local settings may change, while the UI implies a full reset.
Reviewed by Cursor Bugbot for commit bf1d0ee. Configure here.
| yield* lifecycle | ||
| .relaunch(`backendMode=${mode}`) | ||
| .pipe(Effect.tapError(() => settings.setBackendMode(previousMode))); | ||
| } |
There was a problem hiding this comment.
Mode change survives failed relaunch
Medium Severity
Changing the backend mode persists the new setting before a guaranteed relaunch. The relaunch function's shutdown and exit logic runs in a detached fiber, so its failures don't trigger a settings rollback. This can leave the app running with the old mode while the new mode is persisted.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bf1d0ee. Configure here.
| /> | ||
| <StatBlock | ||
| label="Parse Errors" | ||
| value={data ? formatCount(data.parseErrorCount) : "..."} |
There was a problem hiding this comment.
Diagnostics show stale offline data
Medium Severity
After the selected environment disconnects, Diagnostics still renders cached processData, resource, and trace query results in stat blocks and tables whenever those atoms hold prior values. Connection messaging and disabled refresh apply only to empty states, so users can see live process counts, PIDs, and history that no longer reflect the offline backend.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bf1d0ee. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Introduces a new desktop backend mode feature and environment-targeted settings with significant runtime behavior changes. Five unresolved medium-severity review comments identify bugs including controls active when disconnected, draft state persisting across environment switches, and mode changes surviving failed relaunches. You can customize Macroscope's approvability policy. Learn more. |
# Conflicts: # apps/desktop/src/ipc/methods/backendMode.test.ts # apps/desktop/src/ipc/methods/backendMode.ts # apps/web/src/components/CommandPalette.tsx # apps/web/src/components/Sidebar.tsx # apps/web/src/components/SidebarV2.tsx # apps/web/src/components/settings/DiagnosticsSettings.tsx # apps/web/src/hooks/useSettings.ts # apps/web/src/routes/_chat.tsx # apps/web/src/state/environments.ts


What Changed
Why
Client-only desktop mode deliberately has no
PrimaryConnectionTarget, but several settings surfaces still read and wrote exclusively through primary-server atoms and hooks. Those controls therefore displayed defaults and silently failed to persist.Settings now resolve an explicit selection first, then the managed primary environment, the active saved environment, and finally the first available environment. This preserves existing managed-client behavior while making client-only and multi-environment operation explicit.
UI Changes
This PR is stacked on
desktop-client-only-discovery, the branch behind #4444. It also carries the environment-scoped provider and diagnostics work needed by the complete settings flow.Checklist
vp test run apps/web/src/components/settings/SettingsPanels.logic.test.ts apps/web/src/components/settings/DiagnosticsSettings.logic.test.ts apps/web/src/hooks/useSettings.test.ts— 31 tests passedvp lint --report-unused-disable-directivesfor all changed filesvp run --filter @t3tools/web buildvp run --filter @t3tools/web typecheckremains blocked by pre-existing unrelated errors inBranchToolbarBranchSelector,ModelPickerContent,PreviewAutomationHosts, and the atom command/query helpersNote
High Risk
Touches Electron startup, custom protocol static file serving (path traversal defenses), and broad settings/routing paths that affect auth, diagnostics RPCs, and multi-environment desktop behavior.
Overview
Introduces desktop backend modes (
managedvsclient-only): persisted preference,--backend-modeCLI override, IPC get/set, and startup that either runs the full backend pool or registers IPC + opens the window from dev proxy or packaged static assets via a refactoredElectronProtocol(proxyvsstatic, path hardening, SPA fallback). Client-only mode skips local bootstraps, avoids stopping backends on shutdown, and the preload fails closed to client-only when mode IPC is missing.On the web client, server-backed settings no longer assume a primary local backend. Keybindings, Diagnostics, and related surfaces use
useSettingsEnvironment/SettingsEnvironmentSelector, read config from the chosen connected environment, and show connection notices instead of spinning when offline. Chat, sidebar, and command palette dropprimaryServer*atoms in favor of thread/project environment config anduseDefaultServerConfig/ environment presence for remote vs local labeling. Connections adds a desktop Backend mode control (relaunch on change). Provider instance dialogs and worktree defaults target the selected environment’s settings.Reviewed by Cursor Bugbot for commit bf1d0ee. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Scope settings panels and server config reads to the selected environment
client-onlydesktop backend mode: when active, the desktop app skips backend startup, serves the renderer as a static protocol or dev proxy, and exposes no local environment bootstraps.isRemoteEnvironmentIdandEnvironmentPresenceScopeso sidebar rows and thread indicators determine remote status based on whether the app owns a local backend, rather than comparing against the primary environment ID.useDefaultServerConfiganduseDefaultEnvironmentIdhooks so keybindings, providers, and server version reads across the UI fall back gracefully when no primary environment is connected.readOptionalPrimaryEnvironmentTargetreturnsnulland the platform connection source emits no registrations, so the app behaves like a hosted-static deployment.📊 Macroscope summarized bf1d0ee. 46 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.