Handle empty server URLs and stale sidebar threads

- Preserve persisted environment IDs on read failures
- Scope sidebar thread lookups to the active environment
- Treat empty server URLs as unset

Author: juliusmarminge

apps/server/src/environment/Layers/ServerEnvironment.test.ts

@@ -1,14 +1,58 @@
+import * as nodePath from "node:path";
 import * as NodeServices from "@effect/platform-node/NodeServices";
 import { expect, it } from "@effect/vitest";
-import { Effect, FileSystem, Layer } from "effect";
+import { Effect, Exit, FileSystem, Layer, PlatformError } from "effect";
 
-import { ServerConfig } from "../../config.ts";
+import { ServerConfig, type ServerConfigShape } from "../../config.ts";
 import { ServerEnvironment } from "../Services/ServerEnvironment.ts";
 import { ServerEnvironmentLive } from "./ServerEnvironment.ts";
 
 const makeServerEnvironmentLayer = (baseDir: string) =>
   ServerEnvironmentLive.pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), baseDir)));
 
+const makeServerConfig = (baseDir: string): ServerConfigShape => {
+  const stateDir = nodePath.join(baseDir, "userdata");
+  const logsDir = nodePath.join(stateDir, "logs");
+  const providerLogsDir = nodePath.join(logsDir, "provider");
+  return {
+    logLevel: "Error",
+    traceMinLevel: "Info",
+    traceTimingEnabled: true,
+    traceBatchWindowMs: 200,
+    traceMaxBytes: 10 * 1024 * 1024,
+    traceMaxFiles: 10,
+    otlpTracesUrl: undefined,
+    otlpMetricsUrl: undefined,
+    otlpExportIntervalMs: 10_000,
+    otlpServiceName: "t3-server",
+    cwd: process.cwd(),
+    baseDir,
+    stateDir,
+    dbPath: nodePath.join(stateDir, "state.sqlite"),
+    keybindingsConfigPath: nodePath.join(stateDir, "keybindings.json"),
+    settingsPath: nodePath.join(stateDir, "settings.json"),
+    worktreesDir: nodePath.join(baseDir, "worktrees"),
+    attachmentsDir: nodePath.join(stateDir, "attachments"),
+    logsDir,
+    serverLogPath: nodePath.join(logsDir, "server.log"),
+    serverTracePath: nodePath.join(logsDir, "server.trace.ndjson"),
+    providerLogsDir,
+    providerEventLogPath: nodePath.join(providerLogsDir, "events.log"),
+    terminalLogsDir: nodePath.join(logsDir, "terminals"),
+    anonymousIdPath: nodePath.join(stateDir, "anonymous-id"),
+    environmentIdPath: nodePath.join(stateDir, "environment-id"),
+    mode: "web",
+    autoBootstrapProjectFromCwd: false,
+    logWebSocketEvents: false,
+    port: 0,
+    host: undefined,
+    authToken: undefined,
+    staticDir: undefined,
+    devUrl: undefined,
+    noBrowser: false,
+  };
+};
+
 it.layer(NodeServices.layer)("ServerEnvironmentLive", (it) => {
   it.effect("persists the environment id across service restarts", () =>
     Effect.gen(function* () {
@@ -30,4 +74,65 @@ it.layer(NodeServices.layer)("ServerEnvironmentLive", (it) => {
       expect(second.capabilities.repositoryIdentity).toBe(true);
     }),
   );
+
+  it.effect("fails instead of overwriting a persisted id when reading the file errors", () =>
+    Effect.gen(function* () {
+      const fileSystem = yield* FileSystem.FileSystem;
+      const baseDir = yield* fileSystem.makeTempDirectoryScoped({
+        prefix: "t3-server-environment-read-error-test-",
+      });
+      const serverConfig = makeServerConfig(baseDir);
+      const environmentIdPath = serverConfig.environmentIdPath;
+      yield* fileSystem.makeDirectory(nodePath.dirname(environmentIdPath), { recursive: true });
+      yield* fileSystem.writeFileString(environmentIdPath, "persisted-environment-id\n");
+      const writeAttempts: string[] = [];
+      const failingFileSystemLayer = FileSystem.layerNoop({
+        exists: (path) => Effect.succeed(path === environmentIdPath),
+        readFileString: (path) =>
+          path === environmentIdPath
+            ? Effect.fail(
+                PlatformError.systemError({
+                  _tag: "PermissionDenied",
+                  module: "FileSystem",
+                  method: "readFileString",
+                  description: "permission denied",
+                  pathOrDescriptor: path,
+                }),
+              )
+            : Effect.fail(
+                PlatformError.systemError({
+                  _tag: "NotFound",
+                  module: "FileSystem",
+                  method: "readFileString",
+                  description: "not found",
+                  pathOrDescriptor: path,
+                }),
+              ),
+        writeFileString: (path) => {
+          writeAttempts.push(path);
+          return Effect.void;
+        },
+      });
+
+      const exit = yield* Effect.gen(function* () {
+        const serverEnvironment = yield* ServerEnvironment;
+        return yield* serverEnvironment.getDescriptor;
+      }).pipe(
+        Effect.provide(
+          ServerEnvironmentLive.pipe(
+            Layer.provide(
+              Layer.merge(Layer.succeed(ServerConfig, serverConfig), failingFileSystemLayer),
+            ),
+          ),
+        ),
+        Effect.exit,
+      );
+
+      expect(Exit.isFailure(exit)).toBe(true);
+      expect(writeAttempts).toEqual([]);
+      expect(yield* fileSystem.readFileString(environmentIdPath)).toBe(
+        "persisted-environment-id\n",
+      );
+    }),
+  );
 });

apps/server/src/environment/Layers/ServerEnvironment.ts

@@ -42,10 +42,9 @@ export const makeServerEnvironment = Effect.fn("makeServerEnvironment")(function
       return null;
     }
 
-    const raw = yield* fileSystem.readFileString(serverConfig.environmentIdPath).pipe(
-      Effect.orElseSucceed(() => ""),
-      Effect.map((value) => value.trim()),
-    );
+    const raw = yield* fileSystem
+      .readFileString(serverConfig.environmentIdPath)
+      .pipe(Effect.map((value) => value.trim()));
 
     return raw.length > 0 ? raw : null;
   });

