From 889e37a22420e62bdf5cb34b79b71ab2929547ce Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 04:34:40 +0000 Subject: [PATCH 1/2] fix(windows): Invoke-Checked rejected the empty argument list every no-arg test uses (#512) `windows-msvc-cpu` has failed on every pull request, ~21 minutes in, with zero compile diagnostics: Cannot bind argument to parameter 'Arguments' because it is an empty array. `Invoke-Checked` declared `Arguments` as `[Parameter(Mandatory)][string[]]`. PowerShell's `Mandatory` validation treats an empty collection as "not supplied", so every call that runs a test executable taking no arguments died at parameter binding before the process was ever started. There are six such production call sites, not four -- the two forced-CPU-tier invocations (`VT_CPU_MATMUL_TIER=portable` / `avx2`) are on the same path and would have failed next. The fix is `[AllowEmptyCollection()]` alongside `Mandatory`, not a `= @()` default. The existing intent is that a caller must state its argument list; `AllowEmptyCollection` keeps the omission an error while permitting an explicitly empty list, whereas a default would silently accept a call that forgot the parameter entirely. The contract step ran green all along because the suite never executed an empty-argument invocation, which is why a 21-minute build step caught what a seconds-long contract step should have. `Invoke-CheckedContractTests` closes that: it injects a recording runner -- mirroring the `DumpbinRunner` and unsupported-tier-probe seams already in this file -- and asserts the empty and non-empty argument lists are forwarded verbatim and that a nonzero status still throws on both. `Invoke-Checked` gains the matching optional `-Runner` seam. RED-first, run under pwsh 7.6.4: the new contract test fails with the exact CI message above before `[AllowEmptyCollection()]` is applied, and passes after. Five mutations of the claimed guarantees are each caught by their intended assertion (drop `AllowEmptyCollection`; swallow the argument list; never throw on nonzero; forward the wrong program; truncate the arguments), and the real `& $Program @Arguments` path -- not just the fake runner -- was exercised against `/bin/true`, `/bin/echo a b`, and `/bin/false`. The pre-existing unsupported-tier contract assertion is untouched and proven still non-vacuous: making the probe send an empty, a wrong, or a two-element argument list each still trips "did not receive one exact filter argument". That empty-args mutation also shows the fake-runner scriptblock parameters need no `AllowEmptyCollection` of their own -- being non-mandatory, they bind `@()` and fail on the assertion rather than on binding. This is a distinct defect from #514, the POSIX `setenv`/`unsetenv` C3861 error that fails `windows-msvc-vulkan`; that one is fixed on its own branch and `windows-msvc-vulkan` stays red here until it lands. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- scripts/build-windows-release.ps1 | 65 +++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/scripts/build-windows-release.ps1 b/scripts/build-windows-release.ps1 index b8725cce1..7c339dbcf 100644 --- a/scripts/build-windows-release.ps1 +++ b/scripts/build-windows-release.ps1 @@ -19,10 +19,66 @@ if ($ArtifactId -ne "windows-x86_64-msvc-$Backend") { } function Invoke-Checked { param([Parameter(Mandatory)][string]$Program, - [Parameter(Mandatory)][string[]]$Arguments) - & $Program @Arguments - if ($LASTEXITCODE -ne 0) { - throw "$Program exited with status $LASTEXITCODE" + [Parameter(Mandatory)][AllowEmptyCollection()][string[]]$Arguments, + [scriptblock]$Runner) + if ($null -eq $Runner) { + & $Program @Arguments + $exitCode = $LASTEXITCODE + } else { + $exitCode = [int](& $Runner $Program $Arguments) + } + if ($exitCode -ne 0) { + throw "$Program exited with status $exitCode" + } +} + +# Most of this script's checked invocations run a test executable that takes no +# arguments, so `Invoke-Checked` must bind an explicitly empty argument list and +# still forward it verbatim (#512). +function Invoke-CheckedContractTests { + $calls = [System.Collections.Generic.List[object]]::new() + $recorder = { + param([string]$Program, [string[]]$Arguments) + $calls.Add([pscustomobject]@{ + Program = $Program + Arguments = @($Arguments) + }) | Out-Null + return 0 + }.GetNewClosure() + + Invoke-Checked "fake-empty.exe" @() -Runner $recorder + Invoke-Checked "fake-args.exe" @("--help", "--verbose") -Runner $recorder + + if ($calls.Count -ne 2) { + throw "checked-invocation fake runner was not invoked exactly twice" + } + if ($calls[0].Program -ne "fake-empty.exe" -or $calls[1].Program -ne "fake-args.exe") { + throw "checked invocation did not forward its exact program" + } + if ($calls[0].Arguments.Count -ne 0) { + throw "checked invocation did not forward an explicitly empty argument list" + } + if ($calls[1].Arguments.Count -ne 2 -or + $calls[1].Arguments[0] -ne "--help" -or + $calls[1].Arguments[1] -ne "--verbose") { + throw "checked invocation did not forward its exact argument list" + } + + $failing = { param([string]$Program, [string[]]$Arguments) return 3 } + foreach ($rejectedName in @("empty", "non-empty")) { + $rejected = $false + try { + if ($rejectedName -eq "empty") { + Invoke-Checked "fake-fail.exe" @() -Runner $failing + } else { + Invoke-Checked "fake-fail.exe" @("--help") -Runner $failing + } + } catch { + $rejected = $true + } + if (-not $rejected) { + throw "nonzero $rejectedName-argument exit status was accepted" + } } } @@ -157,6 +213,7 @@ function Invoke-UnsupportedTierContractTests { } if ($ContractTest) { + Invoke-CheckedContractTests Invoke-CrtContractTests Invoke-UnsupportedTierContractTests Write-Host "Windows PowerShell/CRT contract tests OK" From b00d6199f878a37b827dc51af27cb0ff4c2aa963 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 06:14:02 +0000 Subject: [PATCH 2/2] test(windows): pin the real-process branch and the Arguments binding contract (#512) A fresh review of #583 found two guarantees that nothing pins. Both are tightening, not repair: the behavior is already correct on the PR head, and this change only makes an edit that breaks it go red. The real-process branch of `Invoke-Checked` -- `$null -eq $Runner`, the one the release gate actually runs -- had no coverage at all, because a fake runner never executes `& $Program @Arguments`. Setting `$exitCode = 0` unconditionally there survived the whole contract suite, which is the failure class this repo keeps paying for: a Windows gate reporting success for tests that failed. It now drives the real branch for a success, an expected throw carrying the child's own nonzero status, argv distinctness, and an explicitly empty argument list. The program it drives is the PowerShell host executing the script, resolved from the running process. That is the one executable guaranteed to exist wherever this script can run, so the identical assertions execute on the Windows runners and on POSIX developer boxes. A `cmd.exe`/`/bin/sh` pair or a platform guard would give the Windows and POSIX runners different arms, and an arm that silently no-ops on one platform is its own version of the bug being fixed here. `Arguments` is `Mandatory` + `[AllowEmptyCollection()]` rather than defaulted to `@()` precisely so that omission stays a hard error while an explicitly empty list binds -- the argument the fix form rests on. Dropping `Mandatory` and adding `[AllowNull()]` both survived. An explicit `$null` now has to be rejected, and so does omission. Omission is asserted in an API runspace rather than in-process. An omitted mandatory parameter PROMPTS in an interactive console host: asserted directly, it hangs a developer's terminal on `Arguments[0]:` forever, and only reaches the binding error in CI where stdin is not a tty. A runspace host cannot prompt and reports the error instead, so the assertion means the same thing in both places. The function under test is rebuilt from the live definition's own source text, so it tracks any edit to the real parameter block. Mutation results, contract suite only, `pwsh` 7.6.4 on Linux. All eleven red, against four that survived on the #583 head (3768bd31): drop AllowEmptyCollection RED (was RED) runner forwards @() instead of $Arguments RED (was RED) exit-status check -> if ($false) RED (was RED) runner forwards "wrong.exe" RED (was RED) runner forwards only the first element RED (was RED) invert the $null -eq $Runner guard RED (was RED) runner called with no args argument at all RED (was RED) real branch sets $exitCode = 0 RED (was GREEN) real branch drops the @ splat RED (was GREEN) drop Mandatory from Arguments RED (was GREEN) add [AllowNull()] to Arguments RED (was GREEN) One caveat is worth recording rather than glossing. For a native executable, `& $Program $Arguments` and `& $Program @Arguments` are indistinguishable -- verified identical for a `[string[]]` at 0, 1 and 3 elements under all three `$PSNativeCommandArgumentPassing` modes, including `Windows`, which is the runner default. The splat only becomes observable when the program is a PowerShell script, where an empty list otherwise arrives as one array argument instead of no arguments. That is what the empty-argument probe catches, and it is the #512 contract stated exactly. Each new arm was also shown to catch a defect on its own, so none is dead weight: joining argv red-lines the distinctness probe, truncating argv red-lines it too, and dropping `Mandatory` with the null assertion deleted still red-lines the omission arm. Verified locally: contract suite green with stdin closed and under a pty (no prompt hang), `check-windows-portability.py` rc=0, `agent-preflight.sh --staged` all green. `windows-msvc-cpu` cannot go green regardless -- #512 unmasked a `STATUS_STACK_BUFFER_OVERRUN` in `test_openai_api_server.exe` (#584), which dies before doctest prints a summary line -- and `windows-msvc-vulkan` stays red on #514. The identical missing `AllowEmptyCollection` on `Assert-CrtPolicy` is #585 and is out of scope here. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- scripts/build-windows-release.ps1 | 109 ++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/scripts/build-windows-release.ps1 b/scripts/build-windows-release.ps1 index 7c339dbcf..f85e16586 100644 --- a/scripts/build-windows-release.ps1 +++ b/scripts/build-windows-release.ps1 @@ -32,6 +32,112 @@ function Invoke-Checked { } } +# `Arguments` is mandatory *and* `[AllowEmptyCollection()]` rather than defaulted +# to `@()`, so that an explicitly empty list binds while an omitted or null one +# stays a hard binding error. A default would silently turn "forwarded nothing" +# into "forwarded an empty list", which is the confusion #512 came from, so both +# halves of that design are pinned here. +function Invoke-CheckedBindingContractTests { + $recorder = { param([string]$Program, [string[]]$Arguments) return 0 } + + $nullRejected = $false + try { + Invoke-Checked "fake-null.exe" $null -Runner $recorder + } catch { + $nullRejected = $true + } + if (-not $nullRejected) { + throw "checked invocation bound a null argument list" + } + + # An omitted mandatory parameter *prompts* in an interactive console host, so + # asserting the omission in-process would hang a developer's terminal. An API + # runspace has a host that cannot prompt and reports the binding failure + # instead. The function under test is rebuilt from the live definition's own + # source text, so any edit to the real parameter block is what gets asserted. + $runspace = [powershell]::Create() + $omissionRejected = $false + try { + $null = $runspace.AddScript(@' +param([string]$Body) +Set-Item -LiteralPath function:Invoke-Checked -Value ([scriptblock]::Create($Body)) +Invoke-Checked "fake-omitted.exe" -Runner { param([string]$Program, [string[]]$Arguments) return 0 } +'@).AddArgument(${function:Invoke-Checked}.ToString()) + try { + $null = $runspace.Invoke() + } catch { + $omissionRejected = + $_.Exception.InnerException -is [System.Management.Automation.ParameterBindingException] + if (-not $omissionRejected) { throw } + } + $omissionRejected = $omissionRejected -or @($runspace.Streams.Error | Where-Object { + $_.Exception -is [System.Management.Automation.ParameterBindingException] + }).Count -gt 0 + } finally { + $runspace.Dispose() + } + if (-not $omissionRejected) { + throw "omitting the argument list was not a mandatory-parameter binding error" + } +} + +# The fake-runner arm below never executes `& $Program @Arguments`, so on its own +# it cannot catch an edit that stops propagating the child's exit status or stops +# forwarding argv. This arm drives the real branch end to end. +# +# The program it drives is the PowerShell host executing this script. That is the +# one executable guaranteed to exist wherever this script can run, so the same +# assertions execute on the Windows runners and on POSIX developer boxes with no +# platform branch that could silently no-op on one of them (#512). +function Invoke-CheckedRealProcessContractTests { + $pwshPath = (Get-Process -Id $PID).Path + if (-not $pwshPath) { + throw "real-process contract test could not resolve the running PowerShell host" + } + $scratch = Join-Path ([System.IO.Path]::GetTempPath()) ` + ("vllm-cpp-checked-" + [guid]::NewGuid().ToString("n")) + New-Item -ItemType Directory -Force -Path $scratch | Out-Null + try { + Invoke-Checked $pwshPath @("-NoProfile", "-Command", "exit 0") + + $nonzeroRejected = $false + try { + Invoke-Checked $pwshPath @("-NoProfile", "-Command", "exit 3") + } catch { + $nonzeroRejected = $true + if ($_.Exception.Message -notmatch 'exited with status 3$') { + throw "real-process failure did not report the child's own exit status: $($_.Exception.Message)" + } + } + if (-not $nonzeroRejected) { + throw "real-process nonzero exit status was accepted" + } + + # Exits 0 only for three *distinct* argv entries, the first of which holds + # a space: joining, re-quoting, truncating or reordering the forwarded + # list all land on a different exit status. + $argvProbe = Join-Path $scratch "argv-probe.ps1" + @' +if ($args.Count -ne 3) { exit 21 } +if ($args[0] -ne 'one two' -or $args[1] -ne 'three' -or $args[2] -ne 'four') { exit 22 } +exit 0 +'@ | Set-Content -LiteralPath $argvProbe -Encoding utf8NoBOM + Invoke-Checked $pwshPath @("-NoProfile", "-File", $argvProbe, "one two", "three", "four") + + # The production calls this branch exists for forward an explicitly empty + # list to a program that takes no arguments, so drive that shape for real + # rather than only through the fake runner (#512). + $emptyProbe = Join-Path $scratch "empty-probe.ps1" + @' +if ($args.Count -ne 0) { exit 23 } +exit 0 +'@ | Set-Content -LiteralPath $emptyProbe -Encoding utf8NoBOM + Invoke-Checked $emptyProbe @() + } finally { + Remove-Item -Recurse -Force -LiteralPath $scratch -ErrorAction SilentlyContinue + } +} + # Most of this script's checked invocations run a test executable that takes no # arguments, so `Invoke-Checked` must bind an explicitly empty argument list and # still forward it verbatim (#512). @@ -80,6 +186,9 @@ function Invoke-CheckedContractTests { throw "nonzero $rejectedName-argument exit status was accepted" } } + + Invoke-CheckedBindingContractTests + Invoke-CheckedRealProcessContractTests } function Assert-CrtPolicy {