docs: Correct claims the PR reviews found wrong

[tablackburn]

Corrections from the six independent reviews of #186, #187, #188, #190, #192, and #194. Every item is a documented claim that the code does not actually honour — the kind a consumer acts on and is wrong.

1. The private-key guarantee was false (the serious one)

build.properties.ps1, the Get-PSBuildCertificate docstring, and CHANGELOG.md all said a private key is required in every case. It is not:

# Get-PSBuildCertificate.ps1
if ($cert -and -not $SkipValidation -and ($resolvedSource -eq 'EnvVar' -or $resolvedSource -eq 'PfxFile')) {
    # Check for private key          <-- first thing inside the block SkipValidation gates
    if (-not $cert.HasPrivateKey) { throw ... }

-not $SkipValidation gates the whole block, private-key check included. The reviewer proved it rather than inferring it: a public-only .cer fed through the EnvVar source with -SkipValidation came back with HasPrivateKey = False.

The same edit had also dropped "private key" from the list of checks those two sources skip — a list the docstring it replaced had correctly included. So it overclaimed the guarantee and under-reported what gets skipped.

Failure scenario: a consumer exports the public certificate instead of the PFX into a CI secret, has SkipCertificateValidation = $true, reads that a private key is always required, and expects Get-PSBuildCertificate to catch it. It does not — Set-AuthenticodeSignature fails later with a much worse message.

The guarantee is real only for Store and Thumbprint, where the private key is part of how the certificate is selected rather than a check layered on afterwards. All three files now say that, and the README gained a paragraph making the EnvVar/PfxFile behavior explicit.

2. The README documented pre-#194 behavior

The SkipCertificateValidation row said the setting applies only to EnvVar and PfxFile — accurate until #194 extended it to the store-backed sources as a fallback, stale the moment #194 merged. Two reviewers independently flagged that neither PR owned the reconciliation. The row and the Store/Thumbprint bullets now describe the fallback.

Column alignment preserved: every settings row still measures 61/45/174.

3. Get-PSBuildHelpLocale was mis-described

resolves the locale used for help generation

It returns the docs subdirectories that are help locales — plural, and a classification rather than a resolution. Its own synopsis: "Return the directories under a docs path that are help locales." An agent reading the old wording would come here to change the default locale, which is $PSBPreference.Help.DefaultLocale and lives somewhere else entirely. Corrected in both places it appeared — the reviewer caught the helper list, and the test table row had the same error.

4. CI is not sequential

The instructions said the shared workflow lints "then runs" the tests. The reviewer fetched the live ModuleCI.yml from psake/.github and found no needs: relationship between the lint and test jobs, with cspell running at strict: false so it cannot gate anything. Someone debugging a red run would go looking at the wrong job.

5. The contradiction #186 fixed in the README was still in the source

# This path is relative to the directory containing Pester tests
OutputFile = [IO.Path]::Combine($env:BHProjectPath, 'testResults.xml')   # absolute

Now states the default is absolute and that a relative value resolves against the test directory, because Test-PSBuildPester.ps1:92 pushes into it. CodeCoverage.OutputFile was checked for the same problem and does not have it.

Verification

• Full suite 503 passed, 0 failed, 3 skipped; Analyze clean.
• Settings-table alignment verified with awk — 61/45/174 on every row.
• All five touched files verified CRLF-only, no lone LF, no \r\r\n.

Not fixed here

Two items from the reviews are filed separately rather than folded in — both need a decision or verification beyond correcting a sentence.

• Verify whether Sign.TimestampServer is documented as the right protocol #196 — Sign.TimestampServer is documented as RFC 3161, but Set-AuthenticodeSignature -TimestampServer may use the legacy Authenticode protocol instead. Inherited wording, and settling it needs a signing experiment rather than a reading of this repository.
• Store and Thumbprint certificate tests never run on Windows PowerShell 5.1 #197 — -Skip:(-not $IsWindows) skips the Store/Thumbprint tests on Windows PowerShell 5.1, because $IsWindows does not exist there. So fix: Make Sign.SkipCertificateValidation actually skip validation #194's fix is never CI-tested on the engine where store-based signing is most common. It is the existing convention across the whole file, and un-skipping may surface mock failures on 5.1, so it wants its own change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01U1Jhu7fgTRJq7LK5MuKteE

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Test Results

    4 files  ±0    785 suites  ±0   2m 22s ⏱️ -4s
  508 tests ±0    505 ✅ ±0   3 💤 ±0  0 ❌ ±0 
2 015 runs  ±0  1 921 ✅ ±0  94 💤 ±0  0 ❌ ±0 

Results for commit baff21d. ± Comparison against base commit 3a45b56.