Skip to content

fix(lazy-deps): skip ensurepip bootstrap on managed/package-manager installs - #69475

Draft
alt-glitch wants to merge 1 commit into
mainfrom
fix/lazy-deps-managed-skip
Draft

fix(lazy-deps): skip ensurepip bootstrap on managed/package-manager installs#69475
alt-glitch wants to merge 1 commit into
mainfrom
fix/lazy-deps-managed-skip

Conversation

@alt-glitch

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #48637 by @liuhao1024; fixes #48628.

On managed installs (NixOS et al.) the sealed venv is read-only, yet tools/lazy_deps.py ran the ensurepip→pip bootstrap on every launch — ~20s of CPU that can never persist anything. The PR's branch head was a merge commit and the raw pick conflicted with current main, so this lands the contributor's change rebuilt on today's code (single commit, their authorship) with one behavioral refinement over the original: managed mode skips only the ensurepip bootstrap rather than raising FeatureUnavailable outright — a truly missing optional feature still produces an actionable error naming the dependency group to enable (e.g. the nix .#messaging variant), and legitimate installs still reach the installer when lazy installs are allowed.

Test plan

  • New tests/tools/test_lazy_deps_managed.py: 6 passed (managed skip, error wording names dependency group, installer still reachable)
  • Wider lazy_deps selection + touched modules: 91 passed, 1 skipped (opt-in network test)
  • py_compile clean

…nstalls

On managed installs (NixOS, Homebrew, distro packages) the sealed venv is
read-only: the ensurepip -> pip bootstrap in tools/lazy_deps.py runs on
every launch, burns ~20s of CPU, and can never persist anything (#48628).
Skip only the bootstrap on managed installs; missing optional features
still raise an actionable error naming the nix dependency group /
package variant to install instead.
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

running on 21b7282

CI timings

CI timings · View job

Wall time 7m14s vs 9m59s (-27.5%). 10 job(s) slower, 8 faster, 2 unchanged.

  • Build&Test Docker image / build (amd64, ubuntu-latest, linux/amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=do...: -150.0s
  • Build&Test Docker image / build (arm64, ubuntu-24.04-arm, linux/arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope...: -44.0s
  • Python tests / Run tests slice 1/8: +37.0s
  • Python tests / Run tests slice 6/8: +18.0s
  • Python tests / Run tests slice 5/8: +17.0s

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Three PRs address #48628's repeated ensurepip-to-pip bootstrap on managed, read-only installations: #48637 adds an early managed-install guard, #69475 skips ensurepip while preserving the installer path and adds package-manager remediation, and merged #77500 skips the install ladder unless a writable durable target is configured.

Related pull requests

Duplicates

#48637, #69475, and #77500 target the same managed-install bootstrap failure; #48637 is superseded by #77500, while #69475 overlaps #77500 on the root fix but contains distinct remediation and detection work.

Suggested consolidation

Author action on #69475: split out the Nix dependency-group remediation and read-only /nix/store detection that are not present in merged #77500, then close the remaining managed-install bootstrap fix as a duplicate of #77500. Keep #48637 closed as superseded by #77500; this departs from its earlier keep_open contributor review because #77500 now implements the guarded fast-fail path and additionally preserves the writable durable-target deployment.

Complex graph

flowchart 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
    I48628(["issue #48628 (closed)"])
    subgraph Dup48637 ["PRs duplicating each other"]
        P48637["PR #48637 (closed)"]
        P69475["PR #69475 (open)"]
        P77500["PR #77500 (merged)"]
    end
    P69475 -->|best fix| I48628
    class I48628 closed
    class P48637 closed
    class P69475 open
    class P77500 merged
    class P48637 best
    class P69475 best
    class P77500 best
    class P69475 target
    click I48628 "https://github.com/NousResearch/hermes-agent/issues/48628"
    click P48637 "https://github.com/NousResearch/hermes-agent/pull/48637"
    click P69475 "https://github.com/NousResearch/hermes-agent/pull/69475"
    click P77500 "https://github.com/NousResearch/hermes-agent/pull/77500"
Loading

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 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 25 kB of PR diffs, 12 kB of issue/PR text, 2 kB of discussion (4 comments), 7 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets area/nix Nix flake, NixOS module, container packaging P2 Medium — degraded but workaround exists labels Aug 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator Author

Call-graph-assisted review (calldiff over tools/lazy_deps.py + diff and surrounding-code read). The core fix is right: on a managed/sealed prefix, ensurepip --upgrade can't write, so short-circuiting to an actionable remediation instead of a confusing bootstrap failure is the correct behavior, and the test matrix (managed skip, read-only-prefix detection, writable-unmanaged still bootstraps, Homebrew) is thorough.

🔴 Issues

  • tools/lazy_deps.py:810 — the managed skip only fires when the pip probe fails. On a Nix/managed venv where pip is present but the prefix is read-only, the probe succeeds, so the code falls through to Tier-2 pip install (:830), which fails with a raw permission error and no remediation attached. The whole point (name the dependency group / nixos-rebuild) is lost in that path. Consider checking _managed_install_system() before attempting the pip install (or wrapping the Tier-2 failure to attach remediation when managed), so the guidance surfaces regardless of whether pip happens to exist.
  • Same gap for Tier-1 uv (:786): on a managed prefix uv install fails and is swallowed to logger.debug; only the probe branch produces remediation. A single managed-prefix guard near the top of the pip ladder would cover uv, pip-present, and pip-absent uniformly.

🟡 Duplication / abstraction

  • _NIX_DEPENDENCY_GROUPS (:314-346) is a second dict keyed on the exact same feature names as LAZY_DEPS (provider.anthropic, tts.elevenlabs, platform.telegram, …). Adding a new lazy feature now silently requires touching two dicts, with no guard that the key sets stay aligned. Add a test asserting set(_NIX_DEPENDENCY_GROUPS) <= set(LAZY_DEPS) (and ideally that every group is a real [dependency-groups] name in pyproject) so a stale entry fails CI instead of shipping a wrong nixos-rebuild hint.
  • _managed_install_system (:548) partially re-implements NixOS detection. hermes_cli/config.py already owns the Nix-store path constant and the managed-system logic; the new prefix startswith("/nix/store") fallback duplicates that string. Reuse the config module's constant / a shared helper rather than re-hardcoding the store path.

✅ Looks good

  • remediation threaded cleanly through _InstallResultFeatureUnavailable._format(); the "no bare pip hint on managed" case is tested (:56).
  • The comment justifying a manual map (feature names intentionally differ from extras, e.g. tts.elevenlabstts-premium) is a fair reason not to auto-infer.

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor area/nix Nix flake, NixOS module, container packaging comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: lazy_deps runs the ensurepip→pip bootstrap on every launch for managed/read-only installs (NixOS) — ~20s CPU/start, never persists

4 participants