fix(server): make remote updates rollback-safe#5181

🟠 High src/serviceLauncher.ts:369

stop() sends the requested signal to the child and then waits indefinitely for it to exit via waitForExit, with no grace timeout or SIGKILL fallback. A child that handles or ignores SIGTERM/SIGINT blocks #completion from resolving, so the launcher hangs during shutdown until an external timeout kills the process tree. terminateChild() already implements a grace period followed by SIGKILL — stop() should use the same strategy so shutdown always completes.

✅ Resolved in 32ae738ae53a35a89fb8caf9ad9982a95512b3da

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 32ae738ae. stop() now runs through the serialized transition queue and uses the shared terminate helper, which preserves the requested signal and escalates to SIGKILL after the grace period.

🟠 High Layers/ProviderRuntimeIngestion.ts:1808

forkParked delays Stream.runForEach(providerService.streamEvents, ...) until after activation completes, so the ingestion subscriber is not installed while the server waits at the activation boundary. ProviderService.streamEvents is a fresh PubSub subscription that does not replay missed events, so any provider events published during the prepared-to-activated interval are permanently dropped — lifecycle and message projections miss those events after handoff. The stream subscription needs to start before parking so events are queued, with processing deferred until activation completes.

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Verified as not reachable in this lifecycle. A fresh candidate has no active provider session or provider process that can publish runtime events: all HTTP and WebSocket ingress now waits on command readiness, orchestration roots and the session reaper are parked, and ProviderService only starts or recovers an adapter session in response to those commands. There is therefore no producer during the prepared-to-activated interval. Starting subscriptions early would add buffering machinery without an event source.

🟠 High Layers/ProviderCommandReactor.ts:1355

clearInterruptedThreadTitleRegenerations now runs behind activation via forkParked, so it reads the projection snapshot only after activation — concurrently with the newly-started domain-event stream. A thread.meta-updated regeneration request that arrives right after activation is picked up by this cleanup pass and completed with no title as if it were stale/interrupted, racing the worker that is actively processing the same request and silently discarding the generated title. The stale-request scan should capture request IDs before activation and defer only the correlated completions, or otherwise run cleanup before the stream starts accepting new events.

✅ Resolved in 32ae738ae53a35a89fb8caf9ad9982a95512b3da

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 32ae738ae. The reactor now snapshots the exact (threadId, requestId) pairs before installing the stream or waiting for activation, then dispatches only those correlated completions after activation.

🟡 Medium cloud/pinnedRuntime.ts:198

Effect.tapError only runs its handler on typed failures, not on interruption. If the install fiber is interrupted during npm install, validation, sentinel writing, or rename, the .staging-* temp directory is left behind. Repeated interrupted installs accumulate full node_modules trees under runtime/versions, consuming disk space. Consider using Effect.ensuring so cleanup runs on both failure and interruption.

🟠 High src/serviceLauncher.ts:363

#fatal() calls disconnect() and unref() on the managed child but never terminates it. Because disconnect() only closes the IPC channel and unref() only lets the parent exit independently, the child process keeps running. Any transition failure while a child is alive (e.g., a durable state write or IPC send failure) rejects the launcher's completion promise while leaving the server process orphaned and still serving, which can conflict with a restarted launcher's new child. Terminate the child before rejecting completion.

🟠 High cloud/pinnedRuntime.ts:88

An interrupted install now leaves the pinned-runtime directory in a state that permanently blocks all future installs of that version. When versionDir exists but the sentinel or entry file is missing (the exact partial state left by a killed npm install), the function returns PinnedRuntimeInstallError with step "checking an incomplete pinned runtime" instead of cleaning up and retrying. Every subsequent ensurePinnedRuntimeInstalled call for that version hits the same check and fails identically, so the runtime can never repair itself without manual filesystem cleanup.

The previous implementation removed and rebuilt the incomplete directory; this change replaced that with an early failure. Consider either removing the incomplete versionDir and proceeding with a fresh install, or keeping the cleanup-and-retry path.

