Summary
For pull request runs, Test.commit stores the PR's head SHA, but the binary that actually runs is built from GitHub's merge ref — a merge of that head into master. The commit shown in the UI and returned by the API was never built.
Evidence
Sampled 22 completed runs from the last 100, using read-only API access only:
|
recorded commit == binary's own commit |
| master runs |
8 / 8 |
| PR runs |
2 / 14 |
The binary reports its own commit in the build log (Git commit: in the version block). For all 12 mismatching PR runs, that commit is a merge commit whose parents are exactly (a master commit, the recorded head):
run 9490 recorded 6e1e22a3b8 binary e98f1a2f81
e98f1a2f81 parents 128175ea7b + 6e1e22a3b8
"Merge 6e1e22a3b84da8190629648690a49e7d9974d9ca into 128175ea7b..."
run 9486 recorded 73a7b90ac6 binary 0531cf3e8f
0531cf3e8f parents 128175ea7b + 73a7b90ac6
run 9474 recorded b9d6c45671 binary 5facb55537
5facb55537 parents 6077cf5c61 + b9d6c45671
The two PR runs that did match (9492, 9493) fit the same explanation: their binary commit aee9693292 has a single parent, 128175ea7b, which was the master tip at the time. That branch was already even with master, so GitHub's merge ref resolved to the head itself and there was nothing to differ.
So: the recorded and built commits diverge whenever GitHub produces a merge commit — that is, whenever the PR branch is not already sitting on the master tip.
Cause
mod_ci/controllers.py:1994 takes the head SHA from the webhook payload:
commit_hash = payload['pull_request']['head']['sha']
...
add_test_entry(g.db, commit_hash, TestType.pull_request, pr_nr=pr_nr)
That value is right as far as it goes — schedule_tests depends on it resolving through repository.get_commit() and on comparing it against test_pr.head.sha. The gap is that nothing records the commit that was actually built.
Impact
- Reproduction gives you different code. Checking out the commit shown on the run page and building it does not produce the binary that ran.
- Master's changes are present in the tested binary but not in the recorded commit. A PR run's binary contains master commits that the PR's own base does not, so a comparison against a baseline at the recorded commit is not comparing what it appears to compare.
Correction to an earlier version of this issue: I originally cited "unrelated PRs reporting identical sets of new failures" as motivation here. That symptom is explained by #1173 — the expected file rolling forward from the previous run — not by this one. The commit mismatch above stands on its own evidence; I have removed the conflated claim.
Suggested fix
Record the built commit alongside the head rather than instead of it — e.g. a built_commit column on Test, populated from the artifact's workflow run, or parsed from the Git commit: line the binary already prints. The UI can then show "PR head X, tested as merge Y", and API consumers can tell the two apart.
Reproducing
sp run show 9490 # recorded commit
sp run logs 9490 --all | grep 'Git commit' # what the binary says
Found while reviewing every open CCExtractor pull request through sp_cli, without shell access to the server.
Summary
For pull request runs,
Test.commitstores the PR's head SHA, but the binary that actually runs is built from GitHub's merge ref — a merge of that head into master. The commit shown in the UI and returned by the API was never built.Evidence
Sampled 22 completed runs from the last 100, using read-only API access only:
The binary reports its own commit in the build log (
Git commit:in the version block). For all 12 mismatching PR runs, that commit is a merge commit whose parents are exactly (a master commit, the recorded head):The two PR runs that did match (9492, 9493) fit the same explanation: their binary commit
aee9693292has a single parent,128175ea7b, which was the master tip at the time. That branch was already even with master, so GitHub's merge ref resolved to the head itself and there was nothing to differ.So: the recorded and built commits diverge whenever GitHub produces a merge commit — that is, whenever the PR branch is not already sitting on the master tip.
Cause
mod_ci/controllers.py:1994takes the head SHA from the webhook payload:That value is right as far as it goes —
schedule_testsdepends on it resolving throughrepository.get_commit()and on comparing it againsttest_pr.head.sha. The gap is that nothing records the commit that was actually built.Impact
Correction to an earlier version of this issue: I originally cited "unrelated PRs reporting identical sets of new failures" as motivation here. That symptom is explained by #1173 — the expected file rolling forward from the previous run — not by this one. The commit mismatch above stands on its own evidence; I have removed the conflated claim.
Suggested fix
Record the built commit alongside the head rather than instead of it — e.g. a
built_commitcolumn onTest, populated from the artifact's workflow run, or parsed from theGit commit:line the binary already prints. The UI can then show "PR head X, tested as merge Y", and API consumers can tell the two apart.Reproducing
Found while reviewing every open CCExtractor pull request through sp_cli, without shell access to the server.