fix(desktop): stop telling installer users they installed Hermes from the command line - #66287
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good
- Fixes incorrect messaging: desktop installer was telling users they installed from command line even when they used the installer
- Clean fix updating the conditional text
- No security concerns
Reviewed by Hermes Agent
|
Thanks for tracing the installer layouts and keeping the change local to the desktop update seam. Problems
Suggested changes
Current main still unconditionally calls this a CLI install in |
|
Thanks for the catch on the progress-event path. Added a Evidence (from |
…al-update fallback The Windows manual fallback assumed a missing staged updater means a CLI install. Installer-deployed desktops (NSIS/MSI shell outside the managed checkout, or a Tauri Hermes-Setup run whose best-effort self-copy failed) land in the same branch and get told they installed from the command line (NousResearch#66095). Add a pure classifier so the fallback can tell them apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…back applyUpdates() now classifies the install before surfacing the manual `hermes update` fallback and carries the result (installKind) on both the apply result and the progress event, so the renderer can stop claiming installer users installed from the command line (NousResearch#66095). The Tauri installer breadcrumb probed is HERMES_HOME/logs/bootstrap-installer.log, written unconditionally by Hermes-Setup even when its best-effort updater self-copy failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tops The manual dialog told every user without a staged updater 'You installed Hermes from the command line' — false for official installer users (NousResearch#66095: NSIS/MSI shells never get hermes-setup.exe staged, and the Tauri installer's self-copy is best-effort). Thread installKind from the apply result / progress stream through the update store and show a missing-updater explanation instead; the terminal command stays the same because it is the working recovery path either way. New copy localized for en/zh/zh-hant/ja. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The installKind interpolation pushed the rememberLog call in applyUpdates() to 125 chars, past the repo printWidth of 120, so `prettier --check electron/main.ts` failed. Output of `npx prettier --write electron/main.ts`; whitespace only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
7c586f8 to
48b4cec
Compare
|
Closing this P3 installer-copy PR so review attention can go to higher-impact Windows/desktop fixes we still have open. |
What does this PR do?
Windows users who installed Hermes with an official installer get a false dialog when they click Update: "You installed Hermes from the command line, so updates run there too." (#66095, reported with filesystem evidence.)
Root cause: the manual fallback in
applyUpdates()(apps/desktop/electron/main.ts) treats a missing staged updater (HERMES_HOME/hermes-setup.exe) as proof of a CLI install. Two installer flows land in that branch:Hermes.exeoutside the managed checkout, and its first-launch bootstrap (bootstrap-runner.tsdrivinginstall.ps1) never stageshermes-setup.exe. Only the Tauri Hermes-Setup installer stages that binary (paths::copy_self_to_hermes_home). This matches the reporter's evidence exactly: repo cloned under%LOCALAPPDATA%\hermes\hermes-agent, nohermes-setup.exe, no.install_methodinHERMES_HOME.bootstrap.rslogs a warning and continues on failure), so that flow can also end up without the staged updater.This PR tells the two cases apart and shows an honest explanation. The recovery command (branch-pinned
hermes update) stays the same because it works for installer users too:install.ps1'sSet-PathVariablepersists the venvScriptsdir to the user PATH.Detection lives in a new pure module,
apps/desktop/electron/update-install-kind.ts:process.execPathis outsideresolveUpdateRoot()is installer-deployed (a CLIhermes desktoplaunch always runs the packed exe from<checkout>/apps/desktop/release/*-unpacked/, per_desktop_packaged_executableinhermes_cli/main.py);HERMES_HOME/logs/bootstrap-installer.logis a Hermes-Setup breadcrumb, created byinit_logging()at installer startup, so it exists even when the self-copy failed;Scope notes: this PR does not make the installers stage
hermes-setup.exe(a packaging pipeline change; the NSIS shell would have to ship the separate Tauri updater binary), and it does not touch the backend's.install_methoddetection (#61827). The adjacent updater PRs solve different failure modes: #37748 validates a staged updater that exists, #61093 finds one staged in a different Hermes home. Here the updater never existed.Related Issue
Fixes #66095
Type of Change
Changes Made
apps/desktop/electron/update-install-kind.ts(new): pureclassifyWindowsManualUpdate()with win32 path-containment semantics (case-insensitive, both separators, no sibling-prefix false positives).apps/desktop/electron/update-install-kind.test.ts(new): 7 unit tests covering the NSIS layout from the issue, the failed-self-copy Tauri case, case/separator variance, sibling-prefix dirs, dev runs, and missing inputs.apps/desktop/electron/main.ts: the manual fallback computesinstallKindand carries it on the apply result and thehermes:updates:progresspayload; the log line now records which kind was detected.apps/desktop/src/global.d.ts,apps/desktop/src/store/updates.ts: threadinstallKindthroughUpdateApplyStateon both the result and progress paths.apps/desktop/src/app/updates-overlay.tsx:ManualViewshows the missing-updater body wheninstallKind === 'installer'.apps/desktop/src/i18n/{types,en,zh,zh-hant,ja}.ts: newupdates.manualBodyMissingUpdaterstring.apps/desktop/src/store/updates.test.ts: 3 store cases (cli kind, installer kind, and null default when an older main process omits the field).How to Test
%LOCALAPPDATA%\hermes\hermes-setup.exefrom a Hermes-Setup install to simulate the failed self-copy).hermes updatecommand.Verification I actually ran (Windows 11, this branch):
(An earlier revision of this branch had one log line in
main.tsover the Prettier print width; the last commit wraps it, and the check above is from the final tree.)A full
vitest runinapps/desktopshows 6 failing test files (scripts/before-pack.test.mjs,scripts/stage-native-deps.test.mjs,electron/update-relaunch.test.ts,electron/windows-hermes-path.test.ts,src/app/messaging/index.test.tsx,src/app/skills/index.test.tsx). I stashed my changes and re-ran on a clean origin/main checkout: the same 6 files fail there, so they are pre-existing on this machine and unrelated to this change.Honest boundary: I verified the detection and dialog logic at the unit level plus typecheck and lint. I did not build the NSIS installer artifact or drive a packaged desktop end to end; the classifier tests encode the exact filesystem layouts the two install flows produce.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (not run: this PR touches only apps/desktop TypeScript, no Python; I ran the desktop vitest projects, tsc, and eslint instead, output above)Documentation & Housekeeping
docs/, docstrings) — or N/A (behavior documented in code comments; no docs page describes the manual dialog)cli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
See the command output in "How to Test" above.
Written by Stan Shih (stantheman0128) with Claude Code assisting on implementation and tests.