Skip to content

fix: skip redundant pull_request docker-build run for main/development-head PRs - #1236

Merged
Wikid82 merged 1 commit into
mainfrom
fix/docker-build-skip-pr-push-dedup
Aug 10, 2026
Merged

fix: skip redundant pull_request docker-build run for main/development-head PRs#1236
Wikid82 merged 1 commit into
mainfrom
fix/docker-build-skip-pr-push-dedup

Conversation

@Wikid82

@Wikid82 Wikid82 commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

.github/workflows/docker-build.yml triggers on both pull_request (no branch filter) and push (branches: [main, development]). Whenever a commit lands on main or development while an open PR's head branch is also main or development (bot-generated "propagate main→development" / promotion-style sync PRs), both triggers fire for the identical commit SHA and collide in the same concurrency: group, so cancel-in-progress: true kills whichever run GitHub treats as older — reported as a misleading cancelled red X even though nothing actually broke.

This PR adds an AND'd job-level skip condition to the setup job so the pull_request-triggered instance short-circuits to a clean skipped (not cancelled) whenever github.head_ref is main or development — since the push event for that identical commit already covers the build independently. Downstream jobs (build-amd64, build-arm64, merge-and-publish, scan-pr-image) all gate on needs.setup.result == 'success', so the whole graph cascades cleanly to skipped.

  • Plan: docs/plans/current_spec.md ("Plan: Skip redundant pull_request docker-build run for main/development-head PRs") — two rounds of Planning + two rounds of Supervisor review, both approved.
  • Single file changed: .github/workflows/docker-build.yml. One functional line (setup job if:) plus two cross-reference comments (on push.branches and the setup job if:).

Known, intentionally-deferred limitation (§3.4 of the plan)

This fix does not change which of the two triggered runs "wins" GitHub's concurrency-group race — cancel-in-progress: true cancellation is decided at workflow-run registration time, before any job's if: is evaluated, so this job-level change cannot influence that decision. Concretely:

  • If pull_request's setup job resolves to skipped before push registers (the commonly observed ordering in this repo), this is a strict improvement: push finds nothing meaningful to cancel.
  • If push registers before pull_request's job evaluates, pull_request still gets cancelled exactly as today — no regression, same as baseline.
  • The scenario where push's in-progress build gets cancelled by a later-registering pull_request run is neutral under this change — neither created nor improved, since it's decided at registration time, before this fix's if: logic ever runs.

A full fix would disambiguate the concurrency: group key by event type (e.g. append -${{ github.event_name }}) so push and pull_request never share a group at all — but that's a separately-scoped change needing review of every downstream consumer of the shared group semantics. Tracked as a follow-up: #1235.

Validation

  • actionlint .github/workflows/docker-build.yml — zero findings.
  • lefthook run pre-commit — passes cleanly (actionlint, semgrep, and all other staged-file hooks green).
  • Manual character-for-character diff of the final if: line against the plan's §3.2 expression — confirmed identical, parenthesization included.
  • §7.1 (this PR's own live run): this PR's head branch (fix/docker-build-skip-pr-push-dedup) is neither main nor development, so the new skip clause never activates here — this PR itself is the live evidence that normal feature-branch PRs are unaffected. See run results in the PR thread / CI checks below.
  • §7.2 (main/development-head PR skip behavior): not validated live in this PR. The plan's proposed scratch-PR test (head=main against a disposable base) was reconsidered during implementation: since this fix isn't merged to main yet, a head=main PR opened right now would exercise the current, unfixed main copy of docker-build.yml, not this fix — so it wouldn't actually test the new condition. Constructing a branch that both (a) has github.head_ref literally equal to main/development and (b) contains this fix's commit would require pushing to a branch literally named main or development, which risks colliding with/mutating the real branches — assessed as not worth the risk for a test. Per the plan's own §7.2 "honest limitation" section, this is falling back to the already-completed Supervisor truth-table verification (all six event-type cases confirmed correct during plan review) plus the exhaustive edge-case table in plan §5. Live confirmation of the skip firing correctly in the wild will happen naturally the next time a real bot-generated main↔development sync PR occurs post-merge — recommend a follow-up check of gh run list --workflow=docker-build.yml at that time.

Follow-up

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-advanced-security

Copy link
Copy Markdown
Contributor

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Supply Chain Verification Results

PASSED

📦 SBOM Summary

  • Components: 1495

🔍 Vulnerability Scan

Severity Count
🔴 Critical 0
🟠 High 0
🟡 Medium 5
🟢 Low 2
Total 14

📎 Artifacts

  • SBOM (CycloneDX JSON) and Grype results available in workflow artifacts

Generated by Supply Chain Verification workflow • View Details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants