Skip to content

Gate smoke build on test-release (back-port from PlexCleaner port) - #413

Merged
ptr727 merged 1 commit into
developfrom
backport-cicd-fixes
Jun 4, 2026
Merged

Gate smoke build on test-release (back-port from PlexCleaner port)#413
ptr727 merged 1 commit into
developfrom
backport-cicd-fixes

Conversation

@ptr727

@ptr727 ptr727 commented Jun 4, 2026

Copy link
Copy Markdown
Owner

Back-port from the PlexCleaner two-phase CI/CD port (ptr727/PlexCleaner#723), where this repo's patterns were audited for the same issues.

Applied

  • Gate smoke-build on test-release (needs: [changes, test-release]). The smoke build builds Docker images, so it shouldn't be spent when the test job is already failing; a failed test-release now leaves smoke-build skipped and the aggregator blocks on the test failure directly. Brings NxWitness in line with the sibling repos' PR workflows.

Audited — already present in NxWitness (no change needed)

Noted for maintainer follow-up (NOT changed here)

  • build-main feeds the versioned release from a moving ref: main while get-version resolves the release version separately — the same race fixed in PlexCleaner/ProjectTemplate by pinning leaf builds to GitCommitId. That pin is not safe to apply here: build-docker-task overloads inputs.ref as the image-matrix branch selector (select(.Branch == $ref)), so passing a SHA would match no rows. A proper fix decouples ref (checkout/version) from branch (matrix selector) — a small refactor better owned by the maintainer. (Low practical risk: requires a commit landing on main mid-publish.)

All workflows pass actionlint. Handed to the maintainer for merge.

Back-port from the PlexCleaner two-phase port (ptr727/PlexCleaner#723):
the smoke build builds Docker images, so it shouldn't be spent when the
test job is already failing. smoke-build now needs [changes, test-release]
so a failed test leaves it skipped and the aggregator blocks on the test
failure directly. Matches the sibling repos' PR workflows.

Audited the other discoveries from that port; NxWitness already has them
(github-release GitCommitId checkout + dispatch-refresh, gha cache
ignore-error, no push trigger on the publisher, explicit ref: main on the
docker-readme task). The one remaining gap — build-main feeds the
versioned release from a moving ref: main — is NOT safely fixable by
pinning to GitCommitId here, because build-docker-task overloads inputs.ref
as the image-matrix branch selector (.Branch == $ref); a SHA would match
no rows. Decoupling ref (checkout/version) from branch (matrix selector)
is a maintainer-owned refactor, noted in the PR for follow-up.
Copilot AI review requested due to automatic review settings June 4, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown

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 updates the pull request CI workflow to avoid running the Docker image smoke build when the release test job has already failed, aligning NxWitness’ PR pipeline behavior with the audited two-phase CI/CD pattern used in related repos.

Changes:

  • Make smoke-build depend on test-release (needs: [changes, test-release]) so smoke builds don’t run if test-release fails.
  • Add inline documentation explaining the gating rationale and the expected effect on the aggregator job behavior.

@ptr727
ptr727 merged commit f7e1ceb into develop Jun 4, 2026
8 checks passed
ptr727 added a commit that referenced this pull request Jun 8, 2026
…unbook (#422)

Promotes `develop` to `main`. Realignment with ProjectTemplate (#419):
shared markdownlint config, PR Review Etiquette + Copilot review
runbook, CRLF normalization. Also rolls up a CI back-port (#413/#415)
and a dependency bump (#420).
@ptr727
ptr727 deleted the backport-cicd-fixes branch July 7, 2026 21:06
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