build(solution): Consume ploch-common as released packages

.github/workflows/build-dotnet.yml

@@ -40,17 +40,24 @@ jobs:
         with:
           fetch-depth: 0
 
-      # The solution references sibling repositories by relative path (../ploch-common and
-      # ../mrploch-development) - the workspace layout where every mrploch repo is cloned next
-      # to the others. Reproduce that layout beside the workspace, otherwise restore fails with
-      # MSB3202 for every ../ploch-common project. Cloning the siblings (rather than checking
-      # this repository out into a named sub-directory) keeps this repository at the workspace
-      # root, which is what the Codacy reporter and SonarScanner both expect.
+      # Reproduce the workspace layout beside the checkout: every mrploch repo cloned next to
+      # the others. Cloning the siblings (rather than checking this repository out into a named
+      # sub-directory) keeps this repository at the workspace root, which is what the Codacy
+      # reporter and SonarScanner both expect.
+      #
+      # mrploch-development is required unconditionally: Directory.Packages.props imports the
+      # shared version files from it.
+      #
+      # ploch-common is required by exactly ONE step - "Build sample application", which runs
+      # -p:UsePlochProjectReferences=true so a library change cannot break the sample silently.
+      # The main solution restore no longer needs it: in the default package mode the Ploch
+      # dependencies come from nuget.org, so the old MSB3202-on-every-ploch-common-project
+      # rationale no longer applies. Whether this clone should go away entirely is #51.
       #
       # ploch-common is cloned in full because it uses Nerdbank.GitVersioning too, and NBGV
       # cannot compute a version height from a shallow clone.
       - name: Clone ploch-common (referenced projects)
-        run: git clone https://github.com/mrploch/ploch-common.git --branch master ../ploch-common
+        run: git clone https://github.com/mrploch/ploch-common.git --branch main ../ploch-common
       - name: Clone mrploch-development (shared build config)
         run: git clone --depth 1 https://github.com/mrploch/mrploch-development.git ../mrploch-development
 
@@ -179,8 +186,25 @@ jobs:
       # between begin and end, and the sample is deliberately excluded from analysis (it also
       # already sits in sonar.coverage.exclusions). Not continue-on-error - a broken sample is a
       # broken build.
+      #
+      # -p:GeneratePackageOnBuild=false is defence in depth, not a fix for a live bug. In
+      # UsePlochProjectReferences mode this step pulls the four library projects in as
+      # ProjectReferences, and they set GeneratePackageOnBuild=true, so in principle they could
+      # pack a second time - over the package-mode nupkgs this workflow later publishes - with
+      # ploch-common resolved as a ProjectReference and therefore a PRERELEASE dependency
+      # version. That is exactly the #47 defect this branch exists to remove.
+      #
+      # It does not currently happen: measured on a fully clean tree, this step packs the
+      # libraries into bin/Debug, while publish-nuget-packages.sh globs */bin/Release/*.nupkg,
+      # so the published artefacts are untouched. But the only thing preventing it is an
+      # incidental configuration-mapping quirk - the library projects are not members of the
+      # sample solution, so they do not inherit its Release mapping. Adding them to that
+      # solution would silently arm the bug. Suppressing the pack removes the class outright.
+      #
+      # Raised by the Copilot (Grok 4.6) reviewer on PR #52 as a live defect; investigated and
+      # refuted independently by the Codex reviewer; refutation confirmed by measurement.
       - name: Build sample application
-        run: dotnet build ./samples/SampleApp/Ploch.CommandLine.Spectre.SampleApp.slnx -c Release -p:UsePlochProjectReferences=true
+        run: dotnet build ./samples/SampleApp/Ploch.CommandLine.Spectre.SampleApp.slnx -c Release -p:UsePlochProjectReferences=true -p:GeneratePackageOnBuild=false
 
       # The main ruleset has a code_coverage rule, which reads coverage GitHub itself holds -
       # SonarCloud's and Codacy's copies are invisible to it. actions/upload-code-coverage takes a

.github/workflows/publish-docs.yml

@@ -26,9 +26,12 @@ jobs:
       url: ${{ steps.deployment.outputs.page_url }}
     runs-on: ubuntu-latest
     steps:
-      # The solution references sibling repositories by relative path - the workspace layout where
-      # every mrploch repo is cloned next to the others. Reproduce that layout here, otherwise the
-      # docfx metadata pass fails with MSB3202 for every ../ploch-common project.
+      # Reproduce the workspace layout here - every mrploch repo cloned next to the others.
+      # mrploch-development is required: Directory.Packages.props imports the shared version
+      # files from it. The ploch-common clone below is now belt-and-braces: in the default
+      # package mode the docfx metadata pass resolves Ploch dependencies from nuget.org, so the
+      # old MSB3202-on-every-ploch-common-project rationale no longer applies. Whether it can
+      # be dropped is #51.
       - name: Checkout
         uses: actions/checkout@v4
         with:
@@ -39,7 +42,7 @@ jobs:
         uses: actions/checkout@v4
         with:
           repository: mrploch/ploch-common
-          ref: master
+          ref: main
           path: ploch-common
           fetch-depth: 0
 
@@ -65,6 +68,13 @@ jobs:
 
       - name: Build documentation site
         working-directory: ploch-commandline
+        # Scoped to this step, not the job, so the package credential is not exposed to
+        # checkout, upload or deployment actions (least privilege, raised by the CodeRabbit
+        # reviewer on PR #52). nuget.config maps Ploch.* to both nuget.org and the
+        # authenticated GitHub Packages feed; without the token that feed 401s on every Ploch
+        # package and the nuget.org fallback is a race. Full reasoning in release.yml.
+        env:
+          GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }}
         run: dotnet docfx DocumentationSite/docfx.json
 
       - name: Upload artifact

.github/workflows/qodana_code_quality.yml

@@ -26,7 +26,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           repository: mrploch/ploch-common
-          ref: master
+          ref: main
           path: ploch-common
           fetch-depth: 0
       - uses: actions/checkout@v4
@@ -63,5 +63,9 @@ jobs:
         with:
           pr-mode: false
         env:
+          # Scoped to this step, not the job (least privilege, raised by the CodeRabbit reviewer
+          # on PR #52). The scan restores the solution inside its container, which resolves
+          # Ploch.* from the authenticated GitHub Packages feed. Full reasoning in release.yml.
+          GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }}
           QODANA_TOKEN: ${{ secrets.QODANA_TOKEN_657107159 }}
           QODANA_ENDPOINT: 'https://qodana.cloud'

.github/workflows/release.yml

@@ -27,8 +27,9 @@ jobs:
     env:
       RELEASE_VERSION: ${{ inputs.release_version }}
       NEXT_VERSION_INPUT: ${{ inputs.next_version }}
-    # Every `run` step operates inside the checked-out repository. Sibling repositories are
-    # checked out next to it, reproducing the workspace layout the solution expects.
+    # Every `run` step operates inside the checked-out repository. mrploch-development - the
+    # only sibling this workflow still needs - is checked out next to it, reproducing the
+    # workspace layout Directory.Packages.props expects.
     defaults:
       run:
         working-directory: ploch-commandline
@@ -68,6 +69,32 @@ jobs:
           fi
           echo "GH_TOKEN is valid"
 
+      # Validate GH_PACKAGES_TOKEN too. Assigning it is not the same as having it: an unset or
+      # expired secret still leaves nuget.config sending an empty password to the GitHub
+      # Packages feed, which 401s and puts the restore back in the NU1301 race described on the
+      # job env above - the exact failure the token was added to remove. Fail here, with a
+      # readable message, rather than in the middle of a restore.
+      # Raised by the Copilot (Grok 4.6) reviewer on PR #52.
+      - name: Validate GH_PACKAGES_TOKEN secret
+        working-directory: .
+        env:
+          GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }}
+        run: |
+          if [ -z "$GH_PACKAGES_TOKEN" ]; then
+            echo "::error::GH_PACKAGES_TOKEN secret is not set. A token with 'read:packages' scope is required so the GitHub Packages feed mapped for Ploch.* in nuget.config can be authenticated."
+            exit 1
+          fi
+          # A presence check is not enough: an expired token, or one without read:packages,
+          # is non-empty and still 401s during restore - which is the failure this step
+          # exists to pre-empt. So actually authenticate against the feed nuget.config uses.
+          # Basic auth with the token as the password is how NuGet presents it.
+          HTTP_STATUS=$(curl -s -o /dev/null -w "%{http_code}"             -u "x-access-token:$GH_PACKAGES_TOKEN"             https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json)
+          if [ "$HTTP_STATUS" != "200" ]; then
+            echo "::error::GH_PACKAGES_TOKEN is invalid, expired, or lacks 'read:packages' (HTTP $HTTP_STATUS from the GitHub Packages NuGet index). Regenerate the token and update the repository secret."
+            exit 1
+          fi
+          echo "GH_PACKAGES_TOKEN authenticates against GitHub Packages"
+
       # Use the default GITHUB_TOKEN for checkout - it always works and has read access.
       # GH_TOKEN (PAT) is configured separately before push steps, because it is the
       # only token that can trigger subsequent workflows when pushing commits.
@@ -77,13 +104,12 @@ jobs:
           path: ploch-commandline
           fetch-depth: 0
 
