Conversation
Resolves #414. ## Problem `publish-release.yml` built the versioned release from a **moving branch ref**: `build-main` called `build-docker-task.yml` with `ref: main`, which re-resolved its own version internally from the moving `main` ref. If a commit landed on `main` between `get-version` and `build-main` during the weekly scheduled publish, the pushed image's embedded version could come from a **newer commit** than the GitHub release of the same `SemVer2` tag — breaking release reproducibility. The sibling-repo one-line fix (`ref: ${{ needs.get-version.outputs.GitCommitId }}`) couldn't be applied directly because `build-docker-task.yml` overloaded `inputs.ref` as both (1) the git checkout/version ref and (2) the `Make/Matrix.json` `.Branch` selector. A commit SHA would match no `.Branch` rows and build nothing. ## Fix Decouple the two concerns, converging onto the canonical ProjectTemplate `branch`/`ref` split: - **`build-docker-task.yml`** — replace `smoke_branch` with a single `branch` input driving the matrix `.Branch` filter and the GHA `cache-to` scope; `ref` is now purely the immutable checkout/version ref. - **`publish-release.yml`** — `build-main` now `needs: [get-version, build-base]` and passes `branch: main` + `ref: ${{ needs.get-version.outputs.GitCommitId }}`, pinning the build to the exact versioned commit. `build-develop` passes `branch`/`ref: develop` (mutable `:develop` tag only, no versioned release → no pin needed). - **`test-pull-request.yml`** — smoke caller passes `branch` instead of `smoke_branch`. ## Verification - `actionlint` (Docker) on all three workflows: clean. - Replicated all three `jq` filter branches against the real `Make/Matrix.json`: `branch=main`/`develop` each yield 18 single-branch rows; smoke yields the 2 `NxMeta`/`NxMeta-LSIO` rows per branch (4 when `branch` empty). All assertions passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
) 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) - `github-release` checks out NBGV `GitCommitId` and allows `workflow_dispatch` through the skip-if-exists gate (#408/#412). - Docker `cache-to` already uses `ignore-error=true` (gha cache). - `publish-release.yml` has **no `push` trigger**, so the no-op-push concurrency/badge issues don't arise. - `publish-docker-readme-task` is called with an explicit `ref: main`, so it renders the README from the right branch. ## 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.
Updated [csharpier](https://github.com/belav/csharpier) from 1.2.6 to 1.3.0. <details> <summary>Release notes</summary> _Sourced from [csharpier's releases](https://github.com/belav/csharpier/releases)._ ## 1.3.0 # 1.3.0 ## Breaking Changes ### Change xml formatting to return error when it runs into syntax error so it is consistent with c# [#1854](belav/csharpier#1854) Previously CSharpier treated an invalid xml file as a warning instead of an error. This was inconsistent with how it treated c# files. Invalid c# or xml files are not treated as errors. The `--compilation-errors-as-warnings` argument has been renamed to `--syntax-errors-as-warnings` and can be used to return warnings instead of errors when encountering invalid files. ## What's Changed ### Feature: Configurable whitespace handling for xml [#1790](belav/csharpier#1790) CSharpier now supports two types of xml whitespace formatting strict or ignore. By default all xml except `xaml` or `axaml` is treated as strict whitespace. See [details](https://csharpier.com/docs/Configuration#xml-whitespace-sensitivity) ### Feature: Move closing bracket for xml elements to the same line. [#1598](belav/csharpier#1598) With strict xml whitespace handling, csharpier now keeps the closing bracket for an element on the same line instead of breaking it to a new line. ```xml <!-- input & expected output --> <ElementWithAttribute Attribute="AttributeValue__________________" >TextValue</ElementWithAttribute> <!-- 1.2.6 --> <ElementWithAttribute Attribute="AttributeValue__________________" >TextValue</ElementWithAttribute > ``` ### Feature: Support for csharpier-ignore with XML formatter [#1788](belav/csharpier#1788) CSharpier now supports `csharpier-ignore` in xml files. See [details](https://csharpier.com/docs/Ignore#xml) ### Feature: Add MSBuild transitive and multi-target support [#1833](belav/csharpier#1833) CSharpier.MSBuild can now work as a transitive dependency. ### Feature: allow checking formatting with cache [#1830](belav/csharpier#1830) The `csharpier check` command now supports a `--use-cache` option. ### Feature: remove dependency on Microsoft.AspNetCore.App [#1508](belav/csharpier#1508) Previously CSharpier required that Microsoft.AspNetCore.App be installed. CSharpier has been modified to use an HttpListener when it is run using `server` to remove the need for this dependency. ### Fix: csharpier-ignore comment removes linespaces before block [#1867](belav/csharpier#1867) CSharpier was removing blank lines before csharpier-ignore comments in some cases ```c# // input and expected output var x = 1; // csharpier-ignore var y=1; /// 1.2.6 var x = 1; // csharpier-ignore var y=1; ... (truncated) Commits viewable in [compare view](belav/csharpier@1.2.6...1.3.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Addresses #418 - realign with ptr727/ProjectTemplate. - **Shared lint config:** carry `.markdownlint-cli2.jsonc` verbatim + add the `[*.{json,jsonc}]` editorconfig glob. Docs pass clean. - **Review-loop contract:** replace the lighter `Pull Request Review Process` section in `AGENTS.md` with the template's authoritative **PR Review Etiquette**. - **Copilot runbook:** append the **GitHub Copilot Review Runbook** to `.github/copilot-instructions.md` (owner/repo adapted to `ptr727`/`NxWitness`) - the provider mechanics were previously absent. - **Line endings:** normalize both touched files to CRLF per `.editorconfig` (they had pre-existing mixed CRLF/LF). The `Develop branch`/`Main branch` rulesets were also renamed to `develop`/`main` via API (rules/enforcement/signed-commits/required-check preserved and verified).
There was a problem hiding this comment.
Pull request overview
This PR promotes develop to main while realigning the repository with the shared ProjectTemplate: adopting a shared markdownlint configuration, formalizing the PR review loop contract (with Copilot mechanics documented separately), normalizing line-ending/editorconfig behavior, and rolling forward CI workflow improvements plus a tooling dependency bump.
Changes:
- Add shared
.markdownlint-cli2.jsoncconfiguration and expand.editorconfigJSON globs to include*.jsonc. - Replace the older “Pull Request Review Process” guidance with a more explicit PR Review Etiquette contract, and add a detailed GitHub Copilot Review Runbook documenting reliable re-review mechanics.
- Update CI reusable workflow inputs (
smoke_branch→branch, decouplebranchvsref) and gate PR smoke builds ontest-release; pin main publish builds to the versioned commit SHA for reproducibility.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AGENTS.md |
Replaces the PR review section with a more explicit review-loop contract and triage/resolution expectations. |
.markdownlint-cli2.jsonc |
Introduces a shared markdownlint-cli2 configuration for consistent Markdown lint behavior. |
.github/workflows/test-pull-request.yml |
Gates smoke-build on test-release and updates the reusable workflow input name to branch. |
.github/workflows/publish-release.yml |
Pins main image builds to GitCommitId and passes branch separately from ref. |
.github/workflows/build-docker-task.yml |
Decouples matrix branch selection (branch) from immutable checkout/version ref (ref) and updates cache scoping. |
.github/copilot-instructions.md |
Adds a Copilot review runbook (how to request/poll/verify/resolve Copilot reviews via APIs). |
.editorconfig |
Expands JSON matching from *.json to *.{json,jsonc}. |
.config/dotnet-tools.json |
Bumps csharpier tool version 1.2.6 → 1.3.0. |
ptr727
added a commit
that referenced
this pull request
Jun 20, 2026
Completes the last open item of #418 (realign with `ptr727/ProjectTemplate`). Items 2–5 were already resolved by #422 and the rulesets are correct (`develop` squash-only, `main` merge-only, lowercase names); only `.github/copilot-instructions.md` still diverged. ## Change Carry the template's whole-file [`.github/copilot-instructions.md`](https://github.com/ptr727/ProjectTemplate/blob/main/.github/copilot-instructions.md) (verbatim drop-in): `# Copilot Instructions` → **Commit Messages and Pull Request Titles** → **GitHub Copilot Review Runbook** (unchanged from #422) → **When in Doubt**. Drops the NxWitness-specific preamble (Purpose, Solution Summary, CI Pipeline, standalone Versioning, etc.) — all already in [AGENTS.md](../blob/develop/AGENTS.md). ### Adaptations (per the carry contract) - Keep only the .NET `CODESTYLE.md` pointer (drop Python); drop devcontainer mentions (none here). - Fill `owner`/`repo` as `ptr727`/`NxWitness` in the runbook snippets; keep `<N>` placeholders. - Retarget AGENTS.md cross-links to this repo's sections (`#release-model` → `#versioning`; keep `#pr-review-etiquette`). The template's "Files…Must Carry Verbatim" and "Staying in Sync" links point upstream, since this repo's AGENTS.md has no equivalent sections. ## Out of scope Versioning is unchanged — it already matches the template; making it functional is a separate template-level effort. ## Notes Doc-only, no code. An upstream issue will be filed against `ptr727/ProjectTemplate` re: the carried file linking to AGENTS.md sections a derived repo only has if it also carries the template's AGENTS.md structure. Closes #418.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
developtomain. 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).