From 46acd9ac26d5056902b78b957feb49afcc6830a9 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 13 Oct 2026 14:14:04 -0400 Subject: [PATCH 1/3] fix(cli): reject unrecognized nonempty patch input --- .changeset/recognized-patch-input.md | 5 ++ .../src/core/changeset/loaders.patch.test.ts | 57 ++++++++++++++++ .../hunk/src/core/changeset/loaders.test.ts | 28 ++++---- packages/hunk/src/core/changeset/loaders.ts | 18 ++++- .../hunk/src/core/theme/detection.test.ts | 29 +++++++- packages/hunk/src/core/theme/detection.ts | 4 ++ test/cli/patch-input.test.ts | 67 +++++++++++++++++++ test/pty/invalid-patch-integration.test.ts | 34 ++++++++++ 8 files changed, 225 insertions(+), 17 deletions(-) create mode 100644 .changeset/recognized-patch-input.md create mode 100644 packages/hunk/src/core/changeset/loaders.patch.test.ts create mode 100644 test/cli/patch-input.test.ts create mode 100644 test/pty/invalid-patch-integration.test.ts diff --git a/.changeset/recognized-patch-input.md b/.changeset/recognized-patch-input.md new file mode 100644 index 000000000..8b0432b83 --- /dev/null +++ b/.changeset/recognized-patch-input.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Reject nonempty patch input without recognizable file changes instead of silently opening an empty review. diff --git a/packages/hunk/src/core/changeset/loaders.patch.test.ts b/packages/hunk/src/core/changeset/loaders.patch.test.ts new file mode 100644 index 000000000..45492d694 --- /dev/null +++ b/packages/hunk/src/core/changeset/loaders.patch.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from "bun:test"; +import { loadAppBootstrap } from "./loaders"; + +const textPatch = + "diff --git a/a.txt b/a.txt\n--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-before\n+after\n"; + +/** Load explicit patch text through the same input boundary as files and stdin. */ +function loadTestPatch(text: string) { + return loadAppBootstrap({ kind: "patch", text, options: {} }); +} + +describe("explicit patch input validation", () => { + test.each(["not a patch\n", "error: upstream command failed\n", "diff --git\n"])( + "rejects nonempty input without any recognized files (%s)", + async (text) => { + await expect(loadTestPatch(text)).rejects.toThrow( + "No recognizable file changes in patch input", + ); + }, + ); + + test.each(["", " \n\t\r\n"])("preserves intentionally empty input (%s)", async (text) => { + expect((await loadTestPatch(text)).changeset.files).toHaveLength(0); + }); + + test.each([ + ["unified", textPatch], + ["traditional", "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-before\n+after\n"], + [ + "mail envelope", + "From 0123456789 Mon Sep 17 00:00:00 2001\nSubject: [PATCH] Update\n\n" + + textPatch + + "-- \n2.50.0\n", + ], + ["mode only", "diff --git a/a.txt b/a.txt\nold mode 100644\nnew mode 100755\n"], + [ + "rename only", + "diff --git a/a.txt b/b.txt\nsimilarity index 100%\nrename from a.txt\nrename to b.txt\n", + ], + [ + "empty added file", + "diff --git a/empty.txt b/empty.txt\nnew file mode 100644\nindex 0000000..e69de29\n", + ], + [ + "binary", + "diff --git a/a.bin b/a.bin\nindex 1234567..7654321 100644\nBinary files a/a.bin and b/a.bin differ\n", + ], + [ + "ANSI colored", + textPatch + .replace("-before", "\u001b[31m-before\u001b[0m") + .replace("+after", "\u001b[32m+after\u001b[0m"), + ], + ])("preserves supported %s patches", async (_name, text) => { + expect((await loadTestPatch(text)).changeset.files).toHaveLength(1); + }); +}); diff --git a/packages/hunk/src/core/changeset/loaders.test.ts b/packages/hunk/src/core/changeset/loaders.test.ts index 75dfb56e0..d00a534e8 100644 --- a/packages/hunk/src/core/changeset/loaders.test.ts +++ b/packages/hunk/src/core/changeset/loaders.test.ts @@ -1510,21 +1510,19 @@ describe("loadAppBootstrap", () => { }); }); - test("treats malformed inline patch text as an empty review instead of throwing", async () => { - const bootstrap = await loadAppBootstrap({ - kind: "patch", - text: [ - "\u001b]0;title\u0007not really a patch", - "--- separator only", - "@@ section heading", - "still plain text", - ].join("\n"), - options: { mode: "auto" }, - }); - - expect(bootstrap.changeset.files).toHaveLength(0); - expect(bootstrap.changeset.title).toContain("Patch review"); - expect(bootstrap.changeset.summary).toContain("not really a patch"); + test("rejects malformed inline patch text instead of treating it as an empty review", async () => { + await expect( + loadAppBootstrap({ + kind: "patch", + text: [ + "\u001b]0;title\u0007not really a patch", + "--- separator only", + "@@ section heading", + "still plain text", + ].join("\n"), + options: { mode: "auto" }, + }), + ).rejects.toThrow("No recognizable file changes in patch input"); }); test("loads colorized git patch files like the real pager stdin stream", async () => { diff --git a/packages/hunk/src/core/changeset/loaders.ts b/packages/hunk/src/core/changeset/loaders.ts index dec46dd5a..05a5a6dda 100644 --- a/packages/hunk/src/core/changeset/loaders.ts +++ b/packages/hunk/src/core/changeset/loaders.ts @@ -15,6 +15,7 @@ import { buildDiffFile, type BuildDiffFileOptions, type DiffFileSourceContext } import { createFileSourceFetcher, type FileSourceSpec } from "./fileSource"; import { changesetFromPatch } from "./fromPatch"; +import { HunkUserError } from "../run/errors"; import { DEFAULT_FILE_GAP, DEFAULT_HUNK_GAP } from "../run/reviewGap"; import { DEFAULT_SCROLL_OFF } from "../run/scrollOff"; import { DEFAULT_TAB_WIDTH } from "../run/tabWidth"; @@ -274,7 +275,22 @@ async function loadPatchChangeset( : await Bun.file(resolvePath(cwd, input.file)).text()); const label = input.file && input.file !== "-" ? input.file : "stdin patch"; - return changesetFromPatch(patchText, `Patch review: ${basename(label)}`, label, sidecar); + const changeset = changesetFromPatch( + patchText, + `Patch review: ${basename(label)}`, + label, + sidecar, + ); + + // Explicit patch input must not disguise an upstream error as a successful empty review. + // Empty input remains valid; binary and metadata-only patches have recognized files too. + if (patchText.trim() && changeset.files.length === 0) { + throw new HunkUserError(`No recognizable file changes in patch input (${label}).`, [ + "Provide a unified diff, or check the command producing the patch.", + ]); + } + + return changeset; } /** Resolve CLI input into the fully loaded app bootstrap state. */ diff --git a/packages/hunk/src/core/theme/detection.test.ts b/packages/hunk/src/core/theme/detection.test.ts index 7c8011ec8..da8430780 100644 --- a/packages/hunk/src/core/theme/detection.test.ts +++ b/packages/hunk/src/core/theme/detection.test.ts @@ -10,10 +10,19 @@ import { class FakeThemeInput extends EventEmitter { isRaw = false; + readableFlowing: boolean | null = null; + setRawMode(mode: boolean) { this.isRaw = mode; } - resume() {} + + resume() { + this.readableFlowing = true; + } + + pause() { + this.readableFlowing = false; + } } /** Unit coverage for the terminal background probe used by auto theme POC. */ @@ -80,6 +89,24 @@ describe("terminal theme detection", () => { expect(input.isRaw).toBe(false); }); + test.each([null, false, true])("restores input flow after a probe (%s)", async (flowing) => { + const input = new FakeThemeInput(); + input.readableFlowing = flowing; + const output = { write: () => queueMicrotask(() => input.emit("data", "\x1b[?1;2c")) }; + + await detectTerminalColors({ input, output, timeoutMs: 5_000 }); + expect(input.readableFlowing).toBe(flowing === true); + expect(input.listenerCount("data")).toBe(0); + }); + + test("pauses previously idle input when terminal probing times out", async () => { + const input = new FakeThemeInput(); + await detectTerminalColors({ input, output: { write: () => {} }, timeoutMs: 1 }); + expect(input.readableFlowing).toBe(false); + expect(input.isRaw).toBe(false); + expect(input.listenerCount("data")).toBe(0); + }); + test("returns null when the terminal answers no color queries", async () => { const input = new FakeThemeInput(); const output = { write: () => queueMicrotask(() => input.emit("data", "\x1b[?1;2c")) }; diff --git a/packages/hunk/src/core/theme/detection.ts b/packages/hunk/src/core/theme/detection.ts index aa1220c11..692c7c9df 100644 --- a/packages/hunk/src/core/theme/detection.ts +++ b/packages/hunk/src/core/theme/detection.ts @@ -19,6 +19,7 @@ interface ThemeProbeInput { pause?(): unknown; setRawMode?(mode: boolean): unknown; isRaw?: boolean; + readableFlowing?: boolean | null; } interface ThemeProbeOutput { @@ -156,6 +157,7 @@ export async function detectTerminalColors({ timeoutMs = 150, }: DetectTerminalThemeOptions): Promise { const wasRaw = input.isRaw; + const wasFlowing = input.readableFlowing === true; let settled = false; let buffer = ""; @@ -167,6 +169,8 @@ export async function detectTerminalColors({ settled = true; clearTimeout(timer); input.removeListener("data", onData); + // A startup failure may never hand input to a renderer; do not leave stdin keeping it alive. + if (!wasFlowing) input.pause?.(); if (wasRaw !== undefined) { input.setRawMode?.(wasRaw); } diff --git a/test/cli/patch-input.test.ts b/test/cli/patch-input.test.ts new file mode 100644 index 000000000..bf3eda24a --- /dev/null +++ b/test/cli/patch-input.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +const entrypoint = resolve("packages/hunk/src/main.tsx"); +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +/** Spawn the real CLI without a terminal so validation must precede renderer takeover. */ +function runTestPatch(args: readonly string[], input = "") { + return Bun.spawnSync([process.execPath, "run", entrypoint, "patch", ...args, "--no-extensions"], { + stdin: Buffer.from(input), + stdout: "pipe", + stderr: "pipe", + env: { ...process.env, HUNK_DISABLE_UPDATE_NOTICE: "1" }, + }); +} + +describe("patch input CLI contract", () => { + test.each([{ args: [] }, { args: ["-"] }])( + "rejects arbitrary nonempty stdin (%j)", + ({ args }) => { + const result = runTestPatch(args, "not a patch\n"); + expect(result.exitCode).toBe(1); + expect(Buffer.from(result.stderr).toString()).toContain( + "No recognizable file changes in patch input (stdin patch)", + ); + expect(Buffer.from(result.stderr).toString()).toContain( + "check the command producing the patch", + ); + expect(Buffer.from(result.stdout).toString()).not.toContain("\u001b[?1049h"); + }, + ); + + test("rejects arbitrary nonempty patch files", () => { + const dir = mkdtempSync(join(tmpdir(), "hunk-invalid-patch-test-")); + tempDirs.push(dir); + const file = join(dir, "invalid.patch"); + writeFileSync(file, "not a patch\n"); + const result = runTestPatch([file]); + expect(result.exitCode).toBe(1); + expect(Buffer.from(result.stderr).toString()).toContain( + "No recognizable file changes in patch input", + ); + expect(Buffer.from(result.stderr).toString()).toContain("invalid.patch"); + }); + + test("accepts empty stdin", () => { + const result = runTestPatch([], ""); + expect(result.exitCode).toBe(0); + expect(Buffer.from(result.stderr).toString()).toBe(""); + }); + + test("accepts a mail-enveloped textual patch", () => { + const result = runTestPatch( + [], + "From 0123456789 Mon Sep 17 00:00:00 2001\nSubject: [PATCH] Update\n\ndiff --git a/a.txt b/a.txt\n--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-before\n+after\n", + ); + expect(result.exitCode).toBe(0); + expect(Buffer.from(result.stderr).toString()).toBe(""); + expect(Buffer.from(result.stdout).toString()).toContain("after"); + }); +}); diff --git a/test/pty/invalid-patch-integration.test.ts b/test/pty/invalid-patch-integration.test.ts new file mode 100644 index 000000000..4010b681b --- /dev/null +++ b/test/pty/invalid-patch-integration.test.ts @@ -0,0 +1,34 @@ +import { afterEach, expect, setDefaultTimeout, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { createPtyHarness } from "./harness"; + +const harness = createPtyHarness(); + +setDefaultTimeout(20_000); + +afterEach(() => { + harness.cleanup(); +}); + +test("invalid patch files report failure and exit even after probing terminal colors", async () => { + const fixture = harness.createSidebarJumpRepoFixture(); + const patch = join(fixture.dir, "invalid.patch"); + writeFileSync(patch, "not a patch\n"); + const command = harness.buildHunkCommand(["patch", patch, "--no-extensions"]); + const session = await harness.launchShellCommand({ + command: `${command}; result=$?; printf '\\nPATCH_EXIT:%s\\n' "$result"`, + cwd: fixture.dir, + cols: 120, + rows: 24, + }); + + try { + const frame = await session.waitForText(/PATCH_EXIT:1/, { timeout: 5_000 }); + expect(frame).toContain("No recognizable file changes in patch input"); + expect(frame).not.toContain("No files match"); + expect(session.getRawOutput()).not.toContain("\u001b[?1049h"); + } finally { + session.close(); + } +}); From 61fc029a8c6ff1837be4974cb3e49b506a70be09 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 13 Oct 2026 16:00:00 -0400 Subject: [PATCH 2/3] test(cli): bound patch-input exit regression checks --- test/cli/patch-input.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/cli/patch-input.test.ts b/test/cli/patch-input.test.ts index bf3eda24a..bd0f56ebf 100644 --- a/test/cli/patch-input.test.ts +++ b/test/cli/patch-input.test.ts @@ -16,6 +16,9 @@ function runTestPatch(args: readonly string[], input = "") { stdin: Buffer.from(input), stdout: "pipe", stderr: "pipe", + // Bound exit regressions at the process level; a synchronous spawn blocks test timers. + timeout: 10_000, + killSignal: "SIGKILL", env: { ...process.env, HUNK_DISABLE_UPDATE_NOTICE: "1" }, }); } From 87f1b9dc4691f02a1260ef5e74ab7011faca19d6 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 13 Oct 2026 16:05:37 -0400 Subject: [PATCH 3/3] test(ci): handle prerelease labels and slower Git fixtures --- .changeset/beta-ci-smoke-coverage.md | 2 ++ packages/hunk-git/src/status.test.ts | 5 ++++- website/tests/changelog-smoke.spec.ts | 10 +++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 .changeset/beta-ci-smoke-coverage.md diff --git a/.changeset/beta-ci-smoke-coverage.md b/.changeset/beta-ci-smoke-coverage.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/beta-ci-smoke-coverage.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/hunk-git/src/status.test.ts b/packages/hunk-git/src/status.test.ts index 86e190bc7..acdbe0055 100644 --- a/packages/hunk-git/src/status.test.ts +++ b/packages/hunk-git/src/status.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { mkdtempSync, mkdirSync, @@ -13,6 +13,9 @@ import { resolveCanonicalPath } from "@hunk/vcs/path"; import { join } from "node:path"; import { createGitStatusCapability, parseGitStatus, parseGitStatusWorktrees } from "./status"; +// Real Git fixture setup and multi-step status reads can exceed five seconds on Windows runners. +setDefaultTimeout(30_000); + const dirs: string[] = []; /** Create a portable Git fixture with local identity and a deterministic branch. */ function createTestRepo(commit = true) { diff --git a/website/tests/changelog-smoke.spec.ts b/website/tests/changelog-smoke.spec.ts index a1c7d14bf..25d31af8a 100644 --- a/website/tests/changelog-smoke.spec.ts +++ b/website/tests/changelog-smoke.spec.ts @@ -1,4 +1,12 @@ import { expect, test } from "@playwright/test"; +import { readFileSync } from "node:fs"; + +// Release channels change independently of this test; verify the browser preserves published labels. +const changelogIndex = readFileSync( + new URL("../src/content/docs/changelog/index.md", import.meta.url), + "utf8", +); +const prereleaseSeriesCount = changelogIndex.match(/^Prerelease ·/gm)?.length ?? 0; test("the changelog is reachable from the main navigation", async ({ page }, testInfo) => { await page.goto("/"); @@ -110,7 +118,7 @@ test("the index lists every series newest first and links each one", async ({ pa expect(major > nextMajor || (major === nextMajor && minor > nextMinor)).toBe(true); } - await expect(page.getByText(/^Prerelease ·/)).toHaveCount(0); + await expect(page.getByText(/^Prerelease ·/)).toHaveCount(prereleaseSeriesCount); await expect(page.getByText(/^Latest ·/)).toHaveCount(1); });