Skip to content

fix(windows): Split hermes update into two-phase pull + post-pull with re-exec - #39325

Closed
ethernet8023 wants to merge 7 commits into
mainfrom
ethie/windows-installer-fixes
Closed

ethernet8023 wants to merge 7 commits into
mainfrom
ethie/windows-installer-fixes

Conversation

@ethernet8023

Copy link
Copy Markdown
Collaborator

What does this PR do?

Splits hermes update into a two-phase pull + post-pull flow with process re-exec, and fixes Windows installer robustness when .git is missing or the clone is partial.

The old monolith (_cmd_update_impl, ~1350 lines) ran post-pull steps (pip install, node deps, skills sync, gateway restart, etc.) under stale bytecode after git pull or ZIP extraction overwrote source files on disk. This caused ImportError on gateway restart and subtle drift bugs because sys.modules / __pycache__ still held the old code.

Phase 1 (_cmd_update_pull_new_version): concurrent guard, backup, git pull / ZIP download+extract, stash handling, syntax guard, rollback, bytecode clear. Returns True if new code landed.

Phase 2 (_cmd_update_post_pull): pip install (managed-uv), node deps, web UI, desktop rebuild, skills sync, config migration, gateway restart, cleanup — now running under freshly-exec'd Python with clean sys.modules.

