feat(server): install provider CLI updates in the background#17585

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Keep the window closed when resume state cannot be read.

If getLimitRecoveryCandidates fails, orElseSucceed(() => []) treats an unknown resume schedule as no upcoming resume. A typed getSettings failure also bypasses the resume check. Unlike the active-thread and scheduled-task reads, these failures can allow an install immediately before an automatic resume. Return a closed window on either read failure. Effect.option maps typed failures to None. (effect.website)

✅ Confirmed as addressed by @maria-rcks

Settings and recovery reads now fail closed. The window checks per-thread armed resumes even when the global default is off, and protects eligible near-due unarmed resumes when the global preference is on. Malformed persisted settings also disable automatic updates and restart continuation without replacing the file.

@maria-rcks Verified the fix in UpdateWindow.ts. Both getSettings and getLimitRecoveryCandidates failures now return a closed window. The recovery check also runs when autoResumeLimitedThreads is off.

The malformed-settings fallback disables automaticUpdates and continueThreadsAfterServerUpdate. 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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Clear Claude threads with no pending background tasks.

The Claude completion path emits status: "active" even when the roster is empty. Both active-work selectors then block updates although no run or background task remains. Set the thread to idle when the completed turn has no roster entries.

✅ Addressed in commits ce07c86 to d0fb53f

@maria-rcks Thanks for clarifying. The supplied PR context shows that Claude emits idle after a completed turn when its background roster is empty. The compact-turn test checks that lifecycle event. Pending background work keeps the provider thread active.

The concern does not apply to the current code. I withdraw this finding.

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Include pending turn items in active-work selection.

When a command_execution, dynamic_tool, or subagent item remains pending after its run ends, this selector can report no active work. The SQL active-work selector at Line 3611 also omits those items. The existing runtime selectors include them and exclude items from rolled-back runs. Apply that same condition in both active-work selectors so an update does not begin over pending background work.

✅ Confirmed as addressed by @maria-rcks

Both active-work selectors now include pending/running/waiting command, tool and subagent items while excluding rolled-back runs. The existing ProjectionStore case exercises those pending items.

@maria-rcks Thanks for the fix. Both active-work selectors in the supplied code now include pending, running, and waiting turn items and exclude items from rolled-back runs. 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.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Check the due time of frequent scheduled tasks.

If an interval task repeats every ten minutes and its next run is one minute away, this branch skips it before the five-minute lookahead check. runIfOpen can then start a provider install that overlaps the task. Do not discard near-due tasks solely because they repeat frequently. Coordinate the task with the install if frequent tasks must remain eligible for an update window.

✅ Confirmed as addressed by @maria-rcks

Removed the frequent-interval exemption. Every enabled task due within five minutes now keeps the window closed. In the lab, the ten-minute task environment deferred updates while its imminent task ran, then updated when the next run was outside the lookahead.

@maria-rcks Thanks for the fix. Removing the exemption addresses this finding. Your lab result confirms that the ten-minute task blocks updates while imminent or running, then permits updates outside the five-minute lookahead.

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Provide ProviderMaintenanceAdmission once from the application layer.

UpdateWindow.layer, ProviderMaintenanceRunner.layer, and ProviderTurnStartService.layer each provide admissionLayer locally. The current server build shares the layer reference, but an independently built graph creates a separate Semaphore.make(1). Such graphs do not share admission.

Remove the local providers and provide admissionLayer once from the application-owned root layer that builds these consumers.

Source: Coding guidelines

✅ Addressed in commits ce07c86 to 7b9e2f9

The production server builds these consumers once in runServer via Layer.launch(layerMakeServer). All three providers reference the same exported admissionLayer, and Effect 4.0.2 propagates the same MemoMap to Layer.provide dependencies. WebSocket handlers receive the server-owned maintenance runner via Layer.succeed at ws.ts:3198; they do not create another semaphore. Independent application graphs would still allocate separate semaphores if provision moved to each root. No current runtime path builds these consumers independently, so no change is needed. Independently rechecked by a final source reviewer on eb9277c4ff.

@maria-rcks, thanks for clarifying. I withdraw this finding.

The shared admissionLayer reference uses the same MemoMap within the production layer build, so the three consumers share one semaphore. WebSocket handlers receive the server-owned maintenance runner rather than construct another instance. Moving provisioning to separate application roots would not make those roots share a semaphore.

No change is needed for this finding.

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium orchestration-v2/ProjectionStore.ts:3631

active-work continues returning a thread with pending background tasks after their owning provider session stops if another session on that thread is still live, so UpdateWindow.blockers prevents automatic CLI updates indefinitely. The query only checks for any live session bound to threads.thread_id; correlate the session with threads.provider_session_id so only the task roster’s owner keeps the thread active.

Fixed in d36093c71e. Both the SQL and the in-memory active-work checks now count a pending task roster only while the provider session named by that provider thread's providerSessionId is live.

🟠 High textGeneration/TextGeneration.ts:64

Text-generation requests run without the admission permit, so a background install can replace the provider CLI while resolution or generation is in progress and cause commit, PR, branch, or title generation to fail. withPermit wraps only Effect.void and releases the permit before resolveInstance or the generation effect starts; hold it around both resolution and the complete generation operation.

