Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2450,8 +2450,8 @@ function buildCommandPreview(
};
}

if (missingPermissions.includes("issues") || missingPermissions.includes("pull_requests")) {
const summary = "GitHub App permissions Issues: write and Pull requests: write are required before a command response can be posted.";
if (missingPermissions.includes("issues")) {
const summary = "GitHub App permission Issues: write is required before a command response can be posted.";
const body = sanitizePublicComment(`Gittensory preview is ready for ${target}, but ${summary}`);
return {
...base,
Expand Down Expand Up @@ -2603,9 +2603,9 @@ function buildCommandPreviewPullRequest(

function commandPreviewMissingPermissions(request: z.infer<typeof commandPreviewSchema>, installation: InstallationHealthRecord | null): string[] {
const configured = new Set([...(installation?.missingPermissions ?? []), ...(request.sample?.missingPermissions ?? [])]);
configured.delete("pull_requests");
const permissions = request.sample?.permissions ?? installation?.permissions;
if (permissions && permissions.issues !== "write") configured.add("issues");
if (permissions && permissions.pull_requests !== "write") configured.add("pull_requests");
return [...configured].sort();
}

Expand All @@ -2621,8 +2621,6 @@ function commandPreviewPermissionWarnings(missingPermissions: string[]) {
message:
permission === "issues"
? "Command responses require GitHub App permission Issues: write; preview will not post while it is missing."
: permission === "pull_requests"
? "Command responses require GitHub App permission Pull requests: write; preview will not post while it is missing."
: `GitHub App permission ${permission}: ${requiredAccess} is missing for this preview scenario.`,
};
});
Expand Down
14 changes: 7 additions & 7 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ export async function refreshContributorActivity(

export const REQUIRED_INSTALLATION_PERMISSIONS: Record<string, string> = {
metadata: "read",
pull_requests: "write",
pull_requests: "read",
issues: "write",
};
export const OPTIONAL_CHECK_RUN_PERMISSION: Record<string, string> = {
Expand Down Expand Up @@ -721,19 +721,19 @@ export async function buildInstallationRepairDiagnostics(env: Env, health: Insta
mode: "comment",
enabled: commentRepoCount > 0,
affectedRepoCount: commentRepoCount,
permission: "pull_requests",
permission: "issues",
requiredAccess: "write",
missing: missingPermissions.has("pull_requests"),
summary: "PR comments are posted on pull requests, so comment mode requires Pull requests: write.",
missing: missingPermissions.has("issues"),
summary: "PR comments use GitHub issue comment endpoints, so comment mode requires Issues: write.",
}),
buildPermissionModeImpact({
mode: "label",
enabled: labelRepoCount > 0,
affectedRepoCount: labelRepoCount,
permission: "pull_requests",
permission: "issues",
requiredAccess: "write",
missing: missingPermissions.has("pull_requests"),
summary: "PR labels are applied to pull requests, so label mode requires Pull requests: write.",
missing: missingPermissions.has("issues"),
summary: "PR labels use GitHub issue label endpoints, so label mode requires Issues: write.",
}),
buildPermissionModeImpact({
mode: "check_run",
Expand Down
6 changes: 3 additions & 3 deletions src/signals/settings-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,9 @@ function buildWarnings(settings: RepositorySettings, decision: PublicSurfaceDeci
return warnings;
}
const missing = new Set(installation.missingPermissions);
if ((decision.willComment || decision.willLabel) && (missing.has("issues") || missing.has("pull_requests"))) {
if ((decision.willComment || decision.willLabel) && missing.has("issues")) {
warnings.push(
"Comments and labels require GitHub App permissions Issues: write and Pull requests: write. Set both repository permissions to write, then approve the change.",
"Comments and labels use GitHub Issues endpoints and require GitHub App permission Issues: write. Set Issues to write, then approve the change.",
);
}
if (settings.checkRunMode === "enabled" && missing.has("checks")) {
Expand Down Expand Up @@ -383,7 +383,7 @@ function buildRepoInstallPreview(args: {
action:
commandAuthorizationStatus === "ready"
? "Use command previews to confirm actor and permission behavior before relying on repo commands."
: "Restore Issues: write and Pull requests: write before enabling public command responses.",
: "Restore Issues: write before enabling public command responses.",
},
{
id: "audit-behavior",
Expand Down
12 changes: 6 additions & 6 deletions test/integration/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ describe("api routes", () => {
expect(installationHealth.status).toBe(200);
await expect(installationHealth.json()).resolves.toMatchObject({
installationId: 123,
requiredPermissions: { metadata: "read", pull_requests: "write", issues: "write" },
requiredPermissions: { metadata: "read", pull_requests: "read", issues: "write" },
optionalPermissions: { checks: "write" },
permissionRemediation: expect.arrayContaining([expect.objectContaining({ permission: "issues", ok: true })]),
repairSteps: ["No repair needed."],
Expand Down Expand Up @@ -1313,15 +1313,15 @@ describe("api routes", () => {
};
expect(repairBody).toMatchObject({
installation: { status: "needs_attention", missingPermissions: ["pull_requests", "issues"], missingEvents: ["issue_comment"] },
requiredPermissions: { metadata: "read", pull_requests: "write", issues: "write" },
requiredPermissions: { metadata: "read", pull_requests: "read", issues: "write" },
optionalPermissions: { checks: "write" },
refresh: { method: "POST", path: "/v1/installations/777/repair/refresh" },
});
expect(repairBody.requiredPermissions).not.toHaveProperty("checks");
expect(repairBody.modeImpacts).toEqual(
expect.arrayContaining([
expect.objectContaining({ mode: "comment", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "pull_requests", missing: true, optional: false })] }),
expect.objectContaining({ mode: "label", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "pull_requests", missing: true, optional: false })] }),
expect.objectContaining({ mode: "comment", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "issues", missing: true, optional: false })] }),
expect.objectContaining({ mode: "label", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "issues", missing: true, optional: false })] }),
expect.objectContaining({ mode: "check_run", enabled: false, affectedRepoCount: 0, requiredPermissions: [expect.objectContaining({ permission: "checks", missing: false, optional: true })] }),
]),
);
Expand Down Expand Up @@ -1368,7 +1368,7 @@ describe("api routes", () => {
await expect(refreshed.json()).resolves.toMatchObject({
refreshed: true,
installation: { status: "healthy", missingPermissions: [], missingEvents: [] },
requiredPermissions: { metadata: "read", pull_requests: "write", issues: "write", checks: "write" },
requiredPermissions: { metadata: "read", pull_requests: "read", issues: "write", checks: "write" },
});
});

Expand Down Expand Up @@ -2095,7 +2095,7 @@ describe("api routes", () => {
);
expect(permissionMapPreview.status).toBe(200);
await expect(permissionMapPreview.json()).resolves.toMatchObject({
preview: { decision: { status: "missing_permission", skipReason: "missing_permission" }, missingPermissions: ["issues", "pull_requests"] },
preview: { decision: { status: "missing_permission", skipReason: "missing_permission" }, missingPermissions: ["issues"] },
});

const checksWarningPreview = await app.request(
Expand Down
8 changes: 4 additions & 4 deletions test/unit/backfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,17 +440,17 @@ describe("GitHub backfill", () => {
installedReposCount: 2,
registeredInstalledCount: 0,
status: "needs_attention",
missingPermissions: ["pull_requests"],
missingPermissions: ["issues"],
missingEvents: [],
permissions: { metadata: "read", pull_requests: "read", issues: "write" },
permissions: { metadata: "read", pull_requests: "read", issues: "read" },
events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"],
checkedAt: "2026-05-28T00:00:00.000Z",
});

expect(repair.modeImpacts).toEqual(
expect.arrayContaining([
expect.objectContaining({ mode: "comment", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "pull_requests", missing: true })] }),
expect.objectContaining({ mode: "label", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "pull_requests", missing: true })] }),
expect.objectContaining({ mode: "comment", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "issues", missing: true })] }),
expect.objectContaining({ mode: "label", enabled: true, affectedRepoCount: 1, requiredPermissions: [expect.objectContaining({ permission: "issues", missing: true })] }),
]),
);
});
Expand Down
Loading