port desktop app to Effect#2546

🔴 Critical src/backendReadiness.ts:60

Effect.timeoutOption(timeout) returns Option.None on timeout as a success, not a failure, so the catchTags({ TimeoutError: ... }) handler never triggers. When the overall timeout expires, the function incorrectly succeeds with Option.None instead of failing with BackendTimeoutError. Consider using Effect.timeout(timeout) instead to correctly raise TimeoutError on expiration.

✅ Resolved in 63f1bdff2eedbc50b04bd6fc7a0f86763d626046

Timeout silently succeeds instead of failing with error

High Severity

The waitForHttpReadyEffect function silently succeeds when the backend readiness check times out. Effect.timeoutOption returns Option.None on timeout, which is implicitly discarded instead of being handled as a failure. This prevents catchTags from catching a TimeoutError, causing the application to proceed as if the backend is ready when it has actually timed out.

^{Reviewed by Cursor Bugbot for commit ac1b4700e55639797aa4d80dc599acc9bea16b6c. Configure here.}

🟠 High src/main.ts:1443

clearStartedBackendState unconditionally sets backendReady = false even when the fiber being finalized is not the current one. If stopBackend() is called and startBackend() is called again before the old fiber's finalizer runs, the old fiber's Effect.ensuring handler will incorrectly clear backendReady after the new backend has already signaled readiness via handleBackendReady(), causing the application to think the backend is not ready when it actually is.

No longer relevant as of 3a55a1d65ed0e6663d7332303ff58f918d8b3bdb

Readiness timeout fires but no window ever created

Medium Severity

When the readiness probe times out (after 1 minute), onReadinessFailure logs a warning but the process keeps running. Meanwhile backendReady stays false, so handleBackendReady never fires and no window is created. The process continues indefinitely without scheduling a restart or creating a window. In the old code, the timeout would log and the ensureInitialBackendWindowOpen promise chain would end—but the process also kept running, so this is comparable. However, the new code's Effect.match onSuccess handler only fires when the process exits, meaning a backend that starts serving but takes longer than the timeout window will leave the user staring at nothing with no recovery path until the process happens to exit.

^{Reviewed by Cursor Bugbot for commit b5c8ea48395f7ed2082891e6efc2ceae78d8f704. Configure here.}

Port fields lose range validation in bootstrap schema

Low Severity

The port and tailscaleServePort fields in DesktopBackendBootstrap use Schema.Number instead of the PortSchema (Schema.Int.check(Schema.isBetween({ minimum: 1, maximum: 65535 }))) that was previously used in the old BootstrapEnvelopeSchema inside cli.ts. This means the server-side bootstrap decoder no longer validates that these values are integers within the valid port range, accepting fractional numbers, zero, negative values, or ports above 65535.

^{Reviewed by Cursor Bugbot for commit b5c8ea48395f7ed2082891e6efc2ceae78d8f704. Configure here.}

🟡 Medium src/desktopBackendManager.ts:166

When stop({ timeout }) times out before the fiber completes, closeRun returns early via Effect.timeoutOption(...).pipe(Effect.ignore), but the old fiber keeps running. Because stop already cleared state.active to None, a subsequent start sees no active run and spawns a new process, leaving the old zombie process running concurrently.

No longer relevant as of 6a07b7adfe4f54ddb9082432ed41d5c46b859940

🔴 Critical

https://github.com/pingdotgg/t3code/blob/add1b6f6cd00e6de9a4c89efd11970e3caeac18c/apps/desktop/src/main.ts#L2195

On line 2199, applyAutoUpdaterChannel(nextChannel) returns an Effect.Effect<void> that is never executed — it is discarded without being yielded or passed to runIpcEffect(). When users change the update channel through UPDATE_SET_CHANNEL_CHANNEL, the autoUpdater.channel, allowPrerelease, and allowDowngrade properties are not actually updated, so the app continues checking for updates on the old channel.

No longer relevant as of 3a55a1d65ed0e6663d7332303ff58f918d8b3bdb

🟢 Low src/backendPort.ts:64

The default value for canListenOnHost uses an unsafe type cast as (port: number, host: string) => Effect.Effect<boolean, Error, R>. When a caller explicitly specifies a type parameter R that differs from NetService but relies on the default, the returned Effect incorrectly claims to require R while actually requiring NetService at runtime. For example, resolveDesktopBackendPortEffect<never>({ host: "localhost" }) returns Effect<number, Error, never> but fails at runtime with a missing NetService dependency. Consider making the parameter required or restructuring to preserve type safety.

No longer relevant as of 6a07b7adfe4f54ddb9082432ed41d5c46b859940

🟠 High src/main.ts:1463

