Skip to content

Commit 314c455

Browse files
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
1 parent aa14762 commit 314c455

File tree

5 files changed

+140
-37
lines changed

5 files changed

+140
-37
lines changed

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

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,58 @@
1+
import * as nodePath from "node:path";
12
import * as NodeServices from "@effect/platform-node/NodeServices";
23
import { expect, it } from "@effect/vitest";
3-
import { Effect, FileSystem, Layer } from "effect";
4+
import { Effect, Exit, FileSystem, Layer, PlatformError } from "effect";
45

5-
import { ServerConfig } from "../../config.ts";
6+
import { ServerConfig, type ServerConfigShape } from "../../config.ts";
67
import { ServerEnvironment } from "../Services/ServerEnvironment.ts";
78
import { ServerEnvironmentLive } from "./ServerEnvironment.ts";
89

910
const makeServerEnvironmentLayer = (baseDir: string) =>
1011
ServerEnvironmentLive.pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), baseDir)));
1112

13+
const makeServerConfig = (baseDir: string): ServerConfigShape => {
14+
const stateDir = nodePath.join(baseDir, "userdata");
15+
const logsDir = nodePath.join(stateDir, "logs");
16+
const providerLogsDir = nodePath.join(logsDir, "provider");
17+
return {
18+
logLevel: "Error",
19+
traceMinLevel: "Info",
20+
traceTimingEnabled: true,
21+
traceBatchWindowMs: 200,
22+
traceMaxBytes: 10 * 1024 * 1024,
23+
traceMaxFiles: 10,
24+
otlpTracesUrl: undefined,
25+
otlpMetricsUrl: undefined,
26+
otlpExportIntervalMs: 10_000,
27+
otlpServiceName: "t3-server",
28+
cwd: process.cwd(),
29+
baseDir,
30+
stateDir,
31+
dbPath: nodePath.join(stateDir, "state.sqlite"),
32+
keybindingsConfigPath: nodePath.join(stateDir, "keybindings.json"),
33+
settingsPath: nodePath.join(stateDir, "settings.json"),
34+
worktreesDir: nodePath.join(baseDir, "worktrees"),
35+
attachmentsDir: nodePath.join(stateDir, "attachments"),
36+
logsDir,
37+
serverLogPath: nodePath.join(logsDir, "server.log"),
38+
serverTracePath: nodePath.join(logsDir, "server.trace.ndjson"),
39+
providerLogsDir,
40+
providerEventLogPath: nodePath.join(providerLogsDir, "events.log"),
41+
terminalLogsDir: nodePath.join(logsDir, "terminals"),
42+
anonymousIdPath: nodePath.join(stateDir, "anonymous-id"),
43+
environmentIdPath: nodePath.join(stateDir, "environment-id"),
44+
mode: "web",
45+
autoBootstrapProjectFromCwd: false,
46+
logWebSocketEvents: false,
47+
port: 0,
48+
host: undefined,
49+
authToken: undefined,
50+
staticDir: undefined,
51+
devUrl: undefined,
52+
noBrowser: false,
53+
};
54+
};
55+
1256
it.layer(NodeServices.layer)("ServerEnvironmentLive", (it) => {
1357
it.effect("persists the environment id across service restarts", () =>
1458
Effect.gen(function* () {
@@ -30,4 +74,65 @@ it.layer(NodeServices.layer)("ServerEnvironmentLive", (it) => {
3074
expect(second.capabilities.repositoryIdentity).toBe(true);
3175
}),
3276
);
77+
78+
it.effect("fails instead of overwriting a persisted id when reading the file errors", () =>
79+
Effect.gen(function* () {
80+
const fileSystem = yield* FileSystem.FileSystem;
81+
const baseDir = yield* fileSystem.makeTempDirectoryScoped({
82+
prefix: "t3-server-environment-read-error-test-",
83+
});
84+
const serverConfig = makeServerConfig(baseDir);
85+
const environmentIdPath = serverConfig.environmentIdPath;
86+
yield* fileSystem.makeDirectory(nodePath.dirname(environmentIdPath), { recursive: true });
87+
yield* fileSystem.writeFileString(environmentIdPath, "persisted-environment-id\n");
88+
const writeAttempts: string[] = [];
89+
const failingFileSystemLayer = FileSystem.layerNoop({
90+
exists: (path) => Effect.succeed(path === environmentIdPath),
91+
readFileString: (path) =>
92+
path === environmentIdPath
93+
? Effect.fail(
94+
PlatformError.systemError({
95+
_tag: "PermissionDenied",
96+
module: "FileSystem",
97+
method: "readFileString",
98+
description: "permission denied",
99+
pathOrDescriptor: path,
100+
}),
101+
)
102+
: Effect.fail(
103+
PlatformError.systemError({
104+
_tag: "NotFound",
105+
module: "FileSystem",
106+
method: "readFileString",
107+
description: "not found",
108+
pathOrDescriptor: path,
109+
}),
110+
),
111+
writeFileString: (path) => {
112+
writeAttempts.push(path);
113+
return Effect.void;
114+
},
115+
});
116+
117+
const exit = yield* Effect.gen(function* () {
118+
const serverEnvironment = yield* ServerEnvironment;
119+
return yield* serverEnvironment.getDescriptor;
120+
}).pipe(
121+
Effect.provide(
122+
ServerEnvironmentLive.pipe(
123+
Layer.provide(
124+
Layer.merge(Layer.succeed(ServerConfig, serverConfig), failingFileSystemLayer),
125+
),
126+
),
127+
),
128+
Effect.exit,
129+
);
130+
131+
expect(Exit.isFailure(exit)).toBe(true);
132+
expect(writeAttempts).toEqual([]);
133+
expect(yield* fileSystem.readFileString(environmentIdPath)).toBe(
134+
"persisted-environment-id\n",
135+
);
136+
}),
137+
);
33138
});

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ export const makeServerEnvironment = Effect.fn("makeServerEnvironment")(function
4242
return null;
4343
}
4444

45-
const raw = yield* fileSystem.readFileString(serverConfig.environmentIdPath).pipe(
46-
Effect.orElseSucceed(() => ""),
47-
Effect.map((value) => value.trim()),
48-
);
45+
const raw = yield* fileSystem
46+
.readFileString(serverConfig.environmentIdPath)
47+
.pipe(Effect.map((value) => value.trim()));
4948

5049
return raw.length > 0 ? raw : null;
5150
});

apps/web/src/components/Sidebar.tsx

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ function resolveThreadPr(
255255
}
256256

257257
interface SidebarThreadRowProps {
258-
thread: SidebarThreadSummary;
258+
thread: SidebarThreadSummary | undefined;
259259
projectCwd: string | null;
260260
orderedProjectThreadIds: readonly ThreadId[];
261261
routeThreadId: ThreadId | null;
@@ -291,16 +291,23 @@ interface SidebarThreadRowProps {
291291

292292
function SidebarThreadRow(props: SidebarThreadRowProps) {
293293
const { thread } = props;
294-
const lastVisitedAt = useUiStateStore((state) => state.threadLastVisitedAtById[thread.id]);
295-
const runningTerminalIds = useTerminalStateStore(
296-
(state) =>
297-
selectThreadTerminalState(state.terminalStateByThreadId, thread.id).runningTerminalIds,
294+
const threadId = thread?.id ?? null;
295+
const lastVisitedAt = useUiStateStore((state) =>
296+
threadId ? state.threadLastVisitedAtById[threadId] : undefined,
297+
);
298+
const runningTerminalIds = useTerminalStateStore((state) =>
299+
threadId
300+
? selectThreadTerminalState(state.terminalStateByThreadId, threadId).runningTerminalIds
301+
: [],
298302
);
299303
const gitCwd = thread?.worktreePath ?? props.projectCwd;
300304
const gitStatus = useGitStatus(thread?.branch != null ? gitCwd : null);
305+
if (!thread || threadId === null) {
306+
return null;
307+
}
301308

302-
const isActive = props.routeThreadId === thread.id;
303-
const isSelected = props.selectedThreadIds.has(thread.id);
309+
const isActive = props.routeThreadId === threadId;
310+
const isSelected = props.selectedThreadIds.has(threadId);
304311
const isHighlighted = isActive || isSelected;
305312
const isThreadRunning =
306313
thread.session?.status === "running" && thread.session.activeTurnId != null;
@@ -674,6 +681,7 @@ function SortableProjectItem({
674681
}
675682

676683
export default function Sidebar() {
684+
const activeEnvironmentId = useStore((store) => store.activeEnvironmentId);
677685
const projects = useStore((store) => store.projects);
678686
const sidebarThreadsByScopedId = useStore((store) => store.sidebarThreadsByScopedId);
679687
const threadScopedIdsByProjectScopedId = useStore(
@@ -785,15 +793,14 @@ export default function Sidebar() {
785793
.filter((thread): thread is NonNullable<typeof thread> => thread !== undefined),
786794
[sidebarThreadsByScopedId, threadScopedIdsByProjectScopedId],
787795
);
788-
const sidebarThreadByScopedId = useMemo(
796+
const activeSidebarThreadById = useMemo(
789797
() =>
790798
new Map(
791-
sidebarThreads.map((thread) => [
792-
getThreadScopedId({ environmentId: thread.environmentId, id: thread.id }),
793-
thread,
794-
]),
799+
sidebarThreads
800+
.filter((thread) => thread.environmentId === activeEnvironmentId)
801+
.map((thread) => [thread.id, thread]),
795802
),
796-
[sidebarThreads],
803+
[activeEnvironmentId, sidebarThreads],
797804
);
798805
const routeTerminalOpen = routeThreadId
799806
? selectThreadTerminalState(terminalStateByThreadId, routeThreadId).terminalOpen
@@ -1144,13 +1151,7 @@ export default function Sidebar() {
11441151

11451152
if (clicked === "mark-unread") {
11461153
for (const id of ids) {
1147-
let thread: SidebarThreadSummary | undefined;
1148-
for (const t of sidebarThreadByScopedId.values()) {
1149-
if (t.id === id) {
1150-
thread = t;
1151-
break;
1152-
}
1153-
}
1154+
const thread = activeSidebarThreadById.get(id);
11541155
markThreadUnread(id, thread?.latestTurn?.completedAt);
11551156
}
11561157
clearSelection();
@@ -1181,7 +1182,7 @@ export default function Sidebar() {
11811182
deleteThread,
11821183
markThreadUnread,
11831184
removeFromSelection,
1184-
sidebarThreadByScopedId,
1185+
activeSidebarThreadById,
11851186
selectedThreadIds,
11861187
],
11871188
);
@@ -1720,7 +1721,7 @@ export default function Sidebar() {
17201721
{shouldShowThreadPanel &&
17211722
renderedThreads.map((thread) => (
17221723
<SidebarThreadRow
1723-
key={thread.id}
1724+
key={getThreadScopedId({ environmentId: thread.environmentId, id: thread.id })}
17241725
thread={thread}
17251726
projectCwd={project.cwd}
17261727
orderedProjectThreadIds={orderedProjectThreadIds}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ describe("isWindowsPlatform", () => {
2424
});
2525

2626
describe("resolveServerUrl", () => {
27+
it("falls back to the bootstrap environment URL when the explicit URL is empty", () => {
28+
expect(resolveServerUrl({ url: "" })).toBe("http://bootstrap.test:4321/");
29+
});
30+
2731
it("uses the bootstrap environment URL when no explicit URL is provided", () => {
2832
expect(resolveServerUrl()).toBe("http://bootstrap.test:4321/");
2933
});

apps/web/src/lib/utils.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,16 @@ export const newThreadId = (): ThreadId => ThreadId.makeUnsafe(randomUUID());
3838
export const newMessageId = (): MessageId => MessageId.makeUnsafe(randomUUID());
3939

4040
const isNonEmptyString = Predicate.compose(Predicate.isString, String.isNonEmpty);
41-
const firstNonEmptyString = (...values: unknown[]): string => {
42-
for (const value of values) {
43-
if (isNonEmptyString(value)) {
44-
return value;
45-
}
46-
}
47-
throw new Error("No non-empty string provided");
48-
};
4941

5042
export const resolveServerUrl = (options?: {
5143
url?: string | undefined;
5244
protocol?: "http" | "https" | "ws" | "wss" | undefined;
5345
pathname?: string | undefined;
5446
searchParams?: Record<string, string> | undefined;
5547
}): string => {
56-
const rawUrl = firstNonEmptyString(options?.url || resolvePrimaryEnvironmentBootstrapUrl());
48+
const rawUrl = isNonEmptyString(options?.url)
49+
? options.url
50+
: resolvePrimaryEnvironmentBootstrapUrl();
5751

5852
const parsedUrl = new URL(rawUrl);
5953
if (options?.protocol) {

0 commit comments

Comments
 (0)