Skip to content

Commit 5bcb8e8

Browse files
heavy-dclaude
andcommitted
test(websocket): make the suite pass on Windows dev machines
Five classes of environment assumptions broke 23 tests on Windows; all fixes keep the tests running unchanged on Linux CI. - Symlink tests (file-api, local-file-access): creating symlinks on Windows needs Developer Mode or elevation. A trySymlink helper skips the test on EPERM instead of failing before the assertion runs. - file:// URI expectations (resolve-media-urls): pathToFileURL prefixes the current drive on Windows (file:///C:/var/...). Expectations are now built with pathToFileURL, same as the source. - path.join literals (trpc-mcp-config, trpc-fonts, trpc-workspace): the routers join paths with the platform separator; tests compared against hardcoded POSIX strings. Expectations now use join()/basename(). JSON fixture keys stay POSIX — they come from the mocked homedir(). - Non-hermetic model routes (models-api-coverage, models-api-worker, trpc-models-worker): GET /all reached the real secret store (live provider calls on a machine with API keys) and scope=local scanned the real HF cache (30s+ on a large one). Both layers are now mocked. - Honest timeouts: trpc-packs live-loads the real minimax pack, which is slow under vitest's transform on Windows (120s budget); the autosave cutover waitForMessages ceiling goes 5s -> 30s — it returns as soon as the expected count arrives, so fast machines pay nothing. The "defaults the root to the home directory" test also had a false premise on Windows: os.tmpdir() lives inside the home directory. It now probes a path derived from the home drive's root, relying on the policy check running before the existence check. websocket suite on Windows: 23 failed -> 0 failed (5 skipped where the OS forbids symlink creation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 3ad78c4 commit 5bcb8e8

12 files changed

Lines changed: 165 additions & 38 deletions

packages/websocket/tests/file-api.test.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,25 @@ function makeRequest(urlPath: string, method = "GET"): Request {
3232
return new Request(`http://localhost${urlPath}`, { method });
3333
}
3434

