feat(pull-requests): say why a merge was refused, and let an administrator merge anyway

apps/server/src/pullRequest/GitHubPullRequestCli.test.ts

@@ -1089,6 +1089,43 @@ layer("GitHubPullRequestCli.layer", (it) => {
     }),
   );
 
+  it.effect("stands the branch's rules down only when asked to", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValue(Effect.succeed(output("")));
+      const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli;
+
+      yield* cli.runPullRequestAction({
+        cwd: "/w",
+        repository: "acme/web",
+        host: "github.com",
+        number: 7,
+        action: "merge",
+        mergeMethod: "squash",
+        bypassRules: true,
+      });
+      expect(callAt(0).args).toEqual([
+        "pr",
+        "merge",
+        "7",
+        "--repo",
+        "github.com/acme/web",
+        "--squash",
+        "--admin",
+      ]);
+
+      yield* cli.runPullRequestAction({
+        cwd: "/w",
+        repository: "acme/web",
+        host: "github.com",
+        number: 7,
+        action: "merge",
+        mergeMethod: "squash",
+        bypassRules: false,
+      });
+      expect(callAt(1).args).not.toContain("--admin");
+    }),
+  );
+
   it.effect("arms auto-merge with the same strategy a merge would have used", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValue(Effect.succeed(output("")));
@@ -2300,7 +2337,12 @@ layer("GitHubPullRequestCli.layer", (it) => {
         // One request, because both answers hang off the same repository object.
         assert.strictEqual(mockedExecute.mock.calls.length, 1);
         expect(callAt(0).args).toContain("number=7");
-        expect(access).toEqual({ canWrite: false, canUpdate: true, didAuthor: true });
+        expect(access).toEqual({
+          canWrite: false,
+          canAdminister: false,
+          canUpdate: true,
+          didAuthor: true,
+        });
       }),
   );
 
@@ -2463,7 +2505,12 @@ layer("GitHubPullRequestCli.layer", (it) => {
       });
 
       assert.strictEqual(mockedExecute.mock.calls.length, 2);
-      expect(access).toEqual({ canWrite: false, canUpdate: true, didAuthor: true });
+      expect(access).toEqual({
+        canWrite: false,
+        canAdminister: false,
+        canUpdate: true,
+        didAuthor: true,
+      });
       yield* TestClock.setTime(Date.parse("2100-01-01T00:00:00Z"));
     }),
   );

apps/server/src/pullRequest/GitHubPullRequestCli.ts

@@ -525,6 +525,8 @@ export class GitHubPullRequestCli extends Context.Service<
       readonly action: PullRequestAction;
       readonly mergeMethod?: PullRequestMergeMethod;
       readonly updateMethod?: PullRequestUpdateMethod;
