diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index 0c33de3c..6fd8968e 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -16,6 +16,46 @@ concurrency: cancel-in-progress: true jobs: + windows-compat: + name: Windows compatibility + runs-on: windows-latest + steps: + - name: Disable automatic CRLF conversion + run: git config --global core.autocrlf false + + - name: Check out repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Set up Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.3.10 + + - name: Set up Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 22 + + - name: Install Jujutsu + uses: taiki-e/install-action@3fa6878dc4ae603f73960271565a082bf196ab96 # v2.77.2 + with: + tool: jj-cli + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Format check + run: bun run format:check + + - name: Lint + run: bun run lint + + - name: Typecheck + run: bun run typecheck + + - name: Test suite + run: bun test ./src ./packages ./scripts ./test/cli ./test/session + pr-validate: name: Typecheck + Test + Smoke runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 31f66ae5..2e9aa120 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,6 +118,12 @@ CLI input - For CLI, config, or pager work: make sure the relevant source invocation still works (`diff`, `show`, `patch`, or `pager`). - Preserve current interaction model unless the user asks to change it explicitly. +## cross-platform support + +- 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). +- 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. +- 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. + ## releases - Maintain the top-level `CHANGELOG.md` as the source of truth for user-visible changes. diff --git a/packages/session-broker/src/daemon.ts b/packages/session-broker/src/daemon.ts index 340abbb5..cbea9a9a 100644 --- a/packages/session-broker/src/daemon.ts +++ b/packages/session-broker/src/daemon.ts @@ -315,8 +315,6 @@ export class SessionBrokerDaemon< this.shutdown(); }, remainingMs); - - this.idleTimer.unref?.(); } private async handleApiRequest(request: Request) { diff --git a/scripts/prebuilt-package-helpers.test.ts b/scripts/prebuilt-package-helpers.test.ts index 25d5e8da..5dc6d13a 100644 --- a/scripts/prebuilt-package-helpers.test.ts +++ b/scripts/prebuilt-package-helpers.test.ts @@ -26,6 +26,9 @@ describe("prebuilt package helpers", () => { test("binaryFilenameForSpec keeps unix package binaries extensionless", () => { for (const spec of PLATFORM_PACKAGE_MATRIX) { + if (spec.os === "windows") { + continue; + } expect(binaryFilenameForSpec(spec)).toBe("hunk"); } }); @@ -106,6 +109,7 @@ describe("prebuilt package helpers", () => { "hunkdiff-darwin-x64", "hunkdiff-linux-arm64", "hunkdiff-linux-x64", + "hunkdiff-windows-x64", ]); }); }); diff --git a/scripts/prebuilt-package-helpers.ts b/scripts/prebuilt-package-helpers.ts index 1a473ec8..b71f6d29 100644 --- a/scripts/prebuilt-package-helpers.ts +++ b/scripts/prebuilt-package-helpers.ts @@ -55,6 +55,13 @@ export const PLATFORM_PACKAGE_MATRIX: PlatformPackageSpec[] = [ binaryName: "hunk", binaryRelativePath: "bin/hunk", }, + { + packageName: "hunkdiff-windows-x64", + os: "windows", + cpu: "x64", + binaryName: "hunk", + binaryRelativePath: "bin/hunk.exe", + }, ] as const; /** Normalize a Node platform string into Hunk's package naming vocabulary. */ diff --git a/src/core/cli.test.ts b/src/core/cli.test.ts index 58419b25..0e752027 100644 --- a/src/core/cli.test.ts +++ b/src/core/cli.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { parseCli } from "./cli"; import { resolveCliVersion } from "./version"; @@ -371,8 +371,8 @@ describe("parseCli", () => { expect(parsed).toEqual({ kind: "session", action: "reload", - selector: { sessionPath: "/tmp/live-window" }, - sourcePath: "/tmp/source-repo", + selector: { sessionPath: resolve("/tmp/live-window") }, + sourcePath: resolve("/tmp/source-repo"), nextInput: { kind: "vcs", staged: false, @@ -626,7 +626,7 @@ describe("parseCli", () => { expect(parsed).toEqual({ kind: "session", action: "navigate", - selector: { repoRoot: "/tmp/repo" }, + selector: { repoRoot: resolve("/tmp/repo") }, commentDirection: "next", output: "text", }); diff --git a/src/core/loaders.test.ts b/src/core/loaders.test.ts index a858cbae..ad1b7717 100644 --- a/src/core/loaders.test.ts +++ b/src/core/loaders.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { platform, tmpdir } from "node:os"; import { join } from "node:path"; import { loadAppBootstrap } from "./loaders"; import type { CliInput } from "./types"; @@ -22,6 +22,12 @@ function createTempDir(prefix: string) { return dir; } +/** Normalize Windows short/long temp path spellings before path equality assertions. */ +function normalizeComparablePath(path: string) { + const resolvedPath = platform() === "win32" ? realpathSync.native(path) : path; + return resolvedPath.replace(/\\/g, "/"); +} + function git(cwd: string, ...cmd: string[]) { const proc = Bun.spawnSync(["git", ...cmd], { cwd, @@ -201,7 +207,9 @@ describe("loadAppBootstrap", () => { ), ); - expect(bootstrap.changeset.sourceLabel).toBe(dir); + expect(normalizeComparablePath(bootstrap.changeset.sourceLabel)).toBe( + normalizeComparablePath(dir), + ); expect(bootstrap.changeset.files[0]?.path).toBe("example.ts"); expect(bootstrap.changeset.files[0]?.agent?.annotations).toHaveLength(1); }); @@ -411,7 +419,7 @@ describe("loadAppBootstrap", () => { writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n"); git(dir, "add", "tracked.ts"); git(dir, "commit", "-m", "initial"); - git(dir, "branch", "main"); + git(dir, "branch", "base-branch"); writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n"); git(dir, "add", "tracked.ts"); @@ -422,7 +430,7 @@ describe("loadAppBootstrap", () => { const bootstrap = await loadFromRepo(dir, { kind: "vcs", - range: "main", + range: "base-branch", staged: false, options: { mode: "auto" }, }); @@ -439,7 +447,7 @@ describe("loadAppBootstrap", () => { writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n"); git(dir, "add", "tracked.ts"); git(dir, "commit", "-m", "initial"); - git(dir, "branch", "main"); + git(dir, "branch", "base-branch"); writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n"); git(dir, "add", "tracked.ts"); @@ -450,7 +458,7 @@ describe("loadAppBootstrap", () => { const bootstrap = await loadFromRepo(dir, { kind: "vcs", - range: "main..HEAD", + range: "base-branch..HEAD", staged: false, options: { mode: "auto" }, }); @@ -489,12 +497,13 @@ describe("loadAppBootstrap", () => { git(dir, "add", "tracked.ts"); git(dir, "commit", "-m", "initial"); - const quoteFile = 'quote"name.txt'; - const tabFile = "tab\tname.txt"; - const backslashFile = "back\\slash.txt"; - writeFileSync(join(dir, quoteFile), "quote\n"); - writeFileSync(join(dir, tabFile), "tab\n"); - writeFileSync(join(dir, backslashFile), "backslash\n"); + const portableFiles = ["space name.txt"]; + const unixOnlyFiles = ['quote"name.txt', "tab\tname.txt", "back\\slash.txt"]; + const fixtureFiles = + platform() === "win32" ? portableFiles : [...portableFiles, ...unixOnlyFiles]; + for (const file of fixtureFiles) { + writeFileSync(join(dir, file), `${file}\n`); + } const bootstrap = await loadFromRepo(dir, { kind: "vcs", @@ -503,10 +512,10 @@ describe("loadAppBootstrap", () => { }); const paths = bootstrap.changeset.files.map((file) => file.path); - expect(paths).toContain(quoteFile); - expect(paths).toContain(tabFile); - expect(paths).toContain(backslashFile); - expect(paths).toHaveLength(3); + for (const file of fixtureFiles) { + expect(paths).toContain(file); + } + expect(paths).toHaveLength(fixtureFiles.length); }); test("still shows an untracked agent sidecar when it lives inside the repo", async () => { @@ -1032,7 +1041,7 @@ describe("loadAppBootstrap", () => { writeFileSync(after, "export const answer = 42;\nexport const added = true;\n"); const diffProc = Bun.spawnSync( - ["git", "diff", "--no-index", "--color=always", "--", before, after], + ["git", "diff", "--no-index", "--color=always", "--", "before.ts", "after.ts"], { cwd: dir, stdin: "ignore", @@ -1103,17 +1112,16 @@ describe("loadAppBootstrap", () => { }); test("loads quoted noprefix patch text emitted for escaped git paths", async () => { - const dir = createTempRepo("hunk-patch-quoted-noprefix-"); - const fileName = "src\tfile.txt"; - - writeFileSync(join(dir, fileName), "one\n"); - git(dir, "add", "."); - git(dir, "commit", "-m", "initial"); - - writeFileSync(join(dir, fileName), "two\n"); - const patchText = git(dir, "-c", "diff.noprefix=true", "diff", "--", fileName); - - expect(patchText).toContain('diff --git "src\\tfile.txt" "src\\tfile.txt"'); + const patchText = [ + 'diff --git "src\\tfile.txt" "src\\tfile.txt"', + "index 5626abf..f719efd 100644", + '--- "src\\tfile.txt"', + '+++ "src\\tfile.txt"', + "@@ -1 +1 @@", + "-one", + "+two", + "", + ].join("\n"); const bootstrap = await loadAppBootstrap({ kind: "patch", diff --git a/src/core/loaders.ts b/src/core/loaders.ts index 900478f5..2f2ba16e 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -53,7 +53,7 @@ const LARGE_DIFF_FILE_SNIFF_BYTES = 256 * 1024; /** Return the final path segment for display-oriented labels. */ function basename(path: string) { - return path.split("/").filter(Boolean).pop() ?? path; + return path.split(/[\\/]/).filter(Boolean).pop() ?? path; } /** Remove git-style a/ and b/ prefixes before matching diff paths. */ diff --git a/src/core/paths.test.ts b/src/core/paths.test.ts index 4d0b1747..6badeed7 100644 --- a/src/core/paths.test.ts +++ b/src/core/paths.test.ts @@ -14,17 +14,19 @@ function createTempRoot(prefix: string) { describe("paths", () => { test("resolves XDG config and state paths", () => { - const env = { XDG_CONFIG_HOME: "/tmp/xdg-home" } as NodeJS.ProcessEnv; + const env = { XDG_CONFIG_HOME: join("/tmp", "xdg-home") } as NodeJS.ProcessEnv; - expect(resolveGlobalConfigPath(env)).toBe("/tmp/xdg-home/hunk/config.toml"); - expect(resolveHunkStatePath(env)).toBe("/tmp/xdg-home/hunk/state.json"); + expect(resolveGlobalConfigPath(env)).toBe(join("/tmp", "xdg-home", "hunk", "config.toml")); + expect(resolveHunkStatePath(env)).toBe(join("/tmp", "xdg-home", "hunk", "state.json")); }); test("falls back to HOME for config and state paths", () => { - const env = { HOME: "/tmp/home" } as NodeJS.ProcessEnv; + const env = { HOME: join("/tmp", "home") } as NodeJS.ProcessEnv; - expect(resolveGlobalConfigPath(env)).toBe("/tmp/home/.config/hunk/config.toml"); - expect(resolveHunkStatePath(env)).toBe("/tmp/home/.config/hunk/state.json"); + expect(resolveGlobalConfigPath(env)).toBe( + join("/tmp", "home", ".config", "hunk", "config.toml"), + ); + expect(resolveHunkStatePath(env)).toBe(join("/tmp", "home", ".config", "hunk", "state.json")); }); test("locates the bundled Hunk review skill from source", () => { diff --git a/src/core/updateNotice.ts b/src/core/updateNotice.ts index 1c2311ec..772ad3dc 100644 --- a/src/core/updateNotice.ts +++ b/src/core/updateNotice.ts @@ -148,7 +148,6 @@ function createFetchTimeoutSignal(timeoutMs: number) { const timeout = setTimeout(() => { controller.abort(); }, timeoutMs); - timeout.unref?.(); return { signal: controller.signal, diff --git a/src/session-broker/brokerLauncher.test.ts b/src/session-broker/brokerLauncher.test.ts index 21e94c1b..997d88f6 100644 --- a/src/session-broker/brokerLauncher.test.ts +++ b/src/session-broker/brokerLauncher.test.ts @@ -1,7 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; import type { ChildProcess } from "node:child_process"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -73,22 +72,21 @@ describe("session daemon launcher", () => { }); test("detects whether some process is already listening on the daemon port", async () => { - const listener = createServer(() => undefined); - await new Promise((resolve, reject) => { - listener.once("error", reject); - listener.listen(0, "127.0.0.1", () => resolve()); + const listener = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch: () => new Response("ok"), }); - - const address = listener.address(); - const port = typeof address === "object" && address ? address.port : 0; + const port = listener.port; + expect(port).toBeDefined(); try { - await expect(isLoopbackPortReachable({ host: "127.0.0.1", port })).resolves.toBe(true); + await expect(isLoopbackPortReachable({ host: "127.0.0.1", port: port! })).resolves.toBe(true); } finally { - await new Promise((resolve) => listener.close(() => resolve())); + listener.stop(true); } - await expect(isLoopbackPortReachable({ host: "127.0.0.1", port })).resolves.toBe(false); + await expect(isLoopbackPortReachable({ host: "127.0.0.1", port: port! })).resolves.toBe(false); }); test("coordinates concurrent ensure calls so only one launcher runs", async () => { diff --git a/src/session-broker/brokerServer.test.ts b/src/session-broker/brokerServer.test.ts index 1537f1b8..e262802c 100644 --- a/src/session-broker/brokerServer.test.ts +++ b/src/session-broker/brokerServer.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; import { createServer } from "node:net"; +import { platform } from "node:os"; import { createTestSessionRegistration, createTestSessionSnapshot, @@ -93,7 +94,6 @@ async function openSessionSocket(port: number) { () => reject(new Error("Timed out waiting for websocket open.")), 500, ); - timeout.unref?.(); socket.addEventListener( "open", @@ -141,7 +141,6 @@ async function waitForSocketClose(socket: WebSocket) { () => reject(new Error("Timed out waiting for websocket close.")), 1_000, ); - timeout.unref?.(); socket.addEventListener( "close", @@ -247,6 +246,12 @@ describe("Hunk session daemon server", () => { }); test("closes snapshots for missing sessions with a specific not-registered reason", async () => { + // Bun's Windows WebSocket client does not reliably surface this immediate server close. + // The daemon-core test covers the close code/reason without the flaky transport layer. + if (platform() === "win32") { + return; + } + const port = await reserveLoopbackPort(); process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); diff --git a/src/session/commands.test.ts b/src/session/commands.test.ts index 0845a8cf..6afdba4c 100644 --- a/src/session/commands.test.ts +++ b/src/session/commands.test.ts @@ -657,7 +657,10 @@ describe("session command compatibility checks", () => { createClient: () => createClient({ reloadSession: async (input) => { - expect(input.selector).toEqual({ sessionPath: "/live-session" }); + expect(input.selector).toEqual({ + repoRoot: undefined, + sessionPath: resolve("/live-session"), + }); expect(input.sourcePath).toBe("/source-repo"); expect(input.nextInput).toEqual({ kind: "vcs", diff --git a/src/ui/hooks/useStartupUpdateNotice.test.tsx b/src/ui/hooks/useStartupUpdateNotice.test.tsx index 1a32cce6..701924f2 100644 --- a/src/ui/hooks/useStartupUpdateNotice.test.tsx +++ b/src/ui/hooks/useStartupUpdateNotice.test.tsx @@ -44,8 +44,7 @@ function ResolverSwapHarness({ onNoticeText }: { onNoticeText?: (value: string | useEffect(() => { const timer = setTimeout(() => { setUseSecondResolver(true); - }, 5); - timer.unref?.(); + }, 0); return () => { clearTimeout(timer); @@ -62,7 +61,7 @@ function ResolverSwapHarness({ onNoticeText }: { onNoticeText?: (value: string | return ( { try { await advance(setup, 0); await advance(setup, 10); - await advance(setup, 20); + await advance(setup, 60); expect(seen).toContain("Update available: 2.0.0"); expect(seen).not.toContain("Update available: 1.0.0");