Commit graph

7 commits

Author SHA1 Message Date
brooklyn!
d880b5be09
fix(update/windows): don't return _UvResult on Windows (subprocess argv crash) (#39820)
PR #39780 made ensure_uv() return a _UvResult — a str subclass whose
__iter__ yields (path, fresh_bootstrap) so old `uv_bin, fresh = ensure_uv()`
call sites survive the update boundary. That trick is unsafe on Windows.

The dependency installer passes uv straight into the command list
(`[uv_bin, "pip", "install", ...]`). On Windows, subprocess serializes argv
via subprocess.list2cmdline, which iterates every entry *as a string*
(`for c in arg`). Because _UvResult overrides __iter__, that iteration yields
(path, fresh_bootstrap) instead of characters, injecting the bool into the
command line and crashing the first update with:

    TypeError: sequence item 1: expected str instance, bool found

This bites the common single-assignment caller (`uv_bin = ensure_uv()`) on
its first update after #39780: the freshly pulled _UvResult flows into the
old in-memory call site and into the argv. Reported in the field on a
~10-commits-behind Windows install.

A single return value cannot satisfy both legacy 2-target unpacking and
Windows char-iteration — both use the iterator protocol with contradictory
results. So gate the wrapper to POSIX: Windows returns a plain str/None
(the historical, subprocess-safe contract). POSIX keeps _UvResult and the
#39780 update-boundary fix.

