feat(server): background services update themselves#17586

🟡 Medium updates/ServerAutoUpdater.ts:101

When a downloaded report arrives while automaticUpdates is false, updateDesktopApp discards that version, so enabling automatic updates later does not install it unless another updater report arrives. The desktop worker only processes receiver.desktopUpdates and does not retry on settings changes; reevaluate the latest downloaded state when the setting changes or retain the version for retry.

No longer relevant as of 0df12e8170ac317b49232aa8c5dfaf2553c07f3e

Automatic desktop relaunch has been removed. A backend cannot prove every desktop window and WSL backend is idle. Desktop downloads now remain ready and install when the user quits, so this downloaded-report loss path no longer exists.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

A long runWhenOpen wait blocks discovery of newer releases.

Effect.repeat(Schedule.spaced(...)) starts the next release check only after updateBootService completes. runWhenOpen keeps looping until the window opens. Suppose the update window stays closed for days, for example because of busy threads or scheduled tasks. The worker then stays bound to the version it staged first and never checks the release index again. A newer release that arrives during the wait goes uninstalled until this target installs. That target may already be superseded.

The desktop path handles this case with Stream.switchMap. To match it, use runIfOpen with a bounded number of retries, or put a timeout on the wait equal to releaseCheckInterval. The next pass then re-resolves the newest version.

Release discovery and idle-window retries are paced separately. Each idle retry is runIfOpen; the hourly release check can replace a pending target while the environment stays busy.

Confirmed in the live service lab: 0.0.52 staged while a 100-second terminal command kept the window closed and installed after it exited.

@maria-rcks Thanks for clarifying. I verified the current apps/server/src/updates/ServerAutoUpdater.ts.

The worker uses runIfOpen for idle-window retries. nextReleaseCheckAt controls release discovery separately, and a new release can replace pendingTarget while the window stays closed.

My finding does not apply to the current implementation. I withdraw it.

✅ Review thread resolved.

You are interacting with an AI system.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Suppress retries after a failed staging attempt.

When selfUpdate.stage(targetVersion) fails, the outer handler logs the error and returns success. Effect.repeat then runs the same target again at the next release-check interval. The failure occurs before launcher.requestUpdate, so launcher.lastOutcome does not record it. This violates the guide's contract that failed T3 Code updates wait for a newer version.

✅ Confirmed as addressed by @maria-rcks

Failed staging marks that target skipped in the current process. Launcher rollback/failure outcomes remain skipped across restarts. Newer versions are discovered independently of the idle-window wait.

The corrupted 0.0.51 archive had one staging attempt, followed by discovery and a successful commit of 0.0.52.

@maria-rcks Verified the staging failure path in the current code. At Line 79, Effect.tapError adds targetVersion to skippedTargets before the outer handler logs the failure. Later release checks exclude that target at Line 71.

The current code also seeds skippedTargets from non-committed launcher outcomes and checks for newer releases before checking the update window. This addresses the finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

write now waits behind the provider-maintenance window on every keystroke.

UpdateWindow.runIfOpen holds withRestartPermit for the whole check(options) call. That call includes hasBusyTerminals, which spawns ps. Each write call takes the same single permit, so terminal input stalls while a check runs. A recheck runs every 30s. For a busy terminal, this adds up to about 1s of input latency for each check. When the window opens, install holds the permit until shutdown (Effect.never). The handoff path intends that hold. The idle-check cost is not intended, and the PR description states that the permit should not delay terminal use. Run the first check without the permit. Then take the permit and recheck only hasBusyTerminals before calling install. This keeps input unblocked in the common case where a terminal is busy.

✅ Confirmed as addressed by @maria-rcks

Confirmed. The full update-window check now holds provider admission without holding terminal admission. Only an open server restart takes the terminal permit, rechecks busy terminals, and keeps that permit through installation and shutdown. Blacksmith ran the actual UpdateWindow layer with a held initial scan: input completed before the scan was released, a busy final scan skipped installation, and input stayed blocked during installation. The server typecheck, lint/formatting, and 122 terminal, self-update and provider-runner tests passed on 1489f98b.

@maria-rcks Thanks for confirming the fix. The updated flow keeps terminal input available during the initial check and holds terminal admission through installation and shutdown. The reported checks cover input responsiveness, the busy-terminal recheck, and input blocking during installation.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the desktop background-download claim.

