Skip to content

refactor: count PR lines from the GitHub API instead of a git checkout - #273

Draft
NicolasMassart wants to merge 8 commits into
mainfrom
fix/pr-line-check-live-base
Draft

refactor: count PR lines from the GitHub API instead of a git checkout#273
NicolasMassart wants to merge 8 commits into
mainfrom
fix/pr-line-check-live-base

Conversation

@NicolasMassart

@NicolasMassart NicolasMassart commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Fixes MCWP-747, where a retargeted pull request could be labelled from a count taken against its former base branch.

Rather than patching the base-branch resolution, this replaces the git-based counting with the pull request files endpoint, which reports per-file additions and deletions against the pull request's current base — the same three-dot diff shown in the Files changed tab. Only the pull request number now comes from the webhook payload, so a payload that lags a base retarget can no longer skew the count.

That removes every moving part the count previously depended on:

  • actions/checkout, so the job no longer clones the repository
  • the depth 1 → 10 → 100 → --unshallow fetch loop and the merge-base probing
  • base-branch resolution, and with it the base-ref input

The action is now a single actions/github-script step, 240 lines down to 79, with no shell. Threshold and labelling behaviour is unchanged.

A pull request touching more than the 3000 files the endpoint lists is labelled size-XL and fails without counting lines, since it is over any configured limit regardless.

Compatibility

This ships under v1 as a non-breaking change.

Every caller invokes the action from a pull_request event, and none of them sets base-ref. Outside a pull request context the previous implementation already failed: the label step read context.payload.pull_request.number before its try block, so a branch run counted the lines, printed them, then threw a TypeError without ever reaching the pass/fail verdict. There is no working non-PR behaviour being removed.

The one new requirement is pull-requests: read, which callers granting pull-requests: write for labelling already have. Fork pull requests are unaffected: the token is read-only there, so the count succeeds and only the labelling is skipped, exactly as before.

contents: read is no longer needed in the calling workflow, though it is harmless to leave in place.

The workflow trigger fix that makes the check re-run on a base retarget is separate: MetaMask/metamask-mobile#33868.

Test plan

  • Open a pull request and confirm the reported additions, deletions and size label match the Files changed tab
  • Confirm files matching ignore-patterns are excluded from the total
  • Retarget a pull request to a base with a different amount of divergence and confirm the recomputed label reflects the new base
  • Confirm an existing, already-correct size label is left untouched, and a stale one is replaced without disturbing other labels
  • Confirm a pull request over max-lines fails the check and is labelled size-XL

This update introduces a new step to resolve the current pull request's base branch dynamically. It ensures that the action uses the live base branch instead of potentially stale data from the webhook payload, improving accuracy in line calculations.
Comment thread .github/actions/pr-line-check/action.yml Outdated
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread .github/actions/pr-line-check/action.yml Outdated
Co-authored-by: Mark Stacey <markjstacey@gmail.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 913b815. Configure here.

Comment thread .github/actions/pr-line-check/action.yml Outdated
NicolasMassart and others added 2 commits July 28, 2026 12:20
Co-authored-by: Cursor <cursoragent@cursor.com>
The pull request files endpoint reports per-file additions and deletions
against the pull request's current base, which removes the need to check
out the repository, resolve the base branch and probe git history for a
merge base. It also makes the count immune to a webhook payload that lags
a base retarget, since only the pull request number comes from the event.

A pull request touching more than the 3000 files the endpoint lists is
labelled size-XL without counting lines, as it is over any limit anyway.

Co-authored-by: Cursor <cursoragent@cursor.com>
@NicolasMassart NicolasMassart changed the title fix: resolve live PR base branch in pr-line-check refactor: count PR lines from the GitHub API instead of a git checkout Jul 28, 2026
@NicolasMassart
NicolasMassart marked this pull request as draft July 28, 2026 16:21
pull Bot pushed a commit to Reality2byte/metamask-mobile that referenced this pull request Jul 28, 2026
## **Description**

`check-pr-max-lines` could report a stale/incorrect size label (e.g.
`size-XL`) after a stacked PR was rebased onto `main` and its base
branch retargeted. The workflow only re-ran on `opened | reopened |
synchronize`, so a base change alone never triggered a fresh check
against the new base.

