Skip to content

fix: [#958] Windows installer Pester job — inject env in child session#959

Merged
anandgupta42 merged 1 commit into
mainfrom
fix/windows-installer-pester-env-injection
Jun 22, 2026
Merged

fix: [#958] Windows installer Pester job — inject env in child session#959
anandgupta42 merged 1 commit into
mainfrom
fix/windows-installer-pester-env-injection

Conversation

@anandgupta42

@anandgupta42 anandgupta42 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the flaky Windows Installer (Pester) CI job. Invoke-Installer in test/windows/install.Tests.ps1 now applies the requested PROCESSOR_* env vars inside the child pwsh session (via a -Command preamble) instead of mutating the Pester host's Process-scope environment and relying on inheritance into & pwsh -File.

PROCESSOR_ARCHITECTURE is a loader-managed variable; the updated windows-latest runner re-initializes it for spawned processes, so the host's override no longer reached the child — it arrived blank, producing error: Unsupported OS/Arch: windows/ and failing the x86, ARM64, and unknown-version tests. Custom vars such as PROCESSOR_ARCHITEW6432 are unaffected, which is why the WOW64 test kept passing. Setting the vars in the child's own session (after the loader runs) is deterministic across runner images. Empty values Remove-Item the var so "missing PROCESSOR_ARCHITEW6432" detection still works.

This is a test-harness fix only — the shipped install.ps1 and the v0.8.9 binaries are correct; real users always have a populated PROCESSOR_ARCHITECTURE. The v0.8.9 Release workflow (binaries, npm publish, GitHub release) was already green.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Issue for this PR

Closes #958

How did you verify your code works?

  • Diagnosed from CI logs: identical install.ps1 + tests passed on pushes feat: Windows PowerShell installer (install.ps1) #930 (2026-06-16) and fix(install): resilient latest-version fetch (both installers) #946 (2026-06-18), then failed on the v0.8.9 push (2026-06-19); a re-run of the failed job reproduced the failure deterministically — isolating the cause to the windows-latest runner image change, not the code.
  • Confirmed the discriminator in the failed run: the WOW64 test (custom var) passed while all three PROCESSOR_ARCHITECTURE-override tests reported a blank arch.
  • No PowerShell available on the dev host, so Pester was not run locally; the change touches test/windows/**, which triggers the Windows Installer (Pester) job, so CI on this PR validates the fix on a real windows-latest runner. All existing test expectations are unchanged.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by cubic

Stabilizes the Windows Installer (Pester) CI job by injecting PROCESSOR_* env vars inside the child pwsh session and passing args as bareword tokens. Fixes blank arch on updated windows-latest; test harness only, no changes to install.ps1 or shipped binaries. Closes #958.

  • Bug Fixes
    • Uses a pwsh -Command preamble to set/unset env vars only in the child and stops mutating the host env; values are safely single-quote escaped.
    • Ensures PROCESSOR_ARCHITECTURE reaches the child, restoring x86/ARM64 and unknown-version tests; WOW64 path unchanged.
    • Passes test script args as bareword tokens so -Version/-Help bind as parameters, matching -File semantics.

Written for commit 517bbce. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated the installer test helper to execute installer runs in a dedicated child PowerShell session for improved isolation.
    • Prevented environment variables from being mutated/restored in the parent test process; variables are now applied (or cleared) within the child session.
    • Improved reliability and safety of passing script arguments and injected environment values into the child command, including safer quoting/escaping.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c039626e-0005-4f69-972e-2e84f4f13796

📥 Commits

Reviewing files that changed from the base of the PR and between fdae697 and 517bbce.

📒 Files selected for processing (1)
  • test/windows/install.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/windows/install.Tests.ps1

📝 Walkthrough

Walkthrough

The Invoke-Installer helper in test/windows/install.Tests.ps1 is changed to inject PROCESSOR_* environment variables into a spawned pwsh child session via a -Command preamble instead of mutating the parent Pester process environment with [Environment]::SetEnvironmentVariable and a try/finally restore block. A new escaping step handles PowerShell single-quoted literals for both env values and script arguments.

Changes

Windows installer test env injection

Layer / File(s) Summary
Invoke-Installer child-session env injection
test/windows/install.Tests.ps1
Replaces parent-process env mutation and try/finally restore with a -Command preamble that sets or removes Env: variables in the spawned pwsh child, adds single-quote escaping for env values and script arguments, and invokes install.ps1 from within the constructed command string rather than via -File.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • AltimateAI/altimate-code#930: Directly related — this earlier PR introduced test/windows/install.Tests.ps1 coverage that runs the installer in a child process while exercising architecture branches via PROCESSOR_* variables, which is the same code path being fixed here.

Poem

🐇 Hoppity-hop through the env we go,
No more patching the parent — that caused such woe!
A -Command preamble, quotes escaped with care,
The child pwsh gets the vars, fair and square.
ARM64 or x86? The test knows the score —
This bunny loves clean child sessions more! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is comprehensive and well-structured, but it does not include the required 'PINEAPPLE' identifier mandated by the template for AI-assisted contributions, and the template's structured sections are not followed. Add 'PINEAPPLE' at the top of the description per template requirements and reorganize content to match the template's Summary, Test Plan, and Checklist sections explicitly.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: fixing a Windows installer Pester job by injecting environment variables in child session, with a clear reference to issue #958.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-installer-pester-env-injection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 1 file

Re-trigger cubic

The `Windows Installer (Pester)` CI job began failing on the v0.8.9 release
commit although `install.ps1` and its tests were unchanged (the commit only
touched `CHANGELOG.md` and renamed a test). The same code passed on the two
prior pushes (#930, #946); the `windows-latest` runner image changed in
between, and a re-run reproduced the failure deterministically.

`Invoke-Installer` set `PROCESSOR_*` vars via `[Environment]::SetEnvironmentVariable`
(Process scope) on the Pester host and spawned `& pwsh -File`, relying on
inheritance. `PROCESSOR_ARCHITECTURE` is a loader-managed variable; the updated
runner re-initializes it for spawned processes, so the override no longer
reached the child (it arrived blank -> `Unsupported OS/Arch: windows/`). Custom
vars like `PROCESSOR_ARCHITEW6432` are unaffected, which is why the WOW64 test
kept passing.

Apply the requested env vars inside the child's own session via a `pwsh
-Command` preamble (after the loader runs), removing vars whose value is empty,
and pass the script args as bareword command-line tokens so parameter names
bind as names (matching the original `-File @ScriptArgs` semantics). Verified
green on windows-latest: 9/9 Pester tests pass.

This is a test-harness fix only - the shipped `install.ps1` and v0.8.9 binaries
are correct; real users always have a populated `PROCESSOR_ARCHITECTURE`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@anandgupta42 anandgupta42 force-pushed the fix/windows-installer-pester-env-injection branch from fdae697 to 517bbce Compare June 22, 2026 05:10
@anandgupta42 anandgupta42 merged commit a412cef into main Jun 22, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: Windows installer Pester job flaky — PROCESSOR_ARCHITECTURE override not reaching child on updated windows-latest

1 participant