From b1820d4ede52363404b96f30338113ccaf2626ee Mon Sep 17 00:00:00 2001 From: Russ Taylor Date: Wed, 18 Feb 2026 06:35:40 -0800 Subject: [PATCH 1/4] fix: Don't report check to old sha This fixes an issue where a failed git checkout (because of a missing ref, network, or auth issues) would cause a failure to be posted to GitHub for the wrong sha - most often the last successfully built sha. This has caused confusion when one or more checks was reported as passing only to be unexpectedly changed to failing for an unrelated reason. It looks for cases where the git build data doesn't match the current build and returning an empty string for the sha if that's the case. Otherwise the behavior should remain the same. I added a new test to cover this case and I've tested it both with a manual repro in a sandbox environment and in a production environment (we run approximately 100k PR checks through this plugin on an average day) to confirm that it's working as expected when build failures occur before or during checkout. Prior to this fix - a failure during git checkout would report the failure to the previous successful sha: ``` > gh api "repos/$ORG/$REPO/commits/$SHA/check-runs?filter=all" | jq '.check_runs[] | {name, head_sha, status, conclusion, completed_at}' { "name": "gh-checks-plugin-fix-test", "head_sha": "71a1cc8f8b956b11b12036b8687acd7a5fafa868", "status": "completed", "conclusion": "failure", "completed_at": "2026-02-18T15:49:53Z" } { "name": "gh-checks-plugin-fix-test", "head_sha": "71a1cc8f8b956b11b12036b8687acd7a5fafa868", "status": "completed", "conclusion": "success", "completed_at": "2026-02-18T15:48:46Z" } ``` And with this fix, the build failed. But, as expected, it skipped reporting the failure to the wrong sha: ``` > gh api "repos/$ORG/$REPO/commits/$SHA/check-runs?filter=all" | jq '.check_runs[] | {name, head_sha, status, conclusion, completed_at}' { "name": "gh-checks-plugin-fix-test", "head_sha": "5dd2fbbbf2d102e0bb72f1ab76e5c00b8b29cf2e", "status": "completed", "conclusion": "success", "completed_at": "2026-02-18T15:58:32Z" } ``` The main caveat with this is that we use Freestyle jobs, not Pipeline jobs, so I'm relying on the existing test case to ensure it's still working as expected for pipelines. --- .../checks/github/GitSCMChecksContext.java | 18 ++++++++--- .../github/GitSCMChecksContextITest.java | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index 358c0dc5..d6c0bb87 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -48,6 +48,17 @@ class GitSCMChecksContext extends GitHubChecksContext { @Override public String getHeadSha() { + // When checkout fails, BuildData is either absent from the current run or + // carries a stale build number from a previous build. In both cases, + // GIT_COMMIT from run.getEnvironment() is also unreliable because + // GitSCM.buildEnvironment() walks back through previous builds' BuildData. + BuildData gitBuildData = run.getAction(BuildData.class); + if (gitBuildData == null + || gitBuildData.lastBuild == null + || gitBuildData.lastBuild.getBuildNumber() != run.getNumber()) { + return StringUtils.EMPTY; + } + try { String head = getGitCommitEnvironment(); if (StringUtils.isNotBlank(head)) { @@ -67,11 +78,8 @@ public String getGitCommitEnvironment() throws IOException, InterruptedException private String getLastBuiltRevisionFromBuildData() { BuildData gitBuildData = run.getAction(BuildData.class); - if (gitBuildData != null) { - Revision lastBuiltRevision = gitBuildData.getLastBuiltRevision(); - if (lastBuiltRevision != null) { - return lastBuiltRevision.getSha1().getName(); - } + if (gitBuildData != null && gitBuildData.getLastBuiltRevision() != null) { + return gitBuildData.getLastBuiltRevision().getSha1().getName(); } return StringUtils.EMPTY; } diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java b/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java index c5450551..acfb75c2 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java @@ -82,4 +82,35 @@ void shouldRetrieveContextFromPipeline(JenkinsRule j) throws Exception { assertThat(gitSCMChecksContext.getCredentialsId()).isEqualTo(CREDENTIALS_ID); assertThat(gitSCMChecksContext.getHeadSha()).isEqualTo(EXISTING_HASH); } + + /** + * Verifies that when a checkout fails on a subsequent build, {@link GitSCMChecksContext#getHeadSha()} + * returns empty rather than returning the stale SHA from the previous successful build. + * This prevents checks from being posted against the wrong commit when checkout fails + * (e.g. due to network flakiness). + */ + @Test + public void shouldReturnEmptyHeadShaWhenCheckoutFails() throws Exception { + FreeStyleProject job = j.createFreeStyleProject(); + + // First build: successful checkout populates BuildData with the correct SHA + GitSCM scm = new GitSCM(GitSCM.createRepoList(HTTP_URL, CREDENTIALS_ID), + Collections.singletonList(new BranchSpec(EXISTING_HASH)), + null, null, Collections.emptyList()); + job.setScm(scm); + Run successfulRun = buildSuccessfully(job); + assertThat(new GitSCMChecksContext(successfulRun, URL_NAME).getHeadSha()).isEqualTo(EXISTING_HASH); + + // Second build: use a non-existent SHA to simulate checkout failure. + // The Git plugin carries over BuildData from the previous build, but since + // the checkout never completes, lastBuild.hudsonBuildNumber still refers + // to build #1. + job.setScm(new GitSCM(GitSCM.createRepoList(HTTP_URL, CREDENTIALS_ID), + Collections.singletonList(new BranchSpec("0000000000000000000000000000000000000001")), + null, null, Collections.emptyList())); + Run failedRun = j.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0, new Action[0])); + + // Must return empty, not the previous build's SHA + assertThat(new GitSCMChecksContext(failedRun, URL_NAME).getHeadSha()).isEmpty(); + } } From e79606096aa416433c2b8db2abf50a7d35351a7c Mon Sep 17 00:00:00 2001 From: Russ Taylor Date: Thu, 19 Feb 2026 08:19:02 -0800 Subject: [PATCH 2/4] fix tests I had made the original changes based on an older version of the plugin so it would be compatible with the older version of Jenkins we're still using and I forgot to re-run the tests after I cherry-picked the commit to the latest on `master`. This is now passing when I run tests locally. --- .../plugins/checks/github/GitSCMChecksContextITest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java b/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java index acfb75c2..d1f68d1b 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitSCMChecksContextITest.java @@ -90,7 +90,7 @@ void shouldRetrieveContextFromPipeline(JenkinsRule j) throws Exception { * (e.g. due to network flakiness). */ @Test - public void shouldReturnEmptyHeadShaWhenCheckoutFails() throws Exception { + void shouldReturnEmptyHeadShaWhenCheckoutFails(JenkinsRule j) throws Exception { FreeStyleProject job = j.createFreeStyleProject(); // First build: successful checkout populates BuildData with the correct SHA @@ -98,7 +98,7 @@ public void shouldReturnEmptyHeadShaWhenCheckoutFails() throws Exception { Collections.singletonList(new BranchSpec(EXISTING_HASH)), null, null, Collections.emptyList()); job.setScm(scm); - Run successfulRun = buildSuccessfully(job); + Run successfulRun = buildSuccessfully(j, job); assertThat(new GitSCMChecksContext(successfulRun, URL_NAME).getHeadSha()).isEqualTo(EXISTING_HASH); // Second build: use a non-existent SHA to simulate checkout failure. From ac66e5b996fdd17499c0d0e8c5dc0cf1b8ce80ce Mon Sep 17 00:00:00 2001 From: Russ Taylor Date: Thu, 19 Feb 2026 16:46:23 -0800 Subject: [PATCH 3/4] Avoid potential null pointer exception to fix spotbugs --- .../jenkins/plugins/checks/github/GitSCMChecksContext.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index d6c0bb87..58909d89 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -78,8 +78,11 @@ public String getGitCommitEnvironment() throws IOException, InterruptedException private String getLastBuiltRevisionFromBuildData() { BuildData gitBuildData = run.getAction(BuildData.class); - if (gitBuildData != null && gitBuildData.getLastBuiltRevision() != null) { - return gitBuildData.getLastBuiltRevision().getSha1().getName(); + if (gitBuildData != null) { + Revision lastBuiltRevision = gitBuildData.getLastBuiltRevision(); + if (lastBuiltRevision != null) { + return lastBuiltRevision.getSha1().getName(); + } } return StringUtils.EMPTY; } From a5e82387ff746d578184d0c0361ea305fb79cccd Mon Sep 17 00:00:00 2001 From: Russ Taylor Date: Thu, 19 Feb 2026 17:33:47 -0800 Subject: [PATCH 4/4] mock builddata --- .../checks/github/GitHubChecksPublisherFactoryTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java index 8e111835..097468fd 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java @@ -6,6 +6,8 @@ import hudson.model.TaskListener; import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; +import hudson.plugins.git.util.Build; +import hudson.plugins.git.util.BuildData; import jenkins.scm.api.SCMHead; import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; @@ -78,6 +80,12 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitSCM() throws SCMFacade scmFacade = mock(SCMFacade.class); EnvVars envVars = mock(EnvVars.class); + BuildData buildData = mock(BuildData.class); + Build lastBuild = mock(Build.class); + buildData.lastBuild = lastBuild; + when(lastBuild.getBuildNumber()).thenReturn(1); + when(run.getNumber()).thenReturn(1); + when(run.getAction(BuildData.class)).thenReturn(buildData); when(run.getParent()).thenReturn(job); when(run.getEnvironment(TaskListener.NULL)).thenReturn(envVars); when(envVars.get("GIT_COMMIT")).thenReturn("a1b2c3");