Keeping this as a wait, matching provider turn starts. The permit is a single process-wide semaphore, so holding it for the whole generation would serialize every title, commit, PR, and branch generation across all threads. The overlap the wait leaves open needs an install to begin while a generation is running. That can't happen in practice: an install only starts inside the update window, and the window stays closed for 15 minutes after any client interaction. Commit, PR, and branch generation follow a click, and title generation comes with a turn start, which already keeps the window closed.

🟡 Medium server/maintenanceResolver.ts:501

A symlink located under BUN_INSTALL/bin or PNPM_HOME makes this resolver grant automatic updates to a different installation, potentially updating the wrong package. The prefix checks inspect commandPaths, which includes the visible resolved path, while the ownership guard checks only context.realCommandPath; require the real path to match these prefixes before granting ownership.

Fixed in 35b7f96ebd: bun and pnpm ownership now requires the real command path to sit under BUN_INSTALL/bin or PNPM_HOME, so a symlink into another installation is left manual. Covered by "does not grant bun/pnpm ownership to a symlink targeting another installation".

🟡 Medium provider/ProviderMaintenanceHostLock.ts:258

While any local reader is active, acquireRead admits subsequent turns and CLI calls without withGate. Those arrivals can keep increasing readCount after a writer closes admission, preventing the writer from draining; track inherited read ownership separately so only helper launches joining their own turn bypass the gate.

Fixed in 35b7f96ebd: a waiting install now holds back new readers so it can drain, while helpers of an already admitted turn still join it. Covered by "blocks unrelated readers behind a waiting writer while admitting turn helpers".

🟠 High provider/ProviderInstallSupervisor.ts:271

When stderr contains any text, beat never samples or compacts .stdout, so a verbose, long-running installer can grow that redirected file without the intended 20 KB bound and exhaust disk space. Read both streams before selecting stderr or stdout for progress.

Fixed in 35b7f96ebd: the supervisor reads and compacts both installer streams on every beat; stderr still drives progress. Covered by "finishes the installer after verbose-output".

🟡 Medium components/ProviderUpdatePrimaryNotification.tsx:263

When a provider is removed while an earlier updateProvider call is awaited, this continue leaves the removed provider in providerInstanceIds and providerCount. The completion view then waits for a snapshot that cannot arrive, leaving activeToastRef stuck on an update and suppressing later prompts. Remove skipped providers from the tracked set and count, and settle the batch when no providers remain.

🟠 High provider/ProviderMaintenanceHostLock.ts:175

If the second installIsAlive(directory) check or lease-file removal fails, the acquired gate is never released, so its heartbeat keeps the host-wide lock fresh and blocks provider launches and updates on every server. These operations run before acquireGate returns the release finalizer; ensure result.success() runs on every failure path after acquisition.

Fixed in 2934544e9f: a failure in the second liveness check or the lease-file removal now releases the acquired gate before propagating, so no heartbeat keeps the host lock alive. Covered in providerMaintenanceRunner.test.ts.

🟡 Medium server/maintenanceResolver.ts:584

An explicitly pinned binary still receives an installationKey in the npm branch, so ProviderAutoUpdater admits it for background updates and can modify an install the pinned guard is meant to protect. This branch constructs the key directly instead of using keyFor; use keyFor(npmPrefix) so pinned installs remain excluded.

Fixed in 2934544e9f: the npm branch now uses keyFor(npmPrefix), so an explicitly pinned binary gets no installation key and stays out of background updates. Covered in maintenanceResolver.test.ts.

🟠 High orchestration-v2/ProviderSessionManager.ts:1789

A startTurn continuation deadlocks behind a maintenance writer even though its session lease is still held for pending background work: the writer waits for that lease, while this outer read blocks before the inner leased read can run. Pass hostLeases.get(runtime) to this outer read and ensure the prerequisite operations can join the same lease.

Fixed in 2934544e9f: start, resume, ensure, and snapshot for a session that already holds a host lease join that lease instead of queueing behind a waiting writer, and helpers inherit their own active token after the original lease ends. Unrelated new turns still wait behind the writer. Regression cases are in ProviderSessionManager.test.ts.

🟠 High provider/ProviderMaintenanceHostLock.ts:71

A stale install.json blocks every provider launch when its PID is reused by an unrelated live process, because isProcessAlive(pid) returns true and acquireGate keeps waiting without reclaiming the lease. The lease stores only the PID, so persist and validate the supervisor's process and boot identity before treating it as live, while retaining the child-process check.

Fixed in b574fd0b3a: install leases now record the supervisor's process start identity (plus the boot identity on Linux). A lease whose pid is alive but belongs to a different process is reclaimed; leases written without an identity, or whose identity cannot be read, stay conservative. Covered by "reclaims an install lease whose live PID belongs to another process".

🟠 High provider/ProviderMaintenanceHostLock.ts:150

