fix(windows): align install.ps1 MSI with per-machine scope (#913)

[M3gA-Mind]

Summary

• Remove MSIINSTALLPERUSER / ALLUSERS=2 from msiexec so silent install matches the Tauri WiX package (InstallScope="perMachine"), addressing MSI 1603 failures from install.ps1 (see Windows installer exits with MSI error 1603 during OpenHuman install #913).
• Start elevated msiexec via -Verb RunAs when the shell is not already admin so UAC can approve a machine-wide install (still works with irm … | iex).
• Extract script-scoped helpers and a dot-source guard for unit tests.
• Add scripts/tests/OpenHumanWindowsInstall.Tests.ps1, pnpm test:install-ps1, and enable the Windows job in installer-smoke.yml.

Problem

GitHub #913: install.ps1 verified the MSI download and SHA256, then msiexec exited 1603. The script passed per-user public properties while the shipped MSI is authored per-machine (Program Files, HKLM), which commonly produces a fatal install.

Solution

• Use a fixed argument list: /i <msi> /qn /norestart with no conflicting ALLUSERS/MSIINSTALLPERUSER overrides.
• Request elevation only for the msiexec child process when needed.
• PowerShell tests assert the MSI argument contract and asset selection; CI runs them on windows-latest.

Submission Checklist

• Tests added or updated — PowerShell tests for MSI arg contract and release asset selection.
• Diff coverage ≥ 80% — N/A: PowerShell + workflow only (no Vitest/Rust diff-cover surface).
• Coverage matrix — N/A.
• Feature IDs in matrix — N/A.
• No new external network dependencies — same GitHub release API as before for real installs.
• Manual smoke checklist — N/A.
• Linked issue — see Related.

Impact

• Windows users: one UAC prompt when installing the MSI without an elevated shell.
• CI: installer-smoke runs Windows tests + install.ps1 -DryRun.

Related

Closes #913

Summary by CodeRabbit

• New Features

  • Installer now performs elevation-aware Windows installs and adapts how it launches the installer when admin rights are needed.

• Tests

  • Added a local Windows installer test suite validating asset selection and installer arguments.
  • Added automated Windows smoke tests in CI to run the installer tests and a dry-run.

• Chores

  • Installer version bumped to 1.1.0.
  • Added a convenience script entry to run the Windows installer tests.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds Windows-specific installer helpers and elevation-aware install flow to the PowerShell installer, a PowerShell test runner validating those helpers, CI updates to run the tests then perform an installer dry-run, and bumps the reported installer version to 1.1.0.

Changes

Windows installer refactor & CI test

Layer / File(s)	Summary
Helper functions scripts/install.ps1	Adds Get-OpenHumanMsiexecInstallArgumentList, Test-OpenHumanWindowsProcessElevated, and Select-OpenHumanWindowsAssetFromRelease.
Core installation flow scripts/install.ps1	Install logic now uses the new helpers, removes injected MSIINSTALLPERUSER/ALLUSERS, prints JSON arg lists on -DryRun, performs elevation-aware msiexec invocation (direct vs -Verb RunAs), updates displayed version to 1.1.0, and guards against dot-sourcing auto-run.
Tests scripts/tests/OpenHumanWindowsInstall.Tests.ps1	New PowerShell test runner with Assert-Equal/Assert-True, tests for msiexec arg shape, asset selection (MSI preferred, EXE fallback, null), and elevation detection; exits non-zero on failures.
CI / Tooling .github/workflows/installer-smoke.yml, package.json	Adds smoke-windows job that runs the PowerShell tests then installer dry-run on windows-latest; adds test:install-ps1 npm script to run the test file.

Sequence Diagram(s)

sequenceDiagram
  participant GH_Actions as "GitHub Actions\n(installer-smoke)"
  participant PWsh as "PowerShell Tests\n(OpenHumanWindowsInstall.Tests.ps1)"
  participant Installer as "Install Script\n(scripts/install.ps1)"
  participant GH_API as "GitHub Releases API"
  participant MSI as "msiexec / UAC"

  GH_Actions->>PWsh: checkout repo\nrun test script
  PWsh->>Installer: dot-source helpers\nrun unit tests
  PWsh-->>GH_Actions: exit 0/1
  GH_Actions->>Installer: run install.ps1 -DryRun
  Installer->>GH_API: query latest release assets
  GH_API-->>Installer: return assets
  Installer->>Installer: Select asset & build msiexec args
  alt not elevated
    Installer->>MSI: display RunAs instruction (DryRun) / start msiexec with -Verb RunAs
  else elevated
    Installer->>MSI: display direct msiexec args (DryRun) / start msiexec directly
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix(windows): fix install script and add Windows build workflow #281 — Modifies scripts/install.ps1 and related installer execution/asset selection logic.

Poem

🐰
I hopped through scripts at break of day,
Built args and checks to guide the way.
Tests first, then dry-run — UAC in view,
Version bumped, installer steps made true.
A tiny rabbit cheers: "Install anew!" 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main fix: aligning install.ps1 MSI with per-machine scope, which directly addresses the core issue being resolved.
Linked Issues check	✅ Passed	The pull request directly addresses issue #913 by removing per-user MSI properties, using fixed per-machine msiexec arguments, adding elevation handling, extracting helpers, and adding tests and CI coverage as required.
Out of Scope Changes check	✅ Passed	All changes are in scope: helper functions for MSI args and asset selection, elevation detection, PowerShell tests, workflow updates, and a package.json script entry, all directly supporting the per-machine MSI fix and its verification.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/install.ps1`:
- Around line 202-207: The dry-run branch always prints a "(non-admin)
Start-Process ... -Verb RunAs" message even when the process is elevated; modify
the DryRun handling in the if ($DryRun) block so it calls
Test-OpenHumanWindowsProcessElevated and conditionally prints either the
non-admin RunAs message or an "(already elevated) Start-Process msiexec -Wait
-ArgumentList <above>" message accordingly; locate the DryRun branch that uses
Get-OpenHumanMsiexecInstallArgumentList and update it to branch on
Test-OpenHumanWindowsProcessElevated to emit the correct hint for elevated vs
non-elevated execution.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e5ec95f-e37d-474c-bd24-c661da4db214

📥 Commits

Reviewing files that changed from the base of the PR and between 995e5cc and f5e8988.

📒 Files selected for processing (4)

• .github/workflows/installer-smoke.yml
• package.json
• scripts/install.ps1
• scripts/tests/OpenHumanWindowsInstall.Tests.ps1

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/install.ps1`:
- Around line 23-33: The Get-OpenHumanMsiexecInstallArgumentList function
currently returns the raw $MsiPath as an array element which can be split if it
contains spaces; update the function so the MSI path element is properly quoted
(e.g., wrap $MsiPath in double quotes) before returning the ArgumentList array
so Start-Process will pass the full path as a single argument to msiexec; keep
the rest of the returned switches (/i, '/qn', '/norestart') unchanged and ensure
the quoted path is the second array element.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 91ff2301-fa61-4273-90df-a86fd8d80524

📥 Commits

Reviewing files that changed from the base of the PR and between f5e8988 and cc77aaa.

📒 Files selected for processing (1)

• scripts/install.ps1

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the full context of the install.ps1 file
head -250 scripts/install.ps1 | tail -+1

Repository: tinyhumansai/openhuman

Length of output: 134

---

🏁 Script executed:

# Also search for where this helper is being called
rg "Get-OpenHumanMsiexecInstallArgumentList" scripts/install.ps1 -A 5 -B 2

Repository: tinyhumansai/openhuman

Length of output: 1212

---

🌐 Web query:

Start-Process -ArgumentList PowerShell spaces quoting behavior

💡 Result:

PowerShell's Start-Process cmdlet with -ArgumentList has a longstanding bug where it joins array elements with spaces without automatically quoting arguments containing spaces or special characters. This causes arguments with spaces to be split incorrectly by the target process. Key behavior from official docs: - -ArgumentList accepts a string array or single string. - Array elements are joined into a single string separated by single spaces. - No automatic quoting: arguments with spaces or quotes must have embedded escaped double quotes (e.g., ""path with spaces""). - Best practice: Use a single string for all arguments with proper embedded quoting. Examples: Single string (recommended): Start-Process cmd.exe -ArgumentList "/c dir "C:\Program Files"" Array (requires manual quoting): Start-Process cmd.exe -ArgumentList "/c", "dir", ""C:\Program Files"" This bug (#5576) remains open as of 2025; won't be fixed for backward compatibility. Workaround: manual embedded quoting or direct invocation (& exe args) where PowerShell auto-quotes. For paths/files with spaces, always embed quotes like '"path with spaces"' in the argument string/array element.

Citations:

• 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.management/start-process?view=powershell-7.5
• 2: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.management/start-process?view=powershell-7.6
• 3: Start-Process doesn't pass arguments with embedded whitespace and double quotes correctly PowerShell/PowerShell#5576
• 4: https://github.com/MicrosoftDocs/PowerShell-Docs/blob/main/reference/7.4/Microsoft.PowerShell.Management/Start-Process.md
• 5: https://stackoverflow.com/questions/75048543/pass-arguments-to-start-process-with-spaces
• 6: https://stackoverflow.com/questions/79388953/running-a-powershell-start-process-command-with-arguments-containing-spaces-fr
• 7: https://stackoverflow.com/questions/67217049/passing-an-argument-list-with-spaces-in-powershell
• 8: https://stackoverflow.com/questions/70479960/how-to-pass-an-array-to-the-arguments-in-start-process-in-powershell
• 9: https://serverfault.com/questions/216790/pass-quoted-argument-string-to-start-process-in-powershell

---

🏁 Script executed:

# Check where $tmpFile is defined and used
rg '\$tmpFile' scripts/install.ps1 -B 2 -A 2 | head -60

Repository: tinyhumansai/openhuman

Length of output: 1692

---

🏁 Script executed:

# Check the full context of the MSI invocation to ensure quoting is complete
sed -n '195,230p' scripts/install.ps1

Repository: tinyhumansai/openhuman

Length of output: 1588

---

🏁 Script executed:

# Let's verify the proposed fix syntax is valid PowerShell
# and check if there are any other arguments that need quoting
cat > /tmp/test_quote.ps1 << 'EOF'
# Test the proposed quoting syntax
$MsiPath = "C:\Users\Test User\AppData\Local\Temp\OpenHuman.msi"

# Current implementation (WRONG)
$currentImpl = @('/i', $MsiPath, '/qn', '/norestart')
Write-Host "Current implementation:"
Write-Host ($currentImpl | ConvertTo-Json -Compress)
Write-Host "Joined: $($currentImpl -join ' ')"
Write-Host ""

# Proposed implementation (CORRECT)
$proposedImpl = @('/i', ('"{0}"' -f $MsiPath), '/qn', '/norestart')
Write-Host "Proposed implementation:"
Write-Host ($proposedImpl | ConvertTo-Json -Compress)
Write-Host "Joined: $($proposedImpl -join ' ')"
EOF
pwsh -NoProfile -File /tmp/test_quote.ps1

Repository: tinyhumansai/openhuman

Length of output: 450

---

🏁 Script executed:

# Check if there are any other MSI-related invocations that might need review
rg "msiexec" scripts/install.ps1 -i

Repository: tinyhumansai/openhuman

Length of output: 1037

---

Quote the MSI path to handle spaces in the temp directory.

Line 32 currently returns the raw MSI path as an array element to -ArgumentList. PowerShell's Start-Process joins array elements with spaces without automatic quoting, so the path will be incorrectly split if $env:TEMP contains spaces (common in Windows profiles with user names like "User Name"). This causes msiexec to receive multiple arguments instead of one quoted path, breaking silent installs.

Suggested fix

 function Get-OpenHumanMsiexecInstallArgumentList {
   <#
   .SYNOPSIS
     Argument list for Start-Process msiexec.exe (no per-user MSI overrides).
   #>
   param(
     [Parameter(Mandatory = $true)]
     [string]$MsiPath
   )
-  return @('/i', $MsiPath, '/qn', '/norestart')
+  return @('/i', ('"{0}"' -f $MsiPath), '/qn', '/norestart')
 }

🧰 Tools

🪛 PSScriptAnalyzer (1.25.0)

[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'

(PSUseBOMForUnicodeEncodedFile)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/install.ps1` around lines 23 - 33, The
Get-OpenHumanMsiexecInstallArgumentList function currently returns the raw
$MsiPath as an array element which can be split if it contains spaces; update
the function so the MSI path element is properly quoted (e.g., wrap $MsiPath in
double quotes) before returning the ArgumentList array so Start-Process will
pass the full path as a single argument to msiexec; keep the rest of the
returned switches (/i, '/qn', '/norestart') unchanged and ensure the quoted path
is the second array element.

[coderabbitai]

♻️ Duplicate comments (1)

scripts/install.ps1 (1)

32-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Quote the MSI path token before passing it to Start-Process -ArgumentList.

At Line 35, returning raw $MsiPath can break installs when the temp path contains spaces, because Start-Process argument-list joining may not preserve it as one native argv token for msiexec. This is the root cause for intermittent path-based failures.

Suggested fix

-  return @('/i', $MsiPath, '/qn', '/norestart')
+  return @('/i', ('"{0}"' -f $MsiPath), '/qn', '/norestart')

In PowerShell 7.x, when using Start-Process -ArgumentList with a string array, are elements containing spaces automatically quoted for the native process, or must embedded quotes be supplied manually?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/install.ps1` around lines 32 - 35, The returned argument array
currently returns the raw $MsiPath token which can be split when the temp path
contains spaces; update the array construction in scripts/install.ps1 so the MSI
path element is explicitly quoted before being returned (ensure the element
passed to Start-Process -ArgumentList is a single quoted string token containing
the full path), e.g., wrap $MsiPath in quotes or otherwise escape it so
Start-Process receives one native argv for msiexec; keep the other tokens
('/i','/qn','/norestart') unchanged and verify the change is used by the code
that calls Start-Process -ArgumentList.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@scripts/install.ps1`:
- Around line 32-35: The returned argument array currently returns the raw
$MsiPath token which can be split when the temp path contains spaces; update the
array construction in scripts/install.ps1 so the MSI path element is explicitly
quoted before being returned (ensure the element passed to Start-Process
-ArgumentList is a single quoted string token containing the full path), e.g.,
wrap $MsiPath in quotes or otherwise escape it so Start-Process receives one
native argv for msiexec; keep the other tokens ('/i','/qn','/norestart')
unchanged and verify the change is used by the code that calls Start-Process
-ArgumentList.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f78e47c7-9b2d-4d8b-9c5e-e1a192253d2c

📥 Commits

Reviewing files that changed from the base of the PR and between cc77aaa and 73aee04.

📒 Files selected for processing (2)

• scripts/install.ps1
• scripts/tests/OpenHumanWindowsInstall.Tests.ps1