Bind release.ps1 parameters by name from the release workflow - #431
Merged
Merged
Conversation
The packaging step built its arguments as an array and splatted it. PowerShell array splatting passes elements positionally, so -Tag itself bound to -Tag and the tag bound to -OutputDirectory. release.ps1 has several optional [string] parameters, so the whole vector was absorbed silently instead of failing on an unexpected argument, and the run died later with release tag does not name a publishable version: '-Tag' Observed on a real workflow_dispatch of v0.1.74, which was the first end-to-end execution of this workflow. Switch to a hashtable so the parameters bind by name. Contract 8 could not catch this: it asserts the workflow's YAML text mentions release.ps1, not that its invocation reaches the script correctly. Add contract 9, which extracts the packaging step's own run block and executes it against a probe mirroring release.ps1's parameter block, asserting -Tag, -OutputDirectory and both publishing switches arrive as intended. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
coneilen
force-pushed
the
fix-release-workflow-splatting
branch
from
September 23, 2026 07:36
43349cf to
35d8f38
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
.github/workflows/windows-release.ymlbuilt its arguments torelease.ps1as an array and splatted it:PowerShell array splatting passes elements positionally. So
-Tagitself bound to-Tag, and the actual tag bound to-OutputDirectory. Becauserelease.ps1declares several optional[string]parameters, the whole vector was absorbed silently rather than failing on an unexpected argument, and the run died later atrelease.ps1:49:Reproduced locally, exactly:
How it was found
By dispatching the workflow for the first time (
workflow_dispatch,v0.1.74,publish: false) — run 35830173241. #425 noted this end-to-end path was the one thing its suite could not prove; it was right, and the very first execution failed. Steps 1-6 passed, step 7 failed.No release was touched:
publishandallow_unsigned_publishboth defaulted to false.Why the existing tests missed it
Contract 8 asserts the workflow's YAML text references
Tools/windows/release.ps1, along with trigger shape, pinned action SHAs, and input defaults. All still true with the bug present. The suite's ownInvoke-Releasehelper calls the script with correctly-formed named arguments, so it exercisedrelease.ps1thoroughly while never exercising the workflow's call into it.That is a test that runs and passes while proving nothing about the path it appears to cover.
The fix
Bind by name with a hashtable, with a comment recording why.
New contract 9
Extracts the packaging step's own run block from the workflow (indentation-aware, so it terminates at the next step like a real YAML block scalar), redirects the script path at a probe mirroring
release.ps1's parameter block — including the optional[string]parameters that let a positional splat bind silently — and asserts-Tag,-OutputDirectory, and both publishing switches arrive as intended. Covers thepublish: falseandpublish: true+allow_unsigned_publish: truecases.It runs the workflow's real lines rather than a copy, so the two cannot drift apart.
Evidence
RED before the fix, reproducing CI's failure locally with no build:
GREEN after: 9/9.
Mutation-tested, all caught:
PublishforwardingAllowUnsignedPublishforwardingScope
Ledger rows
Install progressandRelaunch promptstay Blocked — unchanged and unrelated. No certificate was generated, obtained, or committed. No file undergraphcode-windows/src/touched.The workflow still has not completed end-to-end; this fixes the first failure that attempt exposed. I will re-dispatch once this lands and report what the full pinned build does.
TDD evidence
RED: pwsh -File Tools/windows/Tests/Release.Tests.ps1 -> fails with "the release workflow bound the wrong value to -Tag: '-Tag' (array splatting passes elements positionally; use a hashtable)", reproducing the dispatched run's release.ps1:49 failure locally with no build
GREEN: pwsh -File Tools/windows/Tests/Release.Tests.ps1 -> 9/9 PASS, ending "Release workflow invocation binding: PASS", after switching the packaging step to hashtable splatting
REGRESSION: 5 mutations of the packaging step -> all 5 caught (revert to array splatting; drop Publish forwarding; drop AllowUnsignedPublish forwarding; hardcode a wrong tag; wrong output directory), and the suite returns to 9/9 PASS once the workflow is restored