Pr Gate

PR gate

A pass/fail check on a pull request with one question in mind: can this be merged? This is not a thorough review. Style, naming, architecture opinions and "I would have done it differently" are out of scope unless they cause a real defect. The job is to catch the things that would make merging a mistake, and to say "go" quickly when nothing does.

Three questions, in order. Everything else is noise.

1. Does it do what it says? The diff matches the stated intent (title, description, linked issue).
2. Does it break anything? No regressions in the code it touches or the code that depends on it.
3. Is it tested enough? The new or changed behavior has tests that would fail if it were wrong, and CI is green.

Input

A PR reference in any form: a URL, owner/repo#123, or just #123 when the repo is already clear from the conversation. If the repo is ambiguous, ask — never guess a repo.

Step 1 — Read the PR

Use the GitHub MCP connector (pull_request_read) when it is available. In Claude Code without the connector, the gh CLI covers the same ground: gh pr view <n> --json title,body,baseRefName,headRefName,mergeable,closingIssuesReferences,reviews,comments, gh pr diff <n>, gh pr view <n> --json files, gh pr checks <n>, gh issue view <n>, and gh pr review <n> --approve|--request-changes --body for posting. The steps below name the MCP methods; map them to gh when needed.

• method get — title, body, base/head, mergeable state, linked issues.
• method get_diff — the full diff. Read all of it; a gate that skims is not a gate.
• method get_files — the list of touched files, to spot test files vs. source files.
• method get_status — CI checks.
• method get_review_comments and get_reviews — what others already flagged, so the verdict doesn't repeat or contradict existing feedback without reason.