-      - name: Checkout ploch-common (sibling project references)
-        uses: actions/checkout@v4
-        with:
-          repository: mrploch/ploch-common
-          ref: master
-          path: ploch-common
-          fetch-depth: 0
+      # No ploch-common checkout. The release build consumes ploch-common as released NuGet
+      # packages (UsePlochProjectReferences defaults to false), so the sibling sources are not
+      # needed - and checking them out at the moving `master` branch is what made this workflow
+      # able to pack a stable package with prerelease Ploch.Common dependencies (issue #47).
+      # mrploch-development is still required: Directory.Packages.props imports the shared
+      # version files from it.
 
       - name: Checkout mrploch-development (shared build config)
         uses: actions/checkout@v4
@@ -134,6 +160,36 @@ jobs:
           echo "NuGet package version: $NUGET_VERSION"
 
       - name: Restore dependencies
+        # Scoped to this step rather than the job, so the package credential is not exposed
+        # to checkout, upload, release or other third-party actions (least privilege, raised
+        # by the CodeRabbit reviewer on PR #52).
+        #
+        # nuget.config maps Ploch.* to both nuget.org and the GitHub Packages feed, and the
+        # GitHub feed authenticates with %GH_PACKAGES_TOKEN%. Without it, every Ploch restore
+        # queries that feed unauthenticated and takes a 401.
+        #
+        # This is a reliability fix, and the mechanism is worth stating precisely because the
+        # obvious reading of it is wrong in both directions. An unauthenticated eligible feed
+        # does not reliably fail - a local test with an isolated packages folder and no token
+        # warned, retried, fell back to nuget.org and exited 0 - but neither is it merely
+        # cosmetic. NuGet queries sources concurrently and a restore succeeds as soon as one
+        # supplies the requested version, while a terminal protocol failure from another source
+        # is rethrown rather than ignored. Which of those happens first is a race: if the
+        # GitHub feed exhausts its retries before nuget.org returns the match, restore can fail
+        # with NU1301 even though the package exists on nuget.org. One green run is a sample of
+        # that race, not proof it cannot go the other way.
+        #
+        # It is also load-bearing, not just defensive: prerelease Ploch builds exist ONLY on the
+        # GitHub feed, so the moment the shared Ploch.Packages.props names a prerelease the
+        # nuget.org fallback finds nothing and restore fails hard with NU1103.
+        #
+        # It does NOT fix that for pull requests from forks - GitHub does not expose repository
+        # secrets to fork PRs at all, so no workflow can authenticate for them. Tracked in #54.
+        #
+        # Raised by four PR reviewers (codeant-ai, qodo, copilot, chatgpt-codex) and sharpened
+        # by the Codex and Antigravity reviewers on PR #52.
+        env:
+          GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }}
         run: dotnet restore ./Ploch.CommandLine.Spectre.slnx
 
       - name: Build (Release)

Directory.Build.props

@@ -78,6 +78,32 @@
     <LangVersion>default</LangVersion>
   </PropertyGroup>
 
