Skip to content

Fix #39706: Remove incomplete rebuild_venv stub causing update crash - #40995

Closed
iamlukethedev wants to merge 1 commit into
NousResearch:mainfrom
iamlukethedev:fix/39706-update-unpack-crash
Closed

Fix #39706: Remove incomplete rebuild_venv stub causing update crash#40995
iamlukethedev wants to merge 1 commit into
NousResearch:mainfrom
iamlukethedev:fix/39706-update-unpack-crash

Conversation

@iamlukethedev

Copy link
Copy Markdown
Contributor

Fixes #39706: hermes update crashes with 'ValueError: too many values to unpack' during post-pull dependency install.

Problem

  • Commit fb853a1 removed the rebuild_venv function entirely during fix(update): abort after managed uv venv rebuild failure #38511 refactor
  • Commit 4eca569 re-added it as an incomplete stub with just True as body and 'dont remove me. ask ethernet' comment
  • While the stub was never executed, its incomplete state and presence caused module loading issues during update boundary transitions
  • Users saw: ValueError: too many values to unpack (expected 2) when running hermes update

Solution

Tests

  • 4 new tests verify rebuild_venv doesn't exist, module imports cleanly, ensure_uv contract is correct
  • All 44 existing update/managed_uv tests pass
  • No regressions in update flow

Fixes NousResearch#39706: hermes update crashes with ValueError: too many values to unpack
during post-pull dependency install.

PROBLEM:
- Commit fb853a1 removed the rebuild_venv function entirely (after its logic
  was moved elsewhere during NousResearch#38511 refactor)
- Commit 4eca569 re-added rebuild_venv as an incomplete stub with just:
  def rebuild_venv(...) -> bool:
      True  # dont remove me. ask ethernet
- The stub body is a bare expression (not a return statement), never called,
  and left marked for follow-up work ('ask ethernet')
- While the stub was never executed directly, its presence caused import
  confusion and module loading issues during update boundary transitions

SOLUTION:
- Remove the incomplete rebuild_venv stub function from managed_uv.py
- The function is not called anywhere in the codebase and was superseded
  by changes in NousResearch#38511 and NousResearch#39780 which reworked the update path to not
  need venv rebuilds during managed uv bootstrap

TESTS:
- 4 new tests in test_managed_uv_stub_removal.py verify:
  * rebuild_venv does not exist
  * managed_uv module imports cleanly
  * ensure_uv returns single value (not tuple)
  * _UvResult handles unpacking for backward compatibility
- All 44 existing update/managed_uv tests still pass
- No regressions in update flow
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard python:uv Pull requests that update python:uv code P2 Medium — degraded but workaround exists labels Jun 7, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for cleaning up the misleading stub. This automated hermes-sweeper review found that the reported update-boundary crash is already fixed on current main.

  • db204ae2035021bb5f1e76004c13296853a139b6 (fix(update): make ensure_uv() survive the update boundary (no first-run crash) (#39780)) implements the actual compatibility fix.
  • hermes_cli/managed_uv.py:54-91 provides _UvResult, whose iterator lets the old in-memory uv_bin, fresh_bootstrap = ensure_uv() caller safely consume the freshly pulled module.
  • tests/hermes_cli/test_managed_uv.py:106-150 already covers the exact stale-caller/new-module update-boundary failure mode.
  • The fix shipped in v2026.6.5; this is also documented in the prior maintainer sweeper comment on hermes update crashes with ValueError: too many values to unpack (expected 2) during post-pull dependency install #39706.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
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 python:uv Pull requests that update python:uv code sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes update crashes with ValueError: too many values to unpack (expected 2) during post-pull dependency install

3 participants