Under multiplex_profiles the Hindsight provider's writer, daemon-start and
prefetch threads were spawned as bare threading.Thread, so they started with
an empty contextvars Context: no profile secret scope and no HERMES_HOME
override. get_secret() fails closed there, so the local_embedded daemon never
booted and every retain raised UnscopedSecretError, even though the spawning
thread (initialize()/sync_turn() inside the gateway's copy_context'd turn) had
the scope all along.
Spawn each thread with contextvars.copy_context().run so the child inherits
the spawner's scope + home override. No environ fallback, no re-parsed .env.
The shared hindsight-loop thread needs no wrap: coroutines submitted via
run_coroutine_threadsafe already run in the submitter's context per call.
Fixes#92608Fixes#94933
Co-authored-by: KIAgent01 <297567825+KIAgent01@users.noreply.github.com>
Co-authored-by: Parker Fawcett <259203091+Parker-Fawcett@users.noreply.github.com>
Remove the implicit hermes peer and the peer question from new connection setup. Preserve explicit peer settings and keep memory paths consistent with the captured client identity.
Add setup, configuration, request, recall, and session regression tests, plus upgrade guidance.
hermes_cli/memory_setup.py::_write_env_vars() wrote provider-controlled
.env entries with a direct Path.write_text() + post-hoc chmod, bypassing
the denylist/regex/CRLF-stripping/atomic-replace validation that
hermes_cli/config.py::save_env_value() already provides for every other
.env writer in the codebase. A malicious or buggy memory-provider plugin
declaring a crafted env-var name/value in its setup schema could inject
arbitrary lines into .env.
Routes memory-provider env writes through save_env_value(), and fixes a
regression this surfaced in plugins/memory/supermemory/__init__.py::
post_setup(), which called the old two-parameter _write_env_vars(env_path,
values) signature — restores the caller via context-local
hermes_constants.set_hermes_home_override()/reset_hermes_home_override()
instead of a removed env_path parameter, so explicit HERMES_HOME overrides
during setup still resolve correctly.
Adds test_env_file_created_with_secure_permissions, guarded on Windows
(POSIX mode bits aren't enforced there, mirroring the existing skip in
test_openviking_provider.py / test_supermemory_provider.py) since
save_env_value's atomic-replace path creates the temp file at 0o600 before
writing content, closing the TOCTOU window the old direct-write + chmod
implementation had.
Anonymous probes (_anonymous_json) are designed to probe server identity
before disclosing credentials. Sending the Hermes version on these probes
would fingerprint the exact version to an untrusted/MITM endpoint.
Keep User-Agent on authenticated requests (_headers) and multipart uploads
(_multipart_headers), which already send credentials.
- Key _user_space_cache on _conn_snapshot instead of client object identity,
so _new_client() results from the same connection share the cached user
(previously every on_memory_write triggered an uncached /api/v1/system/status
probe with a 30s default timeout)
- Thread a short timeout (0.05s) through the write-path identity probe
- Harden _tool_remember to snapshot the client before URI construction + POST,
matching the pattern already established in on_memory_write
- Remove dead instance method _user_scoped_uri (zero callers; all call sites
use the module-level function directly)
Co-authored-by: ehz0ah <haozhe4547@gmail.com>
Review follow-up to the viking://~ migration: the ~ home alias only
expands for USER/ADMIN roles. The DEFAULT dev auth mode (no
server.auth_mode, no root_api_key) resolves every request as ROOT,
which bypasses current-user expansion — the canonical parser rejects
viking://~ with 400 'Home alias URI is not canonical' (verified on a
live 0.4.16 server). A deployment upgrading to 0.4.16 with an
untouched ov.conf is in dev mode, so the ~ spelling would break
exactly the way the old uid-less one will.
Mirror the upstream first-party plugin pattern instead: resolve the
user space client-side from /api/v1/system/status (result.user,
'default' fallback) and emit explicit-uid
viking://user/<user>/memories/... URIs, which are canonical under
every auth mode (dev/ROOT, trusted/USER, api-key) and every server
version. viking://~/... input typed by the user keeps passing through
untouched. (#91995)
Upstream OpenViking removed the uid-less viking://user/<segment>
shorthand (#4196, merged 2026-08-21): reserved segments like memories
and peers no longer expand to the caller's space and the server
rejects them with HTTP 400 (NamespaceShapeError). First-party clients
were migrated to viking://~ in the same change; the Hermes plugin was
not (#91995).
Migrate every URI the plugin constructs — the profile/preferences/
entities session-start reads, the _build_memory_uri memory-mirroring
write path, and the tool-schema example — to viking://~/... README
uid-less references updated to match; canonical user-scoped forms
(viking://user/default/...) are unchanged. The ~ alias requires
OpenViking server >= 0.4.16 (#4167).
Adds an optional occurred_at (ISO-8601 date/datetime) parameter to the
hindsight_retain tool schema, threaded into the retain item's timestamp
field. When absent, the item timestamp defaults to the configured event
clock (base from PR #82928 by @ragingbulld, authorship preserved) so the
Hindsight server can resolve relative time phrases; previously no item
timestamp was ever sent and temporal memories landed with null
occurred_start/occurred_end.
Fixes#93568. Salvages #82928.
Use Hermes timezone-aware timestamps for retained events and turn messages. Pass the public timestamp field supported by hindsight-client 0.6.1 and cover the final serialized request field.
Follow-up to the #88347 salvage: after release_all_under() force-closes a
profile's shared connection, a store re-created on the same path registers
a fresh entry under the same key. A stale holder that later calls close()
would pop that fresh entry (its refs were transferred nowhere), letting a
third store open a second connection to the same database — exactly the
multi-writer contention the shared registry exists to prevent. close()
now evicts the registry entry only when it is still its own.
The desktop's main serve process opens memory_store.db for every known
profile and nothing closed those connections before delete_profile's
rmtree — on Windows the open SQLite handles make the removal fail with
WinError 32 for both the CLI and the DELETE /api/profiles/<name> route
(#88347). POSIX unlinking of open files hid the same leak.
MemoryStore.close() is refcount-driven, so a live holder keeps the
handle forever; add MemoryStore.release_all_under(directory) to
force-close every shared connection under a directory, and call it in
delete_profile after stopping the profile backends. Inside serve the
handles live in that very process and get released; from the CLI it is
a no-op.
Fixes#88347
f1ea4a56c ("cover the remaining setup-time .env reads with utf-8-sig",
following 75afc47ba 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 #30281 / #60587, so it is left alone here.
(cherry picked from commit 175c6852c2c255b3219575b5de0b1b70f1f0efcb)
SessionDB could leave native SQLite handles open when construction failed
partway through schema/pragma/FTS/repair/lock/interrupt handling. Other
short-lived callers (MCP reads/polling, session search, reactions, trace
upload, insights, shutdown recovery) opened temporary SessionDB handles
without a complete ownership boundary. API-server profile caches and
RetainDB shutdown had similar late-close races. Under sustained load this
exhausted file descriptors (EMFILE).
- Close partially initialized SessionDB connections on every constructor
exception path via a finally block guarded by an initialization-complete
flag.
- Close temporary/cross-profile SessionDB handles in finally blocks across
CLI, MCP, search, trace, reactions, insights, and recovery paths.
- Add API-server per-profile cache ownership and disconnect cleanup.
- Make RetainDB writer-queue shutdown exception-safe: track connections per
thread, close on worker exit, reject new enqueues after shutdown starts,
and sweep any connections left by short-lived threads.
- Add regression coverage for constructor failures, worker-thread readers,
API disconnect failures, shutdown recovery, RetainDB late enqueue, and
foreign-loop async clients.
Salvage notes: the original PR's per-thread WAL-reader ownership changes
were superseded by main's read-connection pool (permits + checkout/return);
its cron timeout-abandon fix is credited separately to #72822's earlier
identical fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds on the three salvaged commits: adds the sources and integration points
they leave out, so a pip-installed memory provider is not a second-class
citizen next to a directory install.
Discovery
- Project-local providers (./.hermes/plugins/<name>/), gated on
HERMES_ENABLE_PROJECT_PLUGINS exactly as PluginManager gates its own project
scan. Completes the four sources CONTRIBUTING.md and AGENTS.md already
promised; memory was the only discovery system missing two of them.
- find_provider_dir() now resolves a package entry point to its directory.
This is load-bearing: config_schema.py (the dashboard panel) and cli.py (the
`hermes <provider>` subcommands) are read from disk rather than imported, so
without a directory a pip-installed provider silently lost both.
- list_memory_provider_names() includes entry-point providers, so they appear
in the dashboard's memory.provider dropdown.
Resolution stays import-free. hermes_cli.plugins.resolve_module_origin() is
extracted from _resolve_module_source() (added by the salvaged #76567) and
shared, so discovery walks a module's file layout instead of importing it.
find_provider_dir() is called from the dashboard and from argparse setup, long
before the operator has chosen a provider — importing every installed candidate
would execute third-party code on the strength of a package being present.
A test asserts the resolution leaves no side effects and no sys.modules entry.
Registration
- PluginContext gains register_memory_provider(). Memory was the only provider
category without one; context engine, image gen, video gen, web search,
browser, TTS, transcription, secret source, dashboard auth and platform all
have one.
- _ProviderCollector delegates unknown register_* calls to a real
PluginContext instead of carrying three hand-written no-ops. It silently
dropped register_tool/register_hook, and had no register_auxiliary_task at
all — despite PluginContext.register_auxiliary_task documenting a memory
provider (hindsight's pre-retain dedup) as its worked example. It can no
longer drift behind PluginContext.
- A raise after register_memory_provider() no longer costs the provider. The
loader caught it into a debug log, discarded the registered instance, and
fell through to "instantiate any MemoryProvider subclass" — returning a
different, unconfigured provider. A silent downgrade that looked like
success, and the exact outcome of calling register_auxiliary_task.
Activation is unchanged: still gated on memory.provider naming the plugin, and
covered by a test so the real PluginContext cannot start requiring
plugins.enabled — that would break every existing user-installed provider.
Verified end to end against a real third-party provider (kainappsinc/elephant)
installed by pip alone, with no directory copy: it appears in the dropdown,
resolves its directory, loads with its tools, and renders its dashboard panel.
Closes#40101.
on_memory_write spawns a fire-and-forget daemon thread that was never
stored on self, so shutdown() couldn't join it — the exact problem the
PR fixes for the async writer thread. Store as self._memwrite_thread
and include it in the shutdown join loop.
Review follow-up for salvaged PR #83500.
The previous gate compared session.user_peer_id against a fresh
_resolve_user_peer_id() call on the same manager. Both values come from
the same resolver with the same inputs, so a non-owner triggering a new
session in a shared channel passed the check and received the owner's
MEMORY.md/USER.md under their peer.
The owner is now a config fact: _declared_owner_peer_id() returns the
sanitized peerName, and migration runs only when the session's user peer
is that peer. Without a declared peerName, migration runs only when no
runtime gateway identity is present (the single-operator CLI path).
Aliases still work: a platform ID mapped onto peerName resolves to the
owner peer before the comparison.
Tests now derive each session's user peer from the real resolver instead
of hand-picking mismatched ids, so the non-owner test fails against the
old gate.
The owner gate from #82038 compared against config.peer_name directly,
which is None for most single-user setups — sanitizing None would raise
and the gate never accounted for pinned/runtime/aliased identities.
Resolve the owner the same way sessions do, and add the non-owner skip
regression test the original PR shipped without.
Co-authored-by: menhguin <menhguin@users.noreply.github.com>
migrate_memory_files() uploads USER.md/MEMORY.md with peer=user_peer — the
session's runtime user. In shared channels, a non-owner's new thread uploads
the owner's full profile under the NON-OWNER's peer; Honcho's deriver then
attributes the owner's psychometrics/medical/biography to that person. This
was the root contamination vector (55/70 contaminated sessions carried the
payload). Skip migration unless the session user is the configured owner.
SOUL.md unaffected (uploads under assistant peer).
Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
sync_turn called manager._flush_session() directly, which flushes
synchronously every turn no matter what writeFrequency says — the
"async", "session", and every-N-turns modes were dead configuration
on the main turn path. Route through save(), the dispatcher that
actually implements those modes.
Same bug class reported in #19650 (starship-s) and #72708 (Diaspar4u);
this takes the minimal one-line routing fix without their broader
lifecycle refactors.
Co-authored-by: starship-s <45587122+starship-s@users.noreply.github.com>
Provider shutdown() only called manager.flush_all(), which drains the
queue but never joins the async-writer thread — manager.shutdown()
exists and nothing called it. The writer thread could still be blocked
in httpx I/O at interpreter exit (the #37632 crash class). Now
shutdown() calls manager.shutdown() (flush + join) when persistence is
enabled, and a new manager.stop_async_writer() (join only, no flush)
when saveMessages is false, so containment and clean teardown compose.
The containment commit skipped the whole turn when either side was
empty, which would drop a real user message on interrupted or
tool-only turns. Keep the guard for fully-empty turns only and skip
empty sides individually inside the sync loop.
Salvages #67559 — original gated sync_turn/on_memory_write/on_session_end but missed shutdown(), whose flush_all() still persisted on exit. hermes-sweeper review (salvageability=high) flagged this as the one gap.
Guard sits after the worker-thread joins, not at the top: cleanup is independent of persistence, and a top-of-method return would leak _prefetch_thread/_sync_thread. Adds TestShutdown and clarifies the saveMessages=false README row.
Credit @Matroskin86 (original PR author).
