From 69fec1146df28b458ae9d7ad15212a614d0dfd65 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 20 Apr 2026 18:33:04 +0000 Subject: [PATCH 1/3] fix: fetch stale origin ref before denying on merged PR in require-pr-before-stop When a PR is merged but origin/main hasn't been fetched, the policy incorrectly denies and asks Claude to create a new PR. Now fetches the base branch ref and re-verifies before denying on MERGED state. Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 3 + __tests__/hooks/builtin-policies.test.ts | 80 +++++++++++++++++++++++- src/hooks/builtin-policies.ts | 30 +++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d68f40c4..69438fb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Fixes +- Fix `require-pr-before-stop` falsely denying when PR is already merged and `origin/main` is stale (#112) + ## 0.0.5 — 2026-04-17 ### Fixes diff --git a/__tests__/hooks/builtin-policies.test.ts b/__tests__/hooks/builtin-policies.test.ts index d3fa80e3..bc8b1ef0 100644 --- a/__tests__/hooks/builtin-policies.test.ts +++ b/__tests__/hooks/builtin-policies.test.ts @@ -2169,7 +2169,7 @@ describe("hooks/builtin-policies", () => { expect(result.reason).toContain("gh pr create"); }); - it("denies when PR is merged and file changes exist", async () => { + it("denies when PR is merged and file changes exist after fetch", async () => { mockPrScenario({ prResult: { number: 42, url: "https://github.com/org/repo/pull/42", state: "MERGED" } }); const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } }); const result = await policy.fn(ctx); @@ -2177,6 +2177,84 @@ describe("hooks/builtin-policies", () => { expect(result.reason).toContain("merged"); }); + it("allows when PR is merged and branch is up to date after fetch (regular merge)", async () => { + let fetched = false; + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (typeof cmd === "string" && cmd.includes("gh --version")) return "/usr/bin/gh\n"; + if (typeof cmd === "string" && cmd.includes("rev-parse --abbrev-ref")) return "feat/branch\n"; + if (typeof cmd === "string" && cmd.includes("gh pr view")) { + return JSON.stringify({ number: 42, url: "https://github.com/org/repo/pull/42", state: "MERGED" }); + } + return ""; + }); + vi.mocked(execFileSync).mockImplementation((_cmd: string, args?: readonly string[]) => { + const joined = args?.join(" ") ?? ""; + if (joined.includes("fetch")) { fetched = true; return ""; } + if (joined.includes("log") && joined.includes("..HEAD")) { + return fetched ? "" : "abc123 some commit\n"; + } + if (joined.includes("diff") && joined.includes("--stat")) { + return fetched ? "" : " src/index.ts | 2 +-\n 1 file changed\n"; + } + return ""; + }); + const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } }); + const result = await policy.fn(ctx); + expect(result.decision).toBe("allow"); + expect(result.reason).toContain("was merged"); + expect(result.reason).toContain("up to date"); + }); + + it("allows when PR is merged and no file diff after fetch (squash merge)", async () => { + let fetched = false; + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (typeof cmd === "string" && cmd.includes("gh --version")) return "/usr/bin/gh\n"; + if (typeof cmd === "string" && cmd.includes("rev-parse --abbrev-ref")) return "feat/branch\n"; + if (typeof cmd === "string" && cmd.includes("gh pr view")) { + return JSON.stringify({ number: 42, url: "https://github.com/org/repo/pull/42", state: "MERGED" }); + } + return ""; + }); + vi.mocked(execFileSync).mockImplementation((_cmd: string, args?: readonly string[]) => { + const joined = args?.join(" ") ?? ""; + if (joined.includes("fetch")) { fetched = true; return ""; } + if (joined.includes("log") && joined.includes("..HEAD")) { + return "abc123 old squash commit\n"; + } + if (joined.includes("diff") && joined.includes("--stat")) { + return fetched ? "" : " src/index.ts | 2 +-\n 1 file changed\n"; + } + return ""; + }); + const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } }); + const result = await policy.fn(ctx); + expect(result.decision).toBe("allow"); + expect(result.reason).toContain("was merged"); + expect(result.reason).toContain("no file changes"); + }); + + it("falls through to deny when fetch fails on merged PR", async () => { + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (typeof cmd === "string" && cmd.includes("gh --version")) return "/usr/bin/gh\n"; + if (typeof cmd === "string" && cmd.includes("rev-parse --abbrev-ref")) return "feat/branch\n"; + if (typeof cmd === "string" && cmd.includes("gh pr view")) { + return JSON.stringify({ number: 42, url: "https://github.com/org/repo/pull/42", state: "MERGED" }); + } + return ""; + }); + vi.mocked(execFileSync).mockImplementation((_cmd: string, args?: readonly string[]) => { + const joined = args?.join(" ") ?? ""; + if (joined.includes("fetch")) throw new Error("network error"); + if (joined.includes("log") && joined.includes("..HEAD")) return "abc123 commit\n"; + if (joined.includes("diff") && joined.includes("--stat")) return " src/index.ts | 2 +-\n"; + return ""; + }); + const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } }); + const result = await policy.fn(ctx); + expect(result.decision).toBe("deny"); + expect(result.reason).toContain("merged"); + }); + it("allows with reason when gh is not installed", async () => { mockPrScenario({ ghInstalled: false }); const ctx = makeCtx({ eventType: "Stop", session: { cwd: "/repo" } }); diff --git a/src/hooks/builtin-policies.ts b/src/hooks/builtin-policies.ts index 13f47382..08d7b97c 100644 --- a/src/hooks/builtin-policies.ts +++ b/src/hooks/builtin-policies.ts @@ -1064,6 +1064,36 @@ function requirePrBeforeStop(ctx: PolicyContext): PolicyResult { return allow(`PR #${pr.number} exists: ${pr.url}`); } + // PR is merged/closed. The earlier origin/{baseBranch} checks may have + // used a stale ref. Fetch and re-verify before denying. + if (pr.state === "MERGED") { + try { + execFileSync("git", ["fetch", "origin", baseBranch], { + cwd, + encoding: "utf8", + timeout: 10000, + }); + const freshAhead = execFileSync( + "git", + ["log", `origin/${baseBranch}..HEAD`, "--oneline"], + { cwd, encoding: "utf8", timeout: 5000 }, + ).trim(); + if (!freshAhead) { + return allow(`PR #${pr.number} was merged; branch is up to date with ${baseBranch}.`); + } + const freshDiff = execFileSync( + "git", + ["diff", "--stat", `origin/${baseBranch}`, "HEAD"], + { cwd, encoding: "utf8", timeout: 5000 }, + ).trim(); + if (!freshDiff) { + return allow(`PR #${pr.number} was merged; no file changes vs ${baseBranch}.`); + } + } catch { + // Fetch or git command failed — fall through to deny + } + } + return deny( `Pull request for branch "${branch}" is ${pr.state.toLowerCase()}. Run now: gh pr create`, ); From c02dcae36fe7955a7725874377b2c6fc93ff46dd Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 20 Apr 2026 18:39:10 +0000 Subject: [PATCH 2/3] chore: remind changelog policy to check version from package.json Co-Authored-By: Claude Opus 4.6 --- .failproofai/policies/workflow-policies.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.failproofai/policies/workflow-policies.mjs b/.failproofai/policies/workflow-policies.mjs index 85d309b7..80e874dd 100644 --- a/.failproofai/policies/workflow-policies.mjs +++ b/.failproofai/policies/workflow-policies.mjs @@ -17,7 +17,8 @@ customPolicies.add({ return instruct( "Check whether CHANGELOG.md needs an update for this commit. " + "Every PR must include an entry under the `## Unreleased` section. " + - "Use the appropriate subsection: Features, Fixes, Docs, or Dependencies." + "Use the appropriate subsection: Features, Fixes, Docs, or Dependencies.\n" + + "Check the version in package.json and ensure the changelog entry matches the current version." ); } return allow(); From a818de362b1fb4f47f7e7a152fcf41ac06acfafd Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 20 Apr 2026 18:43:02 +0000 Subject: [PATCH 3/3] fix: use explicit refspec in git fetch to update remote-tracking ref Bare `git fetch origin ` only writes to FETCH_HEAD, not refs/remotes/origin/. Use explicit refspec to guarantee the remote-tracking ref is updated before re-checking divergence. Also moves changelog entry under 0.0.6-beta.0 for release. Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 2 ++ src/hooks/builtin-policies.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69438fb8..bd189729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +## 0.0.6-beta.0 — 2026-04-20 + ### Fixes - Fix `require-pr-before-stop` falsely denying when PR is already merged and `origin/main` is stale (#112) diff --git a/src/hooks/builtin-policies.ts b/src/hooks/builtin-policies.ts index 08d7b97c..9fdea588 100644 --- a/src/hooks/builtin-policies.ts +++ b/src/hooks/builtin-policies.ts @@ -1068,7 +1068,7 @@ function requirePrBeforeStop(ctx: PolicyContext): PolicyResult { // used a stale ref. Fetch and re-verify before denying. if (pr.state === "MERGED") { try { - execFileSync("git", ["fetch", "origin", baseBranch], { + execFileSync("git", ["fetch", "origin", `+refs/heads/${baseBranch}:refs/remotes/origin/${baseBranch}`], { cwd, encoding: "utf8", timeout: 10000,