Adopt shared markdownlint config, review-loop contract, and Copilot runbook

.config/dotnet-tools.json

@@ -3,7 +3,7 @@
   "isRoot": true,
   "tools": {
     "csharpier": {
-      "version": "1.2.6",
+      "version": "1.3.0",
       "commands": [
         "csharpier"
       ],

.editorconfig

@@ -37,7 +37,7 @@ end_of_line = crlf
 indent_size = 2
 
 # Json files
-[*.json]
+[*.{json,jsonc}]
 end_of_line = crlf
 
 # Linux scripts

.github/copilot-instructions.md

@@ -56,3 +56,126 @@ This repository builds and publishes Docker images for Network Optix VMS product
 - Follow the zero-warnings policy and formatting requirements in [CODESTYLE.md](../CODESTYLE.md).
 - Use explicit types (no `var`), Allman braces, file-scoped namespaces, and other conventions as defined in the master style guide.
 - Respect line endings and encoding rules from the repository configuration, including UTF-8 without BOM.
+
+## GitHub Copilot Review Runbook
+
+Use this section for provider-specific mechanics. The expected review loop *contract* (request review on every push, verify head-SHA coverage, triage findings, reply + resolve, escalate when stuck) is defined in [AGENTS.md -> PR Review Etiquette](../AGENTS.md#pr-review-etiquette). This section only describes how to make GitHub Copilot reliably execute it.
+
+### Triggering and Polling
+
+Auto-review on push is configured (via the branch ruleset's `copilot_code_review` rule with `review_on_push: true`) but fires inconsistently in practice - treat it as best-effort, not guaranteed. After every push, **re-request a review programmatically** via the GraphQL `requestReviews` mutation, passing the Copilot reviewer's bot node id in `botIds`. This now works reliably (it previously did not - a maintainer had to click "re-request review" in the UI; the agent can now drive the loop end-to-end without that hand-off).
+
+> **The reviewer login differs by API - this is intentional, not a typo.** In **GraphQL** (`gh api graphql` and `gh pr view --json reviews`, which is GraphQL-backed) the `Bot.login` is `copilot-pull-request-reviewer` - **no `[bot]` suffix**. In the **REST** API (`gh api repos/.../issues|pulls/...`) the same account's `user.login` is `copilot-pull-request-reviewer[bot]` - **with** the suffix. Each query below uses the correct form for its API; match the API, not a single spelling, when adapting them.
+
+```sh
+# 1. PR node id + the Copilot reviewer's bot node id (read from any existing
+#    Copilot review; the reviewer login is `copilot-pull-request-reviewer`).
+PR_NODE=$(gh pr view <N> --json id --jq '.id')
+BOT_ID=$(gh api graphql -f query='
+{
+  repository(owner: "ptr727", name: "NxWitness") {
+    pullRequest(number: <N>) {
+      reviews(first: 50) { nodes { author { __typename login ... on Bot { id } } } }
+    }
+  }
+}' --jq '[.data.repository.pullRequest.reviews.nodes[]
+          | select(.author.login == "copilot-pull-request-reviewer")
+          | .author.id] | first')
+
+# 2. Re-request a Copilot review on the current head.
+gh api graphql -f query='
+mutation($pr: ID!, $bot: ID!) {
+  requestReviews(input: { pullRequestId: $pr, botIds: [$bot], union: true }) {
+    pullRequest { id }
+  }
+}' -F pr="$PR_NODE" -F bot="$BOT_ID"
+```
+
+The bot node id is read from an existing Copilot review, so step 1 needs at least one prior review on the PR - the auto-review-on-open normally supplies the first one. If no Copilot review exists yet and auto-review didn't fire, request `Copilot` once through the GitHub PR UI to seed it, then use the mutation for every subsequent re-request.
+
+**Do NOT post `@Copilot review` as a PR comment.** That comment triggers the Copilot *coding agent* (`copilot-swe-agent[bot]`), which makes code changes rather than posting a review.
+
+Known non-working request paths (don't rely on them - use the `requestReviews` mutation above instead):
+
+- `POST /requested_reviewers` with `reviewers=[Copilot]` can return 200 but no-op.
+- `copilot-pull-request-reviewer` as a requested reviewer slug returns 422.
+
+### Verify Review Covered Current Head
+
+Before merging, confirm Copilot reviewed the current PR head SHA. Copilot may respond as either a formal review (carries an exact commit SHA) or an issue comment (no SHA - use the most recent Copilot comment for manual confirmation). Check both.
+
+```sh
+PR_HEAD=$(gh pr view <N> --json headRefOid --jq '.headRefOid')
+
+# 1. Formal review - exact SHA match.
+gh pr view <N> --json reviews --jq \
+  '.reviews[] | select(.author.login=="copilot-pull-request-reviewer") | .commit.oid' \
+  | grep -q "$PR_HEAD" && echo "covered via formal review"
+
+# 2. Issue comment - show the most recent Copilot comment for manual
+#    confirmation. This is the REST API, so the login carries the `[bot]` suffix.
+gh api repos/ptr727/NxWitness/issues/<N>/comments --jq \
+  '[.[] | select(.user.login=="copilot-pull-request-reviewer[bot]")] | last | {created_at, body: .body[:200]}'
+```
+
+Coverage is confirmed when (1) exits 0. For issue comments (path 2), body content is the only reliable signal - `created_at` is not: `git log -1 --format=%cI` is the **commit** timestamp, not the push timestamp, so amended or rebased commits can have an earlier timestamp and an older Copilot comment could satisfy a time check even though Copilot never saw the current head. Treat path (2) as confirmed only when the comment body explicitly refers to the current changes.
+
+### Bounded Retry Workflow
+
+If a review did not run on the current head, retry:
+
+1. Wait briefly and check head-SHA coverage (see above).
+1. Re-request the review via the `requestReviews` mutation (see "Triggering and Polling"); fall back to the GitHub PR UI only if the mutation no-ops.
+1. Retry up to two more times (three total).
+1. If still missing, mark review as blocked and escalate to the user/maintainer with what was attempted.
+
+### Reply and Thread Resolution Workflow
+
+List unresolved threads. Use `first: 100` with cursor-based pagination; if `hasNextPage` is true, re-run with `after: "<endCursor>"` to retrieve the next page:
+
+```sh
+gh api graphql -f query='
+{
+  repository(owner: "ptr727", name: "NxWitness") {
+    pullRequest(number: <N>) {
+      reviewThreads(first: 100) {
+        nodes {
+          id isResolved path
+          comments(first: 1) { nodes { author { login } body } }
+        }
+        pageInfo { hasNextPage endCursor }
+      }
+    }
+  }
+}' | jq '
+  .data.repository.pullRequest.reviewThreads |
+  (.pageInfo | "hasNextPage=\(.hasNextPage) endCursor=\(.endCursor)"),
+  (.nodes[] | select(.isResolved == false))
+'
+```
+
+Reply on a thread, then resolve it:
+
+```sh
+gh api graphql -f query='
+mutation($threadId: ID!, $body: String!) {
+  addPullRequestReviewThreadReply(input: { pullRequestReviewThreadId: $threadId, body: $body }) {
+    comment { id }
+  }
+}' -F threadId="PRRT_..." -F body="Fixed in <SHA>: <one-line summary>."
+
+gh api graphql -f query='
+mutation($threadId: ID!) {
+  resolveReviewThread(input: { threadId: $threadId }) { thread { id isResolved } }
+}' -F threadId="PRRT_..."
+```
+
+Issue-level Copilot comments (those in `issues/<N>/comments`) have no resolution action - GitHub provides no API or UI to resolve them. Reply if the finding warrants it; no resolution step is needed or possible.
+
+Reply-body conventions:
+
+- Accepted bug/style fix: include fixing commit SHA and a one-line summary.
+- Declined style comment: cite the rule (AGENTS.md or language CODESTYLE) and the existing-tree precedent.
+- Declined architecture proposal: one-sentence rationale.
+
+After the final push, sweep-resolve stale older threads for removed code paths.

.github/workflows/build-docker-task.yml

@@ -15,17 +15,22 @@ on:
         required: false
         type: boolean
         default: false
-      # In smoke mode, restrict the build to this branch's rows (the PR
-      # base branch). Empty builds both branches' rows. Lets a PR onto
-      # develop smoke-test the develop images and a PR onto main the main
-      # images, without building the other branch.
-      smoke_branch:
+      # Logical branch whose Matrix.json rows to build/push and whose GHA
+      # cache scope to use. Decoupled from `ref` so `ref` can be pinned to an
+      # immutable commit (e.g. the publisher pins main to the versioned SHA)
+      # while this still selects the right branch's rows. Empty in smoke mode
+      # builds both branches' rows; the publisher passes main and develop so
+      # both branches' tags are produced from one scheduled run. Empty in a
+      # non-smoke build falls back to github.ref_name for the cache scope.
+      branch:
         required: false
         type: string
         default: ''
-      # Branch to check out and whose images to build/push. Empty uses the
-      # triggering ref (PR context). The publisher passes main and develop
-      # so both branches' tags are produced from one scheduled run.
+      # Immutable git ref to check out / version, decoupled from branch/tag
+      # selection (see `branch`). Empty uses the triggering ref (PR context).
+      # The publisher pins this to the exact versioned commit so the image's
+      # embedded version matches the GitHub release tag even if the branch
+      # advances mid-run.
       ref:
         required: false
         type: string
@@ -57,27 +62,26 @@ jobs:
         id: getmatrix
         env:
           SMOKE: ${{ inputs.smoke }}
-          SMOKE_BRANCH: ${{ inputs.smoke_branch }}
-          REF: ${{ inputs.ref }}
+          BRANCH: ${{ inputs.branch }}
         run: |
-          # $ref and $sb below are jq variables (passed via --arg), not shell
-          # variables, so they must stay single-quoted / unexpanded.
+          # $b below is a jq variable (passed via --arg), not a shell
+          # variable, so it must stay single-quoted / unexpanded.
           # shellcheck disable=SC2016
           if [[ "$SMOKE" == "true" ]]; then
             # One Ubuntu + one LSIO variant exercises the shared Dockerfile
             # build logic and the branch's build args; tags are irrelevant
-            # since smoke never pushes. Restrict to the PR base branch ($sb)
+            # since smoke never pushes. Restrict to the PR base branch ($b)
             # so a PR onto develop validates the develop rows and a PR onto
-            # main validates the main rows; empty $sb builds both branches.
-            FILTER='.Images |= (map(select((.Name == "NxMeta" or .Name == "NxMeta-LSIO") and ($sb == "" or .Branch == $sb))) | unique_by([.Name, .Branch]))'
-          elif [[ -n "$REF" ]]; then
+            # main validates the main rows; empty $b builds both branches.
+            FILTER='.Images |= (map(select((.Name == "NxMeta" or .Name == "NxMeta-LSIO") and ($b == "" or .Branch == $b))) | unique_by([.Name, .Branch]))'
+          elif [[ -n "$BRANCH" ]]; then
             # Publish: build only the rows targeting the branch being built
             # (avoids building the other branch's rows just to discard them).
-            FILTER='.Images |= map(select(.Branch == $ref))'
+            FILTER='.Images |= map(select(.Branch == $b))'
           else
             FILTER='.'
           fi
-          echo "matrix=$(jq --arg ref "$REF" --arg sb "$SMOKE_BRANCH" --compact-output "$FILTER" ./Make/Matrix.json)" >> "$GITHUB_OUTPUT"
+          echo "matrix=$(jq --arg b "$BRANCH" --compact-output "$FILTER" ./Make/Matrix.json)" >> "$GITHUB_OUTPUT"
 
   get-version:
     name: Get version information job
@@ -172,5 +176,5 @@ jobs:
             type=gha,scope=main-${{ matrix.images.Name }}
             ${{ github.event.pull_request && format('type=gha,scope=pr-{0}-{1}', github.event.pull_request.number, matrix.images.Name) || '' }}
           cache-to: |
-            ${{ inputs.push && format('type=gha,mode=min,scope={0}-{1},ignore-error=true', inputs.ref != '' && inputs.ref || github.ref_name, matrix.images.Name) || '' }}
+            ${{ inputs.push && format('type=gha,mode=min,scope={0}-{1},ignore-error=true', inputs.branch != '' && inputs.branch || github.ref_name, matrix.images.Name) || '' }}
             ${{ github.event.pull_request && !github.event.pull_request.head.repo.fork && format('type=gha,mode=min,scope=pr-{0}-{1},ignore-error=true', github.event.pull_request.number, matrix.images.Name) || '' }}

.github/workflows/publish-release.yml

@@ -47,12 +47,17 @@ jobs:
 
   build-main:
     name: Build main images job
-    needs: [build-base]
+    needs: [get-version, build-base]
     uses: ./.github/workflows/build-docker-task.yml
     secrets: inherit
     with:
       push: true
-      ref: main
+      branch: main
+      # Pin to the exact commit get-version computed the release version from,
+      # not the moving `main` ref: this closes the race where a commit landing
+      # on main mid-run could make the pushed image's embedded version come
+      # from a newer commit than the GitHub release of the same SemVer2 tag.
+      ref: ${{ needs.get-version.outputs.GitCommitId }}
       build_base: false
 
   build-develop:
@@ -62,6 +67,10 @@ jobs:
     secrets: inherit
     with:
       push: true
+      # Develop only publishes the mutable :develop tag (no versioned GitHub
+      # release), so the moving ref can't cause a tag/release mismatch; no
+      # commit pin needed.
+      branch: develop
       ref: develop
       build_base: false
 

.github/workflows/test-pull-request.yml

@@ -53,23 +53,28 @@ jobs:
   # scheduled publish-release.yml, so a smoke pass is fast PR feedback,
   # not a publish gate.
   #
-  # smoke_branch targets the PR base branch so a PR onto develop validates
-  # the develop image rows and a PR onto main validates the main rows.
+  # branch targets the PR base branch so a PR onto develop validates the
+  # develop image rows and a PR onto main validates the main rows. ref is
+  # left unset so checkout uses the PR ref being tested.
   #
   # build_base is only enabled when a base Dockerfile changed: the product
   # smoke build pulls the published base from Docker Hub, so building the
   # base otherwise is wasted work. When a base Dockerfile does change, build
   # it to validate it still compiles (e.g. a Dependabot ubuntu:noble bump).
   smoke-build:
     name: Smoke build job
-    needs: [changes]
+    # Also gate on test-release: the smoke build builds Docker images, so don't
+    # spend it when the test job is already failing. A failed test-release
+    # leaves this job skipped (needs unsatisfied) and the aggregator blocks on
+    # the test-release failure directly.
+    needs: [changes, test-release]
     if: ${{ needs.changes.outputs.image == 'true' }}
     uses: ./.github/workflows/build-docker-task.yml
     secrets: inherit
     with:
       push: false
       smoke: true
-      smoke_branch: ${{ github.base_ref }}
+      branch: ${{ github.base_ref }}
       build_base: ${{ needs.changes.outputs.base == 'true' }}
 
   # TODO: Workaround for GitHub Actions not supporting status checks on conditional jobs

.markdownlint-cli2.jsonc

@@ -0,0 +1,14 @@
+{
+    "config": {
+        // Prose paragraphs and data-heavy tables/URLs are intentionally long;
+        // reflowing at 80 cols hurts readability and churns diffs.
+        "MD013": false,
+        // Inline HTML is used for reference-link section dividers.
+        "MD033": false,
+        // Require fenced code blocks over the legacy 4-space-indented style.
+        "MD046": { "style": "fenced" },
+        // Wide tables are intentional where wrapping cells breaks GitHub rendering.
+        "MD060": false
+    },
+    "gitignore": true
+}