From 9621f903254db25f2c98dd5130043b6852c0026b Mon Sep 17 00:00:00 2001 From: xxxigm Date: Tue, 4 Aug 2026 13:35:43 -0600 Subject: [PATCH 1/3] fix(install): resolve 8.3 profile aliases so a built desktop app stops reporting failure Windows aliases a profile folder whose name has a space, a dot, or an accented character (FIRST~1.LAS, STONE~1.ZEN, RUBN~1). PowerShell's FileSystem provider then throws "does not exist" the moment such a path reaches a provider cmdlet, which every Node/Electron stage hits through Tee-Object and the desktop stage hits again probing the binary it just built. The install fails on an artifact that is sitting on disk. install.ps1 already tried to expand these, but only via COM and only for TEMP/TMP. COM cannot expand an alias on a non-English locale, and it cannot expand one at all when 8dot3 generation is disabled or the alias is stale -- both return the short path unchanged. LOCALAPPDATA was never normalized either, so InstallDir stayed short even when TEMP got fixed. Three resolvers now run in order, each covering what the last one cannot: kernel32!GetLongPathNameW (locale-independent), COM (P/Invoke blocked), and profile-root substitution (nothing to resolve -- rebuild on a root we can prove is long). All five profile-rooted variables are normalized, and HermesHome/InstallDir are re-derived from them. An explicitly passed -HermesHome/-InstallDir is normalized in place, never replaced. Every resolver degrades to returning its input, so a host where none apply behaves exactly as before. Rewrites are logged to stderr: this bug class has only ever been reported as a bare "does not exist" with no hint that a short alias was involved. Co-authored-by: Sahil-SS9 <218421507+Sahil-SS9@users.noreply.github.com> --- scripts/install.ps1 | 294 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 265 insertions(+), 29 deletions(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index fdfdd7aa07..960e014fc2 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -43,6 +43,15 @@ param( [switch]$NonInteractive, [switch]$Json, + # Print the paths this install would use, as JSON, and exit without + # touching anything. The first question on any "installer says a path + # doesn't exist" report is which paths it actually resolved -- especially + # on profiles Windows exposes through an 8.3 alias, where what the user + # sees in Explorer and what the installer receives differ. + # + # powershell -File install.ps1 -ShowResolvedPaths + [switch]$ShowResolvedPaths, + # --- Ensure mode (dep_ensure.py entry point) --- [string]$Ensure = "", [switch]$PostInstall, @@ -97,45 +106,267 @@ try { # ============================================================================ # 8.3 short-path normalization # ============================================================================ -# When the Windows user-profile folder name contains a space (e.g. -# "First Last"), Windows generates an 8.3 short alias for it (e.g. FIRST~1.LAS) -# and may expose %TEMP%/%TMP% in that short form: +# Windows generates an 8.3 short alias for a user-profile folder whose name +# contains a space ("First Last" -> FIRST~1.LAS), a dot ("Stone.ZEN8" -> +# STONE~1.ZEN), or an accented character ("Ruben" spelled with an acute e -> +# RUBN~1). It can then expose %TEMP%, %TMP%, %LOCALAPPDATA%, %APPDATA% and +# %USERPROFILE% -- plus everything derived from them, including the default +# HERMES_HOME and InstallDir -- in that short form: # C:\Users\FIRST~1.LAS\AppData\Local\Temp -# PowerShell's FileSystem provider mishandles the "~1.ext" component when such a -# path is handed to a provider cmdlet like `Tee-Object -FilePath` / -# `Out-File -FilePath`, throwing: -# "An object at the specified path C:\Users\FIRST~1.LAS does not exist." -# Every Node/Electron build+install stage streams its log to %TEMP% via -# Tee-Object, so they all abort with that error, while the Python/uv stages -- -# which never write a side log to %TEMP% through a provider cmdlet -- complete -# fine. Expanding %TEMP%/%TMP% back to their long form once, up front, lets -# every downstream cmdlet (and child process) see a path the provider can -# resolve. (GH: Windows desktop installer fails at Node/Electron stages.) +# +# PowerShell's FileSystem provider mishandles the aliased component when such a +# path reaches a provider cmdlet (`Tee-Object -FilePath`, `Out-File`, +# `New-Item`, `Test-Path`), throwing "An object at the specified path +# C:\Users\FIRST~1.LAS does not exist" -- localized on non-English hosts. +# Every Node/Electron stage streams its build log to %TEMP% via Tee-Object and +# the desktop stage probes the binary it produced under the profile-derived +# InstallDir, so the bootstrap aborts even though the artifact built fine. +# The Python/uv stages, which never hand a %TEMP% path to a provider cmdlet, +# sail through -- which is why the failure looks Node-specific. +# +# Expanding every profile-rooted path back to long form once, up front, lets +# every downstream cmdlet and child process see something the provider can +# resolve. Three resolvers, tried in order, because no single one covers every +# host: +# +# 1. kernel32!GetLongPathNameW -- expands any 8.3 component regardless of +# locale, including the accented-username aliases the COM resolver misses. +# 2. Scripting.FileSystemObject -- fallback for hosts where P/Invoke is +# blocked. +# 3. Profile-root substitution -- when the volume has 8.3 generation disabled +# or the alias is stale, neither resolver can expand the name because it +# no longer maps to anything on disk. The aliased component is always the +# profile folder itself (everything below it was created long), so swap in +# a profile root we can prove is long and reattach the tail. +# +# All three degrade to returning the input untouched, so a host where none of +# them apply -- including non-Windows -- behaves exactly as it did before. -function ConvertTo-LongPath { +$script:LongProfileRoot = $null + +function Write-PathDiag { + # Diagnostics for this block go to stderr, never stdout: the stage protocol + # hands drivers a single line of JSON on stdout and a stray note would break + # anything parsing it. + # + # Suppressed entirely under -ShowResolvedPaths, which is a machine-readable + # query: Windows PowerShell 5.1 wraps any native-command stderr in a + # NativeCommandError and folds it back into the caller's own stream, so a + # child writing here at all is enough to corrupt a 5.1 caller's capture. + # The JSON already carries everything these lines say. + # + # [Console]::Error.WriteLine specifically -- verified reaching a caller on a + # windows-latest runner. $host.UI.WriteErrorLine was tried and silently + # produced nothing there under a non-interactive host. + param([string]$Message) + if ($ShowResolvedPaths) { return } + [Console]::Error.WriteLine("[hermes] $Message") +} + +function Get-LongProfileRoot { + # The user's profile directory in long form, or '' when every source we + # can reach is itself aliased. Cached: this runs per env var. + if ($null -ne $script:LongProfileRoot) { return $script:LongProfileRoot } + $script:LongProfileRoot = '' + + # %USERPROFILE% first: it is what the rest of the install derives from, and + # on a host handing us aliased paths the .NET known-folder lookup tends to + # be aliased in exactly the same way. Then the HOMEDRIVE/HOMEPATH pair, then + # the profile's parent (C:\Users never carries an alias) plus %USERNAME%, + # which stays the long account name even when every path is short. + $envProfile = [Environment]::GetEnvironmentVariable('USERPROFILE') + $shellProfile = [Environment]::GetFolderPath('UserProfile') + $candidates = @($envProfile, $shellProfile, "$env:HOMEDRIVE$env:HOMEPATH") + foreach ($anchor in @($envProfile, $shellProfile)) { + if ($anchor -and $env:USERNAME) { + $parent = Split-Path -Parent $anchor.TrimEnd('\', '/') + if ($parent) { $candidates += (Join-Path $parent $env:USERNAME) } + } + } + + foreach ($candidate in $candidates) { + if ([string]::IsNullOrWhiteSpace($candidate)) { continue } + # Trailing separators make Split-Path -Parent return the directory + # itself, which would silently break the ancestry check downstream. + $candidate = $candidate.TrimEnd('\', '/') + if (-not $candidate) { continue } + if ($candidate -match '~\d') { continue } + try { + if (Test-Path -LiteralPath $candidate -PathType Container) { + $script:LongProfileRoot = $candidate + break + } + } catch { + # Unreadable candidate (denied, malformed): try the next one. + } + } + + # Say which root we landed on. When someone reports "still broken" this is + # the first thing worth knowing, and it costs one line on the rare path + # where an alias actually showed up. + if ($script:LongProfileRoot) { + Write-PathDiag "long profile root: $script:LongProfileRoot" + } else { + Write-PathDiag "no long profile root found; 8.3 paths left as-is (tried: $($candidates -join ', '))" + } + return $script:LongProfileRoot +} + +function Expand-ShortProfileRoot { + # Rebuild $Path onto a known-long profile root when its aliased component + # is the profile folder. Returns $Path unchanged when it isn't, so a custom + # TEMP on another volume (D:\SHORT~1\Temp) is never rewritten. param([string]$Path) - if ([string]::IsNullOrWhiteSpace($Path)) { return $Path } - # Only 8.3 short names carry a tilde+digit ("~1"); skip the COM round-trip - # for ordinary long paths. - if ($Path -notmatch '~\d') { return $Path } - try { - $fso = New-Object -ComObject Scripting.FileSystemObject - if ($fso.FolderExists($Path)) { return $fso.GetFolder($Path).Path } - if ($fso.FileExists($Path)) { return $fso.GetFile($Path).Path } - } catch { - # COM unavailable / locked-down host: fall back to the original path. + + $longRoot = Get-LongProfileRoot + if (-not $longRoot) { return $Path } + $longRootParent = Split-Path -Parent $longRoot + if (-not $longRootParent) { return $Path } + + $node = $Path + $tail = '' + while ($node -and ($node -match '~\d')) { + $leaf = Split-Path -Leaf $node + $parent = Split-Path -Parent $node + if (-not $parent) { return $Path } + if ($leaf -match '~\d') { + # Candidate profile folder. Only substitute when it sits in the + # same directory as the real profile (both C:\Users). + if ($parent -ne $longRootParent) { return $Path } + if ($tail) { return (Join-Path $longRoot $tail) } + return $longRoot + } + $tail = if ($tail) { Join-Path $leaf $tail } else { $leaf } + $node = $parent } return $Path } -foreach ($tmpVar in @('TEMP', 'TMP')) { - $current = [Environment]::GetEnvironmentVariable($tmpVar) - if ($current) { +function ConvertTo-LongPath { + param([string]$Path) + if ([string]::IsNullOrWhiteSpace($Path)) { return $Path } + # Only 8.3 short names carry a tilde+digit ("~1"); skip every resolver for + # ordinary long paths, which is the overwhelmingly common case. + if ($Path -notmatch '~\d') { return $Path } + + # 1. kernel32. Compiled on first use only, so a normal profile never pays + # the Add-Type cost (this file is re-entered once per install stage). + try { + if (-not ([System.Management.Automation.PSTypeName]'HermesInstall.LongPath').Type) { + Add-Type -Namespace 'HermesInstall' -Name 'LongPath' -MemberDefinition @' +[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] +public static extern int GetLongPathNameW(string lpszShortPath, System.Text.StringBuilder lpszLongPath, int cchBuffer); +'@ + } + $buffer = New-Object System.Text.StringBuilder 4096 + $length = [HermesInstall.LongPath]::GetLongPathNameW($Path, $buffer, $buffer.Capacity) + if ($length -gt $buffer.Capacity) { + $buffer = New-Object System.Text.StringBuilder $length + $length = [HermesInstall.LongPath]::GetLongPathNameW($Path, $buffer, $buffer.Capacity) + } + if ($length -gt 0) { + $expanded = $buffer.ToString() + if ($expanded -and $expanded -notmatch '~\d') { + $script:LastResolver = 'kernel32' + return $expanded + } + } + } catch { + # Not Windows, or P/Invoke denied by policy: try the next resolver. + } + + # 2. COM. Validate the result the same way the kernel32 branch does: this + # resolver can report success and still hand back a path that carries the + # alias (observed on a windows-latest runner, where it "resolved" + # C:\Users\FIRST~1.LAS\... to itself). Accepting that silently is what let a + # short path reach the provider cmdlets in the first place, so an + # unexpanded result counts as failure and falls through. + try { + $fso = New-Object -ComObject Scripting.FileSystemObject + $resolved = $null + if ($fso.FolderExists($Path)) { $resolved = $fso.GetFolder($Path).Path } + elseif ($fso.FileExists($Path)) { $resolved = $fso.GetFile($Path).Path } + if ($resolved -and $resolved -notmatch '~\d') { + $script:LastResolver = 'com' + return $resolved + } + } catch { + # COM unavailable / locked-down host: try the next resolver. + } + + # 3. The alias resolves to nothing. Rebuild from a long profile root. + $rebuilt = Expand-ShortProfileRoot $Path + $script:LastResolver = if ($rebuilt -ne $Path) { 'profile-root' } else { 'none' } + return $rebuilt +} + +function Set-LongProfileEnvVars { + # Normalize every profile-rooted variable the install reads, not just + # %TEMP%: the desktop stage derives InstallDir from %LOCALAPPDATA%, and a + # short root there fails the post-build probe after a successful build. + # Returns $true when anything was rewritten. + $rewrote = $false + $script:NormalizedPathRewrites = @{} + foreach ($name in @('TEMP', 'TMP', 'LOCALAPPDATA', 'APPDATA', 'USERPROFILE')) { + $current = [Environment]::GetEnvironmentVariable($name) + if (-not $current) { continue } $expanded = ConvertTo-LongPath $current if ($expanded -and $expanded -ne $current) { - Set-Item -Path "Env:$tmpVar" -Value $expanded + Set-Item -Path "Env:$name" -Value $expanded + $rewrote = $true + $script:NormalizedPathRewrites[$name] = $expanded + # Rewriting a profile path is rare and corrective; say so. Every + # report of this bug class arrived as a bare "does not exist" with + # no hint that a short alias was involved. stderr, so the stage + # protocol's stdout JSON stays parseable. + Write-PathDiag "expanded 8.3 short path in %$name%: $current -> $expanded" } } + return $rewrote +} + +$script:NormalizedProfilePaths = Set-LongProfileEnvVars + +# Re-derive the install paths now that the env vars behind their defaults are +# long. An explicitly passed -HermesHome / -InstallDir is normalized in place +# rather than replaced, so a caller's choice is never overwritten by a default. +# $PSBoundParameters is only meaningful at script scope, so this stays inline. +if ($PSBoundParameters.ContainsKey('HermesHome')) { + $HermesHome = ConvertTo-LongPath $HermesHome +} else { + $HermesHome = ConvertTo-LongPath $( + if ($env:HERMES_HOME) { $env:HERMES_HOME } else { "$env:LOCALAPPDATA\hermes" } + ) +} +if ($PSBoundParameters.ContainsKey('InstallDir')) { + $InstallDir = ConvertTo-LongPath $InstallDir +} else { + $InstallDir = ConvertTo-LongPath $( + if ($env:HERMES_HOME) { "$env:HERMES_HOME\hermes-agent" } else { "$env:LOCALAPPDATA\hermes\hermes-agent" } + ) +} +if ($script:NormalizedProfilePaths) { + # Which paths the install actually settled on. Absent from every report of + # this bug class, and the whole question once a short alias is in play. + Write-PathDiag "resolved install paths: HermesHome=$HermesHome InstallDir=$InstallDir" +} + +# Captured here, where the values are final, and emitted from the entry-point +# dispatch at the bottom (alongside -ProtocolVersion / -Manifest) so +# -ShowResolvedPaths exits before any stage runs. +# +# The report goes to STDOUT as JSON: on Windows a child's stderr does not +# reliably reach a parent process -- three separate capture mechanisms each came +# back empty on a windows-latest runner while stdout arrived intact -- and the +# first question on any "installer says a path doesn't exist" report is which +# paths it actually resolved. +$script:ResolvedPathReport = @{ + long_profile_root = (Get-LongProfileRoot) + normalized = $script:NormalizedPathRewrites + resolver = $script:LastResolver + temp = $env:TEMP + hermes_home = $HermesHome + install_dir = $InstallDir } # ============================================================================ @@ -3233,7 +3464,7 @@ function Install-Desktop { throw "apps/desktop build failed (exit $code)" } Write-Success "Desktop app built" - Remove-Item -Force $buildLog -ErrorAction SilentlyContinue + Remove-Item -LiteralPath $buildLog -Force -ErrorAction SilentlyContinue } catch { if ($prevEAP) { $ErrorActionPreference = $prevEAP } Pop-Location @@ -3955,6 +4186,11 @@ try { exit 0 } + if ($ShowResolvedPaths) { + $script:ResolvedPathReport | ConvertTo-Json -Depth 5 -Compress | Write-Output + exit 0 + } + if ($Manifest) { $payload = @{ protocol_version = $InstallStageProtocolVersion From dae7e5477e39a4c59e14208c66ab2c09b6a418fb Mon Sep 17 00:00:00 2001 From: Sahil-SS9 <218421507+Sahil-SS9@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:35:44 -0600 Subject: [PATCH 2/3] test(install): exercise 8.3 normalization by running install.ps1, not by parsing it The previous suite pulled ConvertTo-LongPath out of install.ps1 via the AST and dot-sourced the extracted text. AGENTS.md bans source-reading tests, and this one showed why: it never executed the script-level Add-Type the kernel32 resolver depends on, so the resolver that does the actual work was untestable by construction. Each case now spawns install.ps1 as a real subprocess with a crafted environment. -ProtocolVersion is a side-effect-free early exit below the normalization block, so the whole block runs exactly as it does mid-install and the assertions read what it reports back. Verified RED against the pre-fix install.ps1 (10 of 24 assertions fail) and GREEN after. The profile-root substitution is pure path arithmetic, so those cases run on any host including non-Windows CI. Co-authored-by: xxxigm --- scripts/tests/test-install-ps1-longpath.ps1 | 323 +++++++++++++++++--- 1 file changed, 280 insertions(+), 43 deletions(-) diff --git a/scripts/tests/test-install-ps1-longpath.ps1 b/scripts/tests/test-install-ps1-longpath.ps1 index a93acb0d9a..997738df80 100644 --- a/scripts/tests/test-install-ps1-longpath.ps1 +++ b/scripts/tests/test-install-ps1-longpath.ps1 @@ -1,21 +1,36 @@ -# Unit tests for install.ps1's ConvertTo-LongPath helper. +# Tests for install.ps1's 8.3 short-path normalization. # # Run from a PowerShell prompt: # -# powershell -NoProfile -ExecutionPolicy Bypass -File scripts/tests/test-install-ps1-longpath.ps1 +# pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/tests/test-install-ps1-longpath.ps1 # -# Background: on a Windows profile whose folder name contains a space (e.g. -# "First Last"), %TEMP%/%TMP% can be exposed as an 8.3 short path +# Background: when the Windows profile folder's name contains a space +# ("First Last"), a dot ("Stone.ZEN8"), or an accented character, Windows can +# expose %TEMP%, %LOCALAPPDATA% and friends as an 8.3 alias # (C:\Users\FIRST~1.LAS\...). PowerShell's FileSystem provider chokes on the -# "~1.ext" component when it reaches a provider cmdlet (Tee-Object -FilePath), -# aborting the Node/Electron install+build stages. install.ps1 expands such -# paths to their long form up front; this verifies the helper's contract. +# aliased component once it reaches a provider cmdlet (Tee-Object -FilePath), +# aborting the Node/Electron stages and the desktop post-build probe. +# install.ps1 expands those paths up front; this asserts that contract. # -# We extract just the function from install.ps1 via the AST so the installer's -# top-level body never runs (dot-sourcing would execute the whole script). -# The COM-backed expansion only fires for inputs containing "~"; the -# pass-through and graceful-fallback paths are assertable on any host (incl. -# non-Windows pwsh, where the COM object is simply unavailable). +# HOW THIS RUNS THE CODE: by executing install.ps1 as a real subprocess with a +# crafted environment and reading what it reports back. `-ProtocolVersion` is a +# side-effect-free early exit that sits BELOW the normalization block, so the +# whole block -- including the script-level Add-Type the kernel32 resolver +# needs -- executes exactly as it does during an install. Nothing here parses +# install.ps1's source (AGENTS.md bans source-reading tests: they pass on +# broken code and fail on correct refactors). +# +# HERMETIC ENVIRONMENT: every case sets all five profile variables explicitly. +# GitHub's own Windows runners hand down a genuinely 8.3-aliased TEMP/TMP +# (C:\Users\RUNNER~1\AppData\Local\Temp), so an inherited variable is a live +# instance of the very bug under test and would contaminate any case that +# didn't override it. +# +# Portability: resolver 3 (profile-root substitution) is pure path arithmetic, +# so the substitution assertions run everywhere, including non-Windows CI. The +# kernel32 and COM resolvers only have anything to expand on a real Windows +# volume; on other hosts they no-op and fall through, which is itself the +# graceful-degradation contract asserted below. $ErrorActionPreference = "Stop" $repoRoot = Split-Path -Parent (Split-Path -Parent (Split-Path -Parent $MyInvocation.MyCommand.Path)) @@ -26,61 +41,283 @@ if (-not (Test-Path $installScript)) { } $failures = 0 +$script:lastRaw = '' + function Assert-Equal { - param([Parameter(Mandatory = $true)] $Expected, - [Parameter(Mandatory = $true)] $Actual, - [Parameter(Mandatory = $true)] [string]$Label) + param($Expected, $Actual, [Parameter(Mandatory = $true)][string]$Label) if ($Expected -ne $Actual) { Write-Host "FAIL: $Label" -ForegroundColor Red Write-Host " expected: $Expected" Write-Host " actual: $Actual" + if ($script:lastRaw) { + # The installer's own account of what it did, plus the environment + # it was handed. Without both, a failure on a host you cannot reach + # is pure guesswork. + Write-Host " installer reported: $script:lastRaw" + Write-Host " environment sent: $script:lastEnv" + } $script:failures++ } else { Write-Host "OK: $Label" -ForegroundColor Green } } -# --- Load ConvertTo-LongPath from install.ps1 without executing the script --- -$tokens = $null -$errors = $null -$ast = [System.Management.Automation.Language.Parser]::ParseFile($installScript, [ref]$tokens, [ref]$errors) -$fnAst = $ast.FindAll( - { - param($node) - $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and - $node.Name -eq 'ConvertTo-LongPath' - }, $true) | Select-Object -First 1 +# --- Harness --------------------------------------------------------------- +# The real profile root the installer will substitute in, derived the same way +# install.ps1 derives it so these assertions hold on any host and any account. +$profileDir = [Environment]::GetFolderPath('UserProfile') +$usersDir = Split-Path -Parent $profileDir +$sep = [System.IO.Path]::DirectorySeparatorChar -if (-not $fnAst) { - throw "ConvertTo-LongPath not found in install.ps1 -- did the helper get renamed/removed?" +# Starting guess for the baseline environment; replaced by the probe below with +# whatever root the installer itself resolves. +$script:baseRoot = $profileDir + +# A profile alias that cannot resolve: no such folder exists, so kernel32 and +# COM both fail and only the profile-root substitution can handle it. +$shortProfile = Join-Path $usersDir 'FIRST~1.LAS' + +function Join-Parts { + # Join path segments with the platform separator. Literal forward slashes + # inside a path would make Split-Path's behavior host-dependent, which is + # noise this suite doesn't need. + param([string[]]$Parts) + return ($Parts -join $sep) } -. ([scriptblock]::Create($fnAst.Extent.Text)) -# --- Tests --- +# Ask install.ps1 what paths it resolves under a given environment. +# +# -ShowResolvedPaths prints a JSON object on STDOUT and exits without touching +# anything, so the whole normalization block -- including the script-level +# Add-Type the kernel32 resolver needs -- has already run by the time it is +# printed. Stdout, deliberately: three separate stderr capture mechanisms +# (ProcessStartInfo.RedirectStandardError, `2>$file`, and a merged `2>&1` +# pipeline) were each verified to come back EMPTY from the installer on a +# windows-latest runner while stdout arrived intact. The installer's human +# diagnostics still go to stderr; the machine-readable contract is on stdout, +# which is the only stream that survives everywhere. +# +# Environment overrides are applied to this process and restored afterwards, +# since that is what the child inherits. +function Invoke-Normalization { + param( + [hashtable]$Environment = @{}, + [string[]]$ExtraArgs = @() + ) + + # Start from a long, self-consistent profile so nothing is inherited; + # callers override only the variables their case is about. $script:baseRoot + # is the test's best guess until the probe below replaces it with the root + # the installer actually resolves. + $root = $script:baseRoot + $env0 = @{ + TEMP = (Join-Parts @($root, 'AppData', 'Local', 'Temp')) + TMP = (Join-Parts @($root, 'AppData', 'Local', 'Temp')) + LOCALAPPDATA = (Join-Parts @($root, 'AppData', 'Local')) + APPDATA = (Join-Parts @($root, 'AppData', 'Roaming')) + USERPROFILE = $root + HERMES_HOME = '' + } + foreach ($key in $Environment.Keys) { $env0[$key] = $Environment[$key] } + + $psExe = (Get-Process -Id $PID).Path + $outFile = [System.IO.Path]::GetTempFileName() + $errFile = [System.IO.Path]::GetTempFileName() + $saved = @{} + foreach ($key in $env0.Keys) { $saved[$key] = [Environment]::GetEnvironmentVariable($key) } + + try { + foreach ($key in $env0.Keys) { Set-Item -Path "Env:$key" -Value $env0[$key] } + $callArgs = @('-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', $installScript) + $ExtraArgs + @('-ShowResolvedPaths') + # The call operator, not Start-Process: on Windows Start-Process does + # not hand the parent's modified environment block to the child, so the + # installer saw the runner's real TEMP instead of the aliased one this + # case sets, and every rewrite assertion came back "not rewritten". + # `&` inherits the environment on every host. + # + # stderr is merged into the same file rather than redirected separately: + # Windows PowerShell 5.1 wraps ANY stderr from a native command in a + # NativeCommandError record, and a bare `2>$file` still emits that + # record into this script's error stream, which fails the 5.1 lane even + # under 'Continue'. Merging with 2>&1 keeps the bytes and produces no + # error record. The installer's stdout here is a single JSON object and + # its diagnostics are all `[hermes] `-prefixed, so the two separate + # cleanly on the way back out. + $prevEAP = $ErrorActionPreference + $ErrorActionPreference = 'Continue' + $global:LASTEXITCODE = 0 + try { + & $psExe @callArgs *> $outFile + } finally { + $ErrorActionPreference = $prevEAP + } + $exitCode = $LASTEXITCODE + $raw = @(Get-Content -LiteralPath $outFile -ErrorAction SilentlyContinue) + $stderr = ($raw | Where-Object { $_ -like '`[hermes`]*' }) -join "`n" + $stdout = ($raw | Where-Object { $_ -notlike '`[hermes`]*' }) -join "`n" + } finally { + foreach ($key in $saved.Keys) { + if ($null -eq $saved[$key]) { + Remove-Item -LiteralPath "Env:$key" -ErrorAction SilentlyContinue + } else { + Set-Item -Path "Env:$key" -Value $saved[$key] + } + } + Remove-Item -LiteralPath $outFile, $errFile -Force -ErrorAction SilentlyContinue + } + + if ($null -eq $stdout) { $stdout = '' } + $stdout = $stdout.Trim() + $script:lastRaw = if ($stdout) { $stdout } else { '(child produced no stdout)' } + $script:lastEnv = ($env0.Keys | Sort-Object | ForEach-Object { "$_=$($env0[$_])" }) -join '; ' + + $paths = $null + if ($stdout) { + try { $paths = $stdout | ConvertFrom-Json } catch { $paths = $null } + } + + # normalized is an object keyed by variable name; flatten to a hashtable so + # callers can ask "was TEMP rewritten, and to what". + $rewrites = @{} + if ($paths -and $paths.normalized) { + foreach ($prop in $paths.normalized.PSObject.Properties) { + $rewrites[$prop.Name] = "$($prop.Value)" + } + } + + return @{ + ExitCode = $exitCode + Stdout = $stdout + Rewrites = $rewrites + InstallDir = $(if ($paths) { $paths.install_dir } else { $null }) + HermesHome = $(if ($paths) { $paths.hermes_home } else { $null }) + LongRoot = $(if ($paths) { $paths.long_profile_root } else { $null }) + } +} + +function Get-Rewrite { + # '' rather than $null for an untouched variable, so a failure prints + # something legible instead of a blank. + param($Result, [string]$Name) + if ($Result.Rewrites.ContainsKey($Name)) { return $Result.Rewrites[$Name] } + return '' +} + +# Ask the installer once, up front, which long root it resolves on this host, +# and assert every expectation against that. Deriving it independently in the +# test would only prove the two derivations agree, not that the fix works -- +# and on GitHub's Windows runners they don't agree, because the runner hands +# down a genuinely 8.3-aliased profile. +$probe = Invoke-Normalization @{ USERPROFILE = $shortProfile } +$longRoot = $probe.LongRoot + Write-Host "" -Write-Host "-- ConvertTo-LongPath --" +Write-Host "-- the installer resolves a long profile root --" +if ([string]::IsNullOrEmpty($longRoot)) { + # Nothing below can mean anything without this, so show the child's whole + # output rather than leaving a bare assertion failure on an unreachable host. + Write-Host "FAIL: a long profile root is found" -ForegroundColor Red + Write-Host " probe exit code: $($probe.ExitCode)" + Write-Host " probe stdout: $($probe.Stdout)" + Write-Host " probe env: $script:lastEnv" + Write-Host " probe stdout (raw):" + foreach ($line in ($script:lastRaw -split "`r?`n")) { + if ($line.Trim()) { Write-Host " $line" } + } + Write-Host "FAILED: cannot continue without a long profile root" -ForegroundColor Red + exit 1 +} +Write-Host "OK: a long profile root is found ($longRoot)" -ForegroundColor Green +Assert-Equal -Expected $false -Actual ($longRoot -match '~\d') -Label "the resolved root carries no 8.3 alias" +# Every subsequent case's baseline is now the installer's own root, so a +# "nothing to expand" case really has nothing to expand even on a runner whose +# inherited profile is itself aliased. +$script:baseRoot = $longRoot -Assert-Equal -Expected "" -Actual (ConvertTo-LongPath "") -Label "empty string returns empty" -Assert-Equal -Expected $null -Actual (ConvertTo-LongPath $null) -Label "null returns null" +Write-Host "" +Write-Host "-- normalization is a no-op for ordinary paths --" -# No 8.3 component -> returned verbatim (even with spaces). -$longish = "C:\Users\First Last\AppData\Local\Temp" -Assert-Equal -Expected $longish -Actual (ConvertTo-LongPath $longish) -Label "long path with spaces is unchanged" +# A profile name with a space is NOT itself a short path; nothing to expand. +$result = Invoke-Normalization +Assert-Equal -Expected 0 -Actual $result.ExitCode -Label "long paths: install.ps1 still reaches its early exit" +Assert-Equal -Expected 0 -Actual $result.Rewrites.Count -Label "long paths: nothing rewritten" +Assert-Equal -Expected $false -Actual ($result.InstallDir -match '~\d') -Label "long paths: InstallDir passes through clean" -$noTilde = "/tmp/some/long/path" -Assert-Equal -Expected $noTilde -Actual (ConvertTo-LongPath $noTilde) -Label "tilde-free path is unchanged" +Write-Host "" +Write-Host "-- an unresolvable profile alias is rebuilt on the long profile root --" -# Looks like an 8.3 name but does not exist -> graceful fallback to the input -# (FolderExists/FileExists both false, or COM unavailable on this host). -$fakeShort = "C:\Users\FIRST~1.LAS\does\not\exist" -Assert-Equal -Expected $fakeShort -Actual (ConvertTo-LongPath $fakeShort) -Label "nonexistent 8.3 path falls back to input" +# The reported failure: TEMP under an 8.3 profile alias that no resolver can +# expand (8dot3 disabled, or a stale alias). GH #52842, GH #57526. +$shortTemp = Join-Parts @($shortProfile, 'AppData', 'Local', 'Temp') +$expectedTemp = Join-Parts @($profileDir, 'AppData', 'Local', 'Temp') -# --- Summary --- +$result = Invoke-Normalization @{ TEMP = $shortTemp; TMP = $shortTemp } +Assert-Equal -Expected 0 -Actual $result.ExitCode -Label "short TEMP: install.ps1 still reaches its early exit" +$expectedTemp = "$longRoot${sep}AppData${sep}Local${sep}Temp" +Assert-Equal -Expected $expectedTemp -Actual (Get-Rewrite $result 'TEMP') -Label "short TEMP is rebuilt on the long profile root" +Assert-Equal -Expected $expectedTemp -Actual (Get-Rewrite $result 'TMP') -Label "short TMP is rebuilt on the long profile root" +Assert-Equal -Expected 2 -Actual $result.Rewrites.Count -Label "short TEMP: only the aliased variables are touched" + +# The profile root itself, with no tail to reattach. USERPROFILE is also where +# the installer looks first for a long root, so this exercises the fallback to +# HOMEDRIVE/HOMEPATH and %USERNAME%. +$result = Invoke-Normalization @{ USERPROFILE = $shortProfile } +Assert-Equal -Expected $longRoot -Actual (Get-Rewrite $result 'USERPROFILE') -Label "bare short profile root expands to the long root" + +Write-Host "" +Write-Host "-- every profile-rooted variable is covered, not just TEMP --" + +# The desktop stage derives InstallDir from %LOCALAPPDATA%; a short root there +# fails the post-build probe after the build already succeeded (GH #52842). +$result = Invoke-Normalization @{ + TEMP = $shortTemp + TMP = $shortTemp + LOCALAPPDATA = (Join-Parts @($shortProfile, 'AppData', 'Local')) + APPDATA = (Join-Parts @($shortProfile, 'AppData', 'Roaming')) + USERPROFILE = $shortProfile +} +foreach ($name in @('TEMP', 'TMP', 'LOCALAPPDATA', 'APPDATA', 'USERPROFILE')) { + $value = Get-Rewrite $result $name + # Assert it was rewritten AND that the result is clean. Checking only for + # the absence of a tilde passes vacuously on a variable nothing touched. + Assert-Equal -Expected $true -Actual ($value.StartsWith($longRoot)) -Label "$name is rebuilt on the long profile root" + Assert-Equal -Expected $false -Actual ($value -match '~\d') -Label "$name no longer carries an 8.3 alias" +} + +# ...and the install paths derived from them are re-derived, not left short. +# This is the difference between "the build works" and "the installer stops +# claiming a successful build failed". Composed with literal backslashes +# because that is how install.ps1 itself builds the default Windows path. +$expectedInstallDir = "$($longRoot)${sep}AppData${sep}Local" + '\hermes\hermes-agent' +Assert-Equal -Expected $expectedInstallDir -Actual $result.InstallDir -Label "InstallDir is re-derived from the long LOCALAPPDATA" + +Write-Host "" +Write-Host "-- substitution is scoped to the profile folder --" + +# We can only prove the long spelling of the profile root itself. A short +# component anywhere else must be left exactly as the caller set it. +$belowProfile = Join-Parts @($profileDir, 'DEEPLY~1', 'Temp') +$result = Invoke-Normalization @{ TEMP = $belowProfile; TMP = $belowProfile } +Assert-Equal -Expected '' -Actual (Get-Rewrite $result 'TEMP') -Label "a short component below the profile root is left alone" + +# A custom TEMP on another volume has no profile root to substitute. +$otherVolume = Join-Parts @("D:", 'SHORT~1', 'Temp') +$result = Invoke-Normalization @{ TEMP = $otherVolume; TMP = $otherVolume } +Assert-Equal -Expected '' -Actual (Get-Rewrite $result 'TEMP') -Label "short TEMP outside the profile is left alone" + +Write-Host "" +Write-Host "-- an explicit -InstallDir is normalized, never replaced --" + +$result = Invoke-Normalization -Environment @{ TEMP = $shortTemp; TMP = $shortTemp } ` + -ExtraArgs @('-InstallDir', (Join-Path $shortProfile 'custom-hermes')) +Assert-Equal -Expected (Join-Path $longRoot 'custom-hermes') -Actual $result.InstallDir -Label "explicit -InstallDir keeps the caller's directory, on the long root" + +# --- Summary --------------------------------------------------------------- Write-Host "" if ($failures -gt 0) { Write-Host "FAILED: $failures assertion(s) failed" -ForegroundColor Red exit 1 } else { - Write-Host "All ConvertTo-LongPath tests passed." -ForegroundColor Green + Write-Host "All 8.3 short-path normalization tests passed." -ForegroundColor Green exit 0 } From 34833303f55984a3ad96baeca47b227f4aadc8ff Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 4 Aug 2026 13:35:44 -0600 Subject: [PATCH 3/3] ci(install): actually run the PowerShell installer tests scripts/tests/ has held three PowerShell suites that no workflow ever invoked -- there is no Windows runner in CI, so they have been inert since they landed. A regression test nothing executes is worse than none: it reads as coverage. Adds a windows-latest job, gated on a new `installer` lane so it only fires for PRs touching install.ps1 or its tests. The 8.3 suite runs under both pwsh 7 and Windows PowerShell 5.1, since install.ps1 arrives via `irm | iex` into whichever shell the user already has and 5.1 is what ships with Windows. Only the 8.3 suite is wired up. The other two fail on main today for unrelated reasons; they can join once they are fixed. --- .github/actions/detect-changes/action.yml | 3 ++ .github/workflows/ci.yml | 9 ++++++ .github/workflows/installer-tests.yml | 38 +++++++++++++++++++++++ scripts/ci/classify_changes.py | 12 +++++++ tests/ci/test_classify_changes.py | 12 ++++++- 5 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/installer-tests.yml diff --git a/.github/actions/detect-changes/action.yml b/.github/actions/detect-changes/action.yml index cecbd4515c..a5c0b6f9ba 100644 --- a/.github/actions/detect-changes/action.yml +++ b/.github/actions/detect-changes/action.yml @@ -36,6 +36,9 @@ outputs: npm_lock: description: Post/update the semantic package-lock.json diff PR comment. value: ${{ steps.classify.outputs.npm_lock }} + installer: + description: Run the PowerShell installer tests on a Windows runner. + value: ${{ steps.classify.outputs.installer }} mcp_catalog: description: Require MCP catalog security review label. value: ${{ steps.classify.outputs.mcp_catalog }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index beb9c41f6a..9b6129d225 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,7 @@ jobs: scan: ${{ steps.classify.outputs.scan }} deps: ${{ steps.classify.outputs.deps }} npm_lock: ${{ steps.classify.outputs.npm_lock }} + installer: ${{ steps.classify.outputs.installer }} docker_meta: ${{ steps.classify.outputs.docker_meta }} mcp_catalog: ${{ steps.classify.outputs.mcp_catalog }} ci_review: ${{ steps.classify.outputs.ci_review }} @@ -87,6 +88,13 @@ jobs: if: needs.detect.outputs.frontend == 'true' uses: ./.github/workflows/js-tests.yml + installer-tests: + name: Installer tests + needs: detect + # Windows-only, and only for PRs that touch install.ps1 or its tests. + if: needs.detect.outputs.installer == 'true' + uses: ./.github/workflows/installer-tests.yml + e2e-desktop: name: Desktop E2E needs: detect @@ -275,6 +283,7 @@ jobs: - tests - lint - js-tests + - installer-tests - e2e-desktop - docs-site - history-check diff --git a/.github/workflows/installer-tests.yml b/.github/workflows/installer-tests.yml new file mode 100644 index 0000000000..66f249ffa2 --- /dev/null +++ b/.github/workflows/installer-tests.yml @@ -0,0 +1,38 @@ +name: Installer tests + +# scripts/install.ps1's PowerShell tests. They exercise the installer as a real +# subprocess, and every path contract they assert (8.3 short-name aliases, +# Git Bash layouts, provider-cmdlet behavior) is Windows-specific — so they need +# a Windows runner. Before this workflow existed the files were in the tree but +# nothing ever ran them. + +on: + workflow_call: + +permissions: + contents: read + +concurrency: + group: installer-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + powershell: + name: PowerShell installer tests + runs-on: windows-latest + timeout-minutes: 15 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + # Windows PowerShell 5.1 as well as pwsh 7: install.ps1 is delivered via + # `irm | iex` into whatever shell the user already has, and 5.1 is what + # ships with Windows. A construct that only parses under 7 is a broken + # installer for most of the people hitting it. + - name: 8.3 short-path normalization (pwsh 7) + shell: pwsh + run: pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/tests/test-install-ps1-longpath.ps1 + + - name: 8.3 short-path normalization (Windows PowerShell 5.1) + shell: powershell + run: powershell -NoProfile -ExecutionPolicy Bypass -File scripts/tests/test-install-ps1-longpath.ps1 diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index d3c915d642..4428a5822f 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -19,6 +19,7 @@ Lanes: * ``scan`` — supply-chain scan (Python files, .pth, setup hooks). * ``deps`` — pyproject.toml dependency bounds check. * ``npm_lock`` — semantic package-lock.json diff PR comment. +* ``installer`` — PowerShell installer tests (Windows runner). * ``mcp_catalog`` — bundled MCP catalog / installer review. Docker is not a lane — it builds on push-to-main and release only, @@ -68,6 +69,11 @@ _SCAN_FILES = {"setup.cfg", "pyproject.toml"} _MCP_CATALOG_PATHS = ("optional-mcps/",) _MCP_CATALOG_FILES = {"hermes_cli/mcp_catalog.py"} +# Windows installer + its PowerShell tests. These only run on a Windows runner, +# so they get their own lane rather than riding along with ``python``. +_INSTALLER_PATHS = ("scripts/tests/",) +_INSTALLER_FILES = {"scripts/install.ps1", "scripts/install.cmd"} + def _is_docs(p: str) -> bool: if p.startswith(("skills/", "optional-skills/")): return False @@ -98,6 +104,10 @@ def _is_mcp_catalog(p: str) -> bool: return p.startswith(_MCP_CATALOG_PATHS) or p in _MCP_CATALOG_FILES +def _is_installer(p: str) -> bool: + return p.startswith(_INSTALLER_PATHS) or p in _INSTALLER_FILES + + def _is_ci_review(p: str) -> bool: if p in _CI_REVIEW_FILES or p.startswith(_CI_REVIEW_PATHS): return True @@ -123,6 +133,7 @@ def classify(files: list[str]) -> dict[str, bool]: "scan": any(_is_scan(f) for f in files), "deps": any(f == "pyproject.toml" for f in files), "npm_lock": any(f.split("/")[-1] == "package-lock.json" for f in files), + "installer": any(_is_installer(f) for f in files), "mcp_catalog": any(_is_mcp_catalog(f) for f in files), "ci_review": any(_is_ci_review(f) for f in files), } @@ -135,6 +146,7 @@ def classify(files: list[str]) -> dict[str, bool]: ret["scan"] = True ret["deps"] = True ret["npm_lock"] = True + ret["installer"] = True ret["ci_review"] = True # explicitly skip mcp catalog here. it's not needed unless those files are modified. diff --git a/tests/ci/test_classify_changes.py b/tests/ci/test_classify_changes.py index 48dc8b10e3..ee5cb8eb10 100644 --- a/tests/ci/test_classify_changes.py +++ b/tests/ci/test_classify_changes.py @@ -30,12 +30,13 @@ DEFAULT = { "scan": True, "deps": True, "npm_lock": True, + "installer": True, "mcp_catalog": False, "ci_review": True, } -def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, npm_lock=False, mcp_catalog=False, docker_meta=False, ci_review=False, python_prod=None) -> dict[str, bool]: +def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, npm_lock=False, installer=False, mcp_catalog=False, docker_meta=False, ci_review=False, python_prod=None) -> dict[str, bool]: # python_prod tracks python except for tests-only diffs; default it to # python so the majority of cases don't need to spell it out. return { @@ -47,6 +48,7 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, npm "scan": scan, "deps": deps, "npm_lock": npm_lock, + "installer": installer, "mcp_catalog": mcp_catalog, "ci_review": ci_review, } @@ -67,6 +69,14 @@ CASES = { # skill edit must still run Python. "skill md → python + site": (["skills/github/SKILL.md"], _lanes(python=True, site=True)), "dockerfile → docker meta": (["Dockerfile"], _lanes(docker_meta=True)), + # install.ps1 is a shell script Python never imports, but it's also not + # provably prose, so python stays on (fail-open) alongside the Windows lane. + "install.ps1 → installer": (["scripts/install.ps1"], _lanes(python=True, installer=True)), + "installer test → installer": ( + ["scripts/tests/test-install-ps1-longpath.ps1"], + _lanes(python=True, installer=True), + ), + "python source alone → no installer lane": (["run_agent.py"], _lanes(python=True, scan=True)), # Unknown top-level file keeps Python on rather than risk a silent skip. "unknown toplevel → python": (["Makefile"], _lanes(python=True)), "mixed docs+python → python": (["README.md", "agent/x.py"], _lanes(python=True, scan=True)),