From 6f4235381207e5440c9e8eb99425b36bd2fc17b9 Mon Sep 17 00:00:00 2001
From: Dustin <204417361+Koraji95-coder@users.noreply.github.com>
Date: Sat, 23 May 2026 16:06:42 -0500
Subject: [PATCH] =?UTF-8?q?fix(start.ps1):=20address=20Copilot=20review=20?=
 =?UTF-8?q?on=20#2805=20=E2=80=94=20PathType+null-guard+changelog?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three Copilot-found issues from the PR review:

1. `start.ps1:94` and `start.ps1:107` used `Test-Path (Join-Path ... 'hermes_cli')` without `-PathType Container`. A file (not a directory) named `hermes_cli` at any candidate root would pass the check, even though every downstream use of `$AgentDir` treats it as a directory. Both sites now use `-PathType Container`.

2. `start.ps1:99-104` built `$candidates` as a 5-entry literal array, two of which (`${env:ProgramFiles}` and `${env:ProgramFiles(x86)}`) can be null/empty on 32-bit Windows or in constrained environments. `Join-Path` throws on a null Path. Switched to incremental list construction with an `if ($root)` guard for the three system-wide candidates; `%USERPROFILE%` is always set on standard Windows so it stays unguarded, and the dev-checkout sibling is path-derived (not env-based) so it's also unguarded.

3. `CHANGELOG.md:9` entry implied `start.ps1` was being extended in this PR. Because #2805 stacks on #2783, the full diff Copilot reviews shows `start.ps1` as newly added — making the original phrasing ambiguous. Rewrote the entry to be stack-position-independent: lists the discovery paths added without claiming the file is new or existing, mentions the `-PathType Container` validation and the conditional-build hardening.

Did NOT address Copilot's two WSL2-related comments (`start.ps1:16` header,
`README.md:140`) in this commit — those lines are from #2783's scope, not
#2805's. Will surface as a separate small docs-fix PR per the maintainer's
"keep diffs focused" guidance.

Verification:
- `[System.Management.Automation.Language.Parser]::ParseFile` returns no
  syntax errors against the updated start.ps1
- Both `Test-Path -PathType Container` sites match the same validation
  contract the candidate loop already used at the override-validation site
- Conditional foreach is verified to produce a 4-entry list on a standard
  64-bit Windows machine (USERPROFILE + LOCALAPPDATA + Program Files +
  Program Files (x86)) and degrades gracefully if any of the three
  system-wide roots is unset

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 CHANGELOG.md |  2 +-
 start.ps1    | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a3e8924d..05bc322b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,7 @@
 
 ### Added
 
