diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index 0368dae627d6dbce054c68b03489e532bae65e44..99fe8bd4f8abee19c4c7f923c9e2175a9bf43bd4 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -1017,7 +1017,12 @@ describe("repository access decoding", () => { // The one place an unknown answer is not granted: a Merge button a reader cannot use wastes // the press, where a missing one still leaves the pull request open on its host. expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson())).canWrite).toBe(false); - expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson(null))).canWrite).toBe(false); + }); + + it("grants write and triage to a GitHub App, which GitHub answers with a null permission", () => { + const access = expectSuccess(decodeViewerPermissionsJson(repositoryJson(null))); + expect(access.canWrite).toBe(true); + expect(access.canTriage).toBe(true); }); }); diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index b69e6c1302e7305d3aaab911384a3b3bc67a737d..ea756b5bc54fa0879bb0d299513401aeb066df75 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -708,9 +708,9 @@ const RawRepositoryAccessSchema = Schema.Struct({ squashMergeAllowed: Schema.Boolean, rebaseMergeAllowed: Schema.Boolean, /** - * ADMIN, MAINTAIN, WRITE, TRIAGE, READ or NONE. Optional rather than required, unlike the - * three above: an install that does not report it leaves the viewer's standing unknown, which - * is answered by granting rather than by failing the whole detail read. + * ADMIN, MAINTAIN, WRITE, TRIAGE, READ or NONE, or null for a GitHub App. Optional rather than + * required, unlike the three above: an install that does not report it still decodes rather + * than failing the whole detail read, and `toCanWrite` decides what that standing allows. */ viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), }); @@ -2919,8 +2919,13 @@ export interface GitHubRepositoryAccess { * need, and offering those to a reader who cannot use them wastes the press and reads as the app * being wrong. Everything softer — commenting, reviewing, resolving — keeps the granting default, * because being unable to say something is the worse failure there. + * + * A null permission is not that. GitHub answers null only to a GitHub App installation, and no + * API says what an installation may do, so null is granted and GitHub refuses whatever the app + * was not given. A person always gets a role, READ at the least. */ -function toCanWrite(viewerPermission: string | null | undefined): boolean { +export function toCanWrite(viewerPermission: string | null | undefined): boolean { + if (viewerPermission === null) return true; switch (viewerPermission?.trim().toUpperCase()) { case "ADMIN": case "MAINTAIN": diff --git a/packages/source-control-github/src/server/githubStackActions.test.ts b/packages/source-control-github/src/server/githubStackActions.test.ts index 846d88332a60b93d94688f2c031c826e16060302..8fb72f8127553441c9a1d7dd22cc23dbb2f71387 100644 --- a/packages/source-control-github/src/server/githubStackActions.test.ts +++ b/packages/source-control-github/src/server/githubStackActions.test.ts @@ -325,6 +325,26 @@ it.effect("allows a fork that explicitly permits maintainer updates", () => }), ); +it.effect("rebases for a GitHub App, which GitHub gives no permission to read", () => + Effect.gen(function* () { + gitCalls = []; + const api = fake([ + stack, + { + data: { + repository: { + pr2: { headRepository: { viewerPermission: null }, maintainerCanModify: false }, + pr3: { headRepository: { viewerPermission: null }, maintainerCanModify: false }, + }, + }, + }, + ...rebaseResponses, + ]); + yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); + expect(gitCalls.filter((args) => args[0] === "push")).toHaveLength(2); + }), +); + it.effect("bounds polling and reports a still-running merge without claiming success", () => Effect.gen(function* () { const api = fake([ diff --git a/packages/source-control-github/src/server/githubStackActions.ts b/packages/source-control-github/src/server/githubStackActions.ts index a6d90711620f29deeec0450901ed4b964fcc1260..428319909e901736dbf39a57d7c056b776e87f6e 100644 --- a/packages/source-control-github/src/server/githubStackActions.ts +++ b/packages/source-control-github/src/server/githubStackActions.ts @@ -10,7 +10,7 @@ import * as Schema from "effect/Schema"; import * as GitHubApi from "./GitHubApi.ts"; import { aliasedGraphQlDocument } from "./githubGraphQl.ts"; -import { decodePullRequestStacksJson } from "./gitHubPullRequestJson.ts"; +import { decodePullRequestStacksJson, toCanWrite } from "./gitHubPullRequestJson.ts"; import { cascadeRebaseStack } from "./githubStackRebase.ts"; const stackErrorIdentity = { @@ -216,8 +216,7 @@ export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* const pr = access.data.repository?.[`pr${layer.number}`]; return ( !pr?.headRepository || - (!pr.maintainerCanModify && - !["ADMIN", "MAINTAIN", "WRITE"].includes(pr.headRepository.viewerPermission ?? "")) + (!pr.maintainerCanModify && !toCanWrite(pr.headRepository.viewerPermission)) ); }) )