test(websocket): make the suite pass on Windows dev machines - #4709
Merged
Conversation
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the packages/websocket Vitest suite to be Windows-compatible by removing POSIX-only filesystem/path assumptions and making several tests hermetic against developer-machine state (secrets + HF cache), while preserving Linux CI behavior.
Changes:
- Make path expectations platform-correct (drive-prefixed
file://URIs on Windows,path.joinseparators, and path segment checks viabasename). - Make tests hermetic by mocking HuggingFace cache access and secret lookup to avoid live provider calls / large local cache scans.
- Reduce Windows flakiness by skipping symlink tests when forbidden and increasing timeouts where cold imports are slow.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/websocket/tests/trpc-workspace.test.ts | Avoid POSIX endsWith("/...") assumptions by checking path segments with basename() in stat mocks. |
| packages/websocket/tests/trpc-packs.test.ts | Increase per-test timeouts for slow cold-import pack loading on Windows. |
| packages/websocket/tests/trpc-models-worker.test.ts | Mock HF cache helpers to prevent scanning a real developer cache in local scope. |
| packages/websocket/tests/trpc-mcp-config.test.ts | Build expected config paths with path.join to match platform separators. |
| packages/websocket/tests/trpc-fonts.test.ts | Build expected user font dir with path.join for Windows separators. |
| packages/websocket/tests/resolve-media-urls-owner-prefix.test.ts | Build expected file:// URIs using pathToFileURL (Windows drive prefix). |
| packages/websocket/tests/resolve-media-urls-coverage.test.ts | Same pathToFileURL expectation fix for file:// URIs. |
| packages/websocket/tests/models-api-worker.test.ts | Mock HF cache helpers to keep local-scope routes hermetic. |
| packages/websocket/tests/models-api-coverage.test.ts | Mock HF cache helpers + secret store lookup to prevent real cache scans and live provider calls. |
| packages/websocket/tests/local-file-access.test.ts | Add trySymlink helper and skip symlink tests on Windows when symlink creation is denied. |
| packages/websocket/tests/generation-autosave-cutover.test.ts | Increase waitForMessages timeout ceiling to reduce Windows slowness flakes. |
| packages/websocket/tests/file-api.test.ts | Add trySymlink helper and adjust “default root” test to use a path guaranteed outside home across platforms. |
Suppressed comments (1)
packages/websocket/tests/file-api.test.ts:175
- The symlink-escape tests create symlinks targeting
/etcand/etc/passwd. On Windows machines where symlink creation succeeds, those targets generally don’t exist, soresolveLocalPathcan’trealpaththrough the symlink and the handler may return 404 (missing target) instead of 403 (escape) — making these tests non-hermetic/flaky. Use a real temp directory/file outsidetmpDiras the symlink target so the escape check is exercised cross-platform.
it("denies a symlink that escapes the roots", async (ctx) => {
const link = path.join(tmpDir, "escape.txt");
if (!(await trySymlink("/etc/passwd", link))) return ctx.skip();
const res = await handleFileRequest(localRequest(link));
expect(res.status).toBe(403);
});
it("denies a path that escapes through a symlinked parent", async (ctx) => {
const linkDir = path.join(tmpDir, "outside");
if (!(await trySymlink("/etc", linkDir))) return ctx.skip();
const res = await handleFileRequest(
localRequest(path.join(linkDir, "passwd"))
);
expect(res.status).toBe(403);
});
Comment on lines
+118
to
+128
| it("rejects a symlink pointing outside the roots", async (ctx) => { | ||
| const link = path.join(tmpDir, "escape"); | ||
| await fs.symlink("/etc/passwd", link); | ||
| if (!(await trySymlink("/etc/passwd", link))) return ctx.skip(); | ||
| const result = await resolveLocalPath(link, [tmpDir]); | ||
| expect(result).toEqual({ ok: false, reason: "outside_roots" }); | ||
| }); | ||
|
|
||
| it("rejects a leaf reached through a symlinked parent", async () => { | ||
| await fs.symlink("/etc", path.join(tmpDir, "outside")); | ||
| it("rejects a leaf reached through a symlinked parent", async (ctx) => { | ||
| if (!(await trySymlink("/etc", path.join(tmpDir, "outside")))) { | ||
| return ctx.skip(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five classes of environment assumptions broke 23 tests on Windows; all fixes keep the tests running unchanged on Linux CI.
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).