[Bug]: Settings-preview install guidance re-introduces the PR-write overprivilege that #427 removed — `requiredInstallPermissions`/`activeMissingPermissions` demand `pull_requests: write` while the rest of the app (and the same file's own warning) say `read` + `issues: write`

[joaovictor712]

Summary

src/signals/settings-preview.ts now contradicts itself on what GitHub App permissions a repo needs to publish PR comments/labels. Two recently-merged PRs reached opposite conclusions on the same question and both landed:

• fix(github-app): avoid PR write overprivilege #427 (fix(github-app): avoid PR write overprivilege) — its commit message states: "Require pull request read access and issues write access for public comment/label flows instead of broad pull request write permission." It set REQUIRED_INSTALLATION_PERMISSIONS.pull_requests = "read" (src/github/backfill.ts), rewrote buildWarnings to require only issues: write, and updated the tests to assert pull_requests: read.
• fix(settings): require and surface pull_requests:write in the install preview for PR comment/label output #420 (fix(settings): require and surface pull_requests:write …) — merged after fix(github-app): avoid PR write overprivilege #427, it changed the other two helpers in the same file to require and flag pull_requests: write.

The result is a live self-contradiction inside settings-preview.ts:

// buildWarnings (line 328-331)  — from #427, CORRECT
if ((decision.willComment || decision.willLabel) && missing.has("issues")) {
  warnings.push("Comments and labels use GitHub Issues endpoints and require GitHub App permission Issues: write. Set Issues to write, then approve the change.");
}

// requiredInstallPermissions (line 484-488)  — from #420, now wrong
// PR conversation comments and PR labels are gated by GitHub on the Pull requests permission (write),
// matching REQUIRED_INSTALLATION_PERMISSIONS; reading PR metadata only needs read.        <-- comment is now false
const writesPrSurface = writesPrPublicSurface(settings, decision);
const permissions = new Set(["metadata: read", writesPrSurface ? "pull_requests: write" : "pull_requests: read"]);
if (writesPrSurface) permissions.add("issues: write");

// activeMissingPermissions (line 497-499)  — from #420, now wrong
if (writesPrSurface && missing.has("pull_requests")) active.push("pull_requests");
if (writesPrSurface && missing.has("issues")) active.push("issues");

buildWarnings says comment/label need issues: write (Issues endpoints). requiredInstallPermissions says they need pull_requests: write. The requiredInstallPermissions comment even claims it is "matching REQUIRED_INSTALLATION_PERMISSIONS" — but that constant now reads pull_requests: "read", so the comment is provably false against the current code.

Why this is wrong

PR conversation comments (POST /repos/{owner}/{repo}/issues/{n}/comments) and PR labels (/issues/{n}/labels) are GitHub Issues endpoints; a GitHub App can call them with issues: write. The app never writes through the Pull requests API (it only reads PRs), which is exactly the reasoning #427 applied when it reduced the requirement to pull_requests: read to avoid asking maintainers for a broader scope than the app uses.

REQUIRED_INSTALLATION_PERMISSIONS (src/github/backfill.ts) — the app's authoritative declared baseline — is now:

export const REQUIRED_INSTALLATION_PERMISSIONS: Record<string, string> = {
  metadata: "read",
  pull_requests: "read",   // #427
  issues: "write",
};

So requiredInstallPermissions/activeMissingPermissions (which still demand pull_requests: write) disagree with the app's declared baseline, with the install-health permission diagnostics, and with the warning text shown in the very same preview.

Downstream impact

buildRepoInstallPreview builds the maintainer-facing permission checklist from these helpers:

• installPreview.permissions.required lists pull_requests: write for comment/label output — telling maintainers to grant a write scope the app does not need and that fix(github-app): avoid PR write overprivilege #427 deliberately stopped requesting (the overprivilege regression).
• installPreview.permissions.missing (from activeMissingPermissions) flags pull_requests as a blocker for comment/label output. A maintainer who has granted pull_requests: read + issues: write (everything the app actually needs) is told they are still missing pull_requests and pushed to grant write — while the same preview's warning text tells them only Issues: write is required. The guidance is internally inconsistent and over-asks.

This directly defeats #427's stated goal ("avoid PR write overprivilege") and re-creates the broad-permission ask for every repo previewing comment/label output.

Steps to reproduce

1. Build a repo settings preview (buildRepoSettingsPreview → buildRepoInstallPreview) for a repo with comment or label output enabled.
2. Inspect installPreview.permissions.required and the permissions checklist/warnings.
3. Observe required contains pull_requests: write and missing can contain pull_requests, while buildWarnings and REQUIRED_INSTALLATION_PERMISSIONS say only issues: write (with pull_requests: read) is needed — a direct contradiction within the same preview.

Expected behavior

Settings-preview permission guidance matches REQUIRED_INSTALLATION_PERMISSIONS and #427's decision: comment/label outputs require issues: write (and pull_requests: read as the baseline), and a missing pull_requests permission is not presented as a comment/label blocker. Consistent with buildWarnings in the same file.

Actual behavior

requiredInstallPermissions lists pull_requests: write for comment/label output and activeMissingPermissions flags missing pull_requests, re-introducing the PR-write overprivilege #427 removed and contradicting the same file's warning text and the app's baseline constant.

Suggested fix

Align the two helpers with #427 / REQUIRED_INSTALLATION_PERMISSIONS (this reverts the source change from #420, which conflicted with the earlier #427 overprivilege fix):

• requiredInstallPermissions: keep pull_requests: read as the baseline (do not upgrade to write for comment/label); comment/label outputs add issues: write (already present). Remove/replace the now-false "gated on the Pull requests permission (write)" comment.
• activeMissingPermissions: drop the missing.has("pull_requests") check for comment/label outputs; keep the issues/checks checks (mirroring buildWarnings).
• Update the settings-preview test that asserts pull_requests: write back to read, matching the assertions fix(github-app): avoid PR write overprivilege #427 already established elsewhere.

Add fail-on-revert coverage: a preview with comment/label output enabled lists pull_requests: read (not write) in required, and an installation missing only pull_requests (with issues: write granted) is not flagged as needing attention for comment/label output.

[JSONbored]

These issue titles are ridiculously long, short / succinct titling would be appreciated moving forward.