Skip to content

fix(ci): correctly detect release tag/stable branch pipelines triggered via GitHub mirror - #450

Merged
wdconinc merged 5 commits into
masterfrom
fix-tag-push-ci-detection
Oct 8, 2026
Merged

wdconinc merged 5 commits into
masterfrom
fix-tag-push-ci-detection

Conversation

@veprbl

@veprbl veprbl commented Oct 5, 2026

Copy link
Copy Markdown
Member

Problem

The GitLab CI version job failed to recognize v26.10.0-stable tag pushes mirrored
from GitHub (pipeline https://eicweb.phy.anl.gov/containers/eic_container/-/pipelines/150175),
causing it to tag images as unstable-mr- (empty PR number) instead of 26.10.0-stable,
and to skip the Docker Hub push entirely.

Root cause

.github/workflows/mirror.yaml sets GITHUB_REPOSITORY=eic/containers for every
eicweb-triggered pipeline from this repo (branch pushes, tag pushes, and PRs alike),
while GITHUB_PR is only populated for actual pull_request events.

In .gitlab-ci.yml's version job, the trigger-sourced branch checked
GITHUB_REPOSITORY == "eic/containers" to decide "is this a PR build?" — but that's
true for every trigger from this repo, not just PRs. So tag/stable-branch pushes were
misdetected as PR builds and never reached the CI_COMMIT_TAG detection logic below.

Separately, two regexes further down escape (, |, ) as literal characters in a
bash extended regex ([[ =~ ]]), where they need to be unescaped to mean
grouping/alternation — so pushes of e.g. v26.10-stable as a branch never matched
either.

Fix

  • Check -n "${GITHUB_PR}" instead of the repo name to detect actual PR builds.
  • Keep the downstream (epic/EICrecon) trigger case as a separate branch, now correctly
    only taken when there's no GITHUB_PR and the repo isn't eic/containers, so that
    our own tag/branch pushes fall through to the existing CI_COMMIT_TAG/CI_COMMIT_BRANCH
    detection unchanged.
  • Un-escape the (alpha|beta|stable) groups in the two stable-branch regexes.

Verified both code paths (PR-triggered and tag-triggered) with a standalone bash
simulation of the relevant variables.

…ed via GitHub mirror

The mirror workflow (.github/workflows/mirror.yaml) sets
GITHUB_REPOSITORY=eic/containers for every triggered eicweb pipeline from
this repo, not just for PR builds; GITHUB_PR is only non-empty for actual
pull_request events. The version job's "trigger" branch checked
GITHUB_REPOSITORY instead of GITHUB_PR, so tag pushes (e.g.
v26.10.0-stable) and stable-branch pushes mirrored from GitHub were
misdetected as unstable PR builds ("unstable-mr-" with empty PR number),
skipping the Docker Hub push and never reaching the CI_COMMIT_TAG
detection below.

Also fix two further-down conditions that escape '(' '|' ')' as literal
characters in a bash extended regex (where they must be unescaped to mean
grouping/alternation), which meant stable branch pushes (e.g.
v26.10-stable) never matched either.
Copilot AI balanced review requested due to automatic review settings October 5, 2026 22:20

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.

Copilot review overview

🟡 Changes recommended

The new regexes fail under Alpine’s BusyBox shell, and downstream PR image tags can collide.

Review effort: Balanced
Findings: 2 High severity · 1 Low severity

Open (3)
What changed in this PR

Corrects GitLab version detection for GitHub-mirrored pipelines.

Changes:

  • Detects PR pipelines using GITHUB_PR.
  • Separates downstream triggers from repository tag/branch pushes.
  • Adjusts stable-branch regexes.
File Description
.gitlab-ci.yml Updates mirrored pipeline classification and version matching.

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

Comment thread .gitlab-ci.yml
Comment thread .gitlab-ci.yml Outdated
Comment thread .gitlab-ci.yml Outdated
Address review: the PR-build branch must require GITHUB_REPOSITORY ==
eic/containers in addition to GITHUB_PR being set, otherwise eic/epic and
eic/EICrecon downstream PR triggers with the same PR number would collide
on the same unstable-mr-<N> tag and (since GH_PUSH was no longer disabled
for them) could overwrite each other's ghcr.io image. Restores the
original downstream-trigger behavior (CI_COMMIT_BRANCH version, both
GH_PUSH and DH_PUSH disabled) unchanged.
Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:17

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.

Copilot review overview

🔵 Needs a closer look

The unescaped inline regex groups cause BusyBox shell parse failures in the version job.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
Resolved since last review (1)

Address review: the version job runs in the top-level alpine image, whose
/bin/sh is BusyBox ash. Unescaped parens after [[ =~ are parsed as shell
grammar there and abort the whole script with a hard syntax error, rather
than just failing to match as in bash. Reproduced locally with dash
(same failure mode). Split each alternation into separate ||-joined
[[ =~ ]] tests instead of using a (alpha|beta|stable) group, so no raw
parens appear in the script.

Also generalized a comment that named only eic/epic and eic/EICrecon as
downstream triggerers; eic/EDM4eic uses the same path.
Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:20

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.

Copilot review overview

🟢 Approval recommended

The changes correctly address the reported trigger and shell parsing failures without altering unrelated pipeline behavior.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:30

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.

Copilot review overview

🟢 Approval recommended

The changes correctly distinguish PR, downstream, tag, and branch triggers without introducing unresolved issues.

Review effort: Balanced
Findings: None

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Capybara summary for PR 450

Copilot AI balanced review requested due to automatic review settings October 6, 2026 14:41

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.

Copilot review overview

🟢 Approval recommended

The updated conditions correctly preserve downstream behavior while allowing mirrored tags and stable branches to reach version detection.

Review effort: Balanced
Findings: None

@wdconinc
wdconinc enabled auto-merge (squash) October 8, 2026 02:20
@wdconinc
wdconinc merged commit d11fd52 into master Oct 8, 2026
96 checks passed
@wdconinc
wdconinc deleted the fix-tag-push-ci-detection branch October 8, 2026 06:32

This branch was successfully deployed

1 active deployment
github-pages — ab364592 Deployed Oct 6, 2026 by veprbl via deploy-artifacts-page #4273
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.

3 participants