Skip to content

fix(memory): extend embedded Hindsight install timeout - #85409

Open
neilinger wants to merge 1 commit into
NousResearch:mainfrom
neilinger:fix/hindsight-install-timeout
Open

fix(memory): extend embedded Hindsight install timeout#85409
neilinger wants to merge 1 commit into
NousResearch:mainfrom
neilinger:fix/hindsight-install-timeout

Conversation

@neilinger

Copy link
Copy Markdown

What

Give embedded Hindsight package installs a 600-second budget while keeping the existing 120-second timeout for lightweight memory-provider SDKs.

This applies consistently to:

Why

Hermes already treats Hindsight as a supported shipped memory provider. For local_embedded, setup installs the full runtime and hermes update restores it after rebuilding the managed venv.

That supported repair path currently passes the full Hindsight stack to install_specs(..., timeout=120). A real managed update on Apple Silicon exceeded that generic budget, logged a warning, continued successfully, and left the configured embedded provider absent. The same forced dependency restoration completes when allowed a realistic multi-minute budget.

This follows up the provider-healing work merged in #72363. It also addresses the 120-second embedded install timeout reported during #7718 without moving Hindsight to an external service or adding private dependency pins.

Scope

  • Detect package names from versioned/extra-qualified specs.
  • Use 600 seconds only when the dependency set includes hindsight-all, hindsight-all-slim, or hindsight-api-slim.
  • Preserve 120 seconds for hindsight-client cloud/external mode and other providers.
  • Reuse the same timeout selector at both setup and update restoration call sites.

This PR deliberately does not address the stale hindsight-client==0.6.1 runtime pin. That known issue is already covered by #80390 and the broader existing fix in #80517.

Verification

  • Focused timeout/update tests: 5 passed.
  • Ruff: clean on all four changed files.
  • git diff --check: clean.
  • uv.lock: unchanged.
  • Sabotage test: restoring the historical fixed 120-second selector makes all three embedded-runtime regression assertions fail; restoring this fix makes them pass.
  • Live operational acceptance on the affected installation after dependency restoration: embedded daemon healthy, PostgreSQL connected, retain accepted, exact nonce recalled on first structured recall, disposable bank deleted, client closed.

Related

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Aug 13, 2026
@spfcraze

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

this PR legitimately extends it to 600 seconds for heavyweight embedded packages and should be kept.

Summary: On main, both install paths hardcode timeout=120 — hermes_cli/memory_setup.py:173 (setup and update restoration) and plugins/memory/hindsight/init.py:915 (post_setup). This diff adds a _dependency_install_timeout() selector that returns 600 only when the dependency set contains hindsight-all, hindsight-all-slim, or hindsight-api-slim, and 120 for everything else, wired into both call sites. It addresses the real embedded install timeout reported in #7718 and follows up the provider-healing work merged in #72363.

Problems: None blocking. The cloud hindsight-client path at plugins/memory/hindsight/init.py:1477 is correctly left at 120s, matching the PR's declared scope. The stale hindsight-client==0.6.1 runtime pin is intentionally out of scope (#80390, #80517). No competing open PR extends this install timeout.

Solution: Keep. Fixes a real reported symptom, is not implemented on main, and is not a duplicate.


Triage verdict: PASS — keep.

Evidence

no deterministic fact backs this claim — model belief, not executed or read evidence


Checked against 54ce7e5 — the PR head when this was written — and 8d4b1e4, main at the same moment.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(memory): extend embedded Hindsight install timeout

Reasonable, targeted change with good test coverage.

  • hermes_cli/memory_setup.py (_dependency_install_timeout): the name-extraction regex does not normalize underscores. PyPI treats hindsight_all and hindsight-all as the same package, but a spec written with _ would miss _EMBEDDED_HINDSIGHT_PACKAGES and stay on the 120s budget. Consider normalizing _- (after lowercasing) before the set comparison.
  • The (re.match(...) or [spec])[0] fallback: for a spec that does not start with an alnum/underscore (e.g. -e . or ./path), the whole spec string is compared — harmless today, but stripping to the bare package name would be more predictable.
  • plugins/memory/hindsight/__init__.py now imports _dependency_install_timeout, a private helper from hermes_cli.memory_setup, from a plugin. With two call sites this is effectively a shared API — consider promoting it to a public helper (e.g. in tools/lazy_deps) so the private-name contract is not relied on across module boundaries.
  • The 600s constant is asserted by tests as a behavior contract — fine.

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

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants