Add a diff-scoped comment-hygiene ratchet for build.ps1/test.ps1 and CI

.claude/skills/commit-messages/SKILL.md

@@ -0,0 +1,72 @@
+---
+name: commit-messages
+description: MUST use before writing any git commit message in this repository. Covers the gitlint rules CI enforces (title/body length, blank line, trailing punctuation) and how to write a compliant message on the first try.
+---
+
+# Commit Messages
+
+CI (`.github/workflows/CommitMessage.yml`) runs `gitlint` against every
+commit on a PR and fails the build on any violation. `.gitlint` in the repo
+root only exempts `Agent-Logs-Url:` and the Copilot Autofix co-author
+trailer from the body-length rule -- every other line, in every commit,
+is checked. There is no leniency for "just the summary" or "just this once."
+
+## The rules that actually fire in practice
+
+| Rule | Limit | Notes |
+|---|---|---|
+| Title length (T1) | 72 characters | Counts the whole subject line, including the `type:` prefix. |
+| Body line length (B1) | 80 characters | Per line, not per paragraph. A heredoc does NOT auto-wrap -- you must break lines yourself. |
+| Blank line after title (B4) | required | One empty line between the subject and the body. |
+| Title trailing punctuation (T3) | none | No period, no colon, at the end of the subject line. |
+| Trailing whitespace (T2, B2) | none | Watch for trailing spaces left by hand-wrapped lines. |
+| Hard tabs (B3) | none | Use spaces in the body. |
+
+## Writing a compliant message the first time
+
+- Count the title before committing to it. "docs: remove doc-pointers and
+  provenance narration from PR #964" is 63 characters; adding a scope like
+  "and cleanup notes" on top of an already-full title is how T1 fails.
+- Wrap body prose by hand at well under 80 characters per line -- a heredoc
+  passed to `git commit -F -`/`-m` reproduces exactly the line breaks you
+  typed, it does not reflow them. Aim for ~70 so a `Co-Authored-By:` trailer
+  or an indented list item added later doesn't push a line over.
+- Prefer several short lines over one long one, and several short
+  paragraphs over one dense one -- a commit message is read in a `git log`
+  pane, not a text editor with wrapping.
+
+## Verify before considering a commit done
+
+Run the same check CI runs, scoped to the current branch:
+
+```powershell
+gitlint --ignore body-is-missing --commits main..HEAD
+```
+
+(Substitute the actual base branch if not `main`.) A clean run prints
+nothing and exits 0. `Build/Agent/commit-messages.ps1` wraps this with the
+same base-ref auto-detection CI uses, if you want the base resolved for
+you instead of naming it.
+
+## Fixing a violation after the fact
+
+If a commit already landed non-compliant and hasn't been pushed to a shared
+branch, reword it without a full interactive rebase:
+
+```powershell
+git rebase <target>^ --exec 'if [ "$(git rev-parse HEAD)" = "<target-full-sha>" ]; then git commit --amend -F <message-file>; fi'
+```
+
+Git's `--exec` always runs the quoted command through its own bundled `sh`,
+even on Windows, so that inner string stays POSIX syntax regardless of
+which shell you type this from -- only the outer PowerShell single-quotes
+(passed through verbatim, unlike bash's backslash-escaped double-quotes)
+change here.
+
+This replays history non-interactively (no editor, no `-i` prompt) and
+amends only the one commit whose SHA matches, at the point in the replay
+where it is HEAD. Never do this on a branch that has already been pushed
+and could have a PR or other work based on it -- check
+`git rev-parse --abbrev-ref --symbolic-full-name @{u}` and
+`git ls-remote --heads origin <branch>` first, and confirm with the user if
+either shows the branch is shared.

.claude/skills/powershell/SKILL.md

@@ -3,7 +3,7 @@ name: powershell
 description: >
   PowerShell best practices for scripts used in FieldWorks (dev scripts & CI helpers).
   Use when writing or modifying PowerShell scripts in scripts/ or Build/Agent/.
-allowed-tools: "Read,Bash(pwsh:*)"
+allowed-tools: "Read,PowerShell"
 version: "1.0.0"
 ---
 
@@ -13,11 +13,81 @@ Conventions and safety patterns for PowerShell scripts in `scripts/` and CI.
 
 ## Style and Linting
 
-- Use `pwsh`/PowerShell Core syntax where possible and `Set-StrictMode -Version Latest`.
+- Scripts in `Build/Agent/`, and anything else reached from `build.ps1` or `test.ps1`,
+  must run under **both** Windows PowerShell 5.1 and PowerShell 7. CI executes the
+  build and test steps under 5.1, so 6+-only syntax that parses cleanly on 7 can
+  still fail or silently misbehave there: the backtick u{} escape resolves to
+  literal text under 5.1 instead of a code point, and `-Encoding utf8BOM`/`utf8NoBOM`
+  throw a parameter-binding error. Run `Build/Agent/powershell-compat.ps1` to check,
+  and prefer syntax both engines share over anything PowerShell Core adds.
+- Use `Set-StrictMode -Version Latest`.
 - Use `Write-Host` sparingly; prefer `Write-Output` and `Write-Error` for correct streams.
 - Use `-ErrorAction Stop` in helper functions when errors should abort execution.
 - **No Unicode icons or emojis** in output messages (e.g., `✓`, `✗`, `⚠`, `🔧`). Use plain ASCII text like `[OK]`, `[FAIL]`, `[WARN]`, `ERROR:` instead. Unicode causes encoding issues in CI logs.
 
+## Traps that produce a wrong answer instead of an error
+
+The first one below is the dangerous one: it yields a plausible result with no
+warning, so nothing prompts you to look. The others fail loudly, but only under
+`Set-StrictMode -Version Latest`, which this repo requires.
+
+### An operator after a bare function call binds as an argument
+
+`-replace`, `-split`, `-match`, and friends written after an unparenthesized
+function call are parsed as further *arguments* to that call, not applied to its
+result. The operator is silently ignored.
+
+```powershell
+# BAD: -replace and '\s+' become arguments 2 and 3 of Norm; nothing is replaced
+$key = Norm ($text) -replace '\s+', ''
+
+# GOOD: parenthesize the call, then apply the operator to its result
+$key = (Norm $text) -replace '\s+', ''
+```
+
+### Measure-Object -Sum over an empty collection returns $null
+
+Reading `.Sum` (or `.Maximum`, `.Average`) off that result then throws
+"The property 'Sum' cannot be found on this object" -- which surfaces far from
+the empty input that caused it.
+
+```powershell
+# BAD: throws whenever $items happens to be empty
+$total = ($items | Measure-Object -Property Length -Sum).Sum
+
+# GOOD
+$total = 0
+foreach ($item in $items) { $total += $item.Length }
+```
+
+### Returning a collection from a function unrolls it
+
+`return $list` enumerates into the pipeline: an empty collection becomes `$null`
+and a single element becomes a scalar, so the caller's `.Count` throws. Prefix
+with a comma to return the collection itself.
+
+```powershell
+# BAD: (Get-Ids).Count throws when the list is empty
+function Get-Ids { $ids = New-Object System.Collections.Generic.List[int]; return $ids }
+
+# GOOD
+function Get-Ids { $ids = New-Object System.Collections.Generic.List[int]; return ,$ids }
+```
+
+### The stop-parsing token consumes the rest of the line
+
+`--%` passes everything after it to the native command verbatim, including any
+closing bracket you meant PowerShell to read. It cannot appear inside `@(...)`,
+`$(...)`, or any other expression that has to be closed.
+
+```powershell
+# BAD: --% swallows the closing paren; parse error, not a runtime error
+$msg = @(git --% log -1 --format=%B)
+
+# GOOD: keep --% on a statement of its own, or drop it when it is not needed
+$msg = @(git log -1 --pretty=%B)
+```
+
 ## Security
 
 - Avoid embedding secrets in scripts; read from env vars and prefer platform secret stores.

.claude/skills/pr-pitch/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: pr-pitch
-description: Compose a PR body as a pitch that answers the unknowns a reviewer arrives with, with the branch's decisions, provenance, and paths-not-taken folded into collapsed accordions below it, while evicting those files from the repo. Use when writing or refreshing a PR description, when a branch carries working markdown that should not merge, or when pr-preflight reaches its PR step.
+description: "NOT an entrypoint -- pr-preflight calls this for the write-up step; use pr-preflight for a fresh 'write/make/open a PR' request. Invoke this directly only to redo the write-up on a PR that already exists. Composes a PR body as a pitch that answers the unknowns a reviewer arrives with, with the branch's decisions, provenance, and paths-not-taken folded into collapsed accordions below it, while evicting those files from the repo."
 argument-hint: "Optional PR number (defaults to the PR for the current branch)"
 ---
 

.claude/skills/pr-preflight/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: pr-preflight
-description: "Use when preparing a FieldWorks branch or pull request for review: pre-PR review, branch readiness, author interview, review summary generation, validation evidence, or PR description preparation."
+description: "The required entrypoint whenever asked to write, make, open, create, update, or ship a PR for this repo -- do not post a PR body without running this first. Also use for pre-PR review, branch readiness, author interview, review summary generation, or validation evidence."
 argument-hint: "Optional branch purpose or PR goal"
 user-invocable: true
 ---
@@ -236,17 +236,18 @@ After writing the summary, tell the author:
 >
 > Please review it, make changes where appropriate, and run `/pr-preflight` again until you are ready to post the PR.
 >
-> If you do not want to make any changes and are ready for review, would you like me to commit any uncommitted changes, push, and post the PR? I will check whether one already exists for this branch and update it, or create a new one if not. The write-up runs through `pr-pitch`, which will also triage any research or working markdown on the branch into collapsed PR comments and out of the tree -- you approve that triage before anything is deleted."
+> If you do not want to make any changes and are ready for review, would you like me to commit any uncommitted changes, push, and post the PR? I will check whether one already exists for this branch and update it, or create a new one if not. The write-up runs through `pr-pitch`, which will also triage any research or working markdown on the branch into collapsed sections in the PR body and out of the tree -- you approve that triage before anything is deleted."
 
 Only create or update a PR after the author confirms.
 
 ## PR Description
 
 This skill is the single entrypoint for making a PR, but it does not compose
 the description itself. Once the author confirms readiness, invoke the
-`pr-pitch` skill and let it own the write-up. It produces three artifacts
-together: the PR body as a pitch, provenance in collapsed PR comments, and a
-commit evicting the branch's research and working markdown from the tree.
+`pr-pitch` skill and let it own the write-up. It produces two artifacts
+together: the PR body (a pitch above the fold, provenance in collapsed
+accordions below it) and a commit evicting the branch's research and working
+markdown from the tree.
 
 Hand `pr-pitch` the branch purpose, the findings, and `.review/summary.md`.
 

.github/instructions/build.instructions.md

@@ -13,14 +13,19 @@ FieldWorks is **Windows-first** and **x64-only**. Use the repo scripts so build
 ## Quick start (PowerShell)
 ```powershell
 # Full traversal build (Debug/x64 defaults)
-.\build.ps1
+.\build.ps1 -CommentHygiene
 
 # Release build
-.\build.ps1 -Configuration Release
+.\build.ps1 -CommentHygiene -Configuration Release
 ```
 
 ## Non-negotiable rules
 - Use `.\build.ps1` for builds and `.\test.ps1` for tests.
+- Pass `-CommentHygiene` on every build and test run. It fails the run on any
+	comment-hygiene violation in the lines your branch adds, so you fix your own
+	comments before review. Humans omit it and never see the gate; CI reports
+	violations as warning annotations without failing. Never drop the flag to get
+	a run to pass.
 - Avoid ad-hoc `msbuild`/`dotnet build` invocations unless you are explicitly debugging build infrastructure.
 - Do not change COM/registry behavior without an explicit plan and tests.
 

.github/instructions/dotnet-upgrade.instructions.md

@@ -48,11 +48,11 @@ To identify dependencies:
 - Use the following approaches:
   - **Visual Studio** → `Dependencies` in Solution Explorer.  
   - **dotnet CLI** → run:
-    ```bash
+    ```powershell
     dotnet list <ProjectName>.csproj reference
     ```
   - **Dependency Graph Generator**:
-    ```bash
+    ```powershell
     dotnet msbuild <SolutionName>.sln /t:GenerateRestoreGraphFile /p:RestoreGraphOutputPath=graph.json
     ```
     Inspect `graph.json` to see the dependency order.
@@ -79,16 +79,16 @@ For each project:
    - `TargetFramework` → Change to the desired version (e.g., `net8.0`).
    - `PackageReference` → Verify if each NuGet package supports the new framework.  
      - Run:
-       ```bash
+       ```powershell
        dotnet list package --outdated
        ```
        Update packages:
-       ```bash
+       ```powershell
        dotnet add package <PackageName> --version <LatestVersion>
        ```
 
 3. If `packages.config` is used (legacy), migrate to `PackageReference`:
-   ```bash
+   ```powershell
    dotnet migrate <ProjectPath>
    ```
 
@@ -131,11 +131,11 @@ BlobServiceClient client = new BlobServiceClient(connectionString);
 2. Update NuGet packages to versions compatible with the target framework.
 3. After upgrading and restoring the latest DLLs, review code for any required changes.
 4. Rebuild the project:
-   ```bash
+   ```powershell
    dotnet build <ProjectName>.csproj
    ```
 5. Run unit tests if any:
-   ```bash
+   ```powershell
    dotnet test
    ```
 6. Fix build or runtime issues before proceeding.
@@ -168,7 +168,7 @@ After all projects are upgraded:
 
 ## 7. Tools & Automation
 - **.NET Upgrade Assistant**(Optional):
-  ```bash
+  ```powershell
   dotnet tool install -g upgrade-assistant
   upgrade-assistant upgrade <SolutionName>.sln```
 

.github/instructions/testing.instructions.md

@@ -15,20 +15,22 @@ Guidance for writing and running deterministic unit and integration tests for Fi
 
 ## Running Tests (Managed)
 
-Use `.\test.ps1` for all managed (C#) tests.
+Use `.\test.ps1` for all managed (C#) tests. Always pass `-CommentHygiene`: it
+fails the run on any comment-hygiene violation in the lines your branch adds.
+Humans omit it and never see the gate; CI annotates without failing.
 
 ```powershell
 # Run all tests (builds first)
-.\test.ps1
+.\test.ps1 -CommentHygiene
 
 # Run specific project
-.\test.ps1 -TestProject "Src/Common/FwUtils/FwUtilsTests"
+.\test.ps1 -CommentHygiene -TestProject "Src/Common/FwUtils/FwUtilsTests"
 
 # Run with filter
-.\test.ps1 -TestFilter "TestCategory!=Slow"
+.\test.ps1 -CommentHygiene -TestFilter "TestCategory!=Slow"
 
 # Run without rebuilding (faster iteration)
-.\test.ps1 -NoBuild -TestProject "FwUtilsTests"
+.\test.ps1 -CommentHygiene -NoBuild -TestProject "FwUtilsTests"
 ```
 
 ## Running Tests (Native C++)

.github/pull_request_template.md

@@ -10,7 +10,7 @@
   git log --check --pretty=format:"---% h% s" origin/<base>..
   git diff --check --cached
   ```
-- [ ] Builds/tests pass locally (or I've run the CI-style build via Bash script or MSBuild).
+- [ ] Builds/tests pass locally (or I've run the CI-style build via `build.ps1`/`test.ps1` or MSBuild).
 - [ ] If this is core-developer AI-assisted work, I followed `Docs/workflows/ai-pr-workflow.md` and ran `pr-preflight` or the equivalent branch-readiness review before requesting review.
 - [ ] For any `Src/**` folders touched, corresponding `AGENTS.md` files are updated or explicitly confirmed still accurate.
 

.github/workflows/CI.yml

@@ -23,6 +23,32 @@ jobs:
       - name: Checkout Files
         uses: actions/checkout@v7
         id: checkout
+        with:
+          # comment-hygiene.ps1 diffs against origin/<default-branch>; a
+          # shallow, single-branch checkout leaves that ref unresolvable
+          # and the gate fails every build with "bad revision".
+          fetch-depth: 0
+
+      # Real-runtime check: this repo is authored on PowerShell 7 but
+      # build.ps1/test.ps1 run under Windows PowerShell 5.1 below. A script
+      # can parse identically on both and still resolve differently at
+      # runtime (see comment-hygiene's ASCII-replacement map, which used to
+      # do exactly that). windows-2022 ships both engines, so run the
+      # comment-hygiene fixture suite under each rather than assuming one
+      # implies the other.
+      - name: Comment hygiene fixture tests (PowerShell 7)
+        id: comment-hygiene-tests-pwsh
+        shell: pwsh
+        run: |
+          Build/Agent/CommentHygiene.Tests.ps1
+          if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
+
+      - name: Comment hygiene fixture tests (Windows PowerShell 5.1)
+        id: comment-hygiene-tests-winps
+        shell: powershell
+        run: |
+          Build\Agent\CommentHygiene.Tests.ps1
+          if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
 
       - name: Build with tests
         id: build

.github/workflows/CommitMessage.yml

@@ -13,26 +13,33 @@ jobs:
       with:
         fetch-depth: 0
     - name: Install dependencies
-      run: |
-        pip install --upgrade gitlint
+      shell: pwsh
+      run: pip install --upgrade gitlint
     - name: Lint git commit messages
-      shell: bash
+      shell: pwsh
       # run the linter and tee the output to a file, this will make the check fail but allow us to use the results in summary
-      run: gitlint --ignore body-is-missing --commits origin/$GITHUB_BASE_REF..  2>&1 | tee check_results.log
+      run: |
+        # Pre-create the file: Tee-Object never creates (or even truncates) its target when the
+        # piped command emits zero objects, which is exactly what a clean gitlint run does.
+        New-Item -ItemType File -Path check_results.log -Force | Out-Null
+        gitlint --ignore body-is-missing --commits "origin/$env:GITHUB_BASE_REF.." 2>&1 | Tee-Object -FilePath check_results.log
+        exit $LASTEXITCODE
     - name: Propegate Error Summary
       if: always()
-      shell: bash
+      shell: pwsh
       # put the output of the commit message linting into the summary for the job and in an environment variable
       run: |
         # Change the commit part of the log into a markdown link to the commit
-        commitsUrl="https:\/\/github.com\/${{ github.repository_owner }}\/${{ github.event.repository.name }}\/commit\/"
-        sed -i "s/Commit \([0-9a-f]\{7,40\}\)/[commit \1]($commitsUrl\1)/g" check_results.log
+        $commitsUrl = "https://github.com/${{ github.repository_owner }}/${{ github.event.repository.name }}/commit/"
+        $replacement = '[commit $1](' + $commitsUrl + '$1)'
+        $log = (Get-Content check_results.log -Raw) -replace 'Commit ([0-9a-f]{7,40})', $replacement
+        Set-Content -Path check_results.log -Value $log -NoNewline
         # Put the results into the job summary
-        cat check_results.log >> "$GITHUB_STEP_SUMMARY"
+        Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value $log
         # Put the results into a multi-line environment variable to use in the next step
-        echo "check_results<<###LINT_DELIMITER###" >> "$GITHUB_ENV"
-        echo "$(cat check_results.log)" >> "$GITHUB_ENV"
-        echo "###LINT_DELIMITER###" >> "$GITHUB_ENV"
+        Add-Content -Path $env:GITHUB_ENV -Value 'check_results<<###LINT_DELIMITER###'
+        Add-Content -Path $env:GITHUB_ENV -Value $log
+        Add-Content -Path $env:GITHUB_ENV -Value '###LINT_DELIMITER###'
     # add a comment on the PR if the commit message linting failed
     - name: Comment on PR
       if: failure()