apps/web/src/lib/utils.test.ts

@@ -24,6 +24,10 @@ describe("isWindowsPlatform", () => {
 });
 
 describe("resolveServerUrl", () => {
+  it("falls back to the bootstrap environment URL when the explicit URL is empty", () => {
+    expect(resolveServerUrl({ url: "" })).toBe("http://bootstrap.test:4321/");
+  });
+
   it("uses the bootstrap environment URL when no explicit URL is provided", () => {
     expect(resolveServerUrl()).toBe("http://bootstrap.test:4321/");
   });

apps/web/src/lib/utils.ts

@@ -38,22 +38,16 @@ export const newThreadId = (): ThreadId => ThreadId.makeUnsafe(randomUUID());
 export const newMessageId = (): MessageId => MessageId.makeUnsafe(randomUUID());
 
 const isNonEmptyString = Predicate.compose(Predicate.isString, String.isNonEmpty);
-const firstNonEmptyString = (...values: unknown[]): string => {
-  for (const value of values) {
-    if (isNonEmptyString(value)) {
-      return value;
-    }
-  }
-  throw new Error("No non-empty string provided");
-};
 
 export const resolveServerUrl = (options?: {
   url?: string | undefined;
   protocol?: "http" | "https" | "ws" | "wss" | undefined;
   pathname?: string | undefined;
   searchParams?: Record<string, string> | undefined;
 }): string => {
-  const rawUrl = firstNonEmptyString(options?.url || resolvePrimaryEnvironmentBootstrapUrl());
+  const rawUrl = isNonEmptyString(options?.url)
+    ? options.url
+    : resolvePrimaryEnvironmentBootstrapUrl();
 
   const parsedUrl = new URL(rawUrl);
   if (options?.protocol) {