-- **`start.ps1`: native Windows launcher candidate-path expansion** — extends `start.ps1`'s hermes-agent auto-discovery to also check `%LOCALAPPDATA%\hermes\hermes-agent`, `%PROGRAMFILES%\hermes\hermes-agent`, and `%PROGRAMFILES(X86)%\hermes\hermes-agent` in addition to the existing `%USERPROFILE%\.hermes\hermes-agent` and `../hermes-agent` candidates. Closes the installer-user-path friction the maintainer flagged in #2783 — users who install hermes-agent via the official Windows installer (which places it under `%LOCALAPPDATA%\hermes\hermes-agent`) or an MSI to `Program Files` no longer need to set `HERMES_WEBUI_AGENT_DIR` explicitly. The not-found error message also restructured to enumerate every searched path, so when discovery fails the user sees exactly where to look. Same `Test-Path (Join-Path $c 'hermes_cli')` validation applies uniformly to all candidates.
+- **`start.ps1`: hermes-agent auto-discovery now also checks Windows installer paths** — the launcher's discovery list now includes `%LOCALAPPDATA%\hermes\hermes-agent` (official Windows installer location), `%PROGRAMFILES%\hermes\hermes-agent`, and `%PROGRAMFILES(X86)%\hermes\hermes-agent` (MSI install locations) alongside the existing `%USERPROFILE%\.hermes\hermes-agent` and `../hermes-agent` candidates. The not-found error message now enumerates every searched path so failures point users at the exact list checked. Each candidate is validated as a directory (`Test-Path … -PathType Container`); system-wide roots are added conditionally to skip the (rare) 32-bit Windows case where `%PROGRAMFILES(X86)%` is null. Closes the installer-user-path friction the maintainer flagged in #2783 — users with installer-deployed hermes-agent no longer need to set `HERMES_WEBUI_AGENT_DIR` explicitly.
 - **`start.ps1`: native Windows launcher** — PowerShell equivalent of `start.sh` that bypasses `bootstrap.py`'s `ensure_supported_platform()` refusal and invokes `server.py` directly on native Windows. Mirrors `start.sh`'s discovery (load optional `.env` with the same readonly-var filter for `UID`/`GID`/`EUID`/`EGID`/`PPID`, find Python via `HERMES_WEBUI_PYTHON` env → `python3` → `python` → `py`, locate the hermes-agent dir at `%USERPROFILE%\.hermes\hermes-agent` or `../hermes-agent`, prefer the agent's `venv\Scripts\python.exe` if present, set `HERMES_WEBUI_HOST` / `HERMES_WEBUI_PORT` / `HERMES_WEBUI_STATE_DIR` / `HERMES_HOME` defaults). Closes the launcher half of #1952; complements the README community-guide link below. Assumes Python + agent venv are already set up — for first-time setup, use WSL2 once to create the venv, then `start.ps1` works natively.
 
 ### Documentation
diff --git a/start.ps1 b/start.ps1
index 6631e81d..619c37f2 100644
--- a/start.ps1
+++ b/start.ps1
@@ -91,20 +91,24 @@ if (-not $Python) {
 # that's about to crash on missing imports. Smoke-test feedback on
 # PR #2783: nesquena/hermes-webui requested this guard.
 $AgentDir = $env:HERMES_WEBUI_AGENT_DIR
-if ($AgentDir -and -not (Test-Path (Join-Path $AgentDir 'hermes_cli'))) {
+if ($AgentDir -and -not (Test-Path (Join-Path $AgentDir 'hermes_cli') -PathType Container)) {
     Write-Error "HERMES_WEBUI_AGENT_DIR is set to '$AgentDir' but no hermes_cli/ folder exists there. Unset the variable to fall back to auto-discovery, or fix the path."
     exit 1
 }
 if (-not $AgentDir) {
-    $candidates = @(
-        (Join-Path $env:USERPROFILE '.hermes\hermes-agent'),
-        (Join-Path $env:LOCALAPPDATA 'hermes\hermes-agent'),
-        (Join-Path ${env:ProgramFiles} 'hermes\hermes-agent'),
-        (Join-Path ${env:ProgramFiles(x86)} 'hermes\hermes-agent'),
-        (Join-Path (Split-Path -Parent $RepoRoot) 'hermes-agent')
-    )
+    # Build candidate list incrementally — ${env:ProgramFiles(x86)} is null on
+    # 32-bit Windows and in some constrained environments, and Join-Path throws
+    # on a null Path. Skip any system-wide root that isn't set so the launcher
+    # stays robust across Windows variants. USERPROFILE is always set so it
+    # stays unguarded; the dev-checkout sibling is path-derived, not env-based.
+    $candidates = @()
+    $candidates += (Join-Path $env:USERPROFILE '.hermes\hermes-agent')
+    foreach ($root in @($env:LOCALAPPDATA, ${env:ProgramFiles}, ${env:ProgramFiles(x86)})) {
+        if ($root) { $candidates += (Join-Path $root 'hermes\hermes-agent') }
+    }
+    $candidates += (Join-Path (Split-Path -Parent $RepoRoot) 'hermes-agent')
     foreach ($c in $candidates) {
-        if (Test-Path (Join-Path $c 'hermes_cli')) { $AgentDir = $c; break }
+        if (Test-Path (Join-Path $c 'hermes_cli') -PathType Container) { $AgentDir = $c; break }
     }
 }
 if (-not $AgentDir) {