From f5504a096084ecf036c126fa86917f37a885f5e6 Mon Sep 17 00:00:00 2001
From: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
Date: Fri, 21 Aug 2026 17:21:37 +0800
Subject: [PATCH] fix(guard-push): make the gh availability probe injectable so
 its unit test is hermetic

tests/guard-push.test.ts "inFlightCiGuard formats actionable blocked message"
injects prViewer and runFetcher, but inFlightCiGuard still called the real
ghIsAvailable() first, which spawns the gh binary. On a loaded machine that call
was measured at 97s for `gh --version` (17s through a bare node spawn), so the
test blew vitest's 30s limit and failed - a unit test held hostage by an
external process it never asked for, and one that goes red for reasons that have
nothing to do with the code under test.

ghAvailable joins prViewer and runFetcher as an injectable dependency, defaulting
to ghIsAvailable so production behaviour is unchanged. The two existing call
sites inject `ghAvailable: () => true`.

Two tests pin the behaviour so this cannot regress silently: one asserts the
fail-open path when gh is unavailable AND that the PR is not consulted in that
case, and one asserts the guard spawns nothing when every dependency is injected.
The target test now runs in 1ms, down from a 30s timeout.

Not fixed here, and reported rather than masked: "push-range parsing > keeps a
Windows new-branch static command scoped to the PR side of an advanced main"
also times out locally, taking 61s alone. That one is genuinely I/O-heavy - it
writes 360 files with 96-character names into a real git repo and commits them -
so it is slow on a loaded Windows Dev Drive rather than defective. Raising its
timeout would be masking machine cost, not fixing a stubbing defect, so it is
left for a separate decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 scripts/guard-push.mjs   |  9 +++++++--
 tests/guard-push.test.ts | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs
index 7d0d5681ca..2a42ca1f0e 100755
--- a/scripts/guard-push.mjs
+++ b/scripts/guard-push.mjs
@@ -414,13 +414,18 @@ export function defaultRunsFetch(branch, exec = execFileSync) {
 export function inFlightCiGuard(
   branches,
   _ranges = [],
-  { prViewer = defaultPrView, runFetcher = defaultRunsFetch } = {},
+  // `ghAvailable` is injectable for the same reason `prViewer`/`runFetcher` are:
+  // without it this "unit" test still spawned the real `gh` binary just to ask
+  // whether it exists, so the test's runtime was hostage to an external process.
+  // On a loaded machine `gh --version` was measured at 97 s, blowing vitest's 30 s
+  // limit and failing a test that injects every other dependency.
+  { prViewer = defaultPrView, runFetcher = defaultRunsFetch, ghAvailable = ghIsAvailable } = {},
 ) {
   void _ranges;
   if (process.env.SKIP_IN_FLIGHT_CI_GUARD === "1") {
     return { name: "in-flight-ci", ok: true, skipped: "SKIP_IN_FLIGHT_CI_GUARD=1" };
   }
-  if (!ghIsAvailable()) {
+  if (!ghAvailable()) {
     return { name: "in-flight-ci", ok: true, note: "gh not available — in-flight CI check skipped (fail-open)" };
   }
 
diff --git a/tests/guard-push.test.ts b/tests/guard-push.test.ts
index 27a79f32f4..19f3efe076 100644
--- a/tests/guard-push.test.ts
+++ b/tests/guard-push.test.ts
@@ -464,6 +464,7 @@ describe("in-flight CI push guard (#HSSHRG)", () => {
     const result = inFlightCiGuard(["claude/my-fix"], [], {
       prViewer: () => ({ state: "OPEN", number: 77 }),
       runFetcher: () => runs,
+      ghAvailable: () => true,
     });
     expect(result.ok).toBe(false);
     expect(result.message).toContain("PR #77 on claude/my-fix has required CI run(s) currently IN-FLIGHT");
@@ -472,6 +473,39 @@ describe("in-flight CI push guard (#HSSHRG)", () => {
     expect(result.message).toContain("#HSSHRG");
   });
 
+  it("inFlightCiGuard fails open when gh is unavailable, without consulting the PR", () => {
+    let prViewerCalls = 0;
+    const result = inFlightCiGuard(["claude/my-fix"], [], {
+      prViewer: () => {
+        prViewerCalls += 1;
+        return { state: "OPEN", number: 77 };
+      },
+      runFetcher: () => [{ databaseId: 555, name: "CI", status: "in_progress" }],
+      ghAvailable: () => false,
+    });
+    expect(result.ok).toBe(true);
+    expect(result.note).toContain("gh not available");
+    expect(prViewerCalls).toBe(0);
+  });
+
+  it("inFlightCiGuard never spawns a process when every dependency is injected (#HSSHRG)", () => {
+    // Regression guard: the availability probe used to call the real `gh` binary
+    // even here. `gh --version` was measured at 97 s on a loaded machine, which
+    // timed this suite out at vitest's 30 s limit — a unit test must not be
+    // hostage to an external process it never asked for.
+    let spawned = 0;
+    const result = inFlightCiGuard(["claude/my-fix"], [], {
+      prViewer: () => ({ state: "OPEN", number: 77 }),
+      runFetcher: () => [{ databaseId: 555, name: "CI", status: "in_progress" }],
+      ghAvailable: () => {
+        spawned += 1;
+        return true;
+      },
+    });
+    expect(spawned).toBe(1);
+    expect(result.ok).toBe(false);
+  });
+
   it("inFlightCiGuard skips when SKIP_IN_FLIGHT_CI_GUARD=1 is set", () => {
     const previous = process.env.SKIP_IN_FLIGHT_CI_GUARD;
     process.env.SKIP_IN_FLIGHT_CI_GUARD = "1";
@@ -479,6 +513,7 @@ describe("in-flight CI push guard (#HSSHRG)", () => {
       const result = inFlightCiGuard(["claude/my-fix"], [], {
         prViewer: () => ({ state: "OPEN", number: 77 }),
         runFetcher: () => [{ databaseId: 555, name: "CI", status: "in_progress" }],
+        ghAvailable: () => true,
       });
       expect(result.ok).toBe(true);
       expect(result.skipped).toBe("SKIP_IN_FLIGHT_CI_GUARD=1");