This adds `edited` to the `pull_request` trigger types, gated so the job
only re-runs for base-branch edits (`github.event.changes.base !=
null`), not unrelated title/body edits.

Related fix in the shared `pr-line-check` action (resolves the live PR
base instead of trusting the webhook payload):
MetaMask/github-tools#273

## **Changelog**

CHANGELOG entry: null

## **Related issues**

Refs:
[MCWP-747](https://consensyssoftware.atlassian.net/browse/MCWP-747)

## **Manual testing steps**

```gherkin
Feature: PR size check re-runs on base retarget

  Scenario: a stacked PR is rebased onto main and its base is retargeted
    Given a PR is stacked on another branch and the size check has already passed
    When the PR head is rebased onto main and the PR base is retargeted to main
    Then the check-pr-max-lines check re-runs and reports the correct size label against main
```

## **Screenshots/Recordings**

N/A - CI workflow change, no UI impact.

### **Before**

N/A

### **After**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

#### Performance checks (if applicable)

N/A - CI workflow-only change, no app runtime impact.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

[MCWP-747]:
https://consensyssoftware.atlassian.net/browse/MCWP-747?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> CI trigger-only change with no app, auth, or data impact; `edited` may
also run the job on title/body edits.
> 
> **Overview**
> **`check-pr-max-lines`** now listens for **`pull_request` `edited`**
in addition to `opened`, `reopened`, and `synchronize`.
> 
> That lets the shared **`pr-line-check`** action run again when a PR is
updated without a new push—e.g. after **retargeting the base branch**
following a rebase—so size labels reflect the diff against the current
base instead of staying stale.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
756f1cf. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Signed-off-by: Nicolas MASSART <nicolas.massart@consensys.net>
Co-authored-by: Cursor <cursoragent@cursor.com>
roz0n pushed a commit to MetaMask/metamask-mobile that referenced this pull request Jul 29, 2026
## **Description**

`check-pr-max-lines` could report a stale/incorrect size label (e.g.
`size-XL`) after a stacked PR was rebased onto `main` and its base
branch retargeted. The workflow only re-ran on `opened | reopened |
synchronize`, so a base change alone never triggered a fresh check
against the new base.

This adds `edited` to the `pull_request` trigger types, gated so the job
only re-runs for base-branch edits (`github.event.changes.base !=
null`), not unrelated title/body edits.

Related fix in the shared `pr-line-check` action (resolves the live PR
base instead of trusting the webhook payload):
MetaMask/github-tools#273

## **Changelog**

CHANGELOG entry: null

## **Related issues**

Refs:
[MCWP-747](https://consensyssoftware.atlassian.net/browse/MCWP-747)

## **Manual testing steps**

```gherkin
Feature: PR size check re-runs on base retarget

  Scenario: a stacked PR is rebased onto main and its base is retargeted
    Given a PR is stacked on another branch and the size check has already passed
    When the PR head is rebased onto main and the PR base is retargeted to main
    Then the check-pr-max-lines check re-runs and reports the correct size label against main
```

## **Screenshots/Recordings**

N/A - CI workflow change, no UI impact.

### **Before**

N/A

### **After**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

#### Performance checks (if applicable)

N/A - CI workflow-only change, no app runtime impact.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

[MCWP-747]:
https://consensyssoftware.atlassian.net/browse/MCWP-747?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> CI trigger-only change with no app, auth, or data impact; `edited` may
also run the job on title/body edits.
> 
> **Overview**
> **`check-pr-max-lines`** now listens for **`pull_request` `edited`**
in addition to `opened`, `reopened`, and `synchronize`.
> 
> That lets the shared **`pr-line-check`** action run again when a PR is
updated without a new push—e.g. after **retargeting the base branch**
following a rebase—so size labels reflect the diff against the current
base instead of staying stale.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
756f1cf. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Signed-off-by: Nicolas MASSART <nicolas.massart@consensys.net>
Co-authored-by: Cursor <cursoragent@cursor.com>
NicolasMassart and others added 3 commits July 31, 2026 10:11
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-mobile-platform Label for the mobile platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants