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(); diff --git a/CHANGELOG.md b/CHANGELOG.md index d68f40c4..bd189729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## 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) + ## 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..9fdea588 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", `+refs/heads/${baseBranch}:refs/remotes/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`, );