Skip to content

fix(openviking): harden local server and .env handling - #87604

Merged
kshitijk4poor merged 4 commits into
NousResearch:mainfrom
ehz0ah:fix/openviking-env-reliability
Aug 17, 2026
Merged

kshitijk4poor merged 4 commits into
NousResearch:mainfrom
ehz0ah:fix/openviking-env-reliability

Conversation

@ehz0ah

@ehz0ah ehz0ah commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR consolidates #78219 and #78940. It fixes two OpenViking setup and startup failures.

First, the Hermes Desktop backend can include the Hermes virtual-environment packages in PYTHONPATH. An auto-started openviking-server inherited this path and could import those packages instead of its own. On Windows, loaded .pyd files could lock that environment and block hermes update.

Second, the OpenViking .env writer used strict UTF-8. A UTF-8 BOM prevented the first key from matching, which could leave a stale duplicate credential. A non-UTF-8 byte could stop setup. The new read path removes a BOM and preserves undecodable bytes when it updates another value.

The original authorship from #78219 and #78940 is preserved. The final follow-up commit changes the original replacement-character behavior so unrelated bytes remain unchanged.

Related Issue

Fixes #78153

Consolidates #78219 and #78940.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Remove PYTHONPATH from the environment of an auto-started OpenViking server. Preserve all other environment variables.
  • Read the Hermes .env file with utf-8-sig so a leading BOM does not change the first key.
  • Use surrogateescape on read and write so unrelated non-UTF-8 bytes remain unchanged.
  • Add regression tests for child environment isolation, BOM handling, byte preservation, and normal UTF-8 updates.

How to Test

  1. Run the focused OpenViking tests:

    HERMES_TEST_FILE_RETRIES=0 scripts/run_tests.sh tests/openviking_plugin/ tests/plugins/memory/test_openviking_endpoint_always_blocked.py tests/plugins/memory/test_openviking_provider.py tests/plugins/memory/test_openviking_shutdown.py -q
  2. Run Ruff and the Windows footgun scan:

    ruff check .
    python scripts/check-windows-footguns.py --all
  3. Confirm that the focused result is 105 passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.6 and targeted Windows compatibility checks

The complete macOS suite was started through scripts/run_tests.sh tests/ -q. It cannot be an all-green local gate in the standard development environment because some unrelated tests require optional Teams and WeCom dependencies, and one Linux abstract-socket test runs on macOS. The branch-specific OpenViking suite passes with retries disabled.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

OpenViking scope: 105 passed, 0 failed
Ruff: All checks passed
Windows footgun scan: No Windows footguns found (973 files scanned)

@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 area/config Config system, migrations, profiles labels Aug 16, 2026
kyssta-exe and others added 4 commits August 17, 2026 01:41
f1ea4a5 ("cover the remaining setup-time .env reads with utf-8-sig",
following 75afc47 for mem0/hindsight) swept this class; openviking's
_write_env_vars was missed and still reads with strict utf-8.

It copies every existing line through on each update, so the read decides
whether a credential update lands:

  BOM'd .env  -> the first key never matches, so the old line survives and
                 the new value is appended as a duplicate. .env loaders keep
                 the first occurrence, so the update silently does nothing.
  cp1252 .env -> UnicodeDecodeError aborts setup outright.

Read exactly like the canonical hermes_cli/config.py save_env_value
(utf-8-sig + errors="replace"). A plain UTF-8 file rewrites byte-identically.

Scope: hermes_cli/memory_setup.py has the same read but is already the
subject of NousResearch#30281 / NousResearch#60587, so it is left alone here.

(cherry picked from commit 175c685)
Clarify that the Desktop backend can add Hermes venv packages to PYTHONPATH and that current .env loaders use the last duplicate value.
@ehz0ah
ehz0ah force-pushed the fix/openviking-env-reliability branch from a9d1741 to 2854fab Compare August 16, 2026 17:45
@ehz0ah

ehz0ah commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Updated after the accuracy review:

  • Rebasing now uses current upstream main (06b914110).
  • The PYTHONPATH comment and PR body now state the verified Desktop behavior: the backend can include the Hermes virtual-environment packages. Fixes #78153 remains because this is the reported process path.
  • The duplicate-key comment now states the actual rule: Hermes and python-dotenv use the last occurrence. The test still requires one unambiguous stored value.
  • The original functional commits remain unchanged. The explanation corrections are in a separate maintainer-authored commit.
  • The general hermes_cli/config.py .env writer remains outside this PR and should be handled separately.

Validation on the rebased branch:

  • OpenViking scope: 105 passed, 0 failed
  • Ruff: passed
  • Windows footgun scan: passed
  • git diff --check: passed

@ehz0ah
ehz0ah marked this pull request as ready for review August 17, 2026 06:24
@kshitijk4poor
kshitijk4poor merged commit b202293 into NousResearch:main Aug 17, 2026
45 checks passed
@ehz0ah
ehz0ah deleted the fix/openviking-env-reliability branch August 17, 2026 07:07
bobaba76 pushed a commit to bobaba76/hermes-agent that referenced this pull request Aug 27, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles 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.

[Bug]: OpenViking autostart inherits PYTHONPATH, locks Hermes venv DLLs and blocks updates

5 participants