Skip to content

fix: retry gh transient 404 on check-run update - #2922

Open
chmouel wants to merge 1 commit into
tektoncd:mainfrom
chmouel:retry-github-check-run-not-found
Open

fix: retry gh transient 404 on check-run update#2922
chmouel wants to merge 1 commit into
tektoncd:mainfrom
chmouel:retry-github-check-run-not-found

Conversation

@chmouel

@chmouel chmouel commented Aug 12, 2026

Copy link
Copy Markdown
Member

📝 Description of the Change

GitHub can answer with a 404 when Pipelines-as-Code updates a check run
it just created, causing PAC to abort PipelineRun creation and leave
the pull request with a failed check that /retest cannot recover
when triggering is restricted to pull_request events.

This fix creates the check run fully formed (output, annotations, and
conclusion when already finished) in a single call, so a freshly
created check run no longer needs an immediate follow-up update. For
check runs whose id is restored from a PipelineRun annotation, the
update is retried on an explicit 404 with a short exponential backoff
(500ms, 1s, 2s), since another reconcile may have created that check
run only moments earlier and GitHub can still report it missing.

Every other 404 remains terminal: an id discovered through a
check-run lookup, or a transport error/timeout, is never retried, so
deleted or inaccessible check runs and requests that may have already
reached GitHub still surface as errors instead of being retried
unsafely.

🔗 Linked GitHub Issue

Fixes #2920
Jira: SRVKP-13334

🧪 Testing Strategy

  • Unit tests
  • Integration tests
  • End-to-end tests
  • Manual testing
  • Not Applicable

Comprehensive unit tests were added covering:

  • Successful retry recovery from transient 404s
  • Terminal 404s for non-created check runs
  • Server errors and context cancellation
  • Exponential backoff timing

🤖 AI Assistance

  • I have used AI assistance for this PR.

This PR involved AI-assisted code generation and refinement. The implementation has been thoroughly reviewed and tested to ensure it meets the project's standards and correctly handles the edge cases around check-run creation and update retries.

✅ Submitter Checklist

  • 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
  • ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
  • ♽ I have run make test and make lint locally to check for and fix any issues.
  • 📖 I have added or updated documentation for any user-facing changes.
  • 🧪 I have added sufficient unit tests for my code changes.
  • 🎁 I have added end-to-end tests where feasible. See README for more details.
  • 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
  • If adding a provider feature, I have filled in the following and updated the provider documentation:
    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

Copilot AI lite review requested due to automatic review settings August 12, 2026 10:17
@chmouel chmouel added bug Something isn't working ok-to-test labels Aug 12, 2026
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.74074% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.71%. Comparing base (aeed7ee) to head (82afba7).

Files with missing lines Patch % Lines
pkg/provider/github/status.go 90.74% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2922      +/-   ##
==========================================
+ Coverage   80.66%   80.71%   +0.04%     
==========================================
  Files         164      164              
  Lines       13910    13937      +27     
==========================================
+ Hits        11221    11249      +28     
  Misses       1967     1967              
+ Partials      722      721       -1     
Flag Coverage Δ
unit-tests 80.71% <90.74%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pipelines-as-code

Copy link
Copy Markdown

Paco Review ✅

This PR adds a targeted retry mechanism in the GitHub provider so that a check-run update returning HTTP 404 immediately after Pipelines-as-Code itself created that check run is retried a few times with exponential backoff, instead of failing outright, since GitHub can briefly report a freshly-created check run as missing. It introduces isNotFoundError and updateCheckRun helpers, wires a new createdNow flag through getOrUpdateCheckRunStatus to gate the retry to only just-created check runs, adds a defensive nil check for checkRunID, and includes new table-driven tests plus a configmap documentation update describing the behavior.

Review difficulty: 3/5 (Moderate) — The change is moderate in size but touches a core, widely-used status-update path and introduces new retry/timing logic that needs careful validation of edge cases and test determinism.

No new review comments found at this time. Nice work!

Reviewed commit: 727bd28

@pipelines-as-code pipelines-as-code Bot added the paco/review-moderate Paco review difficulty label Aug 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves reliability of the GitHub provider by retrying only the check-run update call when GitHub returns a transient HTTP 404 immediately after Pipelines-as-Code created the check run in the same operation (to mitigate GitHub read-after-write lag), while keeping all other 404s terminal.

Changes:

  • Add isNotFoundError and updateCheckRun(...) helper to retry Checks.UpdateCheckRun on explicit 404 with short exponential backoff (500ms, 1s, 2s) when the check run was created “now”.
  • Wire getOrUpdateCheckRunStatus to call the new retrying helper only for newly-created check runs.
  • Add unit tests covering retry/no-retry behavior, context cancellation, and 404 detection; document this retry behavior in the configmap API docs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/provider/github/status.go Adds explicit 404 detection and a bounded backoff retry for check-run update only when the check run was created in the same operation.
pkg/provider/github/status_test.go Adds focused tests for the new retry behavior and helper logic using a fake clock and GitHub test server.
docs/content/docs/api/configmap.md Documents the always-on GitHub transient-404 retry for immediate post-create check-run updates.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@chmouel
chmouel force-pushed the retry-github-check-run-not-found branch from 727bd28 to e199e3a Compare August 12, 2026 10:25
Comment thread pkg/provider/github/status.go
@chmouel
chmouel force-pushed the retry-github-check-run-not-found branch from e199e3a to 80ce43d Compare August 13, 2026 07:49
@chmouel
chmouel force-pushed the retry-github-check-run-not-found branch from 80ce43d to 46c35cd Compare August 17, 2026 09:48
@zakisk
zakisk force-pushed the retry-github-check-run-not-found branch from b25c400 to 2f47f63 Compare August 31, 2026 10:24
GitHub can answer with 404 for a check run that Pipelines as Code has
just created and immediately updates. PAC treated that answer as
terminal, so it aborted PipelineRun creation and left the pull request
with a failed check that /retest could not recover when triggering is
restricted to pull_request events.

Create the check run fully formed, with its output, annotations, and
conclusion when the run is already finished, so a freshly created
check run needs no follow-up update at all.

Retry the update briefly when the check-run id came from the
PipelineRun annotation, since another reconcile may have created that
check run moments earlier and GitHub can still report it as missing.
Use three retries at 500ms, 1s, and 2s, which covers the create-to-
update lag observed in the report while still failing quickly on a
genuine error.

Keep every other 404 terminal. An id discovered through a check-run
lookup is not retried, so a deleted or inaccessible check run still
surfaces as an error instead of being hidden behind repeated requests.

Retry only an explicit 404 status response. Transport errors and
timeouts are excluded because such a request may already have reached
GitHub, and repeating it could apply the same update twice. Return the
original error once the retries are exhausted.

Leave the generic provider retry transport unchanged. Only this call
site knows whether the identifier came from the PipelineRun
annotation, which is the condition that makes the retry safe.

Fixes tektoncd#2920
Jira: https://issues.redhat.com/browse/SRVKP-13334
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
@zakisk
zakisk force-pushed the retry-github-check-run-not-found branch from 2f47f63 to 82afba7 Compare September 1, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ok-to-test paco/review-moderate Paco review difficulty

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHub check-run update 404s right after creation (read-after-write lag) aborts PipelineRun creation — needs retry

5 participants