+  <!--
+    How this repository consumes the sibling Ploch repositories (ploch-common).
+
+    Default (false): PackageReference against the released packages on nuget.org, versioned
+    centrally from mrploch-development/dependencies/Ploch.Packages.props. This is what CI and
+    the release build use, and it is the only mode that produces a correct package at pack
+    time - a cross-repo ProjectReference becomes a NuGet *dependency* whose version comes from
+    whatever the sibling checkout happens to be, which is how a stable Ploch.CommandLine.Spectre
+    would otherwise have shipped depending on a prerelease Ploch.Common (issue #47, NU5104).
+    It also removes the requirement for a sibling checkout to exist at all.
+
+    Opt in with -p:UsePlochProjectReferences=true to build against a local ploch-common
+    checkout instead - for cross-repo development, where a change in ploch-common needs to be
+    seen here before it is published. Note the local checkout may be ahead of the released
+    packages (ploch-common main is on 4.1-prerelease), so the two modes are not interchangeable.
+
+    CI only exercises the DEFAULT (package) mode for the main solution. The one
+    -p:UsePlochProjectReferences=true invocation in build-dotnet.yml builds the *sample*
+    solution and does not run the main test projects against the sibling dependency graph, so
+    project-reference mode is verified locally but not guarded by CI. Raised by the Codex
+    reviewer on PR #52; adding a second CI job for it is tracked in #51.
+  -->
+  <PropertyGroup>
+    <UsePlochProjectReferences Condition="'$(UsePlochProjectReferences)' == ''">false</UsePlochProjectReferences>
+  </PropertyGroup>
+
   <!-- Nerdbank.GitVersioning and SourceLink for all projects. -->
   <ItemGroup>
     <PackageReference Include="Nerdbank.GitVersioning" PrivateAssets="all" />
@@ -102,4 +128,33 @@
   <ItemGroup Condition="$(IsTestProject)">
     <PackageReference Include="coverlet.msbuild" PrivateAssets="all" />
   </ItemGroup>
+
+  <!--
+    Test harness, declared here so every test project owns the runner it needs.
+
+    These used to arrive transitively: a ProjectReference propagates the referenced project's
+    entire PackageReference closure, so the test projects free-rode on the harness of the
+    sibling Ploch.TestingSupport.XUnit3.* projects. A PackageReference does not work that way -
+    only what the .nuspec declares flows - and the published
+    Ploch.TestingSupport.XUnit3.Dependencies package declares Microsoft.NET.Test.Sdk with
+    exclude="Build,Analyzers" (which strips the MSBuild targets that register the test host)
+    and does not declare xunit.runner.visualstudio at all. The result after the switch to
+    packages was a clean build in which `dotnet test` discovered nothing: "No test is available
+    ... Make sure that test discoverer & executors are registered".
+
+    A test project should declare its own runner regardless of where its assertions come from,
+    so this is the correct home for it rather than a transitive accident.
+  -->
+  <ItemGroup Condition="$(IsTestProject)">
+    <!--
+      PrivateAssets on the runner infrastructure but not on xunit.v3, matching the dotnet SDK
+      test template: the host and the VSTest adapter are build-time infrastructure, whereas
+      xunit.v3 is a real reference the test code compiles against. Currently cosmetic - test
+      projects are IsPackable=false so nothing flows anywhere - but the template shape is the
+      one a reader expects. Raised by the Copilot (Grok 4.6) reviewer on PR #52.
+    -->
+    <PackageReference Include="Microsoft.NET.Test.Sdk" PrivateAssets="all" />
+    <PackageReference Include="xunit.v3" />
+    <PackageReference Include="xunit.runner.visualstudio" PrivateAssets="all" />
+  </ItemGroup>
 </Project>

Directory.Packages.props

@@ -1,4 +1,4 @@
-<Project>
+<Project>
   <PropertyGroup>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
   </PropertyGroup>
@@ -10,7 +10,42 @@
     <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.2" />
     <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.115" />
-    <PackageVersion Include="Ploch.Common" Version="2.0.1" />
+  </ItemGroup>
+  <!--
+    Ploch package versions (ploch-common, ploch-data) come from the shared file so every
+    repository in the workspace resolves the same builds. This replaces a local
+    `Ploch.Common 2.0.1` pin that was two major versions behind the packages this
+    repository's own published package depends on (issues #46, #47).
+  -->
+  <Import Project="../mrploch-development/dependencies/Ploch.Packages.props" />
+  <!--
+    Ploch.Common.Apps.Shared is published (4.0.47) but is not listed in the shared
+    Ploch.Packages.props, so it cannot be centrally versioned from there yet.
+    Tracked by: mrploch/mrploch-development#21
+
+    Written as Remove-then-Include rather than a plain Include, and versioned from the shared
+    family property rather than a literal, so that it survives the upstream fix landing:
+
+      * A second PackageVersion for an id that the imported file also defines is a hard error,
+        not a warning - verified: "error NU1506: Warning As Error: Duplicate 'PackageVersion'
+        items found". Since CI checks out mrploch-development at the moving `main` branch, a
+        merge in *that* repository would otherwise break restore here with no commit in this
+        one. The Remove makes the collision impossible.
+      * Taking the version from $(PlochCommonPackagesVersion) means that once the id is listed
+        upstream this block resolves to the identical version the rest of the family gets, so
+        it degrades into a harmless no-op that can be deleted at leisure instead of urgently.
+        The literal is only a fallback for the case where that property does not exist -
+        mrploch/mrploch-development#5 proposes renaming it to per-repo variables.
+
+    Raised by the Codex reviewer on PR #52.
+  -->
+  <PropertyGroup>
+    <PlochAppsSharedVersion>$(PlochCommonPackagesVersion)</PlochAppsSharedVersion>
+    <PlochAppsSharedVersion Condition="'$(PlochAppsSharedVersion)' == ''">4.0.47</PlochAppsSharedVersion>
+  </PropertyGroup>
+  <ItemGroup>
+    <PackageVersion Remove="Ploch.Common.Apps.Shared" />
+    <PackageVersion Include="Ploch.Common.Apps.Shared" Version="$(PlochAppsSharedVersion)" />
   </ItemGroup>
   <Import Project="../mrploch-development/dependencies/Common.Packages.props" />
   <Import Project="../mrploch-development/dependencies/Serilog.Logging.Packages.props" />