Honor GitHub PR approvals as CR signoffs

[ianrohde]

Summary

• Treat a GitHub native Approve review (state: APPROVED) as a CR signoff, so reviewers who approve on GitHub no longer need to add a CR :emoji: comment.
• Gated behind useGithubApprovalForCr (default on); skipped when the review body already produces a CR signature.

Problem

Pulldasher had no concept of a GitHub PR approval. CR signoff required manually adding a CR :emoji: comment on every PR, even when the reviewer had already approved natively on GitHub.

Solution

• models/signature.js: parseReview maps an APPROVED review to a CR signature, deduped against an existing CR body tag. Works for both the pull_request_review webhook and full PR refresh.
• This is read-only: Pulldasher reflects approvals made on GitHub. It does not submit approvals on a user's behalf.

Deployment notes

• Enable the Pull request reviews webhook event on tracked repositories.
• Set useGithubApprovalForCr: false to keep legacy emoji-only behavior.

Test plan

• npm test (APPROVED-to-CR mapping, dedup, CHANGES_REQUESTED ignored)
• npm run lint
• Manual: approve a PR on GitHub, confirm the CR bubble fills in Pulldasher
• Manual: dismiss the approval on GitHub, confirm the CR signoff drops after refresh

[ianrohde]

QA: logic + webhook wiring verified. Live GitHub E2E not run (DB/infra blockers below).

Done

• npm test passes 3/3: APPROVED→CR, dedup against a CR :emoji: body, CHANGES_REQUESTED ignored.
• Ran 5 more cases directly against Signature.parseReview, all as expected: COMMENTED→0 CR; DISMISSED→0 CR; APPROVED with body: null→1 CR and no throw (the hasTag(body || "") fix); APPROVED + QA :tada: body→1 CR (QA also parsed); APPROVED with useGithubApprovalForCr: false→0 CR.
• Traced both live callers of parseReview — both carry review.state: the pull_request_review webhook (submitted → insertSignatures, instant) in controllers/githubHooks.js, and full refresh via getReviews→pulls.listReviews in lib/git-manager.js. Also confirmed by reading that an approval-derived CR goes inactive after a new commit (stale logic in git-manager.js), and DB-stored reviews aren't re-parsed, so there's no stale-state path.

Not done, and why

The two manual boxes — approve on GitHub fills the CR bubble, dismiss drops it after refresh — weren't verified end-to-end. Test machine has no MySQL/Docker for the app's DB, and real webhook delivery needs a GitHub OAuth app, a public tunnel, and a PR approvable by a non-author (can't approve your own). Not reproducible headlessly.

Net: the parsing logic and the webhook/refresh wiring feeding it are verified; only GitHub's live review-event delivery and the resulting UI bubble are unverified. Recommend a manual approve+dismiss pass on a tracked/staging repo before relying on it, with the Pull request reviews webhook event enabled.

[kwiens]

Seems reasonable, let's make sure this logs to the db the same as a regular CR.

This is all because github predates GH approvals by many years. We were first!

[ianrohde]

Confirmed same write path, both triggers:

• parseReview (approval) and parseComment (regular CR) both build a Signature with the same .data shape — models/signature.js:69 vs :91.
• Both flow → insertSignatures → insertSignature → new DBSignature → INSERT INTO pull_signatures SET ? (models/db_signature.js:27). Same table, same columns.
• Webhook path: pull_request_review submitted → insertSignature (controllers/githubHooks.js:120-125), mirrors the issue_comment path at :91-96.
• Full-refresh path: git-manager.js:195-199 parses reviews into the same signatures array as comments, then updateAllPullData delete-then-reinsert (db-manager.js:53-57).

[ianrohde]

@danielbeardsley would you mind giving this a quick check since you're most familiar?

[danielbeardsley]

CR 👍

Nice!