Tests: list2cmdline canary proving _UvResult breaks Windows, plus Windows
returns-plain-str and POSIX dual-contract coverage.
2026-06-05 07:54:08 -05:00
brooklyn!
db204ae203
fix(update): make ensure_uv() survive the update boundary (no first-run crash) (#39780)
* fix(update): make ensure_uv() survive the update boundary (no first-run crash)

`hermes update` runs the `ensure_uv()` call site from the old, already-imported
`hermes_cli.main` against the *freshly pulled* `managed_uv` (managed_uv is only
ever lazily imported, so it loads from disk post-pull). `ensure_uv()`'s return
arity flipped from a single path string to `(path, fresh_bootstrap)` (4df280d51)
and back to a single string (fb853a178). Installs parked on a 2-tuple release
unpack `uv_bin, fresh_bootstrap = ensure_uv()` against the new single-value
module and crash the first update with
`ValueError: not enough values to unpack (expected 2, got 1)` — inside the
dependency-install step, *before* the PR #39763 subprocess hand-off can run.

Return a `_UvResult` (a `str` subclass) that is usable as the bare path AND
unpackable as `(path|None, fresh_bootstrap)`. Missing uv is `""` (falsy) instead
of `None` so legacy 2-target call sites can unpack a failure without raising,
while `if not uv_bin` keeps working for single-value callers. fresh_bootstrap is
always False (the rebuild-venv path it gated was scrapped in fb853a178).

* docs(update): correct the verified error string + mechanism for ensure_uv()

A hermetic repro (old 2-target call site vs the freshly-pulled single-value
module) shows the first-update crash is exactly the string from PR #39763's
report: `ValueError: too many values to unpack (expected 2)` — not "not enough".
The returned path is a plain `str`, which is iterable, so `uv_bin, fresh =
ensure_uv()` walks its characters; the failure path's `None` return raises
`TypeError: cannot unpack non-iterable NoneType`. Both are fixed by `_UvResult`.
Comment/test wording updated to match; no behavior change.
2026-06-05 07:08:43 -05:00
ethernet
4eca569bf4 fix: temp for update 2026-06-04 23:32:48 -04:00
ethernet
fb853a1783 fix(install): scrap rebuild venv 2026-06-04 23:20:29 -04:00
Jeff
1f347ee543 fix(uv): move venv aside instead of gutting it in place on Windows rebuild
hermes update can brick a Windows install. When 'hermes update --force' runs
past the concurrent-process guard, rebuild_venv runs while the venv is still in
use: shutil.rmtree(ignore_errors=True) deletes site-packages + certifi's cert
bundle but can't remove the locked python.exe, leaving a half-gutted venv that
uv venv then refuses to overwrite. Every later HTTPS call dies with
FileNotFoundError for the missing cacert and there is no recovery.

--clear alone (the c136eb4de retry path) does not fix the real lock case: when
the locked interpreter is *inside* the venv being rebuilt, neither rmtree nor
uv venv --clear can delete it. os.replace of the parent directory *is* allowed
on Windows (a running .exe is tracked by handle, not path), so we move the old
venv aside atomically to <venv>.old, rebuild with --clear in its place, and the
still-running gateway/desktop keep using the moved-aside copy until they
restart. If the venv genuinely can't be moved, we abort cleanly and leave it
fully intact; if the rebuild fails, we restore the moved-aside copy.

Folds in the call-site guards from #38511 (@f3rs3n):
- rebuild_venv() returns False (and restores the backup) if uv exits 0 without
  producing an interpreter.
- both hermes update venv-rebuild call sites abort with RuntimeError instead of
  continuing into dependency install when rebuild_venv() returns False.

Also gitignore /venv.old/ so the update autostash (git stash --include-untracked)
doesn't sweep the moved-aside venv on every run.

Root-cause fix for #37881. Supersedes the --clear-only retry from c136eb4de.

Co-authored-by: f3rs3n <32328813+f3rs3n@users.noreply.github.com>
2026-06-04 12:18:38 -04:00
teknium1
c136eb4de1 fix(update): harden venv rebuild + verify core deps after install
Two complementary fixes for a silent partial-install failure that bit
``hermes update`` in the wild: a fresh checkout pulled 145 commits,
``rebuild_venv`` failed to recreate the venv on Windows because
``shutil.rmtree(ignore_errors=True)`` couldn't delete files held open by
the running ``hermes.exe`` shim. ``uv venv`` then refused with
"A directory already exists at: venv" and the update fell back to
installing on top of the stale venv. The resulting partial install
missed exactly one newly-added base dep — ``pathspec==1.1.1`` — which
``hermes desktop --build-only`` imports at the top of its content-hash
check. The desktop rebuild died with ModuleNotFoundError and the parent
update only logged "⚠ Desktop build failed (non-fatal)". Same root cause
made the "default: sync failed" line in the skill-sync stage, because
that sync subprocess hit the same missing import.

Fix 1: ``rebuild_venv`` retries with ``--clear``
------------------------------------------------
If ``uv venv`` fails with "already exists" in stderr (which is what uv
prints, and what uv's own hint tells you to fix with --clear), retry
once with ``--clear``. Only this specific failure pattern triggers the
retry — disk-full / interpreter-download failures still surface as
before so we don't mask real problems.

Fix 2: post-install dep verification
------------------------------------
Belt-and-suspenders so future uv resolver quirks (or any other cause of
partial installs) surface immediately instead of hours later in a
downstream subprocess. After ``_install_python_dependencies_with_optional_fallback``
runs, ``_verify_core_dependencies_installed``:

  1. Reads ``[project.dependencies]`` straight from pyproject.toml
     (so we don't trust the venv's stale metadata).
  2. Filters by environment markers via ``packaging.requirements.Requirement``
     so cross-platform exclusions (``ptyprocess ; sys_platform != 'win32'``)
     don't false-positive on Windows.
  3. Runs ``importlib.metadata.version()`` for each remaining dep inside
     the *target* venv interpreter (resolved from ``VIRTUAL_ENV``, not
     ``sys.executable``).
  4. If anything is missing, reinstalls the base group with
     ``--reinstall`` to force re-resolution. If a second probe still
     reports missing deps, force-installs each one with its pinned spec.
  5. Treats final failure as a warning rather than a hard error — a
     single broken-on-PyPI dep shouldn't block an otherwise-successful
     update — but the message points at ``hermes update --force`` and
     names the missing packages so the user knows what's wrong.

Tests
-----
- ``TestRebuildVenv::test_retries_with_clear_when_dir_already_exists`` —
  simulates the rmtree-couldn't-delete-it failure mode and asserts the
  ``--clear`` retry path is taken and succeeds.
- ``TestRebuildVenv::test_does_not_retry_when_first_failure_is_not_dir_exists``
  — guards against masking real failures (disk full, etc.).
- ``test_verify_core_dependencies.py`` — 7 tests covering the happy
  path, the regression (missing pathspec triggers --reinstall), the
  per-package fallback when --reinstall doesn't help, the platform-
  marker filter so Windows doesn't try to install ptyprocess, the
  missing-pyproject noop, and the VIRTUAL_ENV resolver.

Co-authored-by: Kyssta <218078013+kyssta-exe@users.noreply.github.com>
2026-06-04 06:05:41 -07:00
ethernet
4df280d511 refactor(uv): single managed-uv path, delete fts5 installer escalation
Replace the multi-path UV resolution chain (PATH probing, conda guards,
5-location trust ordering, temp-dir fallback installs) with a single
managed uv binary at $HERMES_HOME/bin/uv. Every code path that needs
uv resolves it from that one location; if missing, ensure_uv()
bootstraps it via the official standalone installer.

Key changes:

- New hermes_cli/managed_uv.py: managed_uv_path(), resolve_uv(),
  ensure_uv() (returns (path, freshly_bootstrapped) tuple),
  update_managed_uv(), rebuild_venv(), installer internals.
- hermes_cli/main.py: replace all shutil.which('uv') with ensure_uv(),
  add venv rebuild on first-time managed uv bootstrap, update_managed_uv
  before dep install on all 3 update paths.
- scripts/install.sh: install_uv() always installs to
  $HERMES_HOME/bin/uv; delete ensure_fts5, _python_has_fts5,
  _reinstall_python_with_fts5, _warn_no_fts5 (61 lines).
  Managed uv always installs current Python with FTS5.
- scripts/install.ps1: Install-Uv always installs to
  $HermesHome\bin\uv.exe; Resolve-UvCmd checks managed location first.
- hermes_state.py: simplified FTS5 warning now suggests 'hermes update'
  as the fix instead of blaming install method.
- tests: 15 tests in test_managed_uv.py, autouse _patch_managed_uv
  fixture in test_cmd_update.py.

Closes #37605, Closes #37622
2026-06-02 20:29:54 -04:00