Skip to content

Back-port CI/CD fixes discovered during the PlexCleaner port - #107

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

Back-port CI/CD fixes discovered during the PlexCleaner port#107
ptr727 merged 1 commit into
developfrom
backport-cicd-fixes

Conversation

@ptr727

@ptr727 ptr727 commented Jun 4, 2026

Copy link
Copy Markdown
Owner

While porting the two-phase CI/CD pattern into PlexCleaner (ptr727/PlexCleaner#723), several issues were found that also apply to ProjectTemplate (the source of truth). Applying them here so the template and its downstreams converge.

Correctness

  • Pin release-leaf builds to the resolved commit. build-nugetlibrary, build-pypilibrary, build-executable, build-docker were passed the orchestrator's inputs.ref (a branch name on publisher runs) and re-resolved their own version, so a commit landing mid-run could build artifacts from a different commit than the release tag (GitCommitId) points at. Added needs: [get-version] and pinned their ref to needs.get-version.outputs.GitCommitId.
  • github-release Apply PhotoCleaner round-2 feedback: rationale wording, MD033 clarity, CPM property owner #408/Sweep clause-splicing semicolons from the verbatim AGENTS sections #412 refinements (already in NxWitness): check out GitCommitId instead of the moving inputs.ref; allow workflow_dispatch through the skip-if-exists gate; make the skip log message event-aware ("will refresh" on dispatch vs "skipping").

Robustness / hygiene

  • build-docker cache-to ignore-error=true so a transient registry cache export can't fail an otherwise-good publish.
  • Publisher concurrency scoped to real publishes (schedule/dispatch/PUBLISH_ON_MERGE); no-op push runs get a unique per-run group so they don't queue behind or delay a publish.
  • Gate smoke-build on unit-test so the Docker image build isn't spent when unit tests are already failing.
  • Status badges filtered to event=schedule so they reflect the weekly publisher's health, not green no-op push runs.
  • Copilot runbook: fix the REST issue-comments filter to use copilot-pull-request-reviewer[bot] (REST carries the [bot] suffix; GraphQL doesn't), and document that the login form differs by API.

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

Fixes surfaced while porting the two-phase pattern to PlexCleaner
(ptr727/PlexCleaner#723), applied here at the source:

- Pin release-leaf builds (nuget/pypi/executable/docker) to the resolved
  GitCommitId with needs: [get-version], so a commit landing mid-run can't
  build artifacts from a different commit than the release tag points at.
- github-release: check out GitCommitId (not the moving inputs.ref);
  allow workflow_dispatch through the skip-if-exists gate; make the
  'skipping' vs 'will refresh' log message event-aware (the #408/#412
  refinements already in NxWitness).
- build-docker cache-to: add ignore-error=true so a transient registry
  cache export can't fail an otherwise-good publish.
- publish-release concurrency: scope the global group to real publishes
  (schedule/dispatch/PUBLISH_ON_MERGE); no-op push runs get a unique
  per-run group so they don't queue behind or delay a publish.
- Gate smoke-build on unit-test so the Docker build isn't spent when
  unit tests are already failing.
- Status badges: filter to event=schedule so they reflect the weekly
  publisher, not green no-op push runs.
- Copilot runbook: fix the REST issue-comments filter to use the
  copilot-pull-request-reviewer[bot] login, and document that the login
  form differs by API (GraphQL no-suffix vs REST [bot]).
Copilot AI review requested due to automatic review settings June 4, 2026 13:48

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 back-ports CI/CD improvements (found during the PlexCleaner two-phase pipeline port) into ProjectTemplate to keep the template’s release/publish workflows correct, robust, and aligned with downstream consumers.

Changes:

  • Pin release build jobs (NuGet/PyPI/executable/Docker + GitHub release checkout) to the immutable GitCommitId resolved by get-version, avoiding mid-run branch movement producing mismatched artifacts.
  • Improve publisher hygiene: smarter concurrency grouping for no-op push runs, gate smoke-build behind unit tests, and filter README status badges to scheduled publisher health.
  • Add workflow/runbook refinements: Docker cache export is made non-fatal, and Copilot runbook REST login filtering is corrected.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Filters workflow status shields to event=schedule to reflect weekly publisher health instead of no-op pushes.
.github/workflows/test-pull-request.yml Gates smoke-build on unit-test to avoid spending Docker builds when unit tests already fail.
.github/workflows/publish-release.yml Adjusts concurrency grouping so real publishes serialize, while no-op pushes don’t block publishes.
.github/workflows/build-release-task.yml Pins downstream build jobs and release checkout to GitCommitId; refines GitHub release “skip if exists” gating.
.github/workflows/build-docker-task.yml Makes registry cache export non-fatal via ignore-error=true to reduce transient publish failures.
.github/copilot-instructions.md Documents GraphQL vs REST bot login differences and fixes REST issue-comments filter to include [bot].

Comment thread .github/workflows/build-release-task.yml

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

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

@ptr727
ptr727 merged commit 7f327e7 into develop Jun 4, 2026
9 checks passed
ptr727 added a commit that referenced this pull request Jun 5, 2026
Release of develop → main. Carries three changes:

- **Decouple GitHub-release assembly from per-target builds (#109)** —
`github-release` collects assets by the `release-asset-<branch>-*`
convention instead of naming build jobs, so the release orchestration is
reusable verbatim by downstreams. Fixes #108.
- **Back-port CI/CD fixes discovered during the PlexCleaner port
(#107)** — commit-pinned release leaves, event-aware github-release
gating, Docker cache-export tolerance, publish concurrency scoping,
smoke-build gating, badge filtering, Copilot runbook login fix.
- **Update codegen files (#106).**

Merge-commit (not squash) per the branching model, so main records
develop's tip as the second parent.
@ptr727
ptr727 deleted the backport-cicd-fixes branch July 7, 2026 21:05
ptr727 added a commit that referenced this pull request Aug 6, 2026
`scripts/pr_review.py --repo` defaulted to `ptr727/ProjectTemplate`, so
a run that omitted it read a pull request **here** rather than the one
in front of it. The number resolved, the digest rendered, and no field
in the output disagreed. Two runs in other repositories digested a pull
request here before the maintainer caught it, one of them recovering
with "my run digested the hub's #107".

A default cannot be made safe for this argument. Every pull request
number is valid in every repository, so the wrong target is never a
lookup failure and never surfaces as one.

## Two changes, because the argument and the output each hid the miss

| Surface | Change |
| --- | --- |
| `scripts/pr_review.py` argument | `--repo` is required with no
default, and its value must be `OWNER/NAME` |
| `scripts/pr_review.py` digest | The summary line leads with
`repo=OWNER/NAME` |

A bare `ProjectTemplate` is the near-miss a required argument still
admits, and it previously raised an unpacking traceback rather than
naming which half was missing. It is now rejected as `--repo takes
OWNER/NAME, not 'ProjectTemplate'`.

The digest names the repository because a digest of the wrong pull
request is well-formed, and a number alone reads as correct anywhere.
That is what makes a misdirected run visible in its own output rather
than only to a reader who already suspects it.

## Coverage

`TestCli` runs go through a helper that supplies the repository, and
three cases were added: a run naming none is rejected, each malformed
shape is rejected by name, and the digest names what it read.

The `--pickup-grace -1` case now passes `--repo` and asserts on the
grace error. It was about to exit on the missing argument instead, which
is the same `SystemExit` and would have passed for the wrong reason.

## Carried surfaces

`GOVERNANCE.md` "PR Review Etiquette" and `scripts/README.md` update
their invocations to match. The `GOVERNANCE.md` section is carried
verbatim fleet-wide, so it reaches downstream repositories on the next
re-vendor, where the corrected invocation matters most.

## Verification

```text
python3 scripts/test_pr_review.py      70 tests, OK
python3 scripts/test_prose_lint.py    163 tests, OK
python3 scripts/test_repo_gate.py      23 tests, OK
python3 scripts/repo_gate.py           eol 0, sha-pin 0
prose_lint, both CI invocations        clean on all four changed files
```

Not addressed: `TODO.md` still records the open question of whether
`GOVERNANCE.md` may name `scripts/pr_review.py` at all, given the fleet
carries the section but not the script. This makes that pointer correct
without settling it.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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