Effect.catchCause is applied to the entire Effect.forever chain, so when checkForUpdates("poll") fails, the error is logged but forever receives a successful void effect and terminates. The polling loop stops after the first failure instead of continuing. Move catchCause inside forever so failures are logged per-iteration without terminating the loop.

No longer relevant as of 6a07b7adfe4f54ddb9082432ed41d5c46b859940

🔴 Critical src/main.ts:1563

autoUpdater.checkForUpdates() can reject per the electron-updater API, but Effect.promise treats rejections as defects rather than recoverable failures. Since Effect.catch only catches failures, the error handler on lines 1565-1572 never executes when the promise rejects, leaving the update state stuck at "checking" instead of transitioning to "error". Use Effect.tryPromise to convert rejections into failures that Effect.catch can handle.

No longer relevant as of 3a55a1d65ed0e6663d7332303ff58f918d8b3bdb

Start leaks restart fiber without interrupting it

Medium Severity

The start function clears the restartFiber reference in the manager's state without interrupting any currently scheduled restart fiber. This orphans the fiber, which can then unexpectedly restart the backend later, potentially undoing an explicit stop() and leading to unintended behavior.

^{Reviewed by Cursor Bugbot for commit add1b6f6cd00e6de9a4c89efd11970e3caeac18c. Configure here.}

Restart backoff resets on spawn, not readiness

Medium Severity

The restartAttempt counter resets to 0 in the onStarted callback (when the process obtains a PID), rather than in the onReady callback (when the health check passes). This defeats the exponential backoff for backends that crash during initialization — they will always restart with the initial 500ms delay, creating a tight restart loop with no backoff.

^{Reviewed by Cursor Bugbot for commit 6d22abb9f53598811eaffbf1e16e0ccb19ad43ab. Configure here.}

🟡 Medium electron/ElectronMenu.ts:140

In popupTemplate, Electron.Menu.buildFromTemplate(...).popup(...) executes immediately when the function is called, not when the returned Effect.void is executed. This causes the popup to appear before the Effect runs, breaking lazy evaluation. Consider wrapping the popup call in Effect.sync and returning that Effect.

✅ Resolved in f57d34aa6225052977ef11f1b739342f839b7bfa

Side effects in popupTemplate escape Effect boundary

Medium Severity

The popupTemplate method executes Electron.Menu.buildFromTemplate and .popup() as bare side effects before returning Effect.void, rather than wrapping them in Effect.sync like setApplicationMenu does on line 137. This means the side effects run eagerly at call time (not when the Effect is executed), and any exception thrown by the Electron APIs will propagate as an unhandled throw instead of being captured by the Effect error channel.

^{Reviewed by Cursor Bugbot for commit 04cb1806d720c41a849017316aeadcc33c75874c. Configure here.}

Restart backoff defeated by premature attempt counter reset

Medium Severity

The onStarted callback resets restartAttempt to 0 immediately after the backend process spawns, before it has proven healthy via onReady. A crash-looping process that spawns successfully but exits immediately will always see restartAttempt === 0 when scheduleRestart runs in finalizeRun, so calculateRestartDelay always returns the initial 500ms delay. The exponential backoff (up to MAX_RESTART_DELAY of 10s) never kicks in. The reset belongs in onReady instead, so only a process that becomes fully ready resets the backoff counter.

^{Reviewed by Cursor Bugbot for commit c9345f4cd9cf84003f11674f7b0c0d7ceed4a44e. Configure here.}

🟢 Low effect-acp/tsconfig.json:17

The include array was narrowed to ["src"], but scripts/ and test/ directories still contain .ts files with no separate tsconfig.json. TypeScript will no longer type-check these files, so type errors in scripts/generate.ts, test/examples/cursor-acp-client.example.ts, and test/fixtures/acp-mock-peer.ts will go undetected during compilation.

✅ Resolved in 3947f31cc22dad121863892c2b0003981a9f6b15

🟠 High backend/DesktopBackendManager.ts:540

When stop() is called, desktopState.backendReady is not set to false, leaving the UI state inconsistent. The finalizeRun callback (line 418) only runs via matchEffect handlers, but when stop() interrupts the fiber by closing runScope, Effect's interruption semantics skip matchEffect and run only the ensuring finalizer—so backendReady never resets. Consider explicitly setting desktopState.backendReady to false in the stop function.

✅ Resolved in f57d34aa6225052977ef11f1b739342f839b7bfa

🟠 High updates/DesktopUpdates.ts:638

setChannel calls updatePersistedSettings before checking if the channel actually changed. When nextChannel === state.channel, this still writes to disk and incorrectly sets updateChannelConfiguredByUser: true even though the user made no change. Consider moving the persistence after the early-return check at line 643.

✅ Resolved in f57d34aa6225052977ef11f1b739342f839b7bfa

🟠 High electron/ElectronMenu.ts:140

In popupTemplate, when input.template.length === 0, the code returns Effect.void from inside Effect.sync. Effect.sync expects its callback to return a plain value that becomes the effect's success value; returning Effect.void makes that object itself the success value rather than executing it. Consider using return (or restructuring with Effect.when or Effect.flatMap) to properly handle the empty case.

✅ Resolved in 01e167316e57ae03c07cd6a5912e4df6223c4fc3

Returning Effect value from sync thunk instead of void

Low Severity

The popupTemplate implementation uses return Effect.void inside Effect.sync(), which returns an unexecuted Effect object as the success value of the sync thunk rather than performing a no-op. The developer likely intended a plain return (or return undefined) to exit early when the template is empty. While the behavior is correct (nothing happens for empty templates), this pattern is misleading — it looks like it's trying to "run" Effect.void but actually just produces a dead value that's discarded by the caller.

^{Reviewed by Cursor Bugbot for commit f57d34aa6225052977ef11f1b739342f839b7bfa. Configure here.}

Gitignore patterns now overly broad across monorepo

Low Severity

Moving dist-electron/ and .electron-runtime/ from apps/desktop/.gitignore to the root .gitignore broadens matching scope from just apps/desktop/ to any directory at any depth in the monorepo. Gitignore patterns without a leading path separator match anywhere in the tree. Since this is a monorepo with multiple apps/packages, any future package that happens to use a dist-electron/ directory would have it silently ignored.

^{Reviewed by Cursor Bugbot for commit 01e167316e57ae03c07cd6a5912e4df6223c4fc3. Configure here.}

🟡 Medium settings/DesktopAppSettings.ts:145

When updateChannelConfiguredByUser is true but settings.updateChannel equals defaults.updateChannel, the updateChannel field is omitted from the persisted document (line 145-147). On reload, normalizeDesktopSettingsDocument sees no saved channel and applies the current default (lines 123-125). If the app's default channel changed between versions, the user's explicit choice is lost and silently replaced with the new default.

Consider always persisting updateChannel when updateChannelConfiguredByUser is true, regardless of whether it matches the current default.

No longer relevant as of ccee7cecd56505518b51df28e9d41f51942c4810

🟢 Low updates/DesktopUpdates.ts:408

The polling loop at startUpdatePollers breaks permanently when a defect (runtime exception) occurs during checkForUpdates. The Effect.catchCause is placed outside Effect.forever, so when an uncaught defect propagates from the inner effect, catchCause logs it and returns, terminating the fiber. Since checkForUpdates only uses Effect.catch for typed errors, any unexpected throw from electronUpdater.checkForUpdates or other operations stops update polling until app restart. Consider moving catchCause inside forever so each iteration is isolated, or wrapping checkForUpdates with Effect.catchAllDefect to ensure defects are caught and logged without breaking the loop.

Misleading log emitted when backend start is skipped

Low Severity

The log message "bootstrap backend start requested" at line 182 fires unconditionally, even when the backend start was actually skipped because state.quitting is true. This produces misleading diagnostic output during shutdown races, making it harder to determine from logs whether the backend was actually started.

^{Reviewed by Cursor Bugbot for commit daf956bf6dc59d8a37d7cafb8c2405f0aad5ca91. Configure here.}

desktopState.backendReady is never set to true

High Severity

The onReady callback updates the internal manager state's ready field but never sets desktopState.backendReady to true. The ref is only ever set to false (on start, finalize, and stop), so any consumer reading desktopState.backendReady will always see false even after the backend is confirmed ready via HTTP. The test works around this by manually setting the ref to true externally.

^{Reviewed by Cursor Bugbot for commit 413d2f9dedf6ecf724e494543cf43515c5af2257. Configure here.}

🟢 Low backend/DesktopBackendManager.ts:507

When stop() is called after a restart is scheduled but before it fires, the restart fiber at line 507 still proceeds to call start() because it only checked desiredRunning at scheduling time (line 482). The fiber updates restartFiber to Option.none() at line 509 and then unconditionally calls start() at line 514, even though desiredRunning may now be false. Consider re-checking desiredRunning atomically when clearing restartFiber before invoking start.

✅ Resolved in 79b34209267f9b18abaf1f6086b3b6e6be05bcef

🟢 Low electron/ElectronWindow.ts:121

In syncAllAppearance, the code iterates over all windows from Electron.BrowserWindow.getAllWindows() and directly calls sync(window) without checking window.isDestroyed() first. If a window is destroyed between the getAllWindows() call and the sync execution, the callback will receive a destroyed window, potentially causing errors. Other methods in this service (sendAll, reveal, liveMain, etc.) consistently filter out destroyed windows with isDestroyed() checks.

✅ Resolved in dc3aa278f8e8c9c2f627b14f72fd7ac585abc010