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/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 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 } 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)),