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");