fix(cli): add fallback arch detection in PowerShell installer#529
fix(cli): add fallback arch detection in PowerShell installer#529
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the PowerShell CLI installer by introducing a more resilient architecture detection mechanism. It addresses an issue where the primary .NET "RuntimeInformation" API might fail on certain Windows systems, ensuring that the installer can reliably determine the system's architecture using a fallback to environment variables, thereby improving compatibility and user experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PowerShell installer now attempts to read RuntimeInformation::OSArchitecture inside a try/catch and falls back to PROCESSOR_ARCHITECTURE when unavailable, mapping detected values to amd64/arm64 and emitting explicit errors for unsupported architectures. Several compose and env files add MEM0_TELEMETRY=false. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Invoker
participant Script as install.ps1
participant Runtime as RuntimeInformation
participant Env as PROCESSOR_ARCHITECTURE
rect rgba(200,220,255,0.5)
User->>Script: invoke installer
Script->>Runtime: try read OSArchitecture
alt Runtime returns value
Runtime-->>Script: OSArchitecture (X64/Arm64/other)
Script->>Script: map to amd64/arm64 or error
else Runtime unavailable or null
Script->>Env: read PROCESSOR_ARCHITECTURE
Env-->>Script: AMD64/ARM64/other
Script->>Script: map to amd64/arm64 or error
end
Script->>User: proceed with download/install or error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Code Review
The pull request successfully adds a fallback mechanism for architecture detection in the PowerShell installer, which is a great improvement for compatibility. I've suggested a refactoring of the detection logic to make it more concise and maintainable by reducing code duplication.
| $OsArch = $null | ||
| try { | ||
| $OsArch = [System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture | ||
| } catch { | ||
| # Type not available — fall through to env var detection. | ||
| } | ||
|
|
||
| if ($null -ne $OsArch) { | ||
| $WinArch = switch ($OsArch) { | ||
| ([System.Runtime.InteropServices.Architecture]::X64) { "amd64" } | ||
| ([System.Runtime.InteropServices.Architecture]::Arm64) { "arm64" } | ||
| default { Write-Error "Unsupported architecture: $OsArch"; exit 1 } | ||
| } | ||
| } else { | ||
| $WinArch = switch ($env:PROCESSOR_ARCHITECTURE) { | ||
| "AMD64" { "amd64" } | ||
| "ARM64" { "arm64" } | ||
| default { Write-Error "Unsupported architecture: $env:PROCESSOR_ARCHITECTURE"; exit 1 } | ||
| } | ||
| } |
There was a problem hiding this comment.
While the logic is correct, the architecture detection can be simplified to reduce code duplication. You can determine the architecture source first (either from .NET RuntimeInformation or the environment variable), normalize the resulting string, and then use a single, simpler switch statement to determine the $WinArch. This makes the code more concise and easier to maintain.
$DetectedArchStr = $null
try {
$arch = [System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture
if ($null -ne $arch) {
$DetectedArchStr = $arch.ToString()
}
} catch {
# Type not available, fall through to env var.
}
if (-not $DetectedArchStr) {
$DetectedArchStr = $env:PROCESSOR_ARCHITECTURE
}
# Normalize architecture string (e.g. "X64" -> "AMD64")
$NormalizedArch = $DetectedArchStr.ToUpper() -replace "X64", "AMD64"
$WinArch = switch ($NormalizedArch) {
"AMD64" { "amd64" }
"ARM64" { "arm64" }
default { Write-Error "Unsupported architecture: $DetectedArchStr"; exit 1 }
}
There was a problem hiding this comment.
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 `@cli/scripts/install.ps1`:
- Around line 41-43: The empty catch block should emit a verbose message for
observability: inside the catch after the type-detection try, call Write-Verbose
with a concise message such as "Type not available — falling back to env var
detection" and include the caught error details (e.g. $_ or
$Error[0].Exception.Message) so users running the script with -Verbose can see
why execution fell through; update the catch block in install.ps1 accordingly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: a1c59e7a-6c5c-484e-9481-373795a6c7cf
📒 Files selected for processing (1)
cli/scripts/install.ps1
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
🪛 PSScriptAnalyzer (1.24.0)
cli/scripts/install.ps1
[warning] 41-43: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'
(PSUseBOMForUnicodeEncodedFile)
🔇 Additional comments (2)
cli/scripts/install.ps1 (2)
33-36: LGTM!Clear documentation explaining the two-tier detection strategy and referencing the root cause issue.
45-57: LGTM!Architecture detection logic correctly implements both paths with consistent output values (
amd64/arm64) and appropriate error handling for unsupported architectures.Note: The
exit 1statements afterWrite-Errorare technically unreachable since$ErrorActionPreference = "Stop"promotes the error to terminating, but they serve as defensive guards if that preference is ever changed.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/scripts/install.ps1`:
- Around line 42-43: The PowerShell script cli/scripts/install.ps1 contains
non-ASCII characters (e.g., the string literal "RuntimeInformation unavailable;
using PROCESSOR_ARCHITECTURE fallback.") but is not saved with a UTF-8 BOM;
reopen cli/scripts/install.ps1 in your editor and resave the file as UTF-8 with
BOM (UTF-8 with signature) so it satisfies PSUseBOMForUnicodeEncodedFile and
avoids encoding issues on Windows PowerShell 5.1—no source changes needed, only
change the file encoding and commit.
- Around line 53-57: The architecture detection switch that sets $WinArch
currently reads only $env:PROCESSOR_ARCHITECTURE and fails on WOW64; update the
logic used when computing $WinArch (the switch block) to first check
$env:PROCESSOR_ARCHITEW6432 (if defined and non-empty) and use that as the
architecture value, then fall back to $env:PROCESSOR_ARCHITECTURE, keeping the
same mapping cases ("AMD64" -> "amd64", "ARM64" -> "arm64") and the same error
path; ensure this change is applied where $WinArch is assigned so WOW64 (32-bit
PowerShell on 64-bit OS) resolves correctly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: fe09be2a-c9f3-49a1-8853-344363d5be9f
📒 Files selected for processing (1)
cli/scripts/install.ps1
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
🪛 PSScriptAnalyzer (1.24.0)
cli/scripts/install.ps1
[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'
(PSUseBOMForUnicodeEncodedFile)
🔇 Additional comments (1)
cli/scripts/install.ps1 (1)
33-51: Good hardening of architecture detection path.The try/catch + null check flow is clean, preserves existing behavior when
RuntimeInformationworks, and keeps explicit unsupported-arch errors.
cli/scripts/install.ps1
Outdated
| # Type not available — fall through to env var detection. | ||
| Write-Verbose "RuntimeInformation unavailable; using PROCESSOR_ARCHITECTURE fallback." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
p = Path("cli/scripts/install.ps1")
b = p.read_bytes()
print("has_utf8_bom:", b.startswith(b"\xef\xbb\xbf"))
print("non_ascii_byte_count:", sum(x > 127 for x in b))
PYRepository: Aureliolo/synthorg
Length of output: 105
Save install.ps1 as UTF-8 with BOM to resolve PowerShell encoding rule violation.
The file contains 9 non-ASCII characters but lacks UTF-8 BOM encoding, which triggers PSUseBOMForUnicodeEncodedFile in PSScriptAnalyzer and can impact compatibility with Windows PowerShell 5.1. Either save the file with UTF-8 BOM or remove the non-ASCII characters.
🧰 Tools
🪛 PSScriptAnalyzer (1.24.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 `@cli/scripts/install.ps1` around lines 42 - 43, The PowerShell script
cli/scripts/install.ps1 contains non-ASCII characters (e.g., the string literal
"RuntimeInformation unavailable; using PROCESSOR_ARCHITECTURE fallback.") but is
not saved with a UTF-8 BOM; reopen cli/scripts/install.ps1 in your editor and
resave the file as UTF-8 with BOM (UTF-8 with signature) so it satisfies
PSUseBOMForUnicodeEncodedFile and avoids encoding issues on Windows PowerShell
5.1—no source changes needed, only change the file encoding and commit.
RuntimeInformation::OSArchitecture returns $null on some Windows systems with older .NET runtimes, causing the installer to fail with "Unsupported architecture:". Fall back to $env:PROCESSOR_ARCHITECTURE which is available on all Windows versions. Closes #521
Addresses PSScriptAnalyzer PSAvoidUsingEmptyCatchBlock warning. Users running with -Verbose can now see why the fallback was used.
- Check PROCESSOR_ARCHITEW6432 before PROCESSOR_ARCHITECTURE to get the real OS architecture when running 32-bit PowerShell on 64-bit Windows (WOW64 scenario) - Replace em-dash with ASCII dash to avoid PSUseBOMForUnicodeEncodedFile warning and potential encoding issues on Windows PowerShell 5.1
1f51653 to
92399a7
Compare
🤖 I have created a release *beep* *boop* --- ## [0.3.2](v0.3.1...v0.3.2) (2026-03-17) ### Features * **settings:** route structural data reads through SettingsService ([#525](#525)) ([289f604](289f604)) ### Bug Fixes * **cli:** add fallback arch detection in PowerShell installer ([#529](#529)) ([0250afb](0250afb)), closes [#521](#521) ### CI/CD * bump the minor-and-patch group with 2 updates ([#517](#517)) ([46bdd1a](46bdd1a)) * bump wrangler from 4.73.0 to 4.74.0 in /.github in the minor-and-patch group ([#511](#511)) ([903b71a](903b71a)) ### Maintenance * bump node from `7a4ef57` to `44bcbf4` in /docker/sandbox ([#515](#515)) ([3cbddd1](3cbddd1)) * bump python from `6a27522` to `584e89d` in /docker/backend ([#513](#513)) ([0715910](0715910)) * bump python from `6a27522` to `584e89d` in /docker/sandbox ([#514](#514)) ([787dfe1](787dfe1)) * bump the minor-and-patch group across 1 directory with 2 updates ([#527](#527)) ([e96c0d4](e96c0d4)) * bump the minor-and-patch group across 2 directories with 3 updates ([#512](#512)) ([b95ba3d](b95ba3d)) * **docker:** disable Mem0 telemetry in container config ([#531](#531)) ([9fc29eb](9fc29eb)) * improve GitHub issue templates with structured forms ([#528](#528)) ([4fb66cf](4fb66cf)), closes [#522](#522) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
RuntimeInformation::OSArchitecturereturns$nullon some Windows systems where the .NET type isn't loaded (older CLR, misconfigured environments)RuntimeInformationcall intry/catchand falls back to$env:PROCESSOR_ARCHITECTURE— available on every Windows version since XP, every PowerShell since 2.0amd64/arm64outputTest plan
$env:PROCESSOR_ARCHITECTURE = "AMD64"after forcing the try/catch to failCloses #521