Skip to content

Commit f026a96

Browse files
authored
fix(github): count reviewCheckMode toward installation health's Checks permission (#5377)
refreshInstallationHealthRecords only checked checkRunMode ("Gittensory Context") when deciding whether an installation needs the Checks: write permission, missing the separate reviewCheckMode axis ("Gittensory Orb Review Agent"). buildInstallationRepairDiagnostics already ORs both (checkRunRepoCount / gateCheckRepoCount); the persisted health record was missing the second arm. Confirmed live against JSONbored's own 3 production repos, none of which set checkRunMode but all of which set reviewCheckMode -- the health check has never correctly flagged the Checks permission requirement for them.
1 parent 7935bb4 commit f026a96

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

src/github/backfill.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1193,7 +1193,12 @@ async function refreshInstallationHealthRecords(env: Env, installations: Install
11931193
const installedRepos = repositories.filter((repo) => repo.installationId === currentInstallation.id && repo.isInstalled);
11941194
const registeredInstalled = installedRepos.filter((repo) => repo.isRegistered);
11951195
const installedSettings = await Promise.all(installedRepos.map((repo) => resolveRepositorySettings(env, repo.fullName)));
1196-
const requiresChecks = installedSettings.some((settings) => settings.checkRunMode === "enabled");
1196+
// #5355: also require Checks: write when the review-agent check-run (reviewCheckMode) publishes, not
1197+
// just when the separate context check (checkRunMode) is enabled -- buildInstallationRepairDiagnostics
1198+
// above already ORs both (checkRunRepoCount / gateCheckRepoCount); this persisted health record was
1199+
// missing the second arm, so an installation with only reviewCheckMode set never got flagged for the
1200+
// Checks permission it actually needs.
1201+
const requiresChecks = installedSettings.some((settings) => settings.checkRunMode === "enabled" || shouldPublishReviewCheck(settings.reviewCheckMode));
11971202
const requiresPrWrite = installedSettings.some((settings) => agentRequiresPrWrite(settings.autonomy));
11981203
const requiresContentsWrite = installedSettings.some((settings) => agentRequiresContentsWrite(settings.autonomy));
11991204
const requiredPermissions = {

test/unit/backfill.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,55 @@ describe("GitHub backfill", () => {
10201020
);
10211021
});
10221022

1023+
it("REGRESSION (#5355): requires Checks write for a repo with only reviewCheckMode (the Orb Review Agent check) set, not checkRunMode", async () => {
1024+
// Before the fix, requiresChecks only looked at checkRunMode ("Gittensory Context" check) and missed
1025+
// the separate reviewCheckMode axis ("Gittensory Orb Review Agent" check) entirely -- so an installation
1026+
// whose repos only ever published the review-agent check (true for JSONbored's own 3 production repos,
1027+
// none of which set checkRunMode) was never flagged as needing the Checks permission.
1028+
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
1029+
await seedRegisteredRepo(env);
1030+
await upsertInstallation(env, {
1031+
installation: {
1032+
id: 123,
1033+
account: { login: "JSONbored", id: 1, type: "User" },
1034+
repository_selection: "selected",
1035+
permissions: { metadata: "read", pull_requests: "write", issues: "write" },
1036+
events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"],
1037+
},
1038+
});
1039+
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 123);
1040+
await upsertRepositorySettings(env, {
1041+
repoFullName: "JSONbored/gittensory",
1042+
reviewCheckMode: "required", // checkRunMode left at its default ("off")
1043+
});
1044+
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
1045+
const url = input.toString();
1046+
if (url.endsWith("/app/installations/123")) {
1047+
return Response.json({
1048+
id: 123,
1049+
account: { login: "JSONbored", id: 1, type: "User" },
1050+
repository_selection: "selected",
1051+
permissions: { metadata: "read", pull_requests: "write", issues: "write" },
1052+
events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"],
1053+
});
1054+
}
1055+
return new Response("not found", { status: 404 });
1056+
});
1057+
1058+
const refreshed = await refreshInstallationHealth(env);
1059+
1060+
expect(refreshed.installations).toEqual(
1061+
expect.arrayContaining([
1062+
expect.objectContaining({
1063+
installationId: 123,
1064+
status: "needs_attention", // was falsely "healthy" before the fix
1065+
missingPermissions: ["checks"],
1066+
requiredPermissions: expect.objectContaining({ checks: "write" }),
1067+
}),
1068+
]),
1069+
);
1070+
});
1071+
10231072
it("REGRESSION (#audit-install-health): an acting autonomy requires pull_requests:write, so read-only is needs_attention not healthy", async () => {
10241073
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
10251074
await seedRegisteredRepo(env);

0 commit comments

Comments
 (0)