If the body references an issue (Closes #N, Fixes #N, or a plain #N), read it with issue_read (method get). The issue is often the real spec; the PR body is a summary of it.

If the diff is large (roughly 800+ changed lines), say so up front and gate it in sections rather than pretending to have absorbed it in one pass. Very large PRs are themselves a merge risk worth naming.

Step 2 — Intent check

State in one sentence what the PR claims to do, then check the diff against it:

• Every claim in the title/body/issue is visible in the diff. A description that promises three things and a diff that does two is a "request changes" (or at least a question), not an approve.
• Nothing significant in the diff is unmentioned. Drive-by refactors, unrelated fixes, dependency bumps or config changes that the description doesn't cover get called out. They are not automatically blockers, but they widen the blast radius and the author should either own them in the description or split them out.
• If there is a linked issue, the acceptance criteria in it are met, not just the headline.

Step 3 — Regression check

Look for the ways this change could break something that currently works. Focus on what changed, not on the file in general:

• Changed signatures, return types, exported names, defaults, or removed code. For each one, find the callers. search_code scoped to the repo (repo:owner/name <symbol>) is the cheapest way, or rg <symbol> in a local checkout; get_file_contents for the ones that matter. A caller that wasn't updated is a bug. Do this check; it is the single most common way a "small" PR breaks production.
• Behavior changes in shared code (utilities, base classes, middleware, schemas, DB migrations, API contracts, FHIR resource shapes on Medplum projects). Anything many things depend on gets extra attention.
• Error and edge paths: null/undefined, empty collections, concurrency, retries, timezone/locale, partial failure. Only raise these where the diff plausibly introduces the problem, not as a checklist to recite.
• Data and migrations: is the migration reversible, does it run against existing rows, does the code work both before and after it applies.
• Config, env vars, secrets, feature flags: anything new must be set somewhere, or the deploy fails.
• Removed or weakened tests, skipped tests, loosened assertions. These are red flags regardless of what the code does.

Don't speculate. Every regression concern must point at a concrete line in the diff and say what would break. "This might cause issues" is not a finding.

Step 4 — Test check

The bar is "would the tests catch it if the change were wrong", not "is coverage complete".

• New behavior has at least one test that exercises the new path and would fail without the change.
• Bug fixes have a test that reproduces the bug. A fix with no regression test is a nit for trivial fixes and a blocker for anything that reached production or touches data.
• Changed behavior has its existing tests updated deliberately, not just made to pass. Tests rewritten to match the new output without asserting anything meaningful count as no test.
• CI is green. Red or missing CI is a blocker; don't reason around it.
• Pure refactors with no behavior change and existing coverage that still passes are fine without new tests. Say so explicitly rather than leaving the test section empty.

Judge by what the repo's tests look like. If the repo has no test suite at all, don't demand one for this PR; note it once and gate on the other two questions.

Step 5 — Verdict

Exactly one of:

• Approve — all three questions pass. Nothing further needed.
• Approve with nits — mergeable as-is; small optional improvements the author can take or leave. Nits never include anything from Step 3 that is a real defect.
• Request changes — at least one concrete blocker: intent not met, a real regression, missing/failing tests for non-trivial behavior, red CI, or an unexplained scope expansion large enough to matter.

When it is borderline, prefer "Request changes" with a single precise ask over "Approve" with a worried paragraph. The point of a gate is that "approve" means something.

Output

Keep it short. The reader is deciding whether to click merge.

**PR:** <repo> #<n> — <title>  (<+adds/-dels>, <n> files, CI <green/red/none>)
**Intent:** <one sentence: what it claims to do>

**Does it do what it says:** yes / partly / no — <one line>
**Breaks anything:** no / possibly / yes — <one line per concrete concern, with file:line>
**Tested enough:** yes / marginal / no — <one line>

**Verdict: Approve / Approve with nits / Request changes**
<blockers as a short list, if any; nits as a short list, if any>

**Suggested comment:**
<the comment, see below>

Findings reference path/to/file.ts:123 so they can be checked in one click. No long explanations; if a finding needs a paragraph, it is probably not a gate-level finding.

Deliver this inline in chat. No file, no artifact.

The PR comment

Draft the comment Gonzalo will post on the PR. Default assumption: he posts it himself — do not post it unless he explicitly says to ("post it", "submit the review", "approve it on GitHub"). If he does, use pull_request_review_write with event APPROVE for approvals (with or without nits) and REQUEST_CHANGES for requests, body set to the comment.

The comment is written in Gonzalo's voice as a reviewer, to the PR author. Rules:

• Two to five sentences. Plain, direct, friendly, no fluff, no emoji.
• Approve: say what was checked and that it looks good. One sentence is fine. Optionally list nits as "non-blocking:" in a second line.
• Request changes: name the specific thing(s) that need to change and why, each with a file reference. No hedging, no apologizing, no "great work but".
• Never restate the diff back to the author. Never include the internal three-question scaffold.
• Match the language of the PR. If the PR title/description and existing comments are in Spanish (Fuego / Rossi repos), write the comment in es-AR Spanish; otherwise English.

Example — approve:

Checked the diff against #142, the new retry path is covered by the added tests and the two existing callers of fetchOrders were updated. LGTM.

Example — approve with nits:

Looks good and does what #88 asks. Non-blocking: parseSlot in src/turnos/slots.ts:41 swallows the parse error silently, a log line there would save someone an afternoon later.

Example — request changes:

Two things before merge: OrderService.cancel changed its return type but src/jobs/reconcile.ts:77 still expects the old shape, so reconciliation will throw on cancelled orders. And the fix for the timezone bug has no test that reproduces it, so we can't tell if it's actually fixed. Happy to re-review once those are in.

What this skill is not

• Not a style review. Formatting, naming, comment quality, import order: ignore unless the repo enforces them in CI (then CI catches it, not this review).
• Not an architecture review. "This should be a strategy pattern" is not a gate finding.
• Not a rubber stamp. If the diff wasn't fully read, the callers weren't checked, or CI wasn't looked at, don't approve — say what's missing.

Verify before presenting

• The diff was read in full (or the PR was declared too large and gated in sections).
• Every changed public symbol had its callers checked, and the output says so.
• CI status was fetched, not assumed.
• Every "breaks anything" finding names a file and line.
• The verdict is one of the three allowed values and matches the findings (no "Approve" alongside an unresolved regression).
• The suggested comment is in the right language, under five sentences, and was not posted to GitHub.