Skip to content

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

Merged
kshitijk4poor merged 5 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/openviking-explicit-user-uris
Aug 25, 2026
Merged

kshitijk4poor merged 5 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/openviking-explicit-user-uris

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Summary

Replaces uid-less viking://user/memories/... URIs (which OpenViking main now rejects with HTTP 400) with explicit-uid viking://user/{user}/memories/... URIs resolved client-side from /api/v1/system/status, mirroring the upstream first-party plugin pattern.

Why

OpenViking removed the uid-less viking://user/<reserved-segment> shorthand in upstream commit #4196. The Hermes plugin still emits that form for session-start reads and memory writes, which will 400 on the next OpenViking release. The viking://~ alias only expands for USER/ADMIN roles, not the default dev/ROOT mode — so explicit-uid URIs are the only spelling that works under every auth mode and server version.

Changes

  • plugins/memory/openviking/__init__.py:
    • Add _resolve_user_space() — probes /api/v1/system/status for the server-asserted current user
    • Add _user_space() — caches the resolved user keyed on the connection snapshot (_conn_snapshot), so all clients from the same connection share the cache
    • Replace hardcoded _PROFILE_URI/_PREFERENCES_URI/_ENTITIES_URI constants with suffix-based URI construction via _user_scoped_uri()
    • Thread the resolved user through session-start reads, memory block assembly, and memory write URI construction
    • Harden on_memory_write to snapshot the client before identity resolution + URI construction + POST
    • Harden _tool_remember to snapshot the client (matching on_memory_write)
    • Thread a short timeout through the write-path identity probe
    • Remove dead instance method _user_scoped_uri (zero callers)
  • plugins/memory/openviking/README.md: Updated URI documentation
  • Tests: 115 tests covering user resolution, cache binding, concurrent probe safety, transient failure retry, and connection-scoped cache invalidation

Salvage note

Includes commits from #91998 by @liuhao1024 (migrate to viking://~, then explicit-uid URIs) and #93985 by @ehz0ah (connection-scoped cache, operation consistency). Follow-up commit fixes cache keying, adds write-path timeout, hardens _tool_remember, and removes dead code.

Closes #91995
Closes #93985
Closes #91998

liuhao1024 and others added 5 commits August 25, 2026 12:12
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).
…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)
…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>
@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 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #91995 and the competing URI repairs #91998 and #93985. This active salvage uses server-resolved explicit user IDs rather than the home-alias approach.

@kshitijk4poor
kshitijk4poor merged commit b0cf259 into NousResearch:main Aug 25, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

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