Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/workflows/pr-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions packages/session-broker/src/daemon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,6 @@ export class SessionBrokerDaemon<

this.shutdown();
}, remainingMs);

this.idleTimer.unref?.();
}

private async handleApiRequest(request: Request) {
Expand Down
4 changes: 4 additions & 0 deletions scripts/prebuilt-package-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
});
Expand Down Expand Up @@ -106,6 +109,7 @@ describe("prebuilt package helpers", () => {
"hunkdiff-darwin-x64",
"hunkdiff-linux-arm64",
"hunkdiff-linux-x64",
"hunkdiff-windows-x64",
]);
});
});
7 changes: 7 additions & 0 deletions scripts/prebuilt-package-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Comment on lines +58 to +64
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The Windows entry's binaryRelativePath is "bin/hunk" without the .exe extension, while the dedicated test for this spec (in prebuilt-package-helpers.test.ts) explicitly constructs its fixture with binaryRelativePath: "bin/hunk.exe". Although binaryRelativePath is not read by any current script (binaryFilenameForSpec is used instead), the two definitions are inconsistent and whoever adds code to consume binaryRelativePath for Windows will get the wrong path from the production matrix.

Suggested change
{
packageName: "hunkdiff-windows-x64",
os: "windows",
cpu: "x64",
binaryName: "hunk",
binaryRelativePath: "bin/hunk",
},
{
packageName: "hunkdiff-windows-x64",
os: "windows",
cpu: "x64",
binaryName: "hunk",
binaryRelativePath: "bin/hunk.exe",
},
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/prebuilt-package-helpers.ts
Line: 58-64

Comment:
The Windows entry's `binaryRelativePath` is `"bin/hunk"` without the `.exe` extension, while the dedicated test for this spec (in `prebuilt-package-helpers.test.ts`) explicitly constructs its fixture with `binaryRelativePath: "bin/hunk.exe"`. Although `binaryRelativePath` is not read by any current script (`binaryFilenameForSpec` is used instead), the two definitions are inconsistent and whoever adds code to consume `binaryRelativePath` for Windows will get the wrong path from the production matrix.

```suggestion
  {
    packageName: "hunkdiff-windows-x64",
    os: "windows",
    cpu: "x64",
    binaryName: "hunk",
    binaryRelativePath: "bin/hunk.exe",
  },
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 59aabcb by changing the Windows package spec to binaryRelativePath: "bin/hunk.exe", matching the dedicated helper test.

This comment was generated by Pi using OpenAI GPT-5

] as const;

/** Normalize a Node platform string into Hunk's package naming vocabulary. */
Expand Down
8 changes: 4 additions & 4 deletions src/core/cli.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
});
Expand Down
64 changes: 36 additions & 28 deletions src/core/loaders.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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");
Expand All @@ -422,7 +430,7 @@ describe("loadAppBootstrap", () => {

const bootstrap = await loadFromRepo(dir, {
kind: "vcs",
range: "main",
range: "base-branch",
staged: false,
options: { mode: "auto" },
});
Expand All @@ -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");
Expand All @@ -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" },
});
Expand Down Expand Up @@ -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",
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/core/loaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
14 changes: 8 additions & 6 deletions src/core/paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
1 change: 0 additions & 1 deletion src/core/updateNotice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ function createFetchTimeoutSignal(timeoutMs: number) {
const timeout = setTimeout(() => {
controller.abort();
}, timeoutMs);
timeout.unref?.();

return {
signal: controller.signal,
Expand Down
20 changes: 9 additions & 11 deletions src/session-broker/brokerLauncher.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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<void>((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<void>((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 () => {
Expand Down
9 changes: 7 additions & 2 deletions src/session-broker/brokerServer.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -93,7 +94,6 @@ async function openSessionSocket(port: number) {
() => reject(new Error("Timed out waiting for websocket open.")),
500,
);
timeout.unref?.();

socket.addEventListener(
"open",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading