fix: read the PR number from the event payload, not only from GITHUB_REF - #6
Merged
Merged
Conversation
A pull_request run whose GITHUB_REF is not refs/pull/<number>/merge aborted with "Could not determine PR number from GITHUB_REF" and exit 1, before Layer 1 had measured anything. To the author that is indistinguishable from a coverage verdict: the check goes red having never looked at the diff. Observed on 2026-08-12 in Ostico/bruno-mcp-studio#180. Editing the PR body fired the `edited` activity type, which produced a second run on a head sha whose earlier run was green; that run failed this way, and because GitHub surfaces the latest run per check name, the merge button showed a red required gate for a commit that had passed. Every pull_request payload carries `.pull_request.number`, for every activity type. GITHUB_REF carries it only while GitHub has a merge ref to point the run at. So the payload at GITHUB_EVENT_PATH is now consulted first and GITHUB_REF remains the fallback, which keeps every case that worked before working. `_pr_number_from_event` returns None rather than raising for anything unexpected — no GITHUB_EVENT_PATH, an unreadable or malformed file, a payload for another event, a number that is not an int (`true` is valid JSON in that position, and bool is an int subclass). Each of those still has GITHUB_REF to fall back on, and raising would turn a recoverable run into a failed one. main.py's message now names both sources it tried and says outright that nothing was measured. As written it sent the reader to GITHUB_REF even once the payload was being consulted. One existing test needed a change for a reason worth stating: test_pr_number_from_env set GITHUB_REF and asserted 42, and this suite runs inside Actions, where GITHUB_EVENT_PATH points at a real payload naming a real PR. Reading the payload first means that test would have measured the CI run's own PR — passing locally and failing in CI. It now unsets GITHUB_EVENT_PATH explicitly. Twelve tests added. Each of the four ways to break this — never consulting the payload, letting the ref win, accepting a non-int number, letting a malformed payload raise — was verified to turn the suite red.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ✅ PASSChanged lines: 100.0% covered (threshold: 100%) 📋 2 files: 2 ✅ pass
Result: ✅ PASS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure
A
pull_requestrun whoseGITHUB_REFis notrefs/pull/<number>/mergeaborts withand exit 1 — before Layer 1 measures anything. From the author's side that is indistinguishable from a coverage verdict: the required check goes red having never looked at the diff.
Observed 2026-08-12 on
Ostico/bruno-mcp-studio#180. Editing the PR body fired theeditedactivity type, which produced a second run on a head sha whose earlier run was green. That second run failed this way, and because GitHub surfaces the latest run per check name, the merge button showed a red required gate for a commit that had already passed.The change
.pull_request.numberis present in everypull_requestpayload, for every activity type.GITHUB_REFcarries the number only while GitHub has a merge ref to point the run at.So
parse_confignow reads the payload atGITHUB_EVENT_PATHfirst and keepsGITHUB_REFas the fallback — every case that worked before still works._pr_number_from_eventreturnsNonerather than raising for anything unexpected: noGITHUB_EVENT_PATH, an unreadable or malformed file, a payload for another event, or a number that is not anint. That last one is not paranoia —trueis valid JSON in that position andboolis anintsubclass, so a plainisinstance(number, int)would accept it and passTruedownstream as PR number 1. Each of these cases still hasGITHUB_REFto fall back on, and raising would turn a recoverable run into a failed one.main.py's message now names both sources it tried and states that nothing was measured. As written it pointed the reader atGITHUB_REFeven once the payload was being consulted.One existing test changed, deliberately
test_pr_number_from_envsetGITHUB_REFand asserted42. This suite runs inside Actions, whereGITHUB_EVENT_PATHpoints at a real payload naming a real PR — so reading the payload first would have made that test measure CI's own PR number. It would have passed locally and failed in CI. It now unsetsGITHUB_EVENT_PATHexplicitly, with a comment saying why.Verification
12 tests added (47 in
test_config.py, 455 in the suite).ruff check src/ tests/clean.Mutation-checked — each of these turns the suite red:
intnumberWhat this does not do
Consumers pin
ostico/test-guard@v2, so merging changes nothing for them. The fix reachesbruno-mcp-studioonly when thev2tag moves to include this commit — a deliberate act, not done here.