Skip to content

Commit f0ac68b

Browse files
fix(windows): Windows compatibility fixes for paths, packaging, and tests (#257)
Co-authored-by: Ben Vinegar <ben@benv.ca>
1 parent 5d7c231 commit f0ac68b

14 files changed

Lines changed: 129 additions & 60 deletions

File tree

.github/workflows/pr-ci.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,46 @@ concurrency:
1616
cancel-in-progress: true
1717

1818
jobs:
19+
windows-compat:
20+
name: Windows compatibility
21+
runs-on: windows-latest
22+
steps:
23+
- name: Disable automatic CRLF conversion
24+
run: git config --global core.autocrlf false
25+
26+
- name: Check out repository
27+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
28+
29+
- name: Set up Bun
30+
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
31+
with:
32+
bun-version: 1.3.10
33+
34+
- name: Set up Node
35+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
36+
with:
37+
node-version: 22
38+
39+
- name: Install Jujutsu
40+
uses: taiki-e/install-action@3fa6878dc4ae603f73960271565a082bf196ab96 # v2.77.2
41+
with:
42+
tool: jj-cli
43+
44+
- name: Install dependencies
45+
run: bun install --frozen-lockfile
46+
47+
- name: Format check
48+
run: bun run format:check
49+
50+
- name: Lint
51+
run: bun run lint
52+
53+
- name: Typecheck
54+
run: bun run typecheck
55+
56+
- name: Test suite
57+
run: bun test ./src ./packages ./scripts ./test/cli ./test/session
58+
1959
pr-validate:
2060
name: Typecheck + Test + Smoke
2161
runs-on: ubuntu-latest

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ CLI input
118118
- For CLI, config, or pager work: make sure the relevant source invocation still works (`diff`, `show`, `patch`, or `pager`).
119119
- Preserve current interaction model unless the user asks to change it explicitly.
120120

121+
## cross-platform support
122+
123+
- Hunk should work on macOS, Linux, and Windows. Keep tests and CI portable unless a case is explicitly Unix-only (PTY/TTY smoke coverage is Unix-only).
124+
- In tests, avoid hard-coded POSIX paths, separators, shell syntax, and filenames invalid on Windows; use Node path helpers for real filesystem paths while preserving user-provided/protocol paths when pass-through is intentional.
125+
- If Windows-only Bun behavior appears around timers, sockets, or line endings, prefer a small compatibility fix or a narrowly scoped skip with a comment over broadening Unix assumptions.
126+
121127
## releases
122128

123129
- Maintain the top-level `CHANGELOG.md` as the source of truth for user-visible changes.

packages/session-broker/src/daemon.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,6 @@ export class SessionBrokerDaemon<
315315

316316
this.shutdown();
317317
}, remainingMs);
318-
319-
this.idleTimer.unref?.();
320318
}
321319

322320
private async handleApiRequest(request: Request) {

scripts/prebuilt-package-helpers.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ describe("prebuilt package helpers", () => {
2626

2727
test("binaryFilenameForSpec keeps unix package binaries extensionless", () => {
2828
for (const spec of PLATFORM_PACKAGE_MATRIX) {
29+
if (spec.os === "windows") {
30+
continue;
31+
}
2932
expect(binaryFilenameForSpec(spec)).toBe("hunk");
3033
}
3134
});
@@ -106,6 +109,7 @@ describe("prebuilt package helpers", () => {
106109
"hunkdiff-darwin-x64",
107110
"hunkdiff-linux-arm64",
108111
"hunkdiff-linux-x64",
112+
"hunkdiff-windows-x64",
109113
]);
110114
});
111115
});

scripts/prebuilt-package-helpers.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ export const PLATFORM_PACKAGE_MATRIX: PlatformPackageSpec[] = [
5555
binaryName: "hunk",
5656
binaryRelativePath: "bin/hunk",
5757
},
58+
{
59+
packageName: "hunkdiff-windows-x64",
60+
os: "windows",
61+
cpu: "x64",
62+
binaryName: "hunk",
63+
binaryRelativePath: "bin/hunk.exe",
64+
},
5865
] as const;
5966

6067
/** Normalize a Node platform string into Hunk's package naming vocabulary. */

src/core/cli.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { afterEach, describe, expect, test } from "bun:test";
22
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
4-
import { join } from "node:path";
4+
import { join, resolve } from "node:path";
55
import { parseCli } from "./cli";
66
import { resolveCliVersion } from "./version";
77