Separately, when validate fails on an already-pinned runtime (sentinel and entry exist but the runtime is corrupt), the function fails without removing the bad directory. Since the prior removePinnedRuntimeInstallation cleanup was removed in this PR, every retry re-validates the same corrupt runtime and fails again. Consider removing or rebuilding the runtime when validation fails so retries can recover.

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 32ae738ae. Incomplete final directories are removed and rebuilt. A completed runtime that fails install validation is also removed and reinstalled, while a distinct tagged preflight-blocked error preserves valid runtimes that are merely incompatible with the current migration frontier.

🟠 High src/serviceLauncher.ts:403

stop() runs outside the #transitions queue, so shutdown races with in-progress startup or rollback transitions. If a signal arrives while #recover() or #startChild() is awaiting runtimeExists() or spawn, stop() sees #child === null, resolves completion, and then the transition installs a new child afterward — that child never receives the shutdown signal and is left running. The same race affects #rollback: after it writes state but before #startChild spawns, stop() can clear the current child and resolve, after which the transition still spawns a new child because #startChild does not check #stopping. Serialize stop() through #transitions or re-check #stopping inside #startChild before publishing the spawned child.

cloud/selfUpdate.ts:166

PinnedRuntimeInstallError.step is a diagnostic stage name (message renders it as Pinned runtime install failed while ${step}.), but here a full user-facing sentence from preflight.reason is stuffed into it and the caller recovers it by prefix-sniffing (startsWith("This version includes")). That both derives the wrapper message from an unstructured field and uses a discriminator-by-string to choose caller-visible text.

Suggest a distinct tagged error for the blocked-preflight case (e.g. ServicePreflightBlockedError with version and reason, message derived from them) failed from validate, and recovering it with Effect.catchTags({ ServicePreflightBlockedError: ... }) instead of prefix matching on step.

No longer relevant as of 32ae738ae53a35a89fb8caf9ad9982a95512b3da

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 32ae738ae. A blocked preflight now fails with PinnedRuntimePreflightBlockedError { version, reason }, and the caller branches on the tag rather than inspecting a diagnostic step string.

isServiceLauncherTrial() re-reads and re-decodes the launcher context from the global process.env inside a layer, duplicating state that ServiceLauncherClient already exposes as trial — and with different strictness (the client fails on invalid context, this silently returns false, so a corrupt context would run migrations in a trial child).

Suggest acquiring the flag from the service instead, e.g. const launcher = yield* ServiceLauncherClient.ServiceLauncherClient in this Layer.unwrap (or threading trial in from makeServerLayer, which already provides that layer), so persistence has one visible source of truth.

^{Posted via Macroscope — Effect Service Conventions}

cloud/serviceLauncherClient.ts:213

make (and isServiceLauncherManaged / isServiceLauncherTrial) default to the global process and read process.env directly, so the launcher context is acquired outside the Effect environment and the options.process parameter is a test-only seam rather than a declared dependency. packages/shared/src/hostProcess.ts already models this with HostProcessEnvironment (and HostProcessExecutablePath, etc.), which this file's own dependents use.

Suggest reading the context with const env = yield* HostProcessEnvironment inside make, and — if the IPC handle itself must be overridable — exposing it as a Context.Reference defaulting to process instead of an options.process default, so the requirement shows up in layer. The sync helpers called from Effect code (resolveServerSelfUpdateCapability, persistence/Layers/Sqlite.ts) should then read the same reference rather than the global.

No longer relevant as of 32ae738ae53a35a89fb8caf9ad9982a95512b3da

Broken pinned runtimes block retries

High Severity

ensurePinnedRuntimeInstalled now hard-fails on an incomplete versionDir, and when an already-pinned tree fails validate it leaves that tree in place. The previous path removed broken installs so the same version could be retried; without that cleanup, remote updates and service install can stay stuck on a known-bad runtime until manual deletion.

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

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 32ae738ae. Incomplete installs are deleted and rebuilt, completed installs that fail validation are reinstalled, staging is cleaned on interruption, and focused tests cover all three recovery paths.

Trial opens database before commit

High Severity

Trial setup turns PRAGMA query_only off immediately after the migration check, long before the launcher commits. Ingress that is not parked behind activation—such as HTTP orchestration.dispatch—can therefore write to the shared database while the candidate is still uncommitted, so a rolled-back trial can leave durable mutations behind.

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

Find path swallows fiber interrupts

Medium Severity

findInterruptedThreadTitleRegenerations is wrapped in catchCause that logs and returns [] for every cause, including interrupts. Nearby clear/dispatch paths re-raise interrupt-only causes with Cause.hasInterruptsOnly. During scope shutdown this lets start keep forking parked work instead of aborting.

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

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed on the latest head. Pure interruption is re-raised in both startup cleanup paths instead of being logged and recovered; the focused ProviderCommandReactor suite passes.

🟠 High cloud/pinnedRuntime.ts:100

Two concurrent callers can delete each other's published runtime. With the process-wide lock removed, both callers can observe an incomplete versionDir. Caller A removes it and atomically publishes a valid staged runtime via fs.rename. Caller B — which already passed the versionDirExists && !alreadyPinned check — then executes its delayed fs.remove(paths.versionDir), deleting A's freshly published runtime. If B's own install subsequently fails, no valid runtime remains. The fs.remove at line 101 runs unconditionally based on a stale observation, with no check that the directory is still the same incomplete one that was inspected. The removal must be serialized or made conditional on the directory not having changed between the check and the delete.

✅ Resolved in 73c62840a6e5f4435dba4d549121f404148ec862

This file previously imported the service module as a namespace (import * as BootService from "./bootService.ts"). A bare make import erases the module namespace at a service boundary; consider keeping import * as BootService from "./bootService.ts" and calling BootService.make(...) / BootService.renderBootServiceUnit(...).

Same pattern in the two sibling tests: selfUpdate.test.ts:14 (import { make } from "./selfUpdate.ts") and serviceLauncherClient.test.ts:12 (import { make, ServiceLauncherHostProcess } from "./serviceLauncherClient.ts").

^{Posted via Macroscope — Effect Service Conventions}

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in d3b491995. All three service-boundary tests now use namespace imports.

update still has several behavioral branches that no longer have any test: non-exact target versions, the desktop-managed refusal, the concurrent-update guard, and surfacing PinnedRuntimePreflightBlockedError.reason verbatim to the caller (the migration-boundary message users are told to act on). The previous suite covered the equivalent branches and was removed. Consider adding focused tests for the blocked-preflight reason and at least one rejection branch.

^{Posted via Macroscope — Effect Service Conventions}

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

I am keeping the reduced focused coverage here. servicePreflight.test.ts directly asserts the actionable migration-block message, while this test proves install → preflight → launcher acceptance ordering and ID propagation. Restoring the deleted guard matrix would re-expand scope around small existing guards without improving the new state-machine proof.

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Added focused coverage in d393dabf5 for invalid versions, desktop-managed refusal, verbatim preflight-block reasons, and the concurrent-update guard, alongside the existing stage/preflight/accept ordering test.

install gained a new multi-artifact rollback (stop the unit, restore the previous launcher/state/unit, daemon-reload, then restart or disable) and status.current gained new launcher/sentinel/state conditions, but the rewrite leaves only this happy-path test. The previous suite's coverage for failed activation cleanup, uninstall, the non-Linux unsupported path, and reconcileService idempotency was deleted without replacement, and all of that behavior still exists. Consider restoring focused tests for at least the rollback path and uninstall.

^{Posted via Macroscope — Effect Service Conventions}

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Added focused coverage in d393dabf5 for current versus pending status, uninstall, unsupported platforms, and restarting an installed unit when repair fails. The shared fixture keeps those cases compact.

🟠 High src/server.ts:437

When launcher.prepareTrial fails (IPC disconnect or rejection), the activation Deferred is never completed, so awaitActivation inside runtimeStateLayer's acquire blocks forever. This prevents makeServerLayer from finishing launching, and the trial process hangs instead of exiting so the service launcher can roll it back. The tailscaleServeLayer acquire has the same issue — it also awaits awaitActivation and blocks indefinitely on the same path. Consider failing the activation Deferred (or failing the layer) when startup fails, so the auxiliary layers complete their acquire and the process can exit.

✅ Resolved in 06f189cfe8ebdd50f840ea4b475bbba1157b2533

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO\n-----\n\nFixed in 06f189cfe. Startup failure now defects the existing activation Deferred. The auxiliary layer acquisitions unwind without opening the gate, so Layer.launch exits and the launcher can roll the trial back immediately.

