Add client-only desktop backend mode#4444
Conversation
|
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a significant new feature (client-only desktop mode) with substantial architectural changes to desktop bootstrap, IPC, and environment handling. Additionally, there are 3 unresolved medium-severity findings regarding error handling and resource cleanup that should be addressed. You can customize Macroscope's approvability policy. Learn more. |
a0f2a4f to
9d6b3f5
Compare
| yield* revoke(yield* Ref.get(currentAccessInfo)); | ||
| }); | ||
|
|
||
| const initialPublish = yield* Effect.exit(publish(initialAccessInfo)); |
There was a problem hiding this comment.
🟡 Medium src/localServerAdvertisement.ts:177
If the startup fiber is interrupted after publish(initialAccessInfo) succeeds at line 177 but before forkScoped(rotate…) runs at line 224, no cleanup is registered: the advertisement file stays on disk and the newly issued pairing credential remains valid until it expires. The cleanup finalizer is only attached to the rotation fiber via Effect.ensuring inside forkScoped, so any interruption in between leaks both resources. Consider wrapping the initial publish and the rotation fork in a single Effect.acquireRelease (or otherwise registering cleanup before the first write) so the finalizer is in place before the file is created.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/localServerAdvertisement.ts around line 177:
If the startup fiber is interrupted after `publish(initialAccessInfo)` succeeds at line 177 but before `forkScoped(rotate…)` runs at line 224, no cleanup is registered: the advertisement file stays on disk and the newly issued pairing credential remains valid until it expires. The `cleanup` finalizer is only attached to the rotation fiber via `Effect.ensuring` inside `forkScoped`, so any interruption in between leaks both resources. Consider wrapping the initial publish and the rotation fork in a single `Effect.acquireRelease` (or otherwise registering cleanup before the first write) so the finalizer is in place before the file is created.
32f076d to
048f271
Compare
colonelpanic8
changed the title
github-actions
Bot
added
size:XL
bb2c567 to
704649e
Compare
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>
704649e to
20ba333
Compare
| yield* desktopWindow.handleRendererReady.pipe( | ||
| Effect.catch((error) => | ||
| logBootstrapWarning("failed to open main window after renderer readiness", { | ||
| error: error.message, | ||
| }), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 Medium app/DesktopApp.ts:202
In the client-only branch of bootstrap, any DesktopWindowError from desktopWindow.handleRendererReady is caught and logged, then the function returns successfully. If initial window creation or renderer loading fails, startup proceeds to shutdown.awaitRequest and waits indefinitely — there is no window from which a user can trigger a shutdown or retry, leaving a headless desktop process. The non-client-only path surfaces such failures through fatalStartupCause, but this path silently swallows them. Consider propagating the error to fatalStartupCause (or explicitly requesting shutdown) instead of only logging it.
- if (!(yield* Ref.get(state.quitting))) {
- yield* desktopWindow.handleRendererReady.pipe(
- Effect.catch((error) =>
- logBootstrapWarning("failed to open main window after renderer readiness", {
- error: error.message,
- }),
- ),
- );
- }
+ if (!(yield* Ref.get(state.quitting))) {
+ yield* desktopWindow.handleRendererReady.pipe(
+ Effect.catch((error) =>
+ logBootstrapWarning("failed to open main window after renderer readiness", {
+ error: error.message,
+ }).pipe(Effect.andThen(Effect.fail(error))),
+ ),
+ );
+ }🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/app/DesktopApp.ts around lines 202-209:
In the `client-only` branch of `bootstrap`, any `DesktopWindowError` from `desktopWindow.handleRendererReady` is caught and logged, then the function returns successfully. If initial window creation or renderer loading fails, `startup` proceeds to `shutdown.awaitRequest` and waits indefinitely — there is no window from which a user can trigger a shutdown or retry, leaving a headless desktop process. The non-client-only path surfaces such failures through `fatalStartupCause`, but this path silently swallows them. Consider propagating the error to `fatalStartupCause` (or explicitly requesting shutdown) instead of only logging it.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 20ba333. Configure here.
| yield* logStartupInfo("runtime logging configured", { logDir: environment.logDir }); | ||
| yield* desktopSettings.load; | ||
| const settings = yield* desktopSettings.load; | ||
| const launchMode = yield* backendMode.latch(settings.backendMode); |
There was a problem hiding this comment.
Invalid backend-mode flag silent failure
Medium Severity
When launch arguments include an invalid or repeated --backend-mode, backendMode.latch fails during startup before the existing fatal-startup handlers run. The desktop process exits without the standard startup error dialog, so a bad shortcut or script flag can fail opaquely instead of explaining the flag problem.
Reviewed by Cursor Bugbot for commit 20ba333. Configure here.
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>
1 participant
What Changed
Let the desktop app run as a pure client for remote or independently managed backends, instead of always owning a local backend process.
managedandclient-onlyoptions, plus a launch-only--backend-modeoverride, exposed over IPC.client-only, bootstrap skips the backend pool entirely and serves the renderer from packaged static assets (or the dev-server proxy), opens the window on renderer readiness rather than backend readiness, and skips backend shutdown on quit.ElectronProtocolnow takes an explicitsource: "proxy" | "static"(dropping the separatebackendOrigin). Static serving adds path-safe file resolution with SPA fallback, MIME types, CSP, and HEAD support.Why
The desktop app should be usable as a pure client without stranding users who have no saved connections.
Scope note
This PR previously also contained Linux local
t3 servediscovery. That half has been split into a separate PR so the two can be reviewed independently — the discovery work carries a distinct security surface (a live pairing credential on disk) and is Linux-only, whereas this change is cross-platform.UI Changes
Packaged desktop evidence
Final privacy-reviewed evidence for this PR is pinned to immutable asset commit
d23321ea7d7200ef4d5e14f20dd2367997731706.Capture context:
bb2c567e6879cf762910a02a8d20943edc2689ceece05087a70e94efcd57441337fa1249559362ba5ba3b7bd0995f401993706689105f41f51c16b02/nix/store/86n9kxipsprgw6nbzyw7qhrw0wl4lfwx-t3code-0.0.29-patched-main-20260724The packaged Electron app started with
--backend-mode=client-onlywithoutstarting or owning a local backend, resolved its installed renderer assets,
and paired successfully to an independently started packaged
t3 servebackend. The saved environment then appeared online.
Client-only startup with no saved environment
Connections settings and active CLI override
Independent backend paired and online
Connected home state ·
capture manifest
After this GUI capture, the maintained stack advanced its upstream base to
41a430a88e8dde9c428f59d54dd328aa6a66a8fd. The intervening upstream deltawas isolated to #4472 model registration and did not overlap this desktop
change. The resulting final installed package
/nix/store/brp90sqhjxnnpczsnw9nmp1rlzi0qjgk-t3code-0.0.29-patched-main-20260724built and activated successfully. The screenshots above remain specifically
attributable to the exact earlier package and synthetic integration listed
above; they are not represented as a GUI recapture of the later package.
Checklist
corepack pnpm exec vp check(0 errors; 11 pre-existing unrelated warnings)corepack pnpm exec vp run --filter @t3tools/contracts --filter @t3tools/shared --filter t3 --filter @t3tools/desktop --filter @t3tools/web typecheck— cleanCI=true vitest run apps/desktop/src apps/web/src/components/settings apps/web/src/environments apps/server/src/startupAccess.test.ts— 63 files, 437 testsNote
Add client-only desktop backend mode that serves static renderer assets without starting a backend
client-onlybackend mode alongside the existingmanagedmode, controlled via a newDesktopBackendModeservice in DesktopBackendMode.ts. The mode can be set via CLI flag (--backend-mode) or persisted in desktop settings.registerDesktopProtocolnow uses a discriminated union input (source: "proxy"orsource: "static"); the static handler includes path traversal protection, SPA fallback, MIME types, CSP, and proper HEAD/405 responses.getBackendModeStateandsetBackendModeIPC endpoints (exposed viadesktopBridge) so the renderer can read and change the backend mode; switching modes triggers a relaunch and reverts the setting if relaunch fails.isDesktopClientOnlyMode()is true, and a newisRemoteEnvironmentIdutility centralizes remote-environment detection.registerDesktopProtocolcallers must now supply asourcediscriminator;backendOriginis no longer accepted.Macroscope summarized 4e25680.
Note
High Risk
Touches Electron custom-protocol static serving (path traversal defenses), startup/shutdown paths, and auth/routing when no local primary exists; mode changes require relaunch.
Overview
Introduces client-only desktop mode alongside the existing managed mode: the app can run without starting or owning a local backend, using paired/saved environments instead.
Desktop startup & protocol: Mode is persisted in settings, overridable via
--backend-mode, and latched at startup. In client-only, bootstrap skips the backend pool, registers IPC, and opens the window on renderer readiness (not backend readiness); shutdown skips stopping backend instances. Production uses packaged client roots (packagedClientRootCandidates); dev still proxies the Vite server.ElectronProtocolnow takessource: "proxy" | "static"(replacing separatebackendOrigin); static mode adds path-safe resolution, SPA fallback, MIME types, CSP, and GET/HEAD handling.IPC & lifecycle:
getBackendModeState/setBackendModeon the bridge; changing mode persists and relaunches (settings roll back if relaunch fails). Preload fails closed to client-only if mode state is missing. Local environment bootstraps return empty in client-only.Web UI: Client-only is routed like hosted static (no implicit primary from
t3code://).EnvironmentPresenceScope/ownsLocalEnvironmentfix sidebar and thread “remote” labeling when the app has no local backend. Connections adds a Backend mode control (blocks client-only without a saved non-local environment).Reviewed by Cursor Bugbot for commit 4e25680. Bugbot is set up for automated code reviews on this repo. Configure here.