[tsapeta]

Why

A workflow that reviews on a label event knows the commit the maintainer labeled, but ecr ci starts minutes later and reviews the PR's head at that time. A push in between gets reviewed, with the workflow's secrets in place, instead of the labeled code. On fork PRs that's content no maintainer has seen (raised on expo/expo#52375).

The diff had a second, smaller gap: ecr ci reads the head commit once and checks the tree out at it, but gh pr diff returns the PR's current diff, so a push between those calls gave the agents a diff from a newer commit than the tree.

Change

• GitHubPRSource.getChangedFiles reads the PR's head again after gh pr diff and throws HeadMovedError when it isn't the OID getMetadata read. The check runs whenever the metadata has a head OID, with or without the new flag.
• ecr ci --expect-head <sha> takes a full commit hash. Both CI paths compare it with the head they read, after the break-glass check and before any model call, and stop when the head differs or is unknown.
• The single-config path then posts a "not reviewed: the PR's head changed after the review was requested" note (couldNotComplete, so the inline sync stays additive and the ready label goes). It posts its usual failure note for a HeadMovedError from the diff check.
• The routed path only logs and removes the reviewer's ready label, for both cases. Its root comment holds the aggregate and its per-scope state, and a single-review render would replace that state, so a later --scopes run would lose the other scopes' results. The routed break-glass check now runs before the diff is fetched, so a skip note is never replaced either.
• A malformed flag fails as a usage error instead of being ignored: --expect-head=<sha>, an empty value (a workflow expression that's empty for the event), or a short hash. An ignored flag would review an unpinned head without a word.
• README (trust model), LLP 0007 and LLP 0008 describe both. LLP 0008 also states the one case the diff check can't see: a push and a force-push back to the same OID between the two reads.

A workflow can then pass github.event.pull_request.head.sha on label events. expo/expo will do that in a follow-up once this is released.

Test Plan

bun test (927 pass), typecheck, lint, fmt:check and llp:check pass. src/__tests__/github-pr-head.test.ts covers --expect-head parsing (full hash only, lowercased; the = form and an empty value are errors), the mismatch rule (an unknown head fails closed), and, through a fake gh, that the head is read again after the diff and that a moved head throws HeadMovedError. The CI paths' use of the mismatch rule, and the order of the break-glass and head checks, aren't covered by a test that runs ecr ci end to end.

[github-actions]

🤖 AI code review

Decision: Ready for human review

Overall PR risk: Medium. The change modifies the CI review path by switching getChangedFiles to use GitHub's compare endpoint instead of gh pr diff, and reorders when the break-glass and ready-label checks run in routed paths. The compare endpoint can differ from gh pr diff for certain PR shapes or hit size limits, potentially changing the diff reviewed, and early-exit behavior in routed runs differs. All CI reviews use the changed diff path, though the --expect-head gate is opt-in, and a revert directly restores the old behavior.

🔎 Reviewer summaries (3)

• consistency: I compared the new --expect-head flag, the pinned diff, and the new tests with their sibling code. The flag follows the parseValueFlag usage-error path and the CI_USAGE help format. The not-reviewed note matches the existing couldNotComplete reports. I found no consistency…
• correctness: I checked the --expect-head parsing, the head check in the legacy and routed CI paths, and the OID-pinned diff in getChangedFiles. I found no defects in the changed code.
• security: Reviewed --expect-head parsing, the head-mismatch gate on both CI paths, and the pinned compare diff in getChangedFiles. The code validates the values and passes them as argument arrays. It fails closed on an unknown head. I found no security defect.

🔗 Review setup:

• This PR changes code the reviewer prompts cite (src/tests). Check that the guidance quoting it is still correct.

No findings.

---

This review is advisory — it never blocks a merge and never auto-approves.

[brentvatne]

I pushed two fixes from a multi-model review:

• ea53703: the diff now compares the base and head commits that the metadata read (gh api repos/<o>/<r>/compare/<base>...<head> with the diff media type). It does not run gh pr diff and then read the head again. The old check missed a push followed by a force-push back to the same commit. On this PR and three expo/expo fork PRs, the compare diff matched gh pr diff byte for byte. This removes liveHeadOid and HeadMovedError. The --expect-head note now gives the head it saw and the commit it expected, or says the head is unknown.
• b3e8631: a repeated --expect-head is now a usage error. The README and LLP 0007 say that older releases ignore the flag.

In the description, the first "Change" bullet and the LLP 0008 caveat about the push and force-push now describe the old approach.

Known gaps, left out of this PR:

• No test runs ecr ci end to end, so the order of the checks in both CI paths has no test.
• A manual re-run of an old labeled run can replace a newer review with the "not reviewed" note. A correct fix needs the reviewed head stored in the comment state.