@@ -371,8 +371,8 @@ describe("parseCli", () => {
371371
expect(parsed).toEqual({
372372
kind: "session",
373373
action: "reload",
374-
selector: { sessionPath: "/tmp/live-window" },
375-
sourcePath: "/tmp/source-repo",
374+
selector: { sessionPath: resolve("/tmp/live-window") },
375+
sourcePath: resolve("/tmp/source-repo"),
376376
nextInput: {
377377
kind: "vcs",
378378
staged: false,
@@ -626,7 +626,7 @@ describe("parseCli", () => {
626626
expect(parsed).toEqual({
627627
kind: "session",
628628
action: "navigate",
629-
selector: { repoRoot: "/tmp/repo" },
629+
selector: { repoRoot: resolve("/tmp/repo") },
630630
commentDirection: "next",
631631
output: "text",
632632
});

src/core/loaders.test.ts

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { afterEach, describe, expect, test } from "bun:test";
22
import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
3-
import { tmpdir } from "node:os";
3+
import { platform, tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { loadAppBootstrap } from "./loaders";
66
import type { CliInput } from "./types";
@@ -22,6 +22,12 @@ function createTempDir(prefix: string) {
2222
return dir;
2323
}
2424

25+
/** Normalize Windows short/long temp path spellings before path equality assertions. */
26+
function normalizeComparablePath(path: string) {
27+
const resolvedPath = platform() === "win32" ? realpathSync.native(path) : path;
28+
return resolvedPath.replace(/\\/g, "/");
29+
}
30+
2531
function git(cwd: string, ...cmd: string[]) {
2632
const proc = Bun.spawnSync(["git", ...cmd], {
2733
cwd,
@@ -201,7 +207,9 @@ describe("loadAppBootstrap", () => {
201207
),
202208
);
203209

204-
expect(bootstrap.changeset.sourceLabel).toBe(dir);
210+
expect(normalizeComparablePath(bootstrap.changeset.sourceLabel)).toBe(
211+
normalizeComparablePath(dir),
212+
);
205213
expect(bootstrap.changeset.files[0]?.path).toBe("example.ts");
206214
expect(bootstrap.changeset.files[0]?.agent?.annotations).toHaveLength(1);
207215
});
@@ -411,7 +419,7 @@ describe("loadAppBootstrap", () => {
411419
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
412420
git(dir, "add", "tracked.ts");
413421
git(dir, "commit", "-m", "initial");
414-
git(dir, "branch", "main");
422+
git(dir, "branch", "base-branch");
415423

416424
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
417425
git(dir, "add", "tracked.ts");
@@ -422,7 +430,7 @@ describe("loadAppBootstrap", () => {
422430

423431
const bootstrap = await loadFromRepo(dir, {
424432
kind: "vcs",
425-
range: "main",
433+
range: "base-branch",
426434
staged: false,
427435
options: { mode: "auto" },
428436
});
@@ -439,7 +447,7 @@ describe("loadAppBootstrap", () => {
439447
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
440448
git(dir, "add", "tracked.ts");
441449
git(dir, "commit", "-m", "initial");
442-
git(dir, "branch", "main");
450+
git(dir, "branch", "base-branch");
443451

444452
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
445453
git(dir, "add", "tracked.ts");
@@ -450,7 +458,7 @@ describe("loadAppBootstrap", () => {
450458

451459
const bootstrap = await loadFromRepo(dir, {
452460
kind: "vcs",
453-
range: "main..HEAD",
461+
range: "base-branch..HEAD",
454462
staged: false,
455463
options: { mode: "auto" },
456464
});
@@ -489,12 +497,13 @@ describe("loadAppBootstrap", () => {
489497
git(dir, "add", "tracked.ts");
490498
git(dir, "commit", "-m", "initial");
491499

492-
const quoteFile = 'quote"name.txt';
493-
const tabFile = "tab\tname.txt";
494-
const backslashFile = "back\\slash.txt";
495-
writeFileSync(join(dir, quoteFile), "quote\n");
496-
writeFileSync(join(dir, tabFile), "tab\n");
497-
writeFileSync(join(dir, backslashFile), "backslash\n");
500+
const portableFiles = ["space name.txt"];
501+
const unixOnlyFiles = ['quote"name.txt', "tab\tname.txt", "back\\slash.txt"];
502+
const fixtureFiles =
503+
platform() === "win32" ? portableFiles : [...portableFiles, ...unixOnlyFiles];
504+
for (const file of fixtureFiles) {
505+
writeFileSync(join(dir, file), `${file}\n`);
506+
}
498507

499508
const bootstrap = await loadFromRepo(dir, {
500509
kind: "vcs",
@@ -503,10 +512,10 @@ describe("loadAppBootstrap", () => {
503512
});
504513
const paths = bootstrap.changeset.files.map((file) => file.path);
505514

506-
expect(paths).toContain(quoteFile);
507-
expect(paths).toContain(tabFile);
508-
expect(paths).toContain(backslashFile);
509-
expect(paths).toHaveLength(3);
515+
for (const file of fixtureFiles) {
516+
expect(paths).toContain(file);
517+
}
518+
expect(paths).toHaveLength(fixtureFiles.length);
510519
});
511520

512521
test("still shows an untracked agent sidecar when it lives inside the repo", async () => {
@@ -1032,7 +1041,7 @@ describe("loadAppBootstrap", () => {
10321041
writeFileSync(after, "export const answer = 42;\nexport const added = true;\n");
10331042

10341043
const diffProc = Bun.spawnSync(
1035-
["git", "diff", "--no-index", "--color=always", "--", before, after],
1044+
["git", "diff", "--no-index", "--color=always", "--", "before.ts", "after.ts"],
10361045
{
10371046
cwd: dir,
10381047
stdin: "ignore",
@@ -1185,17 +1194,16 @@ describe("loadAppBootstrap", () => {
11851194
});
11861195

11871196
test("loads quoted noprefix patch text emitted for escaped git paths", async () => {
1188-
const dir = createTempRepo("hunk-patch-quoted-noprefix-");
1189-
const fileName = "src\tfile.txt";
1190-
1191-
writeFileSync(join(dir, fileName), "one\n");
1192-
git(dir, "add", ".");
1193-
git(dir, "commit", "-m", "initial");
1194-
1195-
writeFileSync(join(dir, fileName), "two\n");
1196-
const patchText = git(dir, "-c", "diff.noprefix=true", "diff", "--", fileName);
1197-
1198-
expect(patchText).toContain('diff --git "src\\tfile.txt" "src\\tfile.txt"');
1197+
const patchText = [
1198+
'diff --git "src\\tfile.txt" "src\\tfile.txt"',
1199+
"index 5626abf..f719efd 100644",
1200+
'--- "src\\tfile.txt"',
1201+
'+++ "src\\tfile.txt"',
1202+
"@@ -1 +1 @@",
1203+
"-one",
1204+
"+two",
1205+
"",
1206+
].join("\n");
11991207

12001208
const bootstrap = await loadAppBootstrap({
12011209
kind: "patch",

src/core/loaders.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const LARGE_DIFF_FILE_SNIFF_BYTES = 256 * 1024;
5353

5454
/** Return the final path segment for display-oriented labels. */
5555
function basename(path: string) {
56-
return path.split("/").filter(Boolean).pop() ?? path;
56+
return path.split(/[\\/]/).filter(Boolean).pop() ?? path;
5757
}
5858

5959
/** Remove git-style a/ and b/ prefixes before matching diff paths. */

src/core/paths.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,19 @@ function createTempRoot(prefix: string) {
1414

1515
describe("paths", () => {
1616
test("resolves XDG config and state paths", () => {
17-
const env = { XDG_CONFIG_HOME: "/tmp/xdg-home" } as NodeJS.ProcessEnv;
17+
const env = { XDG_CONFIG_HOME: join("/tmp", "xdg-home") } as NodeJS.ProcessEnv;
1818

19-
expect(resolveGlobalConfigPath(env)).toBe("/tmp/xdg-home/hunk/config.toml");
20-
expect(resolveHunkStatePath(env)).toBe("/tmp/xdg-home/hunk/state.json");
19+
expect(resolveGlobalConfigPath(env)).toBe(join("/tmp", "xdg-home", "hunk", "config.toml"));
20+
expect(resolveHunkStatePath(env)).toBe(join("/tmp", "xdg-home", "hunk", "state.json"));
2121
});
2222

2323
test("falls back to HOME for config and state paths", () => {
24-
const env = { HOME: "/tmp/home" } as NodeJS.ProcessEnv;
24+
const env = { HOME: join("/tmp", "home") } as NodeJS.ProcessEnv;
2525

26-
expect(resolveGlobalConfigPath(env)).toBe("/tmp/home/.config/hunk/config.toml");
27-
expect(resolveHunkStatePath(env)).toBe("/tmp/home/.config/hunk/state.json");
26+
expect(resolveGlobalConfigPath(env)).toBe(
27+
join("/tmp", "home", ".config", "hunk", "config.toml"),
28+
);
29+
expect(resolveHunkStatePath(env)).toBe(join("/tmp", "home", ".config", "hunk", "state.json"));
2830
});
2931

3032
test("locates the bundled Hunk review skill from source", () => {

src/core/updateNotice.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ function createFetchTimeoutSignal(timeoutMs: number) {
148148
const timeout = setTimeout(() => {
149149
controller.abort();
150150
}, timeoutMs);
151-
timeout.unref?.();
152151

153152
return {
154153
signal: controller.signal,

0 commit comments

Comments
 (0)