On Windows, installChildrenAreAlive returns false when the installer exits after spawning a still-running grandchild, so the gate and install lease can be released while that helper is still writing. The scan rebuilds descendants from the supervisor PID, but Win32_Process.ParentProcessId can point to the already-terminated intermediate process, leaving the helper unlinked; retain descendant identities while ancestors are alive or use lifetime tracking.

addressed in 92972640. windows installs use a named kernel job that retains descendants after intermediate parents exit. durable descendant identities also preserve orphan helpers across supervisor loss, and admission stays closed until every helper exits. the existing maintenance tests cover orphan helpers, supervisor death, identity reuse, and failed process queries. native windows execution remains unverified.

🟠 High provider/ProviderMaintenanceHostLock.ts:359

On Windows, a crashed reader's stale lease can keep withWrite waiting forever after its PID is reused, blocking new turns and probes across servers. registerRead stores an empty identity because processIdentity returns null off Linux, and drained cannot distinguish the reused PID from the original reader; store and compare readProcessIdentity for reader leases.

addressed in 92972640. reader leases now persist readProcessIdentity on windows and compare that creation identity before retaining a stale lease. the existing maintenance regressions cover pid reuse and unreadable process identity, keeping uncertain live readers protected.

🟡 Medium provider/ProviderInstallSupervisor.ts:177

When the supervisor finishes between readCurrent and this liveness check, run fails with “Provider install supervisor exited without a result” even though the completed result is durable, so the caller marks a successful update as failed. Re-read the receipt after detecting the dead supervisor and return its result before reporting failure.

addressed in 92972640. when liveness reports that the supervisor exited, the runner rereads the durable receipt and returns its result before reporting failure. the existing regression makes the result appear between the first receipt read and the liveness check.

🟠 High provider/providerMaintenanceRunner.ts:185

If the server is killed after the detached installer starts but before recordChildPid(pid) completes, the lock still names the dead server, so another server can start a second installer while the first is modifying the installation. The installer can outlive its parent because it is detached; gate its installation work until lock ownership is durably transferred to a surviving process.

Not changing this. The window is the spawn event plus one small file write, and a second installer also needs another server to try the same install target inside that window, right as this server is SIGKILLed. Closing it fully needs the installer itself to own the lock before it runs, which is the supervisor process this PR removed to keep the design small. A server killed in that interval is the accepted gap.

🟠 High provider/ProviderInstallLock.ts:164

untilIdle reports completion when anyHeld fails to read the lock directory or a lock file, so ProviderTurnStartService.start can launch a CLI while a detached installer is still replacing it. The orElseSucceed(() => false) treats read errors as idle; keep admission closed and retry instead.

Leaving this as is on purpose. A missing directory or lock file already counts as free, so this path only handles a lock directory that cannot be read at all (permissions, a broken cache mount). Keeping admission closed there would block every turn start on the server forever, with no way out short of fixing the filesystem. Letting the turn start is the smaller failure: a background install also needs that directory, so it cannot be running from this server in that state.

🟠 High provider/ProviderInstallLock.ts:111

Concurrent tryRun calls can both acquire the same lockKey and run installers, allowing concurrent writes to the provider installation. After both calls see the stale PID as dead, one can recreate the lock while the other unconditionally removes that new lock at line 111 and recreates it; coordinate stale-lock reclamation exclusively before removing the file.

Fixed in 0483ac0e74. Reclaiming a stale lock now takes a .reclaim sentinel created with wx, rechecks the lock under it, then removes and recreates it. A sentinel older than 10 s counts as abandoned. A test races two servers on a lock holding a dead pid and checks exactly one runs.

🟠 High provider/ProviderInstallLock.ts:125

Two servers can both acquire the reclaim sentinel and reclaim the same lock, allowing concurrent installers for one target. When both stat an expired .reclaim file, the unconditional removal here lets the second server delete the first server's fresh sentinel; both exclusive creates can then succeed. Use an ownership-safe stale-sentinel reclaim protocol so only one server can proceed.

🟠 High background/BackgroundPolicy.ts:278

Idle mobile clients send recentlyInteracted: true on every 25-second foreground heartbeat, so this branch keeps resetting lastClientInteractionNanosRef. As a result, sinceLastClientInteraction never reaches 15 minutes and automatic provider updates remain suppressed; update the timestamp only for genuine interaction events, not foreground activity.

Leaving this as is. Mobile reports recentlyInteracted: active, and active is only true while the app is open in the foreground. When the phone locks or the app is backgrounded, it reports false, and the 15-minute idle window starts. Treating an open, foregrounded app as presence is intended: the background updater should not swap a provider CLI while someone has T3 Code in front of them and may start a turn. Web reports real input interaction, so desktop and browser idle detection is finer-grained.

🟡 Medium settings/SettingsPanels.tsx:2837

Update automatically remains enabled when enableProviderUpdateChecks is off, even though the server requires both settings to install updates, so the UI presents an unusable setting as active. Disable this switch when provider update checks are off.

No longer relevant as of 36021ba04f4c876eaad4478a2b4bd1de56954115

Fixed in 36021ba04f on web and mobile: Update automatically is disabled while Provider update checks is off and shows "Turn on Provider update checks to update automatically." Before/after screenshots are in the PR body.