feat(ci): report scopes against the pull request head - #1794
Conversation
Merge Protections🔴 2 of 6 protections blocking · waiting on 👀 reviews
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
There was a problem hiding this comment.
🟢 Approval recommended
The implementation is consistent with existing HTTP/error-handling patterns and is backed by focused unit + live smoke tests, with only a minor spelling nit noted.
Pull request overview
This PR updates mergify ci scopes-send to report scope sets against a pull request’s head commit SHA (via PUT /v1/repos/{owner}/{repo}/commits/{sha}/scopes) to prevent stale uploads from overwriting the authoritative scopes for the current PR head, while preserving a PR-number fallback for compatibility and missing-head scenarios.
Changes:
- Add CI head-SHA detection and a
--head-shaoption, validate it as a full 40-hex SHA, and normalize to lowercase before pathing. - Add an HTTP helper to probe PUT endpoints with “404 means route absent” semantics, enabling a safe fallback to the existing PR-number POST endpoint.
- Extend unit tests and live smoke coverage to exercise the commit-scoped route and ensure fallback behavior is observable.
File summaries
| File | Description |
|---|---|
| skills/mergify-ci/SKILL.md | Updates CLI skill docs to describe head-SHA addressing, fallback behavior, and the new --head-sha option. |
| crates/mergify-core/src/http.rs | Adds put_no_response_if_exists (404→Ok(false)) and tests for the new behavior. |
| crates/mergify-cli/tests/live_smoke.rs | Adds a live smoke case to exercise the commit-scoped scopes endpoint and ensure it doesn’t silently fall back. |
| crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap | Updates the CLI schema snapshot to reflect the new --head-sha option and help text. |
| crates/mergify-cli/src/main.rs | Wires --head-sha through clap, dispatch, and native command execution into mergify-ci. |
| crates/mergify-ci/src/testing.rs | Adds write_github_event helper to reduce duplication in CI event-payload tests. |
| crates/mergify-ci/src/scopes_send.rs | Implements head-SHA resolution, commit-scoped reporting with 404 fallback, and adds targeted tests for the new behavior. |
| crates/mergify-ci/src/detector.rs | Adds head-SHA detection for supported CI providers plus clap parsing/validation for --head-sha, with tests. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`ci scopes-send` addressed its report by pull request number alone, so
nothing tied an uploaded scope set to the head it was computed for and
whichever upload landed last won. A cancelled job, a failed upload step,
or a replayed workflow run therefore left an obsolete head's scopes
authoritative, and the merge queue batched on them.
It now reports against the pull request's head SHA, through the engine's
`PUT /v1/repos/{owner}/{repository}/commits/{sha}/scopes`, so the report
says which revision produced it.
The head comes from `--head-sha`, else from the CI environment: the
GitHub Actions event payload's `.pull_request.head.sha`, or the head
commit Buildkite and CircleCI check out. Not `get_head_sha`, which
answers the different question "which revision did the tests run
against" and falls back to `GITHUB_SHA` — on a `pull_request` event that
is the synthetic merge commit and on `pull_request_target` the base
branch, so a report keyed on either would be stored where nothing ever
reads it. Jenkins is left uncovered for the same reason: the Branch
Source plugin builds a pull request merged into its target by default,
and `GIT_COMMIT` is then that merge commit.
Whatever the source, the value is held to a full 40-character hex SHA
and lowercased before it reaches the path: the endpoint answers 422 to
any other shape, and a row stored under a spelling no reader looks for
is worse than a rejected one.
The number-addressed endpoint stays as the fallback for the two cases
the commit route cannot serve, rather than turning either into a failure
of a command that works today:
- No head SHA can be named. `scopes-send` runs on an explicit
`--pull-request` from anywhere, and on Buildkite with `BUILDKITE_COMMIT`
unset; neither names a revision.
- The deployment answers 404. The commit endpoint shipped in monorepo
#39078; an on-prem Mergify older than that does not serve it, and
breaking scope uploads there would degrade the queue far worse than
reporting at the precision it already had. The fallback stays on POST
rather than the pull-request route's own PUT, which landed only four
months earlier and may be missing from the same deployments.
Both fallbacks say so on stderr, so a run never silently reports less
than it could.
The live smoke suite gains a case for the commit route. Its exit status
is not the assertion: a 404 and a fall back also exit 0, which is
exactly the absence the case exists to catch, so it requires that the
fallback stayed quiet.
Fixes MRGFY-8884
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MdqicbiJyjj1TNvYuVNL3b
Change-Id: I2376d2ad10966becb9e814c54ad8ed9ea5c8470d
fe052bd to
5a5898c
Compare
Revision history
|
ci scopes-sendaddressed its report by pull request number alone, sonothing tied an uploaded scope set to the head it was computed for and
whichever upload landed last won. A cancelled job, a failed upload step,
or a replayed workflow run therefore left an obsolete head's scopes
authoritative, and the merge queue batched on them.
It now reports against the pull request's head SHA, through the engine's
PUT /v1/repos/{owner}/{repository}/commits/{sha}/scopes, so the reportsays which revision produced it.
The head comes from
--head-sha, else from the CI environment: theGitHub Actions event payload's
.pull_request.head.sha, or the headcommit Buildkite and CircleCI check out. Not
get_head_sha, whichanswers the different question "which revision did the tests run
against" and falls back to
GITHUB_SHA— on apull_requestevent thatis the synthetic merge commit and on
pull_request_targetthe basebranch, so a report keyed on either would be stored where nothing ever
reads it. Jenkins is left uncovered for the same reason: the Branch
Source plugin builds a pull request merged into its target by default,
and
GIT_COMMITis then that merge commit.Whatever the source, the value is held to a full 40-character hex SHA
and lowercased before it reaches the path: the endpoint answers 422 to
any other shape, and a row stored under a spelling no reader looks for
is worse than a rejected one.
The number-addressed endpoint stays as the fallback for the two cases
the commit route cannot serve, rather than turning either into a failure
of a command that works today:
scopes-sendruns on an explicit--pull-requestfrom anywhere, and on Buildkite withBUILDKITE_COMMITunset; neither names a revision.
#39078; an on-prem Mergify older than that does not serve it, and
breaking scope uploads there would degrade the queue far worse than
reporting at the precision it already had. The fallback stays on POST
rather than the pull-request route's own PUT, which landed only four
months earlier and may be missing from the same deployments.
Both fallbacks say so on stderr, so a run never silently reports less
than it could.
The live smoke suite gains a case for the commit route. Its exit status
is not the assertion: a 404 and a fall back also exit 0, which is
exactly the absence the case exists to catch, so it requires that the
fallback stayed quiet.
Fixes MRGFY-8884
Co-Authored-By: Claude Opus 5 (1M context) [email protected]
Claude-Session: https://claude.ai/code/session_01MdqicbiJyjj1TNvYuVNL3b