🟠 High cloud/serviceLauncherClient.ts:182

exchange registers IPC listeners and sends a message but never times out waiting for a reply. If the launcher stays connected but never sends an acceptable response (malformed/unexpected messages or a stall), the effect never completes — requestUpdate hangs with the self-update lock held, and prepareTrial blocks indefinitely at the server activation boundary, leaving the trial server hung. Add a bounded timeout that cleans up the listeners and fails the exchange.

✅ Resolved in 06f189cfe8ebdd50f840ea4b475bbba1157b2533

Trial failure no longer exits

High Severity

Startup runs in a forked fiber and now only fails the command gate on error. The trial-specific shutdown.request(1) path was removed with ServerShutdown, so a failed trial stays alive until the launcher’s 120s prepared timeout instead of exiting so the launcher can roll back immediately.

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

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO\n-----\n\nFixed in 06f189cfe without restoring the shutdown coordinator. Startup failure defects the closed activation gate, which fails the waiting application-layer acquisitions and causes the trial process to exit immediately.

Publish can erase pinned runtime

High Severity

Publishing deletes versionDir before renaming the staging tree, then Effect.ensuring always deletes staging. If rename fails after the remove, both the previous runtime and the staged replacement are gone, contradicting the “never leave no valid runtime” comment and leaving that version uninstallable until a full reinstall.

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

Service install lost failure rollback

Medium Severity

install still stops an existing unit and durably rewrites launcher, state, and unit files, but the previous restore-and-restart tapError path is gone. If daemon-reload, enable, linger, or restart fails afterward, the machine can be left with a stopped service and partially applied admin state and no automatic return to the prior unit.

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

Layers/Sqlite.ts:10

serviceLauncherClient.ts is a service module consumed here at a service boundary (yield* ServiceLauncherClient on line 74), so it should be imported as a namespace and used through its module shape — the other consumers added in this PR (server.ts, serverRuntimeStartup.ts, selfUpdate.ts) already do.

and at the use site:

No longer relevant as of 73c62840a6e5f4435dba4d549121f404148ec862

writeDurably re-derives the parent directory by slicing on a literal / and creates it with NodeFSP.mkdir, even though make already acquires Path.Path and FileSystem.FileSystem from the environment (and atomicWrite.ts already owns the temp-write/rename part). Consider defining the helper inside make (or passing path) so directory derivation and creation go through the acquired services and the node:fs/promises escape hatch is limited to the fsync work that Effect's FileSystem cannot express.

^{Posted via Macroscope — Effect Service Conventions}

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO\n-----\n\nFixed in 73c62840a. The helper now lives inside make and uses the acquired Path and FileSystem services for directory derivation, temp creation, atomic rename, and both fsync operations.

🟡 Medium src/serverRuntimeStartup.ts:436

The generic welcome event published after options.activate can overwrite the auto-bootstrapped welcome event that was published in the parked fiber, silently dropping bootstrapProjectId and bootstrapThreadId. When forkParked releases the auto-bootstrap fiber at the activation boundary, startup immediately proceeds to commandGate.signalCommandReady and publishes a second welcome event without waiting for the parked fiber to complete. If the generic event arrives last, ServerLifecycleEvents replaces the snapshot's welcome event, so clients connecting afterward receive a welcome snapshot missing the bootstrap targets. Consider awaiting the auto-bootstrap fiber before publishing the generic welcome, or reordering so the generic welcome is published before the parked fiber is released.

✅ Resolved in 74e600f8332ec159539f590fdd97aec281f82817

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in 74e600f83. After durable commit, the generic welcome is published while the activation gate is still closed. Opening the gate afterward means the richer parked auto-bootstrap welcome can only arrive later and remain the snapshot.

cloud/serviceProtocol.ts:54

This is selfUpdate.ts's EXACT_VERSION_PATTERN moved here, but its documented invariant was lost: exact versions only (never dist-tags) so the acknowledgement names the version that was actually installed, and so the value stays safe to pass to npm and to embed in filesystem paths. Consider restoring that comment above the helper.

