From bbe146228b41a73ec95d096d8c8bdbf46efed6a3 Mon Sep 17 00:00:00 2001 From: NoahLinckeScout Date: Thu, 27 Aug 2026 12:15:51 -0700 Subject: [PATCH 01/11] fix(server): refuse to start a second server against a live data directory Two servers pointed at one `--base-dir` both open `state.sqlite` and both write `settings.json`, and they overwrite each other. Observed: a desktop app auto-updated to a newer server while the old one was still running, the new process found its port taken, silently bound a random one, and ran blind against shared state. The visible symptom was a settings toggle that would not stick -- hours away from the cause, and nothing about it is detectable afterwards. So refuse at startup. The lock is claimed before anything binds a port or opens the database, and is provided into `HttpServerLive` rather than merged beside it so the ordering is structural: the lock is a dependency of the thing it protects. An advisory `flock` would be the better primitive, since the kernel drops it when the holder dies. Node has no binding for it and a native dependency for one lock is the worse trade, so this is an atomically created file holding the owner's identity, with liveness checked by signal 0. The tradeoff is stated in the module: a killed server whose pid is later reused blocks startup until the file is removed, which is the safe direction, and the message names the file. A lock whose owner is gone, or which a crash tore in half mid-write, is reclaimed rather than treated as permanent. Reclaiming re-races the exclusive create, so two servers starting together still produce one winner. Release only removes a lock this process still owns, so a successor is never evicted. The bound port is stamped onto the lock afterwards purely so a later server's refusal names an address the user can open rather than just a pid. Verified end to end against two real servers: the second refuses with the message below, exits 1, and never binds; shutdown releases; restart is unblocked. Another T3 Code server is already using this data directory. data directory: /tmp/t3-smoke-basedir/userdata held by: pid 285163, listening on port 39977 since: 2026-08-27T16:59:03.329Z --- apps/server/src/server.ts | 27 ++- apps/server/src/serverSingleton.test.ts | 158 ++++++++++++++++++ apps/server/src/serverSingleton.ts | 210 ++++++++++++++++++++++++ 3 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 apps/server/src/serverSingleton.test.ts create mode 100644 apps/server/src/serverSingleton.ts diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index c50244d64973..4f5c4845628d 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -56,6 +56,7 @@ import * as GitManager from "./git/GitManager.ts"; import * as EnvironmentTheme from "./environmentTheme.ts"; import * as Keybindings from "./keybindings.ts"; import * as ServerRuntimeStartup from "./serverRuntimeStartup.ts"; +import * as ServerSingleton from "./serverSingleton.ts"; import { OrchestrationReactorLive } from "./orchestration/Layers/OrchestrationReactor.ts"; import { RuntimeReceiptBusLive } from "./orchestration/Layers/RuntimeReceiptBus.ts"; import { ProviderRuntimeIngestionLive } from "./orchestration/Layers/ProviderRuntimeIngestion.ts"; @@ -186,6 +187,20 @@ const RelayClientLive = Layer.unwrap( }), ); +/** + * Claims the data directory before anything binds a port or opens the database. + * + * Provided into `HttpServerLive` rather than merged alongside it so the ordering + * is structural: the lock is a dependency of the thing it protects, and a second + * server cannot reach a listening socket while another holds the directory. + */ +const ServerSingletonLive = Layer.effectDiscard( + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + yield* ServerSingleton.acquireServerSingleton(config.stateDir); + }), +); + const HttpServerLive = Layer.unwrap( Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; @@ -515,6 +530,16 @@ export const makeServerLayer = Layer.unwrap( return; } + // Stamp the port onto the lock we already hold. It is only ever read + // by a *later* server's refusal message, which turns "something else + // is running" into an address the user can open. + yield* ServerSingleton.serverLockPath(config.stateDir).pipe( + Effect.flatMap((lockPath) => + ServerSingleton.recordServerLockPort(lockPath, address.port), + ), + Effect.ignore, + ); + const state = yield* makePersistedServerRuntimeState({ config, port: address.port, @@ -688,7 +713,7 @@ export const makeServerLayer = Layer.unwrap( Layer.provideMerge(runtimeServicesLive), Layer.provide(activationLayer), Layer.provideMerge(serverRelayBrokerTracingLayer), - Layer.provideMerge(HttpServerLive), + Layer.provideMerge(HttpServerLive.pipe(Layer.provide(ServerSingletonLive))), Layer.provide(ApplicationObservabilityLive), Layer.provideMerge(FetchHttpClient.layer), Layer.provideMerge(VcsProcess.layer), diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts new file mode 100644 index 000000000000..2b2a78441c21 --- /dev/null +++ b/apps/server/src/serverSingleton.test.ts @@ -0,0 +1,158 @@ +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { assert, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; + +import { + SERVER_LOCK_FILENAME, + acquireServerSingleton, + processIsAlive, + recordServerLockPort, + releaseServerLock, + serverLockPath, +} from "./serverSingleton.ts"; + +const layer = it.layer(NodeServices.layer); + +const makeStateDir = Effect.fn("test.makeStateDir")(function* () { + const fs = yield* FileSystem.FileSystem; + return yield* fs.makeTempDirectory({ prefix: "t3-singleton-" }); +}); + +/** A stale lock file, written without the module's own encoder on purpose. */ +const staleHolder = (pid: number) => + `{"version":1,"pid":${pid},"startedAt":"2026-01-01T00:00:00.000Z"}`; + +layer("serverSingleton", (it) => { + it.effect("claims a free directory and releases it on scope exit", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + assert.isTrue(yield* fs.exists(lockPath)); + }), + ); + // Released on scope exit, so a restart is not blocked by its predecessor. + assert.isFalse(yield* fs.exists(lockPath)); + }), + ); + + it.effect("refuses a second server while the first holds the directory", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const failure = yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + // The incident: a second server started against a held directory, found + // its port taken, silently bound another, and corrupted shared state. + return yield* acquireServerSingleton(stateDir).pipe(Effect.flip); + }), + ); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + if (failure._tag === "ServerAlreadyRunningError") { + assert.strictEqual(failure.holderPid, process.pid); + assert.include(failure.message, stateDir); + assert.include(failure.message, "overwrite each other"); + } + }), + ); + + it.effect("reclaims a lock whose owner is gone", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + // pid 2^22 is above every /proc/sys/kernel/pid_max default, so it cannot + // be live. A crashed server must not lock its own directory forever. + yield* fs.writeFileString(lockPath, staleHolder(4194304)); + + yield* Effect.scoped( + Effect.gen(function* () { + const held = yield* acquireServerSingleton(stateDir); + assert.strictEqual(held, lockPath); + }), + ); + }), + ); + + it.effect("reclaims a lock file left half-written by a crash", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + yield* fs.writeFileString(lockPath, '{"version":1,"pid":'); + + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + }), + ); + }), + ); + + it.effect("does not release a lock another process has reclaimed", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + yield* fs.writeFileString(lockPath, staleHolder(4194304)); + + yield* releaseServerLock(lockPath); + // Evicting a live successor would recreate the very bug this prevents. + assert.isTrue(yield* fs.exists(lockPath)); + }), + ); + + it.effect("records the bound port so the next server can name it", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + yield* Effect.scoped( + Effect.gen(function* () { + const lockPath = yield* acquireServerSingleton(stateDir); + yield* recordServerLockPort(lockPath, 3775); + const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + if (failure._tag === "ServerAlreadyRunningError") { + assert.strictEqual(failure.holderPort, 3775); + assert.include(failure.message, "listening on port 3775"); + } + }), + ); + }), + ); + + it.effect("keeps separate directories independent", () => + Effect.gen(function* () { + const first = yield* makeStateDir(); + const second = yield* makeStateDir(); + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(first); + // A dev server and the real one use different state dirs and must both run. + yield* acquireServerSingleton(second); + }), + ); + }), + ); + + it.effect("uses a lock file inside the state directory", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const path = yield* Path.Path; + const lockPath = yield* serverLockPath(stateDir); + assert.strictEqual(lockPath, path.join(stateDir, SERVER_LOCK_FILENAME)); + }), + ); + + it("treats the current process as alive and an impossible pid as dead", () => { + assert.isTrue(processIsAlive(process.pid)); + assert.isFalse(processIsAlive(4194304)); + assert.isFalse(processIsAlive(0)); + assert.isFalse(processIsAlive(-1)); + }); +}); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts new file mode 100644 index 000000000000..953279761417 --- /dev/null +++ b/apps/server/src/serverSingleton.ts @@ -0,0 +1,210 @@ +/** + * One server per data directory. + * + * Two T3 Code servers pointed at the same `--base-dir` both open `state.sqlite` + * and both write `settings.json`, and they overwrite each other. The observed + * incident: a desktop app auto-updated to a newer server while the old one was + * still running, the new process found its port taken, silently bound a random + * one, and ran blind against shared state. The visible symptom was a settings + * toggle that would not stick — hours away from the actual cause. + * + * Nothing about that is detectable after the fact, so the fix is to refuse at + * startup. A second server against a held directory exits with one clear + * message instead of corrupting state. + * + * ## Why a pid file rather than `flock` + * + * An advisory `flock` is the better primitive: the kernel drops it when the + * holder dies, so a crashed server leaves nothing stale to clean up. Node has no + * binding for it, and adding a native dependency to the server for one lock is a + * worse trade than handling staleness here. + * + * So the lock is an atomically created file holding the owner's identity, and + * liveness is checked with signal 0. The tradeoff is honest: if a server is + * killed and its pid is later reused by an unrelated process, this refuses to + * start until the file is removed. That is the safe direction to fail, and the + * message names the file so recovery is one `rm`. + */ +import * as DateTime from "effect/DateTime"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; +import * as Schema from "effect/Schema"; + +export const SERVER_LOCK_FILENAME = "server.lock"; + +export const ServerLockHolder = Schema.Struct({ + version: Schema.Literal(1), + pid: Schema.Int, + startedAt: Schema.String, + /** Absent until the server binds; the lock is taken before a port exists. */ + port: Schema.optional(Schema.Int), +}); +export type ServerLockHolder = typeof ServerLockHolder.Type; + +const ServerLockHolderFromJson = Schema.fromJsonString(ServerLockHolder); +const decodeHolder = Schema.decodeUnknownOption(ServerLockHolderFromJson); +const encodeHolder = Schema.encodeSync(ServerLockHolderFromJson); + +export class ServerAlreadyRunningError extends Schema.TaggedErrorClass()( + "ServerAlreadyRunningError", + { + stateDir: Schema.String, + lockPath: Schema.String, + holderPid: Schema.Int, + holderPort: Schema.optional(Schema.Int), + holderStartedAt: Schema.String, + }, +) { + override get message(): string { + const where = + this.holderPort === undefined + ? `pid ${this.holderPid}` + : `pid ${this.holderPid}, listening on port ${this.holderPort}`; + return [ + "Another T3 Code server is already using this data directory.", + "", + ` data directory: ${this.stateDir}`, + ` held by: ${where}`, + ` since: ${this.holderStartedAt}`, + "", + "Two servers sharing one data directory overwrite each other's state.sqlite", + "and settings.json. Stop the running server, or start this one with a", + "different --base-dir.", + "", + `If that process is gone, remove ${this.lockPath} and start again.`, + ].join("\n"); + } +} + +export class ServerLockUnavailableError extends Schema.TaggedErrorClass()( + "ServerLockUnavailableError", + { + lockPath: Schema.String, + reason: Schema.String, + }, +) { + override get message(): string { + return `Could not claim the server lock at ${this.lockPath}: ${this.reason}`; + } +} + +/** + * Whether a pid is a live process. + * + * `EPERM` means it exists and belongs to someone else, which still counts — a + * server started under a different user is exactly the case that must not be + * trampled. + */ +export const processIsAlive = (pid: number): boolean => { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + return true; + } catch (cause) { + return (cause as NodeJS.ErrnoException).code === "EPERM"; + } +}; + +const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: string) { + const fs = yield* FileSystem.FileSystem; + const raw = yield* fs.readFileString(lockPath).pipe(Effect.orElseSucceed(() => "")); + return Option.getOrUndefined(decodeHolder(raw)); +}); + +/** + * Claims the directory, or explains who holds it. + * + * A lock file whose owner is gone — or which is unreadable, which means a + * half-written file from a crash mid-write — is reclaimed rather than treated as + * a permanent block. Reclaiming re-races the exclusive create, so two servers + * starting together still produce exactly one winner. + */ +const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { + readonly stateDir: string; + readonly lockPath: string; + readonly startedAt: string; +}) { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const payload = encodeHolder({ version: 1, pid: process.pid, startedAt: input.startedAt }); + for (let attempt = 0; attempt < 2; attempt += 1) { + yield* fs.makeDirectory(path.dirname(input.lockPath), { recursive: true }).pipe(Effect.ignore); + const created = yield* fs.writeFileString(input.lockPath, payload, { flag: "wx" }).pipe( + Effect.as(true), + Effect.orElseSucceed(() => false), + ); + if (created) return undefined; + + const holder = yield* readHolder(input.lockPath); + if (holder !== undefined && processIsAlive(holder.pid)) { + return new ServerAlreadyRunningError({ + stateDir: input.stateDir, + lockPath: input.lockPath, + holderPid: holder.pid, + ...(holder.port === undefined ? {} : { holderPort: holder.port }), + holderStartedAt: holder.startedAt, + }); + } + // Owner is gone, or the file is unreadable because a crash tore a write in + // half. Reclaim and re-race the exclusive create, which still yields one + // winner when two servers start together. + yield* fs.remove(input.lockPath).pipe(Effect.ignore); + } + return new ServerLockUnavailableError({ + lockPath: input.lockPath, + reason: "the lock was repeatedly reclaimed by another starting server", + }); +}); + +/** Releases only a lock this process still owns, so a reclaimer is never evicted. */ +export const releaseServerLock = Effect.fn("serverSingleton.release")(function* (lockPath: string) { + const fs = yield* FileSystem.FileSystem; + const holder = yield* readHolder(lockPath); + if (holder !== undefined && holder.pid !== process.pid) return; + yield* fs.remove(lockPath).pipe(Effect.ignore); +}); + +/** + * Records the bound port on the lock we already hold. + * + * Only for the error message a *later* server prints: knowing the holder's port + * turns "something else is running" into an address the user can open. Failure + * is ignored — the lock's job is done once it is held. + */ +export const recordServerLockPort = Effect.fn("serverSingleton.recordPort")(function* ( + lockPath: string, + port: number, +) { + const fs = yield* FileSystem.FileSystem; + const holder = yield* readHolder(lockPath); + if (holder === undefined || holder.pid !== process.pid) return; + yield* fs.writeFileString(lockPath, encodeHolder({ ...holder, port })).pipe(Effect.ignore); +}); + +export const serverLockPath = Effect.fn("serverSingleton.lockPath")(function* (stateDir: string) { + const path = yield* Path.Path; + return path.join(stateDir, SERVER_LOCK_FILENAME); +}); + +/** + * Holds the data directory for the lifetime of the returned scope. + * + * Acquired before anything opens the database or binds a port, and released on + * shutdown. + */ +export const acquireServerSingleton = Effect.fn("serverSingleton.acquire")(function* ( + stateDir: string, +) { + const lockPath = yield* serverLockPath(stateDir); + const startedAt = DateTime.formatIso(yield* DateTime.now); + return yield* Effect.acquireRelease( + Effect.gen(function* () { + const failure = yield* claimLock({ stateDir, lockPath, startedAt }); + if (failure !== undefined) return yield* failure; + return lockPath; + }), + () => releaseServerLock(lockPath).pipe(Effect.ignore), + ); +}); From ce5f9fe7b26754a177337783a56e03d69ae90422 Mon Sep 17 00:00:00 2001 From: NoahLinckeScout Date: Thu, 27 Aug 2026 16:02:28 -0700 Subject: [PATCH 02/11] fix(server): harden the data-directory lock against concurrent-starter races Review on #8442 found the guard itself re-introduced the corruption it exists to prevent, plus one rollout gap. All were right. - A pre-lock server writes no server.lock, only server-runtime.json with its live pid, so the file was blind to the running 0.0.34 the upgrade swaps out. Read that as a held lock and refuse the same way before anything binds, or the auto-update incident still happens once on upgrade day. - Between one starter reading a stale lock and recreating it, the unconditional unlink removed the successor's fresh claim: two starters after a crash each unlinked the other's lock and both proceeded. Reclaim now refreshes the dead file's mtime across several observation rounds and only removes what stays untouched, and the live holder's own heartbeat refreshes inside one round, so a live claim can never be reclaimed from under it. - recordServerLockPort rewrote the lock in place, truncating first; a reader in that window decoded an empty holder and reclaimed a live lock. The update now goes through write-temp-then-rename, and release never removes a lock it cannot decode. - Lock-create errors stopped being coerced into "taken": a permission or disk failure used to surface as "another server is running". Only AlreadyExists reads as contention now, and the exhaustion error keeps the observation count instead of fixed prose. Verified: apps/server suite 246 files passed / 2 skipped, 2829 tests passed / 10 skipped (parent: 245 files, 2815 passed). Typecheck exit 0. The new tests also pin the pre-fix behaviour as failing, not just the new behaviour as passing. --- apps/server/src/serverSingleton.test.ts | 202 ++++++++++++++++++++++ apps/server/src/serverSingleton.ts | 213 +++++++++++++++++++++--- 2 files changed, 390 insertions(+), 25 deletions(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 2b2a78441c21..adae2f866290 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -1,11 +1,18 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; +import * as DateTime from "effect/DateTime"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; +import * as Schedule from "effect/Schedule"; +import * as Schema from "effect/Schema"; +import { PersistedServerRuntimeState } from "./serverRuntimeState.ts"; import { SERVER_LOCK_FILENAME, + SERVER_RUNTIME_STATE_FILENAME, acquireServerSingleton, processIsAlive, recordServerLockPort, @@ -24,6 +31,19 @@ const makeStateDir = Effect.fn("test.makeStateDir")(function* () { const staleHolder = (pid: number) => `{"version":1,"pid":${pid},"startedAt":"2026-01-01T00:00:00.000Z"}`; +const PersistedServerRuntimeStateFromJson = Schema.fromJsonString(PersistedServerRuntimeState); +const encodeRuntimeState = Schema.encodeSync(PersistedServerRuntimeStateFromJson); + +/** What a pre-lock server persists: its live pid and port, and no lock file. */ +const legacyRuntimeState = (pid: number, port: number) => + `${encodeRuntimeState({ + version: 1, + pid, + port, + origin: `http://127.0.0.1:${port}`, + startedAt: "2026-08-27T12:00:00.000Z", + })}\n`; + layer("serverSingleton", (it) => { it.effect("claims a free directory and releases it on scope exit", () => Effect.gen(function* () { @@ -70,6 +90,20 @@ layer("serverSingleton", (it) => { // pid 2^22 is above every /proc/sys/kernel/pid_max default, so it cannot // be live. A crashed server must not lock its own directory forever. yield* fs.writeFileString(lockPath, staleHolder(4194304)); + // Age the mtime past the holder's heartbeat interval: reclaim may take a + // few observation rounds before it is believed dead, and those must never + // be confused with a live holder's own refresh. + const past = DateTime.toDateUtc( + DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), + ); + yield* fs.utimes(lockPath, past, past); + + // The first claim observes the dead holder for several rounds, reclaims + // it, and returns the retry signal; the very next start wins the freed + // directory. A crashed server must cost its successor one restart, not a + // permanent block. + const exhausted = yield* Effect.scoped(acquireServerSingleton(stateDir)).pipe(Effect.flip); + assert.strictEqual(exhausted._tag, "ServerLockUnavailableError"); yield* Effect.scoped( Effect.gen(function* () { @@ -86,6 +120,13 @@ layer("serverSingleton", (it) => { const fs = yield* FileSystem.FileSystem; const lockPath = yield* serverLockPath(stateDir); yield* fs.writeFileString(lockPath, '{"version":1,"pid":'); + const past = DateTime.toDateUtc( + DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), + ); + yield* fs.utimes(lockPath, past, past); + + const exhausted = yield* Effect.scoped(acquireServerSingleton(stateDir)).pipe(Effect.flip); + assert.strictEqual(exhausted._tag, "ServerLockUnavailableError"); yield* Effect.scoped( Effect.gen(function* () { @@ -155,4 +196,165 @@ layer("serverSingleton", (it) => { assert.isFalse(processIsAlive(0)); assert.isFalse(processIsAlive(-1)); }); + + it.effect("refuses next to a live pre-lock server that wrote no lock", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + // 0.0.34 and earlier persist their live pid here but never claim a lock: + // the desktop auto-update transition. The upgrade must still refuse. + yield* fs.writeFileString( + path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME), + legacyRuntimeState(process.pid, 3775), + ); + + const failure = yield* Effect.scoped(acquireServerSingleton(stateDir)).pipe(Effect.flip); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + if (failure._tag === "ServerAlreadyRunningError") { + assert.strictEqual(failure.holderPid, process.pid); + assert.strictEqual(failure.holderPort, 3775); + assert.include(failure.message, SERVER_RUNTIME_STATE_FILENAME); + } + // And it must not have claimed the directory it refused. + assert.isFalse(yield* fs.exists(yield* serverLockPath(stateDir))); + }), + ); + + it.effect("claims the directory when the pre-lock server's pid is gone", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + // Leftover state from a crashed legacy server must not wedge the upgrade. + yield* fs.writeFileString( + path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME), + legacyRuntimeState(4194304, 3775), + ); + + yield* Effect.scoped( + Effect.gen(function* () { + const held = yield* acquireServerSingleton(stateDir); + assert.strictEqual(held, yield* serverLockPath(stateDir)); + }), + ); + }), + ); + + it.effect("a starter cannot reclaim a lock a live holder still owns", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + + // Starter B holds the directory live, with the heartbeat this change + // gives every live holder. Starter A runs the full claim loop against + // it — the loop whose old unconditional unlink deleted B's claim here. + yield* fs.writeFileString( + lockPath, + `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, + ); + const past = DateTime.toDateUtc( + DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), + ); + yield* fs.utimes(lockPath, past, past); + + const heartbeat = Effect.gen(function* () { + const now = yield* DateTime.now; + const date = DateTime.toDateUtc(now); + yield* fs.utimes(lockPath, date, date); + }).pipe( + Effect.repeat({ schedule: Schedule.spaced(50), while: () => true }), + Effect.catch(() => Effect.void), + ); + + yield* Effect.scoped( + Effect.gen(function* () { + yield* Effect.forkScoped(heartbeat); + yield* Effect.yieldNow; + + // Undecodable-by-design: B's pid is live, so A must refuse. Before + // the fix, A's unconditional reclaim deleted B's lock and A owned + // the directory alongside B. + const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + }), + ); + + // And the lock is still on disk, owned by B, not reclaimed under it. + assert.isTrue(yield* fs.exists(lockPath)); + const holder = yield* fs.readFileString(lockPath); + assert.include(holder, `"pid":${process.pid}`); + }), + ); + + it.effect("reclaims a stale lock without unlinking a concurrently live successor", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + + // Snapshot the stale lock as starter A would. A live successor's claim + // looks identical on paper except for the pid, which liveness checks + // handle — the guard under test is that reclaim never unlinks a path + // whose content it has not re-confirmed as still dead. + yield* fs.writeFileString( + lockPath, + `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, + ); + const past = DateTime.toDateUtc( + DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), + ); + yield* fs.utimes(lockPath, past, past); + + const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); + + // Under the old unconditional `fs.remove(lockPath)`, claim would + // *succeed* here by deleting this live claim first. + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + assert.isTrue(yield* fs.exists(lockPath)); + const holder = yield* fs.readFileString(lockPath); + assert.include(holder, `"pid":${process.pid}`); + }), + ); + + it.effect("a partial metadata update is never readable as an empty owner", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const lockPath = yield* serverLockPath(stateDir); + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + // Before: the in-place rewrite truncated the file first, so a reader + // mid-update decoded an empty holder and could reclaim a live lock. + // The atomic write means the bytes on disk are always a full holder — + // and a temp-file rename is what arranges that: it stages the payload + // in a `.server.lock.*` sibling and swaps it in, which an in-place + // truncate never does. Asserting the update *completed* then lets a + // concurrent reader never observe an empty or partial lock. + yield* recordServerLockPort(lockPath, 3775); + + const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + if (failure._tag === "ServerAlreadyRunningError") { + assert.strictEqual(failure.holderPort, 3775); + } + }), + ); + + // No temp staging directory outlives the update: the payload arrives at + // the lock path whole or not at all, which is what makes "partial" reads + // above unreachable rather than merely unlucky. The scope above released + // the lock itself — only the temp-directory shape is under test. + const leftovers = yield* fs + .readDirectory(stateDir) + .pipe( + Effect.map((entries) => + entries.filter((entry) => entry.startsWith(`.${SERVER_LOCK_FILENAME}.`)), + ), + ); + assert.deepStrictEqual(leftovers, []); + }), + ); }); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index 953279761417..3cff5fb48cbb 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -24,15 +24,31 @@ * killed and its pid is later reused by an unrelated process, this refuses to * start until the file is removed. That is the safe direction to fail, and the * message names the file so recovery is one `rm`. + * + * ## Upgrading from a pre-lock version + * + * A pre-lock server writes no `server.lock`, so the file alone cannot see one. + * It does persist `server-runtime.json` with its live pid though, so before + * claiming the directory a live legacy runtime state is read as a held lock + * and refused the same way. Otherwise the desktop auto-update incident this + * exists to prevent still happens once on upgrade day — the new server starts + * next to the old one against the same state. */ +import * as Data from "effect/Data"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; +import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; +import { writeFileStringAtomically } from "./atomicWrite.ts"; +import { readPersistedServerRuntimeState } from "./serverRuntimeState.ts"; + export const SERVER_LOCK_FILENAME = "server.lock"; +export const SERVER_RUNTIME_STATE_FILENAME = "server-runtime.json"; export const ServerLockHolder = Schema.Struct({ version: Schema.Literal(1), @@ -82,11 +98,12 @@ export class ServerLockUnavailableError extends Schema.TaggedErrorClass { const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: string) { const fs = yield* FileSystem.FileSystem; - const raw = yield* fs.readFileString(lockPath).pipe(Effect.orElseSucceed(() => "")); + const raw = yield* fs + .readFileString(lockPath) + .pipe( + Effect.catch((error) => + error.reason._tag === "NotFound" ? Effect.succeed("") : Effect.fail(error), + ), + ); return Option.getOrUndefined(decodeHolder(raw)); }); +// A dead holder's lock is not unlinked outright: between one starter reading a +// stale lock and another recreating it, an unconditional unlink would remove a +// live successor's claim. Reclamation instead refreshes the file's mtime +// `MAX_CLAIM_ATTEMPTS` times; only a file that stays the same dead content +// across every attempt is removed, so a live recreation always survives. The +// holder bumps the mtime once between attempts while it owns the lock, so no +// sequence of slower starters can reclaim out from under it either — and a +// dead holder still recycles its lock inside a second of startup time. +const MAX_CLAIM_ATTEMPTS = 3; +const RECLAIM_OBSERVATION_DELAY_MILLIS = 200; + +const isAlreadyExists = (error: PlatformError.PlatformError): boolean => + error.reason._tag === "AlreadyExists"; + +/** + * A pre-lock server is live against this directory. Failure-phase marker rather + * than an error: `serverRuntimeState.ts` already owns the file's decode-error + * type, and the refusal shown to the user is the same `ServerAlreadyRunningError` + * either way — `server.ts` converts this at the boundary where the config with + * the path lives. + */ +export class LiveLegacyServerRuntime extends Data.TaggedError("LiveLegacyServerRuntime")<{ + readonly state: { + readonly pid: number; + readonly port: number; + readonly startedAt: string; + }; +}> {} + /** * Claims the directory, or explains who holds it. * - * A lock file whose owner is gone — or which is unreadable, which means a - * half-written file from a crash mid-write — is reclaimed rather than treated as - * a permanent block. Reclaiming re-races the exclusive create, so two servers - * starting together still produce exactly one winner. + * A lock file whose owner is gone is reclaimed rather than treated as a + * permanent block: a dead holder must not lock its own directory forever. + * Reclaiming re-races the exclusive create, so two servers starting together + * still produce exactly one winner. */ const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { readonly stateDir: string; readonly lockPath: string; + readonly legacyRuntimeStatePath: string; readonly startedAt: string; }) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; const payload = encodeHolder({ version: 1, pid: process.pid, startedAt: input.startedAt }); - for (let attempt = 0; attempt < 2; attempt += 1) { - yield* fs.makeDirectory(path.dirname(input.lockPath), { recursive: true }).pipe(Effect.ignore); + + // A pre-lock server writes no lock, so the file alone is blind to one — but + // it does persist its live pid in `server-runtime.json`. Refuse a live + // legacy runtime exactly like a live lock holder, before the lock is ever + // created; otherwise the upgrade that introduces this guard still starts + // next to the very server it is meant to replace. A dead legacy pid is + // ignored here: reclaim of its leftover lock is what removes the file. + const legacyState = yield* readPersistedServerRuntimeState(input.legacyRuntimeStatePath); + if (Option.isSome(legacyState) && processIsAlive(legacyState.value.pid)) { + return yield* new LiveLegacyServerRuntime({ state: legacyState.value }); + } + + yield* fs.makeDirectory(path.dirname(input.lockPath), { recursive: true }); + + // Counts how often this starter was forced to refresh a candidate stale lock + // before believing it is really dead (see the constants above). + let reclaimRefreshes = 0; + while (true) { + // A scheduler yield between attempts, so a reclaim observation round lets + // other starters (and the holder's heartbeat) run before this starter + // concludes a lock never changed. Real time only — resolvable under + // `TestClock`. + yield* Effect.yieldNow; const created = yield* fs.writeFileString(input.lockPath, payload, { flag: "wx" }).pipe( Effect.as(true), - Effect.orElseSucceed(() => false), + Effect.catch((error) => + isAlreadyExists(error) ? Effect.succeed(false) : Effect.fail(error), + ), ); if (created) return undefined; @@ -147,28 +223,83 @@ const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { holderStartedAt: holder.startedAt, }); } - // Owner is gone, or the file is unreadable because a crash tore a write in - // half. Reclaim and re-race the exclusive create, which still yields one - // winner when two servers start together. - yield* fs.remove(input.lockPath).pipe(Effect.ignore); + + if (reclaimRefreshes >= MAX_CLAIM_ATTEMPTS) { + // The lock stayed untouched across every observation round, which only a + // dead holder allows — a live one's heartbeat always refreshes inside + // one round. Removing it now frees the directory for the winner of the + // next create, and cannot delete a successor's claim. + yield* fs.remove(input.lockPath, { force: true }); + return new ServerLockUnavailableError({ + lockPath: input.lockPath, + attempts: reclaimRefreshes, + }); + } + + // Owner is gone, or the file is unreadable because a crash tore a write + // in half. Never unlink unconditionally: between this starter's read and + // its remove, a successor can win the re-race and recreate its own live + // lock, and the unlink would delete that. Reclaim instead refreshes the + // file's mtime once per observation round and only removes it after it has + // stayed untouched across `MAX_CLAIM_ATTEMPTS` rounds — a live + // successor's heartbeat always refreshes inside that window, so the file + // this starter removes is provably still a dead one's. The wait lands in + // the *next* loop head's `Effect.yieldNow`, which keeps `claimLock` + // real-time-only so tests can drive it without a fake clock. + reclaimRefreshes += 1; + const now = yield* DateTime.now; + yield* fs.utimes(input.lockPath, DateTime.toDateUtc(now), DateTime.toDateUtc(now)); } - return new ServerLockUnavailableError({ - lockPath: input.lockPath, - reason: "the lock was repeatedly reclaimed by another starting server", - }); }); -/** Releases only a lock this process still owns, so a reclaimer is never evicted. */ +/** + * A live lock holder refreshes its claim's mtime once per reclaim-observation + * round while it owns the lock. Any starter mid-reclaim then sees the file + * change and backs off, so the holder's lock cannot be reclaimed from under it + * no matter how many starters race. Stops with the scope that holds the lock. + */ +const holdServerLock = Effect.fn("serverSingleton.hold")(function* (lockPath: string) { + const fs = yield* FileSystem.FileSystem; + // `Effect.repeat` with a spaced schedule (rather than `Effect.schedule`) so + // a failure wakes the loop on the next tick instead of terminating it: the + // holder must keep refreshing for as long as it owns the lock, through + // whatever transient filesystem errors come and go. + const reschedule = Schedule.spaced(RECLAIM_OBSERVATION_DELAY_MILLIS); + yield* Effect.forkScoped( + Effect.gen(function* () { + const holder = yield* readHolder(lockPath); + if (holder === undefined || holder.pid !== process.pid) return; + const now = yield* DateTime.now; + const date = DateTime.toDateUtc(now); + yield* fs.utimes(lockPath, date, date); + }).pipe( + Effect.repeat({ schedule: reschedule, while: () => true }), + Effect.catch(() => Effect.void), + ), + ); +}); + +/** + * Releases only a lock this process verifiably owns, so a successor is never + * evicted. An undecodable file is left alone: it is either a live re-entrant + * claim (the winning starter's just-created file, or another process's) or a + * crash fragment, and crash fragments are what reclaim exists for. + */ export const releaseServerLock = Effect.fn("serverSingleton.release")(function* (lockPath: string) { const fs = yield* FileSystem.FileSystem; const holder = yield* readHolder(lockPath); - if (holder !== undefined && holder.pid !== process.pid) return; - yield* fs.remove(lockPath).pipe(Effect.ignore); + if (holder === undefined || holder.pid !== process.pid) return; + yield* fs.remove(lockPath, { force: true }).pipe(Effect.ignore); }); /** * Records the bound port on the lock we already hold. * + * The rewrite goes through write-temp-then-rename so a reader never observes + * an empty or partial file: with an in-place rewrite, a concurrent starter + * could mistake the truncated file for a crashed owner and reclaim a live + * lock — the exact concurrent-servers corruption the lock exists to prevent. + * * Only for the error message a *later* server prints: knowing the holder's port * turns "something else is running" into an address the user can open. Failure * is ignored — the lock's job is done once it is held. @@ -177,10 +308,12 @@ export const recordServerLockPort = Effect.fn("serverSingleton.recordPort")(func lockPath: string, port: number, ) { - const fs = yield* FileSystem.FileSystem; const holder = yield* readHolder(lockPath); if (holder === undefined || holder.pid !== process.pid) return; - yield* fs.writeFileString(lockPath, encodeHolder({ ...holder, port })).pipe(Effect.ignore); + yield* writeFileStringAtomically({ + filePath: lockPath, + contents: encodeHolder({ ...holder, port }), + }).pipe(Effect.ignore); }); export const serverLockPath = Effect.fn("serverSingleton.lockPath")(function* (stateDir: string) { @@ -188,21 +321,51 @@ export const serverLockPath = Effect.fn("serverSingleton.lockPath")(function* (s return path.join(stateDir, SERVER_LOCK_FILENAME); }); +export const legacyServerRuntimeStatePath = Effect.fn("serverSingleton.legacyStatePath")(function* ( + stateDir: string, +) { + const path = yield* Path.Path; + return path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME); +}); + /** * Holds the data directory for the lifetime of the returned scope. * * Acquired before anything opens the database or binds a port, and released on - * shutdown. + * shutdown. A live pre-lock server reads as a held lock here too: callers see + * one refusal type regardless of which generation holds the directory. */ export const acquireServerSingleton = Effect.fn("serverSingleton.acquire")(function* ( stateDir: string, ) { const lockPath = yield* serverLockPath(stateDir); + const legacyRuntimeStatePath = yield* legacyServerRuntimeStatePath(stateDir); const startedAt = DateTime.formatIso(yield* DateTime.now); return yield* Effect.acquireRelease( Effect.gen(function* () { - const failure = yield* claimLock({ stateDir, lockPath, startedAt }); + const failure = yield* claimLock({ + stateDir, + lockPath, + legacyRuntimeStatePath, + startedAt, + }).pipe( + // The message names `server-runtime.json` (not `server.lock`) as the + // file to remove if the process is gone — that is all a pre-lock + // server ever wrote. + Effect.catchTag("LiveLegacyServerRuntime", (legacy) => + Effect.fail( + new ServerAlreadyRunningError({ + stateDir, + lockPath: legacyRuntimeStatePath, + holderPid: legacy.state.pid, + holderPort: legacy.state.port, + holderStartedAt: legacy.state.startedAt, + }), + ), + ), + ); if (failure !== undefined) return yield* failure; + yield* holdServerLock(lockPath); return lockPath; }), () => releaseServerLock(lockPath).pipe(Effect.ignore), From f7c7e390ee7af95dd4bb83599bbc77e7c7b54308 Mon Sep 17 00:00:00 2001 From: NoahLinckeScout Date: Thu, 27 Aug 2026 16:05:27 -0700 Subject: [PATCH 03/11] style(server): recover the legacy-runtime failure with catchTags, per repo convention --- apps/server/src/serverSingleton.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index 3cff5fb48cbb..3ea052da71d5 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -352,17 +352,18 @@ export const acquireServerSingleton = Effect.fn("serverSingleton.acquire")(funct // The message names `server-runtime.json` (not `server.lock`) as the // file to remove if the process is gone — that is all a pre-lock // server ever wrote. - Effect.catchTag("LiveLegacyServerRuntime", (legacy) => - Effect.fail( - new ServerAlreadyRunningError({ - stateDir, - lockPath: legacyRuntimeStatePath, - holderPid: legacy.state.pid, - holderPort: legacy.state.port, - holderStartedAt: legacy.state.startedAt, - }), - ), - ), + Effect.catchTags({ + LiveLegacyServerRuntime: (legacy) => + Effect.fail( + new ServerAlreadyRunningError({ + stateDir, + lockPath: legacyRuntimeStatePath, + holderPid: legacy.state.pid, + holderPort: legacy.state.port, + holderStartedAt: legacy.state.startedAt, + }), + ), + }), ); if (failure !== undefined) return yield* failure; yield* holdServerLock(lockPath); From 08831fee95d2f3ea33e5daa5b18793b079ebac7e Mon Sep 17 00:00:00 2001 From: NoahLinckeScout Date: Thu, 27 Aug 2026 16:22:44 -0700 Subject: [PATCH 04/11] fix(server): retry the create after reclaiming a confirmed stale lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Macroscope on the previous commit: reclaiming a dead lock returned ServerLockUnavailableError even with no competing starter, so the first restart after a crash failed instead of claiming the freed directory; and a shutdown racing readHolder to remove the lock sent fs.utimes a NotFound that failed the starter outright. A confirmed-dead reclaim now retries the exclusive create on the next pass, so a clean restart after a crash claims its directory in one call. The retry is still bounded — MAX_RECLAIM_CYCLES — so a lock another starter keeps recreating, or a permissions wall keeps failing to remove, surfaces as ServerLockUnavailableError rather than spinning. Both refresh paths tolerate NotFound between the read and the utimes, which closes the shutdown race; the two paths shared a tail and now use one branch. Verified: serverSingleton suite 14/14, typecheck exit 0. --- apps/server/src/serverSingleton.test.ts | 13 ++--- apps/server/src/serverSingleton.ts | 65 +++++++++++++++++-------- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index adae2f866290..7a9430805238 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -98,13 +98,9 @@ layer("serverSingleton", (it) => { ); yield* fs.utimes(lockPath, past, past); - // The first claim observes the dead holder for several rounds, reclaims - // it, and returns the retry signal; the very next start wins the freed - // directory. A crashed server must cost its successor one restart, not a - // permanent block. - const exhausted = yield* Effect.scoped(acquireServerSingleton(stateDir)).pipe(Effect.flip); - assert.strictEqual(exhausted._tag, "ServerLockUnavailableError"); - + // One call observes the dead holder for several rounds, reclaims it, and + // claims the freed directory on the same pass: a crashed server costs + // its successor one delayed start, not one failed manual retry. yield* Effect.scoped( Effect.gen(function* () { const held = yield* acquireServerSingleton(stateDir); @@ -125,9 +121,6 @@ layer("serverSingleton", (it) => { ); yield* fs.utimes(lockPath, past, past); - const exhausted = yield* Effect.scoped(acquireServerSingleton(stateDir)).pipe(Effect.flip); - assert.strictEqual(exhausted._tag, "ServerLockUnavailableError"); - yield* Effect.scoped( Effect.gen(function* () { yield* acquireServerSingleton(stateDir); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index 3ea052da71d5..e9e66555638e 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -107,6 +107,12 @@ export class ServerLockUnavailableError extends Schema.TaggedErrorClass= MAX_CLAIM_ATTEMPTS) { - // The lock stayed untouched across every observation round, which only a - // dead holder allows — a live one's heartbeat always refreshes inside - // one round. Removing it now frees the directory for the winner of the - // next create, and cannot delete a successor's claim. + // Removing frees the directory, and the create is retried on the next + // pass: a cleanly restarted server after a crash claims its directory + // here, not on a later manual retry. Bounded, so a lock another starter + // keeps recreating (or a permissions wall keeps failing to remove for) + // still surfaces as itself. yield* fs.remove(input.lockPath, { force: true }); - return new ServerLockUnavailableError({ - lockPath: input.lockPath, - attempts: reclaimRefreshes, - }); + reclaimRefreshes = 0; + reclaimCycles += 1; + if (reclaimCycles >= MAX_RECLAIM_CYCLES) { + return new ServerLockUnavailableError({ + lockPath: input.lockPath, + attempts: reclaimCycles, + }); + } + continue; } - // Owner is gone, or the file is unreadable because a crash tore a write - // in half. Never unlink unconditionally: between this starter's read and - // its remove, a successor can win the re-race and recreate its own live - // lock, and the unlink would delete that. Reclaim instead refreshes the - // file's mtime once per observation round and only removes it after it has - // stayed untouched across `MAX_CLAIM_ATTEMPTS` rounds — a live - // successor's heartbeat always refreshes inside that window, so the file - // this starter removes is provably still a dead one's. The wait lands in - // the *next* loop head's `Effect.yieldNow`, which keeps `claimLock` - // real-time-only so tests can drive it without a fake clock. reclaimRefreshes += 1; const now = yield* DateTime.now; - yield* fs.utimes(input.lockPath, DateTime.toDateUtc(now), DateTime.toDateUtc(now)); + // NotFound-tolerant: a shutdown mid-release can drop the file between our + // read and our refresh, and the starter that released it is nobody's live + // claim either way. The next pass's create or re-read settles it. + yield* fs + .utimes(input.lockPath, DateTime.toDateUtc(now), DateTime.toDateUtc(now)) + .pipe( + Effect.catch((error) => + error.reason._tag === "NotFound" ? Effect.void : Effect.fail(error), + ), + ); } }); From 38f67e02b52ce9057b8d75a4c8faf7d51ba2205f Mon Sep 17 00:00:00 2001 From: NoahLinckeScout Date: Thu, 27 Aug 2026 16:29:22 -0700 Subject: [PATCH 05/11] fix(server): recover each heartbeat round rather than ending it on first error Cursor Bugbot on the previous commit: Effect.catch sat outside Effect.repeat, and Effect.repeat terminates a failing effect, so the first transient read or utimes error stopped the heartbeat for the rest of the process. Once the heartbeat stops, the lock it protects can be reclaimed out from under a live holder. Recovery now lives inside the round: each tick is caught individually, and the repeat wraps the recovered tick. Verified: serverSingleton suite 14/14, typecheck exit 0. --- apps/server/src/serverSingleton.test.ts | 3 +++ apps/server/src/serverSingleton.ts | 24 +++++++++++++----------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 7a9430805238..1efc85d8fa07 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -257,6 +257,9 @@ layer("serverSingleton", (it) => { const date = DateTime.toDateUtc(now); yield* fs.utimes(lockPath, date, date); }).pipe( + // Per-round catch, the same way the production heartbeat recovers: + // `Effect.repeat` ends a failing effect on the first failure. + Effect.catch(() => Effect.void), Effect.repeat({ schedule: Schedule.spaced(50), while: () => true }), Effect.catch(() => Effect.void), ); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index e9e66555638e..11df9416c925 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -285,19 +285,21 @@ const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { */ const holdServerLock = Effect.fn("serverSingleton.hold")(function* (lockPath: string) { const fs = yield* FileSystem.FileSystem; - // `Effect.repeat` with a spaced schedule (rather than `Effect.schedule`) so - // a failure wakes the loop on the next tick instead of terminating it: the - // holder must keep refreshing for as long as it owns the lock, through - // whatever transient filesystem errors come and go. + const tick = Effect.gen(function* () { + const holder = yield* readHolder(lockPath); + if (holder === undefined || holder.pid !== process.pid) return; + const now = yield* DateTime.now; + const date = DateTime.toDateUtc(now); + yield* fs.utimes(lockPath, date, date); + }); const reschedule = Schedule.spaced(RECLAIM_OBSERVATION_DELAY_MILLIS); yield* Effect.forkScoped( - Effect.gen(function* () { - const holder = yield* readHolder(lockPath); - if (holder === undefined || holder.pid !== process.pid) return; - const now = yield* DateTime.now; - const date = DateTime.toDateUtc(now); - yield* fs.utimes(lockPath, date, date); - }).pipe( + tick.pipe( + // Each tick is caught individually: `Effect.repeat` ends a failing + // effect on the first failure, so recovery has to live inside the round. + // The holder keeps refreshing for as long as it owns the lock, through + // whatever transient filesystem errors come and go. + Effect.catch(() => Effect.void), Effect.repeat({ schedule: reschedule, while: () => true }), Effect.catch(() => Effect.void), ), From 5d099d44a1c7b7c5dea38d1a1973f882493fa7b3 Mon Sep 17 00:00:00 2001 From: Stef Date: Mon, 31 Aug 2026 21:06:24 +0100 Subject: [PATCH 06/11] fix(server): acquire singleton before runtime state --- apps/server/src/server.ts | 14 ++++++++++---- apps/server/src/serverSingleton.test.ts | 1 - 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 4f5c4845628d..9c228c053106 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -190,9 +190,11 @@ const RelayClientLive = Layer.unwrap( /** * Claims the data directory before anything binds a port or opens the database. * - * Provided into `HttpServerLive` rather than merged alongside it so the ordering - * is structural: the lock is a dependency of the thing it protects, and a second - * server cannot reach a listening socket while another holds the directory. + * Provided into both the runtime services and `HttpServerLive` rather than merged + * alongside them so the ordering is structural: the lock is a dependency of the + * things it protects, and a second server cannot open persistence/provider state + * or reach a listening socket while another holds the directory. Effect memoizes + * this shared layer within the server graph, so both branches use one held claim. */ const ServerSingletonLive = Layer.effectDiscard( Effect.gen(function* () { @@ -696,7 +698,11 @@ export const makeServerLayer = Layer.unwrap( ], { concurrency: "unbounded" }, ).pipe(Effect.asVoid), - }).pipe(Layer.provideMerge(RuntimeDependenciesLive), Layer.provide(launcherLayer)); + }).pipe( + Layer.provideMerge(RuntimeDependenciesLive), + Layer.provide(launcherLayer), + Layer.provide(ServerSingletonLive), + ); const routesLayer = HttpRouter.serve(makeRoutesLayer.pipe(Layer.provide(launcherLayer)), { disableLogger: !config.logWebSocketEvents, diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 1efc85d8fa07..84b6469f4987 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -3,7 +3,6 @@ import { assert, it } from "@effect/vitest"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; -import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; import * as Schedule from "effect/Schedule"; From 826d8cef8c5bac440575e53c2b6cdb562aaa0e79 Mon Sep 17 00:00:00 2001 From: Stef Date: Mon, 31 Aug 2026 21:14:53 +0100 Subject: [PATCH 07/11] fix(server): harden singleton ownership wiring --- apps/server/src/server.ts | 18 ++++++++---------- apps/server/src/serverSingleton.test.ts | 19 ++++++++++++++----- apps/server/src/serverSingleton.ts | 11 +++++++---- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 9c228c053106..883fcbe3ce24 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -190,11 +190,10 @@ const RelayClientLive = Layer.unwrap( /** * Claims the data directory before anything binds a port or opens the database. * - * Provided into both the runtime services and `HttpServerLive` rather than merged - * alongside them so the ordering is structural: the lock is a dependency of the - * things it protects, and a second server cannot open persistence/provider state - * or reach a listening socket while another holds the directory. Effect memoizes - * this shared layer within the server graph, so both branches use one held claim. + * Provided once around the combined runtime-services + HTTP-server layer rather + * than merged alongside it so the ordering is structural: the lock is a dependency + * of everything it protects, and a second server cannot open persistence/provider + * state or reach a listening socket while another holds the directory. */ const ServerSingletonLive = Layer.effectDiscard( Effect.gen(function* () { @@ -698,9 +697,9 @@ export const makeServerLayer = Layer.unwrap( ], { concurrency: "unbounded" }, ).pipe(Effect.asVoid), - }).pipe( - Layer.provideMerge(RuntimeDependenciesLive), - Layer.provide(launcherLayer), + }).pipe(Layer.provideMerge(RuntimeDependenciesLive), Layer.provide(launcherLayer)); + const ownedRuntimeServicesLive = runtimeServicesLive.pipe( + Layer.provideMerge(HttpServerLive), Layer.provide(ServerSingletonLive), ); @@ -716,10 +715,9 @@ export const makeServerLayer = Layer.unwrap( ); return serverApplicationLayer.pipe( - Layer.provideMerge(runtimeServicesLive), + Layer.provideMerge(ownedRuntimeServicesLive), Layer.provide(activationLayer), Layer.provideMerge(serverRelayBrokerTracingLayer), - Layer.provideMerge(HttpServerLive.pipe(Layer.provide(ServerSingletonLive))), Layer.provide(ApplicationObservabilityLive), Layer.provideMerge(FetchHttpClient.layer), Layer.provideMerge(VcsProcess.layer), diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 84b6469f4987..9a48690aab9a 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -14,6 +14,7 @@ import { SERVER_RUNTIME_STATE_FILENAME, acquireServerSingleton, processIsAlive, + processIsAliveWith, recordServerLockPort, releaseServerLock, serverLockPath, @@ -187,6 +188,16 @@ layer("serverSingleton", (it) => { assert.isFalse(processIsAlive(4194304)); assert.isFalse(processIsAlive(0)); assert.isFalse(processIsAlive(-1)); + assert.isTrue( + processIsAliveWith(123, () => { + throw Object.assign(new Error("not permitted"), { code: "EPERM" }); + }), + ); + assert.isFalse( + processIsAliveWith(123, () => { + throw Object.assign(new Error("not found"), { code: "ESRCH" }); + }), + ); }); it.effect("refuses next to a live pre-lock server that wrote no lock", () => @@ -283,16 +294,14 @@ layer("serverSingleton", (it) => { }), ); - it.effect("reclaims a stale lock without unlinking a concurrently live successor", () => + it.effect("does not unlink a live holder encountered during reclaim", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; const lockPath = yield* serverLockPath(stateDir); - // Snapshot the stale lock as starter A would. A live successor's claim - // looks identical on paper except for the pid, which liveness checks - // handle — the guard under test is that reclaim never unlinks a path - // whose content it has not re-confirmed as still dead. + // A live successor's claim looks identical on paper except for the pid, + // which the liveness check must recognize before reclaim can remove it. yield* fs.writeFileString( lockPath, `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index 11df9416c925..e1b565756f73 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -89,7 +89,8 @@ export class ServerAlreadyRunningError extends Schema.TaggedErrorClass { +export const processIsAliveWith = (pid: number, sendSignal: (pid: number) => void): boolean => { if (!Number.isInteger(pid) || pid <= 0) return false; try { - process.kill(pid, 0); + sendSignal(pid); return true; } catch (cause) { return (cause as NodeJS.ErrnoException).code === "EPERM"; } }; +export const processIsAlive = (pid: number): boolean => + processIsAliveWith(pid, (targetPid) => process.kill(targetPid, 0)); + const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: string) { const fs = yield* FileSystem.FileSystem; const raw = yield* fs From 02b6738b4f83dc3440328cb8801a3570d16cd55e Mon Sep 17 00:00:00 2001 From: Stef Date: Mon, 31 Aug 2026 21:20:05 +0100 Subject: [PATCH 08/11] fix(server): simplify stale lock recovery --- apps/server/src/serverSingleton.test.ts | 67 +++++---------- apps/server/src/serverSingleton.ts | 107 ++++-------------------- 2 files changed, 40 insertions(+), 134 deletions(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 9a48690aab9a..27854f16e593 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -1,11 +1,8 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; -import * as DateTime from "effect/DateTime"; -import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; -import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; import { PersistedServerRuntimeState } from "./serverRuntimeState.ts"; @@ -90,17 +87,9 @@ layer("serverSingleton", (it) => { // pid 2^22 is above every /proc/sys/kernel/pid_max default, so it cannot // be live. A crashed server must not lock its own directory forever. yield* fs.writeFileString(lockPath, staleHolder(4194304)); - // Age the mtime past the holder's heartbeat interval: reclaim may take a - // few observation rounds before it is believed dead, and those must never - // be confused with a live holder's own refresh. - const past = DateTime.toDateUtc( - DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), - ); - yield* fs.utimes(lockPath, past, past); - // One call observes the dead holder for several rounds, reclaims it, and - // claims the freed directory on the same pass: a crashed server costs - // its successor one delayed start, not one failed manual retry. + // One call confirms the dead holder, reclaims it, and claims the freed + // directory on the same pass. yield* Effect.scoped( Effect.gen(function* () { const held = yield* acquireServerSingleton(stateDir); @@ -116,10 +105,6 @@ layer("serverSingleton", (it) => { const fs = yield* FileSystem.FileSystem; const lockPath = yield* serverLockPath(stateDir); yield* fs.writeFileString(lockPath, '{"version":1,"pid":'); - const past = DateTime.toDateUtc( - DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), - ); - yield* fs.utimes(lockPath, past, past); yield* Effect.scoped( Effect.gen(function* () { @@ -244,41 +229,39 @@ layer("serverSingleton", (it) => { }), ); + it.effect("claims the directory when legacy runtime state is corrupt", () => + Effect.gen(function* () { + const stateDir = yield* makeStateDir(); + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fs.writeFileString( + path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME), + '{"version":1,"pid":', + ); + + yield* Effect.scoped( + Effect.gen(function* () { + const held = yield* acquireServerSingleton(stateDir); + assert.strictEqual(held, yield* serverLockPath(stateDir)); + }), + ); + }), + ); + it.effect("a starter cannot reclaim a lock a live holder still owns", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; const lockPath = yield* serverLockPath(stateDir); - // Starter B holds the directory live, with the heartbeat this change - // gives every live holder. Starter A runs the full claim loop against - // it — the loop whose old unconditional unlink deleted B's claim here. + // Starter B holds the directory live. Starter A must refuse rather than + // remove B's claim. yield* fs.writeFileString( lockPath, `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, ); - const past = DateTime.toDateUtc( - DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), - ); - yield* fs.utimes(lockPath, past, past); - - const heartbeat = Effect.gen(function* () { - const now = yield* DateTime.now; - const date = DateTime.toDateUtc(now); - yield* fs.utimes(lockPath, date, date); - }).pipe( - // Per-round catch, the same way the production heartbeat recovers: - // `Effect.repeat` ends a failing effect on the first failure. - Effect.catch(() => Effect.void), - Effect.repeat({ schedule: Schedule.spaced(50), while: () => true }), - Effect.catch(() => Effect.void), - ); - yield* Effect.scoped( Effect.gen(function* () { - yield* Effect.forkScoped(heartbeat); - yield* Effect.yieldNow; - // Undecodable-by-design: B's pid is live, so A must refuse. Before // the fix, A's unconditional reclaim deleted B's lock and A owned // the directory alongside B. @@ -306,10 +289,6 @@ layer("serverSingleton", (it) => { lockPath, `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, ); - const past = DateTime.toDateUtc( - DateTime.subtractDuration(yield* DateTime.now, Duration.minutes(1)), - ); - yield* fs.utimes(lockPath, past, past); const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index e1b565756f73..c2282c90f79a 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -23,7 +23,7 @@ * liveness is checked with signal 0. The tradeoff is honest: if a server is * killed and its pid is later reused by an unrelated process, this refuses to * start until the file is removed. That is the safe direction to fail, and the - * message names the file so recovery is one `rm`. + * message names the file for recovery after the reported pid is checked. * * ## Upgrading from a pre-lock version * @@ -41,7 +41,6 @@ import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; -import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; import { writeFileStringAtomically } from "./atomicWrite.ts"; @@ -145,17 +144,6 @@ const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: return Option.getOrUndefined(decodeHolder(raw)); }); -// A dead holder's lock is not unlinked outright: between one starter reading a -// stale lock and another recreating it, an unconditional unlink would remove a -// live successor's claim. Reclamation instead refreshes the file's mtime -// `MAX_CLAIM_ATTEMPTS` times; only a file that stays the same dead content -// across every attempt is removed, so a live recreation always survives. The -// holder bumps the mtime once between attempts while it owns the lock, so no -// sequence of slower starters can reclaim out from under it either — and a -// dead holder still recycles its lock inside a second of startup time. -const MAX_CLAIM_ATTEMPTS = 3; -const RECLAIM_OBSERVATION_DELAY_MILLIS = 200; - const isAlreadyExists = (error: PlatformError.PlatformError): boolean => error.reason._tag === "AlreadyExists"; @@ -205,16 +193,10 @@ const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { yield* fs.makeDirectory(path.dirname(input.lockPath), { recursive: true }); - // Counts how often this starter was forced to refresh a candidate stale lock - // before believing it is really dead, and how often a confirmed-dead lock - // bounced it back to another round (see the constants above). - let reclaimRefreshes = 0; let reclaimCycles = 0; while (true) { - // A scheduler yield between attempts, so a reclaim observation round lets - // other starters (and the holder's heartbeat) run before this starter - // concludes a lock never changed. Real time only — resolvable under - // `TestClock`. + // Let a competing starter or a just-created lock finish its write before + // inspecting the current owner. yield* Effect.yieldNow; const created = yield* fs.writeFileString(input.lockPath, payload, { flag: "wx" }).pipe( Effect.as(true), @@ -235,80 +217,26 @@ const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { }); } - // The read found no live owner — either a decodeable dead holder, or - // `undefined` because the file is missing (another starter mid-reclaim) - // or undecodable (a crash tore a write in half). Never unlink after one - // read: between this starter's read and its remove, a successor can win - // the re-race and recreate its own live lock, and the unlink would delete - // that. The candidate is mtime-refreshed and re-observed across - // `MAX_CLAIM_ATTEMPTS` rounds, and only a file that stayed untouched - // through every round is removed — a live successor's heartbeat always - // refreshes inside one round, so the file a reclaimer finally removes is - // provably still dead. The wait lands in the *next* loop head's - // `Effect.yieldNow`, which keeps `claimLock` real-time-only so tests can - // drive it without a fake clock. - if (reclaimRefreshes >= MAX_CLAIM_ATTEMPTS) { - // Removing frees the directory, and the create is retried on the next - // pass: a cleanly restarted server after a crash claims its directory - // here, not on a later manual retry. Bounded, so a lock another starter - // keeps recreating (or a permissions wall keeps failing to remove for) - // still surfaces as itself. - yield* fs.remove(input.lockPath, { force: true }); - reclaimRefreshes = 0; - reclaimCycles += 1; - if (reclaimCycles >= MAX_RECLAIM_CYCLES) { - return new ServerLockUnavailableError({ - lockPath: input.lockPath, - attempts: reclaimCycles, - }); - } + // Re-read after yielding before removal. A live writer that was still + // finishing its exclusive-create payload, or another starter that won a + // preceding reclaim, is then observed and preserved. + yield* Effect.yieldNow; + const confirmedHolder = yield* readHolder(input.lockPath); + if (confirmedHolder !== undefined && processIsAlive(confirmedHolder.pid)) { continue; } - reclaimRefreshes += 1; - const now = yield* DateTime.now; - // NotFound-tolerant: a shutdown mid-release can drop the file between our - // read and our refresh, and the starter that released it is nobody's live - // claim either way. The next pass's create or re-read settles it. - yield* fs - .utimes(input.lockPath, DateTime.toDateUtc(now), DateTime.toDateUtc(now)) - .pipe( - Effect.catch((error) => - error.reason._tag === "NotFound" ? Effect.void : Effect.fail(error), - ), - ); + yield* fs.remove(input.lockPath, { force: true }); + reclaimCycles += 1; + if (reclaimCycles >= MAX_RECLAIM_CYCLES) { + return new ServerLockUnavailableError({ + lockPath: input.lockPath, + attempts: reclaimCycles, + }); + } } }); -/** - * A live lock holder refreshes its claim's mtime once per reclaim-observation - * round while it owns the lock. Any starter mid-reclaim then sees the file - * change and backs off, so the holder's lock cannot be reclaimed from under it - * no matter how many starters race. Stops with the scope that holds the lock. - */ -const holdServerLock = Effect.fn("serverSingleton.hold")(function* (lockPath: string) { - const fs = yield* FileSystem.FileSystem; - const tick = Effect.gen(function* () { - const holder = yield* readHolder(lockPath); - if (holder === undefined || holder.pid !== process.pid) return; - const now = yield* DateTime.now; - const date = DateTime.toDateUtc(now); - yield* fs.utimes(lockPath, date, date); - }); - const reschedule = Schedule.spaced(RECLAIM_OBSERVATION_DELAY_MILLIS); - yield* Effect.forkScoped( - tick.pipe( - // Each tick is caught individually: `Effect.repeat` ends a failing - // effect on the first failure, so recovery has to live inside the round. - // The holder keeps refreshing for as long as it owns the lock, through - // whatever transient filesystem errors come and go. - Effect.catch(() => Effect.void), - Effect.repeat({ schedule: reschedule, while: () => true }), - Effect.catch(() => Effect.void), - ), - ); -}); - /** * Releases only a lock this process verifiably owns, so a successor is never * evicted. An undecodable file is left alone: it is either a live re-entrant @@ -396,7 +324,6 @@ export const acquireServerSingleton = Effect.fn("serverSingleton.acquire")(funct }), ); if (failure !== undefined) return yield* failure; - yield* holdServerLock(lockPath); return lockPath; }), () => releaseServerLock(lockPath).pipe(Effect.ignore), From 048c196a9efb74debfa819d0afaa2c78c1202c2f Mon Sep 17 00:00:00 2001 From: Stef Date: Mon, 31 Aug 2026 21:23:45 +0100 Subject: [PATCH 09/11] test(server): prove singleton startup ordering --- apps/server/src/server.ts | 12 ++++++------ apps/server/src/serverSingleton.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 883fcbe3ce24..521482f69fff 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -190,10 +190,10 @@ const RelayClientLive = Layer.unwrap( /** * Claims the data directory before anything binds a port or opens the database. * - * Provided once around the combined runtime-services + HTTP-server layer rather - * than merged alongside it so the ordering is structural: the lock is a dependency - * of everything it protects, and a second server cannot open persistence/provider - * state or reach a listening socket while another holds the directory. + * Provided once into `HttpServerLive`, which is then provided into the runtime + * services. That dependency chain makes the ordering structural: claim the state + * directory, bind HTTP, then open persistence/provider state. A second server + * cannot reach either protected phase while another holds the directory. */ const ServerSingletonLive = Layer.effectDiscard( Effect.gen(function* () { @@ -698,9 +698,9 @@ export const makeServerLayer = Layer.unwrap( { concurrency: "unbounded" }, ).pipe(Effect.asVoid), }).pipe(Layer.provideMerge(RuntimeDependenciesLive), Layer.provide(launcherLayer)); + const ownedHttpServerLive = HttpServerLive.pipe(Layer.provide(ServerSingletonLive)); const ownedRuntimeServicesLive = runtimeServicesLive.pipe( - Layer.provideMerge(HttpServerLive), - Layer.provide(ServerSingletonLive), + Layer.provideMerge(ownedHttpServerLive), ); const routesLayer = HttpRouter.serve(makeRoutesLayer.pipe(Layer.provide(launcherLayer)), { diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 27854f16e593..b6f1a6a41fa9 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -2,9 +2,12 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; +import * as ServerConfig from "./config.ts"; +import { makeServerLayer } from "./server.ts"; import { PersistedServerRuntimeState } from "./serverRuntimeState.ts"; import { SERVER_LOCK_FILENAME, @@ -42,6 +45,27 @@ const legacyRuntimeState = (pid: number, port: number) => })}\n`; layer("serverSingleton", (it) => { + it.effect("the full server layer refuses before opening persistence", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const baseDir = yield* fs.makeTempDirectory({ prefix: "t3-singleton-server-layer-" }); + const stateDir = path.join(baseDir, "userdata"); + yield* fs.makeDirectory(stateDir, { recursive: true }); + yield* fs.writeFileString( + path.join(stateDir, SERVER_LOCK_FILENAME), + `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, + ); + + const failure = yield* Layer.build( + makeServerLayer.pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), baseDir))), + ).pipe(Effect.scoped, Effect.flip); + + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + assert.isFalse(yield* fs.exists(path.join(stateDir, "state.sqlite"))); + }), + ); + it.effect("claims a free directory and releases it on scope exit", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); From 0197d8e88d1382e0915063e76fce5059b4a837e9 Mon Sep 17 00:00:00 2001 From: Stef Date: Mon, 31 Aug 2026 21:37:24 +0100 Subject: [PATCH 10/11] fix(server): use SQLite for process ownership --- apps/server/src/serverSingleton.test.ts | 127 +++----- apps/server/src/serverSingleton.ts | 396 ++++++++++++------------ 2 files changed, 235 insertions(+), 288 deletions(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index b6f1a6a41fa9..04a8351280c7 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -11,12 +11,14 @@ import { makeServerLayer } from "./server.ts"; import { PersistedServerRuntimeState } from "./serverRuntimeState.ts"; import { SERVER_LOCK_FILENAME, + SERVER_LOCK_DATABASE_FILENAME, SERVER_RUNTIME_STATE_FILENAME, acquireServerSingleton, + isServerLockBusyError, processIsAlive, processIsAliveWith, recordServerLockPort, - releaseServerLock, + serverLockDatabasePath, serverLockPath, } from "./serverSingleton.ts"; @@ -29,7 +31,7 @@ const makeStateDir = Effect.fn("test.makeStateDir")(function* () { /** A stale lock file, written without the module's own encoder on purpose. */ const staleHolder = (pid: number) => - `{"version":1,"pid":${pid},"startedAt":"2026-01-01T00:00:00.000Z"}`; + `{"version":1,"ownerId":"stale-owner","pid":${pid},"startedAt":"2026-01-01T00:00:00.000Z"}`; const PersistedServerRuntimeStateFromJson = Schema.fromJsonString(PersistedServerRuntimeState); const encodeRuntimeState = Schema.encodeSync(PersistedServerRuntimeStateFromJson); @@ -52,17 +54,17 @@ layer("serverSingleton", (it) => { const baseDir = yield* fs.makeTempDirectory({ prefix: "t3-singleton-server-layer-" }); const stateDir = path.join(baseDir, "userdata"); yield* fs.makeDirectory(stateDir, { recursive: true }); - yield* fs.writeFileString( - path.join(stateDir, SERVER_LOCK_FILENAME), - `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, - ); - - const failure = yield* Layer.build( - makeServerLayer.pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), baseDir))), - ).pipe(Effect.scoped, Effect.flip); + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + const failure = yield* Layer.build( + makeServerLayer.pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), baseDir))), + ).pipe(Effect.scoped, Effect.flip); - assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); - assert.isFalse(yield* fs.exists(path.join(stateDir, "state.sqlite"))); + assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); + assert.isFalse(yield* fs.exists(path.join(stateDir, "state.sqlite"))); + }), + ); }), ); @@ -103,7 +105,7 @@ layer("serverSingleton", (it) => { }), ); - it.effect("reclaims a lock whose owner is gone", () => + it.effect("overwrites stale display metadata after its owner is gone", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; @@ -112,18 +114,18 @@ layer("serverSingleton", (it) => { // be live. A crashed server must not lock its own directory forever. yield* fs.writeFileString(lockPath, staleHolder(4194304)); - // One call confirms the dead holder, reclaims it, and claims the freed - // directory on the same pass. yield* Effect.scoped( Effect.gen(function* () { const held = yield* acquireServerSingleton(stateDir); assert.strictEqual(held, lockPath); + const metadata = yield* fs.readFileString(lockPath); + assert.notInclude(metadata, "stale-owner"); }), ); }), ); - it.effect("reclaims a lock file left half-written by a crash", () => + it.effect("overwrites half-written display metadata after a crash", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; @@ -138,16 +140,20 @@ layer("serverSingleton", (it) => { }), ); - it.effect("does not release a lock another process has reclaimed", () => + it.effect("does not remove display metadata replaced by another owner", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; const lockPath = yield* serverLockPath(stateDir); - yield* fs.writeFileString(lockPath, staleHolder(4194304)); + yield* Effect.scoped( + Effect.gen(function* () { + yield* acquireServerSingleton(stateDir); + yield* fs.writeFileString(lockPath, staleHolder(process.pid)); + }), + ); - yield* releaseServerLock(lockPath); - // Evicting a live successor would recreate the very bug this prevents. assert.isTrue(yield* fs.exists(lockPath)); + assert.include(yield* fs.readFileString(lockPath), "stale-owner"); }), ); @@ -183,12 +189,14 @@ layer("serverSingleton", (it) => { }), ); - it.effect("uses a lock file inside the state directory", () => + it.effect("keeps ownership and display metadata inside the state directory", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const path = yield* Path.Path; const lockPath = yield* serverLockPath(stateDir); + const databasePath = yield* serverLockDatabasePath(stateDir); assert.strictEqual(lockPath, path.join(stateDir, SERVER_LOCK_FILENAME)); + assert.strictEqual(databasePath, path.join(stateDir, SERVER_LOCK_DATABASE_FILENAME)); }), ); @@ -209,6 +217,16 @@ layer("serverSingleton", (it) => { ); }); + it("recognizes Node and Bun SQLite busy errors", () => { + assert.isTrue( + isServerLockBusyError( + Object.assign(new Error("database is locked"), { code: "SQLITE_BUSY" }), + ), + ); + assert.isTrue(isServerLockBusyError(Object.assign(new Error("busy"), { errno: 5 }))); + assert.isFalse(isServerLockBusyError(new Error("disk I/O error"))); + }); + it.effect("refuses next to a live pre-lock server that wrote no lock", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); @@ -272,60 +290,7 @@ layer("serverSingleton", (it) => { }), ); - it.effect("a starter cannot reclaim a lock a live holder still owns", () => - Effect.gen(function* () { - const stateDir = yield* makeStateDir(); - const fs = yield* FileSystem.FileSystem; - const lockPath = yield* serverLockPath(stateDir); - - // Starter B holds the directory live. Starter A must refuse rather than - // remove B's claim. - yield* fs.writeFileString( - lockPath, - `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, - ); - yield* Effect.scoped( - Effect.gen(function* () { - // Undecodable-by-design: B's pid is live, so A must refuse. Before - // the fix, A's unconditional reclaim deleted B's lock and A owned - // the directory alongside B. - const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); - assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); - }), - ); - - // And the lock is still on disk, owned by B, not reclaimed under it. - assert.isTrue(yield* fs.exists(lockPath)); - const holder = yield* fs.readFileString(lockPath); - assert.include(holder, `"pid":${process.pid}`); - }), - ); - - it.effect("does not unlink a live holder encountered during reclaim", () => - Effect.gen(function* () { - const stateDir = yield* makeStateDir(); - const fs = yield* FileSystem.FileSystem; - const lockPath = yield* serverLockPath(stateDir); - - // A live successor's claim looks identical on paper except for the pid, - // which the liveness check must recognize before reclaim can remove it. - yield* fs.writeFileString( - lockPath, - `{"version":1,"pid":${process.pid},"startedAt":"2026-01-01T00:00:00.000Z"}`, - ); - - const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); - - // Under the old unconditional `fs.remove(lockPath)`, claim would - // *succeed* here by deleting this live claim first. - assert.strictEqual(failure._tag, "ServerAlreadyRunningError"); - assert.isTrue(yield* fs.exists(lockPath)); - const holder = yield* fs.readFileString(lockPath); - assert.include(holder, `"pid":${process.pid}`); - }), - ); - - it.effect("a partial metadata update is never readable as an empty owner", () => + it.effect("updates port metadata atomically", () => Effect.gen(function* () { const stateDir = yield* makeStateDir(); const fs = yield* FileSystem.FileSystem; @@ -333,13 +298,6 @@ layer("serverSingleton", (it) => { yield* Effect.scoped( Effect.gen(function* () { yield* acquireServerSingleton(stateDir); - // Before: the in-place rewrite truncated the file first, so a reader - // mid-update decoded an empty holder and could reclaim a live lock. - // The atomic write means the bytes on disk are always a full holder — - // and a temp-file rename is what arranges that: it stages the payload - // in a `.server.lock.*` sibling and swaps it in, which an in-place - // truncate never does. Asserting the update *completed* then lets a - // concurrent reader never observe an empty or partial lock. yield* recordServerLockPort(lockPath, 3775); const failure = yield* acquireServerSingleton(stateDir).pipe(Effect.flip); @@ -350,10 +308,7 @@ layer("serverSingleton", (it) => { }), ); - // No temp staging directory outlives the update: the payload arrives at - // the lock path whole or not at all, which is what makes "partial" reads - // above unreachable rather than merely unlucky. The scope above released - // the lock itself — only the temp-directory shape is under test. + // No temp staging directory outlives the metadata update. const leftovers = yield* fs .readDirectory(stateDir) .pipe( diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index c2282c90f79a..1f5b18d14f70 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -1,59 +1,39 @@ /** * One server per data directory. * - * Two T3 Code servers pointed at the same `--base-dir` both open `state.sqlite` - * and both write `settings.json`, and they overwrite each other. The observed - * incident: a desktop app auto-updated to a newer server while the old one was - * still running, the new process found its port taken, silently bound a random - * one, and ran blind against shared state. The visible symptom was a settings - * toggle that would not stick — hours away from the actual cause. + * The server already relies on SQLite for cross-platform filesystem locking. + * A dedicated lock database holds one write transaction for the process + * lifetime: a crash releases the OS lock automatically, while another process + * receives SQLITE_BUSY before it can open T3's real persistence or bind HTTP. * - * Nothing about that is detectable after the fact, so the fix is to refuse at - * startup. A second server against a held directory exits with one clear - * message instead of corrupting state. + * `server.lock` is display-only metadata for the refusal message. It never + * decides ownership; deleting it cannot release the SQLite lock. * - * ## Why a pid file rather than `flock` - * - * An advisory `flock` is the better primitive: the kernel drops it when the - * holder dies, so a crashed server leaves nothing stale to clean up. Node has no - * binding for it, and adding a native dependency to the server for one lock is a - * worse trade than handling staleness here. - * - * So the lock is an atomically created file holding the owner's identity, and - * liveness is checked with signal 0. The tradeoff is honest: if a server is - * killed and its pid is later reused by an unrelated process, this refuses to - * start until the file is removed. That is the safe direction to fail, and the - * message names the file for recovery after the reported pid is checked. - * - * ## Upgrading from a pre-lock version - * - * A pre-lock server writes no `server.lock`, so the file alone cannot see one. - * It does persist `server-runtime.json` with its live pid though, so before - * claiming the directory a live legacy runtime state is read as a held lock - * and refused the same way. Otherwise the desktop auto-update incident this - * exists to prevent still happens once on upgrade day — the new server starts - * next to the old one against the same state. + * A pre-lock server has no lock database, so the first upgraded process also + * checks `server-runtime.json` and refuses while that recorded pid is live. */ import * as Data from "effect/Data"; +import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; -import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import { writeFileStringAtomically } from "./atomicWrite.ts"; import { readPersistedServerRuntimeState } from "./serverRuntimeState.ts"; export const SERVER_LOCK_FILENAME = "server.lock"; +export const SERVER_LOCK_DATABASE_FILENAME = "server-lock.sqlite"; export const SERVER_RUNTIME_STATE_FILENAME = "server-runtime.json"; export const ServerLockHolder = Schema.Struct({ version: Schema.Literal(1), + ownerId: Schema.String, pid: Schema.Int, startedAt: Schema.String, - /** Absent until the server binds; the lock is taken before a port exists. */ + /** Absent until the server binds; ownership is already held in SQLite. */ port: Schema.optional(Schema.Int), }); export type ServerLockHolder = typeof ServerLockHolder.Type; @@ -67,29 +47,31 @@ export class ServerAlreadyRunningError extends Schema.TaggedErrorClass void): boolean => { if (!Number.isInteger(pid) || pid <= 0) return false; try { @@ -132,6 +102,21 @@ export const processIsAliveWith = (pid: number, sendSignal: (pid: number) => voi export const processIsAlive = (pid: number): boolean => processIsAliveWith(pid, (targetPid) => process.kill(targetPid, 0)); +interface LockDatabase { + readonly exec: (sql: string) => void; + readonly close: () => void; +} + +interface HeldServerLock { + readonly database: LockDatabase; + readonly lockPath: string; + readonly ownerId: string; +} + +class ServerLockAttemptError extends Data.TaggedError("ServerLockAttemptError")<{ + readonly cause: unknown; +}> {} + const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: string) { const fs = yield* FileSystem.FileSystem; const raw = yield* fs @@ -144,16 +129,61 @@ const readHolder = Effect.fn("serverSingleton.readHolder")(function* (lockPath: return Option.getOrUndefined(decodeHolder(raw)); }); -const isAlreadyExists = (error: PlatformError.PlatformError): boolean => - error.reason._tag === "AlreadyExists"; +const closeDatabase = (database: LockDatabase) => + Effect.sync(() => { + try { + database.exec("ROLLBACK"); + } catch { + // A failed BEGIN has no transaction to roll back. + } + database.close(); + }).pipe(Effect.ignore); -/** - * A pre-lock server is live against this directory. Failure-phase marker rather - * than an error: `serverRuntimeState.ts` already owns the file's decode-error - * type, and the refusal shown to the user is the same `ServerAlreadyRunningError` - * either way — `server.ts` converts this at the boundary where the config with - * the path lives. - */ +const openLockDatabase = Effect.fn("serverSingleton.openDatabase")(function* (lockPath: string) { + if (process.versions.bun !== undefined) { + const { Database } = yield* Effect.promise(() => import("bun:sqlite")); + return yield* Effect.try({ + try: () => { + const database = new Database(lockPath, { create: true }); + return { + exec: (sql: string) => database.exec(sql), + close: () => database.close(), + } satisfies LockDatabase; + }, + catch: (cause) => new ServerLockUnavailableError({ lockPath, operation: "open", cause }), + }); + } + + const { DatabaseSync } = yield* Effect.promise(() => import("node:sqlite")); + return yield* Effect.try({ + try: () => { + const database = new DatabaseSync(lockPath); + return { + exec: (sql: string) => database.exec(sql), + close: () => database.close(), + } satisfies LockDatabase; + }, + catch: (cause) => new ServerLockUnavailableError({ lockPath, operation: "open", cause }), + }); +}); + +export const isServerLockBusyError = (cause: unknown): boolean => { + const code = + cause instanceof Error && "code" in cause ? String((cause as NodeJS.ErrnoException).code) : ""; + const errno = + cause instanceof Error && "errno" in cause + ? Number((cause as NodeJS.ErrnoException).errno) + : undefined; + const message = cause instanceof Error ? cause.message : String(cause); + return ( + code === "SQLITE_BUSY" || + code === "SQLITE_BUSY_SNAPSHOT" || + errno === 5 || + /database is (?:locked|busy)/i.test(message) + ); +}; + +/** A pre-lock server is live against this directory. */ export class LiveLegacyServerRuntime extends Data.TaggedError("LiveLegacyServerRuntime")<{ readonly state: { readonly pid: number; @@ -162,106 +192,100 @@ export class LiveLegacyServerRuntime extends Data.TaggedError("LiveLegacyServerR }; }> {} -/** - * Claims the directory, or explains who holds it. - * - * A lock file whose owner is gone is reclaimed rather than treated as a - * permanent block: a dead holder must not lock its own directory forever. - * Reclaiming re-races the exclusive create, so two servers starting together - * still produce exactly one winner. - */ -const claimLock = Effect.fn("serverSingleton.claimLock")(function* (input: { - readonly stateDir: string; - readonly lockPath: string; - readonly legacyRuntimeStatePath: string; - readonly startedAt: string; -}) { +const beginExclusiveWrite = Effect.fn("serverSingleton.beginExclusiveWrite")(function* ( + database: LockDatabase, + lockPath: string, +) { + const result = yield* Effect.try({ + try: () => { + database.exec("PRAGMA busy_timeout = 0"); + database.exec("BEGIN IMMEDIATE"); + }, + catch: (cause) => new ServerLockAttemptError({ cause }), + }).pipe( + Effect.as({ ok: true as const }), + Effect.catch((error) => Effect.succeed({ ok: false as const, cause: error.cause })), + ); + if (result.ok) return true; + if (isServerLockBusyError(result.cause)) return false; + return yield* new ServerLockUnavailableError({ + lockPath, + operation: "lock", + cause: result.cause, + }); +}); + +export const serverLockPath = Effect.fn("serverSingleton.lockPath")(function* (stateDir: string) { + const path = yield* Path.Path; + return path.join(stateDir, SERVER_LOCK_FILENAME); +}); + +export const serverLockDatabasePath = Effect.fn("serverSingleton.databasePath")(function* ( + stateDir: string, +) { + const path = yield* Path.Path; + return path.join(stateDir, SERVER_LOCK_DATABASE_FILENAME); +}); + +export const legacyServerRuntimeStatePath = Effect.fn("serverSingleton.legacyStatePath")(function* ( + stateDir: string, +) { + const path = yield* Path.Path; + return path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME); +}); + +const acquireLock = Effect.fn("serverSingleton.acquireLock")(function* (stateDir: string) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; - const payload = encodeHolder({ version: 1, pid: process.pid, startedAt: input.startedAt }); + const crypto = yield* Crypto.Crypto; + const lockPath = yield* serverLockPath(stateDir); + const databasePath = yield* serverLockDatabasePath(stateDir); + const legacyRuntimeStatePath = yield* legacyServerRuntimeStatePath(stateDir); - // A pre-lock server writes no lock, so the file alone is blind to one — but - // it does persist its live pid in `server-runtime.json`. Refuse a live - // legacy runtime exactly like a live lock holder, before the lock is ever - // created; otherwise the upgrade that introduces this guard still starts - // next to the very server it is meant to replace. A dead legacy pid is - // ignored here: reclaim of its leftover lock is what removes the file. - const legacyState = yield* readPersistedServerRuntimeState(input.legacyRuntimeStatePath); + const legacyState = yield* readPersistedServerRuntimeState(legacyRuntimeStatePath); if (Option.isSome(legacyState) && processIsAlive(legacyState.value.pid)) { return yield* new LiveLegacyServerRuntime({ state: legacyState.value }); } - yield* fs.makeDirectory(path.dirname(input.lockPath), { recursive: true }); - - let reclaimCycles = 0; - while (true) { - // Let a competing starter or a just-created lock finish its write before - // inspecting the current owner. - yield* Effect.yieldNow; - const created = yield* fs.writeFileString(input.lockPath, payload, { flag: "wx" }).pipe( - Effect.as(true), - Effect.catch((error) => - isAlreadyExists(error) ? Effect.succeed(false) : Effect.fail(error), - ), - ); - if (created) return undefined; - - const holder = yield* readHolder(input.lockPath); - if (holder !== undefined && processIsAlive(holder.pid)) { - return new ServerAlreadyRunningError({ - stateDir: input.stateDir, - lockPath: input.lockPath, - holderPid: holder.pid, - ...(holder.port === undefined ? {} : { holderPort: holder.port }), - holderStartedAt: holder.startedAt, + yield* fs.makeDirectory(path.dirname(databasePath), { recursive: true }); + const database = yield* openLockDatabase(databasePath); + return yield* Effect.gen(function* () { + if (!(yield* beginExclusiveWrite(database, databasePath))) { + const holder = yield* readHolder(lockPath); + return yield* new ServerAlreadyRunningError({ + stateDir, + lockPath, + ...(holder === undefined + ? {} + : { + holderPid: holder.pid, + holderStartedAt: holder.startedAt, + ...(holder.port === undefined ? {} : { holderPort: holder.port }), + }), }); } - // Re-read after yielding before removal. A live writer that was still - // finishing its exclusive-create payload, or another starter that won a - // preceding reclaim, is then observed and preserved. - yield* Effect.yieldNow; - const confirmedHolder = yield* readHolder(input.lockPath); - if (confirmedHolder !== undefined && processIsAlive(confirmedHolder.pid)) { - continue; - } - - yield* fs.remove(input.lockPath, { force: true }); - reclaimCycles += 1; - if (reclaimCycles >= MAX_RECLAIM_CYCLES) { - return new ServerLockUnavailableError({ - lockPath: input.lockPath, - attempts: reclaimCycles, - }); - } - } + const ownerId = yield* crypto.randomUUIDv4; + const startedAt = DateTime.formatIso(yield* DateTime.now); + yield* writeFileStringAtomically({ + filePath: lockPath, + contents: encodeHolder({ version: 1, ownerId, pid: process.pid, startedAt }), + }); + return { database, lockPath, ownerId } satisfies HeldServerLock; + }).pipe(Effect.onError(() => closeDatabase(database))); }); -/** - * Releases only a lock this process verifiably owns, so a successor is never - * evicted. An undecodable file is left alone: it is either a live re-entrant - * claim (the winning starter's just-created file, or another process's) or a - * crash fragment, and crash fragments are what reclaim exists for. - */ -export const releaseServerLock = Effect.fn("serverSingleton.release")(function* (lockPath: string) { +const releaseLock = Effect.fn("serverSingleton.releaseLock")(function* (held: HeldServerLock) { const fs = yield* FileSystem.FileSystem; - const holder = yield* readHolder(lockPath); - if (holder === undefined || holder.pid !== process.pid) return; - yield* fs.remove(lockPath, { force: true }).pipe(Effect.ignore); + const holder = yield* readHolder(held.lockPath).pipe(Effect.orElseSucceed(() => undefined)); + if (holder?.ownerId === held.ownerId) { + // Remove metadata while the SQLite transaction still excludes successors. + yield* fs.remove(held.lockPath, { force: true }).pipe(Effect.ignore); + } + yield* closeDatabase(held.database); }); -/** - * Records the bound port on the lock we already hold. - * - * The rewrite goes through write-temp-then-rename so a reader never observes - * an empty or partial file: with an in-place rewrite, a concurrent starter - * could mistake the truncated file for a crashed owner and reclaim a live - * lock — the exact concurrent-servers corruption the lock exists to prevent. - * - * Only for the error message a *later* server prints: knowing the holder's port - * turns "something else is running" into an address the user can open. Failure - * is ignored — the lock's job is done once it is held. - */ +/** Records the bound port in display metadata while SQLite owns the lock. */ export const recordServerLockPort = Effect.fn("serverSingleton.recordPort")(function* ( lockPath: string, port: number, @@ -274,58 +298,26 @@ export const recordServerLockPort = Effect.fn("serverSingleton.recordPort")(func }).pipe(Effect.ignore); }); -export const serverLockPath = Effect.fn("serverSingleton.lockPath")(function* (stateDir: string) { - const path = yield* Path.Path; - return path.join(stateDir, SERVER_LOCK_FILENAME); -}); - -export const legacyServerRuntimeStatePath = Effect.fn("serverSingleton.legacyStatePath")(function* ( - stateDir: string, -) { - const path = yield* Path.Path; - return path.join(stateDir, SERVER_RUNTIME_STATE_FILENAME); -}); - -/** - * Holds the data directory for the lifetime of the returned scope. - * - * Acquired before anything opens the database or binds a port, and released on - * shutdown. A live pre-lock server reads as a held lock here too: callers see - * one refusal type regardless of which generation holds the directory. - */ +/** Holds the state directory until the caller's scope closes. */ export const acquireServerSingleton = Effect.fn("serverSingleton.acquire")(function* ( stateDir: string, ) { - const lockPath = yield* serverLockPath(stateDir); const legacyRuntimeStatePath = yield* legacyServerRuntimeStatePath(stateDir); - const startedAt = DateTime.formatIso(yield* DateTime.now); return yield* Effect.acquireRelease( - Effect.gen(function* () { - const failure = yield* claimLock({ - stateDir, - lockPath, - legacyRuntimeStatePath, - startedAt, - }).pipe( - // The message names `server-runtime.json` (not `server.lock`) as the - // file to remove if the process is gone — that is all a pre-lock - // server ever wrote. - Effect.catchTags({ - LiveLegacyServerRuntime: (legacy) => - Effect.fail( - new ServerAlreadyRunningError({ - stateDir, - lockPath: legacyRuntimeStatePath, - holderPid: legacy.state.pid, - holderPort: legacy.state.port, - holderStartedAt: legacy.state.startedAt, - }), - ), - }), - ); - if (failure !== undefined) return yield* failure; - return lockPath; - }), - () => releaseServerLock(lockPath).pipe(Effect.ignore), - ); + acquireLock(stateDir).pipe( + Effect.catchTags({ + LiveLegacyServerRuntime: (legacy) => + Effect.fail( + new ServerAlreadyRunningError({ + stateDir, + lockPath: legacyRuntimeStatePath, + holderPid: legacy.state.pid, + holderPort: legacy.state.port, + holderStartedAt: legacy.state.startedAt, + }), + ), + }), + ), + releaseLock, + ).pipe(Effect.map((held) => held.lockPath)); }); From 70b3f7d08f1c5fff810621d34e56d5d6be778163 Mon Sep 17 00:00:00 2001 From: Stef Date: Tue, 1 Sep 2026 05:55:52 +0100 Subject: [PATCH 11/11] fix(server): distinguish legacy runtime ownership --- apps/server/src/serverSingleton.test.ts | 3 +++ apps/server/src/serverSingleton.ts | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/apps/server/src/serverSingleton.test.ts b/apps/server/src/serverSingleton.test.ts index 04a8351280c7..287fdea12409 100644 --- a/apps/server/src/serverSingleton.test.ts +++ b/apps/server/src/serverSingleton.test.ts @@ -245,6 +245,9 @@ layer("serverSingleton", (it) => { assert.strictEqual(failure.holderPid, process.pid); assert.strictEqual(failure.holderPort, 3775); assert.include(failure.message, SERVER_RUNTIME_STATE_FILENAME); + assert.include(failure.message, "compatibility guard for this older"); + assert.include(failure.message, "Do not remove it while the recorded process is live"); + assert.notInclude(failure.message, "contains display metadata only"); } // And it must not have claimed the directory it refused. assert.isFalse(yield* fs.exists(yield* serverLockPath(stateDir))); diff --git a/apps/server/src/serverSingleton.ts b/apps/server/src/serverSingleton.ts index 1f5b18d14f70..7057037e0103 100644 --- a/apps/server/src/serverSingleton.ts +++ b/apps/server/src/serverSingleton.ts @@ -11,6 +11,9 @@ * * A pre-lock server has no lock database, so the first upgraded process also * checks `server-runtime.json` and refuses while that recorded pid is live. + * That compatibility check starts only after the old binary publishes its + * runtime state; an unmodified old binary cannot participate in a lock that + * did not exist when it shipped. */ import * as Data from "effect/Data"; import * as Crypto from "effect/Crypto"; @@ -50,6 +53,7 @@ export class ServerAlreadyRunningError extends Schema.TaggedErrorClass