+      /** Only read for `merge`: gh has no bypass for anything else, and refuses it with `--auto`. */
+      readonly bypassRules?: boolean;
     }) => Effect.Effect<void, GitHubPullRequestCliError>;
 
     readonly commentOnPullRequest: (input: {
@@ -845,10 +847,14 @@ function actionArgs(
   action: PullRequestAction,
   mergeMethod: PullRequestMergeMethod | undefined,
   updateMethod: PullRequestUpdateMethod | undefined,
+  bypassRules: boolean | undefined,
 ): ReadonlyArray<string> {
   switch (action) {
+    // `--admin` is gh's name for merging with the repository's rules stood down. It is refused
+    // by GitHub itself for anyone the repository does not allow that, so it is asked for only
+    // where the viewer's permissions already said yes.
     case "merge":
-      return ["merge", `--${mergeMethod ?? "merge"}`];
+      return ["merge", `--${mergeMethod ?? "merge"}`, ...(bypassRules === true ? ["--admin"] : [])];
     // `--auto` arms the same command instead of running it, and still needs the strategy: GitHub
     // stores the strategy with the standing instruction rather than choosing one at merge time.
     case "enable-auto-merge":
@@ -1752,6 +1758,7 @@ export const make = Effect.gen(function* () {
         input.action,
         input.mergeMethod,
         input.updateMethod,
+        input.bypassRules,
       );
       return github
         .execute({

apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts

@@ -3,13 +3,50 @@ import * as Effect from "effect/Effect";
 import * as Layer from "effect/Layer";
 import type { PullRequestReaction } from "@t3tools/contracts";
 
+import * as GitHubCli from "../sourceControl/GitHubCli.ts";
 import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts";
-import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts";
+import {
+  gitHubProviderFailure,
+  gitHubViewerPermissions,
+  loginAvatarUrl,
+  make,
+} from "./GitHubPullRequestProvider.ts";
 import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts";
 
+describe("gitHubProviderFailure", () => {
+  it("keeps which refusal it was, not only that the request failed", () => {
+    expect(
+      gitHubProviderFailure(
+        new GitHubCli.GitHubCliRefusedError({
+          command: "gh",
+          cwd: "/repo",
+          cause: null,
+          refusal: "merge-conflict",
+          detail: "The two branches conflict, so no merge commit can be created from them.",
+        }),
+      ),
+    ).toEqual({ reason: "failed", refusal: "merge-conflict" });
+  });
+
+  it("says no more than failed about a plain command failure", () => {
+    expect(
+      gitHubProviderFailure(
+        new GitHubCli.GitHubCliCommandError({ command: "gh", cwd: "/repo", cause: null }),
+      ),
+    ).toEqual({ reason: "failed" });
+  });
+});
+
 describe("gitHubViewerPermissions", () => {
-  it("offers everything to a viewer who can write to the repository", () => {
-    expect(gitHubViewerPermissions({ canWrite: true, canUpdate: true, didAuthor: false })).toEqual({
+  it("offers everything to a viewer who administers the repository", () => {
+    expect(
+      gitHubViewerPermissions({
+        canWrite: true,
+        canUpdate: true,
+        didAuthor: false,
+        canAdminister: true,
+      }),
+    ).toEqual({
       // Arming a merge for later is the merge, so it travels with it.
       actions: [
         "merge",
@@ -24,33 +61,59 @@ describe("gitHubViewerPermissions", () => {
       resolve: true,
       verdicts: ["comment", "approve", "request-changes"],
       requestReviewers: true,
+      mergeBypass: true,
     });
   });
 
+  it("keeps merging past the branch's rules to the administrators who may", () => {
+    expect(
+      gitHubViewerPermissions({
+        canWrite: true,
+        canUpdate: true,
+        didAuthor: false,
+        canAdminister: false,
+      }).mergeBypass,
+    ).toBe(false);
+  });
+
   it("leaves a passer-by on a repository they can only read nothing but the review", () => {
     // Every open-source pull request somebody else opened: GitHub says no to all five actions
     // and to resolving, and yes to commenting and to every verdict.
     expect(
-      gitHubViewerPermissions({ canWrite: false, canUpdate: false, didAuthor: false }),
+      gitHubViewerPermissions({
+        canWrite: false,
+        canUpdate: false,
+        didAuthor: false,
+        canAdminister: false,
+      }),
     ).toEqual({
       actions: [],
       comment: true,
       resolve: false,
       verdicts: ["comment", "approve", "request-changes"],
       // Asking somebody else to review is the one thing read access never stretches to.
       requestReviewers: false,
+      mergeBypass: false,
     });
   });
 
   it("keeps an author's own pull request theirs to close, with read access and no more", () => {
-    expect(gitHubViewerPermissions({ canWrite: false, canUpdate: true, didAuthor: true })).toEqual({
+    expect(
+      gitHubViewerPermissions({
+        canWrite: false,
+        canUpdate: true,
+        didAuthor: true,
+        canAdminister: false,
+      }),
+    ).toEqual({
       // Merging is the one thing writing is needed for, now or later; the rest an author may do.
       actions: ["ready", "draft", "close", "reopen"],
       comment: true,
       resolve: true,
       // GitHub refuses an author's approval of their own change, so the page does not offer one.
       verdicts: ["comment"],
       requestReviewers: false,
+      mergeBypass: false,
     });
   });
 
@@ -70,6 +133,7 @@ describe("gitHubViewerPermissions", () => {
         resolve: false,
         verdicts: ["comment", "approve", "request-changes"],
         requestReviewers: false,
+        mergeBypass: false,
       });
     }).pipe(
       Effect.provide(
@@ -110,7 +174,12 @@ describe("gitHubViewerPermissions", () => {
               mergeCapabilities: { merge: true, squash: true, rebase: true },
             }),
           getViewerAccess: () =>
-            Effect.succeed({ canWrite: false, canUpdate: true, didAuthor: false }),
+            Effect.succeed({
+              canWrite: false,
+              canUpdate: true,
+              didAuthor: false,
+              canAdminister: false,
+            }),
         }),
       ),
     ),
@@ -157,7 +226,8 @@ describe("getViewerPermissions", () => {
     Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({
       getPullRequestDetail: () => Effect.succeed(openDetail),
       getPullRequestBaseComparison: () => comparison,
-      getViewerAccess: () => Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }),
+      getViewerAccess: () =>
+        Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false, canAdminister: false }),
     });
 
   it.effect("offers update-branch when the comparison grants it", () =>
@@ -203,7 +273,7 @@ describe("getViewerPermissions", () => {
           getViewerAccess: (input) =>
             Effect.sync(() => {
               viewerAllowReserve = input.allowReserve;
-              return { canWrite: true, canUpdate: true, didAuthor: false };
+              return { canWrite: true, canUpdate: true, didAuthor: false, canAdminister: false };
             }),
         }),
       ),
@@ -238,7 +308,12 @@ describe("getViewerPermissions", () => {
               }),
             ),
           getViewerAccess: () =>
-            Effect.succeed({ canWrite: true, canUpdate: true, didAuthor: false }),
+            Effect.succeed({
+              canWrite: true,
+              canUpdate: true,
+              didAuthor: false,
+              canAdminister: false,
+            }),
         }),
       ),
     ),

apps/server/src/pullRequest/GitHubPullRequestProvider.ts

@@ -30,6 +30,7 @@ const CAPABILITIES: PullRequestCapabilities = {
     "disable-auto-merge",
   ],
   mergeMethods: ["merge", "squash", "rebase"],
+  mergeBypass: true,
   updateMethods: ["merge", "rebase"],
   search: true,
   reactions: true,
@@ -79,6 +80,10 @@ export function gitHubViewerPermissions(access: GitHubViewerAccess): PullRequest
     // leaves them commenting, which is what an author has to say about their own change anyway.
     verdicts: access.didAuthor ? (["comment"] as const) : CAPABILITIES.review.verdicts,
     requestReviewers: access.canWrite,
+    // Only an administrator, and only where the plain merge is already theirs: a bypass is that
+    // same merge with the repository's rules stood down, not a way into a repository this
+    // account may not write to at all.
+    mergeBypass: access.canWrite && access.canAdminister,
     ...(access.canUpdateBranch === true ? { updateMethods: CAPABILITIES.updateMethods } : {}),
   };
 }
@@ -93,6 +98,11 @@ export function gitHubProviderFailure(
   if (error._tag === "SourceControlRateLimitPausedError") {
     return { reason: "rate-limited", retryAt: error.retryAt };
   }
+  // A refusal is still a failed request; what it adds is which one, so the page can offer the
+  // way out where there is one rather than leave the reader with a sentence and no button.
+  if (error._tag === "GitHubCliRefusedError") {
+    return { reason: "failed", refusal: error.refusal };
+  }
   return { reason: "failed" };
 }
 
@@ -431,6 +441,7 @@ export const make = Effect.gen(function* () {
           action: input.action,
           ...(input.mergeMethod === undefined ? {} : { mergeMethod: input.mergeMethod }),
           ...(input.updateMethod === undefined ? {} : { updateMethod: input.updateMethod }),
+          ...(input.bypassRules === undefined ? {} : { bypassRules: input.bypassRules }),
         })
         .pipe(Effect.mapError(fail("runAction"))),
 

apps/server/src/pullRequest/PullRequestProvider.ts

@@ -32,7 +32,10 @@ import type {
   PullRequestViewerPermissions,
   SourceControlProviderKind,
 } from "@t3tools/contracts";
-import { SourceControlProviderKind as SourceControlProviderKindSchema } from "@t3tools/contracts";
+import {
+  PullRequestRefusal,
+  SourceControlProviderKind as SourceControlProviderKindSchema,
+} from "@t3tools/contracts";
 
 /**
  * The one failure shape every provider reports, so the service can decide what a failure means
@@ -49,6 +52,8 @@ export class PullRequestProviderError extends Schema.TaggedErrorClass<PullReques
     operation: Schema.String,
     reason: Schema.Literals(["missing-tool", "unauthenticated", "rate-limited", "failed"]),
     detail: Schema.String,
+    /** The refusal behind a `failed`, where the host named one. */
+    refusal: Schema.optional(PullRequestRefusal),
     retryAt: Schema.optional(Schema.Number),
     cause: Schema.optional(Schema.Defect()),
   },
@@ -61,6 +66,7 @@ export class PullRequestProviderError extends Schema.TaggedErrorClass<PullReques
 export interface PullRequestProviderFailure {
   readonly reason: PullRequestProviderError["reason"];
   readonly retryAt?: number | undefined;
+  readonly refusal?: PullRequestRefusal | undefined;
 }
 
 /** A change request as the provider sees it, before the service attaches project context. */
@@ -393,6 +399,11 @@ export interface PullRequestProviderApi {
       readonly mergeMethod?: PullRequestMergeMethod;
       /** Only meaningful for `update-branch`; absent takes the host's own default. */
       readonly updateMethod?: PullRequestUpdateMethod;
+      /**
+       * Merge with the branch's rules stood down. Only meaningful for `merge`, and only ever
+       * passed to a provider that reports `capabilities.mergeBypass`.
+       */
+      readonly bypassRules?: boolean;
     },
   ) => Effect.Effect<void, PullRequestProviderError>;