diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 46fc4c80..211c59e3 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -86,9 +86,25 @@ jobs:
runs-on: windows-latest
steps:
- uses: actions/checkout@v7
+ # The shared native-command guard, before the suites that depend on it: a
+ # failure here explains a failure below rather than being diagnosed twice.
+ # This one step covers both hosts on its own -- it re-invokes itself under
+ # the other shell and fails if it cannot find one.
+ - name: Run test-common.ps1
+ shell: pwsh
+ run: ./tools/test-common.ps1
- name: Run test-run-sabotage.ps1
shell: pwsh
run: ./tools/test-run-sabotage.ps1
+ # And again under Windows PowerShell 5.1, which is not a duplicate of the
+ # step above. The harness and its suite both capture native output, and the
+ # rule that makes that capture terminate under `Stop` exists ONLY on 5.1 --
+ # so a defect in that path is invisible to a pwsh-only job, which is
+ # exactly how one reached `main` and survived review. `powershell` is the
+ # Windows PowerShell 5.1 that ships on every windows runner.
+ - name: Run test-run-sabotage.ps1 (Windows PowerShell 5.1)
+ shell: powershell
+ run: ./tools/test-run-sabotage.ps1
# The edition, MSRV, and pinned channel are declared once in Cargo.toml and
# rust-toolchain.toml, then restated a dozen times -- in this file's `msrv`
diff --git a/DESIGN-NOTES.md b/DESIGN-NOTES.md
index 16ff0f4b..2418f1aa 100644
--- a/DESIGN-NOTES.md
+++ b/DESIGN-NOTES.md
@@ -1645,3 +1645,120 @@ the caller's own parsing mode -- `GetFullPathNameW`'s output, or
it after normalisation preserves what the path meant; doing it before silently
reinterprets it. Both crates arrived at this independently, which is why it is
written down once.
+
+## Shared support in `tools/` is dot-sourced, never a module
+
+`tools/common.ps1` holds the support every script there shares, and it is delivered by
+dot-sourcing (`. (Join-Path $PSScriptRoot 'common.ps1')`). **Converting it to a `.psm1` and
+importing it silently breaks it on Windows PowerShell 5.1, which is the only host it exists
+to protect**, so the delivery mechanism is a correctness requirement rather than a
+preference.
+
+The single thing it carries today is `Invoke-Native`, the guard against 5.1's treatment of
+native stderr: there, a native command that writes to stderr while `$ErrorActionPreference`
+is `Stop` raises a **terminating** error when its stderr is redirected with `2>&1`.
+PowerShell 7 does not. The guard flips the preference to `Continue` for the duration of the
+call, and **that flip is function-local rather than restored afterwards**: PowerShell
+assignment always writes to the current scope, so the caller's value is never modified and
+the local one is discarded on return, while `& $Command` still inherits it through the child
+scope. An earlier version wrapped the call in a `try`/`finally` that restored a copy nobody
+could observe; it was dead code, and three cases in
+[tools/test-common.ps1](tools/test-common.ps1) claimed to cover it while being unable to
+fail. The property that does need testing is the opposite one -- that the flip never
+*escapes* into the caller, which is what a `$script:`-scoped mistake would do.
+
+**Why a module fails.** A scriptblock carries the session state it was created in.
+`Invoke-Native { cargo build }` builds that scriptblock in the *caller's* script scope, so
+`& $Command` runs it there, not in the module's scope. A module copy sets
+`$ErrorActionPreference` in the module's own scope, the flip never reaches the scriptblock,
+and the native call still runs under `Stop`. Measured with the identical body in a `.psm1`:
+under 5.1 seven of the eight cases in [tools/test-common.ps1](tools/test-common.ps1) failed,
+while **all eight passed under PowerShell 7**. Dot-sourcing puts the function in the caller's
+own scope, where the plain assignment does reach the call.
+
+A module *can* be made to work by reaching into the caller's session state
+(`$PSCmdlet.SessionState.PSVariable.Set(...)`), and that was measured working on both hosts.
+It is rejected because the guard would then rest on a subtlety that looks removable: anyone
+simplifying it back to a plain assignment reintroduces a defect that still passes on
+PowerShell 7 and in CI. Dot-sourcing makes the property hold by construction.
+
+**The test crosses hosts, and refuses to pass vacuously.**
+[tools/test-common.ps1](tools/test-common.ps1) runs its cases in the invoking host and then
+re-invokes itself in the other one, failing if it cannot find it. A single-host suite is
+worthless for this defect class -- the whole hazard is that it is invisible on the host most
+people run, and CI ran `shell: pwsh` only, which is how the original defect reached `main`
+and survived review. The `sabotage harness tests` job now runs `test-run-sabotage.ps1` under
+both shells for the same reason.
+
+**What is deliberately NOT shared: `Write-Report`.** Six scripts define a function by that
+name, and they are *not* duplicates -- they differ in level vocabulary (`warn` against
+`warning`, `bad` against `error`, plus `good`, `note`, `detail`, `heading`) and in rendering:
+[run-numa-spikes.ps1](tools/run-numa-spikes.ps1) and
+[soak-flush-barrier.ps1](tools/soak-flush-barrier.ps1) emit GitHub Actions annotations
+(`::warning::`), the other four emit console colours. Consolidating them would mean unifying
+those vocabularies, which changes the output of six tools to remove a duplication that is
+only apparent. The shared name is a naming convention -- the repository's one-output-sink
+rule -- not shared code, and it stays that way until some script needs another's rendering.
+
+## Which Copilot reviews are dealt with: resolve threads, and mark suppressed-only reviews
+
+A long-lived pull request accumulates hundreds of Copilot reviews -- PR #56 reached 196 -- and
+GitHub records "this was dealt with" for only some of them. [tools/scan-pr-reviews.ps1](tools/scan-pr-reviews.ps1)
+reports what is genuinely outstanding, and the convention below is what makes that report mean
+something.
+
+**A review's findings arrive in two shapes, and only one of them has state.**
+
+- **Inline comments** become review threads, which can be **resolved**. That flag is durable,
+ visible, and queryable, so it is the tag -- there is nothing to invent. Resolve a thread when
+ its finding is dealt with, including when it is *refuted*: "we measured this and it is not a
+ defect" is a disposition, not an open question.
+- **Suppressed comments** exist only as prose inside the review body's `` block. They
+ create **no thread**, so there is nothing to resolve, and a review is not a reactable object
+ either: `POST /pulls/{n}/reviews/{id}/reactions` returns 404 while the same call against an
+ inline comment succeeds. Nothing anywhere records that a suppressed finding was read.
+
+**So suppressed-only reviews are tagged with a marker comment**, posted by the script:
+
+```
+
+
+
+```
+
+It is an HTML comment, so it does not render; it lives on the pull request rather than in a file
+or a session, so it survives a new machine, a new contributor, and a new agent session; and the
+script reads it back, which is what keeps the report from re-raising a review already handled.
+The script requires a `-Summary` alongside it, because a marker with no account of what was done
+is a claim with no evidence.
+
+**Reading the report.** It separates unresolved threads into *current* and *outdated*. Outdated
+means the anchored line has since changed, which usually means the finding was fixed and the
+thread simply never resolved -- so those are the cheap ones to clear, and they are listed only
+under `-IncludeOutdated` to keep the default output about work that is actually open.
+
+**A marker is only honoured from an account with `admin` or `write` permission**, checked
+against the collaborators permission endpoint rather than inferred from the comment's
+`author_association`. That field reports `COLLABORATOR` for a read-only collaborator as well
+as a writer, so trusting it would enforce something weaker than the control claims. The check
+fails closed -- a 404, a 403 because the account running the scan cannot query permissions, or
+any network failure leaves the marker unhonoured -- because over-reporting a finding that was
+in fact handled is visible and recoverable, while wrongly honouring a marker silently deletes
+the only record that a finding was never read.
+
+Unhonoured markers are counted and reported **separately by cause**, because the two causes
+send a reader to different places. A *denied* marker is a statement about its author: the
+endpoint answered, and the answer was `read` or `none`. An *unverifiable* one is a statement
+about the account running the scan: that endpoint requires the caller to have push access, so
+an account without it gets a flat 403 for every login it asks about -- including a maintainer
+whose markers are perfectly valid -- and reporting that as "the author lacks write access"
+would be an accusation the run never established. Two consequences worth knowing: scanning
+from an account without push access re-reports every marked review as outstanding, and a
+marker posted from a workflow using `GITHUB_TOKEN` is written by `github-actions[bot]`, whose
+permission reads `none`, so it can never be honoured. Both measured.
+
+**A marker asserts the review was read, so do not back-fill in bulk.** PR #56 carries 131
+reviews with suppressed comments and no marker. Almost all were addressed during the rounds
+that followed them, but "almost all" is not evidence, and marking them wholesale would convert
+an honest absence of information into a false record. Mark a historical review only when
+somebody has actually read it.
diff --git a/tools/check-workflow-refs.ps1 b/tools/check-workflow-refs.ps1
index acd44c10..a37b2613 100644
--- a/tools/check-workflow-refs.ps1
+++ b/tools/check-workflow-refs.ps1
@@ -47,6 +47,11 @@ param(
$ErrorActionPreference = 'Stop'
Set-StrictMode -Version Latest
+# `Invoke-Native`, for the cargo call below. Dot-sourced rather than imported: a
+# module copy of that guard does not reach the scriptblock it is handed and
+# silently fails on 5.1 alone -- see [common.ps1](common.ps1).
+. (Join-Path $PSScriptRoot 'common.ps1')
+
# Resolved here rather than as a param default: Windows PowerShell 5.1 does not
# populate $PSScriptRoot while evaluating a default on a [CmdletBinding()]
# script, so the default form fails outright under 5.1 while working under 7.
@@ -63,7 +68,13 @@ $repoRoot = Resolve-Path (Join-Path $PSScriptRoot '..')
# One `cargo metadata` call answers every crate/bin/example question. `--no-deps`
# keeps it to workspace members, which is the only thing a workflow can name.
-$metadataJson = & cargo metadata --no-deps --format-version 1 2>$null
+# Through `Invoke-NativeStdout`, NOT `Invoke-Native`, and the difference is the
+# whole point here: this output is parsed as JSON, so stderr must be DISCARDED
+# rather than merged into it. Cargo writes to stderr routinely -- a cold runner
+# emits rustup's `info: syncing channel updates` -- and merging that in makes
+# `ConvertFrom-Json` fail on the `i`. The guard is still needed because any
+# stderr redirect is a terminating error on Windows PowerShell 5.1 under `Stop`.
+$metadataJson = Invoke-NativeStdout { cargo metadata --no-deps --format-version 1 }
if ($LASTEXITCODE -ne 0) {
Write-Host "::error::cargo metadata failed, so workflow references cannot be resolved"
exit 2
diff --git a/tools/common.ps1 b/tools/common.ps1
new file mode 100644
index 00000000..4c7397d8
--- /dev/null
+++ b/tools/common.ps1
@@ -0,0 +1,191 @@
+# Copyright (c) Mike Grier.
+<#
+.SYNOPSIS
+ Shared support for the scripts in `tools/`. Dot-source it; do not run it.
+
+.DESCRIPTION
+ Dot-sourced rather than imported as a module, and that is a correctness
+ requirement rather than a style choice. See "Why not a module" below before
+ converting this to a `.psm1`.
+
+ Every script here that captures a native command's output needs the same
+ guard against Windows PowerShell 5.1's treatment of stderr, and that guard
+ had been copied into three scripts by the time it was written down. This is
+ the one copy.
+
+.NOTES
+ Usage, from any script in this directory:
+
+ . (Join-Path $PSScriptRoot 'common.ps1')
+
+ `$PSScriptRoot` is populated in a script body on both hosts. It is NOT
+ populated while evaluating a parameter default on a `[CmdletBinding()]`
+ script under 5.1, so keep this call in the body, as the scripts here do for
+ their own `-OutputDirectory` defaults.
+#>
+
+# Render a captured record as plain text.
+#
+# `2>&1` wraps a native command's stderr in ErrorRecords. One of those
+# stringifies to the literal text `System.Management.Automation.RemoteException`,
+# which lands in the middle of a captured compiler diagnostic and makes a
+# transcript less legible than the console output of the same failure.
+function ConvertTo-OutputLines {
+ param([Parameter(ValueFromPipeline = $true)] $Record)
+ process {
+ if ($Record -is [System.Management.Automation.ErrorRecord]) {
+ $Record.Exception.Message
+ }
+ else {
+ "$Record"
+ }
+ }
+}
+
+# Run a native command, capturing merged stdout+stderr as plain strings.
+#
+# Under Windows PowerShell 5.1, a native command that writes to stderr while
+# `$ErrorActionPreference` is `Stop` raises a TERMINATING error when its stderr
+# is redirected with `2>&1`. PowerShell 7 does not. Flipping the preference to
+# `Continue` for the duration of the call is what makes the capture work on both,
+# and keeping that flip function-local is what leaves `Stop` in force for
+# everything that is not a native call.
+#
+# **No restoration is needed, and none is attempted.** `$ErrorActionPreference =
+# 'Continue'` here creates a FUNCTION-LOCAL variable: PowerShell assignment
+# always writes to the current scope, so the caller's own value is untouched and
+# the local one is discarded when this returns. `& $Command` runs the scriptblock
+# in a child of this scope, so it inherits `Continue` -- which is exactly the
+# reach the guard needs -- while nothing outside sees it.
+#
+# An earlier version wrapped the call in `try { } finally { $ErrorActionPreference
+# = $previous }`. That restored the local copy nobody could observe, so it was
+# dead code, and worse: three cases in `test-common.ps1` claimed to cover it and
+# could not fail. Measured on both hosts -- with the `try/finally` deleted
+# outright, the caller still reads `Stop` immediately after the call, identically
+# to the version that had it.
+#
+# The property that DOES need a test is the other direction: that the flip never
+# escapes into the caller. Writing `$script:` or `$global:` here would leave the
+# caller running under `Continue` for everything afterwards, and `test-common.ps1`
+# covers that (a `$script:`-scoped mutant leaves the caller at `Continue` and
+# fails those cases).
+#
+# `$LASTEXITCODE` is global, so a caller still reads the command's exit code
+# after this returns. That matters here: these scripts distinguish a broken
+# instrument from a finding by exactly that code.
+#
+# ## Why not a module
+#
+# Moving this function into a `.psm1` and importing it SILENTLY BREAKS IT under
+# 5.1, which is the only host it exists to protect.
+#
+# A scriptblock carries the session state it was created in. `Invoke-Native
+# { cargo build }` builds that scriptblock in the CALLER's script scope, so
+# `& $Command` runs it there -- not in the module's scope. A module copy of this
+# function sets `$ErrorActionPreference` in the module's own scope, the flip
+# never reaches the scriptblock, and the native call still runs under `Stop`.
+#
+# Measured, because the failure is invisible on the host most people run: with
+# the identical body in a `.psm1`, 5.1 threw `RemoteException` and captured
+# nothing while PowerShell 7 succeeded. Dot-sourcing lands the function in the
+# caller's own scope, where the plain assignment below does reach the call, and
+# both hosts then behave identically.
+#
+# A module CAN be made to work by reaching into the caller's session state
+# (`$PSCmdlet.SessionState.PSVariable.Set(...)`), and that was measured working
+# too. It is rejected because the guard would then depend on a subtlety that
+# looks removable: anyone simplifying it back to a plain assignment would
+# reintroduce a defect that still passes on PowerShell 7 and in CI. Dot-sourcing
+# makes the property hold by construction instead of by counter-measure.
+#
+# [test-common.ps1](test-common.ps1) asserts this on both hosts.
+function Invoke-Native {
+ param([Parameter(Mandatory = $true)][scriptblock] $Command)
+ # Function-local by construction -- see the note above on why there is no
+ # restoration to do. Never `$script:` or `$global:` here.
+ $ErrorActionPreference = 'Continue'
+ & $Command 2>&1 | ConvertTo-OutputLines
+}
+
+# Run a native command and capture ONLY its stdout, discarding stderr.
+#
+# **Use this whenever the output is PARSED rather than shown.** `Invoke-Native`
+# above merges stderr into the capture, which is right for a transcript and
+# wrong for data: a command that writes progress or warnings to stderr while
+# succeeding on stdout produces a capture with the two interleaved, and the
+# parse then fails on text that was never part of the answer.
+#
+# That is not hypothetical. Routing `cargo metadata --no-deps --format-version 1`
+# through the merging helper turned green locally and red in CI, because a warm
+# workspace writes nothing to stderr while a cold runner emits rustup's
+# `info: syncing channel updates` -- so `ConvertFrom-Json` failed with
+# "Unexpected character encountered while parsing value: i". Reproduced on both
+# hosts against a stand-in that writes both streams.
+#
+# The redirect is still what makes this need the same `Continue` flip: on
+# Windows PowerShell 5.1 ANY stderr redirect, `2>$null` included, turns a native
+# command's stderr into a terminating error under `Stop`. Measured -- both
+# spellings throw, an unredirected call does not.
+#
+# `$LASTEXITCODE` survives, so a caller still distinguishes success from
+# failure; what it loses is the diagnostic text, which is the trade a parsed
+# command is making anyway.
+function Invoke-NativeStdout {
+ param([Parameter(Mandatory = $true)][scriptblock] $Command)
+ $ErrorActionPreference = 'Continue'
+ & $Command 2>$null
+}
+
+# Run a native command and capture its streams SEPARATELY.
+#
+# For the case `Invoke-NativeStdout` cannot serve: output that is parsed on
+# success, but whose stderr is the diagnostic worth reporting on failure.
+# Discarding stderr keeps the parse clean and throws away the only explanation
+# of what went wrong, and merging keeps the explanation and corrupts the parse;
+# this keeps both by not choosing.
+#
+# Measured, because which stream carries the message is NOT uniform and the
+# obvious assumption is wrong for the common case. `gh`:
+#
+# REST 404 stdout carries the JSON error body, stderr `gh: Not Found`
+# network failure stdout EMPTY, stderr `error connecting to ...`
+# usage error stdout EMPTY, stderr the usage text
+#
+# So a failure reported from stdout alone is blank exactly when the cause is
+# least guessable -- an unreachable host, a bad flag, an auth problem -- which
+# is the shape a broken-instrument message exists to explain.
+#
+# Returns an object with `Stdout`, `Stderr` and `ExitCode`, both texts already
+# flattened to plain strings. `$LASTEXITCODE` is also left set, so a caller that
+# only wants the code need not unpack anything.
+function Invoke-NativeSplit {
+ param([Parameter(Mandatory = $true)][scriptblock] $Command)
+ $ErrorActionPreference = 'Continue'
+
+ # Merged with `2>&1`, then partitioned by RECORD TYPE: PowerShell wraps a
+ # native command's stderr in ErrorRecords and leaves stdout as plain
+ # strings, so the merge is losslessly separable even though it looks like a
+ # mixed stream.
+ #
+ # A file redirect (`2>$path`) is the obvious alternative and is WRONG on
+ # Windows PowerShell 5.1. There it writes PowerShell's *formatted* error
+ # record to the file -- `cmd.exe : to-err`, then the offending source line, a
+ # caret ruler, CategoryInfo and FullyQualifiedErrorId -- rather than the raw
+ # stderr text, so the diagnostic would arrive wrapped in a stack trace of
+ # this helper. PowerShell 7 writes the raw text, so that version passed there
+ # and failed on 5.1; the cross-host suite caught it.
+ $merged = & $Command 2>&1
+ $code = $LASTEXITCODE
+
+ $stdout = @($merged | Where-Object { $_ -isnot [System.Management.Automation.ErrorRecord] })
+ $stderr = @($merged |
+ Where-Object { $_ -is [System.Management.Automation.ErrorRecord] } |
+ ForEach-Object { $_.Exception.Message }) -join "`n"
+
+ return [pscustomobject]@{
+ Stdout = $stdout
+ Stderr = $stderr
+ ExitCode = $code
+ }
+}
\ No newline at end of file
diff --git a/tools/run-numa-spikes.ps1 b/tools/run-numa-spikes.ps1
index e94faf9d..4ea4f4a9 100644
--- a/tools/run-numa-spikes.ps1
+++ b/tools/run-numa-spikes.ps1
@@ -68,6 +68,23 @@ function Write-Report {
}
}
+# `Invoke-Native` and `ConvertTo-OutputLines`, which every capture below goes
+# through. Dot-sourced rather than imported: a module copy of that guard does
+# not reach the scriptblock it is handed, and silently fails on 5.1 alone. The
+# full argument, and the measurement behind it, is in that file.
+#
+# What it costs THIS script, recorded here because the shape is specific to the
+# spikes: `cargo --quiet` writes nothing to stderr on a clean build, so under
+# 5.1 this ran to completion for as long as every spike was healthy. It threw
+# only when cargo did write there -- a warning, or a failed compile -- which is
+# exactly the case this script exists to report. The throw landed before
+# `$buildExit` was assigned, so the broken-instrument branch never ran: no
+# transcript, no summary, and the one artifact somebody downloads to diagnose a
+# rotted spike was the one case that never produced it. Confirmed both ways
+# against a deliberately uncompilable crate: unguarded, 5.1 threw and captured
+# nothing; guarded, it returned exit 101 with all seven lines of diagnostic.
+. (Join-Path $PSScriptRoot 'common.ps1')
+
$repoRoot = Resolve-Path (Join-Path $PSScriptRoot '..')
$spikeDir = Join-Path $repoRoot 'crates\windows-ioring-sys\design-sessions\spikes'
@@ -138,20 +155,20 @@ windows-sys = { version = "0.61.2", default-features = false, features = [$featu
Write-Report "=== building $($spike.Name) ==="
Push-Location $work
try {
- $build = & cargo build --quiet 2>&1
+ $build = Invoke-Native { cargo build --quiet }
$buildExit = $LASTEXITCODE
$buildOutput = ($build | Out-String)
if ($buildExit -ne 0) {
# A build failure is a defect in the instrument, and is one of the
# two things here worth failing over.
Write-Report "spike $($spike.Name) failed to build" -Level error
- # Echo the Out-String rendering rather than the raw objects. `2>&1`
- # turns cargo's stderr into ErrorRecords, and one of those
- # stringifies to the literal text `System.Management.Automation.
- # RemoteException` in the middle of the compiler diagnostic. Piping
- # the already-rendered text keeps the log and the transcript
- # identical, instead of the artifact being the more legible of the
- # two records of the same failure.
+ # Echo the same text the transcript gets, so the log and the
+ # artifact are two renderings of one capture rather than two
+ # records of one failure that a reader has to reconcile.
+ # `Invoke-Native` has already flattened the ErrorRecords `2>&1`
+ # produces into plain strings, so neither carries the stray
+ # `System.Management.Automation.RemoteException` this used to
+ # splice into the middle of a compiler diagnostic.
$buildOutput.TrimEnd() -split "`n" | ForEach-Object { Write-Report $_.TrimEnd() }
$instrumentFailures++
$sections.Add("### $($spike.Name)`n`n**FAILED TO BUILD** -- the instrument is broken, not the machine.`n")
@@ -159,7 +176,7 @@ windows-sys = { version = "0.61.2", default-features = false, features = [$featu
else {
$built = $true
Write-Report "=== running $($spike.Name) ==="
- $output = & cargo run --quiet 2>&1 | Out-String
+ $output = Invoke-Native { cargo run --quiet } | Out-String
$runExit = $LASTEXITCODE
Write-Report $output
}
diff --git a/tools/run-sabotage.ps1 b/tools/run-sabotage.ps1
index ecf4d939..b1f65898 100644
--- a/tools/run-sabotage.ps1
+++ b/tools/run-sabotage.ps1
@@ -171,6 +171,13 @@ param(
Set-StrictMode -Version Latest
$ErrorActionPreference = 'Stop'
+# `Invoke-Native`, which the git calls below go through. Dot-sourced rather than
+# imported: a module copy of that guard does not reach the scriptblock it is
+# handed, and silently fails on Windows PowerShell 5.1 alone -- see
+# [common.ps1](common.ps1), and [test-common.ps1](test-common.ps1) for the
+# cross-host proof.
+. (Join-Path $PSScriptRoot 'common.ps1')
+
$utf8NoBom = [System.Text.UTF8Encoding]::new($false)
# Script scope so Invoke-Bounded reads it without threading it through three
@@ -209,7 +216,23 @@ function Exit-WithMessage {
}
function Get-RepoRoot {
- $root = git rev-parse --show-toplevel 2>$null
+ # Through `Invoke-NativeStdout`, and BOTH halves of that name are load
+ # bearing here.
+ #
+ # Guarded at all, because any stderr redirect -- `2>$null` as much as `2>&1`
+ # -- makes a native command's stderr a TERMINATING error on Windows
+ # PowerShell 5.1 under `Stop`, and this is the one call in this script whose
+ # failure mode IS stderr: outside a working tree git says `fatal: not a git
+ # repository`. Measured on 5.1, unguarded, the line below was unreachable --
+ # the script died with NativeCommandError and exited 1, which in this script
+ # means "sabotages did not behave as declared" rather than "you ran me in
+ # the wrong directory". PowerShell 7 reached it either way, which is why the
+ # deliberate exit-2 path looked fine.
+ #
+ # Stdout-only rather than the merging `Invoke-Native`, because this output is
+ # PARSED -- it becomes the repository root. Git can warn on stderr while
+ # succeeding, and merging would splice that warning into the path.
+ $root = Invoke-NativeStdout { git rev-parse --show-toplevel }
if ($LASTEXITCODE -ne 0) {
# Reported, not thrown. Under $ErrorActionPreference = 'Stop' a `throw`
# here is a terminating error that prints a stack trace and propagates
@@ -837,7 +860,7 @@ New-Item -ItemType Directory -Force -Path $OutputDirectory | Out-Null
# without bound and looks like nothing until it does.
$outputFull = [System.IO.Path]::GetFullPath($OutputDirectory)
if ($outputFull.StartsWith($repoRootPrefix, [System.StringComparison]::OrdinalIgnoreCase)) {
- git -C $repoRoot check-ignore -q -- $outputFull 2>&1 | Out-Null
+ Invoke-Native { git -C $repoRoot check-ignore -q -- $outputFull } | Out-Null
if ($LASTEXITCODE -ne 0) {
Exit-WithMessage (@(
"-OutputDirectory is inside the repository but git does not ignore it:"
diff --git a/tools/scan-pr-reviews.ps1 b/tools/scan-pr-reviews.ps1
new file mode 100644
index 00000000..c1e1d2c9
--- /dev/null
+++ b/tools/scan-pr-reviews.ps1
@@ -0,0 +1,548 @@
+# Copyright (c) Mike Grier.
+<#
+.SYNOPSIS
+ Report which Copilot reviews on a pull request still need attention, and
+ record the ones that have been dealt with.
+
+.DESCRIPTION
+ A long-lived pull request accumulates hundreds of Copilot reviews, and
+ nothing in the GitHub UI says which have been dealt with. Judging by eye
+ does not scale, and judging by "did a commit follow it" is guesswork. This
+ reports the two signals that are real, and adds the one GitHub does not.
+
+ A review carries findings in one of two shapes, and they need different
+ treatment because GitHub models only one of them:
+
+ INLINE COMMENTS become review threads, which can be RESOLVED. That flag is
+ durable, visible in the UI, and queryable, so it is the tag for this shape
+ -- there is nothing to invent. A thread also reports `isOutdated`, meaning
+ the line it was anchored to has since changed; those are reported
+ separately, because an outdated finding is usually one that was fixed and
+ never resolved rather than one still waiting.
+
+ SUPPRESSED COMMENTS exist only as prose inside the review body's ``
+ block. They create no thread, so there is nothing to resolve; and a review
+ is not a reactable object either -- `POST /pulls/{n}/reviews/{id}/reactions`
+ is 404, while the same call on an inline comment succeeds. A review whose
+ findings were ALL suppressed therefore has no state anywhere saying it was
+ read, which is exactly the gap this script closes.
+
+ For those, `-MarkProcessed` posts a pull-request comment carrying a marker:
+
+
+
+
+
+ A marker is honoured only on a line of its own inside that begin/end block,
+ so a comment that merely MENTIONS one does not retire anything. The id is
+ written here as a placeholder rather than digits on purpose: copying this
+ example verbatim, sentinels included, still matches nothing.
+
+ The marker is an HTML comment, so it does not render, and it lives on the
+ pull request rather than in a file or a session, which is what makes it
+ survive a new machine, a new contributor, and a new agent session. A later
+ run of this script reads those markers back and stops reporting the review.
+
+ Only markers written by someone whose repository permission is `admin` or
+ `write` are honoured, checked against the collaborators permission endpoint
+ rather than inferred from `author_association` -- GitHub reports
+ COLLABORATOR for a read-only collaborator too, so that field is weaker than
+ this control claims to be. This repository is public, so anyone who can
+ comment could otherwise retire a finding that has no other state anywhere.
+ The check fails closed, and markers that were not honoured are counted and
+ reported rather than silently dropped -- separately by cause, because "the
+ author has no write access" and "this account cannot query permissions at
+ all" send a reader to look in different places.
+
+ Two consequences worth knowing before using -MarkProcessed:
+
+ - Running the scan from an account WITHOUT push access to this repository
+ makes every marker unverifiable (the endpoint 403s for every login), so
+ every marked review is re-reported as outstanding.
+ - Posting a marker from a workflow using GITHUB_TOKEN writes it as
+ github-actions[bot], whose permission reads `none`, so that marker can
+ never be honoured. Measured, not assumed. Mark from a real account.
+
+.OUTPUTS
+ Exit code 0 when nothing is outstanding, 1 when there are findings, and 2 when
+ the tool could not run (gh unauthenticated, no such pull request, the marker
+ could not be posted). 1 and 2 are kept distinct so a caller can tell a finding
+ from a broken instrument.
+
+.PARAMETER Pr
+ The pull request number.
+
+.PARAMETER MarkProcessed
+ One or more review ids to record as processed. Posts a single comment
+ carrying a marker for each, with the summary as its visible text.
+
+.PARAMETER Summary
+ The visible text of the marker comment. Required with -MarkProcessed:
+ a marker with no account of what was done is a claim with no evidence.
+
+.PARAMETER IncludeOutdated
+ Also list unresolved threads whose anchor line has since changed.
+
+.EXAMPLE
+ .\tools\scan-pr-reviews.ps1 -Pr 56
+
+.EXAMPLE
+ .\tools\scan-pr-reviews.ps1 -Pr 56 -MarkProcessed 5136043258 `
+ -Summary 'Both suppressed findings were measured and refuted; see commit abc1234.'
+#>
+[CmdletBinding()]
+param(
+ [Parameter(Mandatory = $true)][int] $Pr,
+ [long[]] $MarkProcessed,
+ [string] $Summary,
+ [switch] $IncludeOutdated
+)
+
+Set-StrictMode -Version Latest
+$ErrorActionPreference = 'Stop'
+
+. (Join-Path $PSScriptRoot 'common.ps1')
+
+# Exit codes, kept distinct on purpose. `1` is a FINDING -- the scan ran and
+# there is outstanding work -- while `2` is a BROKEN INSTRUMENT: `gh` is not
+# authenticated, the network is down, the pull request does not exist, the
+# marker could not be posted. A caller gating on the exit code has to be able to
+# tell those apart, or "no findings" and "the tool never ran" look identical,
+# which is the same instrument-versus-finding separation `run-numa-spikes.ps1`
+# and `run-sabotage.ps1` already make. Every error path below leaves through
+# `Exit-Broken` rather than through a bare `throw`, because an unhandled throw
+# from a script also exits 1 and would collide with the finding code.
+$script:ExitFindings = 1
+$script:ExitBroken = 2
+
+# The sentinels that make a marker a marker. Written once so the emitter and the
+# reader cannot drift, which is the failure this whole tool is about.
+$script:MarkerBegin = ''
+$script:MarkerEnd = ''
+
+function Exit-Broken {
+ param([Parameter(Mandatory = $true)][string] $Message)
+ [Console]::Error.WriteLine($Message)
+ exit $script:ExitBroken
+}
+
+# `$LASTEXITCODE` is UNSET until a native command has run in the session, and
+# under `Set-StrictMode -Version Latest` reading an unset variable throws. That
+# is not a hypothetical: with `gh` absent from PATH, the call below fails with
+# CommandNotFoundException before ever setting it, and the script then died on
+# the StrictMode violation rather than on the missing tool -- exiting 1, the
+# code that means "there are findings". Reading it through `Get-Variable`
+# removes the landmine wherever the code path reaches it.
+function Get-LastExitCode {
+ $variable = Get-Variable -Name LASTEXITCODE -Scope Global -ErrorAction SilentlyContinue
+ if ($null -eq $variable -or $null -eq $variable.Value) { return $null }
+ return [int]$variable.Value
+}
+
+# `gh` is the whole instrument here, so its absence is checked once, up front,
+# rather than being discovered as a confusing symptom further in. Without this,
+# a machine without the CLI reported either a StrictMode error about
+# `$LASTEXITCODE` or a JSON parse failure -- both describing the wreckage rather
+# than the cause, and both exiting 1 as though the scan had found something.
+if (-not (Get-Command gh -ErrorAction SilentlyContinue)) {
+ Exit-Broken @'
+gh was not found on PATH, so this tool cannot read the pull request at all.
+Install the GitHub CLI (https://cli.github.com) and run `gh auth login`.
+'@
+}
+
+$script:Owner = 'MikeGrier'
+$script:Name = 'windows-threadpool-sys'
+
+# The single output sink, per the repository's one-output-sink rule.
+function Write-Report {
+ param(
+ [Parameter(Mandatory = $true)][AllowEmptyString()][string] $Message,
+ [ValidateSet('info', 'detail', 'heading', 'warn')][string] $Level = 'info'
+ )
+ switch ($Level) {
+ 'detail' { Write-Host $Message -ForegroundColor DarkGray }
+ 'heading' { Write-Host $Message -ForegroundColor Cyan }
+ 'warn' { Write-Host $Message -ForegroundColor Yellow }
+ default { Write-Host $Message -ForegroundColor Gray }
+ }
+}
+
+function Invoke-GitHubJson {
+ param([string[]] $Arguments)
+ # Streams kept separate, because this needs both and for different reasons:
+ # stdout is parsed as JSON on success and must not have stderr spliced into
+ # it, while on failure stderr is usually the ONLY explanation. Measured on
+ # `gh`: a REST 404 puts its JSON body on stdout, but a network failure or a
+ # usage error leaves stdout EMPTY and says everything on stderr -- so
+ # reporting from stdout alone was blank exactly when the cause was least
+ # guessable.
+ $result = Invoke-NativeSplit { gh @Arguments }
+ $text = $result.Stdout
+ $code = $result.ExitCode
+ if ($null -eq $code -or $code -ne 0) {
+ # stderr first, falling back to stdout: whichever carried the message.
+ $detail = ("$($result.Stderr)").Trim()
+ if (-not $detail) { $detail = ("$($text -join ' ')").Trim() }
+ if (-not $detail) { $detail = '(no output on either stream)' }
+ Exit-Broken "gh $($Arguments -join ' ') failed (exit $code): $detail"
+ }
+ try {
+ return ($text -join "`n") | ConvertFrom-Json
+ }
+ catch {
+ # Reached when gh succeeds but returns something that is not JSON -- a
+ # proxy's HTML error page is the realistic case. Still the instrument,
+ # not a finding.
+ Exit-Broken "gh $($Arguments -join ' ') returned unparsable output: $($_.Exception.Message)"
+ }
+}
+
+# --- marking -----------------------------------------------------------------
+
+if ($MarkProcessed) {
+ if (-not $Summary) {
+ Exit-Broken 'Summary is required with -MarkProcessed: a marker with no account of what was done is a claim with no evidence.'
+ }
+ # Bracketed by sentinels, and the reader below honours a marker ONLY inside
+ # such a block, on a line of its own. Without that, any occurrence of the
+ # marker text anywhere in any comment counted -- so a maintainer quoting one
+ # in ordinary discussion, or pasting an example out of the documentation,
+ # would silently retire a review. The block is what distinguishes "this
+ # comment IS a marker" from "this comment MENTIONS one".
+ $lines = @($Summary, '', $script:MarkerBegin)
+ foreach ($id in $MarkProcessed) { $lines += "" }
+ $lines += $script:MarkerEnd
+ $file = Join-Path ([System.IO.Path]::GetTempPath()) ("mark-" + [guid]::NewGuid().ToString('N') + '.md')
+ [System.IO.File]::WriteAllText($file, ($lines -join "`n"), [System.Text.UTF8Encoding]::new($false))
+ try {
+ # Split for the same reason as the API calls: on success stdout is the
+ # comment URL and is echoed as such, while on failure the explanation is
+ # on stderr. Merging would have reported the two as one string, so a
+ # successful post could echo a warning as though it were the URL.
+ $result = Invoke-NativeSplit {
+ gh pr comment $Pr --repo "$script:Owner/$script:Name" --body-file $file
+ }
+ $code = $result.ExitCode
+ if ($null -eq $code -or $code -ne 0) {
+ $detail = ("$($result.Stderr)").Trim()
+ if (-not $detail) { $detail = ("$($result.Stdout -join ' ')").Trim() }
+ if (-not $detail) { $detail = '(no output on either stream)' }
+ Exit-Broken "posting the marker comment failed (exit $code): $detail"
+ }
+ Write-Report "marked processed: $($MarkProcessed -join ', ')"
+ Write-Report ($result.Stdout -join ' ') -Level detail
+ }
+ finally { Remove-Item $file -ErrorAction SilentlyContinue }
+ exit 0
+}
+
+# --- scanning ----------------------------------------------------------------
+
+# Walk a property chain over API-supplied data, yielding $null rather than
+# throwing when any link is absent or null.
+#
+# Needed because `Set-StrictMode -Version Latest` turns `$x.a.b` into a
+# TERMINATING error the moment `a` is null, and several fields these queries
+# return are nullable BY SCHEMA rather than by accident: GraphQL `author` and
+# `pullRequestReview` are null for a deleted account, REST `user` likewise, and
+# `submitted_at` is null on a PENDING review. Any one of those would leave this
+# script through an unhandled throw -- which exits 1, the code that means
+# "there are findings". A contributor deleting their GitHub account would
+# silently turn this tool into a false positive.
+#
+# Verified on both hosts that all of these throw under StrictMode without it:
+# `$null.login`, `@()[0]`, and `[datetime]$null`.
+function Get-Path {
+ param($Object, [string[]] $Names)
+ $current = $Object
+ foreach ($name in $Names) {
+ if ($null -eq $current) { return $null }
+ $property = $current.PSObject.Properties[$name]
+ if ($null -eq $property) { return $null }
+ $current = $property.Value
+ }
+ return $current
+}
+
+# `??` is PowerShell 7 only, and these tools run on 5.1 too.
+function Get-Text {
+ param($Value)
+ if ($null -eq $Value) { return '' }
+ return [string]$Value
+}
+
+Write-Report "scanning pull request #$Pr" -Level heading
+
+$reviews = Invoke-GitHubJson @('api', "repos/$script:Owner/$script:Name/pulls/$Pr/reviews?per_page=100", '--paginate')
+$issueComments = Invoke-GitHubJson @('api', "repos/$script:Owner/$script:Name/issues/$Pr/comments?per_page=100", '--paginate')
+
+# Whether this author may retire a finding, by ACTUAL repository permission.
+#
+# `author_association` is the cheap answer and it is the wrong one. GitHub sets
+# `COLLABORATOR` for anyone *invited to collaborate*, with no permission
+# qualifier -- a collaborator with `read` or `triage` gets `COLLABORATOR` too --
+# so trusting that value would enforce something weaker than the control claims.
+# The permission endpoint answers the question actually being asked.
+#
+# One call per DISTINCT author who posted a marker, cached, and markers are
+# rare, so this is a call or two per scan rather than one per comment.
+#
+# **Fails closed.** Anything other than a confirmed `admin` or `write` -- a 404
+# because the author is not a collaborator, a 403 because the caller running
+# this scan lacks push access and may not query permissions, a network failure
+# -- leaves the marker unhonoured. That direction is deliberate: an unhonoured
+# marker over-reports a finding that was in fact handled, which is visible and
+# recoverable, while wrongly honouring one silently deletes the only record that
+# a finding was never read.
+# Three outcomes, not two, because two of them have different causes and only one
+# of them is a statement about the author:
+#
+# 'allowed' the endpoint answered `admin` or `write`.
+# 'denied' the endpoint answered, and it was `read` or `none`. Measured:
+# a plain non-collaborator returns exit 0 with `read`, and a bot
+# account returns exit 0 with `none` -- neither is an error.
+# 'unverifiable' the call failed. The endpoint requires the CALLER to have push
+# access, so an account without it gets a flat 403 for every
+# login, including the maintainer who wrote the markers.
+#
+# Collapsing the last two was a defect of exactly the kind this whole tool exists
+# to prevent: the run reported "author lacks write access" in a case where it had
+# established nothing about the author, and the reader would go and check the
+# author's role rather than their own token.
+#
+# All three still fail closed -- only 'allowed' honours a marker -- because
+# over-reporting a handled finding is visible and recoverable, while wrongly
+# honouring one silently deletes the only record that a finding was never read.
+$script:PermissionCache = @{}
+function Get-RetireAuthority {
+ param([string] $Login)
+ # No author at all (a deleted account leaves `user` null). Nothing to
+ # attribute the marker to, which is itself a decided answer rather than an
+ # unanswerable one.
+ if (-not $Login) { return 'denied' }
+ if ($script:PermissionCache.ContainsKey($Login)) { return $script:PermissionCache[$Login] }
+
+ # Deliberately NOT through Invoke-GitHubJson: a non-zero exit here is an
+ # ordinary answer rather than a broken instrument, so it must not exit 2.
+ # Stdout only: the answer is compared against `admin`/`write`, so anything
+ # `gh` writes to stderr would make a genuine writer read as denied.
+ $text = Invoke-NativeStdout {
+ gh api "repos/$script:Owner/$script:Name/collaborators/$Login/permission" --jq '.permission'
+ }
+ $code = Get-LastExitCode
+
+ $authority = if ($null -eq $code -or $code -ne 0) {
+ 'unverifiable'
+ }
+ elseif (@('admin', 'write') -contains (Get-Text ($text -join '')).Trim()) {
+ 'allowed'
+ }
+ else {
+ 'denied'
+ }
+
+ $script:PermissionCache[$Login] = $authority
+ return $authority
+}
+
+# The review ids a comment genuinely marks as processed.
+#
+# A marker counts only when it sits on a line of its own INSIDE a
+# begin/end sentinel block. Matching the marker text anywhere in the body -- what
+# this did first -- cannot tell "this comment IS a marker" from "this comment
+# MENTIONS one", so quoting one in discussion, or pasting the example out of this
+# script's own documentation, would silently retire a review.
+#
+# The documentation deliberately spells its example with a placeholder id rather
+# than digits, so even a verbatim copy of the example, sentinels and all, matches
+# nothing here.
+function Get-MarkerIds {
+ param([string] $Body)
+
+ $ids = @()
+ if (-not $Body) { return $ids }
+
+ # `(?m)` so `^`/`$` bind to lines rather than the whole body, and the
+ # sentinels are matched literally.
+ $blockPattern = '(?ms)^\s*' + [regex]::Escape($script:MarkerBegin) +
+ '\s*$(.*?)^\s*' + [regex]::Escape($script:MarkerEnd) + '\s*$'
+
+ foreach ($block in [regex]::Matches($Body, $blockPattern)) {
+ foreach ($m in [regex]::Matches($block.Groups[1].Value,
+ '(?m)^\s*\s*$')) {
+ $ids += [long]$m.Groups[1].Value
+ }
+ }
+ return $ids
+}
+
+# Reviews already recorded as processed, by marker.
+#
+# This repository is public, so anyone able to comment on a pull request can post
+# a marker; and because a suppressed-only review has no state anywhere else --
+# which is the whole reason this tool exists -- an unauthorised marker would
+# permanently and silently delete the only record that a finding was never read.
+# The counter would simply report a smaller number, with nothing to indicate why.
+$processed = @{}
+$deniedMarkers = 0
+$unverifiableMarkers = 0
+foreach ($c in $issueComments) {
+ $markers = @(Get-MarkerIds (Get-Text $c.body))
+ if ($markers.Count -eq 0) { continue }
+
+ $login = Get-Text (Get-Path $c @('user', 'login'))
+ # Counted by CAUSE rather than lumped together, and never dropped in silence:
+ # a denied marker says something about its author, an unverifiable one says
+ # something about the account running this scan, and telling a reader the
+ # wrong one sends them to look in the wrong place.
+ switch (Get-RetireAuthority $login) {
+ 'allowed' {
+ foreach ($id in $markers) { $processed[$id] = $true }
+ }
+ 'denied' { $deniedMarkers += $markers.Count }
+ default { $unverifiableMarkers += $markers.Count }
+ }
+}
+
+# Unresolved threads, which is the authoritative state for inline findings.
+$query = @'
+query($owner:String!, $name:String!, $pr:Int!, $cursor:String) {
+ repository(owner:$owner, name:$name) {
+ pullRequest(number:$pr) {
+ reviewThreads(first:100, after:$cursor) {
+ pageInfo { hasNextPage endCursor }
+ nodes {
+ isResolved isOutdated path line
+ comments(first:1) {
+ nodes { body author { login } pullRequestReview { databaseId } }
+ }
+ }
+ }
+ }
+ }
+}
+'@
+
+$threads = @()
+$cursor = $null
+do {
+ $arguments = @('api', 'graphql', '-f', "query=$query",
+ '-F', "owner=$script:Owner", '-F', "name=$script:Name", '-F', "pr=$Pr")
+ if ($cursor) { $arguments += @('-F', "cursor=$cursor") }
+ $page = (Invoke-GitHubJson $arguments).data.repository.pullRequest.reviewThreads
+ foreach ($t in $page.nodes) {
+ # A thread with no comments is not a shape this query should produce, but
+ # indexing an empty array is an error rather than $null under StrictMode,
+ # so it is checked rather than assumed.
+ $comments = Get-Path $t @('comments', 'nodes')
+ if ($null -eq $comments -or @($comments).Count -eq 0) { continue }
+ $c = @($comments)[0]
+
+ $login = Get-Path $c @('author', 'login')
+ if ((Get-Text $login) -notmatch '[Cc]opilot') { continue }
+
+ # Null for a deleted review; without it there is nothing to attribute the
+ # thread to, so the thread is skipped rather than attributed to review 0.
+ $reviewId = Get-Path $c @('pullRequestReview', 'databaseId')
+ if ($null -eq $reviewId) { continue }
+
+ $threads += [pscustomobject]@{
+ Review = [long]$reviewId
+ IsResolved = $t.isResolved
+ IsOutdated = $t.isOutdated
+ Path = $t.path
+ Line = $t.line
+ Body = ((Get-Text (Get-Path $c @('body'))) -replace '\s+', ' ')
+ }
+ }
+ $cursor = if ($page.pageInfo.hasNextPage) { $page.pageInfo.endCursor } else { $null }
+} while ($cursor)
+
+$copilotReviews = @($reviews | Where-Object {
+ (Get-Text (Get-Path $_ @('user', 'login'))) -match '[Cc]opilot'
+ })
+
+# A review's suppressed count is only in its body, as rendered prose.
+function Get-SuppressedCount {
+ param([string] $Body)
+ if ($Body -match 'Suppressed comments?\s*\((\d+)\)') { return [int]$Matches[1] }
+ return 0
+}
+
+$openThreads = @($threads | Where-Object { -not $_.IsResolved })
+$current = @($openThreads | Where-Object { -not $_.IsOutdated })
+$outdated = @($openThreads | Where-Object { $_.IsOutdated })
+
+$suppressedOnly = @()
+foreach ($r in $copilotReviews) {
+ $count = Get-SuppressedCount (Get-Text $r.body)
+ if ($count -eq 0) { continue }
+ if ($processed.ContainsKey([long]$r.id)) { continue }
+ # A review whose inline threads are all resolved may still carry suppressed
+ # findings nobody read, so this is judged on the marker alone.
+ # Null on a PENDING review, and `[datetime]$null` is an error rather than a
+ # zero date, so the absence is rendered rather than cast.
+ $submitted = Get-Path $r @('submitted_at')
+ $when = if ($submitted) { ([datetime]$submitted).ToString('yyyy-MM-dd HH:mm') } else { 'pending ' }
+
+ $suppressedOnly += [pscustomobject]@{
+ Id = [long]$r.id
+ When = $when
+ Suppressed = $count
+ Inline = @($threads | Where-Object { $_.Review -eq [long]$r.id }).Count
+ }
+}
+
+Write-Report ''
+Write-Report "Copilot reviews: $($copilotReviews.Count)"
+Write-Report "unresolved threads: $($openThreads.Count) ($($current.Count) current, $($outdated.Count) outdated)"
+Write-Report "reviews with suppressed: $(@($copilotReviews | Where-Object { (Get-SuppressedCount (Get-Text $_.body)) -gt 0 }).Count)"
+Write-Report " of those, unprocessed: $($suppressedOnly.Count)"
+if ($deniedMarkers -gt 0) {
+ Write-Report "markers not honoured: $deniedMarkers (author has no write access here)" -Level warn
+}
+if ($unverifiableMarkers -gt 0) {
+ # Says whose problem it is. This fires when the ACCOUNT RUNNING THE SCAN
+ # cannot query collaborator permissions, which is a 403 for every login it
+ # asks about -- including a maintainer whose markers are perfectly valid --
+ # so pointing at the author would send the reader to check the wrong thing.
+ Write-Report "markers unverifiable: $unverifiableMarkers" -Level warn
+ Write-Report " This account could not query collaborator permissions, so no marker" -Level warn
+ Write-Report " could be confirmed and every marked review is re-reported above. That" -Level warn
+ Write-Report " is this token's push access, not a statement about who wrote them." -Level warn
+}
+Write-Report ''
+
+Write-Report '=== unresolved threads on current lines ===' -Level heading
+if ($current.Count -eq 0) { Write-Report ' none' -Level detail }
+foreach ($t in ($current | Sort-Object Path, Line)) {
+ Write-Report (" review {0} {1}:{2}" -f $t.Review, $t.Path, $t.Line)
+ Write-Report (' ' + $t.Body.Substring(0, [Math]::Min(200, $t.Body.Length))) -Level detail
+}
+
+if ($IncludeOutdated) {
+ Write-Report ''
+ Write-Report '=== unresolved threads whose anchor line has changed ===' -Level heading
+ Write-Report ' Usually fixed and never resolved; resolve them to clear this list.' -Level detail
+ foreach ($t in ($outdated | Sort-Object Path, Line)) {
+ Write-Report (" review {0} {1}:{2}" -f $t.Review, $t.Path, $t.Line)
+ }
+}
+
+Write-Report ''
+Write-Report '=== reviews with suppressed comments and no processed marker ===' -Level heading
+Write-Report ' Suppressed findings create no thread, so nothing else records that they' -Level detail
+Write-Report ' were read. Mark one with -MarkProcessed once it has been dealt with.' -Level detail
+if ($suppressedOnly.Count -eq 0) { Write-Report ' none' -Level detail }
+foreach ($r in ($suppressedOnly | Sort-Object When)) {
+ Write-Report (" {0} {1} suppressed={2} inline={3}" -f $r.Id, $r.When, $r.Suppressed, $r.Inline)
+}
+
+Write-Report ''
+if ($current.Count -gt 0 -or $suppressedOnly.Count -gt 0) {
+ Write-Report 'Outstanding items above.' -Level warn
+ exit $script:ExitFindings
+}
+Write-Report 'Nothing outstanding.'
+exit 0
diff --git a/tools/soak-flush-barrier.ps1 b/tools/soak-flush-barrier.ps1
index 35ff2d8e..a67f073d 100644
--- a/tools/soak-flush-barrier.ps1
+++ b/tools/soak-flush-barrier.ps1
@@ -134,49 +134,22 @@ function Write-Report {
}
}
-# Normalise one cargo invocation's merged output into plain strings.
+# `Invoke-Native` and `ConvertTo-OutputLines`, which every capture site here goes
+# through. Dot-sourced rather than imported: a module copy of that guard does not
+# reach the scriptblock it is handed, and silently fails on 5.1 alone. The full
+# argument, and the measurement behind it, is in that file.
#
-# With 2>&1, native stderr arrives as ErrorRecord objects. For the *empty* lines
-# cargo emits between diagnostics the message is "" while ToString() falls back
-# to the type name, so a bare "$_" renders those as
-# "System.Management.Automation.RemoteException" scattered through the output --
-# which then reaches both the saved log and the Select-String that extracts the
-# detail column.
+# What it costs THIS script, recorded here because the shape is specific to the
+# soak: cargo writes to stderr routinely -- "Compiling ...", and the "did not
+# finalize incremental compilation session directory" notes this workspace emits
+# constantly. Measured: under 5.1 the script died in the build step with
+# NativeCommandError before running a single instrument, having written only the
+# CSV header. Under 7 the same script completed.
#
-# Defined once and used by every capture site on purpose: this was originally
-# fixed at the build step alone, leaving the per-instrument run with the same
-# defect, which is how two copies of one rule drift apart.
-function ConvertTo-OutputLines {
- param([Parameter(ValueFromPipeline = $true)] $Record)
- process {
- if ($Record -is [System.Management.Automation.ErrorRecord]) {
- $Record.Exception.Message
- } else {
- "$Record"
- }
- }
-}
-
-# Run a native command, capturing merged stdout+stderr as plain strings.
-#
-# The ErrorActionPreference dance is what makes this work on Windows PowerShell
-# 5.1. There, a native command writing to stderr under `Stop` raises a
-# TERMINATING error, and cargo writes to stderr routinely -- "Compiling ...",
-# and the "did not finalize incremental compilation session directory" notes
-# this workspace emits constantly. Measured: under 5.1 the script died in the
-# build step with NativeCommandError before running a single instrument, having
-# written only the CSV header. Under 7 the same script completed. Restoring the
-# preference afterwards keeps `Stop` for everything that is not a native call.
-function Invoke-Native {
- param([Parameter(Mandatory = $true)][scriptblock] $Command)
- $previous = $ErrorActionPreference
- $ErrorActionPreference = 'Continue'
- try {
- & $Command 2>&1 | ConvertTo-OutputLines
- } finally {
- $ErrorActionPreference = $previous
- }
-}
+# Used by every capture site rather than one: this was originally fixed at the
+# build step alone, leaving the per-instrument run with the same defect, which is
+# how two copies of one rule drift apart.
+. (Join-Path $PSScriptRoot 'common.ps1')
# The rotation, in the order described above.
$instruments = @(
diff --git a/tools/test-common.ps1 b/tools/test-common.ps1
new file mode 100644
index 00000000..b528566c
--- /dev/null
+++ b/tools/test-common.ps1
@@ -0,0 +1,291 @@
+# Copyright (c) Mike Grier.
+<#
+.SYNOPSIS
+ Tests for [common.ps1](common.ps1), run on BOTH PowerShell hosts.
+
+.DESCRIPTION
+ Runs its cases in the host that invoked it, then re-invokes itself in the
+ other host and requires that run to pass too.
+
+ The cross-host run is the point of this file, not a nicety. The defect
+ `Invoke-Native` exists to prevent appears ONLY under Windows PowerShell 5.1:
+ PowerShell 7 captures a native command's stderr under `Stop` without
+ complaint. A suite that tested one host would report green while the guard
+ was broken on the only host that needs it -- which is how the original
+ defect reached `main` and survived review, since CI runs `shell: pwsh`.
+
+ It refuses to pass vacuously: if the other host cannot be found, that is a
+ FAILURE rather than a skip, because "tested one host" is not the claim this
+ file is here to make.
+
+.PARAMETER SingleHost
+ Run the cases in this process only, without re-invoking the other host.
+ Used internally for the child run; also useful when debugging one host.
+#>
+[CmdletBinding()]
+param(
+ [switch] $SingleHost
+)
+
+Set-StrictMode -Version Latest
+$ErrorActionPreference = 'Stop'
+
+. (Join-Path $PSScriptRoot 'common.ps1')
+
+$script:Failures = 0
+$script:Host51 = 'C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe'
+
+# The single output sink, per the repository's one-sink rule.
+function Write-Report {
+ param(
+ [Parameter(Mandatory = $true)][AllowEmptyString()][string] $Message,
+ [ValidateSet('info', 'good', 'bad', 'heading')][string] $Level = 'info'
+ )
+ switch ($Level) {
+ 'good' { Write-Host $Message -ForegroundColor Green }
+ 'bad' { Write-Host $Message -ForegroundColor Red }
+ 'heading' { Write-Host $Message -ForegroundColor Cyan }
+ default { Write-Host $Message -ForegroundColor Gray }
+ }
+}
+
+function Test-Case {
+ param([string] $Name, [scriptblock] $Body)
+ try {
+ & $Body
+ Write-Report " PASS $Name" -Level good
+ }
+ catch {
+ $script:Failures++
+ Write-Report " FAIL $Name" -Level bad
+ Write-Report " $($_.Exception.Message)"
+ }
+}
+
+function Assert-Equal {
+ param($Expected, $Actual, [string] $What)
+ if ("$Expected" -ne "$Actual") {
+ throw "$What -- expected '$Expected', got '$Actual'"
+ }
+}
+
+Write-Report "common.ps1 tests on $($PSVersionTable.PSVersion)" -Level heading
+
+# The guard's whole purpose. A bare `& { ... } 2>&1` here would throw on 5.1.
+Test-Case 'a native command writing to stderr is captured, not thrown' {
+ $out = Invoke-Native { cmd /c "echo to-stderr 1>&2" }
+ Assert-Equal 'to-stderr' ("$out".Trim()) 'captured text'
+}
+
+Test-Case 'stdout and stderr are merged in one capture' {
+ $out = Invoke-Native { cmd /c "echo on-out & echo on-err 1>&2" }
+ $text = ($out -join ' ')
+ if ($text -notmatch 'on-out' -or $text -notmatch 'on-err') {
+ throw "both streams must appear, got '$text'"
+ }
+}
+
+# The reason the script that motivated this cares: it tells a broken instrument
+# from a finding by the exit code, so the guard must not swallow it.
+Test-Case 'the exit code survives the capture' {
+ $null = Invoke-Native { cmd /c "echo boom 1>&2 & exit 3" }
+ Assert-Equal 3 $LASTEXITCODE 'LASTEXITCODE after a failing native command'
+}
+
+Test-Case 'a failing command still yields its diagnostic text' {
+ $out = Invoke-Native { cmd /c "echo diagnostic-line 1>&2 & exit 1" }
+ if ("$out" -notmatch 'diagnostic-line') {
+ throw "the diagnostic must be captured, got '$out'"
+ }
+}
+
+# Flattening is what keeps a transcript readable; without it a captured stderr
+# line can stringify to `System.Management.Automation.RemoteException`.
+Test-Case 'captured records are plain strings, not ErrorRecords' {
+ $out = @(Invoke-Native { cmd /c "echo to-stderr 1>&2" })
+ foreach ($line in $out) {
+ if ($line -is [System.Management.Automation.ErrorRecord]) {
+ throw 'a raw ErrorRecord escaped ConvertTo-OutputLines'
+ }
+ }
+ if ("$out" -match 'RemoteException') {
+ throw "a record stringified to RemoteException: '$out'"
+ }
+}
+
+# --- Invoke-NativeStdout: the same guard, but for output that is PARSED -------
+#
+# The distinction is not cosmetic and these cases exist because getting it wrong
+# shipped: routing `cargo metadata` through the MERGING helper passed locally and
+# failed in CI, where a cold runner emits rustup's `info:` line on stderr and
+# `ConvertFrom-Json` then choked on the `i`.
+
+Test-Case 'stdout-only capture discards stderr instead of merging it' {
+ $out = Invoke-NativeStdout { cmd /c "echo noise 1>&2 & echo {`"ok`":true}" }
+ $text = ("$out").Trim()
+ Assert-Equal '{"ok":true}' $text 'only stdout may be captured'
+ if ($text -match 'noise') { throw "stderr leaked into a parsed capture: $text" }
+}
+
+Test-Case 'stdout-only capture still yields parseable JSON under stderr noise' {
+ # The exact shape that failed in CI, reduced.
+ $out = Invoke-NativeStdout { cmd /c "echo info: syncing channel updates 1>&2 & echo {`"ok`":true}" }
+ $parsed = $out | ConvertFrom-Json
+ Assert-Equal $true $parsed.ok 'the JSON must survive stderr noise'
+}
+
+Test-Case 'stdout-only capture preserves the exit code' {
+ $null = Invoke-NativeStdout { cmd /c "echo boom 1>&2 & exit 4" }
+ Assert-Equal 4 $LASTEXITCODE 'LASTEXITCODE after a failing parsed command'
+}
+
+Test-Case 'stdout-only capture does not throw on 5.1 despite redirecting' {
+ # `2>$null` is a redirect too, so it hits the same 5.1 rule the merging
+ # helper guards. Without the Continue flip this throws there and passes on 7.
+ $out = Invoke-NativeStdout { cmd /c "echo only-stderr 1>&2" }
+ Assert-Equal '' ("$out").Trim() 'a command writing only to stderr yields nothing, and does not throw'
+}
+
+# --- Invoke-NativeSplit: parse stdout, report stderr --------------------------
+#
+# For output parsed on success whose stderr is the diagnostic on failure.
+# Discarding stderr keeps the parse clean and loses the only explanation of what
+# went wrong; merging keeps the explanation and corrupts the parse. These pin
+# that it does neither.
+
+Test-Case 'split capture keeps the two streams apart' {
+ $r = Invoke-NativeSplit { cmd /c "echo to-err 1>&2 & echo to-out" }
+ Assert-Equal 'to-out' ("$($r.Stdout)").Trim() 'stdout must not carry stderr'
+ if (("$($r.Stderr)").Trim() -ne 'to-err') {
+ throw "stderr must be captured separately, got '$($r.Stderr)'"
+ }
+}
+
+Test-Case 'split capture parses stdout as JSON under stderr noise' {
+ $r = Invoke-NativeSplit { cmd /c "echo info: syncing 1>&2 & echo {`"ok`":true}" }
+ $parsed = $r.Stdout | ConvertFrom-Json
+ Assert-Equal $true $parsed.ok 'the JSON must survive stderr noise'
+}
+
+# The case the whole helper exists for, and the one a stdout-only capture got
+# wrong: measured on `gh`, a network failure or a usage error leaves stdout EMPTY
+# and puts the entire explanation on stderr. Reporting from stdout alone was
+# blank exactly when the cause was least guessable.
+Test-Case 'split capture still has a diagnostic when stdout is empty' {
+ $r = Invoke-NativeSplit { cmd /c "echo only-on-stderr 1>&2 & exit 5" }
+ Assert-Equal 5 $r.ExitCode 'the exit code must survive'
+ Assert-Equal '' ("$($r.Stdout)").Trim() 'this command writes nothing to stdout'
+ if (("$($r.Stderr)").Trim() -ne 'only-on-stderr') {
+ throw "the diagnostic must survive on stderr, got '$($r.Stderr)'"
+ }
+}
+
+Test-Case 'split capture does not throw on 5.1 despite redirecting' {
+ # A file redirect is still a redirect, so this hits the same 5.1 rule.
+ $r = Invoke-NativeSplit { cmd /c "echo x 1>&2" }
+ Assert-Equal 0 $r.ExitCode 'a command that only writes to stderr still succeeds'
+}
+
+Test-Case 'split capture reports an empty stderr as empty, not null' {
+ # StrictMode: a `$null` here would make `.Trim()` at the call sites throw.
+ $r = Invoke-NativeSplit { cmd /c "echo quiet" }
+ Assert-Equal '' ("$($r.Stderr)").Trim() 'a silent command yields empty stderr'
+ if ($null -eq $r.Stderr) { throw 'Stderr must be an empty string rather than $null' }
+}
+
+# The flip must REACH the scriptblock, and this is the only case that shows it
+# directly. The stderr case above shows it too, but only on 5.1 -- PowerShell 7
+# captures either way, so on 7 nothing else here distinguishes a guard that works
+# from one that does nothing. Observing the preference from inside the passed
+# scriptblock is host-independent.
+Test-Case 'the flip reaches the scriptblock it is handed' {
+ $seen = Invoke-Native { $ErrorActionPreference }
+ Assert-Equal 'Continue' ("$seen".Trim()) 'ErrorActionPreference as seen inside the command'
+}
+
+# The three cases below are the OTHER direction: the flip must not escape.
+#
+# They are deliberately not described as testing a "restoration". `Invoke-Native`
+# assigns to a function-local `$ErrorActionPreference`, so the caller's value is
+# never modified and there is nothing to restore -- an earlier version wrapped
+# the call in a `try/finally` that restored a copy nobody could observe, and
+# these three cases could not fail against deleting it. Measured on both hosts.
+#
+# What they do catch is real and is the mutation worth guarding: writing
+# `$script:ErrorActionPreference` or `$global:` in `Invoke-Native` would leave
+# the CALLER running under `Continue` for everything afterwards, silently
+# disarming `Stop` for the rest of the script. A `$script:`-scoped mutant leaves
+# the caller at `Continue` and fails all three.
+# Each of these three sets `$script:ErrorActionPreference` to a known value
+# first, rather than capturing whatever it happens to be. That is not ceremony:
+# a mutant that escapes to script scope leaks `Continue` on its FIRST call, so a
+# later case reading "before" would capture `Continue`, compare it against
+# `Continue` afterwards, and pass -- the contamination hiding itself. Measured:
+# without this reset, a `$script:`-scoped mutant was caught only by the
+# behavioural case below, and the two variable-observing cases passed.
+Test-Case 'the flip does not escape into the caller' {
+ $script:ErrorActionPreference = 'Stop'
+ $null = Invoke-Native { cmd /c "echo to-stderr 1>&2" }
+ Assert-Equal 'Stop' $ErrorActionPreference 'the caller''s ErrorActionPreference after the call'
+}
+
+# The same property observed through behaviour rather than through the variable:
+# `Stop` must still terminate on something that is not the native call.
+Test-Case 'Stop still terminates a non-native error after the call' {
+ $script:ErrorActionPreference = 'Stop'
+ $null = Invoke-Native { cmd /c "echo to-stderr 1>&2" }
+ $threw = $false
+ try { Get-Item 'Q:\no\such\path\at\all.txt' | Out-Null } catch { $threw = $true }
+ if (-not $threw) { throw 'Stop was left disarmed for cmdlet errors' }
+}
+
+# And on the path where the command throws, which is where a scope-escaping
+# assignment would be least likely to be noticed by hand.
+Test-Case 'the flip does not escape when the command throws' {
+ $script:ErrorActionPreference = 'Stop'
+ try { $null = Invoke-Native { throw 'deliberate' } } catch { }
+ Assert-Equal 'Stop' $ErrorActionPreference 'the caller''s ErrorActionPreference after a throw'
+}
+
+if (-not $SingleHost) {
+ # The other host, which is the claim this file exists to make.
+ $isSeven = $PSVersionTable.PSVersion.Major -ge 6
+ $other = if ($isSeven) { $script:Host51 } else { 'pwsh' }
+ $otherName = if ($isSeven) { 'Windows PowerShell 5.1' } else { 'PowerShell 7' }
+
+ $resolved = if ($isSeven) {
+ if (Test-Path $other) { $other } else { $null }
+ }
+ else {
+ $command = Get-Command $other -ErrorAction SilentlyContinue
+ if ($command) { $command.Source } else { $null }
+ }
+
+ if (-not $resolved) {
+ $script:Failures++
+ Write-Report ''
+ Write-Report "FAIL $otherName was not found, so the cross-host claim is untested." -Level bad
+ Write-Report ' This suite exists to prove the guard on BOTH hosts: the defect it'
+ Write-Report ' guards against appears only on 5.1, so a single-host pass is not'
+ Write-Report ' the result this file reports. Install the missing host or run it'
+ Write-Report ' there by hand rather than treating this as a skip.'
+ }
+ else {
+ Write-Report ''
+ Write-Report "=== re-running under $otherName ===" -Level heading
+ & $resolved -NoProfile -File $PSCommandPath -SingleHost
+ if ($LASTEXITCODE -ne 0) {
+ $script:Failures++
+ Write-Report "FAIL the $otherName run reported failures." -Level bad
+ }
+ }
+}
+
+Write-Report ''
+if ($script:Failures -gt 0) {
+ Write-Report "$($script:Failures) failure(s)." -Level bad
+ exit 1
+}
+
+Write-Report 'All cases passed.' -Level good
+exit 0
diff --git a/tools/test-run-sabotage.ps1 b/tools/test-run-sabotage.ps1
index 4e5a90c4..1199ed27 100644
--- a/tools/test-run-sabotage.ps1
+++ b/tools/test-run-sabotage.ps1
@@ -74,6 +74,13 @@ param(
Set-StrictMode -Version Latest
$ErrorActionPreference = 'Stop'
+# `Invoke-Native`, which every native call below goes through. Dot-sourced
+# rather than imported: a module copy of that guard does not reach the
+# scriptblock it is handed, and silently fails on 5.1 alone -- see
+# [common.ps1](common.ps1), and [test-common.ps1](test-common.ps1) for the
+# cross-host proof.
+. (Join-Path $PSScriptRoot 'common.ps1')
+
$script:Harness = Join-Path $PSScriptRoot 'run-sabotage.ps1'
$script:Passed = 0
$script:Failed = 0
@@ -153,12 +160,12 @@ function New-Fixture {
[System.IO.File]::WriteAllText((Join-Path $root '.gitignore'), ".scratch/`n")
[System.IO.File]::WriteAllText((Join-Path $root 'src\lib.rs'), $Source)
- git -C $root init --quiet 2>&1 | Out-Null
+ Invoke-Native { git -C $root init --quiet } | Out-Null
if ($null -ne $Manifest) { Set-Manifest -Root $root -Spec $Manifest }
# Added to the index but not committed: `git ls-files` reads the index,
# which is all the harness needs, and committing would demand identity
# configuration this fixture has no reason to care about.
- git -C $root add -A 2>&1 | Out-Null
+ Invoke-Native { git -C $root add -A } | Out-Null
return $root
}
@@ -166,7 +173,7 @@ function Set-Manifest {
param([string] $Root, $Spec)
$json = $Spec | ConvertTo-Json -Depth 8
[System.IO.File]::WriteAllText((Join-Path $Root 'sabotage.json'), $json)
- git -C $Root add -A 2>&1 | Out-Null
+ Invoke-Native { git -C $Root add -A } | Out-Null
}
# The default manifest: patches the fixture's marker line, expecting it caught.
@@ -234,24 +241,24 @@ function Invoke-Harness {
param([string] $Root, [string[]] $Arguments)
Push-Location $Root
- # $ErrorActionPreference is dropped to Continue for the call, and this is
- # load bearing on Windows PowerShell 5.1. There, a native command's stderr
- # redirected with 2>&1 arrives as an ErrorRecord, which under Stop is a
- # TERMINATING error -- so every case testing a rejection path threw on the
- # harness's own message instead of reading its exit code, and reported the
- # harness's text as the failure. The harness writes to stderr deliberately
- # (that is what Exit-WithMessage is for), so its output is data here, not a
- # fault. PowerShell 7 does not do this, which is why the suite passed there
- # and failed on 5.1 until it was run on both.
- $previous = $ErrorActionPreference
- $ErrorActionPreference = 'Continue'
+ # Through Invoke-Native, and that is load bearing on Windows PowerShell 5.1.
+ # The harness runs here as a CHILD PROCESS, so its `Exit-WithMessage` writes
+ # -- which go straight to the process stderr handle -- are native stderr to
+ # this script. Redirected with 2>&1 under Stop they arrive as ErrorRecords,
+ # which is a TERMINATING error: every case testing a rejection path threw on
+ # the harness's own message instead of reading its exit code, and reported
+ # the harness's text as the failure. The harness writes to stderr
+ # deliberately, so its output is data here, not a fault. PowerShell 7 does
+ # not do this, which is why the suite passed there and failed on 5.1 until
+ # it was run on both.
try {
$shell = if ($PSVersionTable.PSVersion.Major -ge 6) { 'pwsh' } else { 'powershell' }
- $text = & $shell -NoProfile -File $script:Harness @Arguments 2>&1 | Out-String
+ $text = Invoke-Native {
+ & $shell -NoProfile -File $script:Harness @Arguments
+ } | Out-String
return [pscustomobject]@{ ExitCode = $LASTEXITCODE; Output = $text }
}
finally {
- $ErrorActionPreference = $previous
Pop-Location
}
}
@@ -456,6 +463,29 @@ Test-Case 'rejects an output directory inside the repository that git does not i
finally { Remove-Fixture $root }
}
+Test-Case 'rejects being run outside a git repository, on both hosts' {
+ # This one is host-sensitive, and it went unnoticed because the suite only
+ # ever ran the harness INSIDE a fixture repository. `Get-RepoRoot` captured
+ # git with `2>$null`, and on Windows PowerShell 5.1 any stderr redirect makes
+ # a native command's stderr a TERMINATING error under `Stop` -- so outside a
+ # working tree, where git's whole answer is `fatal: not a git repository` on
+ # stderr, the deliberate exit-2 path was unreachable. The harness died with
+ # NativeCommandError and exited 1, which in this script means "sabotages did
+ # not behave as declared": a broken instrument reported as a finding.
+ #
+ # PowerShell 7 reached the intended path either way, which is exactly why
+ # this needs a test rather than a reading.
+ $outside = Join-Path ([System.IO.Path]::GetTempPath()) ('nogit-' + [guid]::NewGuid().ToString('N'))
+ New-Item -ItemType Directory -Force -Path $outside | Out-Null
+ [System.IO.File]::WriteAllText((Join-Path $outside 'sabotage.json'), (New-Spec | ConvertTo-Json -Depth 8))
+ try {
+ $result = Invoke-Harness -Root $outside -Arguments @('-Manifest', 'sabotage.json', '-List')
+ Assert-Equal 2 $result.ExitCode $result.Output
+ Assert-Match 'Not inside a git repository' $result.Output
+ }
+ finally { Remove-Item -Recurse -Force $outside -ErrorAction SilentlyContinue }
+}
+
Test-Case 'validates a manifest timeoutSeconds even when -TimeoutSeconds overrides it' {
# The check used to be gated on "we are about to use this", so a bogus
# manifest value was diagnosed only on the runs that did NOT override it --
@@ -800,7 +830,7 @@ Test-Case 'copies a file whose name git quotes, and drops it when the source doe
try {
$odd = Join-Path $root ('src\caf' + [char]0xE9 + '.rs')
[System.IO.File]::WriteAllText($odd, "// unicode`n")
- git -C $root add -A 2>&1 | Out-Null
+ Invoke-Native { git -C $root add -A } | Out-Null
$stub = New-Stub -Behaviour 'fail' -Root $root
Invoke-Harness -Root $root `
@@ -810,7 +840,7 @@ Test-Case 'copies a file whose name git quotes, and drops it when the source doe
Assert-True (Test-Path -LiteralPath $copied) 'the copy must not silently omit it'
Remove-Item -LiteralPath $odd -Force
- git -C $root add -A 2>&1 | Out-Null
+ Invoke-Native { git -C $root add -A } | Out-Null
Invoke-Harness -Root $root `
-Arguments @('-Manifest', 'sabotage.json', '-CargoCommand', $stub) | Out-Null