perf(watch): stop watch-mode signature checks from stalling the review UI

.changeset/async-watch-signatures.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": minor
+---
+
+Watch mode no longer freezes the review UI while it checks for changes, and VCS extensions can now return a promise from `watchSignature`.

docs/extensions.md

@@ -324,7 +324,10 @@ checkout some other adapter finds.
 `--watch` works through extension adapters. Each operation may add:
 
 - `watchSignature(input, ctx)` — a cheap fingerprint of the reviewed state.
-  Hunk polls it and reloads when it changes.
+  Hunk polls it and reloads when it changes. It may return a promise, and
+  should when it shells out: this runs on every debounced file event and every
+  safety poll, so a blocking implementation stalls the review UI each time.
+  `ctx.signal` aborts when the watcher closes.
 - `watchPlan(input, ctx)` — the filesystem targets that cover that state, so
   Hunk reacts to events instead of polling on a timer.
 

src/app/sessionBootstrap.test.ts

@@ -20,6 +20,15 @@ function createTestConfig(input: CliInput): HunkConfigResolution {
   };
 }
 
+/** Create an externally resolved promise for reload-lifetime tests. */
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  const promise = new Promise<T>((nextResolve) => {
+    resolve = nextResolve;
+  });
+  return { promise, resolve };
+}
+
 /** Build a loader result with a stable changeset for transform assertions. */
 function createTestBootstrap(input: CliInput): AppBootstrap {
   return {
@@ -54,4 +63,27 @@ describe("loadConfiguredSessionBootstrap", () => {
     expect(result.bootstrap.keybindings).toEqual({ "hunk.review.nextHunk": "]" });
     expect(result.bootstrap.viewPreferencesConfigPath).toBe("/tmp/hunk-config.toml");
   });
+
+  test("rejects a bootstrap whose reload signal was aborted while loading", async () => {
+    const input = createTestInput();
+    const load = deferred<AppBootstrap>();
+    const controller = new AbortController();
+    let loaderSignal: AbortSignal | undefined;
+    const pending = loadConfiguredSessionBootstrap({
+      configured: createTestConfig(input),
+      cwd: process.cwd(),
+      signal: controller.signal,
+      loadAppBootstrapImpl: async (_resolvedInput, options) => {
+        loaderSignal = options?.signal;
+        return await load.promise;
+      },
+    });
+
+    await Promise.resolve();
+    expect(loaderSignal).toBe(controller.signal);
+    controller.abort();
+    load.resolve(createTestBootstrap(input));
+
+    await expect(pending).rejects.toMatchObject({ name: "AbortError" });
+  });
 });

src/app/sessionBootstrap.ts

@@ -18,6 +18,8 @@ export interface SessionBootstrapOptions {
   initialThemeMode?: AppBootstrap["initialThemeMode"];
   /** Reloads can reopen another directory; initial launch relies on the loader's default cwd. */
   loadAtCwd?: boolean;
+  /** Abort a retired reload before its bootstrap is returned to the host. */
+  signal?: AbortSignal;
   loadAppBootstrapImpl?: typeof loadAppBootstrap;
 }
 