35+
/**
36+
* Creating symlinks on Windows needs Developer Mode or elevation; when the
37+
* environment can't, the caller skips the test instead of failing on EPERM.
38+
*/
39+
async function trySymlink(target: string, link: string): Promise<boolean> {
40+
try {
41+
await fs.symlink(target, link);
42+
return true;
43+
} catch (error) {
44+
if (
45+
process.platform === "win32" &&
46+
(error as NodeJS.ErrnoException).code === "EPERM"
47+
) {
48+
return false;
49+
}
50+
throw error;
51+
}
52+
}
53+
3554
// ---------------------------------------------------------------------------
3655
// /api/files/local (streaming by absolute path)
3756
// ---------------------------------------------------------------------------
@@ -127,23 +146,28 @@ describe("/api/files/local", () => {
127146

128147
it("defaults the root to the home directory", async () => {
129148
delete process.env["NODETOOL_LOCAL_FILE_ROOTS"];
130-
const file = path.join(tmpDir, "clip.mp4");
131-
await fs.writeFile(file, "video-bytes");
132-
// tmpDir is outside home, so the default policy refuses it.
133-
const res = await handleFileRequest(localRequest(file));
149+
// A path guaranteed to be outside home on every platform (the Windows
150+
// tmpdir lives *inside* home, so tmpDir won't do). The policy check runs
151+
// before the existence check, so the file doesn't have to exist.
152+
const outside = path.join(
153+
path.parse(os.homedir()).root,
154+
"nodetool-file-api-outside",
155+
"clip.mp4"
156+
);
157+
const res = await handleFileRequest(localRequest(outside));
134158
expect(res.status).toBe(403);
135159
});
136160

137-
it("denies a symlink that escapes the roots", async () => {
161+
it("denies a symlink that escapes the roots", async (ctx) => {
138162
const link = path.join(tmpDir, "escape.txt");
139-
await fs.symlink("/etc/passwd", link);
163+
if (!(await trySymlink("/etc/passwd", link))) return ctx.skip();
140164
const res = await handleFileRequest(localRequest(link));
141165
expect(res.status).toBe(403);
142166
});
143167

144-
it("denies a path that escapes through a symlinked parent", async () => {
168+
it("denies a path that escapes through a symlinked parent", async (ctx) => {
145169
const linkDir = path.join(tmpDir, "outside");
146-
await fs.symlink("/etc", linkDir);
170+
if (!(await trySymlink("/etc", linkDir))) return ctx.skip();
147171
const res = await handleFileRequest(
148172
localRequest(path.join(linkDir, "passwd"))
149173
);

packages/websocket/tests/generation-autosave-cutover.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ async function waitForMessages(
5959
ws: MockWS,
6060
predicate: (m: Record<string, unknown>) => boolean,
6161
count: number,
62-
timeoutMs = 5000
62+
// Generous ceiling — the loop returns the moment `count` is reached, so
63+
// this only costs time when events are genuinely late (slow dev machines).
64+
timeoutMs = 30_000
6365
): Promise<Record<string, unknown>[]> {
6466
const start = Date.now();
6567
for (;;) {

packages/websocket/tests/local-file-access.test.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,25 @@ afterEach(async () => {
2626
delete process.env[LOCAL_FILE_ROOTS_ENV];
2727
});
2828

29+
/**
30+
* Creating symlinks on Windows needs Developer Mode or elevation; when the
31+
* environment can't, the caller skips the test instead of failing on EPERM.
32+
*/
33+
async function trySymlink(target: string, link: string): Promise<boolean> {
34+
try {
35+
await fs.symlink(target, link);
36+
return true;
37+
} catch (error) {
38+
if (
39+
process.platform === "win32" &&
40+
(error as NodeJS.ErrnoException).code === "EPERM"
41+
) {
42+
return false;
43+
}
44+
throw error;
45+
}
46+
}
47+
2948
describe("getLocalFileRoots", () => {
3049
it("defaults to the home directory", () => {
3150
expect(getLocalFileRoots()).toEqual([path.resolve(os.homedir())]);
@@ -96,27 +115,29 @@ describe("resolveLocalPath", () => {
96115
expect(result).toEqual({ ok: false, reason: "invalid" });
97116
});
98117

99-
it("rejects a symlink pointing outside the roots", async () => {
118+
it("rejects a symlink pointing outside the roots", async (ctx) => {
100119
const link = path.join(tmpDir, "escape");
101-
await fs.symlink("/etc/passwd", link);
120+
if (!(await trySymlink("/etc/passwd", link))) return ctx.skip();
102121
const result = await resolveLocalPath(link, [tmpDir]);
103122
expect(result).toEqual({ ok: false, reason: "outside_roots" });
104123
});
105124

106-
it("rejects a leaf reached through a symlinked parent", async () => {
107-
await fs.symlink("/etc", path.join(tmpDir, "outside"));
125+
it("rejects a leaf reached through a symlinked parent", async (ctx) => {
126+
if (!(await trySymlink("/etc", path.join(tmpDir, "outside")))) {
127+
return ctx.skip();
128+
}
108129
const result = await resolveLocalPath(
109130
path.join(tmpDir, "outside", "passwd"),
110131
[tmpDir]
111132
);
112133
expect(result).toEqual({ ok: false, reason: "outside_roots" });
113134
});
114135

115-
it("allows a symlink that stays inside the roots", async () => {
136+
it("allows a symlink that stays inside the roots", async (ctx) => {
116137
const target = path.join(tmpDir, "real.txt");
117138
await fs.writeFile(target, "x");
118139
const link = path.join(tmpDir, "alias.txt");
119-
await fs.symlink(target, link);
140+
if (!(await trySymlink(target, link))) return ctx.skip();
120141
const result = await resolveLocalPath(link, [tmpDir]);
121142
expect(result).toEqual({ ok: true, path: link });
122143
});

packages/websocket/tests/models-api-coverage.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,30 @@
99
* outbound fetch (check_servers is left off), and production paths that would
1010
* import HF helpers return early with []/false.
1111
*/
12-
import { describe, it, expect, beforeEach, afterEach } from "vitest";
12+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
13+
14+
// Keep the local-scope routes hermetic: on a developer machine the real
15+
// readCachedHfModels scans a (potentially huge) HF cache and blows the test
16+
// timeout; deleteCachedHfModel would touch the real cache.
17+
vi.mock("@nodetool-ai/huggingface", async (orig) => {
18+
const actual = await orig<typeof import("@nodetool-ai/huggingface")>();
19+
return {
20+
...actual,
21+
readCachedHfModels: vi.fn().mockResolvedValue([]),
22+
deleteCachedHfModel: vi.fn().mockResolvedValue(true)
23+
};
24+
});
25+
26+
// No provider is "configured" in tests: the real secret store on a developer
27+
// machine holds API keys, which turns GET /all into live provider calls.
28+
vi.mock("@nodetool-ai/models", async (orig) => {
29+
const actual = await orig<typeof import("@nodetool-ai/models")>();
30+
return {
31+
...actual,
32+
getSecret: vi.fn().mockResolvedValue(null)
33+
};
34+
});
35+
1336
import {
1437
handleModelsApiRequest,
1538
toUnifiedModelsFromLanguage,

packages/websocket/tests/models-api-worker.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@
77
* /ws/download socket sink.
88
*/
99
import { describe, it, expect, vi } from "vitest";
10+
11+
// Keep the local-scope routes hermetic: on a developer machine the real
12+
// readCachedHfModels scans a (potentially huge) HF cache and blows the test
13+
// timeout; deleteCachedHfModel would touch the real cache.
14+
vi.mock("@nodetool-ai/huggingface", async (orig) => {
15+
const actual = await orig<typeof import("@nodetool-ai/huggingface")>();
16+
return {
17+
...actual,
18+
readCachedHfModels: vi.fn().mockResolvedValue([]),
19+
deleteCachedHfModel: vi.fn().mockResolvedValue(true)
20+
};
21+
});
22+
1023
import {
1124
handleModelsApiRequest,
1225
relayWorkerDownload,

packages/websocket/tests/resolve-media-urls-coverage.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { pathToFileURL } from "node:url";
3+
4+
// The source resolves paths through pathToFileURL, which on Windows prefixes
5+
// the current drive (file:///C:/var/...). Build expectations the same way.
6+
const fileUri = (name: string) => pathToFileURL(`/var/assets/${name}`).href;
27

38
vi.mock("@nodetool-ai/config", () => ({
49
buildAssetUrl: (name: string) => `https://assets.test/${name}`,
@@ -120,7 +125,7 @@ describe("resolveContentForProvider", () => {
120125
it("resolves image asset_id to a file:// URI", () => {
121126
const content = [{ type: "image", image: { asset_id: "abc", mimeType: "image/png" } }];
122127
const out = resolveContentForProvider(content) as any[];
123-
expect(out[0].image.uri).toBe("file:///var/assets/abc.png");
128+
expect(out[0].image.uri).toBe(fileUri("abc.png"));
124129
});
125130

126131
it("does not overwrite an existing uri", () => {
@@ -134,13 +139,13 @@ describe("resolveContentForProvider", () => {
134139
it("resolves video to file:// URI with mp4 fallback", () => {
135140
const content = [{ type: "video", video: { asset_id: "v" } }];
136141
const out = resolveContentForProvider(content) as any[];
137-
expect(out[0].video.uri).toBe("file:///var/assets/v.mp4");
142+
expect(out[0].video.uri).toBe(fileUri("v.mp4"));
138143
});
139144

140145
it("resolves audio to file:// URI with wav fallback", () => {
141146
const content = [{ type: "audio", audio: { asset_id: "a" } }];
142147
const out = resolveContentForProvider(content) as any[];
143-
expect(out[0].audio.uri).toBe("file:///var/assets/a.wav");
148+
expect(out[0].audio.uri).toBe(fileUri("a.wav"));
144149
});
145150

146151
it("passes through primitives and unknown blocks", () => {
@@ -154,6 +159,6 @@ describe("resolveContentForProvider", () => {
154159
{ type: "audio", audio: { asset_id: "a", mimeType: "audio/mpeg" } }
155160
];
156161
const out = resolveContentForProvider(content) as any[];
157-
expect(out[0].audio.uri).toBe("file:///var/assets/a.mp3");
162+
expect(out[0].audio.uri).toBe(fileUri("a.mp3"));
158163
});
159164
});

packages/websocket/tests/resolve-media-urls-owner-prefix.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
* so every assistant-generated image 404'd on re-serve.
66
*/
77
import { describe, it, expect, vi } from "vitest";
8+
import { pathToFileURL } from "node:url";
9+
10+
// pathToFileURL prefixes the current drive on Windows; build expectations
11+
// the same way the source does.
12+
const fileUri = (key: string) => pathToFileURL(`/var/assets/${key}`).href;
813

914
const fsMocks = vi.hoisted(() => ({ existing: new Set<string>() }));
1015

@@ -66,7 +71,7 @@ describe("resolveContentForProvider with a known owner", () => {
6671
[{ type: "image", image: { asset_id: "abc", mimeType: "image/png" } }],
6772
"user-1"
6873
) as Block[];
69-
expect(out[0].image.uri).toBe("file:///var/assets/user-1/abc.png");
74+
expect(out[0].image.uri).toBe(fileUri("user-1/abc.png"));
7075
});
7176

7277
it("falls back to the flat legacy path for pre-migration objects", () => {
@@ -76,6 +81,6 @@ describe("resolveContentForProvider with a known owner", () => {
7681
[{ type: "image", image: { asset_id: "abc", mimeType: "image/png" } }],
7782
"user-1"
7883
) as Block[];
79-
expect(out[0].image.uri).toBe("file:///var/assets/abc.png");
84+
expect(out[0].image.uri).toBe(fileUri("abc.png"));
8085
});
8186
});

packages/websocket/tests/trpc-fonts.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ vi.mock("node:os", async (orig) => {
2525

2626
import { existsSync, readdirSync } from "node:fs";
2727
import { platform } from "node:os";
28+
import { join } from "node:path";
29+
30+
// The router builds the user font dir with path.join (backslashes on
31+
// Windows); mock and match it the same way.
32+
const USER_FONTS = join("/home/user", "Library", "Fonts");
2833

2934
const createCaller = createCallerFactory(appRouter);
3035

@@ -62,14 +67,14 @@ describe("fonts router", () => {
6267
(platform as ReturnType<typeof vi.fn>).mockReturnValue("darwin");
6368
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
6469
(p: string) =>
65-
p === "/Library/Fonts" || p === "/home/user/Library/Fonts"
70+
p === "/Library/Fonts" || p === USER_FONTS
6671
);
6772
(readdirSync as ReturnType<typeof vi.fn>).mockImplementation(
6873
(dir: string) => {
6974
if (dir === "/Library/Fonts") {
7075
return ["Arial.ttf", "Helvetica.otf", "Readme.txt"];
7176
}
72-
if (dir === "/home/user/Library/Fonts") {
77+
if (dir === USER_FONTS) {
7378
return ["CustomFont.ttf"];
7479
}
7580
return [];

packages/websocket/tests/trpc-mcp-config.test.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,17 @@ import {
2929
mkdirSync
3030
} from "node:fs";
3131
import { homedir } from "node:os";
32+
import { join } from "node:path";
33+
34+
// The router builds config paths with path.join, which uses backslashes on
35+
// Windows. Build the expected paths the same way (content keys inside the
36+
// JSON fixtures stay POSIX — they come from the mocked homedir()).
37+
const HOME = "/home/user";
38+
const CLAUDE_JSON = join(HOME, ".claude.json");
39+
const CODEX_DIR = join(HOME, ".codex");
40+
const CODEX_TOML = join(CODEX_DIR, "config.toml");
41+
const OPENCODE_DIR = join(HOME, ".config", "opencode");
42+
const OPENCODE_JSON = join(OPENCODE_DIR, "opencode.json");
3243

3344
const createCaller = createCallerFactory(appRouter);
3445

@@ -119,7 +130,7 @@ describe("mcpConfig router", () => {
119130

120131
it("reads claude installation when .claude.json has nodetool MCP server", async () => {
121132
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
122-
(p: string) => p === "/home/user/.claude.json"
133+
(p: string) => p === CLAUDE_JSON
123134
);
124135
(readFileSync as ReturnType<typeof vi.fn>).mockReturnValue(
125136
JSON.stringify({
@@ -138,12 +149,12 @@ describe("mcpConfig router", () => {
138149
const claude = result.targets.find((t) => t.target === "claude");
139150
expect(claude?.installed).toBe(true);
140151
expect(claude?.url).toBe("http://127.0.0.1:7777/mcp");
141-
expect(claude?.configPath).toBe("/home/user/.claude.json");
152+
expect(claude?.configPath).toBe(CLAUDE_JSON);
142153
});
143154

144155
it("reads codex installation by regex from config.toml", async () => {
145156
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
146-
(p: string) => p === "/home/user/.codex/config.toml"
157+
(p: string) => p === CODEX_TOML
147158
);
148159
(readFileSync as ReturnType<typeof vi.fn>).mockReturnValue(
149160
`# BEGIN NODETOOL MCP
@@ -162,7 +173,7 @@ url = "http://127.0.0.1:7777/mcp"
162173

163174
it("reads opencode installation from opencode.json", async () => {
164175
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
165-
(p: string) => p === "/home/user/.config/opencode/opencode.json"
176+
(p: string) => p === OPENCODE_JSON
166177
);
167178
(readFileSync as ReturnType<typeof vi.fn>).mockReturnValue(
168179
JSON.stringify({
@@ -216,7 +227,7 @@ url = "http://127.0.0.1:7777/mcp"
216227
expect(result.results).toHaveLength(1);
217228
expect(result.results[0]?.target).toBe("claude");
218229
expect(result.results[0]?.success).toBe(true);
219-
expect(result.results[0]?.configPath).toBe("/home/user/.claude.json");
230+
expect(result.results[0]?.configPath).toBe(CLAUDE_JSON);
220231
});
221232

222233
it("uses provided url when specified", async () => {
@@ -233,11 +244,11 @@ url = "http://127.0.0.1:7777/mcp"
233244
(existsSync as ReturnType<typeof vi.fn>).mockReturnValue(false);
234245
const caller = createCaller(makeCtx());
235246
await caller.mcpConfig.install({ targets: ["codex", "opencode"] });
236-
expect(mkdirSync).toHaveBeenCalledWith("/home/user/.codex", {
247+
expect(mkdirSync).toHaveBeenCalledWith(CODEX_DIR, {
237248
recursive: true
238249
});
239250
expect(mkdirSync).toHaveBeenCalledWith(
240-
"/home/user/.config/opencode",
251+
OPENCODE_DIR,
241252
{ recursive: true }
242253
);
243254
});
@@ -246,7 +257,7 @@ url = "http://127.0.0.1:7777/mcp"
246257
(existsSync as ReturnType<typeof vi.fn>).mockReturnValue(false);
247258
(writeFileSync as ReturnType<typeof vi.fn>).mockImplementation(
248259
(p: string) => {
249-
if (p === "/home/user/.claude.json") throw new Error("disk full");
260+
if (p === CLAUDE_JSON) throw new Error("disk full");
250261
}
251262
);
252263

@@ -281,7 +292,7 @@ url = "http://127.0.0.1:7777/mcp"
281292

282293
it("removes nodetool entry from .claude.json when present", async () => {
283294
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
284-
(p: string) => p === "/home/user/.claude.json"
295+
(p: string) => p === CLAUDE_JSON
285296
);
286297
(readFileSync as ReturnType<typeof vi.fn>).mockReturnValue(
287298
JSON.stringify({
@@ -310,7 +321,7 @@ url = "http://127.0.0.1:7777/mcp"
310321

311322
it("removes block from codex config.toml", async () => {
312323
(existsSync as ReturnType<typeof vi.fn>).mockImplementation(
313-
(p: string) => p === "/home/user/.codex/config.toml"
324+
(p: string) => p === CODEX_TOML
314325
);
315326
(readFileSync as ReturnType<typeof vi.fn>).mockReturnValue(
316327
`[something_else]

0 commit comments

Comments
 (0)