fix(update): preserve activated extras across runtime rebuilds - #73310
fix(update): preserve activated extras across runtime rebuilds#73310hxwvaa wants to merge 3 commits into
Conversation
171291f to
a388699
Compare
a388699 to
4bf614f
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the lost pre-rebuild activation state; the current update flow really does re-detect active features only after the core install (hermes_cli/update_cmd.py:3599-3632), while active_features() depends on an anchor package still being present (tools/lazy_deps.py:1023-1041).
Problems
tools/lazy_deps.py:1095directly calls_venv_pip_install(missing)for a restore. That bypasses_allow_lazy_installs(), although current main definessecurity.allow_lazy_installs: falseas an absolute opt-out (tools/lazy_deps.py:487-509) and documents it as disabling runtime installs entirely (website/docs/user-guide/security.md:796). The added test explicitly encodes this bypass.
Suggested changes
- Preserve the snapshot approach, but gate restore installs with
_allow_lazy_installs()and return the existing skipped status when disabled. Update the regression test to assert that no pip call occurs under the security opt-out. - Add an updater-level test for snapshot capture and propagation; the new helper-only test does not exercise
_cmd_update_impl.
Automated hermes-sweeper review.
SummaryTwo PRs address #72924 through the same pre-rebuild snapshot-and-restore strategy. #73309 restores activated LAZY_DEPS backends but bypasses the install opt-out, while #73310 corrects that security behavior and adds updater-level propagation coverage; neither diff demonstrates preservation of extras outside LAZY_DEPS. Related pull requests
Duplicates#73309 and #73310 substantially duplicate the same pre-rebuild snapshot and allowlisted restoration mechanism; #73310 is the corrected open successor, while #73309 is already closed. Suggested consolidationKeep #73310 open with a salvage path as the Verify-selected best existing partial fix: retain its corrected snapshot-and-guarded-restore implementation for previously activated LAZY_DEPS backends, while explicitly limiting its claimed scope or extending it with evidence for the broader extras named in #72924. Keep #73309 closed as a duplicate of #73310 because it implements the same core fix with the unresolved security-opt-out bypass. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I72924(["issue #72924 (open)"])
subgraph Dup73309 ["PRs duplicating each other"]
P73309["PR #73309 (closed)"]
P73310["PR #73310 (open)"]
end
P73310 -->|best fix| I72924
class I72924 open
class P73309 closed
class P73310 open
class P73310 best
class P73310 target
click I72924 "https://github.com/NousResearch/hermes-agent/issues/72924"
click P73309 "https://github.com/NousResearch/hermes-agent/pull/73309"
click P73310 "https://github.com/NousResearch/hermes-agent/pull/73310"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 24 kB of PR diffs, 9 kB of issue/PR text, 5 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
01c8891 to
c3bf4a2
Compare
|
Updated this PR to cover the additional reproduction in #72924. Dependencies installed through hermes tools, including Langfuse, are now preserved across managed-runtime rebuilds and repair paths. The Langfuse plugin also emits a one-time warning when its SDK is unavailable. The PR description has been updated with the full scope and validation details. |
c3bf4a2 to
1929492
Compare
What does this PR do?
Preserves user-activated optional Python dependencies when
hermes updatereplaces or repairs the managed runtime.The updater snapshots optional state before the old
site-packagesdisappears, then restores only that snapshot afterward:LAZY_DEPSpath.hermes toolsare restored from a static allowlist (faster-whisper, KittenTTS, Piper, DDGS, and Langfuse).Root cause: post-rebuild import detection has no historical state after replacement
site-packagesremoves a previously activated dependency.Security behavior:
security.allow_lazy_installs: falseremains an absolute lazy-install opt-out. Lazy restoration uses the guardedensure(..., prompt=False)path. Hermes Tools packages are resolved only through a static internal allowlist; arbitrary captured package names are ignored. Optional restore failures are reported without aborting the core update.Related Issue
Fixes #72924
Type of Change
Changes Made
hermes toolsand restore missing ones after the locked runtime sync.How to Test
hermes tools.hermes update.security.allow_lazy_installs: false; verify lazy restoration is skipped without a pip call.Local results:
git diff --check: passed.test_cmd_update.py: 15 passed; 6 could not enter the tested update path because a live local Hermes gateway held the Windows venv. The running gateway was intentionally left untouched.Checklist
Code
Documentation & Housekeeping