@@ -42,8 +44,10 @@ export async function loadConfiguredSessionBootstrap({
   extensions,
   initialThemeMode,
   loadAtCwd = false,
+  signal,
   loadAppBootstrapImpl = loadAppBootstrap,
 }: SessionBootstrapOptions): Promise<SessionBootstrapResult> {
+  signal?.throwIfAborted();
   const sessionThemes = collectSessionCustomThemes(
     configured.customThemes,
     extensions?.registry.themes,
@@ -69,8 +73,11 @@ export async function loadConfiguredSessionBootstrap({
     ...(loadAtCwd ? { cwd } : {}),
     customThemes: sessionThemes.themes,
     vcsAdapters: applied.vcsAdapters,
+    signal,
   });
+  signal?.throwIfAborted();
   bootstrap.changeset = await applyExtensionChangesetTransforms(extensions, bootstrap.changeset);
+  signal?.throwIfAborted();
   bootstrap.initialThemeMode = initialThemeMode ?? bootstrap.initialThemeMode;
   bootstrap.extensions = extensions;
   bootstrap.viewPreferencesConfigPath = configured.viewPreferencesConfigPath;

src/core/loaders.test.ts

@@ -239,7 +239,7 @@ describe("loadAppBootstrap", () => {
 
       expect(bootstrap.reloadContext.cwd).toBe(dir);
       expect(bootstrap.reloadContext.initialWatchSignature).toBeDefined();
-      expect(computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).not.toBe(
+      expect(await computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).not.toBe(
         bootstrap.reloadContext.initialWatchSignature,
       );
     } finally {
@@ -346,7 +346,7 @@ describe("loadAppBootstrap", () => {
     );
     expect(bootstrap.changeset.files[0]?.path).toBe("example.ts");
     expect(bootstrap.changeset.files[0]?.agent?.annotations).toHaveLength(1);
-    expect(computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).toBe(
+    expect(await computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).toBe(
       bootstrap.reloadContext.initialWatchSignature!,
     );
   });

src/core/loaders.ts

@@ -41,6 +41,8 @@ interface LoadAppBootstrapOptions {
   /** Extension-contributed VCS backends this session may load reviews through. */
   vcsAdapters?: readonly VcsAdapter[];
   gitExecutable?: string;
+  /** Abort a retired reload before it can publish a stale bootstrap. */
+  signal?: AbortSignal;
 }
 
 /** Return the final path segment for display-oriented labels. */
@@ -381,10 +383,12 @@ async function loadVcsChangeset(
   cwd = process.cwd(),
   gitExecutable = "git",
   extensionVcsAdapters: readonly VcsAdapter[] = [],
+  signal?: AbortSignal,
 ) {
   const adapter = getConfiguredVcsAdapter(input.options.vcs, extensionVcsAdapters);
   const operation = operationFromInput(input);
-  const result = await loadVcsReview(adapter, operation, { cwd, gitExecutable });
+  const result = await loadVcsReview(adapter, operation, { cwd, gitExecutable, signal });
+  signal?.throwIfAborted();
   const parsedChangeset = normalizePatchChangeset(
     result.patchText,
     result.title,
@@ -449,19 +453,28 @@ export async function loadAppBootstrap(
     customThemes,
     vcsAdapters,
     gitExecutable = "git",
+    signal,
   }: LoadAppBootstrapOptions = {},
 ): Promise<AppBootstrap> {
+  signal?.throwIfAborted();
   // Capture before loading content so watch mode can detect mutations that race initial loading.
   let initialWatchSignature: string | undefined;
   if (input.options.watch) {
     try {
-      initialWatchSignature = computeWatchSignature(input, { cwd, gitExecutable, vcsAdapters });
+      initialWatchSignature = await computeWatchSignature(input, {
+        cwd,
+        gitExecutable,
+        vcsAdapters,
+        signal,
+      });
     } catch {
+      signal?.throwIfAborted();
       // A transient signature failure must not prevent an otherwise valid initial review.
     }
   }
 
   const agentContext = await loadAgentContext(input.options.agentContext, { cwd });
+  signal?.throwIfAborted();
 
   let changeset: Changeset;
   let repoRoot: string | undefined;
@@ -471,7 +484,14 @@ export async function loadAppBootstrap(
     case "show":
     case "stash-show":
       {
-        const result = await loadVcsChangeset(input, agentContext, cwd, gitExecutable, vcsAdapters);
+        const result = await loadVcsChangeset(
+          input,
+          agentContext,
+          cwd,
+          gitExecutable,
+          vcsAdapters,
+          signal,
+        );
         changeset = result.changeset;
         repoRoot = result.repoRoot;
       }
@@ -487,6 +507,7 @@ export async function loadAppBootstrap(
       break;
   }
 
+  signal?.throwIfAborted();
   changeset = {
     ...changeset,
     files: orderDiffFiles(changeset.files, agentContext),

src/core/vcs/git.test.ts

@@ -1,5 +1,14 @@
 import { afterEach, describe, expect, test } from "bun:test";
-import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs";
+import {
+  chmodSync,
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  realpathSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
 import { tmpdir } from "node:os";
 import { join, resolve } from "node:path";
 import {
@@ -13,6 +22,7 @@ import {
   parseGitNumstat,
   resolveGitMetadata,
   runGitText,
+  runGitTextAsync,
   shouldSkipLargeTrackedDiff,
 } from "./git";
 import type { VcsDiffCommandInput } from "../types";
@@ -56,6 +66,21 @@ function normalizeComparablePath(path: string) {
   return realpathSync.native(path).replace(/\\/g, "/");
 }
 
+/** Quote one path for the POSIX helper scripts used by Unix-only process tests. */
+function shellQuote(path: string) {
+  return `'${path.replaceAll("'", `'\\''`)}'`;
+}
+
+/** Report whether a Unix process still exists without sending it a signal. */
+function processExists(pid: number) {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 function makeGitInput(overrides: Partial<VcsDiffCommandInput> = {}): VcsDiffCommandInput {
   return {
     kind: "vcs",
@@ -150,6 +175,144 @@ describe("git command helpers", () => {
       "Git is required for `hunk diff`, but `definitely-not-a-real-git-binary` was not found in PATH.",
     );
   });
+
+  test.skipIf(process.platform === "win32")(
+    "aborting async Git kills descendants after the root process exits",
+    async () => {
+      const repoRoot = createTempDir("hunk-git-exited-root-abort-");
+      const executablePath = join(repoRoot, "fake-git.sh");
+      const pidPath = join(repoRoot, "child-pid");
+      writeFileSync(
+        executablePath,
+        [
+          "#!/bin/sh",
+          "trap '' HUP TERM",
+          "sleep 30 &",
+          "child=$!",
+          `printf '%s\\n' "$child" > ${shellQuote(pidPath)}`,
+          "exit 0",
+          "",
+        ].join("\n"),
+      );
+      chmodSync(executablePath, 0o755);
+
+      const controller = new AbortController();
+      const pending = runGitTextAsync({
+        input: makeGitInput(),
+        args: ["status"],
+        cwd: repoRoot,
+        gitExecutable: executablePath,
+        signal: controller.signal,
+      });
+      const outcome = pending.then(
+        () => "resolved",
+        (error: Error) => error.name,
+      );
+      let childPid: number | undefined;
+
+      try {
+        for (let attempt = 0; attempt < 100 && !existsSync(pidPath); attempt++) {
+          await Bun.sleep(10);
+        }
+        expect(existsSync(pidPath)).toBe(true);
+        childPid = Number(readFileSync(pidPath, "utf8").trim());
+        expect(processExists(childPid)).toBe(true);
+        // The wrapper has exited, leaving only its child holding stdout open.
+        await Bun.sleep(50);
+
+        controller.abort();
+        expect(await Promise.race([outcome, Bun.sleep(1_000).then(() => "timeout")])).toBe(
+          "AbortError",
+        );
+        for (let attempt = 0; attempt < 50 && processExists(childPid); attempt++) {
+          await Bun.sleep(10);
+        }
+        expect(processExists(childPid)).toBe(false);
+      } finally {
+        controller.abort();
+        if (childPid !== undefined) {
+          try {
+            process.kill(childPid, "SIGKILL");
+          } catch {
+            // The expected path already terminated the descendant group.
+          }
+        }
+      }
+    },
+  );
+
+  test.skipIf(process.platform === "win32")(
+    "aborting an async diff terminates textconv helpers and settles promptly",
+    async () => {
+      const repoRoot = createTempRepo("hunk-git-async-abort-");
+      const helperPath = join(repoRoot, "slow-textconv.sh");
+      const pidPath = join(repoRoot, "textconv-pids");
+      writeFileSync(
+        helperPath,
+        [
+          "#!/bin/sh",
+          // Reproduces helpers that ignore graceful termination; cancellation
+          // must still settle and reap the whole process group.
+          "trap '' TERM",
+          "sleep 30 &",
+          "child=$!",
+          `printf '%s\\n%s\\n' "$$" "$child" > ${shellQuote(pidPath)}`,
+          'wait "$child"',
+          'cat "$1"',
+          "",
+        ].join("\n"),
+      );
+      chmodSync(helperPath, 0o755);
+      git(repoRoot, "config", "diff.slow.textconv", helperPath);
+      writeFileSync(join(repoRoot, ".gitattributes"), "*.slow diff=slow\n");
+      writeFileSync(join(repoRoot, "example.slow"), "before\n");
+      git(repoRoot, "add", ".gitattributes", "example.slow");
+      git(repoRoot, "commit", "-m", "initial");
+      writeFileSync(join(repoRoot, "example.slow"), "after\n");
+
+      const input = makeGitInput();
+      const controller = new AbortController();
+      const pending = runGitTextAsync({
+        input,
+        args: buildGitDiffArgs(input),
+        cwd: repoRoot,
+        signal: controller.signal,
+      });
+      const outcome = pending.then(
+        () => "resolved",
+        (error: Error) => error.name,
+      );
+      let helperPids: number[] = [];
+
+      try {
+        for (let attempt = 0; attempt < 100 && !existsSync(pidPath); attempt++) {
+          await Bun.sleep(10);
+        }
+        expect(existsSync(pidPath)).toBe(true);
+        helperPids = readFileSync(pidPath, "utf8").trim().split("\n").map(Number);
+        expect(helperPids).toHaveLength(2);
+
+        controller.abort();
+        expect(await Promise.race([outcome, Bun.sleep(1_000).then(() => "timeout")])).toBe(
+          "AbortError",
+        );
+
+        for (let attempt = 0; attempt < 50 && helperPids.some(processExists); attempt++) {
+          await Bun.sleep(10);
+        }
+        expect(helperPids.filter(processExists)).toEqual([]);
+      } finally {
+        controller.abort();
+        for (const pid of helperPids) {
+          try {
+            process.kill(pid, "SIGKILL");
+          } catch {
+            // The expected path already terminated the helper process group.
+          }
+        }
+      }
+    },
+  );
 });
 
 describe("listGitIgnoredDirectoryRoots", () => {