ci: wait for validated SDK installer process tree - #136
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the hosted Windows SDK experiment to install the pinned WireSock SDK via winget install (instead of directly executing the downloaded GUI installer), so WinGet handles process-tree waiting and package registration; the README text is updated to match the new behavior.
Changes:
- Replace direct execution of the downloaded SDK installer with a
winget installinvocation using pinned--id+--exact+--version. - Update README documentation to describe “preflight then install via WinGet” rather than “verify then execute installer directly”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/Invoke-HostedSdkExperiment.ps1 | Switches the experiment’s SDK installation step to winget install while keeping the existing preflight checks. |
| README.md | Updates the Hosted SDK experiment description to reflect the new WinGet-orchestrated install flow. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/Invoke-HostedSdkExperiment.ps1:390
- $installedSdk is set to $true before the installer process is started and validated. If Start-Process fails or the installer exits non-zero (and the script throws), the finally block will still attempt
winget uninstall, producing misleading cleanup warnings and potentially masking the real failure. Set the flag only after the installer has completed successfully (exit code 0).
$installedSdk = $true
$installerProcess = Start-Process `
-FilePath $installers[0].FullName `
-ArgumentList @('/S', '/NCRC') `
-Wait `
|
Addressed the suppressed flag-ordering finding from Copilot review 4915415906 in commit cb3d3b3. The script now sets |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/Invoke-HostedSdkExperiment.ps1:384
Start-Process -Waitonly guarantees waiting for the process it starts; some installers/bootstrappers can spawn a detached child process and exit early. Since the next step immediately validates registry/file installation state, add a bounded poll forGet-WireSockSdkLibrariesso the workflow is resilient to delayed registration and avoids intermittent failures.
$installerProcess = Start-Process `
-FilePath $installers[0].FullName `
-ArgumentList @('/S', '/NCRC') `
-Wait `
-PassThru
README.md:125
- This README sentence says the workflow “waits for that validated installer’s full process tree”, but the implementation only uses
Start-Process -Waitplus post-install checks. Consider rephrasing to avoid overpromising process-tree semantics and instead describe the observable guarantee (waits for installation completion / expected artifacts).
The manually dispatched **Hosted WireSock SDK experiment** is an isolated x64 feasibility check and does not replace the protected self-hosted runner policy. It runs only from the current protected `main` tip on a disposable GitHub-hosted Windows VM, bootstraps WinGet when necessary, downloads the exact `NTKERNEL.WireSockVPNClientCLI` SDK version, verifies the installer against the audited WinGet SHA-256 and Authenticode signature, waits for that validated installer's full process tree, builds and installation-tests an unsigned candidate MSI, and exercises the real SDK lifecycle with synthetic profiles restricted to IANA documentation networks. The experiment uses no VPN credentials or repository secrets and uninstalls the SDK before the VM is discarded. Run it from **Actions → Hosted WireSock SDK experiment → Run workflow**; promote this design to x86/ARM64 or release gating only after the x64 driver, routing, and cleanup behavior succeeds consistently.
|
Addressed both suppressed findings from Copilot review 4915483452 in commit d15cf05. The experiment now performs a bounded 120-second poll for registered |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
scripts/Invoke-HostedSdkExperiment.ps1:420
- Some Windows installers legitimately return non-zero success codes (commonly 3010 for 'success, reboot required'). Treating any non-zero as a hard failure can make the workflow flaky or fail after a successful install. Consider allowing known success exit codes (e.g., 0 and 3010) or at least mapping 3010 to a warning with continued execution.
$installerProcess = Start-Process `
-FilePath $installers[0].FullName `
-ArgumentList @('/S', '/NCRC') `
-Wait `
-PassThru
if ($installerProcess.ExitCode -ne 0) {
throw (
"Installing $packageId $packageVersion failed with exit code " +
"$($installerProcess.ExitCode).")
}
scripts/Invoke-HostedSdkExperiment.ps1:203
- This error is actionable but could be more diagnostic for CI failures. Consider including the poll interval and (if applicable) the expected discovery mechanism/location (e.g., registry key/path used by
Get-WireSockSdkLibraries) so it's easier to distinguish 'installer still running' vs 'registration failed'.
throw (
"The SDK installer registered no wgbooster.dll candidate within " +
"$TimeoutSeconds seconds.")
scripts/Invoke-HostedSdkExperiment.ps1:195
- The
do { ... } while ($true)+breakcontrol flow is harder to scan than a bounded loop. Consider rewriting as a time-boundedwhile ($stopwatch.Elapsed.TotalSeconds -lt $TimeoutSeconds)loop and throwing after the loop—same behavior, simpler control flow.
do {
$libraries = @(Get-WireSockSdkLibraries)
if ($libraries.Count -gt 0) {
return $libraries
}
if ($stopwatch.Elapsed.TotalSeconds -ge $TimeoutSeconds) {
break
}
Start-Sleep -Milliseconds $PollIntervalMilliseconds
} while ($true)
|
Addressed all three suppressed findings from Copilot review 4915528721 in commit bc3ead2. Installer exit code 3010 is now treated as success-with-restart and emits a warning; other nonzero codes still fail. The SDK readiness loop is an explicitly time-bounded |
Follow-up to hosted SDK experiment run https://github.com/wiresock/WireSockUI/actions/runs/31582893455. The audited installer download and signature/hash preflight succeeded, but ordinary PowerShell invocation waited only for the bootstrapper: the executable remained locked, no wgbooster.dll registration existed yet, and uninstall found no package. This change executes the exact downloaded and validated installer with Start-Process -Wait, which waits for the process and its descendants, and enforces the installer exit code. It avoids a second download, preserving the guarantee that the bytes verified are the bytes executed. Local validation: PowerShell parser and installer invocation contract; workflow security fixtures; production workflow contracts (7 workflows / 63 action references); git diff check.