The current base disables automatic download and install-on-quit. The polling flow checks for updates, but only the explicit download action calls downloadAvailableUpdate. Defer this text until automatic desktop downloads ship.

✅ Confirmed as addressed by @maria-rcks

The desktop paragraph now belongs to #17588, alongside its background-download and install-on-quit implementation. It has been removed from this server layer and preserved in the complete stack.

@maria-rcks Removing the desktop paragraph from this server layer addresses the finding. Keeping it in #17588 places the documentation with its implementation.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document the queued-work handoff blocker.

Queued background work also keeps the T3 update window closed. Add it to the documented conditions so users know why a staged update may not restart the server.

Source: Coding guidelines

✅ Confirmed as addressed by @maria-rcks

The guide now includes pending background work in the shared update-window conditions. Integrated terminal commands remain documented separately as a server restart blocker, matching UpdateWindow: they do not block provider-only installs. This is a documentation-only correction; production source is unchanged from the verified head.

@maria-rcks Thanks for the correction and clarification. The guide now includes pending background work. Keeping integrated terminal commands separate is correct: they block server restarts, not provider-only installs. My suggestion grouped those conditions too broadly.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Limit the terminal-command condition to server restarts.

This sentence follows a list of provider and T3 Code updates, so it implies terminal commands block both. They defer server restarts, but do not block provider-only updates.

The supplied update-window contract limits this blocker to server restarts.

✅ Addressed in commit f3963cc

The sentence now says "T3 Code server restarts also wait for integrated terminal commands to finish." It names the restart operation explicitly, matching the restartsServer guard and preserving provider-only update behavior. This documentation-only correction is carried through the upper stack layers; production source is unchanged.

@maria-rcks The updated sentence in docs/user/updating.md limits the wait condition to T3 Code server restarts. It no longer implies that integrated terminal commands block provider-only updates. This addresses the finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High updates/UpdateWindow.ts:80

An automatic restart can kill a terminal command or input admitted after hasBusyTerminals reports idle. That check is only a snapshot: blockers still performs asynchronous reads before runIfOpen hands off to the installer, and the provider admission permit does not serialize TerminalManager.write or terminal startup. Hold terminal admission through handoff and perform a final fresh idle check.

Leaving this as is on purpose. An earlier revision did exactly this: one terminal admission permit around open, write and restart, held through the handoff with a final fresh check. It made every keystroke on the server wait behind terminal opens, including opens blocked on a worktree-deletion lease, and it still left a gap (a write that lands just before the permit, before the shell forks, still reads as idle).

The remaining window is the time between the terminal check and the launcher handoff. It only opens after 15 minutes with no client interaction, no live runs, and no terminal command running in the last poll, and a failed poll now counts as busy (66920cb49a). We think that residual risk is better than serializing all terminal I/O.

🟠 High terminal/Manager.ts:3186

When pollSubprocessActivity fails, hasBusyTerminals reports false for a running terminal with cached hasRunningSubprocess === false, so UpdateWindow.runIfOpen(..., { restartsServer: true }) can restart the server and kill an active command. Track failed inspections as busy, or require a successful fresh inspection before reporting the terminal as idle.

Fixed in 66920cb49a. A failed process snapshot or per-terminal inspection now marks the last poll as unreliable, and hasBusyTerminals treats every running terminal as busy until a later poll succeeds.

🟠 High process/ownedProcesses.ts:362

If write(file, next) fails after the SIGSTOP loop, reap rejects without killing or resuming the stopped processes; ServiceLauncher.#reapProviders then swallows that rejection and starts the next runtime while the old providers remain stopped and hold resources or locks. Add error cleanup that terminates or resumes every identity in stopped before propagating the failure.

No longer relevant as of 1e6bfc9dbc5ffd4b4185a9c1731a7a6bb2c13440

Fixed in b367a977ae: if anything fails after the freeze (the record write, or a tree that never reaches a fixed point), every frozen process is resumed before the error propagates, so the next cleanup can retry. Processes found to be protected after freezing are resumed too. Covered by "resumes a provider tree it froze when cleanup cannot finish", which fails without the fix.

🟠 High src/serviceLauncher.ts:606