Between phases, _reexec_for_post_pull replaces the process:

  • POSIX: os.execvp (true exec, same PID, fresh modules)
  • Windows: subprocess.run relay (parent stays alive so bootstrap installer's child.wait() sees the real exit code)

Additional changes:

  • Pip self-update removed entirely. pip/uv/pipx installs now error with recommended_update_command guidance (same as managed installs).
  • Windows installer (install.ps1) now verifies remote reachability (git ls-remote) before assuming a clone is valid.
  • Bootstrap installer (update.rs) comment updated to reflect new file path (hermes_cli/update.py).
  • Allow uv to come from termux.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/main.py — Split _cmd_update_impl into _cmd_update_pull_new_version + _cmd_update_post_pull + _reexec_for_post_pull; rename _update_files_via_zip_download_and_extract_zip (thin delegation wrapper); remove _cmd_update_pip; add --post-pull / --pre-update-snapshot hidden args; pip/uv-tool install guard
  • hermes_cli/managed_uv.py — Allow uv from termux
  • apps/bootstrap-installer/src-tauri/src/update.rs — Comment path fix (main.py → update.py)
  • scripts/install.ps1 — Add git ls-remote reachability check before deeming clone valid
  • tests/hermes_cli/test_cmd_update.py_inline_post_pull autouse fixture; updated mocks for two-phase flow
  • tests/hermes_cli/test_update_autostash.py_inline_post_pull autouse fixture
  • tests/hermes_cli/test_update_concurrent_quarantine.py_inline_post_pull autouse fixture
  • tests/hermes_cli/test_managed_uv.py — Updated for managed-uv changes
  • tests/hermes_cli/test_uv_tool_update.py — Removed _cmd_update_pip tests (function deleted); kept is_uv_tool_install + recommendation helpers
  • tests/hermes_cli/test_update_zip_symlink_reject.py — Renamed imports to match _update_files_via_zip delegation wrapper
  • tests/hermes_cli/test_update_venv_ready.py — Removed (covered by new two-phase flow)
  • PLAN.md — Design document for the two-phase refactor (added)

How to Test

  1. Run hermes update on a POSIX system — should complete two-phase flow with re-exec, same visible output as before
  2. Run hermes update on a Windows system without .git — should use ZIP fallback with subprocess relay
  3. Run hermes update from a pip/uv-tool install — should error with recommended update command
  4. Run pytest tests/hermes_cli/test_cmd_update.py tests/hermes_cli/test_update_autostash.py tests/hermes_cli/test_update_concurrent_quarantine.py tests/hermes_cli/test_uv_tool_update.py tests/hermes_cli/test_managed_uv.py -q
  5. Verify the bootstrap installer's child.wait() still gets correct exit codes on Windows

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux (NixOS)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: ethie/windows-installer-fixes vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9839 on HEAD, 9840 on base (✅ -1)

🆕 New issues (1):

Rule Count
invalid-argument-type 1
First entries
hermes_cli/main.py:10790: [invalid-argument-type] invalid-argument-type: Argument to function `restore_cron_jobs_if_emptied` is incorrect: Expected `str`, found `Any | None`

✅ Fixed issues (1):

Rule Count
invalid-argument-type 1
First entries
hermes_cli/main.py:10880: [invalid-argument-type] invalid-argument-type: Argument to function `restore_cron_jobs_if_emptied` is incorrect: Expected `str`, found `None | str`

Unchanged: 5101 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard platform/whatsapp WhatsApp Business adapter labels Jun 4, 2026
@ethernet8023
ethernet8023 force-pushed the ethie/windows-installer-fixes branch from 2c9303b to 5cc1a91 Compare June 5, 2026 00:24
…h re-exec

_cmd_update_impl (monolith ~1350 lines) is gone. The update flow is now:

Phase 1 (_cmd_update_pull_new_version):
  - concurrent guard, backup, git pull / ZIP download+extract
  - stash handling, syntax guard, rollback, bytecode clear

Phase 2 (_cmd_update_post_pull):
  - pip install (managed-uv), node deps, web UI, desktop rebuild
  - skills sync, config migration, gateway restart, cleanup

Between phases, _reexec_for_post_pull replaces the process:
  - POSIX: os.execvp (true exec, same PID, fresh sys.modules)
  - Windows: subprocess.run relay (parent stays alive so bootstrap
    installer's child.wait() sees the real exit code)

Hidden --post-pull flag routes directly to phase 2.
--pre-update-snapshot carries snapshot ID across exec boundary.

Pip self-update is removed entirely. pip/uv/pipx installs now error
with recommended_update_command guidance, same as managed installs.

Also allow uv to come from termux.

Test updates:
  - _inline_post_pull autouse fixture in test_cmd_update,
    test_update_autostash, test_update_concurrent_quarantine
    (patches _reexec_for_post_pull to call _cmd_update_post_pull
    in-process, preventing os.execvp from nuking pytest)
  - test_uv_tool_update: removed _cmd_update_pip tests (function
    deleted), kept is_uv_tool_install + recommendation helpers
  - test_update_zip_symlink_reject: renamed imports to match
    _update_files_via_zip (now a thin delegation wrapper)
@ethernet8023
ethernet8023 force-pushed the ethie/windows-installer-fixes branch from 5cc1a91 to 6921469 Compare June 5, 2026 01:16
@ethernet8023
ethernet8023 force-pushed the ethie/windows-installer-fixes branch from e2f6767 to 55879eb Compare June 5, 2026 01:33
@mikeumus

mikeumus commented Jun 5, 2026

Copy link
Copy Markdown

Confirming this PR fixes a real, reproducible crash — sharing the root-cause analysis in case it's useful for the description / a linked issue (the "Fixes #" is currently blank).

Symptomhermes update dies right after → Updating Python dependencies... with a garbled traceback whose carets point at comments/brackets and whose line numbers don't match the code:

  File ".../hermes_cli/main.py", line 10195, in _cmd_update_impl
    ],
^^^^^^^
ValueError: too many values to unpack (expected 2)

Root cause — exactly the stale-bytecode-after-pull race this PR targets:

  1. The running process loads the old monolithic _cmd_update_impl, which does its from hermes_cli.managed_uv import ensure_uv lazily, after git pull.
  2. git pull overwrites managed_uv.py on disk, so that lazy import binds the new ensure_uv().
  3. Old caller: uv_bin, fresh_bootstrap = ensure_uv() — but new ensure_uv() returns a bare path string (the (path, fresh) tuple was removed in refactor(uv): single managed-uv path, delete fts5 installer escalation #37660). Unpacking a string into two names iterates its characters → too many values to unpack (expected 2).
  4. The garbled traceback is the tell: linecache reads the new on-disk source at the old line numbers.

So the breaking hop is any update that crosses #37660 (tuple → bare path) while still executing the pre-split updater. The two-phase pull + os.execvp re-exec here is the correct fix — post-pull runs under freshly-loaded code, so caller and callee can never be from different versions again.

(Inherent caveat, not a critique: re-exec can't rescue the first hop that installs it, since the crashing run is still the old monolith. After this lands, future skews are prevented.)

Repro on the affected window: be on a commit ≤ #37660's parent, then hermes update across it in one hop.

@mikeumus

mikeumus commented Jun 5, 2026

Copy link
Copy Markdown

Follow-up — a related sharp edge in the shared-clone / desktop interaction that this PR's update.rs touch is the right place to address (or at least document).

Observed: after a CLI hermes update succeeds, reopening the desktop app reports "needs to repair the install", and after repairing it shows e.g. v0.15.1 (+166) 0401176 — i.e. it reset the shared clone back to a detached-HEAD commit ~166 behind origin/main.

Why: the CLI and the desktop drive the same ~/.hermes/hermes-agent clone but target different refs:

  • Desktop was installed as a detached-HEAD checkout of a pinned build commit, and update.rs deliberately passes hermes update --branch <BUILD_PIN_BRANCH> (per the comment at lines ~156-164) so the desktop updates against its own channel.
  • Bare CLI hermes update defaults to main, moving the shared clone to main's tip.

So the two updaters ping-pong the single clone: CLI pushes it to main → desktop's integrity check sees it's off the pinned commit → "repair" resets it back to the pin (now N commits "behind main", though it isn't really tracking main). The "behind" count is misleading because it's comparing the pin against main, not against the desktop's actual target ref.

This isn't a crash, but it's a confusing loop for anyone who uses both the CLI and the desktop. Two things that might help, if in scope here:

  1. Make the "behind" / repair messaging ref-aware — report distance against the desktop's pinned branch, not main, so a freshly-pinned install doesn't read as "166 behind."
  2. Detect cross-updater drift explicitly — if the clone HEAD differs from the desktop's pin because a newer CLI update moved it forward, surface that ("CLI updated this clone to ; desktop is pinned to ") rather than a generic "needs repair," so users understand the two channels.

Happy to file this as its own issue if you'd prefer to keep the PR focused — just flagging since update.rs and the pin/--branch logic live in this change.

@ethernet8023

Copy link
Copy Markdown
Collaborator Author

too complicated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/whatsapp WhatsApp Business adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants