Skip to content

fix(memory): emit canonical OpenViking user URIs - #93985

Closed
ehz0ah wants to merge 4 commits into
NousResearch:mainfrom
ehz0ah:fix/openviking-explicit-user-uris
Closed

fix(memory): emit canonical OpenViking user URIs#93985
ehz0ah wants to merge 4 commits into
NousResearch:mainfrom
ehz0ah:fix/openviking-explicit-user-uris

Conversation

@ehz0ah

@ehz0ah ehz0ah commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

  • Replace removed uid-less OpenViking user paths with canonical viking://user/{user}/... URIs.
  • Resolve the authenticated user from /api/v1/system/status.
  • Resolve one user for each session-start recall within the existing recall deadline, then reuse it for all reads and labels.
  • Bind the resolved-user cache to the active OpenViking client, so a profile, endpoint, or credential reload cannot reuse another connection's user ID.
  • Keep each mirrored memory URI and write on one captured connection snapshot.
  • Do not cache an unverified fallback after a failed status probe; retry on a later operation.

This PR includes both commits from #91998 by @liuhao1024 with authorship preserved. The two follow-up commits add connection-scoped identity handling, operation consistency, and regression tests.

Why

OpenViking main removed viking://user/<reserved-segment> as a current-user shorthand. Hermes still emits that form for session-start reads and memory writes, which will return HTTP 400 after the next OpenViking release.

Using viking://~ alone is not sufficient because it is not expanded for ROOT requests in the default development mode. Explicit-user URIs work in ROOT, ADMIN, USER, trusted, and API-key configurations.

Hermes can reload OpenViking profiles and credentials without recreating the provider. The selected user, generated URI, request deadline, and client identity must therefore stay consistent for each recall or write operation.

Test plan

  • python -m pytest tests/openviking_plugin/test_openviking.py tests/plugins/memory/test_openviking_provider.py tests/plugins/memory/test_openviking_shutdown.py tests/plugins/memory/test_openviking_endpoint_always_blocked.py -q
    • 115 passed, 0 failed
  • The same 115 focused tests passed after a temporary merge with current upstream main (64a6f42c).
  • ruff check on the changed Python files
  • python scripts/check-windows-footguns.py --diff upstream/main
  • git diff --check
  • OpenViking source compatibility checks against:
    • v0.4.16 (499995f3)
    • upstream main (234a2d9f)

For both OpenViking revisions, the real namespace parser accepted the emitted profile, preferences, entities, and peer-memory URIs for ROOT, ADMIN, and USER roles. The real system status route returned the authenticated user ID used in those URIs.

Live OpenViking compatibility

On precursor head 17c05c5eb1, each OpenViking revision was started as an isolated server with valid Doubao model configuration and exercised through Hermes over HTTP:

OpenViking DEV TRUSTED API key
v0.4.16 (499995f3) pass pass pass
upstream main (234a2d9f) pass pass pass

Each of the six cases verified:

  • /api/v1/system/status and Hermes authenticated-user resolution
  • canonical explicit-user peer-memory URI generation
  • real /api/v1/content/write and /api/v1/content/read round trips
  • canonical user preference and entity paths used by session-start recall

The API-key cases configured an intentionally incorrect user in Hermes and confirmed that Hermes used the user identity asserted by OpenViking instead. The focused follow-up commit 7056a12766 does not change the HTTP or URI contracts; its operation-level timeout and connection-consistency paths are covered by the current 115-test suite.

Platforms

Tested on macOS. The change is platform-independent HTTP and URI handling.

Fixes #91995.

liuhao1024 and others added 3 commits August 24, 2026 23:24
@liuhao1024 @ehz0ah
fix(memory): migrate OpenViking URIs to the viking://~ home alias
5f14148 
Upstream OpenViking removed the uid-less viking://user/<segment>
shorthand (NousResearch#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 (NousResearch#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 (NousResearch#4167).
@liuhao1024 @ehz0ah
fix(memory): emit explicit-uid OpenViking URIs resolved from system s…
084c6b6 
…tatus

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. (NousResearch#91995)
@ehz0ah
fix(memory): scope OpenViking user cache to connection
17c05c5
@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/memory Memory subsystem: store, providers, sync, background reviews sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 24, 2026
@alt-glitch

alt-glitch commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: #91998 targets the same #91995 URI break with viking://~; this PR instead uses explicit server-resolved user URIs for ROOT/dev compatibility. These are competing mechanisms.

@ehz0ah
ehz0ah marked this pull request as ready for review August 25, 2026 02:25
@kshitijk4poor kshitijk4poor mentioned this pull request Aug 25, 2026
Merged
kshitijk4poor added a commit that referenced this pull request Aug 25, 2026
@kshitijk4poor @ehz0ah
fix: follow-up for salvaged PR #93985 — cache key, snapshot, dead code
b0cf259 
- 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>
@kshitijk4poor

kshitijk4poor commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Merged via #94515. Your commits cherry-picked with authorship preserved via rebase-merge. The salvage also includes a follow-up commit that keys the user-space cache on the connection snapshot (so _new_client() results share the cache), adds a short timeout to the write-path identity probe, and hardens _tool_remember to snapshot the client before URI construction.

Thanks for the thorough work on the connection-scoped cache and operation consistency!

@ehz0ah
ehz0ah deleted the fix/openviking-explicit-user-uris branch August 25, 2026 08:10
@andrexibiza andrexibiza mentioned this pull request Aug 25, 2026
Open
4 tasks
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
@kshitijk4poor @ehz0ah @and7777
fix: follow-up for salvaged PR NousResearch#93985 — cache key, snapsh…
740efe2 
…ot, dead code

- 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>
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
@melon-xf @ehz0ah
fix: follow-up for salvaged PR NousResearch#93985 — cache key, snapsh…
891df3b 
…ot, dead code

- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] OpenViking plugin uses uid-less viking://user/<segment> URIs that upstream removed in #4196 — will 400 on next OV release

4 participants

@ehz0ah @alt-glitch @kshitijk4poor @liuhao1024