With a symlinked T3CODE_HOME, pruning fails to identify the launcher runtime and can delete the executable still referenced by the boot service; after enough updates, the next restart or reboot then fails. process.execPath is resolved with realpath, but versionsDir is not, so resolve versionsDir before comparing the paths.

Fixed in 40c86dabae (60c6653397): the runtime versions directory is now resolved with realpath too before it is compared with the launcher executable. If it cannot be resolved, pruning is skipped.

🟠 High updates/ServerAutoUpdater.ts:107

A release withdrawn or superseded while the update window is closed is still installed, so the server can activate a stale version. Because staged !== undefined prevents stageNewest from checking the feed and handOff passes that cached version directly to selfUpdate.update, refresh the candidate on schedule and validate it again before handoff.

Fixed in 40c86dabae (7b9422a822): the hourly feed check keeps running while a release waits for the window. A newer release replaces the staged one, a staged release that is no longer newest-and-newer is dropped, and the same version is not downloaded again. A test stages 1.1.0, publishes 1.2.0, and checks the handoff asks for 1.2.0.

🟠 High src/serviceLauncher.ts:412

Pruning deletes the runtime still named by the installed service unit, so the next service start or reboot cannot launch and the CLI symlink to that runtime dangles. When cli/update.ts declines a restart, BootService.install({ start: false }) can update the unit to version B while launcher A keeps running; after later updates make B neither active nor previous, launcherVersion identifies only A and this loop removes B. Preserve the installed unit's target before deleting completed runtimes.

No longer relevant as of 4e75b612eaf9dcb60520108672110a056dad7bad

Fixed in 93eb189966: pruning keeps the runtime a deferred t3 update repointed the unit and CLI to (.restart-pending), alongside the active, previous, pending and running-launcher versions. A follow-up on this branch also keeps runtimes published within the last hour and the runtime the t3 CLI currently points to.

🟡 Medium src/serviceLauncher.ts:836

After SIGTERM sets #stopRequested, #handleExit() still schedules this backoff timeout because it checks only #stopping; the queued stop then resolves completion without clearing the timer, keeping Node alive for up to 30 seconds. Skip scheduling when #stopRequested is set.

No longer relevant as of 4e75b612eaf9dcb60520108672110a056dad7bad

Fixed in 93eb189966: #handleExit no longer schedules the crash-backoff restart once a stop has been requested, so a child exit handled ahead of the queued stop cannot start a new child after the launcher completes.

🟠 High src/serviceLauncher.ts:419

A concurrent CLI install can leave the service state and unit pointing to a runtime that this loop deletes, so the service cannot start. bootService.install publishes the runtime before writing the restart-pending marker or selecting it; if pruning runs in that interval, the target is absent from keep and is removed before the installer selects it. Coordinate pruning with installation or reserve the target before publishing it.

Fixed in 64c08d28ef: pruning skips any runtime directory modified within the last hour, so a runtime t3 update just published cannot be swept before it records the restart that selects it. 858c5fcae2 also keeps whatever runtime the t3 CLI currently points to: t3 update records the CLI launcher path before swapping, and pruning resolves that link.

🟠 High src/serviceLauncher.ts:422

A chained CLI symlink can cause pruning to delete a runtime that t3 still needs, leaving the interactive command dangling. readlink returns only the immediate target, so cliVersion becomes .. instead of the runtime version; canonicalize both the launcher target and versionsDir before deriving cliVersion.

fixed in ecf64bb505. pruning canonicalizes the cli entry and versions directory before deriving the retained version. the existing pruning case now covers a chained cli symlink and an aliased base directory. the chained-symlink regression deleted the required runtime before the fix; the fixed case keeps it.

The branch head I can inspect still uses a single readlink and a non-canonical versionsDir. Could the fix be on another branch?

🟡 Medium updates/RestartWork.ts:30

Cancelling a track call blocked on gate does not interrupt it, so it stays pending until the launcher handoff releases the gate (potentially after the five-minute backstop); callers such as TerminalManager.open consequently retain their thread lock and block later operations. gate.withPermits runs as acquireUseRelease's masked acquisition, making the semaphore wait uninterruptible. Move the interruptible gate wait outside that masked acquisition while keeping the counter increment and finalizer registration atomic.