Skip to content

ci: wait for validated SDK installer process tree - #136

Merged
wiresock merged 5 commits into
mainfrom
codex/hosted-sdk-install-wait
Aug 12, 2026
Merged

ci: wait for validated SDK installer process tree#136
wiresock merged 5 commits into
mainfrom
codex/hosted-sdk-install-wait

Conversation

@wiresock

@wiresock wiresock commented Aug 12, 2026

Copy link
Copy Markdown
Owner

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 install invocation 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.

Comment thread scripts/Invoke-HostedSdkExperiment.ps1 Outdated
@wiresock wiresock changed the title ci: let winget orchestrate SDK installation ci: wait for validated SDK installer process tree Aug 12, 2026
@wiresock
wiresock requested a lite review from Copilot August 12, 2026 10:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `

@wiresock

Copy link
Copy Markdown
Owner Author

Addressed the suppressed flag-ordering finding from Copilot review 4915415906 in commit cb3d3b3. The script now sets $installedSdk = $true only after Start-Process -Wait completes and the validated installer returns exit code 0, so launch or installer failures do not trigger a misleading WinGet uninstall attempt. Verified with a parser check, an explicit launch → exit-check → installed-flag ordering assertion, workflow security fixtures, and production workflow contracts.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 -Wait only 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 for Get-WireSockSdkLibraries so 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 -Wait plus 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.

@wiresock

Copy link
Copy Markdown
Owner Author

Addressed both suppressed findings from Copilot review 4915483452 in commit d15cf05. The experiment now performs a bounded 120-second poll for registered wgbooster.dll candidates after the validated installer exits successfully, making delayed registry/file publication observable and deterministic. The README now promises the observable behavior—installation completion and expected signed SDK artifacts—rather than process-tree semantics. Validation covers delayed readiness on the third poll, bounded timeout when readiness never appears, PowerShell parsing, workflow security fixtures, and production workflow contracts.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) + break control flow is harder to scan than a bounded loop. Consider rewriting as a time-bounded while ($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)

@wiresock

Copy link
Copy Markdown
Owner Author

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 while, and its timeout now reports duration, poll interval, 32/64-bit HKLM registry paths, InstallLocation, and expected root/sdk/bin library locations. Executable validation covers exit codes 0, 3010, and 1; delayed registration; bounded timeout; diagnostic contents; workflow fixtures; and production contracts.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@wiresock
wiresock merged commit f9659bb into main Aug 12, 2026
24 checks passed
@wiresock
wiresock deleted the codex/hosted-sdk-install-wait branch August 12, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants