Skip to content

fix(memory): auto-install hindsight-all for local_embedded mode - #13101

Closed
LehaoLin wants to merge 1 commit into
NousResearch:mainfrom
LehaoLin:fix/hindsight-local-embedded-dependency
Closed

fix(memory): auto-install hindsight-all for local_embedded mode#13101
LehaoLin wants to merge 1 commit into
NousResearch:mainfrom
LehaoLin:fix/hindsight-local-embedded-dependency

Conversation

@LehaoLin

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a crash when users select local_embedded mode via hermes memory setup. The setup wizard tries to install hindsight-all but the install can silently fail because capture_output=True swallows errors. Since plugin.yaml only declares hindsight-client>=0.4.22 as a dependency, hermes startup validation passes even when hindsight-all is missing. At runtime, from hindsight import HindsightEmbedded crashes with ModuleNotFoundError: No module named 'hindsight'.

This patch adds a guard in HindsightMemoryProvider.initialize() that:

  1. Detects when mode is local_embedded
  2. Attempts to import hindsight — if it fails, auto-installs hindsight-all via uv pip install
  3. Raises a clear RuntimeError with manual install instructions if installation fails
  4. Does not surface errors as swallowed silent failures — errors are logged and propagated

This mirrors the existing auto-upgrade logic for hindsight-client already present in initialize() for cloud mode.

Related Issue

Fixes crash described in hindsight local_embedded setup flow.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • plugins/memory/hindsight/__init__.py: Added auto-install guard for hindsight-all in initialize() when mode is local_embedded
  • plugins/memory/hindsight/plugin.yaml: Added comment documenting the hindsight-all dependency for local_embedded mode

How to Test

  1. Configure hindsight with local_embedded mode but ensure hindsight-all is NOT installed
  2. Start a new hermes session with hindsight memory enabled
  3. Verify that the plugin auto-installs hindsight-all and logs the installation
  4. Verify that if installation fails, a clear RuntimeError is raised with manual instructions
  5. Test cloud mode still works without hindsight-all installed (no regression)

Checklist

  • My commit messages follow Conventional Commits
  • My PR contains only changes related to this fix
  • I have considered cross-platform impact (uses shutil.which("uv") which works on all platforms)
  • Updated relevant docstrings/comments — plugin.yaml comment added

When users select local_embedded mode via hermes memory setup, the
setup wizard attempts to install hindsight-all but the install can
silently fail (capture_output=True swallows errors). Since plugin.yaml
only declares hindsight-client>=0.4.22 as a dependency, hermes startup
validation passes even when hindsight-all is missing. At runtime,
from hindsight import HindsightEmbedded crashes with ModuleNotFoundError.

This patch adds a check in initialize() that detects local_embedded
mode and auto-installs hindsight-all if the hindsight package is not
importable, raising a clear RuntimeError if installation fails. This
mirrors the existing auto-upgrade logic for hindsight-client in cloud
mode. plugin.yaml is also updated with a comment noting the dependency.
@LehaoLin LehaoLin closed this Apr 20, 2026
@LehaoLin LehaoLin reopened this Apr 20, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Apr 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with #7741 (fail-loudly approach) — both address #7718. This PR auto-installs instead of just erroring.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with #7741 (fail-loudly approach) — both address #7718. This PR auto-installs instead of just erroring.

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do it already when you do the hermes memory setup so I don't see why we need it twice

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the local-embedded dependency path. Current main now prevents the PR-era repeated missing-module failure by probing the local runtime and disabling it with a warning when unavailable (plugins/memory/hindsight/__init__.py:1269-1280), but it still does not self-heal manually configured installations.

Problems

  • The added guard checks only import hindsight. Current runtime use requires both hindsight and hindsight_embed.daemon_embed_manager (plugins/memory/hindsight/__init__.py:127-140), so a partial installation can bypass the installer and still be disabled at initialization.
  • The PR diff adds no regression tests for install success/failure or the partial-runtime case.
  • Its manual command omits the active interpreter target, unlike the existing setup guidance at plugins/memory/hindsight/__init__.py:828-839.

Suggested changes

  • Rework this around the full runtime probe, auto-installing only for a verified missing dependency and preserving the current fail-disabled behavior for incompatible runtimes.
  • Re-probe after installation and add focused tests for recovery, install failure, and no effect on cloud/local_external modes.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/memory Memory subsystem: store, providers, sync, background reviews area/install-update Installer, updater, packaging, wheels, doctor labels Jul 12, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #85494 — the local_embedded missing-dependency issue (#7718) is addressed with an actionable install hint surfaced through the unavailable_reason() hook. Thanks for the alternative approach!

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/memory Memory subsystem: store, providers, sync, background reviews comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

5 participants