mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 03:30:36 +00:00
fix(start.ps1): address Copilot review on #2805 — PathType+null-guard+changelog
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>
This commit is contained in:
+1
-1
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user