No longer relevant as of 74e600f8332ec159539f590fdd97aec281f82817

The rationale for this timeout was dropped while the timeout itself moved unchanged. Consider carrying it over so the value is not tuned away later.

^{Posted via Macroscope — Effect Service Conventions}

This rewrite drops invariant documentation for code that is still here: the %-specifier rationale on escapeSystemdSpecifiers/quoteSystemdValue (including that append: paths take the rest of the line literally and must not be quoted), the renderBootServiceUnit notes (pure/byte-for-byte testable, minimal user-manager environment requiring absolute paths, the deliberate absence of After=network-online.target, the StartLimitBurst reasoning, and restart rather than enable --now), and the field/method docs on BootServicePlan, BootServiceStatus.current, and BootService.install/uninstall. Consider restoring the comments that still describe current behavior.

^{Posted via Macroscope — Effect Service Conventions}

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Restored the operational comments that prevent mistakes: systemd percent expansion and append paths, shell/PATH-free unit rendering, and the deliberate absence of a network-online target. I kept self-evident field and method prose removed so the documentation does not grow back into implementation noise.

🟠 High cloud/bootService.ts:285

The install effect stops the running service (line 289–294) before performing several fallible writes and systemctl/loginctl invocations, but the rollback handler that previously restored the service on failure was removed in this PR. If any subsequent step fails — writing the launcher, state, or unit file, daemon-reload, enable, enable-linger, or restart — the effect aborts with the previously working service left stopped and its unit file potentially overwritten, causing an avoidable remote-server outage that requires manual recovery. Consider restoring the prior rollback behavior: snapshot the old unit file before stopping, and on any failure after the stop, restore the old unit (or remove it if none existed), run daemon-reload, and restart the service.

No longer relevant as of 857ff043330324e6f4083a11e349b6c2615d3957

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Addressed in e05b28a6b with smaller recovery. On any failure after stopping an installed unit, the installer attempts to restart that unit before returning the original error. The runtime is prepared first and each administrative artifact is atomically and durably replaced, so systemd restarts either its old loaded selection or a fully written new one. This avoids leaving the service down without reintroducing a three-artifact snapshot/restore transaction.

cloud/serviceLauncherClient.ts:232

This module owns construction (make acquires ServiceLauncherHostProcess and HostProcessEnvironment and can fail), but it exports no canonical layer, so server.ts runs make() by hand and hand-rolls Layer.succeed(ServiceLauncherClient.ServiceLauncherClient, launcherClient). Since ServiceLauncherClient is a Context.Reference with an unmanaged default, a consumer that misses that ad-hoc layer silently gets the fallback instead of a type error.

Consider exporting the canonical layer here and providing that single layer value in server.ts (layer memoization keeps one instance across the two Layer.provide sites):

No longer relevant as of 857ff043330324e6f4083a11e349b6c2615d3957

One error covers eight operations, but its message returns the free-form reason rather than being derived from structural attributes, and the operation discriminator is what selects caller-visible text: selfUpdate.ts does error.operation === "rejected" ? error.reason : "Could not ask the service launcher…". Only the launcher rejection carries a reason authored elsewhere that must reach the user; the other seven reasons are hardcoded sentences that operation already identifies.

Consider splitting the rejection into its own tagged error (e.g. ServiceLauncherRejectedError with targetVersion + launcher reason) so callers can pass it through by tag, and deriving the remaining error's message from operation instead of storing a sentence as data.

^{Posted via Macroscope — Effect Service Conventions}

cloud/serviceLauncherClient.ts:96

This helper's only behavior is new ServiceLauncherClientError({ ... }), so the error's attributes and cause are hidden behind positional arguments at every failure site. Consider constructing the error inline at each failure boundary (or, if the optional cause spread is the motivation, keeping the construction explicit with ...(cause === undefined ? {} : { cause })).

No longer relevant as of ff26b5a2202bbdb58d2842f2d8e368f09621ceb1

[GPT-5.6-SOL] RESPONDING ON BEHALF OF THEO

Fixed in ff26b5a22. Removed the constructor wrapper and made every ServiceLauncherClientError construction explicit at its failure boundary, including cause where present.