From f4156071568db7327c3cbf4b887d0db3eb920da0 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 06:21:20 +0000 Subject: [PATCH] test(windows): pin the real-process branch and the Arguments binding contract (#512) FOLLOWING_AGENTS_PROTOCOL Re-applied from row/ENG-RELEASE-WINDOWS-EMPTY-ARGS-FIX @ b00d6199f onto current main. That branch was stacked on #583, and squash-merging #583 orphaned it -- mergeable=CONFLICTING -- because main now carries one squashed commit where the child's history expects the originals. Force-push is forbidden, so the delta is re-applied here instead of rewriting the branch. The resulting file is BYTE-IDENTICAL to the verified content: git diff b00d6199f on this path is empty. Pins two properties #583 left unguarded. A fresh review found four mutations surviving its contract suite -- most importantly $exitCode = 0 in the real-process branch, and dropping the @ splat. Either means a future edit passes the contract suite SILENTLY while the Windows gate reports success for tests that failed or never ran. That is the failure class this repo keeps paying for, and it is the branch #583 restructured and the branch that surfaced #584. All four now RED: real-branch $exitCode = 0; dropped @ splat; dropped Mandatory; added AllowNull. The seven already caught stay caught, including the one that distinguishes "forwarded an empty array" from "forwarded nothing" -- @($null) has Count 1 and would otherwise sneak past. Two findings worth keeping. A naive omission test would HANG A DEVELOPER'S TERMINAL: an omitted mandatory parameter PROMPTS under an interactive console host, blocking on "Arguments[0]:" until killed at 25 s under a pty, and only reaches the binding error in CI where stdin is not a tty -- a test meaning one thing in CI and another on a desk. It is asserted in an API runspace instead, whose host cannot prompt, and the function under test is rebuilt from the live definition's own source text so it tracks edits to the real parameter block. And the @-splat mutation is nearly an equivalent mutant, disclosed rather than oversold: for a NATIVE executable the splatted and unsplatted forms give identical argv at 0, 1 and 3 elements under all three $PSNativeCommandArgumentPassing modes including Windows. It is only observable when the program is a PowerShell script, where an empty list otherwise arrives as one array argument instead of zero -- which is exactly #512's contract. The real-process arm drives the PowerShell host executing the script, resolved from (Get-Process -Id $PID).Path: the one executable guaranteed to exist wherever the script can run, so the IDENTICAL arm runs on Windows runners and POSIX boxes with no platform branch. An arm that silently no-ops on one platform would be its own version of the bug being fixed. Operator gate: contract suite rc=0 on this tree, and the previously-surviving real-branch mutation re-applied by hand fails with "real-process nonzero exit status was accepted", tree restored byte-for-byte afterwards. Still not green and not claimed to be: windows-msvc-cpu needs #584, vulkan needs #514, #585 remains filed. 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 {